RE: [PATCH v2 7/8] arm64: dts: renesas: rzg3e-smarc-som: Add support for enable SD on SDHI0

[Biju Das <biju.das.jz@xxxxxxxxxxxxxx>]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: 06 February 2025 09:47
> Subject: Re: [PATCH v2 7/8] arm64: dts: renesas: rzg3e-smarc-som: Add support for enable SD on SDHI0
> 
> Hi Biju,
> 
> On Fri, 31 Jan 2025 at 12:25, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > Add support for enabling SD on SDHI0 on RZ/G3E SMARC SoM. It is
> > enabled by setting the macro SW_SD0_DEV_SEL to 1 in board DTS and
> > setting the switch SYS.1 to ON position on the SoM.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> > --- a/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi
> > @@ -5,6 +5,15 @@
> >   * Copyright (C) 2024 Renesas Electronics Corp.
> >   */
> >
> > +/*
> > + * Please set the switch position SYS.1 on the SoM and the
> > +corresponding macro
> > + * SW_SD0_DEV_SEL on the board DTS:
> > + *
> > + * SW_SD0_DEV_SEL:
> > + *      0 - SD0 is connected to eMMC (default)
> > + *      1 - SD0 is connected to uSD0 card
> > + */
> > +
> >  / {
> >         compatible = "renesas,rzg3e-smarcm", "renesas,r9a09g047e57",
> > "renesas,r9a09g047";
> >
> > @@ -43,6 +52,34 @@ &audio_extal_clk {
> >  };
> >
> >  &pinctrl {
> > +#if (SW_SD0_DEV_SEL)
> 
> No need to make this conditional.

OK.

> 
> > +       sdhi0_pins: sd0 {
> 
> sdhi0_usd_pins: sd0-usd {

OK.

> 
> > +               sd0-cd {
> > +                       pinmux = <RZG3E_PORT_PINMUX(5, 0, 8)>;
> > +               };
> > +
> > +               sd0-ctrl {
> > +                       pins = "SD0CLK", "SD0CMD";
> > +                       renesas,output-impedance = <3>;
> > +               };
> > +
> > +               sd0-data {
> > +                       pins = "SD0DAT0", "SD0DAT1", "SD0DAT2", "SD0DAT3";
> > +                       renesas,output-impedance = <3>;
> > +               };
> > +
> > +               sd0-iovs {
> > +                       pins = "SD0IOVS";
> > +                       renesas,output-impedance = <3>;
> > +               };
> > +
> > +               sd0-pwen {
> > +                       pins = "SD0PWEN";
> > +                       renesas,output-impedance = <3>;
> > +               };
> > +       };
> > +#endif
> > +
> >         sdhi0_emmc_pins: sd0emmc {
> >                 sd0-emmc-ctrl {
> >                         pins = "SD0CLK", "SD0CMD"; @@ -96,6 +133,25 @@
> > &rtxin_clk {
> >         clock-frequency = <32768>;
> >  };
> >
> > +#if (SW_SD0_DEV_SEL)
> > +&sdhi0 {
> > +       pinctrl-0 = <&sdhi0_pins>;
> > +       pinctrl-1 = <&sdhi0_pins>;
> > +       pinctrl-names = "default", "state_uhs";
> > +
> > +       vmmc-supply = <&reg_3p3v>;
> > +       vqmmc-supply = <&vqmmc_sdhi0>;
> > +       bus-width = <4>;
> > +       sd-uhs-sdr50;
> > +       sd-uhs-sdr104;
> > +       status = "okay";
> > +};
> > +
> > +&vqmmc_sdhi0 {
> > +       regulator-name = "SD0_PVDD";
> 
> Why override the name?

Will remove as you suggested.

Cheers,
Biju