| Message ID | 20171113205159.6951-1-chris.brandt@renesas.com |
|---|---|
| State | Deferred |
| Delegated to: | Jaehoon Chung |
| Headers | show |
| Series | [U-Boot] mmc: sd_sdhi: add support for 32-bit data buffer | expand |
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; > >
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
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 >
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;
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(-)