Message-ID: <ce378faf-8446-448f-97cf-f40bc5c4581d@kontron.de>
Date: Wed, 23 Apr 2025 13:22:35 +0200
From: Frieder Schrempf <frieder.schrempf@...tron.de>
To: Francesco Dolcini <francesco@...cini.it>
Cc: Wojciech Dubowik <Wojciech.Dubowik@...com>, linux-kernel@...r.kernel.org,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
 Sascha Hauer <s.hauer@...gutronix.de>,
 Pengutronix Kernel Team <kernel@...gutronix.de>,
 Fabio Estevam <festevam@...il.com>, devicetree@...r.kernel.org,
 imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
 Philippe Schenker <philippe.schenker@...ulsing.ch>, stable@...r.kernel.org
Subject: Re: [PATCH v2] arm64: dts: imx8mm-verdin: Link reg_usdhc2_vqmmc to
 usdhc2

Am 23.04.25 um 12:26 schrieb Francesco Dolcini:
> On Wed, Apr 23, 2025 at 10:00:22AM +0200, Frieder Schrempf wrote:
>> Am 23.04.25 um 09:08 schrieb Francesco Dolcini:
>>> On Wed, Apr 23, 2025 at 08:50:54AM +0200, Frieder Schrempf wrote:
>>>> Am 22.04.25 um 14:46 schrieb Wojciech Dubowik:
>>>>>
>>>>> Define vqmmc regulator-gpio for usdhc2 with vin-supply
>>>>> coming from LDO5.
>>>>>
>>>>> 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@...r.kernel.org
>>>>> Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@...com>
> 
> ...
> 
>>> With this solution (that I proposed), the sdcard driver just use the
>>> GPIO to select the right voltage and that's it, simple, no un-needed i2c
>>> communication with the PMIC, and the DT clearly describe the way the HW
>>> is designed.
>>
>> Yes, but your solution relies on the fact that the LDO5 registers
>> actually have the correct values for 1v8 and 3v3 setup. The bootloader
>> might have changed these values. I would prefer it if we could have a
>> solution that puts the LDO5 in a defined state, that is independent from
>> any external conditions.
> 
> I do not think this is a real concern, the PMIC is programmed during
> manufacturing, if the PMIC programming is not correct we have way more
> issues ...

My point is not about the PMIC having wrong values as factory defaults.
My point is about different bootloaders that have PMIC drivers which
also use a mix of the SD_VSEL IO and the configuration registers for
setting the voltage. We don't know how the bootloader will leave the
register values behind.

An example would be that the bootloader uses SD_VSEL in a different way
and the PMIC driver in the bootloader writes 1v8 to the LDO5CTRL_L
register. Linux will then use the wrong voltage and the SD card will not
work.

So with your approach it would be good if the PMIC driver would also
reset the LDO5 registers to their factory defaults at probe time.

Also the logic for the LDO5 is purely embedded in the PMIC chip, so it
feels kind of wrong to have another regulator for SD_VSEL on the board
level.

If someone wants to check the output voltage of LDO5, they will query
the sysfs path for LDO5 and get back the wrong voltage. It will be hard
to find out that you need to read the voltage of the additional GPIO
regulator.

I don't think your approach is bad and of course you are free to move on
and use it. I'm just trying to find out what would be the best way for
everyone. It would be good to use the same approach on all i.MX8M
boards. Currently we have a mix of (at least):

1. MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT without sd-vsel-gpios readback
(everyone)
2. MX8MM_IOMUXC_GPIO1_IO04_USDHC2_VSELECT with sd-vsel-gpios readback
(Kontron)
3. MX8MM_IOMUXC_GPIO1_IO04_GPIO1_IO4 with additional GPIO regulator
(Toradex)

Powered by blists - more mailing lists