Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH 2/3] arm64: dts: renesas: rzg2l-smarc-som: Enable eMMC > on SMARC platform > > Hi Biju, > > On Thu, Oct 7, 2021 at 5:55 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > wrote: > > RZ/G2L SoM has both 64Gb eMMC and micro SD connected to SDHI0. > > > > Both these interfaces are mutually exclusive and the SD0 device > > selection is based on the XOR between GPIO_SD0_DEV_SEL and SW1[2] > > switch position. > > > > This patch sets GPIO_SD0_DEV_SEL to high in DT. Use the below switch > > setting logic for device selection between eMMC and microSD slot > > connected to SDHI0. > > > > Set SW1[2] to position 2/OFF for selecting eMMC Set SW1[2] to position > > 3/ON for selecting micro SD > > > > This patch enables eMMC on RZ/G2L SMARC platform by default. > > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > > Thanks for your patch! > > > > --- a/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi > > +++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi > > @@ -5,14 +5,55 @@ > > * Copyright (C) 2021 Renesas Electronics Corp. > > */ > > > > +#include <dt-bindings/gpio/gpio.h> > > #include <dt-bindings/pinctrl/rzg2l-pinctrl.h> > > > > +/* SW1[2] should be at position 2/OFF to enable 64Gb eMMC */ > > 64 GB (the MTFC64GASAQHD datasheet isn't clear about the exact size and > the meaning of GB) It is 64 GB. > > > +#define EMMC 1 > > + > > +/* > > + * To enable uSD card on CN3, > > + * SW1[2] should be at position 3/ON. > > + * Disable eMMC by setting "#define EMMC 0" above. > > + */ > > +#define SDHI (!EMMC) > > + > > / { > > memory@48000000 { > > device_type = "memory"; > > /* first 128MB is reserved for secure area. */ > > reg = <0x0 0x48000000 0x0 0x78000000>; > > }; > > + > > + reg_1p8v: regulator0 { > > + compatible = "regulator-fixed"; > > + regulator-name = "fixed-1.8V"; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <1800000>; > > + regulator-boot-on; > > + regulator-always-on; > > + }; > > + > > + reg_3p3v: regulator1 { > > + compatible = "regulator-fixed"; > > + regulator-name = "fixed-3.3V"; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + regulator-boot-on; > > + regulator-always-on; > > + }; > > + > > + vccq_sdhi0: regulator-vccq-sdhi0 { > > + compatible = "regulator-gpio"; > > + > > + regulator-name = "SDHI0 VccQ"; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <3300000>; > > + states = <3300000 1 1800000 0>; > > "make dtbs_check" says: > > regulator-vccq-sdhi0: states:0: [3300000, 1, 1800000, 0] is too long > > Please group the two states using angular brackets. OK. > > > + regulator-boot-on; > > + gpios = <&pinctrl RZG2L_GPIO(39, 0) GPIO_ACTIVE_HIGH>; > > + regulator-always-on; > > + }; > > }; > > > > &adc { > > @@ -32,4 +73,108 @@ > > adc_pins: adc { > > pinmux = <RZG2L_PORT_PINMUX(9, 0, 2)>; /* ADC_TRG */ > > }; > > + > > + /* > > + * SDO device selection is XOR between GPIO_SD0_DEV_SEL and > SW1[2] > > + * The below switch logic can be used to select the device > between > > + * eMMC and microSD, after setting GPIO_SD0_DEV_SEL to high in > DT. > > + * SW1[2] should be at position 2/OFF to enable 64Gb eMMC > > + * SW1[2] should be at position 3/ON to enable uSD card CN3 > > + */ > > + gpio_sd0_dev_sel { > > gpio_sd0_dev_sel: $nodename:0: 'gpio_sd0_dev_sel' does not match > '^(hog-[0-9]+|.+-hog(-[0-9]+)?)$' > (for all hogs) > > Please use e.g. "sd0-dev-sel-hog". OK. > > > + gpio-hog; > > + gpios = <RZG2L_GPIO(41, 1) GPIO_ACTIVE_HIGH>; > > + output-high; > > + line-name = "gpio_sd0_dev_sel"; OK. Here as well. > > + }; > > + > > + sd0_pwr_en { > > gpio-sd0-pwr-en-hog (with "gpio-" prefix, to match schematics) OK. > > > + gpio-hog; > > + gpios = <RZG2L_GPIO(4, 1) GPIO_ACTIVE_HIGH>; > > + output-high; > > + line-name = "sd0_pwr_en"; > > gpio_sd0_pwr_en OK. Will add this changes to next version. Regards, Biju > > > + }; > > The rest looks good to me. > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux- > m68k.org > > In personal conversations with technical people, I call myself a hacker. > But when I'm talking to journalists I just say "programmer" or something > like that. > -- Linus Torvalds