[U-Boot] mmc: sd_sdhi: add support for 32-bit data buffer

Message ID 20171113205159.6951-1-chris.brandt@renesas.com
State New
Delegated to: Jaehoon Chung
Headers show
Series
  • [U-Boot] mmc: sd_sdhi: add support for 32-bit data buffer
Related show

Commit Message

Chris Brandt Nov. 13, 2017, 8:51 p.m.
Some controllers have a 32-bit data buffer register and do not allow
any other access besides 32-bit read/write.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 arch/arm/mach-rmobile/include/mach/sh_sdhi.h |  1 +
 drivers/mmc/sh_sdhi.c                        | 21 ++++++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

Comments

Marek Vasut Nov. 14, 2017, 8:40 a.m. | #1
On 11/13/2017 09:51 PM, Chris Brandt wrote:
> Some controllers have a 32-bit data buffer register and do not allow
> any other access besides 32-bit read/write.
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

Can you switch to uniphier-sd instead ? I switched Gen3 away from SH
SDHI and the uniphier driver is so much better.

> ---
>  arch/arm/mach-rmobile/include/mach/sh_sdhi.h |  1 +
>  drivers/mmc/sh_sdhi.c                        | 21 ++++++++++++++++++++-
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-rmobile/include/mach/sh_sdhi.h b/arch/arm/mach-rmobile/include/mach/sh_sdhi.h
> index 00a135faa1..f2dbd851a5 100644
> --- a/arch/arm/mach-rmobile/include/mach/sh_sdhi.h
> +++ b/arch/arm/mach-rmobile/include/mach/sh_sdhi.h
> @@ -164,6 +164,7 @@
>  /* For quirk */
>  #define SH_SDHI_QUIRK_16BIT_BUF		BIT(0)
>  #define SH_SDHI_QUIRK_64BIT_BUF		BIT(1)
> +#define SH_SDHI_QUIRK_32BIT_BUF		BIT(2)
>  
>  int sh_sdhi_init(unsigned long addr, int ch, unsigned long quirks);
>  
> diff --git a/drivers/mmc/sh_sdhi.c b/drivers/mmc/sh_sdhi.c
> index eef061abb2..37879e6c56 100644
> --- a/drivers/mmc/sh_sdhi.c
> +++ b/drivers/mmc/sh_sdhi.c
> @@ -273,6 +273,7 @@ static int sh_sdhi_single_read(struct sh_sdhi_host *host, struct mmc_data *data)
>  	unsigned short blocksize, i;
>  	unsigned short *p = (unsigned short *)data->dest;
>  	u64 *q = (u64 *)data->dest;
> +	u32 *d = (u32 *)data->dest;
>  
>  	if ((unsigned long)p & 0x00000001) {
>  		debug(DRIVER_NAME": %s: The data pointer is unaligned.",
> @@ -296,6 +297,9 @@ static int sh_sdhi_single_read(struct sh_sdhi_host *host, struct mmc_data *data)
>  	if (host->quirks & SH_SDHI_QUIRK_64BIT_BUF)
>  		for (i = 0; i < blocksize / 8; i++)
>  			*q++ = sh_sdhi_readq(host, SDHI_BUF0);
> +	else if (host->quirks & SH_SDHI_QUIRK_32BIT_BUF)
> +		for (i = 0; i < blocksize / 4; i++)
> +			*d++ = sh_sdhi_readq(host, SDHI_BUF0);
>  	else
>  		for (i = 0; i < blocksize / 2; i++)
>  			*p++ = sh_sdhi_readw(host, SDHI_BUF0);
> @@ -314,6 +318,7 @@ static int sh_sdhi_multi_read(struct sh_sdhi_host *host, struct mmc_data *data)
>  	unsigned short blocksize, i, sec;
>  	unsigned short *p = (unsigned short *)data->dest;
>  	u64 *q = (u64 *)data->dest;
> +	u32 *d = (u32 *)data->dest;
>  
>  	if ((unsigned long)p & 0x00000001) {
>  		debug(DRIVER_NAME": %s: The data pointer is unaligned.",
> @@ -339,6 +344,9 @@ static int sh_sdhi_multi_read(struct sh_sdhi_host *host, struct mmc_data *data)
>  		if (host->quirks & SH_SDHI_QUIRK_64BIT_BUF)
>  			for (i = 0; i < blocksize / 8; i++)
>  				*q++ = sh_sdhi_readq(host, SDHI_BUF0);
> +		else if (host->quirks & SH_SDHI_QUIRK_32BIT_BUF)
> +			for (i = 0; i < blocksize / 4; i++)
> +				*d++ = sh_sdhi_readq(host, SDHI_BUF0);
>  		else
>  			for (i = 0; i < blocksize / 2; i++)
>  				*p++ = sh_sdhi_readw(host, SDHI_BUF0);
> @@ -354,6 +362,7 @@ static int sh_sdhi_single_write(struct sh_sdhi_host *host,
>  	unsigned short blocksize, i;
>  	const unsigned short *p = (const unsigned short *)data->src;
>  	const u64 *q = (const u64 *)data->src;
> +	const u32 *d = (const u32 *)data->src;
>  
>  	if ((unsigned long)p & 0x00000001) {
>  		debug(DRIVER_NAME": %s: The data pointer is unaligned.",
> @@ -381,6 +390,9 @@ static int sh_sdhi_single_write(struct sh_sdhi_host *host,
>  	if (host->quirks & SH_SDHI_QUIRK_64BIT_BUF)
>  		for (i = 0; i < blocksize / 8; i++)
>  			sh_sdhi_writeq(host, SDHI_BUF0, *q++);
> +	else if (host->quirks & SH_SDHI_QUIRK_32BIT_BUF)
> +		for (i = 0; i < blocksize / 4; i++)
> +			sh_sdhi_writeq(host, SDHI_BUF0, *d++);
>  	else
>  		for (i = 0; i < blocksize / 2; i++)
>  			sh_sdhi_writew(host, SDHI_BUF0, *p++);
> @@ -399,6 +411,7 @@ static int sh_sdhi_multi_write(struct sh_sdhi_host *host, struct mmc_data *data)
>  	unsigned short i, sec, blocksize;
>  	const unsigned short *p = (const unsigned short *)data->src;
>  	const u64 *q = (const u64 *)data->src;
> +	const u32 *d = (const u32 *)data->src;
>  
>  	debug("%s: blocks = %d, blocksize = %d\n",
>  	      __func__, data->blocks, data->blocksize);
> @@ -418,6 +431,9 @@ static int sh_sdhi_multi_write(struct sh_sdhi_host *host, struct mmc_data *data)
>  		if (host->quirks & SH_SDHI_QUIRK_64BIT_BUF)
>  			for (i = 0; i < blocksize / 8; i++)
>  				sh_sdhi_writeq(host, SDHI_BUF0, *q++);
> +		else if (host->quirks & SH_SDHI_QUIRK_32BIT_BUF)
> +			for (i = 0; i < blocksize / 4; i++)
> +				sh_sdhi_writeq(host, SDHI_BUF0, *d++);
>  		else
>  			for (i = 0; i < blocksize / 2; i++)
>  				sh_sdhi_writew(host, SDHI_BUF0, *p++);
> @@ -775,7 +791,8 @@ int sh_sdhi_init(unsigned long addr, int ch, unsigned long quirks)
>  
>  	if (host->quirks & SH_SDHI_QUIRK_64BIT_BUF)
>  		host->bus_shift = 2;
> -	else if (host->quirks & SH_SDHI_QUIRK_16BIT_BUF)
> +	else if (host->quirks & (SH_SDHI_QUIRK_16BIT_BUF ||
> +				 SH_SDHI_QUIRK_32BIT_BUF))
>  		host->bus_shift = 1;
>  
>  	return ret;
> @@ -854,6 +871,8 @@ static int sh_sdhi_dm_probe(struct udevice *dev)
>  
>  	if (host->quirks & SH_SDHI_QUIRK_64BIT_BUF)
>  		host->bus_shift = 2;
> +	else if (host->quirks & SH_SDHI_QUIRK_32BIT_BUF)
> +		host->bus_shift = 1;
>  	else if (host->quirks & SH_SDHI_QUIRK_16BIT_BUF)
>  		host->bus_shift = 1;
>  
>
Chris Brandt Nov. 14, 2017, 2:17 p.m. | #2
Hi Marek,

On Tuesday, November 14, 2017, Marek Vasut wrote:
> On 11/13/2017 09:51 PM, Chris Brandt wrote:

> > Some controllers have a 32-bit data buffer register and do not allow

> > any other access besides 32-bit read/write.

> >

> > Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

> 

> Can you switch to uniphier-sd instead ? I switched Gen3 away from SH

> SDHI and the uniphier driver is so much better.


Interesting.


However...

Functionally, the SDHI in the RZ/A1 is the same as R-Car.

But, there is 1 difference: The registers are spaced 16-bits apart where
the R-Car is 64-bit spaced apart.

Also, all the RZ/A1 registers are 16-bit, except for the data register 
(SD_BUF0) which is 32-bit. That's why I had to patch the upstream kernel 
driver specifically for RZ/A1.

The uniphier-sd is only setup for 32-bit or 64-bit spacing.

So, if I can get away with just this simple patch:

static u32 uniphier_sd_readl(struct uniphier_sd_priv *priv, unsigned int reg)
{
	if (priv->caps & UNIPHIER_SD_CAP_64BIT)
		return readl(priv->regbase + (reg << 1));
	if (priv->caps & UNIPHIER_SD_CAP_16BIT)
		return readl(priv->regbase + (reg >> 1));
	else
		return readl(priv->regbase + reg);
}

Then maybe I can use it.

I'll try it and see how it goes since I prefer to use the same drivers as R-Car.

Chris
Marek Vasut Nov. 15, 2017, 9:27 a.m. | #3
On 11/14/2017 03:17 PM, Chris Brandt wrote:
> Hi Marek,
> 
> On Tuesday, November 14, 2017, Marek Vasut wrote:
>> On 11/13/2017 09:51 PM, Chris Brandt wrote:
>>> Some controllers have a 32-bit data buffer register and do not allow
>>> any other access besides 32-bit read/write.
>>>
>>> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
>>
>> Can you switch to uniphier-sd instead ? I switched Gen3 away from SH
>> SDHI and the uniphier driver is so much better.
> 
> Interesting.
> 
> 
> However...
> 
> Functionally, the SDHI in the RZ/A1 is the same as R-Car.
> 
> But, there is 1 difference: The registers are spaced 16-bits apart where
> the R-Car is 64-bit spaced apart.
> 
> Also, all the RZ/A1 registers are 16-bit, except for the data register 
> (SD_BUF0) which is 32-bit. That's why I had to patch the upstream kernel 
> driver specifically for RZ/A1.
> 
> The uniphier-sd is only setup for 32-bit or 64-bit spacing.
> 
> So, if I can get away with just this simple patch:
> 
> static u32 uniphier_sd_readl(struct uniphier_sd_priv *priv, unsigned int reg)
> {
> 	if (priv->caps & UNIPHIER_SD_CAP_64BIT)
> 		return readl(priv->regbase + (reg << 1));
> 	if (priv->caps & UNIPHIER_SD_CAP_16BIT)
> 		return readl(priv->regbase + (reg >> 1));
> 	else
> 		return readl(priv->regbase + reg);
> }
> 
> Then maybe I can use it.
> 
> I'll try it and see how it goes since I prefer to use the same drivers as R-Car.

Awesome, thanks !

I'd prefer to decommission sh_sdhi driver sometimes soon too btw :)

> Chris
>

Patch

diff --git a/arch/arm/mach-rmobile/include/mach/sh_sdhi.h b/arch/arm/mach-rmobile/include/mach/sh_sdhi.h
index 00a135faa1..f2dbd851a5 100644
--- a/arch/arm/mach-rmobile/include/mach/sh_sdhi.h
+++ b/arch/arm/mach-rmobile/include/mach/sh_sdhi.h
@@ -164,6 +164,7 @@ 
 /* For quirk */
 #define SH_SDHI_QUIRK_16BIT_BUF		BIT(0)
 #define SH_SDHI_QUIRK_64BIT_BUF		BIT(1)
+#define SH_SDHI_QUIRK_32BIT_BUF		BIT(2)
 
 int sh_sdhi_init(unsigned long addr, int ch, unsigned long quirks);
 
diff --git a/drivers/mmc/sh_sdhi.c b/drivers/mmc/sh_sdhi.c
index eef061abb2..37879e6c56 100644
--- a/drivers/mmc/sh_sdhi.c
+++ b/drivers/mmc/sh_sdhi.c
@@ -273,6 +273,7 @@  static int sh_sdhi_single_read(struct sh_sdhi_host *host, struct mmc_data *data)
 	unsigned short blocksize, i;
 	unsigned short *p = (unsigned short *)data->dest;
 	u64 *q = (u64 *)data->dest;
+	u32 *d = (u32 *)data->dest;
 
 	if ((unsigned long)p & 0x00000001) {
 		debug(DRIVER_NAME": %s: The data pointer is unaligned.",
@@ -296,6 +297,9 @@  static int sh_sdhi_single_read(struct sh_sdhi_host *host, struct mmc_data *data)
 	if (host->quirks & SH_SDHI_QUIRK_64BIT_BUF)
 		for (i = 0; i < blocksize / 8; i++)
 			*q++ = sh_sdhi_readq(host, SDHI_BUF0);
+	else if (host->quirks & SH_SDHI_QUIRK_32BIT_BUF)
+		for (i = 0; i < blocksize / 4; i++)
+			*d++ = sh_sdhi_readq(host, SDHI_BUF0);
 	else
 		for (i = 0; i < blocksize / 2; i++)
 			*p++ = sh_sdhi_readw(host, SDHI_BUF0);
@@ -314,6 +318,7 @@  static int sh_sdhi_multi_read(struct sh_sdhi_host *host, struct mmc_data *data)
 	unsigned short blocksize, i, sec;
 	unsigned short *p = (unsigned short *)data->dest;
 	u64 *q = (u64 *)data->dest;
+	u32 *d = (u32 *)data->dest;
 
 	if ((unsigned long)p & 0x00000001) {
 		debug(DRIVER_NAME": %s: The data pointer is unaligned.",
@@ -339,6 +344,9 @@  static int sh_sdhi_multi_read(struct sh_sdhi_host *host, struct mmc_data *data)
 		if (host->quirks & SH_SDHI_QUIRK_64BIT_BUF)
 			for (i = 0; i < blocksize / 8; i++)
 				*q++ = sh_sdhi_readq(host, SDHI_BUF0);
+		else if (host->quirks & SH_SDHI_QUIRK_32BIT_BUF)
+			for (i = 0; i < blocksize / 4; i++)
+				*d++ = sh_sdhi_readq(host, SDHI_BUF0);
 		else
 			for (i = 0; i < blocksize / 2; i++)
 				*p++ = sh_sdhi_readw(host, SDHI_BUF0);
@@ -354,6 +362,7 @@  static int sh_sdhi_single_write(struct sh_sdhi_host *host,
 	unsigned short blocksize, i;
 	const unsigned short *p = (const unsigned short *)data->src;
 	const u64 *q = (const u64 *)data->src;
+	const u32 *d = (const u32 *)data->src;
 
 	if ((unsigned long)p & 0x00000001) {
 		debug(DRIVER_NAME": %s: The data pointer is unaligned.",
@@ -381,6 +390,9 @@  static int sh_sdhi_single_write(struct sh_sdhi_host *host,
 	if (host->quirks & SH_SDHI_QUIRK_64BIT_BUF)
 		for (i = 0; i < blocksize / 8; i++)
 			sh_sdhi_writeq(host, SDHI_BUF0, *q++);
+	else if (host->quirks & SH_SDHI_QUIRK_32BIT_BUF)
+		for (i = 0; i < blocksize / 4; i++)
+			sh_sdhi_writeq(host, SDHI_BUF0, *d++);
 	else
 		for (i = 0; i < blocksize / 2; i++)
 			sh_sdhi_writew(host, SDHI_BUF0, *p++);
@@ -399,6 +411,7 @@  static int sh_sdhi_multi_write(struct sh_sdhi_host *host, struct mmc_data *data)
 	unsigned short i, sec, blocksize;
 	const unsigned short *p = (const unsigned short *)data->src;
 	const u64 *q = (const u64 *)data->src;
+	const u32 *d = (const u32 *)data->src;
 
 	debug("%s: blocks = %d, blocksize = %d\n",
 	      __func__, data->blocks, data->blocksize);
@@ -418,6 +431,9 @@  static int sh_sdhi_multi_write(struct sh_sdhi_host *host, struct mmc_data *data)
 		if (host->quirks & SH_SDHI_QUIRK_64BIT_BUF)
 			for (i = 0; i < blocksize / 8; i++)
 				sh_sdhi_writeq(host, SDHI_BUF0, *q++);
+		else if (host->quirks & SH_SDHI_QUIRK_32BIT_BUF)
+			for (i = 0; i < blocksize / 4; i++)
+				sh_sdhi_writeq(host, SDHI_BUF0, *d++);
 		else
 			for (i = 0; i < blocksize / 2; i++)
 				sh_sdhi_writew(host, SDHI_BUF0, *p++);
@@ -775,7 +791,8 @@  int sh_sdhi_init(unsigned long addr, int ch, unsigned long quirks)
 
 	if (host->quirks & SH_SDHI_QUIRK_64BIT_BUF)
 		host->bus_shift = 2;
-	else if (host->quirks & SH_SDHI_QUIRK_16BIT_BUF)
+	else if (host->quirks & (SH_SDHI_QUIRK_16BIT_BUF ||
+				 SH_SDHI_QUIRK_32BIT_BUF))
 		host->bus_shift = 1;
 
 	return ret;
@@ -854,6 +871,8 @@  static int sh_sdhi_dm_probe(struct udevice *dev)
 
 	if (host->quirks & SH_SDHI_QUIRK_64BIT_BUF)
 		host->bus_shift = 2;
+	else if (host->quirks & SH_SDHI_QUIRK_32BIT_BUF)
+		host->bus_shift = 1;
 	else if (host->quirks & SH_SDHI_QUIRK_16BIT_BUF)
 		host->bus_shift = 1;