[PATCH] imx: kontron-sl-mx8mm: Fix SD card IO voltage level

[PATCH] imx: kontron-sl-mx8mm: Fix SD card IO voltage level

[Frieder Schrempf]

From: Frieder Schrempf <frieder.schrempf@kontron.de>

The LDO5 of the PCA9450 PMIC can be switched between two different
voltage settings (defaulting to 1.8V and 3.3V) using an external
signal SD_VSEL that is connected to the VSELECT signal of the SD
card interface.

As the regulator driver can't deal with both LDO registers (LDO5CTRL_H
and LDO5CTRL_L) it only uses one of them, which means reading the
voltage from the regulator can potentially return a value that does not
reflect the actual state of the LDO5 output.

In our case, after booting U-Boot we read 1.8V from the regulator
while in fact as the VSELECT signal is still low the regulator outputs
3.3V. This confusion causes the MMC driver to think it is dealing with
a 1.8V-only device. This in turn leads to SD cards being addressed
with 1.8V IO levels even if UHS support is not available or disabled.

Some cards with UHS support still work even if they are addressed
with 1.8V levels in non-UHS modes, but a lot of cards also fail with
timeout errors like:

  Card did not respond to voltage select! : -110

As a workaorund we disable the vqmmc regulator for now so we can make
sure no wrong values are read from the regulator. The switching
between 1.8V and 3.3V still works as the ESDHC driver sets the
VSELECT signal accordingly.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
By the way: I suspect that other boards using the PCA9450 might also be affected
by this. I didn't find a nice generic solution so far. It would be possible to
patch the pca9450 driver to use the PCA9450_LDO5CTRL_L instead of the
PCA9450_LDO5CTRL_H register for LDO5. This would fix this particular case,
but still not the root problem of the regulator driver returning wrong values.
So if anyone got some idea how to properly handle this, let me know.
The same issue is also present in Linux. While I didn't notice any problems
with the SD card being addressed with incorrect voltage levels so far,
reading the regulator doesn't return the correct value if VSELECT is low.
---
---
 arch/arm/dts/imx8mm-kontron-bl-common-u-boot.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/dts/imx8mm-kontron-bl-common-u-boot.dtsi b/arch/arm/dts/imx8mm-kontron-bl-common-u-boot.dtsi
index 5b8b472159..8321424649 100644
--- a/arch/arm/dts/imx8mm-kontron-bl-common-u-boot.dtsi
+++ b/arch/arm/dts/imx8mm-kontron-bl-common-u-boot.dtsi
@@ -137,6 +137,12 @@
 
 &usdhc2 {
 	u-boot,dm-spl;
+	/*
+	 * Delete the reference to the IO voltage regulator in order to prevent
+	 * wrong information being passed to the MMC driver by reading the current
+	 * voltage of the PCA9450 LDO5.
+	 */
+	/delete-property/ vqmmc-supply;
 };
 
 &wdog1 {
-- 
2.39.1

Re: [PATCH] imx: kontron-sl-mx8mm: Fix SD card IO voltage level

[Marek Vasut]

On 1/25/23 14:41, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>

Subject tags should be ARM: dts: imx:

> The LDO5 of the PCA9450 PMIC can be switched between two different
> voltage settings (defaulting to 1.8V and 3.3V) using an external
> signal SD_VSEL that is connected to the VSELECT signal of the SD
> card interface.
> 
> As the regulator driver can't deal with both LDO registers (LDO5CTRL_H
> and LDO5CTRL_L) it only uses one of them, which means reading the
> voltage from the regulator can potentially return a value that does not
> reflect the actual state of the LDO5 output.

Can you fix the regulator driver ?

> In our case, after booting U-Boot we read 1.8V from the regulator
> while in fact as the VSELECT signal is still low the regulator outputs
> 3.3V. This confusion causes the MMC driver to think it is dealing with
> a 1.8V-only device. This in turn leads to SD cards being addressed
> with 1.8V IO levels even if UHS support is not available or disabled.
> 
> Some cards with UHS support still work even if they are addressed
> with 1.8V levels in non-UHS modes, but a lot of cards also fail with
> timeout errors like:
> 
>    Card did not respond to voltage select! : -110
> 
> As a workaorund we disable the vqmmc regulator for now so we can make
> sure no wrong values are read from the regulator. The switching
> between 1.8V and 3.3V still works as the ESDHC driver sets the
> VSELECT signal accordingly.
> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> ---
> By the way: I suspect that other boards using the PCA9450 might also be affected
> by this. I didn't find a nice generic solution so far. It would be possible to
> patch the pca9450 driver to use the PCA9450_LDO5CTRL_L instead of the
> PCA9450_LDO5CTRL_H register for LDO5. This would fix this particular case,
> but still not the root problem of the regulator driver returning wrong values.
> So if anyone got some idea how to properly handle this, let me know.
> The same issue is also present in Linux. While I didn't notice any problems
> with the SD card being addressed with incorrect voltage levels so far,
> reading the regulator doesn't return the correct value if VSELECT is low.

