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

[AngeloGioacchino Del Regno]

Il 03/12/24 11:51, Frank Wunderlich ha scritto:
> Am 3. Dezember 2024 10:40:54 MEZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>:
> > Il 02/12/24 13:25, Frank Wunderlich ha scritto:
> > > From: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx>
> > >
> > > Add regulator nodes used for mmc to Bananapi R4 board.
> > >
> > > Signed-off-by: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx>
> >
> > 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