| Message ID | 20221128133259.38305-3-adrian.hunter@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: sdhci: Fix voltage switch delay | expand |
On 28. 11. 2022. 14:32, Adrian Hunter wrote: > Avoid re-configuring UHS and preset settings when the settings have not > changed, irrespective of whether the clock is turning on. > > Tested-by: Haibo Chen <haibo.chen@nxp.com> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Hi, this is breaking sdhci-msm on IPQ8074 in next-20221130 for me and reverting it makes the eMMC work again. I get a lot of: [ 2.727287] mmc0: tuning execution failed: -5 [ 2.727323] mmc0: error -5 whilst initialising MMC card [ 3.846540] mmc0: tuning execution failed: -5 [ 3.846564] mmc0: error -5 whilst initialising MMC card [ 4.966517] mmc0: tuning execution failed: -5 [ 4.966539] mmc0: error -5 whilst initialising MMC card [ 6.096486] mmc0: tuning execution failed: -5 [ 6.096508] mmc0: error -5 whilst initialising MMC card [ 7.206431] mmc0: tuning execution failed: -5 [ 7.206454] mmc0: error -5 whilst initialising MMC card Regards, Robert > --- > drivers/mmc/host/sdhci.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 64750fbb0ac8..17e5ccf9a855 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2315,7 +2315,6 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > { > struct sdhci_host *host = mmc_priv(mmc); > bool reinit_uhs = host->reinit_uhs; > - bool turning_on_clk = false; > u8 ctrl; > > host->reinit_uhs = false; > @@ -2345,8 +2344,6 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > sdhci_enable_preset_value(host, false); > > if (!ios->clock || ios->clock != host->clock) { > - turning_on_clk = ios->clock && !host->clock; > - > host->ops->set_clock(host, ios->clock); > host->clock = ios->clock; > > @@ -2374,11 +2371,10 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > host->ops->set_bus_width(host, ios->bus_width); > > /* > - * Special case to avoid multiple clock changes during voltage > - * switching. > + * Avoid unnecessary changes. In particular, this avoids multiple clock > + * changes during voltage switching. > */ > if (!reinit_uhs && > - turning_on_clk && > host->timing == ios->timing && > host->version >= SDHCI_SPEC_300 && > !sdhci_presetable_values_change(host, ios))
On 30/11/22 13:54, Robert Marko wrote: > > On 28. 11. 2022. 14:32, Adrian Hunter wrote: >> Avoid re-configuring UHS and preset settings when the settings have not >> changed, irrespective of whether the clock is turning on. >> >> Tested-by: Haibo Chen <haibo.chen@nxp.com> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > > Hi, this is breaking sdhci-msm on IPQ8074 in next-20221130 for me > and reverting it makes the eMMC work again. > > I get a lot of: > > [ 2.727287] mmc0: tuning execution failed: -5 > [ 2.727323] mmc0: error -5 whilst initialising MMC card > [ 3.846540] mmc0: tuning execution failed: -5 > [ 3.846564] mmc0: error -5 whilst initialising MMC card > [ 4.966517] mmc0: tuning execution failed: -5 > [ 4.966539] mmc0: error -5 whilst initialising MMC card > [ 6.096486] mmc0: tuning execution failed: -5 > [ 6.096508] mmc0: error -5 whilst initialising MMC card > [ 7.206431] mmc0: tuning execution failed: -5 > [ 7.206454] mmc0: error -5 whilst initialising MMC card Thanks for the report! Are you able to debug this any more? What transfer mode is it? e.g. HS400? Can you enable debug messages and get more information?
On Wed, 30 Nov 2022 at 13:46, Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 30/11/22 13:54, Robert Marko wrote: > > > > On 28. 11. 2022. 14:32, Adrian Hunter wrote: > >> Avoid re-configuring UHS and preset settings when the settings have not > >> changed, irrespective of whether the clock is turning on. > >> > >> Tested-by: Haibo Chen <haibo.chen@nxp.com> > >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > > > > Hi, this is breaking sdhci-msm on IPQ8074 in next-20221130 for me > > and reverting it makes the eMMC work again. > > > > I get a lot of: > > > > [ 2.727287] mmc0: tuning execution failed: -5 > > [ 2.727323] mmc0: error -5 whilst initialising MMC card > > [ 3.846540] mmc0: tuning execution failed: -5 > > [ 3.846564] mmc0: error -5 whilst initialising MMC card > > [ 4.966517] mmc0: tuning execution failed: -5 > > [ 4.966539] mmc0: error -5 whilst initialising MMC card > > [ 6.096486] mmc0: tuning execution failed: -5 > > [ 6.096508] mmc0: error -5 whilst initialising MMC card > > [ 7.206431] mmc0: tuning execution failed: -5 > > [ 7.206454] mmc0: error -5 whilst initialising MMC card > > Thanks for the report! Are you able to debug this any more? > What transfer mode is it? e.g. HS400? Can you enable debug > messages and get more information? With some guidance yes, it's in HS200 as there is an issue with HS400 to HS200 switch on this SoC so I have HS400 disabled. With CONFIG_MMC_DEBUG and loglevel=8 I dont have any new messages. Regards, Robert >
On 30/11/22 15:00, Robert Marko wrote: > On Wed, 30 Nov 2022 at 13:46, Adrian Hunter <adrian.hunter@intel.com> wrote: >> >> On 30/11/22 13:54, Robert Marko wrote: >>> >>> On 28. 11. 2022. 14:32, Adrian Hunter wrote: >>>> Avoid re-configuring UHS and preset settings when the settings have not >>>> changed, irrespective of whether the clock is turning on. >>>> >>>> Tested-by: Haibo Chen <haibo.chen@nxp.com> >>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> >>> >>> Hi, this is breaking sdhci-msm on IPQ8074 in next-20221130 for me >>> and reverting it makes the eMMC work again. >>> >>> I get a lot of: >>> >>> [ 2.727287] mmc0: tuning execution failed: -5 >>> [ 2.727323] mmc0: error -5 whilst initialising MMC card >>> [ 3.846540] mmc0: tuning execution failed: -5 >>> [ 3.846564] mmc0: error -5 whilst initialising MMC card >>> [ 4.966517] mmc0: tuning execution failed: -5 >>> [ 4.966539] mmc0: error -5 whilst initialising MMC card >>> [ 6.096486] mmc0: tuning execution failed: -5 >>> [ 6.096508] mmc0: error -5 whilst initialising MMC card >>> [ 7.206431] mmc0: tuning execution failed: -5 >>> [ 7.206454] mmc0: error -5 whilst initialising MMC card >> >> Thanks for the report! Are you able to debug this any more? >> What transfer mode is it? e.g. HS400? Can you enable debug >> messages and get more information? > > With some guidance yes, it's in HS200 as there is an issue with HS400 > to HS200 switch on this SoC so I have HS400 disabled. > > With CONFIG_MMC_DEBUG and loglevel=8 I dont have any new > messages. You should get a lot more with: CONFIG_DYNAMIC_DEBUG=y and kernel commandline option: dyndbg="file drivers/mmc/core/* +p;file drivers/mmc/host/* +p"
On Wed, 30 Nov 2022 at 15:16, Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 30/11/22 15:00, Robert Marko wrote: > > On Wed, 30 Nov 2022 at 13:46, Adrian Hunter <adrian.hunter@intel.com> wrote: > >> > >> On 30/11/22 13:54, Robert Marko wrote: > >>> > >>> On 28. 11. 2022. 14:32, Adrian Hunter wrote: > >>>> Avoid re-configuring UHS and preset settings when the settings have not > >>>> changed, irrespective of whether the clock is turning on. > >>>> > >>>> Tested-by: Haibo Chen <haibo.chen@nxp.com> > >>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > >>> > >>> Hi, this is breaking sdhci-msm on IPQ8074 in next-20221130 for me > >>> and reverting it makes the eMMC work again. > >>> > >>> I get a lot of: > >>> > >>> [ 2.727287] mmc0: tuning execution failed: -5 > >>> [ 2.727323] mmc0: error -5 whilst initialising MMC card > >>> [ 3.846540] mmc0: tuning execution failed: -5 > >>> [ 3.846564] mmc0: error -5 whilst initialising MMC card > >>> [ 4.966517] mmc0: tuning execution failed: -5 > >>> [ 4.966539] mmc0: error -5 whilst initialising MMC card > >>> [ 6.096486] mmc0: tuning execution failed: -5 > >>> [ 6.096508] mmc0: error -5 whilst initialising MMC card > >>> [ 7.206431] mmc0: tuning execution failed: -5 > >>> [ 7.206454] mmc0: error -5 whilst initialising MMC card > >> > >> Thanks for the report! Are you able to debug this any more? > >> What transfer mode is it? e.g. HS400? Can you enable debug > >> messages and get more information? > > > > With some guidance yes, it's in HS200 as there is an issue with HS400 > > to HS200 switch on this SoC so I have HS400 disabled. > > > > With CONFIG_MMC_DEBUG and loglevel=8 I dont have any new > > messages. > > You should get a lot more with: > > CONFIG_DYNAMIC_DEBUG=y > > and kernel commandline option: > > dyndbg="file drivers/mmc/core/* +p;file drivers/mmc/host/* +p" Unfortunatelly not: [ 1.630829] mmc0: tuning execution failed: -5 [ 1.634255] mmc0: error -5 whilst initialising MMC card [ 1.693655] Run /init as init process Starting syslogd: OK Starting klogd: OK Running sysctl: OK Populating /dev using udev: [ 1.750807] udevd[123]: starting version 3.2.11 [ 2.813509] mmc0: tuning execution failed: -5 [ 2.813532] mmc0: error -5 whilst initialising MMC card [ 3.933817] mmc0: tuning execution failed: -5 [ 3.933845] mmc0: error -5 whilst initialising MMC card [ 5.053732] mmc0: tuning execution failed: -5 [ 5.053756] mmc0: error -5 whilst initialising MMC card [ 6.173933] mmc0: tuning execution failed: -5 [ 6.173960] mmc0: error -5 whilst initialising MMC card [ 7.303852] mmc0: tuning execution failed: -5 [ 7.303877] mmc0: error -5 whilst initialising MMC card [ 8.415565] mmc0: tuning execution failed: -5 [ 8.415591] mmc0: error -5 whilst initialising MMC card [ 9.539392] mmc0: tuning execution failed: -5 [ 9.539414] mmc0: error -5 whilst initialising MMC card [ 10.654012] mmc0: tuning execution failed: -5 [ 10.654038] mmc0: error -5 whilst initialising MMC card [ 11.774310] mmc0: tuning execution failed: -5 [ 11.774337] mmc0: error -5 whilst initialising MMC card [ 12.353608] random: crng init done [ 12.357231] udevd[124]: starting eudev-3.2.11 done Starting watchdog... Saving random seed: OK Starting network: OK Starting ntpd: OK Unsupported board: qnap,301w Starting dropbear sshd: OK Welcome to Buildroot buildroot login: [ 12.896218] mmc0: tuning execution failed: -5 [ 12.896243] mmc0: error -5 whilst initialising MMC card [ 14.016298] mmc0: tuning execution failed: -5 [ 14.016323] mmc0: error -5 whilst initialising MMC card [ 15.136251] mmc0: tuning execution failed: -5 [ 15.136276] mmc0: error -5 whilst initialising MMC card [ 16.256295] mmc0: tuning execution failed: -5 [ 16.256318] mmc0: error -5 whilst initialising MMC card [ 17.376286] mmc0: tuning execution failed: -5 [ 17.376310] mmc0: error -5 whilst initialising MMC card [ 18.496279] mmc0: tuning execution failed: -5 [ 18.496301] mmc0: error -5 whilst initialising MMC card [ 19.616306] mmc0: tuning execution failed: -5 [ 19.616331] mmc0: error -5 whilst initialising MMC card [ 20.736156] mmc0: tuning execution failed: -5 [ 20.736180] mmc0: error -5 whilst initialising MMC card [ 21.856224] mmc0: tuning execution failed: -5 [ 21.856249] mmc0: error -5 whilst initialising MMC card [ 22.976226] mmc0: tuning execution failed: -5 [ 22.976251] mmc0: error -5 whilst initialising MMC card [ 24.096153] mmc0: tuning execution failed: -5 [ 24.096179] mmc0: error -5 whilst initialising MMC card [ 25.216275] mmc0: tuning execution failed: -5 [ 25.216299] mmc0: error -5 whilst initialising MMC card [ 26.336265] mmc0: tuning execution failed: -5 [ 26.336289] mmc0: error -5 whilst initialising MMC card [ 27.456213] mmc0: tuning execution failed: -5 [ 27.456238] mmc0: error -5 whilst initialising MMC card [ 28.576362] mmc0: tuning execution failed: -5 [ 28.576388] mmc0: error -5 whilst initialising MMC card [ 29.696385] mmc0: tuning execution failed: -5 [ 29.696408] mmc0: error -5 whilst initialising MMC card [ 30.816245] mmc0: tuning execution failed: -5 [ 30.816271] mmc0: error -5 whilst initialising MMC card [ 31.936377] mmc0: tuning execution failed: -5 [ 31.936402] mmc0: error -5 whilst initialising MMC card [ 33.056207] mmc0: tuning execution failed: -5 [ 33.056232] mmc0: error -5 whilst initialising MMC card [ 34.176244] mmc0: tuning execution failed: -5 [ 34.176268] mmc0: error -5 whilst initialising MMC card [ 35.296349] mmc0: tuning execution failed: -5 [ 35.296374] mmc0: error -5 whilst initialising MMC card [ 36.416237] mmc0: tuning execution failed: -5 [ 36.416265] mmc0: error -5 whilst initialising MMC card [ 37.536187] mmc0: tuning execution failed: -5 [ 37.536213] mmc0: error -5 whilst initialising MMC card > >
On 11/30/22 09:24, Robert Marko wrote: > On Wed, 30 Nov 2022 at 15:16, Adrian Hunter <adrian.hunter@intel.com> wrote: >> >> On 30/11/22 15:00, Robert Marko wrote: >>> On Wed, 30 Nov 2022 at 13:46, Adrian Hunter <adrian.hunter@intel.com> wrote: >>>> >>>> On 30/11/22 13:54, Robert Marko wrote: >>>>> >>>>> On 28. 11. 2022. 14:32, Adrian Hunter wrote: >>>>>> Avoid re-configuring UHS and preset settings when the settings have not >>>>>> changed, irrespective of whether the clock is turning on. >>>>>> >>>>>> Tested-by: Haibo Chen <haibo.chen@nxp.com> >>>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> >>>>> >>>>> Hi, this is breaking sdhci-msm on IPQ8074 in next-20221130 for me >>>>> and reverting it makes the eMMC work again. >>>>> >>>>> I get a lot of: >>>>> >>>>> [ 2.727287] mmc0: tuning execution failed: -5 >>>>> [ 2.727323] mmc0: error -5 whilst initialising MMC card >>>>> [ 3.846540] mmc0: tuning execution failed: -5 >>>>> [ 3.846564] mmc0: error -5 whilst initialising MMC card >>>>> [ 4.966517] mmc0: tuning execution failed: -5 >>>>> [ 4.966539] mmc0: error -5 whilst initialising MMC card >>>>> [ 6.096486] mmc0: tuning execution failed: -5 >>>>> [ 6.096508] mmc0: error -5 whilst initialising MMC card >>>>> [ 7.206431] mmc0: tuning execution failed: -5 >>>>> [ 7.206454] mmc0: error -5 whilst initialising MMC card >>>> >>>> Thanks for the report! Are you able to debug this any more? >>>> What transfer mode is it? e.g. HS400? Can you enable debug >>>> messages and get more information? >>> >>> With some guidance yes, it's in HS200 as there is an issue with HS400 >>> to HS200 switch on this SoC so I have HS400 disabled. >>> >>> With CONFIG_MMC_DEBUG and loglevel=8 I dont have any new >>> messages. >> >> You should get a lot more with: >> >> CONFIG_DYNAMIC_DEBUG=y >> >> and kernel commandline option: >> >> dyndbg="file drivers/mmc/core/* +p;file drivers/mmc/host/* +p" > > Unfortunatelly not: Are you sure you have debug messages enabled with your current console loglevel? Might want to add "debug" at the end of your kernel command line and try again.
On Wed, 30 Nov 2022 at 19:39, Florian Fainelli <f.fainelli@gmail.com> wrote: > > On 11/30/22 09:24, Robert Marko wrote: > > On Wed, 30 Nov 2022 at 15:16, Adrian Hunter <adrian.hunter@intel.com> wrote: > >> > >> On 30/11/22 15:00, Robert Marko wrote: > >>> On Wed, 30 Nov 2022 at 13:46, Adrian Hunter <adrian.hunter@intel.com> wrote: > >>>> > >>>> On 30/11/22 13:54, Robert Marko wrote: > >>>>> > >>>>> On 28. 11. 2022. 14:32, Adrian Hunter wrote: > >>>>>> Avoid re-configuring UHS and preset settings when the settings have not > >>>>>> changed, irrespective of whether the clock is turning on. > >>>>>> > >>>>>> Tested-by: Haibo Chen <haibo.chen@nxp.com> > >>>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > >>>>> > >>>>> Hi, this is breaking sdhci-msm on IPQ8074 in next-20221130 for me > >>>>> and reverting it makes the eMMC work again. > >>>>> > >>>>> I get a lot of: > >>>>> > >>>>> [ 2.727287] mmc0: tuning execution failed: -5 > >>>>> [ 2.727323] mmc0: error -5 whilst initialising MMC card > >>>>> [ 3.846540] mmc0: tuning execution failed: -5 > >>>>> [ 3.846564] mmc0: error -5 whilst initialising MMC card > >>>>> [ 4.966517] mmc0: tuning execution failed: -5 > >>>>> [ 4.966539] mmc0: error -5 whilst initialising MMC card > >>>>> [ 6.096486] mmc0: tuning execution failed: -5 > >>>>> [ 6.096508] mmc0: error -5 whilst initialising MMC card > >>>>> [ 7.206431] mmc0: tuning execution failed: -5 > >>>>> [ 7.206454] mmc0: error -5 whilst initialising MMC card > >>>> > >>>> Thanks for the report! Are you able to debug this any more? > >>>> What transfer mode is it? e.g. HS400? Can you enable debug > >>>> messages and get more information? > >>> > >>> With some guidance yes, it's in HS200 as there is an issue with HS400 > >>> to HS200 switch on this SoC so I have HS400 disabled. > >>> > >>> With CONFIG_MMC_DEBUG and loglevel=8 I dont have any new > >>> messages. > >> > >> You should get a lot more with: > >> > >> CONFIG_DYNAMIC_DEBUG=y > >> > >> and kernel commandline option: > >> > >> dyndbg="file drivers/mmc/core/* +p;file drivers/mmc/host/* +p" > > > > Unfortunatelly not: > > Are you sure you have debug messages enabled with your current console > loglevel? Might want to add "debug" at the end of your kernel command > line and try again. Ok, so indeed debug was required, here is the huge bootlog now: https://gist.github.com/robimarko/e370bce66d0d2e7e54a2f5daf9784ee4 Regards, Robert > -- > Florian >
On 30/11/22 21:56, Robert Marko wrote: > On Wed, 30 Nov 2022 at 19:39, Florian Fainelli <f.fainelli@gmail.com> wrote: >> >> On 11/30/22 09:24, Robert Marko wrote: >>> On Wed, 30 Nov 2022 at 15:16, Adrian Hunter <adrian.hunter@intel.com> wrote: >>>> >>>> On 30/11/22 15:00, Robert Marko wrote: >>>>> On Wed, 30 Nov 2022 at 13:46, Adrian Hunter <adrian.hunter@intel.com> wrote: >>>>>> >>>>>> On 30/11/22 13:54, Robert Marko wrote: >>>>>>> >>>>>>> On 28. 11. 2022. 14:32, Adrian Hunter wrote: >>>>>>>> Avoid re-configuring UHS and preset settings when the settings have not >>>>>>>> changed, irrespective of whether the clock is turning on. >>>>>>>> >>>>>>>> Tested-by: Haibo Chen <haibo.chen@nxp.com> >>>>>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> >>>>>>> >>>>>>> Hi, this is breaking sdhci-msm on IPQ8074 in next-20221130 for me >>>>>>> and reverting it makes the eMMC work again. >>>>>>> >>>>>>> I get a lot of: >>>>>>> >>>>>>> [ 2.727287] mmc0: tuning execution failed: -5 >>>>>>> [ 2.727323] mmc0: error -5 whilst initialising MMC card >>>>>>> [ 3.846540] mmc0: tuning execution failed: -5 >>>>>>> [ 3.846564] mmc0: error -5 whilst initialising MMC card >>>>>>> [ 4.966517] mmc0: tuning execution failed: -5 >>>>>>> [ 4.966539] mmc0: error -5 whilst initialising MMC card >>>>>>> [ 6.096486] mmc0: tuning execution failed: -5 >>>>>>> [ 6.096508] mmc0: error -5 whilst initialising MMC card >>>>>>> [ 7.206431] mmc0: tuning execution failed: -5 >>>>>>> [ 7.206454] mmc0: error -5 whilst initialising MMC card >>>>>> >>>>>> Thanks for the report! Are you able to debug this any more? >>>>>> What transfer mode is it? e.g. HS400? Can you enable debug >>>>>> messages and get more information? >>>>> >>>>> With some guidance yes, it's in HS200 as there is an issue with HS400 >>>>> to HS200 switch on this SoC so I have HS400 disabled. >>>>> >>>>> With CONFIG_MMC_DEBUG and loglevel=8 I dont have any new >>>>> messages. >>>> >>>> You should get a lot more with: >>>> >>>> CONFIG_DYNAMIC_DEBUG=y >>>> >>>> and kernel commandline option: >>>> >>>> dyndbg="file drivers/mmc/core/* +p;file drivers/mmc/host/* +p" >>> >>> Unfortunatelly not: >> >> Are you sure you have debug messages enabled with your current console >> loglevel? Might want to add "debug" at the end of your kernel command >> line and try again. > > Ok, so indeed debug was required, here is the huge bootlog now: > https://gist.github.com/robimarko/e370bce66d0d2e7e54a2f5daf9784ee4 Thanks for the log! It shows everything is OK up until the first (HS200) tuning. sdhci-msm takes the clock frequency into account when setting UHS signaling, refer sdhci_msm_set_uhs_signaling(), so that is presumably why the UHS signalling needs to be re-done even if only the clock frequency changes. It would be possible to change sdhci-msm to hook ->set_ios() and set host->reinit_uhs before calling sdhci_set_ios(), which should put back the original behaviour for sdhci-msm. However "mmc: sdhci: Avoid unnecessary re-configuration" is really a "nice-to-have" and it is not impossible other drivers are affected by something similar, but just haven't noticed. Consequently, I tend to think we should just drop the patch "mmc: sdhci: Avoid unnecessary re-configuration" ?
On Thu, 1 Dec 2022 at 09:40, Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 30/11/22 21:56, Robert Marko wrote: > > On Wed, 30 Nov 2022 at 19:39, Florian Fainelli <f.fainelli@gmail.com> wrote: > >> > >> On 11/30/22 09:24, Robert Marko wrote: > >>> On Wed, 30 Nov 2022 at 15:16, Adrian Hunter <adrian.hunter@intel.com> wrote: > >>>> > >>>> On 30/11/22 15:00, Robert Marko wrote: > >>>>> On Wed, 30 Nov 2022 at 13:46, Adrian Hunter <adrian.hunter@intel.com> wrote: > >>>>>> > >>>>>> On 30/11/22 13:54, Robert Marko wrote: > >>>>>>> > >>>>>>> On 28. 11. 2022. 14:32, Adrian Hunter wrote: > >>>>>>>> Avoid re-configuring UHS and preset settings when the settings have not > >>>>>>>> changed, irrespective of whether the clock is turning on. > >>>>>>>> > >>>>>>>> Tested-by: Haibo Chen <haibo.chen@nxp.com> > >>>>>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > >>>>>>> > >>>>>>> Hi, this is breaking sdhci-msm on IPQ8074 in next-20221130 for me > >>>>>>> and reverting it makes the eMMC work again. > >>>>>>> > >>>>>>> I get a lot of: > >>>>>>> > >>>>>>> [ 2.727287] mmc0: tuning execution failed: -5 > >>>>>>> [ 2.727323] mmc0: error -5 whilst initialising MMC card > >>>>>>> [ 3.846540] mmc0: tuning execution failed: -5 > >>>>>>> [ 3.846564] mmc0: error -5 whilst initialising MMC card > >>>>>>> [ 4.966517] mmc0: tuning execution failed: -5 > >>>>>>> [ 4.966539] mmc0: error -5 whilst initialising MMC card > >>>>>>> [ 6.096486] mmc0: tuning execution failed: -5 > >>>>>>> [ 6.096508] mmc0: error -5 whilst initialising MMC card > >>>>>>> [ 7.206431] mmc0: tuning execution failed: -5 > >>>>>>> [ 7.206454] mmc0: error -5 whilst initialising MMC card > >>>>>> > >>>>>> Thanks for the report! Are you able to debug this any more? > >>>>>> What transfer mode is it? e.g. HS400? Can you enable debug > >>>>>> messages and get more information? > >>>>> > >>>>> With some guidance yes, it's in HS200 as there is an issue with HS400 > >>>>> to HS200 switch on this SoC so I have HS400 disabled. > >>>>> > >>>>> With CONFIG_MMC_DEBUG and loglevel=8 I dont have any new > >>>>> messages. > >>>> > >>>> You should get a lot more with: > >>>> > >>>> CONFIG_DYNAMIC_DEBUG=y > >>>> > >>>> and kernel commandline option: > >>>> > >>>> dyndbg="file drivers/mmc/core/* +p;file drivers/mmc/host/* +p" > >>> > >>> Unfortunatelly not: > >> > >> Are you sure you have debug messages enabled with your current console > >> loglevel? Might want to add "debug" at the end of your kernel command > >> line and try again. > > > > Ok, so indeed debug was required, here is the huge bootlog now: > > https://gist.github.com/robimarko/e370bce66d0d2e7e54a2f5daf9784ee4 > > Thanks for the log! It shows everything is OK up until the first > (HS200) tuning. > > sdhci-msm takes the clock frequency into account when setting UHS > signaling, refer sdhci_msm_set_uhs_signaling(), so that is > presumably why the UHS signalling needs to be re-done even if > only the clock frequency changes. > > It would be possible to change sdhci-msm to hook ->set_ios() and > set host->reinit_uhs before calling sdhci_set_ios(), which should > put back the original behaviour for sdhci-msm. > > However "mmc: sdhci: Avoid unnecessary re-configuration" is really > a "nice-to-have" and it is not impossible other drivers are affected > by something similar, but just haven't noticed. > > Consequently, I tend to think we should just drop the patch > "mmc: sdhci: Avoid unnecessary re-configuration" ? That works for me, as I am not skilled to update the sdhci-msm driver instead. Regards, Robert > >
On Thu, 1 Dec 2022 at 09:40, Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 30/11/22 21:56, Robert Marko wrote: > > On Wed, 30 Nov 2022 at 19:39, Florian Fainelli <f.fainelli@gmail.com> wrote: > >> > >> On 11/30/22 09:24, Robert Marko wrote: > >>> On Wed, 30 Nov 2022 at 15:16, Adrian Hunter <adrian.hunter@intel.com> wrote: > >>>> > >>>> On 30/11/22 15:00, Robert Marko wrote: > >>>>> On Wed, 30 Nov 2022 at 13:46, Adrian Hunter <adrian.hunter@intel.com> wrote: > >>>>>> > >>>>>> On 30/11/22 13:54, Robert Marko wrote: > >>>>>>> > >>>>>>> On 28. 11. 2022. 14:32, Adrian Hunter wrote: > >>>>>>>> Avoid re-configuring UHS and preset settings when the settings have not > >>>>>>>> changed, irrespective of whether the clock is turning on. > >>>>>>>> > >>>>>>>> Tested-by: Haibo Chen <haibo.chen@nxp.com> > >>>>>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > >>>>>>> > >>>>>>> Hi, this is breaking sdhci-msm on IPQ8074 in next-20221130 for me > >>>>>>> and reverting it makes the eMMC work again. > >>>>>>> > >>>>>>> I get a lot of: > >>>>>>> > >>>>>>> [ 2.727287] mmc0: tuning execution failed: -5 > >>>>>>> [ 2.727323] mmc0: error -5 whilst initialising MMC card > >>>>>>> [ 3.846540] mmc0: tuning execution failed: -5 > >>>>>>> [ 3.846564] mmc0: error -5 whilst initialising MMC card > >>>>>>> [ 4.966517] mmc0: tuning execution failed: -5 > >>>>>>> [ 4.966539] mmc0: error -5 whilst initialising MMC card > >>>>>>> [ 6.096486] mmc0: tuning execution failed: -5 > >>>>>>> [ 6.096508] mmc0: error -5 whilst initialising MMC card > >>>>>>> [ 7.206431] mmc0: tuning execution failed: -5 > >>>>>>> [ 7.206454] mmc0: error -5 whilst initialising MMC card > >>>>>> > >>>>>> Thanks for the report! Are you able to debug this any more? > >>>>>> What transfer mode is it? e.g. HS400? Can you enable debug > >>>>>> messages and get more information? > >>>>> > >>>>> With some guidance yes, it's in HS200 as there is an issue with HS400 > >>>>> to HS200 switch on this SoC so I have HS400 disabled. > >>>>> > >>>>> With CONFIG_MMC_DEBUG and loglevel=8 I dont have any new > >>>>> messages. > >>>> > >>>> You should get a lot more with: > >>>> > >>>> CONFIG_DYNAMIC_DEBUG=y > >>>> > >>>> and kernel commandline option: > >>>> > >>>> dyndbg="file drivers/mmc/core/* +p;file drivers/mmc/host/* +p" > >>> > >>> Unfortunatelly not: > >> > >> Are you sure you have debug messages enabled with your current console > >> loglevel? Might want to add "debug" at the end of your kernel command > >> line and try again. > > > > Ok, so indeed debug was required, here is the huge bootlog now: > > https://gist.github.com/robimarko/e370bce66d0d2e7e54a2f5daf9784ee4 > > Thanks for the log! It shows everything is OK up until the first > (HS200) tuning. > > sdhci-msm takes the clock frequency into account when setting UHS > signaling, refer sdhci_msm_set_uhs_signaling(), so that is > presumably why the UHS signalling needs to be re-done even if > only the clock frequency changes. > > It would be possible to change sdhci-msm to hook ->set_ios() and > set host->reinit_uhs before calling sdhci_set_ios(), which should > put back the original behaviour for sdhci-msm. > > However "mmc: sdhci: Avoid unnecessary re-configuration" is really > a "nice-to-have" and it is not impossible other drivers are affected > by something similar, but just haven't noticed. > > Consequently, I tend to think we should just drop the patch > "mmc: sdhci: Avoid unnecessary re-configuration" ? Sounds reasonable. I have dropped the patch for now. If we try something like this again later, perhaps better to do that a bit earlier in the release cycle to get it more tested in linux-next too. Kind regards Uffe
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 64750fbb0ac8..17e5ccf9a855 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2315,7 +2315,6 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) { struct sdhci_host *host = mmc_priv(mmc); bool reinit_uhs = host->reinit_uhs; - bool turning_on_clk = false; u8 ctrl; host->reinit_uhs = false; @@ -2345,8 +2344,6 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) sdhci_enable_preset_value(host, false); if (!ios->clock || ios->clock != host->clock) { - turning_on_clk = ios->clock && !host->clock; - host->ops->set_clock(host, ios->clock); host->clock = ios->clock; @@ -2374,11 +2371,10 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) host->ops->set_bus_width(host, ios->bus_width); /* - * Special case to avoid multiple clock changes during voltage - * switching. + * Avoid unnecessary changes. In particular, this avoids multiple clock + * changes during voltage switching. */ if (!reinit_uhs && - turning_on_clk && host->timing == ios->timing && host->version >= SDHCI_SPEC_300 && !sdhci_presetable_values_change(host, ios))