Configure VSELECT pinmux such that it is a function, as it is right now, 
MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT or similar, (so SD controller 
driver can operate the pin via SD controller), but set SION bit so GPIO 
controller can read its state (activate bit 30), i.e.:
MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT 0x40000004

Use sd-vsel-gpios property of PMIC, claim the VSELECT as GPIO in PMIC 
driver (this won't change pinmux, the pin would still be configured as 
function, but SION bit would allow you to read its state), read its 
state, and return the correct LDO5CTRL_H/LDO5CTRL_L value based on that.

If this works, then:
     arm64: dts: imx8mp: Drop sd-vsel-gpios from *
Linux kernel patches should not be applied.

[...]

Re: [PATCH] imx: kontron-sl-mx8mm: Fix SD card IO voltage level

[Frieder Schrempf]

Hi Marek,

On 25.01.23 18:15, Marek Vasut wrote:
> On 1/25/23 14:41, Frieder Schrempf wrote:
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> Subject tags should be ARM: dts: imx:

Ok, how do I know? Because "git log arch/arm/dts" shows me that
"<platform>: <board>:" is used quite often and it's what I used in the
past. But I'm happy to change it if "ARM: dts: imx:" is the way the
prefix should look like.

> 
>> The LDO5 of the PCA9450 PMIC can be switched between two different
>> voltage settings (defaulting to 1.8V and 3.3V) using an external
>> signal SD_VSEL that is connected to the VSELECT signal of the SD
>> card interface.
>>
>> As the regulator driver can't deal with both LDO registers (LDO5CTRL_H
>> and LDO5CTRL_L) it only uses one of them, which means reading the
>> voltage from the regulator can potentially return a value that does not
>> reflect the actual state of the LDO5 output.
> 
> Can you fix the regulator driver ?

So far, I didn't see a way how to do it, but with your suggestion below
it might work.

> 
>> In our case, after booting U-Boot we read 1.8V from the regulator
>> while in fact as the VSELECT signal is still low the regulator outputs
>> 3.3V. This confusion causes the MMC driver to think it is dealing with
>> a 1.8V-only device. This in turn leads to SD cards being addressed
>> with 1.8V IO levels even if UHS support is not available or disabled.
>>
>> Some cards with UHS support still work even if they are addressed
>> with 1.8V levels in non-UHS modes, but a lot of cards also fail with
>> timeout errors like:
>>
>>    Card did not respond to voltage select! : -110
>>
>> As a workaorund we disable the vqmmc regulator for now so we can make
>> sure no wrong values are read from the regulator. The switching
>> between 1.8V and 3.3V still works as the ESDHC driver sets the
>> VSELECT signal accordingly.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>> ---
>> By the way: I suspect that other boards using the PCA9450 might also
>> be affected
>> by this. I didn't find a nice generic solution so far. It would be
>> possible to
>> patch the pca9450 driver to use the PCA9450_LDO5CTRL_L instead of the
>> PCA9450_LDO5CTRL_H register for LDO5. This would fix this particular
>> case,
>> but still not the root problem of the regulator driver returning wrong
>> values.
>> So if anyone got some idea how to properly handle this, let me know.
>> The same issue is also present in Linux. While I didn't notice any
>> problems
>> with the SD card being addressed with incorrect voltage levels so far,
>> reading the regulator doesn't return the correct value if VSELECT is low.
> 
> Configure VSELECT pinmux such that it is a function, as it is right now,
> MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT or similar, (so SD controller
> driver can operate the pin via SD controller), but set SION bit so GPIO
> controller can read its state (activate bit 30), i.e.:
> MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT 0x40000004
> 
> Use sd-vsel-gpios property of PMIC, claim the VSELECT as GPIO in PMIC
> driver (this won't change pinmux, the pin would still be configured as
> function, but SION bit would allow you to read its state), read its
> state, and return the correct LDO5CTRL_H/LDO5CTRL_L value based on that.

Good idea, I will try that.

> 
> If this works, then:
>     arm64: dts: imx8mp: Drop sd-vsel-gpios from *
> Linux kernel patches should not be applied.

