Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal

[Marek Vasut]

On 2/13/23 20:56, Marco Felsch wrote:
> Hi Marek, Frieder,

Hi,

> On 23-02-13, Marek Vasut wrote:
> > On 2/13/23 17:15, Marco Felsch wrote:
> >
> > [...]
> >
> > > > @@ -347,7 +347,7 @@ MX8MM_IOMUXC_SD2_DATA1_USDHC2_DATA1		0x1d6
> > > >    			MX8MM_IOMUXC_SD2_DATA2_USDHC2_DATA2		0x1d6
> > > >    			MX8MM_IOMUXC_SD2_DATA3_USDHC2_DATA3		0x1d6
> > > >    			MX8MM_IOMUXC_SD2_CD_B_GPIO2_IO12		0x019
> > > > -			MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT		0x1d0
> > > > +			MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT		0x400001d0
> > >
> > > The VSELECT pin should be driven by the (u)sdhc core...
> > >
> > > >    		>;
> > > >    	};
> > > >    };
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > > index 5172883717d1..90daaf54e704 100644
> > > > --- a/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mm-kontron-osm-s.dtsi
> > > > @@ -196,6 +196,7 @@ reg_nvcc_sd: LDO5 {
> > > >    				regulator-name = "NVCC_SD (LDO5)";
> > > >    				regulator-min-microvolt = <1800000>;
> > > >    				regulator-max-microvolt = <3300000>;
> > > > +				sd-vsel-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> > >
> > > and by using the sd-vsel-gpios property the IOMUXC_GPIO1_IO04 have to be
> > > muxed as GPIO, which is not the case. So I think that u-boot have a bug
> > > within the (u)sdhc core.
> >
> > The trick here is that the VSELECT is operated by the usdhc block as a
> > function pin, but the PMIC driver can read the current state of the VSELECT
> > pin by reading out the GPIO block SR register. Since the IOMUX SION bit is
> > set on the VSELECT pin, the state of the pin is reflected in the GPIO block
> > SR register even if the pin is muxed as function pin.
>
> Thanks for this explanation :) Why does the regulator driver need to
> know the current state of this pin?

Because that regulator has an input pin which selects between two states 
of that regulator, L and H, and whatever L or H is depends on what is 
configured into the regulator via I2C. To correctly report the state of 
the regulator, you have to know the state of that input (selector) pin.

> Since the voltage switching requires
> some cmd's before the actual voltage level switch. So this must be
> handled within the core.
>
> Also after checking the driver, adding the sd-vsel-gpios will request
> the specified gpio as output-high.

The GPIO would have to be requested as input, obviously.

> Out of curiosity, what's the bug you
> triggering within U-Boot?

AFAICT the readback of the initial state of the regulator (see paragraph 
above), which affects Linux all the same.