linux-kernel - Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading SD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b900238d-b06a-a9d7-6892-6a726603b63b@denx.de>
Date:   Mon, 13 Feb 2023 21:59:27 +0100
From:   Marek Vasut <marex@...x.de>
To:     Marco Felsch <m.felsch@...gutronix.de>
Cc:     Frieder Schrempf <frieder@...s.de>, devicetree@...r.kernel.org,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Rob Herring <robh+dt@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Shawn Guo <shawnguo@...nel.org>,
        Frieder Schrempf <frieder.schrempf@...tron.de>,
        Oleksij Rempel <linux@...pel-privat.de>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        NXP Linux Team <linux-imx@....com>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Heiko Thiery <heiko.thiery@...il.com>,
        Fabio Estevam <festevam@...il.com>
Subject: Re: [PATCH 6/6] arm64: dts: imx8mm-kontron: Add support for reading
 SD_VSEL signal

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.