Separate out Rev.2.0 specific hardware changes into
hihope-common-rev2.dtsi file so that hihope-common.dtsi can be used
by all the variants for RZ/G2M[N] boards.
Signed-off-by: Lad Prabhakar <[email protected]>
Reviewed-by: Marian-Cristian Rotariu <[email protected]>
---
.../boot/dts/renesas/hihope-common-rev2.dtsi | 101 ++++++++++++++++++
.../arm64/boot/dts/renesas/hihope-common.dtsi | 87 +--------------
.../renesas/r8a774a1-hihope-rzg2m-rev2.dts | 1 +
.../renesas/r8a774b1-hihope-rzg2n-rev2.dts | 1 +
4 files changed, 105 insertions(+), 85 deletions(-)
create mode 100644 arch/arm64/boot/dts/renesas/hihope-common-rev2.dtsi
diff --git a/arch/arm64/boot/dts/renesas/hihope-common-rev2.dtsi b/arch/arm64/boot/dts/renesas/hihope-common-rev2.dtsi
new file mode 100644
index 000000000000..66c34fc9e7a5
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/hihope-common-rev2.dtsi
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree Source for the HiHope RZ/G2[MN] main board Rev.2.0 common
+ * parts
+ *
+ * Copyright (C) 2020 Renesas Electronics Corp.
+ */
+
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+ leds {
+ compatible = "gpio-leds";
+
+ bt_active_led {
+ label = "blue:bt";
+ gpios = <&gpio7 0 GPIO_ACTIVE_HIGH>;
+ linux,default-trigger = "hci0-power";
+ default-state = "off";
+ };
+
+ led0 {
+ gpios = <&gpio6 11 GPIO_ACTIVE_HIGH>;
+ };
+
+ led1 {
+ gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
+ };
+
+ led2 {
+ gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>;
+ };
+
+ led3 {
+ gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
+ };
+
+ wlan_active_led {
+ label = "yellow:wlan";
+ gpios = <&gpio7 1 GPIO_ACTIVE_HIGH>;
+ linux,default-trigger = "phy0tx";
+ default-state = "off";
+ };
+ };
+
+ wlan_en_reg: regulator-wlan_en {
+ compatible = "regulator-fixed";
+ regulator-name = "wlan-en-regulator";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ startup-delay-us = <70000>;
+
+ gpio = <&gpio_expander 1 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+};
+
+&hscif0 {
+ bluetooth {
+ compatible = "ti,wl1837-st";
+ enable-gpios = <&gpio_expander 2 GPIO_ACTIVE_HIGH>;
+ };
+};
+
+&i2c4 {
+ gpio_expander: gpio@20 {
+ compatible = "onnn,pca9654";
+ reg = <0x20>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+};
+
+&pfc {
+ sound_clk_pins: sound_clk {
+ groups = "audio_clk_a_a";
+ function = "audio_clk";
+ };
+};
+
+&rcar_sound {
+ pinctrl-0 = <&sound_clk_pins>;
+ pinctrl-names = "default";
+
+ status = "okay";
+
+ /* Single DAI */
+ #sound-dai-cells = <0>;
+
+ rsnd_port: port {
+ rsnd_endpoint: endpoint {
+ remote-endpoint = <&dw_hdmi0_snd_in>;
+
+ dai-format = "i2s";
+ bitclock-master = <&rsnd_endpoint>;
+ frame-master = <&rsnd_endpoint>;
+
+ playback = <&ssi2>;
+ };
+ };
+};
diff --git a/arch/arm64/boot/dts/renesas/hihope-common.dtsi b/arch/arm64/boot/dts/renesas/hihope-common.dtsi
index bd056904e8bd..4efcdebd8842 100644
--- a/arch/arm64/boot/dts/renesas/hihope-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/hihope-common.dtsi
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Device Tree Source for the HiHope RZ/G2[MN] main board common parts
+ * Device Tree Source for the HiHope RZ/G2[MN] main board
+ * Rev.[2.0/3.0/4.0] common parts
*
* Copyright (C) 2019 Renesas Electronics Corp.
*/
@@ -29,40 +30,6 @@
};
};
- leds {
- compatible = "gpio-leds";
-
- bt_active_led {
- label = "blue:bt";
- gpios = <&gpio7 0 GPIO_ACTIVE_HIGH>;
- linux,default-trigger = "hci0-power";
- default-state = "off";
- };
-
- led0 {
- gpios = <&gpio6 11 GPIO_ACTIVE_HIGH>;
- };
-
- led1 {
- gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
- };
-
- led2 {
- gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>;
- };
-
- led3 {
- gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
- };
-
- wlan_active_led {
- label = "yellow:wlan";
- gpios = <&gpio7 1 GPIO_ACTIVE_HIGH>;
- linux,default-trigger = "phy0tx";
- default-state = "off";
- };
- };
-
reg_1p8v: regulator0 {
compatible = "regulator-fixed";
regulator-name = "fixed-1.8V";
@@ -112,17 +79,6 @@
states = <3300000 1>, <1800000 0>;
};
- wlan_en_reg: regulator-wlan_en {
- compatible = "regulator-fixed";
- regulator-name = "wlan-en-regulator";
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <1800000>;
- startup-delay-us = <70000>;
-
- gpio = <&gpio_expander 1 GPIO_ACTIVE_HIGH>;
- enable-active-high;
- };
-
x302_clk: x302-clock {
compatible = "fixed-clock";
#clock-cells = <0>;
@@ -194,11 +150,6 @@
uart-has-rtscts;
status = "okay";
-
- bluetooth {
- compatible = "ti,wl1837-st";
- enable-gpios = <&gpio_expander 2 GPIO_ACTIVE_HIGH>;
- };
};
&hsusb {
@@ -210,13 +161,6 @@
clock-frequency = <400000>;
status = "okay";
- gpio_expander: gpio@20 {
- compatible = "onnn,pca9654";
- reg = <0x20>;
- gpio-controller;
- #gpio-cells = <2>;
- };
-
versaclock5: clock-generator@6a {
compatible = "idt,5p49v5923";
reg = <0x6a>;
@@ -281,11 +225,6 @@
power-source = <1800>;
};
- sound_clk_pins: sound_clk {
- groups = "audio_clk_a_a";
- function = "audio_clk";
- };
-
usb0_pins: usb0 {
groups = "usb0";
function = "usb0";
@@ -309,28 +248,6 @@
};
};
-&rcar_sound {
- pinctrl-0 = <&sound_clk_pins>;
- pinctrl-names = "default";
-
- status = "okay";
-
- /* Single DAI */
- #sound-dai-cells = <0>;
-
- rsnd_port: port {
- rsnd_endpoint: endpoint {
- remote-endpoint = <&dw_hdmi0_snd_in>;
-
- dai-format = "i2s";
- bitclock-master = <&rsnd_endpoint>;
- frame-master = <&rsnd_endpoint>;
-
- playback = <&ssi2>;
- };
- };
-};
-
&rwdt {
timeout-sec = <60>;
status = "okay";
diff --git a/arch/arm64/boot/dts/renesas/r8a774a1-hihope-rzg2m-rev2.dts b/arch/arm64/boot/dts/renesas/r8a774a1-hihope-rzg2m-rev2.dts
index 276f449ad14e..e7eea3225abe 100644
--- a/arch/arm64/boot/dts/renesas/r8a774a1-hihope-rzg2m-rev2.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774a1-hihope-rzg2m-rev2.dts
@@ -8,6 +8,7 @@
/dts-v1/;
#include "r8a774a1.dtsi"
#include "hihope-common.dtsi"
+#include "hihope-common-rev2.dtsi"
/ {
model = "HopeRun HiHope RZ/G2M main board (Rev.2.0) based on r8a774a1";
diff --git a/arch/arm64/boot/dts/renesas/r8a774b1-hihope-rzg2n-rev2.dts b/arch/arm64/boot/dts/renesas/r8a774b1-hihope-rzg2n-rev2.dts
index eb1206b2d97a..ecb527356f37 100644
--- a/arch/arm64/boot/dts/renesas/r8a774b1-hihope-rzg2n-rev2.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774b1-hihope-rzg2n-rev2.dts
@@ -8,6 +8,7 @@
/dts-v1/;
#include "r8a774b1.dtsi"
#include "hihope-common.dtsi"
+#include "hihope-common-rev2.dtsi"
/ {
model = "HopeRun HiHope RZ/G2N main board (Rev.2.0) based on r8a774b1";
--
2.17.1
Hi Prabhakar,
Thanks for your patch!
On Sun, Jun 7, 2020 at 8:41 PM Lad Prabhakar
<[email protected]> wrote:
> Separate out Rev.2.0 specific hardware changes into
> hihope-common-rev2.dtsi file so that hihope-common.dtsi can be used
> by all the variants for RZ/G2M[N] boards.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> Reviewed-by: Marian-Cristian Rotariu <[email protected]>
Reviewed-by: Geert Uytterhoeven <[email protected]>
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/hihope-common-rev2.dtsi
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree Source for the HiHope RZ/G2[MN] main board Rev.2.0 common
> + * parts
> + *
> + * Copyright (C) 2020 Renesas Electronics Corp.
> + */
> +
> +#include <dt-bindings/gpio/gpio.h>
What about adding
#include "hihope-common.dtsi"
here?
Then the *rev2.dts files have to include only "hihope-common-rev2.dtsi",
and get "hihope-common.dtsi" for free?
The same is true for the rev4.dtsi and the rev4.dts files.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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,
Thank you for the review.
On Mon, Jun 8, 2020 at 3:27 PM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar,
>
> Thanks for your patch!
>
> On Sun, Jun 7, 2020 at 8:41 PM Lad Prabhakar
> <[email protected]> wrote:
> > Separate out Rev.2.0 specific hardware changes into
> > hihope-common-rev2.dtsi file so that hihope-common.dtsi can be used
> > by all the variants for RZ/G2M[N] boards.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > Reviewed-by: Marian-Cristian Rotariu <[email protected]>
>
> Reviewed-by: Geert Uytterhoeven <[email protected]>
>
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/renesas/hihope-common-rev2.dtsi
> > @@ -0,0 +1,101 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Device Tree Source for the HiHope RZ/G2[MN] main board Rev.2.0 common
> > + * parts
> > + *
> > + * Copyright (C) 2020 Renesas Electronics Corp.
> > + */
> > +
> > +#include <dt-bindings/gpio/gpio.h>
>
> What about adding
>
> #include "hihope-common.dtsi"
>
> here?
> Then the *rev2.dts files have to include only "hihope-common-rev2.dtsi",
> and get "hihope-common.dtsi" for free?
>
> The same is true for the rev4.dtsi and the rev4.dts files.
>
Agreed.
Cheers,
--Prabhakar
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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 Mon, Jun 8, 2020 at 3:47 PM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Sun, Jun 7, 2020 at 8:41 PM Lad Prabhakar
> <[email protected]> wrote:
> > Separate out Rev.2.0 specific hardware changes into
> > hihope-common-rev2.dtsi file so that hihope-common.dtsi can be used
> > by all the variants for RZ/G2M[N] boards.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > Reviewed-by: Marian-Cristian Rotariu <[email protected]>
>
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/renesas/hihope-common-rev2.dtsi
>
> Perhaps just hihope-rev2.dtsi, i.e. without the "common-"?
>
Yes makes sense.
> > @@ -0,0 +1,101 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Device Tree Source for the HiHope RZ/G2[MN] main board Rev.2.0 common
> > + * parts
> > + *
> > + * Copyright (C) 2020 Renesas Electronics Corp.
> > + */
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +
> > +/ {
> > + leds {
> > + compatible = "gpio-leds";
> > +
> > + bt_active_led {
> > + label = "blue:bt";
> > + gpios = <&gpio7 0 GPIO_ACTIVE_HIGH>;
> > + linux,default-trigger = "hci0-power";
> > + default-state = "off";
> > + };
> > +
> > + led0 {
> > + gpios = <&gpio6 11 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + led1 {
> > + gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + led2 {
> > + gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + led3 {
> > + gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
> > + };
>
> led1, led2, and led3 are present on both, so I'd keep them in
> hihope-common.dtsi.
>
The leds defined in hihope-common-rev4.dtsi are as per the label names
on the schematics/board so that it's easier to identify the LED's by
name.
> > +
> > + wlan_active_led {
> > + label = "yellow:wlan";
> > + gpios = <&gpio7 1 GPIO_ACTIVE_HIGH>;
> > + linux,default-trigger = "phy0tx";
> > + default-state = "off";
> > + };
> > + };
> > +
> > + wlan_en_reg: regulator-wlan_en {
> > + compatible = "regulator-fixed";
> > + regulator-name = "wlan-en-regulator";
> > + regulator-min-microvolt = <1800000>;
> > + regulator-max-microvolt = <1800000>;
> > + startup-delay-us = <70000>;
> > +
> > + gpio = <&gpio_expander 1 GPIO_ACTIVE_HIGH>;
> > + enable-active-high;
> > + };
>
> Same for the WLAN regulator, especially as it is referenced from
> hihope-common.dtsi.
> As the GPIO line differs between the two variants, you just need
> to add the gpio property in the revision-specific file.
>
Agreed will move this to common.
> > +};
> > +
> > +&hscif0 {
> > + bluetooth {
> > + compatible = "ti,wl1837-st";
> > + enable-gpios = <&gpio_expander 2 GPIO_ACTIVE_HIGH>;
> > + };
> > +};
>
> As node is small, and the GPIO line differs from the two variants,
> I think duplicating it in both revision-specific files is fine, though.
>
Agreed.
Cheers,
--Prabhakar
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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 Sun, Jun 7, 2020 at 8:41 PM Lad Prabhakar
<[email protected]> wrote:
> Separate out Rev.2.0 specific hardware changes into
> hihope-common-rev2.dtsi file so that hihope-common.dtsi can be used
> by all the variants for RZ/G2M[N] boards.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> Reviewed-by: Marian-Cristian Rotariu <[email protected]>
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/hihope-common-rev2.dtsi
Perhaps just hihope-rev2.dtsi, i.e. without the "common-"?
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree Source for the HiHope RZ/G2[MN] main board Rev.2.0 common
> + * parts
> + *
> + * Copyright (C) 2020 Renesas Electronics Corp.
> + */
> +
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> + leds {
> + compatible = "gpio-leds";
> +
> + bt_active_led {
> + label = "blue:bt";
> + gpios = <&gpio7 0 GPIO_ACTIVE_HIGH>;
> + linux,default-trigger = "hci0-power";
> + default-state = "off";
> + };
> +
> + led0 {
> + gpios = <&gpio6 11 GPIO_ACTIVE_HIGH>;
> + };
> +
> + led1 {
> + gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
> + };
> +
> + led2 {
> + gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>;
> + };
> +
> + led3 {
> + gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
> + };
led1, led2, and led3 are present on both, so I'd keep them in
hihope-common.dtsi.
> +
> + wlan_active_led {
> + label = "yellow:wlan";
> + gpios = <&gpio7 1 GPIO_ACTIVE_HIGH>;
> + linux,default-trigger = "phy0tx";
> + default-state = "off";
> + };
> + };
> +
> + wlan_en_reg: regulator-wlan_en {
> + compatible = "regulator-fixed";
> + regulator-name = "wlan-en-regulator";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + startup-delay-us = <70000>;
> +
> + gpio = <&gpio_expander 1 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
Same for the WLAN regulator, especially as it is referenced from
hihope-common.dtsi.
As the GPIO line differs between the two variants, you just need
to add the gpio property in the revision-specific file.
> +};
> +
> +&hscif0 {
> + bluetooth {
> + compatible = "ti,wl1837-st";
> + enable-gpios = <&gpio_expander 2 GPIO_ACTIVE_HIGH>;
> + };
> +};
As node is small, and the GPIO line differs from the two variants,
I think duplicating it in both revision-specific files is fine, though.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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 Mon, Jun 8, 2020 at 3:59 PM Lad, Prabhakar
<[email protected]> wrote:
>
> Hi Geert,
>
> On Mon, Jun 8, 2020 at 3:47 PM Geert Uytterhoeven <[email protected]> wrote:
> >
> > Hi Prabhakar,
> >
> > On Sun, Jun 7, 2020 at 8:41 PM Lad Prabhakar
> > <[email protected]> wrote:
> > > Separate out Rev.2.0 specific hardware changes into
> > > hihope-common-rev2.dtsi file so that hihope-common.dtsi can be used
> > > by all the variants for RZ/G2M[N] boards.
> > >
> > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > Reviewed-by: Marian-Cristian Rotariu <[email protected]>
> >
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/renesas/hihope-common-rev2.dtsi
> >
> > Perhaps just hihope-rev2.dtsi, i.e. without the "common-"?
> >
> Yes makes sense.
>
> > > @@ -0,0 +1,101 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Device Tree Source for the HiHope RZ/G2[MN] main board Rev.2.0 common
> > > + * parts
> > > + *
> > > + * Copyright (C) 2020 Renesas Electronics Corp.
> > > + */
> > > +
> > > +#include <dt-bindings/gpio/gpio.h>
> > > +
> > > +/ {
> > > + leds {
> > > + compatible = "gpio-leds";
> > > +
> > > + bt_active_led {
> > > + label = "blue:bt";
> > > + gpios = <&gpio7 0 GPIO_ACTIVE_HIGH>;
> > > + linux,default-trigger = "hci0-power";
> > > + default-state = "off";
> > > + };
> > > +
> > > + led0 {
> > > + gpios = <&gpio6 11 GPIO_ACTIVE_HIGH>;
> > > + };
> > > +
> > > + led1 {
> > > + gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
> > > + };
> > > +
> > > + led2 {
> > > + gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>;
> > > + };
> > > +
> > > + led3 {
> > > + gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
> > > + };
> >
> > led1, led2, and led3 are present on both, so I'd keep them in
> > hihope-common.dtsi.
> >
> The leds defined in hihope-common-rev4.dtsi are as per the label names
> on the schematics/board so that it's easier to identify the LED's by
> name.
>
I was waiting on the above to be confirmed.
Cheers,
--Prabhakar
> > > +
> > > + wlan_active_led {
> > > + label = "yellow:wlan";
> > > + gpios = <&gpio7 1 GPIO_ACTIVE_HIGH>;
> > > + linux,default-trigger = "phy0tx";
> > > + default-state = "off";
> > > + };
> > > + };
> > > +
> > > + wlan_en_reg: regulator-wlan_en {
> > > + compatible = "regulator-fixed";
> > > + regulator-name = "wlan-en-regulator";
> > > + regulator-min-microvolt = <1800000>;
> > > + regulator-max-microvolt = <1800000>;
> > > + startup-delay-us = <70000>;
> > > +
> > > + gpio = <&gpio_expander 1 GPIO_ACTIVE_HIGH>;
> > > + enable-active-high;
> > > + };
> >
> > Same for the WLAN regulator, especially as it is referenced from
> > hihope-common.dtsi.
> > As the GPIO line differs between the two variants, you just need
> > to add the gpio property in the revision-specific file.
> >
> Agreed will move this to common.
>
> > > +};
> > > +
> > > +&hscif0 {
> > > + bluetooth {
> > > + compatible = "ti,wl1837-st";
> > > + enable-gpios = <&gpio_expander 2 GPIO_ACTIVE_HIGH>;
> > > + };
> > > +};
> >
> > As node is small, and the GPIO line differs from the two variants,
> > I think duplicating it in both revision-specific files is fine, though.
> >
> Agreed.
>
> Cheers,
> --Prabhakar
>
> > Gr{oetje,eeting}s,
> >
> > Geert
> >
> > --
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
> >
> > 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 Tue, Jun 23, 2020 at 9:51 AM Lad, Prabhakar
<[email protected]> wrote:
> On Mon, Jun 8, 2020 at 3:59 PM Lad, Prabhakar
> <[email protected]> wrote:
> > On Mon, Jun 8, 2020 at 3:47 PM Geert Uytterhoeven <[email protected]> wrote:
> > > On Sun, Jun 7, 2020 at 8:41 PM Lad Prabhakar
> > > <[email protected]> wrote:
> > > > Separate out Rev.2.0 specific hardware changes into
> > > > hihope-common-rev2.dtsi file so that hihope-common.dtsi can be used
> > > > by all the variants for RZ/G2M[N] boards.
> > > >
> > > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > > Reviewed-by: Marian-Cristian Rotariu <[email protected]>
> > > > @@ -0,0 +1,101 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Device Tree Source for the HiHope RZ/G2[MN] main board Rev.2.0 common
> > > > + * parts
> > > > + *
> > > > + * Copyright (C) 2020 Renesas Electronics Corp.
> > > > + */
> > > > +
> > > > +#include <dt-bindings/gpio/gpio.h>
> > > > +
> > > > +/ {
> > > > + leds {
> > > > + compatible = "gpio-leds";
> > > > +
> > > > + bt_active_led {
> > > > + label = "blue:bt";
> > > > + gpios = <&gpio7 0 GPIO_ACTIVE_HIGH>;
> > > > + linux,default-trigger = "hci0-power";
> > > > + default-state = "off";
> > > > + };
> > > > +
> > > > + led0 {
> > > > + gpios = <&gpio6 11 GPIO_ACTIVE_HIGH>;
> > > > + };
> > > > +
> > > > + led1 {
> > > > + gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
> > > > + };
> > > > +
> > > > + led2 {
> > > > + gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>;
> > > > + };
> > > > +
> > > > + led3 {
> > > > + gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
> > > > + };
> > >
> > > led1, led2, and led3 are present on both, so I'd keep them in
> > > hihope-common.dtsi.
> > >
> > The leds defined in hihope-common-rev4.dtsi are as per the label names
> > on the schematics/board so that it's easier to identify the LED's by
> > name.
> >
> I was waiting on the above to be confirmed.
I can confirm the naming of the LEDs on the rev4 board.
However, following the same reasoning, the rev2 LEDs should be renamed
led2201, led2202, led2203, and led2402 ;-)
Does anyone rely on the names? If not, it may make sense to use the
rev4 names for both, in the common file?
Not even considering the switches...
Seems they forgot to rename switches SW220[123] when renaming LED220[123].
Worse, on rev2, you have SW220_2_/LED220_1_ sharing a GPIO, and
SW220_1_/LED220_2_ sharing another one.
And on rev4, GP6_11/GP_LED/TSW_0_ is driving LED_4_ and SW220_2_?
Conclusion: I don't care how you name them ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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 Tue, Jun 23, 2020 at 9:14 AM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Tue, Jun 23, 2020 at 9:51 AM Lad, Prabhakar
> <[email protected]> wrote:
> > On Mon, Jun 8, 2020 at 3:59 PM Lad, Prabhakar
> > <[email protected]> wrote:
> > > On Mon, Jun 8, 2020 at 3:47 PM Geert Uytterhoeven <[email protected]> wrote:
> > > > On Sun, Jun 7, 2020 at 8:41 PM Lad Prabhakar
> > > > <[email protected]> wrote:
> > > > > Separate out Rev.2.0 specific hardware changes into
> > > > > hihope-common-rev2.dtsi file so that hihope-common.dtsi can be used
> > > > > by all the variants for RZ/G2M[N] boards.
> > > > >
> > > > > Signed-off-by: Lad Prabhakar <[email protected]>
> > > > > Reviewed-by: Marian-Cristian Rotariu <[email protected]>
>
> > > > > @@ -0,0 +1,101 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * Device Tree Source for the HiHope RZ/G2[MN] main board Rev.2.0 common
> > > > > + * parts
> > > > > + *
> > > > > + * Copyright (C) 2020 Renesas Electronics Corp.
> > > > > + */
> > > > > +
> > > > > +#include <dt-bindings/gpio/gpio.h>
> > > > > +
> > > > > +/ {
> > > > > + leds {
> > > > > + compatible = "gpio-leds";
> > > > > +
> > > > > + bt_active_led {
> > > > > + label = "blue:bt";
> > > > > + gpios = <&gpio7 0 GPIO_ACTIVE_HIGH>;
> > > > > + linux,default-trigger = "hci0-power";
> > > > > + default-state = "off";
> > > > > + };
> > > > > +
> > > > > + led0 {
> > > > > + gpios = <&gpio6 11 GPIO_ACTIVE_HIGH>;
> > > > > + };
> > > > > +
> > > > > + led1 {
> > > > > + gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
> > > > > + };
> > > > > +
> > > > > + led2 {
> > > > > + gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>;
> > > > > + };
> > > > > +
> > > > > + led3 {
> > > > > + gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
> > > > > + };
> > > >
> > > > led1, led2, and led3 are present on both, so I'd keep them in
> > > > hihope-common.dtsi.
> > > >
> > > The leds defined in hihope-common-rev4.dtsi are as per the label names
> > > on the schematics/board so that it's easier to identify the LED's by
> > > name.
> > >
> > I was waiting on the above to be confirmed.
>
> I can confirm the naming of the LEDs on the rev4 board.
> However, following the same reasoning, the rev2 LEDs should be renamed
> led2201, led2202, led2203, and led2402 ;-)
I didn't want to change any behaviour if some was using the LED's with names.
> Does anyone rely on the names? If not, it may make sense to use the
> rev4 names for both, in the common file?
>
Not sure, but I'll take your suggestion and just name them as per rev4 naming.
> Not even considering the switches...
> Seems they forgot to rename switches SW220[123] when renaming LED220[123].
> Worse, on rev2, you have SW220_2_/LED220_1_ sharing a GPIO, and
> SW220_1_/LED220_2_ sharing another one.
>
> And on rev4, GP6_11/GP_LED/TSW_0_ is driving LED_4_ and SW220_2_?
>
> Conclusion: I don't care how you name them ;-)
>
:)
Cheers,
--Prabhakar
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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