| Message ID | 20201014183212.475a789d@xhacker.debian |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: sdhci: Use Auto CMD Auto Select only when v4_mode is true | expand |
+ Chunyan On 14/10/20 1:32 pm, Jisheng Zhang wrote: > Auto CMD Auto Select can only be used when v4_mode is enabled. The SDHCI spec. doesn't seem to say that. AFAICS it refers only to v4.1 not v4 mode. > > Fixes: 427b6514d095 ("mmc: sdhci: Add Auto CMD Auto Select support") > Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com> > --- > drivers/mmc/host/sdhci.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 592a55a34b58..5e0ec5df4074 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -1386,7 +1386,8 @@ static inline void sdhci_auto_cmd_select(struct sdhci_host *host, > * Select' is recommended rather than use of 'Auto CMD12 > * Enable' or 'Auto CMD23 Enable'. > */ > - if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) { > + if (host->version >= SDHCI_SPEC_410 && host->v4_mode && > + (use_cmd12 || use_cmd23)) { > *mode |= SDHCI_TRNS_AUTO_SEL; > > ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); >
On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote: > > > + Chunyan > > On 14/10/20 1:32 pm, Jisheng Zhang wrote: > > Auto CMD Auto Select can only be used when v4_mode is enabled. > > The SDHCI spec. doesn't seem to say that. AFAICS it refers only to v4.1 not > v4 mode. 4.10 defines the "Auto CMD Auto Select" mode, v4 mode bit in SDHCI_HOST_CONTROL2 selects V3 compatible or V4 compatible mode, I think the v4 here includes v4.0, v4.1 and v4.2, so if we want to use the mode we have to ensure v4_mode is enabled. thanks > > > > > Fixes: 427b6514d095 ("mmc: sdhci: Add Auto CMD Auto Select support") > > Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com> > > --- > > drivers/mmc/host/sdhci.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > > index 592a55a34b58..5e0ec5df4074 100644 > > --- a/drivers/mmc/host/sdhci.c > > +++ b/drivers/mmc/host/sdhci.c > > @@ -1386,7 +1386,8 @@ static inline void sdhci_auto_cmd_select(struct sdhci_host *host, > > * Select' is recommended rather than use of 'Auto CMD12 > > * Enable' or 'Auto CMD23 Enable'. > > */ > > - if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) { > > + if (host->version >= SDHCI_SPEC_410 && host->v4_mode && > > + (use_cmd12 || use_cmd23)) { > > *mode |= SDHCI_TRNS_AUTO_SEL; > > > > ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > > >
On Thu, 15 Oct 2020 10:12:07 +0800 Jisheng Zhang wrote: > > On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote: > > > > > > > + Chunyan > > > > On 14/10/20 1:32 pm, Jisheng Zhang wrote: > > > Auto CMD Auto Select can only be used when v4_mode is enabled. > > > > The SDHCI spec. doesn't seem to say that. AFAICS it refers only to v4.1 not > > v4 mode. > > 4.10 defines the "Auto CMD Auto Select" mode, v4 mode bit in SDHCI_HOST_CONTROL2 > selects V3 compatible or V4 compatible mode, I think the v4 here includes > v4.0, v4.1 and v4.2, so if we want to use the mode we have to ensure v4_mode So if we want to use the "Auto CMD Auto Select" mode, we have to ensure v4 mode is enabled. > is enabled. > > thanks > > > > > > > > > Fixes: 427b6514d095 ("mmc: sdhci: Add Auto CMD Auto Select support") > > > Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com> > > > --- > > > drivers/mmc/host/sdhci.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > > > index 592a55a34b58..5e0ec5df4074 100644 > > > --- a/drivers/mmc/host/sdhci.c > > > +++ b/drivers/mmc/host/sdhci.c > > > @@ -1386,7 +1386,8 @@ static inline void sdhci_auto_cmd_select(struct sdhci_host *host, > > > * Select' is recommended rather than use of 'Auto CMD12 > > > * Enable' or 'Auto CMD23 Enable'. > > > */ > > > - if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) { > > > + if (host->version >= SDHCI_SPEC_410 && host->v4_mode && > > > + (use_cmd12 || use_cmd23)) { > > > *mode |= SDHCI_TRNS_AUTO_SEL; > > > > > > ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > > > > > >
On 15/10/20 5:38 am, Jisheng Zhang wrote: > On Thu, 15 Oct 2020 10:12:07 +0800 Jisheng Zhang wrote: > > >> >> On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote: >> >>> >>> >>> + Chunyan >>> >>> On 14/10/20 1:32 pm, Jisheng Zhang wrote: >>>> Auto CMD Auto Select can only be used when v4_mode is enabled. >>> >>> The SDHCI spec. doesn't seem to say that. AFAICS it refers only to v4.1 not >>> v4 mode. >> >> 4.10 defines the "Auto CMD Auto Select" mode, v4 mode bit in SDHCI_HOST_CONTROL2 >> selects V3 compatible or V4 compatible mode, I think the v4 here includes >> v4.0, v4.1 and v4.2, so if we want to use the mode we have to ensure v4_mode > > So if we want to use the "Auto CMD Auto Select" mode, we have to ensure v4 mode > is enabled. But that is not exactly what the SDHCI spec. says. It is quite explicit about which registers and bit fields are affected by "Host Version 4 Enable =1". So the question is whether this is standard or a quirk of your controller. > >> is enabled. >> >> thanks >> >>> >>>> >>>> Fixes: 427b6514d095 ("mmc: sdhci: Add Auto CMD Auto Select support") >>>> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com> >>>> --- >>>> drivers/mmc/host/sdhci.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >>>> index 592a55a34b58..5e0ec5df4074 100644 >>>> --- a/drivers/mmc/host/sdhci.c >>>> +++ b/drivers/mmc/host/sdhci.c >>>> @@ -1386,7 +1386,8 @@ static inline void sdhci_auto_cmd_select(struct sdhci_host *host, >>>> * Select' is recommended rather than use of 'Auto CMD12 >>>> * Enable' or 'Auto CMD23 Enable'. >>>> */ >>>> - if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) { >>>> + if (host->version >= SDHCI_SPEC_410 && host->v4_mode && >>>> + (use_cmd12 || use_cmd23)) { >>>> *mode |= SDHCI_TRNS_AUTO_SEL; >>>> >>>> ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); >>>> >>> >> >
On Thu, 15 Oct 2020 08:57:05 +0300 Adrian Hunter wrote: > > > On 15/10/20 5:38 am, Jisheng Zhang wrote: > > On Thu, 15 Oct 2020 10:12:07 +0800 Jisheng Zhang wrote: > > > > > >> > >> On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote: > >> > >>> > >>> > >>> + Chunyan > >>> > >>> On 14/10/20 1:32 pm, Jisheng Zhang wrote: > >>>> Auto CMD Auto Select can only be used when v4_mode is enabled. > >>> > >>> The SDHCI spec. doesn't seem to say that. AFAICS it refers only to v4.1 not > >>> v4 mode. > >> > >> 4.10 defines the "Auto CMD Auto Select" mode, v4 mode bit in SDHCI_HOST_CONTROL2 > >> selects V3 compatible or V4 compatible mode, I think the v4 here includes > >> v4.0, v4.1 and v4.2, so if we want to use the mode we have to ensure v4_mode > > > > So if we want to use the "Auto CMD Auto Select" mode, we have to ensure v4 mode > > is enabled. > > But that is not exactly what the SDHCI spec. says. It is quite explicit > about which registers and bit fields are affected by "Host Version 4 Enable =1". > Just my humble opinion, this is implied, my logic look like: Host Version 4 Enable == 0 => only V3 compatible mode \ => v4 mode is must for auto cmd auto select No "Auto CMD Auto Select" definition in v3 spec / > So the question is whether this is standard or a quirk of your controller. > other v4 controllers can do the same benchmark test after removing sdhci_enable_v4_mode() in the controller's probe. Thanks
On 15/10/20 9:24 am, Jisheng Zhang wrote: > On Thu, 15 Oct 2020 08:57:05 +0300 Adrian Hunter wrote: > >> >> >> On 15/10/20 5:38 am, Jisheng Zhang wrote: >>> On Thu, 15 Oct 2020 10:12:07 +0800 Jisheng Zhang wrote: >>> >>> >>>> >>>> On Wed, 14 Oct 2020 22:44:50 +0300 Adrian Hunter wrote: >>>> >>>>> >>>>> >>>>> + Chunyan >>>>> >>>>> On 14/10/20 1:32 pm, Jisheng Zhang wrote: >>>>>> Auto CMD Auto Select can only be used when v4_mode is enabled. >>>>> >>>>> The SDHCI spec. doesn't seem to say that. AFAICS it refers only to v4.1 not >>>>> v4 mode. >>>> >>>> 4.10 defines the "Auto CMD Auto Select" mode, v4 mode bit in SDHCI_HOST_CONTROL2 >>>> selects V3 compatible or V4 compatible mode, I think the v4 here includes >>>> v4.0, v4.1 and v4.2, so if we want to use the mode we have to ensure v4_mode >>> >>> So if we want to use the "Auto CMD Auto Select" mode, we have to ensure v4 mode >>> is enabled. >> >> But that is not exactly what the SDHCI spec. says. It is quite explicit >> about which registers and bit fields are affected by "Host Version 4 Enable =1". >> > > Just my humble opinion, this is implied, my logic look like: > > Host Version 4 Enable == 0 => only V3 compatible mode \ > => v4 mode is must for auto cmd auto select > No "Auto CMD Auto Select" definition in v3 spec / Ok, we will need the commit message to explain the performance degradation, and which driver / version / platform, and a comment in the code explaining we require Version 4 Mode for Auto CMD Auto Select because some controllers expect it that way.
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 592a55a34b58..5e0ec5df4074 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1386,7 +1386,8 @@ static inline void sdhci_auto_cmd_select(struct sdhci_host *host, * Select' is recommended rather than use of 'Auto CMD12 * Enable' or 'Auto CMD23 Enable'. */ - if (host->version >= SDHCI_SPEC_410 && (use_cmd12 || use_cmd23)) { + if (host->version >= SDHCI_SPEC_410 && host->v4_mode && + (use_cmd12 || use_cmd23)) { *mode |= SDHCI_TRNS_AUTO_SEL; ctrl2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
Auto CMD Auto Select can only be used when v4_mode is enabled. Fixes: 427b6514d095 ("mmc: sdhci: Add Auto CMD Auto Select support") Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com> --- drivers/mmc/host/sdhci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)