* [PATCH] mmc: sdhci: Fix incorrect switch to HS mode
@ 2019-09-03 11:51 Al Cooper
2019-09-03 14:51 ` Ulf Hansson
0 siblings, 1 reply; 6+ messages in thread
From: Al Cooper @ 2019-09-03 11:51 UTC (permalink / raw)
To: linux-kernel; +Cc: Al Cooper, Adrian Hunter, linux-mmc, Ulf Hansson
When switching from any MMC speed mode that requires 1.8v
(HS200, HS400 and HS400ES) to High Speed (HS) mode, the system
ends up configured for SDR12 with a 50MHz clock which is an illegal
mode.
This happens because the SDHCI_CTRL_VDD_180 bit in the
SDHCI_HOST_CONTROL2 register is left set and when this bit is
set, the speed mode is controlled by the SDHCI_CTRL_UHS field
in the SDHCI_HOST_CONTROL2 register. The SDHCI_CTRL_UHS field
will end up being set to 0 (SDR12) by sdhci_set_uhs_signaling()
because there is no UHS mode being set.
The fix is to change sdhci_set_uhs_signaling() to set the
SDHCI_CTRL_UHS field to SDR25 (which is the same as HS) for
any switch to HS mode.
This was found on a new eMMC controller that does strict checking
of the speed mode and the corresponding clock rate. It caused the
switch to HS400 mode to fail because part of the sequence to switch
to HS400 requires a switch from HS200 to HS before going to HS400.
This fix was suggested by Adrian Hunter
Signed-off-by: Al Cooper <alcooperx@gmail.com>
---
drivers/mmc/host/sdhci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 61d845fe0b97..068149640ecd 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1858,7 +1858,9 @@ void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing)
ctrl_2 |= SDHCI_CTRL_UHS_SDR104;
else if (timing == MMC_TIMING_UHS_SDR12)
ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
- else if (timing == MMC_TIMING_UHS_SDR25)
+ else if (timing == MMC_TIMING_SD_HS ||
+ timing == MMC_TIMING_MMC_HS ||
+ timing == MMC_TIMING_UHS_SDR25)
ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
else if (timing == MMC_TIMING_UHS_SDR50)
ctrl_2 |= SDHCI_CTRL_UHS_SDR50;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: sdhci: Fix incorrect switch to HS mode
2019-09-03 11:51 [PATCH] mmc: sdhci: Fix incorrect switch to HS mode Al Cooper
@ 2019-09-03 14:51 ` Ulf Hansson
2019-09-19 11:57 ` Alan Cooper
0 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2019-09-03 14:51 UTC (permalink / raw)
To: Al Cooper; +Cc: Linux Kernel Mailing List, Adrian Hunter, linux-mmc
On Tue, 3 Sep 2019 at 13:51, Al Cooper <alcooperx@gmail.com> wrote:
>
> When switching from any MMC speed mode that requires 1.8v
> (HS200, HS400 and HS400ES) to High Speed (HS) mode, the system
> ends up configured for SDR12 with a 50MHz clock which is an illegal
> mode.
>
> This happens because the SDHCI_CTRL_VDD_180 bit in the
> SDHCI_HOST_CONTROL2 register is left set and when this bit is
> set, the speed mode is controlled by the SDHCI_CTRL_UHS field
> in the SDHCI_HOST_CONTROL2 register. The SDHCI_CTRL_UHS field
> will end up being set to 0 (SDR12) by sdhci_set_uhs_signaling()
> because there is no UHS mode being set.
>
> The fix is to change sdhci_set_uhs_signaling() to set the
> SDHCI_CTRL_UHS field to SDR25 (which is the same as HS) for
> any switch to HS mode.
>
> This was found on a new eMMC controller that does strict checking
> of the speed mode and the corresponding clock rate. It caused the
> switch to HS400 mode to fail because part of the sequence to switch
> to HS400 requires a switch from HS200 to HS before going to HS400.
>
> This fix was suggested by Adrian Hunter
>
> Signed-off-by: Al Cooper <alcooperx@gmail.com>
Should this be applied for fixes and tagged for stable you think?
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 61d845fe0b97..068149640ecd 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1858,7 +1858,9 @@ void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing)
> ctrl_2 |= SDHCI_CTRL_UHS_SDR104;
> else if (timing == MMC_TIMING_UHS_SDR12)
> ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
> - else if (timing == MMC_TIMING_UHS_SDR25)
> + else if (timing == MMC_TIMING_SD_HS ||
> + timing == MMC_TIMING_MMC_HS ||
> + timing == MMC_TIMING_UHS_SDR25)
> ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
> else if (timing == MMC_TIMING_UHS_SDR50)
> ctrl_2 |= SDHCI_CTRL_UHS_SDR50;
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: sdhci: Fix incorrect switch to HS mode
2019-09-03 14:51 ` Ulf Hansson
@ 2019-09-19 11:57 ` Alan Cooper
2019-11-28 8:21 ` Faiz Abbas
0 siblings, 1 reply; 6+ messages in thread
From: Alan Cooper @ 2019-09-19 11:57 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Linux Kernel Mailing List, Adrian Hunter, linux-mmc
This does correct the sequence of switching to HS400 but it might be
safest to just add this to the latest until it gets a little testing
to make sure it doesn't expose some bug in existing controllers.
Thanks
Al
On Tue, Sep 3, 2019 at 10:52 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Tue, 3 Sep 2019 at 13:51, Al Cooper <alcooperx@gmail.com> wrote:
> >
> > When switching from any MMC speed mode that requires 1.8v
> > (HS200, HS400 and HS400ES) to High Speed (HS) mode, the system
> > ends up configured for SDR12 with a 50MHz clock which is an illegal
> > mode.
> >
> > This happens because the SDHCI_CTRL_VDD_180 bit in the
> > SDHCI_HOST_CONTROL2 register is left set and when this bit is
> > set, the speed mode is controlled by the SDHCI_CTRL_UHS field
> > in the SDHCI_HOST_CONTROL2 register. The SDHCI_CTRL_UHS field
> > will end up being set to 0 (SDR12) by sdhci_set_uhs_signaling()
> > because there is no UHS mode being set.
> >
> > The fix is to change sdhci_set_uhs_signaling() to set the
> > SDHCI_CTRL_UHS field to SDR25 (which is the same as HS) for
> > any switch to HS mode.
> >
> > This was found on a new eMMC controller that does strict checking
> > of the speed mode and the corresponding clock rate. It caused the
> > switch to HS400 mode to fail because part of the sequence to switch
> > to HS400 requires a switch from HS200 to HS before going to HS400.
> >
> > This fix was suggested by Adrian Hunter
> >
> > Signed-off-by: Al Cooper <alcooperx@gmail.com>
>
> Should this be applied for fixes and tagged for stable you think?
>
> Kind regards
> Uffe
>
> > ---
> > drivers/mmc/host/sdhci.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index 61d845fe0b97..068149640ecd 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -1858,7 +1858,9 @@ void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing)
> > ctrl_2 |= SDHCI_CTRL_UHS_SDR104;
> > else if (timing == MMC_TIMING_UHS_SDR12)
> > ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
> > - else if (timing == MMC_TIMING_UHS_SDR25)
> > + else if (timing == MMC_TIMING_SD_HS ||
> > + timing == MMC_TIMING_MMC_HS ||
> > + timing == MMC_TIMING_UHS_SDR25)
> > ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
> > else if (timing == MMC_TIMING_UHS_SDR50)
> > ctrl_2 |= SDHCI_CTRL_UHS_SDR50;
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: sdhci: Fix incorrect switch to HS mode
2019-09-19 11:57 ` Alan Cooper
@ 2019-11-28 8:21 ` Faiz Abbas
2019-11-28 9:26 ` Adrian Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Faiz Abbas @ 2019-11-28 8:21 UTC (permalink / raw)
To: Alan Cooper, Ulf Hansson
Cc: Linux Kernel Mailing List, Adrian Hunter, linux-mmc
Hi,
On 19/09/19 5:27 PM, Alan Cooper wrote:
> This does correct the sequence of switching to HS400 but it might be
> safest to just add this to the latest until it gets a little testing
> to make sure it doesn't expose some bug in existing controllers.
>
> Thanks
> Al
>
> On Tue, Sep 3, 2019 at 10:52 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>> On Tue, 3 Sep 2019 at 13:51, Al Cooper <alcooperx@gmail.com> wrote:
>>>
>>> When switching from any MMC speed mode that requires 1.8v
>>> (HS200, HS400 and HS400ES) to High Speed (HS) mode, the system
>>> ends up configured for SDR12 with a 50MHz clock which is an illegal
>>> mode.
>>>
>>> This happens because the SDHCI_CTRL_VDD_180 bit in the
>>> SDHCI_HOST_CONTROL2 register is left set and when this bit is
>>> set, the speed mode is controlled by the SDHCI_CTRL_UHS field
>>> in the SDHCI_HOST_CONTROL2 register. The SDHCI_CTRL_UHS field
>>> will end up being set to 0 (SDR12) by sdhci_set_uhs_signaling()
>>> because there is no UHS mode being set.
>>>
>>> The fix is to change sdhci_set_uhs_signaling() to set the
>>> SDHCI_CTRL_UHS field to SDR25 (which is the same as HS) for
>>> any switch to HS mode.
This change has broken High speed mode in SD card for me in AM65x-evm. I
guess this change only needs to be done for eMMC. SDR25 is decidedly not
the same as high speed for SD card.
Thanks,
Faiz
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: sdhci: Fix incorrect switch to HS mode
2019-11-28 8:21 ` Faiz Abbas
@ 2019-11-28 9:26 ` Adrian Hunter
2019-11-28 10:12 ` Faiz Abbas
0 siblings, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2019-11-28 9:26 UTC (permalink / raw)
To: Faiz Abbas, Alan Cooper, Ulf Hansson; +Cc: Linux Kernel Mailing List, linux-mmc
On 28/11/19 10:21 AM, Faiz Abbas wrote:
> Hi,
>
> On 19/09/19 5:27 PM, Alan Cooper wrote:
>> This does correct the sequence of switching to HS400 but it might be
>> safest to just add this to the latest until it gets a little testing
>> to make sure it doesn't expose some bug in existing controllers.
>>
>> Thanks
>> Al
>>
>> On Tue, Sep 3, 2019 at 10:52 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>
>>> On Tue, 3 Sep 2019 at 13:51, Al Cooper <alcooperx@gmail.com> wrote:
>>>>
>>>> When switching from any MMC speed mode that requires 1.8v
>>>> (HS200, HS400 and HS400ES) to High Speed (HS) mode, the system
>>>> ends up configured for SDR12 with a 50MHz clock which is an illegal
>>>> mode.
>>>>
>>>> This happens because the SDHCI_CTRL_VDD_180 bit in the
>>>> SDHCI_HOST_CONTROL2 register is left set and when this bit is
>>>> set, the speed mode is controlled by the SDHCI_CTRL_UHS field
>>>> in the SDHCI_HOST_CONTROL2 register. The SDHCI_CTRL_UHS field
>>>> will end up being set to 0 (SDR12) by sdhci_set_uhs_signaling()
>>>> because there is no UHS mode being set.
>>>>
>>>> The fix is to change sdhci_set_uhs_signaling() to set the
>>>> SDHCI_CTRL_UHS field to SDR25 (which is the same as HS) for
>>>> any switch to HS mode.
>
> This change has broken High speed mode in SD card for me in AM65x-evm. I
> guess this change only needs to be done for eMMC. SDR25 is decidedly not
> the same as high speed for SD card.
Shall we revert c894e33ddc1910e14d6f2a2016f60ab613fd8b37 and then Alan
could send a patch providing the desired behaviour in ->set_uhs_signaling()
of the relevant driver e.g.
void ???_set_uhs_signaling(struct sdhci_host *host, unsigned int timing)
{
if (timing == MMC_TIMING_SD_HS || timing == MMC_TIMING_MMC_HS)
timing = MMC_TIMING_UHS_SDR25;
sdhci_set_uhs_signaling(host, timing);
}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mmc: sdhci: Fix incorrect switch to HS mode
2019-11-28 9:26 ` Adrian Hunter
@ 2019-11-28 10:12 ` Faiz Abbas
0 siblings, 0 replies; 6+ messages in thread
From: Faiz Abbas @ 2019-11-28 10:12 UTC (permalink / raw)
To: Adrian Hunter, Alan Cooper, Ulf Hansson
Cc: Linux Kernel Mailing List, linux-mmc
Adrian,
On 28/11/19 2:56 PM, Adrian Hunter wrote:
> On 28/11/19 10:21 AM, Faiz Abbas wrote:
>> Hi,
>>
>> On 19/09/19 5:27 PM, Alan Cooper wrote:
>>> This does correct the sequence of switching to HS400 but it might be
>>> safest to just add this to the latest until it gets a little testing
>>> to make sure it doesn't expose some bug in existing controllers.
>>>
>>> Thanks
>>> Al
>>>
>>> On Tue, Sep 3, 2019 at 10:52 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>>>
>>>> On Tue, 3 Sep 2019 at 13:51, Al Cooper <alcooperx@gmail.com> wrote:
>>>>>
>>>>> When switching from any MMC speed mode that requires 1.8v
>>>>> (HS200, HS400 and HS400ES) to High Speed (HS) mode, the system
>>>>> ends up configured for SDR12 with a 50MHz clock which is an illegal
>>>>> mode.
>>>>>
>>>>> This happens because the SDHCI_CTRL_VDD_180 bit in the
>>>>> SDHCI_HOST_CONTROL2 register is left set and when this bit is
>>>>> set, the speed mode is controlled by the SDHCI_CTRL_UHS field
>>>>> in the SDHCI_HOST_CONTROL2 register. The SDHCI_CTRL_UHS field
>>>>> will end up being set to 0 (SDR12) by sdhci_set_uhs_signaling()
>>>>> because there is no UHS mode being set.
>>>>>
>>>>> The fix is to change sdhci_set_uhs_signaling() to set the
>>>>> SDHCI_CTRL_UHS field to SDR25 (which is the same as HS) for
>>>>> any switch to HS mode.
>>
>> This change has broken High speed mode in SD card for me in AM65x-evm. I
>> guess this change only needs to be done for eMMC. SDR25 is decidedly not
>> the same as high speed for SD card.
>
> Shall we revert c894e33ddc1910e14d6f2a2016f60ab613fd8b37 and then Alan
> could send a patch providing the desired behaviour in ->set_uhs_signaling()
> of the relevant driver e.g.
>
> void ???_set_uhs_signaling(struct sdhci_host *host, unsigned int timing)
> {
> if (timing == MMC_TIMING_SD_HS || timing == MMC_TIMING_MMC_HS)
> timing = MMC_TIMING_UHS_SDR25;
> sdhci_set_uhs_signaling(host, timing);
> }
>
Yes. Lets do that. I will send the revert.
Thanks,
Faiz
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-28 10:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-03 11:51 [PATCH] mmc: sdhci: Fix incorrect switch to HS mode Al Cooper
2019-09-03 14:51 ` Ulf Hansson
2019-09-19 11:57 ` Alan Cooper
2019-11-28 8:21 ` Faiz Abbas
2019-11-28 9:26 ` Adrian Hunter
2019-11-28 10:12 ` Faiz Abbas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).