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

[Andre Przywara <andre.przywara@arm.com>]

On Mon, 24 Feb 2025 12:17:42 -0600
Chris Morgan <macromorgan@hotmail.com> wrote:

> 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.

As Jernej mentioned, this is due to subtle hardware limitations in the
Allwinner SoC. I think SDIO and/or the SDIO IRQs only work on MMC1, and
MMC2 is typically the only one with 8 data lines.
So this order is actually expected.

> That way the WiFi at least
> when present is always predictably at the mmc2 alias.

Why would you want that, exactly? I do understand the desire to have fixed
names for the storage devices, but for the WiFi? Doesn't the networking
device get named independently from the MMC device number?

And, FYI: if you really want the aliases, just pull this DT into U-Boot,
and use U-Boot's DT copy when booting the kernel, as U-Boot's build system
adds those MMC aliases automatically (since it's somewhat required for U-Boot's own operation):

> >
> > >           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).
>
> > Best regards,
> > Jernej
> >
> > >   status = "okay";
> > >  };
> > >
> > >
> >
> >
> >
> >
>
> Please let me know about setting alias mmc2, thank you.
> Chris
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.