Yes, but we could also just revert this change once the fix suggested
above works and is in place. Until then the sd-vsel-gpios doesn't do
anything.

Thanks
Frieder

Re: [PATCH] imx: kontron-sl-mx8mm: Fix SD card IO voltage level

[Marek Vasut]

On 1/25/23 18:30, Frieder Schrempf wrote:
> Hi Marek,

Hi,

> On 25.01.23 18:15, Marek Vasut wrote:
>> On 1/25/23 14:41, Frieder Schrempf wrote:
>>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> Subject tags should be ARM: dts: imx:
> 
> Ok, how do I know? Because "git log arch/arm/dts" shows me that
> "<platform>: <board>:" is used quite often and it's what I used in the
> past. But I'm happy to change it if "ARM: dts: imx:" is the way the
> prefix should look like.

I just do what Linux does to keep things aligned.

>>> The LDO5 of the PCA9450 PMIC can be switched between two different
>>> voltage settings (defaulting to 1.8V and 3.3V) using an external
>>> signal SD_VSEL that is connected to the VSELECT signal of the SD
>>> card interface.
>>>
>>> As the regulator driver can't deal with both LDO registers (LDO5CTRL_H
>>> and LDO5CTRL_L) it only uses one of them, which means reading the
>>> voltage from the regulator can potentially return a value that does not
>>> reflect the actual state of the LDO5 output.
>>
>> Can you fix the regulator driver ?
> 
> So far, I didn't see a way how to do it, but with your suggestion below
> it might work.
> 
>>
>>> In our case, after booting U-Boot we read 1.8V from the regulator
>>> while in fact as the VSELECT signal is still low the regulator outputs
>>> 3.3V. This confusion causes the MMC driver to think it is dealing with
>>> a 1.8V-only device. This in turn leads to SD cards being addressed
>>> with 1.8V IO levels even if UHS support is not available or disabled.
>>>
>>> Some cards with UHS support still work even if they are addressed
>>> with 1.8V levels in non-UHS modes, but a lot of cards also fail with
>>> timeout errors like:
>>>
>>>     Card did not respond to voltage select! : -110
>>>
>>> As a workaorund we disable the vqmmc regulator for now so we can make
>>> sure no wrong values are read from the regulator. The switching
>>> between 1.8V and 3.3V still works as the ESDHC driver sets the
>>> VSELECT signal accordingly.
>>>
>>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>>> ---
>>> By the way: I suspect that other boards using the PCA9450 might also
>>> be affected
>>> by this. I didn't find a nice generic solution so far. It would be
>>> possible to
>>> patch the pca9450 driver to use the PCA9450_LDO5CTRL_L instead of the
>>> PCA9450_LDO5CTRL_H register for LDO5. This would fix this particular
>>> case,
>>> but still not the root problem of the regulator driver returning wrong
>>> values.
>>> So if anyone got some idea how to properly handle this, let me know.
>>> The same issue is also present in Linux. While I didn't notice any
>>> problems
>>> with the SD card being addressed with incorrect voltage levels so far,
>>> reading the regulator doesn't return the correct value if VSELECT is low.
>>
>> Configure VSELECT pinmux such that it is a function, as it is right now,
>> MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT or similar, (so SD controller
>> driver can operate the pin via SD controller), but set SION bit so GPIO
>> controller can read its state (activate bit 30), i.e.:
>> MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT 0x40000004
>>
>> Use sd-vsel-gpios property of PMIC, claim the VSELECT as GPIO in PMIC
>> driver (this won't change pinmux, the pin would still be configured as
>> function, but SION bit would allow you to read its state), read its
>> state, and return the correct LDO5CTRL_H/LDO5CTRL_L value based on that.
> 
> Good idea, I will try that.
> 
>>
>> If this works, then:
>>      arm64: dts: imx8mp: Drop sd-vsel-gpios from *
>> Linux kernel patches should not be applied.
> 
> Yes, but we could also just revert this change once the fix suggested
> above works and is in place. Until then the sd-vsel-gpios doesn't do
> anything.

Please give it a go, let's see if that's the way to go.

Re: [PATCH] imx: kontron-sl-mx8mm: Fix SD card IO voltage level

[Frieder Schrempf]

On 25.01.23 19:02, Marek Vasut wrote:
> On 1/25/23 18:30, Frieder Schrempf wrote:
>> Hi Marek,
> 
> Hi,
> 
>> On 25.01.23 18:15, Marek Vasut wrote:
>>> On 1/25/23 14:41, Frieder Schrempf wrote:
>>>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>
>>> Subject tags should be ARM: dts: imx:
>>
>> Ok, how do I know? Because "git log arch/arm/dts" shows me that
>> "<platform>: <board>:" is used quite often and it's what I used in the
>> past. But I'm happy to change it if "ARM: dts: imx:" is the way the
>> prefix should look like.
> 
> I just do what Linux does to keep things aligned.

Linux uses "arm64: dts:", but this doesn't match the directory structure
in U-Boot. Anyway keeping it the same as in Linux is probably best as
lots of DT patches are ported back and forth between the Linux and
U-Boot tree.

> 
>>>> The LDO5 of the PCA9450 PMIC can be switched between two different
>>>> voltage settings (defaulting to 1.8V and 3.3V) using an external
>>>> signal SD_VSEL that is connected to the VSELECT signal of the SD
>>>> card interface.
>>>>
>>>> As the regulator driver can't deal with both LDO registers (LDO5CTRL_H
>>>> and LDO5CTRL_L) it only uses one of them, which means reading the
>>>> voltage from the regulator can potentially return a value that does not
>>>> reflect the actual state of the LDO5 output.
>>>
>>> Can you fix the regulator driver ?
>>
>> So far, I didn't see a way how to do it, but with your suggestion below
>> it might work.
>>
>>>
>>>> In our case, after booting U-Boot we read 1.8V from the regulator
>>>> while in fact as the VSELECT signal is still low the regulator outputs
>>>> 3.3V. This confusion causes the MMC driver to think it is dealing with
>>>> a 1.8V-only device. This in turn leads to SD cards being addressed
>>>> with 1.8V IO levels even if UHS support is not available or disabled.
>>>>
>>>> Some cards with UHS support still work even if they are addressed
>>>> with 1.8V levels in non-UHS modes, but a lot of cards also fail with
>>>> timeout errors like:
>>>>
>>>>     Card did not respond to voltage select! : -110
>>>>
>>>> As a workaorund we disable the vqmmc regulator for now so we can make
>>>> sure no wrong values are read from the regulator. The switching
>>>> between 1.8V and 3.3V still works as the ESDHC driver sets the
>>>> VSELECT signal accordingly.
>>>>
>>>> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>> ---
>>>> By the way: I suspect that other boards using the PCA9450 might also
>>>> be affected
>>>> by this. I didn't find a nice generic solution so far. It would be
>>>> possible to
>>>> patch the pca9450 driver to use the PCA9450_LDO5CTRL_L instead of the
>>>> PCA9450_LDO5CTRL_H register for LDO5. This would fix this particular
>>>> case,
>>>> but still not the root problem of the regulator driver returning wrong
>>>> values.
>>>> So if anyone got some idea how to properly handle this, let me know.
>>>> The same issue is also present in Linux. While I didn't notice any
>>>> problems
>>>> with the SD card being addressed with incorrect voltage levels so far,
>>>> reading the regulator doesn't return the correct value if VSELECT is
>>>> low.
>>>
>>> Configure VSELECT pinmux such that it is a function, as it is right now,
>>> MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT or similar, (so SD controller
>>> driver can operate the pin via SD controller), but set SION bit so GPIO
>>> controller can read its state (activate bit 30), i.e.:
>>> MX8MP_IOMUXC_GPIO1_IO04__USDHC2_VSELECT 0x40000004
>>>
>>> Use sd-vsel-gpios property of PMIC, claim the VSELECT as GPIO in PMIC
>>> driver (this won't change pinmux, the pin would still be configured as
>>> function, but SION bit would allow you to read its state), read its
>>> state, and return the correct LDO5CTRL_H/LDO5CTRL_L value based on that.
>>
>> Good idea, I will try that.
>>
>>>
>>> If this works, then:
>>>      arm64: dts: imx8mp: Drop sd-vsel-gpios from *
>>> Linux kernel patches should not be applied.
>>
>> Yes, but we could also just revert this change once the fix suggested
>> above works and is in place. Until then the sd-vsel-gpios doesn't do
>> anything.
> 
> Please give it a go, let's see if that's the way to go.

So far it looks like this would work. I came up with these patches here
[1]. I don't know if I really should send fixes for other boards (maybe
as RFC). It looks like no one else has reported such problems on other
boards so far and I can't really tell if all are affected the same way
as our Kontron boards.

So if anyone can do some tests on their hardware that would be great.

And there is also still some work to do for applying the same changes in
Linux.

[1] https://github.com/fschrempf/u-boot/commits/pca9450-sd-vsel-fixups