> -----Original Message-----
> From: Barry Song [mailto:21cn...@gmail.com]
> Sent: Thursday, August 21, 2014 6:09 PM
> To: Romain Izard
> Cc: linux-mmc@vger.kernel.org; Minda Chen; DL-SHA-WorkGroupLinux
> Subject: Re: [PATCH v2] mmc: sdhci-sirf: fix 8bit width enable by overwriting
> set_bus_width
>
> 2014-08-20 22:25 GMT+08:00 Romain Izard <romain.izard....@gmail.com>:
> > ["Followup-To:" header set to gmane.linux.kernel.mmc.] On 2014-08-19,
> > Barry Song <21cn...@gmail.com> wrote:
> >> From: Minda Chen <minda.c...@csr.com>
> >>
> >> 8bit-width enable bit of CSR MMC hosts is 3, while stardard hosts use
> >> bit 5. this patch fixes the functionality of 8bit transfer in CSR mmc
> >> controllers and improve performance for mmc0 a lot.
> >>
> >> Signed-off-by: Minda Chen <minda.c...@csr.com>
> >> Signed-off-by: Barry Song <baohua.s...@csr.com>
> >> ---
> >> -v2: check for host->version >= SDHCI_SPEC_300
> >>
> >
> > Hi Barry,
> >
> > Did you check the runtime behaviour of the device after this change ?
> >
> > From the SiRFprimaII/SiRFatlasVI data sheets, it appears that the
> > implementation of the SDHCI controller is a modified version of the
> > one described in the 1.0 specification, and not a normal 3.0 controller.
> > This explains why the independent development of the 8-bit wide
> > transfer bus does not use the same bit as the standard one.
> >
> > As a result, the description of the SPEC_VER field in the
> > SD_SLOT_INT_STATUS register indicates a read-only value of 0, which
> > corresponds to SDHCI_SPEC_100 in the "sdhci.h" header. On a real
> > SiRFatlasVI chip, the value is 0 as well.
> >
> > I believe the correct change would be to remove the control on the
> > version >= SDHCI_SPEC_300 in both 4-bit and 8-bit cases, instead of
> > adding it in both cases. There are no supported SiRF SoCs where the
> > 8-bit bus is not supported at the controller level.
> >
> > In my opinion, this should look like this:
> >
> > +static void sdhci_sirf_set_bus_width(struct sdhci_host *host, int
> > +width) {
> > + u8 ctrl;
> > +
> > + ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
> > + /* The 8-bit bus width configuration bit is not standard */
> > + ctrl &= ~(SDHCI_CTRL_4BITBUS | SDHCI_SIRF_8BITBUS);
> > + if (width == MMC_BUS_WIDTH_8) {
> > + ctrl |= SDHCI_SIRF_8BITBUS;
> > + } else if (width == MMC_BUS_WIDTH_4)
> > + ctrl |= SDHCI_CTRL_4BITBUS;
> > + }
> > + sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); }
> > +
>
> hi Romain,
> thanks very much! Minda will double-check the HW behaviour and confirm
> what you said.
[Barry Song]
Romain,
Minda's version is:
static void sdhci_sirf_set_bus_width(struct sdhci_host *host, int width)
{
u8 ctrl;
ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
ctrl &= ~(SDHCI_CTRL_4BITBUS | SDHCI_SIRF_8BITBUS);
if (width == MMC_BUS_WIDTH_8) {
/*
* CSR atlas7 and prima2 SD host version is not 3.0
* 8bit-width enable bit of CSR SD hosts is 3,
* while stardard hosts use bit 5
*/
ctrl |= SDHCI_SIRF_8BITBUS;
} else if (width == MMC_BUS_WIDTH_4)
ctrl |= SDHCI_CTRL_4BITBUS;
sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
}
It is basically same with you.
>
> >
> > You can use it verbatim with my signoff if you want.
> > Signed-off-by: Romain Izard <romain.izard....@gmail.com>
[Barry Song] do you mean a Reviewed-by?
-barry
Member of the CSR plc group of companies. CSR plc registered in England and
Wales, registered number 4187346, registered office Churchill House, Cambridge
Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our
technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube,
www.youtube.com/user/CSRplc, Facebook,
www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at
www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at
www.aptx.com.
