Message ID | 20220915165256.352843-3-prabhakar.mahadev-lad.rj@bp.renesas.com |
---|---|
State | New |
Headers | show |
Series | None | expand |
Hi Geert, Thank you for the review. On Tue, Oct 25, 2022 at 9:13 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Thu, Sep 15, 2022 at 6:53 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > In preparation to re-use the RZ/G2UL SMARC SoM and carrier DTS/I with the > > RZ/Five add /omit-if-no-ref/ keyword to pinmux entries as the support for > > RZ/Five SMARC EVK will be gradually added. > > > > Once we have full blown support for RZ/Five SMARC EVK we can get rid of > > the /omit-if-no-ref/ keyword. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Thanks for your patch! > > I finally had a deeper look at this... > > Why do you want to disable these nodes? While they are indeed not > used yet on RZ/Five, they are valid hardware descriptions for the > RZ/Five SMARC EVK, and their presence doesn't harm anything. > > I do see a valid use case for marking pin control subnodes with > /omit-if-no-ref/: you can provide all possible configurations as a > convenience for the user, so the user no longer has to look up the > numeric parameters of the RZG2L_PORT_PINMUX() macros. > But IMHO those would belong in the SoC-specific .dtsi, not in a > board .dtsi. See e.g. the massive use of /omit-if-no-ref/ in sunxi > and rockchip .dtsi files. > > Am I missing something? > My intention was to keep the DTB as minimal as possible so that it includes just the required pinmuxes which were enabled on the RZ/Five. For example [0], [1] we do delete the pinctrl for the nodes which are marked as disabled. Do you think we should drop it? But now that things are falling in place for RZ/Five we can ignore this patch. [0] arch/arm64/boot/dts/renesas/rzg2lc-smarc.dtsi [1] arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi Cheers, Prabhakar > > --- a/arch/arm64/boot/dts/renesas/rzg2ul-smarc-pinfunction.dtsi > > +++ b/arch/arm64/boot/dts/renesas/rzg2ul-smarc-pinfunction.dtsi > > @@ -12,12 +12,14 @@ &pinctrl { > > pinctrl-0 = <&sound_clk_pins>; > ^^^^^^^^^^^^^^ > > pinctrl-names = "default"; > > > > + /omit-if-no-ref/ > > can0_pins: can0 { > > pinmux = <RZG2L_PORT_PINMUX(1, 1, 3)>, /* TX */ > > <RZG2L_PORT_PINMUX(1, 2, 3)>; /* RX */ > > }; > > > > #if (SW_ET0_EN_N) > > + /omit-if-no-ref/ > > can0-stb-hog { > > gpio-hog; > > gpios = <RZG2L_GPIO(2, 2) GPIO_ACTIVE_HIGH>; > > > @@ -94,11 +103,13 @@ sd1_mux_uhs { > > }; > > }; > > > > + /omit-if-no-ref/ > > sound_clk_pins: sound_clk { > > FTR, this one is always referenced. > > > pins = "AUDIO_CLK1", "AUDIO_CLK2"; > > input-enable; > > }; > > 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
Hi Prabhakar, On Wed, Oct 26, 2022 at 12:39 AM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Tue, Oct 25, 2022 at 9:13 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Thu, Sep 15, 2022 at 6:53 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > In preparation to re-use the RZ/G2UL SMARC SoM and carrier DTS/I with the > > > RZ/Five add /omit-if-no-ref/ keyword to pinmux entries as the support for > > > RZ/Five SMARC EVK will be gradually added. > > > > > > Once we have full blown support for RZ/Five SMARC EVK we can get rid of > > > the /omit-if-no-ref/ keyword. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Thanks for your patch! > > > > I finally had a deeper look at this... > > > > Why do you want to disable these nodes? While they are indeed not > > used yet on RZ/Five, they are valid hardware descriptions for the > > RZ/Five SMARC EVK, and their presence doesn't harm anything. > > > > I do see a valid use case for marking pin control subnodes with > > /omit-if-no-ref/: you can provide all possible configurations as a > > convenience for the user, so the user no longer has to look up the > > numeric parameters of the RZG2L_PORT_PINMUX() macros. > > But IMHO those would belong in the SoC-specific .dtsi, not in a > > board .dtsi. See e.g. the massive use of /omit-if-no-ref/ in sunxi > > and rockchip .dtsi files. > > > > Am I missing something? > > > My intention was to keep the DTB as minimal as possible so that it > includes just the required pinmuxes which were enabled on the RZ/Five. > For example [0], [1] we do delete the pinctrl for the nodes which are > marked as disabled. Do you think we should drop it? You mean /delete-property/ pinctrl-0; /delete-property/ pinctrl-names; ? These do not delete pinctrl subnodes, but pinctrl properties in disabled device nodes pointing to pinctrl subnodes. The actual pinctrl subnodes are still present. > But now that things are falling in place for RZ/Five we can ignore this patch. Agreed. > [0] arch/arm64/boot/dts/renesas/rzg2lc-smarc.dtsi > [1] arch/arm64/boot/dts/renesas/rzg2ul-smarc.dtsi 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
Hi Geert, On Wed, Oct 26, 2022 at 8:29 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Wed, Oct 26, 2022 at 12:39 AM Lad, Prabhakar > <prabhakar.csengg@gmail.com> wrote: > > On Tue, Oct 25, 2022 at 9:13 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > On Thu, Sep 15, 2022 at 6:53 PM Prabhakar <prabhakar.csengg@gmail.com> wrote: > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > > > In preparation to re-use the RZ/G2UL SMARC SoM and carrier DTS/I with the > > > > RZ/Five add /omit-if-no-ref/ keyword to pinmux entries as the support for > > > > RZ/Five SMARC EVK will be gradually added. > > > > > > > > Once we have full blown support for RZ/Five SMARC EVK we can get rid of > > > > the /omit-if-no-ref/ keyword. > > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > > > Thanks for your patch! > > > > > > I finally had a deeper look at this... > > > > > > Why do you want to disable these nodes? While they are indeed not > > > used yet on RZ/Five, they are valid hardware descriptions for the > > > RZ/Five SMARC EVK, and their presence doesn't harm anything. > > > > > > I do see a valid use case for marking pin control subnodes with > > > /omit-if-no-ref/: you can provide all possible configurations as a > > > convenience for the user, so the user no longer has to look up the > > > numeric parameters of the RZG2L_PORT_PINMUX() macros. > > > But IMHO those would belong in the SoC-specific .dtsi, not in a > > > board .dtsi. See e.g. the massive use of /omit-if-no-ref/ in sunxi > > > and rockchip .dtsi files. > > > > > > Am I missing something? > > > > > My intention was to keep the DTB as minimal as possible so that it > > includes just the required pinmuxes which were enabled on the RZ/Five. > > For example [0], [1] we do delete the pinctrl for the nodes which are > > marked as disabled. Do you think we should drop it? > > You mean > > /delete-property/ pinctrl-0; > /delete-property/ pinctrl-names; > > ? > These do not delete pinctrl subnodes, but pinctrl properties in disabled > device nodes pointing to pinctrl subnodes. The actual pinctrl subnodes > are still present. > Ahh right the pinctrl subnodes will still remain. Cheers, Prabhakar
diff --git a/arch/arm64/boot/dts/renesas/rzg2ul-smarc-pinfunction.dtsi b/arch/arm64/boot/dts/renesas/rzg2ul-smarc-pinfunction.dtsi index bd8bc858c28c..2b400100472e 100644 --- a/arch/arm64/boot/dts/renesas/rzg2ul-smarc-pinfunction.dtsi +++ b/arch/arm64/boot/dts/renesas/rzg2ul-smarc-pinfunction.dtsi @@ -12,12 +12,14 @@ &pinctrl { pinctrl-0 = <&sound_clk_pins>; pinctrl-names = "default"; + /omit-if-no-ref/ can0_pins: can0 { pinmux = <RZG2L_PORT_PINMUX(1, 1, 3)>, /* TX */ <RZG2L_PORT_PINMUX(1, 2, 3)>; /* RX */ }; #if (SW_ET0_EN_N) + /omit-if-no-ref/ can0-stb-hog { gpio-hog; gpios = <RZG2L_GPIO(2, 2) GPIO_ACTIVE_HIGH>; @@ -26,12 +28,14 @@ can0-stb-hog { }; #endif + /omit-if-no-ref/ can1_pins: can1 { pinmux = <RZG2L_PORT_PINMUX(2, 0, 3)>, /* TX */ <RZG2L_PORT_PINMUX(2, 1, 3)>; /* RX */ }; #if (SW_ET0_EN_N) + /omit-if-no-ref/ can1-stb-hog { gpio-hog; gpios = <RZG2L_GPIO(2, 3) GPIO_ACTIVE_HIGH>; @@ -40,11 +44,13 @@ can1-stb-hog { }; #endif + /omit-if-no-ref/ i2c0_pins: i2c0 { pins = "RIIC0_SDA", "RIIC0_SCL"; input-enable; }; + /omit-if-no-ref/ i2c1_pins: i2c1 { pins = "RIIC1_SDA", "RIIC1_SCL"; input-enable; @@ -55,6 +61,7 @@ scif0_pins: scif0 { <RZG2L_PORT_PINMUX(6, 3, 6)>; /* RxD */ }; + /omit-if-no-ref/ sd1-pwr-en-hog { gpio-hog; gpios = <RZG2L_GPIO(0, 3) GPIO_ACTIVE_HIGH>; @@ -62,6 +69,7 @@ sd1-pwr-en-hog { line-name = "sd1_pwr_en"; }; + /omit-if-no-ref/ sdhi1_pins: sd1 { sd1_data { pins = "SD1_DATA0", "SD1_DATA1", "SD1_DATA2", "SD1_DATA3"; @@ -78,6 +86,7 @@ sd1_mux { }; }; + /omit-if-no-ref/ sdhi1_pins_uhs: sd1_uhs { sd1_data_uhs { pins = "SD1_DATA0", "SD1_DATA1", "SD1_DATA2", "SD1_DATA3"; @@ -94,11 +103,13 @@ sd1_mux_uhs { }; }; + /omit-if-no-ref/ sound_clk_pins: sound_clk { pins = "AUDIO_CLK1", "AUDIO_CLK2"; input-enable; }; + /omit-if-no-ref/ ssi1_pins: ssi1 { pinmux = <RZG2L_PORT_PINMUX(3, 0, 2)>, /* BCK */ <RZG2L_PORT_PINMUX(3, 1, 2)>, /* RCK */ @@ -106,12 +117,14 @@ ssi1_pins: ssi1 { <RZG2L_PORT_PINMUX(3, 3, 2)>; /* RXD */ }; + /omit-if-no-ref/ usb0_pins: usb0 { pinmux = <RZG2L_PORT_PINMUX(5, 0, 1)>, /* VBUS */ <RZG2L_PORT_PINMUX(5, 2, 1)>, /* OVC */ <RZG2L_PORT_PINMUX(5, 3, 1)>; /* OTG_ID */ }; + /omit-if-no-ref/ usb1_pins: usb1 { pinmux = <RZG2L_PORT_PINMUX(5, 4, 5)>, /* OVC */ <RZG2L_PORT_PINMUX(6, 0, 1)>; /* VBUS */ diff --git a/arch/arm64/boot/dts/renesas/rzg2ul-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg2ul-smarc-som.dtsi index 2a0feb53f0dc..a0a41a062f55 100644 --- a/arch/arm64/boot/dts/renesas/rzg2ul-smarc-som.dtsi +++ b/arch/arm64/boot/dts/renesas/rzg2ul-smarc-som.dtsi @@ -132,10 +132,12 @@ &ostm2 { }; &pinctrl { + /omit-if-no-ref/ adc_pins: adc { pinmux = <RZG2L_PORT_PINMUX(6, 2, 1)>; /* ADC_TRG */ }; + /omit-if-no-ref/ eth0_pins: eth0 { pinmux = <RZG2L_PORT_PINMUX(4, 5, 1)>, /* ET0_LINKSTA */ <RZG2L_PORT_PINMUX(4, 3, 1)>, /* ET0_MDC */ @@ -154,6 +156,7 @@ eth0_pins: eth0 { <RZG2L_PORT_PINMUX(4, 1, 1)>; /* ET0_RXD3 */ }; + /omit-if-no-ref/ eth1_pins: eth1 { pinmux = <RZG2L_PORT_PINMUX(10, 4, 1)>, /* ET1_LINKSTA */ <RZG2L_PORT_PINMUX(10, 2, 1)>, /* ET1_MDC */ @@ -172,6 +175,7 @@ eth1_pins: eth1 { <RZG2L_PORT_PINMUX(10, 0, 1)>; /* ET1_RXD3 */ }; + /omit-if-no-ref/ sdhi0_emmc_pins: sd0emmc { sd0_emmc_data { pins = "SD0_DATA0", "SD0_DATA1", "SD0_DATA2", "SD0_DATA3", @@ -190,6 +194,7 @@ sd0_emmc_rst { }; }; + /omit-if-no-ref/ sdhi0_pins: sd0 { sd0_data { pins = "SD0_DATA0", "SD0_DATA1", "SD0_DATA2", "SD0_DATA3"; @@ -206,6 +211,7 @@ sd0_mux { }; }; + /omit-if-no-ref/ sdhi0_pins_uhs: sd0_uhs { sd0_data_uhs { pins = "SD0_DATA0", "SD0_DATA1", "SD0_DATA2", "SD0_DATA3"; @@ -222,6 +228,7 @@ sd0_mux_uhs { }; }; + /omit-if-no-ref/ spi1_pins: rspi1 { pinmux = <RZG2L_PORT_PINMUX(4, 0, 2)>, /* CK */ <RZG2L_PORT_PINMUX(4, 1, 2)>, /* MOSI */