Re: [PATCH 2/3] arm64: dts: renesas: rzg2l-smarc-som: Enable eMMC on SMARC platform

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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)

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

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

> +               gpio-hog;
> +               gpios = <RZG2L_GPIO(41, 1) GPIO_ACTIVE_HIGH>;
> +               output-high;
> +               line-name = "gpio_sd0_dev_sel";
> +       };
> +
> +       sd0_pwr_en {

gpio-sd0-pwr-en-hog (with "gpio-" prefix, to match schematics)

> +               gpio-hog;
> +               gpios = <RZG2L_GPIO(4, 1) GPIO_ACTIVE_HIGH>;
> +               output-high;
> +               line-name = "sd0_pwr_en";

gpio_sd0_pwr_en

> +       };

The rest looks good to me.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux