[PATCH] arm64: dts: imx8mm-verdin: Link reg_nvcc_sd to usdhc2

[PATCH] arm64: dts: imx8mm-verdin: Link reg_nvcc_sd to usdhc2

[Wojciech Dubowik]

Link LDO5 labeled reg_nvcc_sd from PMIC to align with
hardware configuration specified in the datasheet.

Without this definition LDO5 will be powered down, disabling
SD card after bootup. This has been introduced in commit
f5aab0438ef1 (regulator: pca9450: Fix enable register for LDO5).

Fixes: f5aab0438ef1 (regulator: pca9450: Fix enable register for LDO5)
Cc: stable@vger.kernel.org

Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
---
 arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
index 7251ad3a0017..6307c5caf3bc 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
@@ -785,6 +785,7 @@ &usdhc2 {
 	pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_cd>;
 	pinctrl-3 = <&pinctrl_usdhc2_sleep>, <&pinctrl_usdhc2_cd_sleep>;
 	vmmc-supply = <&reg_usdhc2_vmmc>;
+	vqmmc-supply = <&reg_nvcc_sd>;
 };
 
 &wdog1 {
-- 
2.47.2

Re: [PATCH] arm64: dts: imx8mm-verdin: Link reg_nvcc_sd to usdhc2

[Francesco Dolcini]

Hello Wojciech,
thanks very much for your patch.

On Thu, Apr 17, 2025 at 01:20:11PM +0200, Wojciech Dubowik wrote:
> Link LDO5 labeled reg_nvcc_sd from PMIC to align with
> hardware configuration specified in the datasheet.
> 
> Without this definition LDO5 will be powered down, disabling
> SD card after bootup. This has been introduced in commit
> f5aab0438ef1 (regulator: pca9450: Fix enable register for LDO5).
> 
> Fixes: f5aab0438ef1 (regulator: pca9450: Fix enable register for LDO5)
> Cc: stable@vger.kernel.org
> 
> Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> index 7251ad3a0017..6307c5caf3bc 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> @@ -785,6 +785,7 @@ &usdhc2 {
>  	pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_cd>;
>  	pinctrl-3 = <&pinctrl_usdhc2_sleep>, <&pinctrl_usdhc2_cd_sleep>;
>  	vmmc-supply = <&reg_usdhc2_vmmc>;
> +	vqmmc-supply = <&reg_nvcc_sd>;

I am worried just doing this will have some side effects.

Before this patch, the switch between 1v8 and 3v3 was done because we
have a GPIO, connected to the PMIC, controlled by the USDHC2 instance
(MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT, see pinctrl_usdhc2).

With your change both the PMIC will be programmed with a different
voltage over i2c and the GPIO will also toggle. It does not sound like
what we want to do.

Maybe we should have a "regulator-gpio" with vin-supply =
<&reg_nvcc_sd>, as we recently did here
https://lore.kernel.org/all/20250414123827.428339-1-ivitro@gmail.com/T/#m2964f1126a6732a66a6e704812f2b786e8237354
?

Francesco

Re: [PATCH] arm64: dts: imx8mm-verdin: Link reg_nvcc_sd to usdhc2

[Dubowik Wojciech LCPF-CH]

> From: Francesco Dolcini <francesco@dolcini.it>
> Sent: Thursday, 17 April 2025 15:03
> To: Dubowik Wojciech LCPF-CH <Wojciech.Dubowik@mt.com>
> Cc: linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>; Pengutronix Kernel Team > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; devicetree@vger.kernel.org <devicetree@vger.kernel.org>; imx@lists.linux.dev <imx@lists.linux.dev>; linux-arm-kernel@lists.infradead.org <linux-arm-kernel@lists.infradead.org>; stable@vger.kernel.org <stable@vger.kernel.org>
> Subject: Re: EXTERNAL - [PATCH] arm64: dts: imx8mm-verdin: Link reg_nvcc_sd to usdhc2
 
> Hello Wojciech,
> thanks very much for your patch.

> On Thu, Apr 17, 2025 at 01:20:11PM +0200, Wojciech Dubowik wrote:
>> Link LDO5 labeled reg_nvcc_sd from PMIC to align with
>> hardware configuration specified in the datasheet.
>>
>> Without this definition LDO5 will be powered down, disabling
>> SD card after bootup. This has been introduced in commit
>> f5aab0438ef1 (regulator: pca9450: Fix enable register for LDO5).
>>
>> Fixes: f5aab0438ef1 (regulator: pca9450: Fix enable register for LDO5)
>> Cc: stable@vger.kernel.org
>>
>> Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
>> ---
>>  arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
>> index 7251ad3a0017..6307c5caf3bc 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
>> @@ -785,6 +785,7 @@ &usdhc2 {
>>        pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_cd>;
>>        pinctrl-3 = <&pinctrl_usdhc2_sleep>, <&pinctrl_usdhc2_cd_sleep>;
>>        vmmc-supply = <&reg_usdhc2_vmmc>;
>> +     vqmmc-supply = <&reg_nvcc_sd>;
>
>I am worried just doing this will have some side effects.
>
>Before this patch, the switch between 1v8 and 3v3 was done because we
>have a GPIO, connected to the PMIC, controlled by the USDHC2 instance
>(MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT, see pinctrl_usdhc2).
>
>With your change both the PMIC will be programmed with a different
>voltage over i2c and the GPIO will also toggle. It does not sound like
>what we want to do.
>
>Maybe we should have a "regulator-gpio" with vin-supply =
><&reg_nvcc_sd>, as we recently did here
>https://lore.kernel.org/all/20250414123827.428339-1-ivitro@gmail.com/T/#m2964f1126a6732a66a6e704812f2b786e8237354
>?

>Francesco
I will have a look at your suggestion and try to test them on verdin. I won't have access to HW over Eastern so the patch will have to wait.

Wojtek

Re: [PATCH] arm64: dts: imx8mm-verdin: Link reg_nvcc_sd to usdhc2

[Philippe Schenker]

[-- Attachment #1: Type: text/plain, Size: 2601 bytes --]

Hi Francesco,

Just for your awareness, I stumbled on this patch this morning and I
actually did the exact same thing on verdin-imx8mp a while ago:
c8d29601fea3080a42731e8535b929a93afa107e

Not sure this causes any side-effects maybe you guys want to
investigate further about this. I needed it due to the strange
requirements I had (described in commit message).
From my point of view it is correct to link the vqmmc-supply so the
voltage can be set also to something different than the default fusing
values.

Philippe

On Thu, 2025-04-17 at 15:03 +0200, Francesco Dolcini wrote:
> Hello Wojciech,
> thanks very much for your patch.
> 
> On Thu, Apr 17, 2025 at 01:20:11PM +0200, Wojciech Dubowik wrote:
> > Link LDO5 labeled reg_nvcc_sd from PMIC to align with
> > hardware configuration specified in the datasheet.
> > 
> > Without this definition LDO5 will be powered down, disabling
> > SD card after bootup. This has been introduced in commit
> > f5aab0438ef1 (regulator: pca9450: Fix enable register for LDO5).
> > 
> > Fixes: f5aab0438ef1 (regulator: pca9450: Fix enable register for
> > LDO5)
> > Cc: stable@vger.kernel.org
> > 
> > Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@mt.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> > index 7251ad3a0017..6307c5caf3bc 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> > @@ -785,6 +785,7 @@ &usdhc2 {
> >   pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_cd>;
> >   pinctrl-3 = <&pinctrl_usdhc2_sleep>, <&pinctrl_usdhc2_cd_sleep>;
> >   vmmc-supply = <&reg_usdhc2_vmmc>;
> > + vqmmc-supply = <&reg_nvcc_sd>;
> 
> I am worried just doing this will have some side effects.
> 
> Before this patch, the switch between 1v8 and 3v3 was done because we
> have a GPIO, connected to the PMIC, controlled by the USDHC2 instance
> (MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT, see pinctrl_usdhc2).
> 
> With your change both the PMIC will be programmed with a different
> voltage over i2c and the GPIO will also toggle. It does not sound
> like
> what we want to do.
> 
> Maybe we should have a "regulator-gpio" with vin-supply =
> <&reg_nvcc_sd>, as we recently did here
> https://lore.kernel.org/all/20250414123827.428339-1-
> ivitro@gmail.com/T/#m2964f1126a6732a66a6e704812f2b786e8237354
> ?
> 
> Francesco
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 717 bytes --]

Re: [PATCH] arm64: dts: imx8mm-verdin: Link reg_nvcc_sd to usdhc2

[Francesco Dolcini]

On Tue, Apr 22, 2025 at 07:57:32AM +0000, Philippe Schenker wrote:
> Hi Francesco,
Hey Philippe!

> Not sure this causes any side-effects maybe you guys want to
> investigate further about this.

Yes, we did, the correct implementation would be the one I linked
in the previous email.

> I needed it due to the strange requirements I had (described in commit
> message).  From my point of view it is correct to link the vqmmc-supply so
> the voltage can be set also to something different than the default fusing
> values.

It does not really work fine, because you have this IO driven by the SDHCI
core that is going to affect the PMIC behavior at the same time as the I2C
communication. And even if you remove it from the pinctrl, it's the default
out-of-reset function, so you would have to override it and set this pin as
GPIO even when not used (this would work, of course).

My request is to fix it in a slighlty different way that matches with the way
the HW was designed.

Re: [PATCH] arm64: dts: imx8mm-verdin: Link reg_nvcc_sd to usdhc2

[Philippe Schenker]

[-- Attachment #1: Type: text/plain, Size: 1614 bytes --]

On Tue, 2025-04-22 at 10:41 +0200, Francesco Dolcini wrote:
> On Tue, Apr 22, 2025 at 07:57:32AM +0000, Philippe Schenker wrote:
> > Hi Francesco,
> Hey Philippe!
> 
> > Not sure this causes any side-effects maybe you guys want to
> > investigate further about this.
> 
> Yes, we did, the correct implementation would be the one I linked
> in the previous email.

Interesting, thanks for your further explanation. I do understand now.
The implementation with your SMARC module indeed looks much cleaner and
is better to understand what happens.

Please let me know if you wish to remove the vqmmc linkage on i.MX 8MP
-> I would need to test for side effects on my side. Please use the old
mail for it as I already hijacked this one enough :-).

https://lore.kernel.org/all/20240109121627.223017-1-dev@pschenker.ch/

> 
> > I needed it due to the strange requirements I had (described in
> > commit
> > message).  From my point of view it is correct to link the vqmmc-
> > supply so
> > the voltage can be set also to something different than the default
> > fusing
> > values.
> 
> It does not really work fine, because you have this IO driven by the
> SDHCI
> core that is going to affect the PMIC behavior at the same time as
> the I2C
> communication. And even if you remove it from the pinctrl, it's the
> default
> out-of-reset function, so you would have to override it and set this
> pin as
> GPIO even when not used (this would work, of course).
> 
> 
> My request is to fix it in a slighlty different way that matches with
> the way
> the HW was designed.
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 717 bytes --]