Re: [PATCH 1/4] arm64: dts: allwinner: h700: Add MMC2 for Anbernic RG35XX

["Jernej Škrabec" <jernej.skrabec@gmail.com>]

Dne ponedeljek, 24. februar 2025 ob 19:17:42 Srednjeevropski standardni čas je Chris Morgan napisal(a):
> On Sat, Feb 22, 2025 at 11:44:39AM +0100, Jernej Škrabec wrote:
> > Hi Chris,
> > 
> > sorry it took so long.
> > 
> > Dne petek, 18. oktober 2024 ob 18:06:14 Srednjeevropski standardni čas je Chris Morgan napisal(a):
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > Add support for the second MMC slot on the Anbernic RG35XX series.
> > > The second MMC card is connected to MMC2 (WiFi if present is MMC1).
> > > The MMC logic is powered by cldo3 via the power domain to which
> > > it is connected, and also has an external 3.3v GPIO controlled
> > > regulator required for functionality.
> > > 
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > ---
> > >  .../sun50i-h700-anbernic-rg35xx-2024.dts      | 27 +++++++++++++++++--
> > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts b/arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
> > > index 80ccab7b5ba7..e2039fd76b3d 100644
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-2024.dts
> > > @@ -18,6 +18,9 @@ / {
> > >  	compatible = "anbernic,rg35xx-2024", "allwinner,sun50i-h700";
> > >  
> > >  	aliases {
> > > +		mmc0 = &mmc0;
> > > +		mmc1 = &mmc2;
> > > +		mmc2 = &mmc1;
> > 
> > Please drop those. As a rule, we don't have it in any DT, since there are more
> > universal to identify root partition than a relying on device order.
> 
> Would it be okay at least to keep the mmc2 alias mapped to physical
> device mmc1? Confusingly enough, this device is such that mmc0 is the
> first mmc card, mmc1 is the SDIO wifi card (not always present depending
> on device), and mmc2 is the second mmc card. That way the WiFi at least
> when present is always predictably at the mmc2 alias.

This is standard for all Allwinner SoCs:

mmc0 -> SD card
mmc1 -> SDIO, more or less only wifi modules
mmc2 -> eMMC or SD card

Such ordering is due to mmc controller capabilities. And no, no exceptions
for labels.

> 
> 
> > 
> > >  		serial0 = &uart0;
> > >  	};
> > >  
> > > @@ -175,6 +178,15 @@ reg_vcc5v: regulator-vcc5v { /* USB-C power input */
> > >  		regulator-min-microvolt = <5000000>;
> > >  		regulator-max-microvolt = <5000000>;
> > >  	};
> > > +
> > > +	reg_vcc3v3_mmc2: regulator-vcc3v3-mmc2 {
> > > +		compatible = "regulator-fixed";
> > > +		enable-active-high;
> > > +		gpio = <&pio 4 4 GPIO_ACTIVE_HIGH>; /* PE4 */
> > > +		regulator-min-microvolt = <3300000>;
> > > +		regulator-max-microvolt = <3300000>;
> > > +		regulator-name = "vcc3v3-mmc2";
> > > +	};
> > >  };
> > >  
> > >  &cpu0 {
> > > @@ -186,10 +198,21 @@ &ehci0 {
> > >  };
> > >  
> > >  &mmc0 {
> > > -	vmmc-supply = <&reg_cldo3>;
> > > -	disable-wp;
> > > +	bus-width = <4>;
> > >  	cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>;  /* PF6 */
> > > +	disable-wp;
> > > +	no-1-8-v;
> > 
> > Please leave properties in same order to minimize patch size. From
> > what I can see, only above property is added, which is not needed anyway.
> > 
> 
> Acknowledged. I just figured I'd alphabetize them, but you're right
> it's a bit anal.
> 
> > > +	vmmc-supply = <&reg_cldo3>;
> > > +	status = "okay";
> > > +};
> > > +
> > > +&mmc2 {
> > >  	bus-width = <4>;
> > > +	cd-gpios = <&pio 4 22 GPIO_ACTIVE_LOW>; /* PE22 */
> > > +	disable-wp;
> > > +	no-1-8-v;
> > > +	vmmc-supply = <&reg_vcc3v3_mmc2>;
> > > +	vqmmc-supply = <&reg_cldo3>;
> > 
> > Is this another SD card slot or eMMC? You configured it as it would be SD card,
> > but mmc2 slots are usually used for eMMC, which needs different configuration.
> > 
> 
> Physical bus mmc0 is the TF1 slot, physical bus mmc2 is the TF2 slot,
> and physical bus mmc1 is the SDIO wifi card (which is present on most
> but not all devices in the 35xx series).

Interesting, 2 SD card slots and no eMMC. That's a first for me.

Best regards,
Jernej
 
> > >  	status = "okay";
> > >  };
> > >  
> > > 
> > 
> > 
> > 
> > 
> 
> Please let me know about setting alias mmc2, thank you.
> Chris
>