Re: [PATCH v2 03/18] arm64: dts: mediatek: mt7988: add fixed regulators for 1v8 and 3v3

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 03/12/24 11:51, Frank Wunderlich ha scritto:
> Am 3. Dezember 2024 10:40:54 MEZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>> Il 02/12/24 13:25, Frank Wunderlich ha scritto:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>
>>> Add regulator nodes used for mmc to Bananapi R4 board.
>>>
>>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>>
>> If you're adding the eMMC regulators, I assume that there's an eMMC, or that even
>> if there's no device in some version of the board, these two regulators are always
>> assigned to the eMMC device - right?
> 
> Like BPi-R3 there is one mmc controller mapped to either sd or emmc. I can move the regulators to the dtbo,but then they are defined twice (at least 3v3).
> 
>> You should, at this point, do exactly that: you can leave the mmc0 disabled and
>> only enable it in a DTBO.
> 
> I enable it only in dtbo...
> 
>> &mmc0 {
>> 	/* eMMC gets enabled by DTBO if present */
>> 	vmmc-supply = <&reg_3p3v>;
>> 	vqmmc-supply = <&reg_1p8v>;
>> };
> 
> Problem is here that sdcard uses only the 3v3 twice
> 
> https://github.com/frank-w/BPI-Router-Linux/blob/6.12-main/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4-sd.dtso
> 
> And emmc needs both
> 
> https://github.com/frank-w/BPI-Router-Linux/blob/6.12-main/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4-emmc.dtso
> 
> Only defining the vmmc-supply in dts and set only the other in dtso is possible but but imho not nice.
> 

If you added that in the commit description in the first place, I wouldn't
have sent you that suggestion ;-)

So, please add the reason that you just explained to me in the description
of this commit and just leave the code as-is.

Cheers!

>> Cheers,
>> Angelo
>>
>>> ---
>>> v2:
>>> - move mmc regulators to board dts of bpi-r4
>>> ---
>>>    .../dts/mediatek/mt7988a-bananapi-bpi-r4.dts   | 18 ++++++++++++++++++
>>>    1 file changed, 18 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dts b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dts
>>> index d914eae2b524..df53512c6890 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dts
>>> +++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dts
>>> @@ -8,6 +8,24 @@ / {
>>>    	compatible = "bananapi,bpi-r4", "mediatek,mt7988a";
>>>    	model = "Banana Pi BPI-R4";
>>>    	chassis-type = "embedded";
>>> +
>>> +	reg_1p8v: regulator-1p8v {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "fixed-1.8V";
>>> +		regulator-min-microvolt = <1800000>;
>>> +		regulator-max-microvolt = <1800000>;
>>> +		regulator-boot-on;
>>> +		regulator-always-on;
>>> +	};
>>> +
>>> +	reg_3p3v: regulator-3p3v {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "fixed-3.3V";
>>> +		regulator-min-microvolt = <3300000>;
>>> +		regulator-max-microvolt = <3300000>;
>>> +		regulator-boot-on;
>>> +		regulator-always-on;
>>> +	};
>>>    };
>>>      &pio {
>>
>>
> 
> 
> regards Frank