diff mbox series

[V2,2/4] mmc: sdhci: Avoid unnecessary re-configuration

Message ID 20221128133259.38305-3-adrian.hunter@intel.com
State New
Headers show
Series mmc: sdhci: Fix voltage switch delay | expand

Commit Message

Adrian Hunter Nov. 28, 2022, 1:32 p.m. UTC
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>
---
 drivers/mmc/host/sdhci.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

Robert Marko Nov. 30, 2022, 11:54 a.m. UTC | #1
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))
Adrian Hunter Nov. 30, 2022, 12:45 p.m. UTC | #2
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?
Robert Marko Nov. 30, 2022, 1 p.m. UTC | #3
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
>
Adrian Hunter Nov. 30, 2022, 2:15 p.m. UTC | #4
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"
Robert Marko Nov. 30, 2022, 5:24 p.m. UTC | #5
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
>
>
Florian Fainelli Nov. 30, 2022, 6:39 p.m. UTC | #6
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.
Robert Marko Nov. 30, 2022, 7:56 p.m. UTC | #7
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
>
Adrian Hunter Dec. 1, 2022, 8:40 a.m. UTC | #8
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" ?
Robert Marko Dec. 1, 2022, 10:48 a.m. UTC | #9
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
>
>
Ulf Hansson Dec. 1, 2022, 10:58 a.m. UTC | #10
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 mbox series

Patch

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))