Re: [PATCH v3 6/8] DT: arm: Add Renesas RZ/N1 SoC base device tree file
Hi Simon, Michel, On Fri, Mar 30, 2018 at 09:25:16AM +0200, Simon Horman wrote: > On Thu, Mar 29, 2018 at 01:04:50PM +0200, jacopo mondi wrote: > > Hi Michel > > > > The subject of all your patches for arch/arm should start with: > > > > ARM: dts: > > > > A git log on that directory clearly shows that's the preferred one. > > > > I would also say that you are missing a symbol definition in > > arch/arm/mach-shmobile/Kconfig > > (even if you got rid of any board file) > > > > I would expect a symbol to select in menuconfig, with your > > dependencies listed there (ie, the serial interface driver) > > > > Something like this (I left the 'xx' out from the part name on purpose) > > > > diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig > > index 280e731..9a519330 100644 > > --- a/arch/arm/mach-shmobile/Kconfig > > +++ b/arch/arm/mach-shmobile/Kconfig > > @@ -114,4 +114,8 @@ config ARCH_SH73A0 > > bool "SH-Mobile AG5 (R8A73A00)" > > select ARCH_RMOBILE > > select RENESAS_INTC_IRQPIN > > + > > +config ARCH_R9A06G0 > > + bool "RZ/N1 (R9A06G0)" > > + select SERIAL_8250_DW > > endif > > > > But please wait for others (preferibly Geert or Simon) to confim this. > > I think that is covered by > "[PATCH v3 5/8] arm: shmobile: Add the RZ/N1 arch to the shmobile Kconfig" > I somehow didn't apply that patch from the series when reviewing. Sorry for the fuss. Thanks j > > On Thu, Mar 29, 2018 at 08:47:02AM +0100, Michel Pollet wrote: > > > This adds the Renesas RZ/N1 Family (Part #R9A06G0xx) SoC > > > bare bone support. > > > > > > This currently only handles generic parts (gic, architected timer) > > > and a UART. > > > For simplicity sake, this also relies on the bootloader to set the > > > pinctrl and clocks. > > > > > > Signed-off-by: Michel Pollet> > > --- > > > arch/arm/boot/dts/r9a06g0xx.dtsi | 96 > > > > > > 1 file changed, 96 insertions(+) > > > create mode 100644 arch/arm/boot/dts/r9a06g0xx.dtsi > > > > > > diff --git a/arch/arm/boot/dts/r9a06g0xx.dtsi > > > b/arch/arm/boot/dts/r9a06g0xx.dtsi > > > new file mode 100644 > > > index 000..c63 > > > --- /dev/null > > > +++ b/arch/arm/boot/dts/r9a06g0xx.dtsi > > > @@ -0,0 +1,96 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Base Device Tree Source for the Renesas RZ/N1 SoC Family of devices > > > + * > > > + * Copyright (C) 2018 Renesas Electronics Europe Limited > > > + * > > > + */ > > > + > > > +#include > > > + > > > +/ { > > > + compatible = "renesas,rzn1"; > > > + #address-cells = <1>; > > > + #size-cells = <1>; > > > + > > > + cpus { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + cpu@0 { > > > + device_type = "cpu"; > > > + compatible = "arm,cortex-a7"; > > > + reg = <0>; > > > + }; > > > + cpu@1 { > > > + device_type = "cpu"; > > > + compatible = "arm,cortex-a7"; > > > + reg = <1>; > > > + }; > > > + }; > > > > I see you don't like empy lines, that's fine, it is not a strict > > requiremen afaik, but I find a few empty lines here and there more > > redable, expecially if the file is going to grow, as it will be. > > Please place one empty line between each node. > > > > + clocks { > > > + /* > > > + * this is fixed clock for now, > > > + * until the clock driver is merged > > > + */ > > > + clkuarts: clkuarts { > > > > You can remove the node lable if it's the same as the node name afaik > > > > > + #clock-cells = <0>; > > > + compatible = "fixed-clock"; > > > + clock-frequency = <47619047>; > > > + }; > > > + }; > > > > Grouping clock nodes under a "clocks" one is now deprecated. > > > > Please see, ie. > > "ARM: dts: r7s72100: stop grouping clocks under a "clocks" subnode" > > Also, please sort sub-nodes of the root node alphabetically. > > > Thanks > >j > > > > > + arch-timer { > > > + compatible = "arm,cortex-a7-timer", > > > + "arm,armv7-timer"; > > > + interrupt-parent = <>; > > > + arm,cpu-registers-not-fw-configured; > > > + interrupts = > > > + > > + IRQ_TYPE_LEVEL_LOW)>, > > > + > > + IRQ_TYPE_LEVEL_LOW)>, > > > + > > + IRQ_TYPE_LEVEL_LOW)>, > > > + > > + IRQ_TYPE_LEVEL_LOW)>; > > I think its nicer not to line-wrap the individual interrupts. > I.e. please make the above like this: > > interrupts = >IRQ_TYPE_LEVEL_LOW)>, >IRQ_TYPE_LEVEL_LOW)>, >IRQ_TYPE_LEVEL_LOW)>, >
Re: [PATCH v3 6/8] DT: arm: Add Renesas RZ/N1 SoC base device tree file
Hi Simon, Michel, On Fri, Mar 30, 2018 at 09:25:16AM +0200, Simon Horman wrote: > On Thu, Mar 29, 2018 at 01:04:50PM +0200, jacopo mondi wrote: > > Hi Michel > > > > The subject of all your patches for arch/arm should start with: > > > > ARM: dts: > > > > A git log on that directory clearly shows that's the preferred one. > > > > I would also say that you are missing a symbol definition in > > arch/arm/mach-shmobile/Kconfig > > (even if you got rid of any board file) > > > > I would expect a symbol to select in menuconfig, with your > > dependencies listed there (ie, the serial interface driver) > > > > Something like this (I left the 'xx' out from the part name on purpose) > > > > diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig > > index 280e731..9a519330 100644 > > --- a/arch/arm/mach-shmobile/Kconfig > > +++ b/arch/arm/mach-shmobile/Kconfig > > @@ -114,4 +114,8 @@ config ARCH_SH73A0 > > bool "SH-Mobile AG5 (R8A73A00)" > > select ARCH_RMOBILE > > select RENESAS_INTC_IRQPIN > > + > > +config ARCH_R9A06G0 > > + bool "RZ/N1 (R9A06G0)" > > + select SERIAL_8250_DW > > endif > > > > But please wait for others (preferibly Geert or Simon) to confim this. > > I think that is covered by > "[PATCH v3 5/8] arm: shmobile: Add the RZ/N1 arch to the shmobile Kconfig" > I somehow didn't apply that patch from the series when reviewing. Sorry for the fuss. Thanks j > > On Thu, Mar 29, 2018 at 08:47:02AM +0100, Michel Pollet wrote: > > > This adds the Renesas RZ/N1 Family (Part #R9A06G0xx) SoC > > > bare bone support. > > > > > > This currently only handles generic parts (gic, architected timer) > > > and a UART. > > > For simplicity sake, this also relies on the bootloader to set the > > > pinctrl and clocks. > > > > > > Signed-off-by: Michel Pollet > > > --- > > > arch/arm/boot/dts/r9a06g0xx.dtsi | 96 > > > > > > 1 file changed, 96 insertions(+) > > > create mode 100644 arch/arm/boot/dts/r9a06g0xx.dtsi > > > > > > diff --git a/arch/arm/boot/dts/r9a06g0xx.dtsi > > > b/arch/arm/boot/dts/r9a06g0xx.dtsi > > > new file mode 100644 > > > index 000..c63 > > > --- /dev/null > > > +++ b/arch/arm/boot/dts/r9a06g0xx.dtsi > > > @@ -0,0 +1,96 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Base Device Tree Source for the Renesas RZ/N1 SoC Family of devices > > > + * > > > + * Copyright (C) 2018 Renesas Electronics Europe Limited > > > + * > > > + */ > > > + > > > +#include > > > + > > > +/ { > > > + compatible = "renesas,rzn1"; > > > + #address-cells = <1>; > > > + #size-cells = <1>; > > > + > > > + cpus { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + cpu@0 { > > > + device_type = "cpu"; > > > + compatible = "arm,cortex-a7"; > > > + reg = <0>; > > > + }; > > > + cpu@1 { > > > + device_type = "cpu"; > > > + compatible = "arm,cortex-a7"; > > > + reg = <1>; > > > + }; > > > + }; > > > > I see you don't like empy lines, that's fine, it is not a strict > > requiremen afaik, but I find a few empty lines here and there more > > redable, expecially if the file is going to grow, as it will be. > > Please place one empty line between each node. > > > > + clocks { > > > + /* > > > + * this is fixed clock for now, > > > + * until the clock driver is merged > > > + */ > > > + clkuarts: clkuarts { > > > > You can remove the node lable if it's the same as the node name afaik > > > > > + #clock-cells = <0>; > > > + compatible = "fixed-clock"; > > > + clock-frequency = <47619047>; > > > + }; > > > + }; > > > > Grouping clock nodes under a "clocks" one is now deprecated. > > > > Please see, ie. > > "ARM: dts: r7s72100: stop grouping clocks under a "clocks" subnode" > > Also, please sort sub-nodes of the root node alphabetically. > > > Thanks > >j > > > > > + arch-timer { > > > + compatible = "arm,cortex-a7-timer", > > > + "arm,armv7-timer"; > > > + interrupt-parent = <>; > > > + arm,cpu-registers-not-fw-configured; > > > + interrupts = > > > + > > + IRQ_TYPE_LEVEL_LOW)>, > > > + > > + IRQ_TYPE_LEVEL_LOW)>, > > > + > > + IRQ_TYPE_LEVEL_LOW)>, > > > + > > + IRQ_TYPE_LEVEL_LOW)>; > > I think its nicer not to line-wrap the individual interrupts. > I.e. please make the above like this: > > interrupts = >IRQ_TYPE_LEVEL_LOW)>, >IRQ_TYPE_LEVEL_LOW)>, >IRQ_TYPE_LEVEL_LOW)>, >IRQ_TYPE_LEVEL_LOW)>; > > > > + }; > > > + soc { >
Re: [PATCH v3 6/8] DT: arm: Add Renesas RZ/N1 SoC base device tree file
Hi Michel, On Thu, Mar 29, 2018 at 9:47 AM, Michel Polletwrote: > This adds the Renesas RZ/N1 Family (Part #R9A06G0xx) SoC > bare bone support. > > This currently only handles generic parts (gic, architected timer) > and a UART. > For simplicity sake, this also relies on the bootloader to set the > pinctrl and clocks. > > Signed-off-by: Michel Pollet Thanks for your patch! > --- /dev/null > +++ b/arch/arm/boot/dts/r9a06g0xx.dtsi As I said on IRC, I'm not so happy with this r9a06g0xx.dtsi file without SoC-specific compatible values, as you have to override all of them in r9a06g032.dtsi and r9a06g033.dtsi Moreover, in this series you dropped r9a06g032.dtsi, so the devices no longer have SoC-specific compatible values? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [PATCH v3 6/8] DT: arm: Add Renesas RZ/N1 SoC base device tree file
Hi Michel, On Thu, Mar 29, 2018 at 9:47 AM, Michel Pollet wrote: > This adds the Renesas RZ/N1 Family (Part #R9A06G0xx) SoC > bare bone support. > > This currently only handles generic parts (gic, architected timer) > and a UART. > For simplicity sake, this also relies on the bootloader to set the > pinctrl and clocks. > > Signed-off-by: Michel Pollet Thanks for your patch! > --- /dev/null > +++ b/arch/arm/boot/dts/r9a06g0xx.dtsi As I said on IRC, I'm not so happy with this r9a06g0xx.dtsi file without SoC-specific compatible values, as you have to override all of them in r9a06g032.dtsi and r9a06g033.dtsi Moreover, in this series you dropped r9a06g032.dtsi, so the devices no longer have SoC-specific compatible values? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [PATCH v3 6/8] DT: arm: Add Renesas RZ/N1 SoC base device tree file
On Thu, Mar 29, 2018 at 01:04:50PM +0200, jacopo mondi wrote: > Hi Michel > > The subject of all your patches for arch/arm should start with: > > ARM: dts: > > A git log on that directory clearly shows that's the preferred one. > > I would also say that you are missing a symbol definition in > arch/arm/mach-shmobile/Kconfig > (even if you got rid of any board file) > > I would expect a symbol to select in menuconfig, with your > dependencies listed there (ie, the serial interface driver) > > Something like this (I left the 'xx' out from the part name on purpose) > > diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig > index 280e731..9a519330 100644 > --- a/arch/arm/mach-shmobile/Kconfig > +++ b/arch/arm/mach-shmobile/Kconfig > @@ -114,4 +114,8 @@ config ARCH_SH73A0 > bool "SH-Mobile AG5 (R8A73A00)" > select ARCH_RMOBILE > select RENESAS_INTC_IRQPIN > + > +config ARCH_R9A06G0 > + bool "RZ/N1 (R9A06G0)" > + select SERIAL_8250_DW > endif > > But please wait for others (preferibly Geert or Simon) to confim this. I think that is covered by "[PATCH v3 5/8] arm: shmobile: Add the RZ/N1 arch to the shmobile Kconfig" > On Thu, Mar 29, 2018 at 08:47:02AM +0100, Michel Pollet wrote: > > This adds the Renesas RZ/N1 Family (Part #R9A06G0xx) SoC > > bare bone support. > > > > This currently only handles generic parts (gic, architected timer) > > and a UART. > > For simplicity sake, this also relies on the bootloader to set the > > pinctrl and clocks. > > > > Signed-off-by: Michel Pollet> > --- > > arch/arm/boot/dts/r9a06g0xx.dtsi | 96 > > > > 1 file changed, 96 insertions(+) > > create mode 100644 arch/arm/boot/dts/r9a06g0xx.dtsi > > > > diff --git a/arch/arm/boot/dts/r9a06g0xx.dtsi > > b/arch/arm/boot/dts/r9a06g0xx.dtsi > > new file mode 100644 > > index 000..c63 > > --- /dev/null > > +++ b/arch/arm/boot/dts/r9a06g0xx.dtsi > > @@ -0,0 +1,96 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Base Device Tree Source for the Renesas RZ/N1 SoC Family of devices > > + * > > + * Copyright (C) 2018 Renesas Electronics Europe Limited > > + * > > + */ > > + > > +#include > > + > > +/ { > > + compatible = "renesas,rzn1"; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + cpus { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + cpu@0 { > > + device_type = "cpu"; > > + compatible = "arm,cortex-a7"; > > + reg = <0>; > > + }; > > + cpu@1 { > > + device_type = "cpu"; > > + compatible = "arm,cortex-a7"; > > + reg = <1>; > > + }; > > + }; > > I see you don't like empy lines, that's fine, it is not a strict > requiremen afaik, but I find a few empty lines here and there more > redable, expecially if the file is going to grow, as it will be. Please place one empty line between each node. > > + clocks { > > + /* > > +* this is fixed clock for now, > > +* until the clock driver is merged > > +*/ > > + clkuarts: clkuarts { > > You can remove the node lable if it's the same as the node name afaik > > > + #clock-cells = <0>; > > + compatible = "fixed-clock"; > > + clock-frequency = <47619047>; > > + }; > > + }; > > Grouping clock nodes under a "clocks" one is now deprecated. > > Please see, ie. > "ARM: dts: r7s72100: stop grouping clocks under a "clocks" subnode" Also, please sort sub-nodes of the root node alphabetically. > Thanks >j > > > + arch-timer { > > + compatible = "arm,cortex-a7-timer", > > +"arm,armv7-timer"; > > + interrupt-parent = <>; > > + arm,cpu-registers-not-fw-configured; > > + interrupts = > > +> + IRQ_TYPE_LEVEL_LOW)>, > > +> + IRQ_TYPE_LEVEL_LOW)>, > > +> + IRQ_TYPE_LEVEL_LOW)>, > > +> + IRQ_TYPE_LEVEL_LOW)>; I think its nicer not to line-wrap the individual interrupts. I.e. please make the above like this: interrupts = , , , ; > > + }; > > + soc { > > + compatible = "simple-bus"; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + interrupt-parent = <>; > > + ranges; > > + > > + gic: gic@44101000 { Please sort subnodes of the soc node using: - Primary key of bus address - Secondary key of IP block type (does not apply here) > > + compatible = "arm,cortex-a7-gic",
Re: [PATCH v3 6/8] DT: arm: Add Renesas RZ/N1 SoC base device tree file
On Thu, Mar 29, 2018 at 01:04:50PM +0200, jacopo mondi wrote: > Hi Michel > > The subject of all your patches for arch/arm should start with: > > ARM: dts: > > A git log on that directory clearly shows that's the preferred one. > > I would also say that you are missing a symbol definition in > arch/arm/mach-shmobile/Kconfig > (even if you got rid of any board file) > > I would expect a symbol to select in menuconfig, with your > dependencies listed there (ie, the serial interface driver) > > Something like this (I left the 'xx' out from the part name on purpose) > > diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig > index 280e731..9a519330 100644 > --- a/arch/arm/mach-shmobile/Kconfig > +++ b/arch/arm/mach-shmobile/Kconfig > @@ -114,4 +114,8 @@ config ARCH_SH73A0 > bool "SH-Mobile AG5 (R8A73A00)" > select ARCH_RMOBILE > select RENESAS_INTC_IRQPIN > + > +config ARCH_R9A06G0 > + bool "RZ/N1 (R9A06G0)" > + select SERIAL_8250_DW > endif > > But please wait for others (preferibly Geert or Simon) to confim this. I think that is covered by "[PATCH v3 5/8] arm: shmobile: Add the RZ/N1 arch to the shmobile Kconfig" > On Thu, Mar 29, 2018 at 08:47:02AM +0100, Michel Pollet wrote: > > This adds the Renesas RZ/N1 Family (Part #R9A06G0xx) SoC > > bare bone support. > > > > This currently only handles generic parts (gic, architected timer) > > and a UART. > > For simplicity sake, this also relies on the bootloader to set the > > pinctrl and clocks. > > > > Signed-off-by: Michel Pollet > > --- > > arch/arm/boot/dts/r9a06g0xx.dtsi | 96 > > > > 1 file changed, 96 insertions(+) > > create mode 100644 arch/arm/boot/dts/r9a06g0xx.dtsi > > > > diff --git a/arch/arm/boot/dts/r9a06g0xx.dtsi > > b/arch/arm/boot/dts/r9a06g0xx.dtsi > > new file mode 100644 > > index 000..c63 > > --- /dev/null > > +++ b/arch/arm/boot/dts/r9a06g0xx.dtsi > > @@ -0,0 +1,96 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Base Device Tree Source for the Renesas RZ/N1 SoC Family of devices > > + * > > + * Copyright (C) 2018 Renesas Electronics Europe Limited > > + * > > + */ > > + > > +#include > > + > > +/ { > > + compatible = "renesas,rzn1"; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + cpus { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + cpu@0 { > > + device_type = "cpu"; > > + compatible = "arm,cortex-a7"; > > + reg = <0>; > > + }; > > + cpu@1 { > > + device_type = "cpu"; > > + compatible = "arm,cortex-a7"; > > + reg = <1>; > > + }; > > + }; > > I see you don't like empy lines, that's fine, it is not a strict > requiremen afaik, but I find a few empty lines here and there more > redable, expecially if the file is going to grow, as it will be. Please place one empty line between each node. > > + clocks { > > + /* > > +* this is fixed clock for now, > > +* until the clock driver is merged > > +*/ > > + clkuarts: clkuarts { > > You can remove the node lable if it's the same as the node name afaik > > > + #clock-cells = <0>; > > + compatible = "fixed-clock"; > > + clock-frequency = <47619047>; > > + }; > > + }; > > Grouping clock nodes under a "clocks" one is now deprecated. > > Please see, ie. > "ARM: dts: r7s72100: stop grouping clocks under a "clocks" subnode" Also, please sort sub-nodes of the root node alphabetically. > Thanks >j > > > + arch-timer { > > + compatible = "arm,cortex-a7-timer", > > +"arm,armv7-timer"; > > + interrupt-parent = <>; > > + arm,cpu-registers-not-fw-configured; > > + interrupts = > > +> + IRQ_TYPE_LEVEL_LOW)>, > > +> + IRQ_TYPE_LEVEL_LOW)>, > > +> + IRQ_TYPE_LEVEL_LOW)>, > > +> + IRQ_TYPE_LEVEL_LOW)>; I think its nicer not to line-wrap the individual interrupts. I.e. please make the above like this: interrupts = , , , ; > > + }; > > + soc { > > + compatible = "simple-bus"; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + interrupt-parent = <>; > > + ranges; > > + > > + gic: gic@44101000 { Please sort subnodes of the soc node using: - Primary key of bus address - Secondary key of IP block type (does not apply here) > > + compatible = "arm,cortex-a7-gic", "arm,gic-400"; > > +
Re: [PATCH v3 6/8] DT: arm: Add Renesas RZ/N1 SoC base device tree file
Hi Michel The subject of all your patches for arch/arm should start with: ARM: dts: A git log on that directory clearly shows that's the preferred one. I would also say that you are missing a symbol definition in arch/arm/mach-shmobile/Kconfig (even if you got rid of any board file) I would expect a symbol to select in menuconfig, with your dependencies listed there (ie, the serial interface driver) Something like this (I left the 'xx' out from the part name on purpose) diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig index 280e731..9a519330 100644 --- a/arch/arm/mach-shmobile/Kconfig +++ b/arch/arm/mach-shmobile/Kconfig @@ -114,4 +114,8 @@ config ARCH_SH73A0 bool "SH-Mobile AG5 (R8A73A00)" select ARCH_RMOBILE select RENESAS_INTC_IRQPIN + +config ARCH_R9A06G0 + bool "RZ/N1 (R9A06G0)" + select SERIAL_8250_DW endif But please wait for others (preferibly Geert or Simon) to confim this. On Thu, Mar 29, 2018 at 08:47:02AM +0100, Michel Pollet wrote: > This adds the Renesas RZ/N1 Family (Part #R9A06G0xx) SoC > bare bone support. > > This currently only handles generic parts (gic, architected timer) > and a UART. > For simplicity sake, this also relies on the bootloader to set the > pinctrl and clocks. > > Signed-off-by: Michel Pollet> --- > arch/arm/boot/dts/r9a06g0xx.dtsi | 96 > > 1 file changed, 96 insertions(+) > create mode 100644 arch/arm/boot/dts/r9a06g0xx.dtsi > > diff --git a/arch/arm/boot/dts/r9a06g0xx.dtsi > b/arch/arm/boot/dts/r9a06g0xx.dtsi > new file mode 100644 > index 000..c63 > --- /dev/null > +++ b/arch/arm/boot/dts/r9a06g0xx.dtsi > @@ -0,0 +1,96 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Base Device Tree Source for the Renesas RZ/N1 SoC Family of devices > + * > + * Copyright (C) 2018 Renesas Electronics Europe Limited > + * > + */ > + > +#include > + > +/ { > + compatible = "renesas,rzn1"; > + #address-cells = <1>; > + #size-cells = <1>; > + > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + cpu@0 { > + device_type = "cpu"; > + compatible = "arm,cortex-a7"; > + reg = <0>; > + }; > + cpu@1 { > + device_type = "cpu"; > + compatible = "arm,cortex-a7"; > + reg = <1>; > + }; > + }; I see you don't like empy lines, that's fine, it is not a strict requiremen afaik, but I find a few empty lines here and there more redable, expecially if the file is going to grow, as it will be. > + clocks { > + /* > + * this is fixed clock for now, > + * until the clock driver is merged > + */ > + clkuarts: clkuarts { You can remove the node lable if it's the same as the node name afaik > + #clock-cells = <0>; > + compatible = "fixed-clock"; > + clock-frequency = <47619047>; > + }; > + }; Grouping clock nodes under a "clocks" one is now deprecated. Please see, ie. "ARM: dts: r7s72100: stop grouping clocks under a "clocks" subnode" Thanks j > + arch-timer { > + compatible = "arm,cortex-a7-timer", > + "arm,armv7-timer"; > + interrupt-parent = <>; > + arm,cpu-registers-not-fw-configured; > + interrupts = > + + IRQ_TYPE_LEVEL_LOW)>, > + + IRQ_TYPE_LEVEL_LOW)>, > + + IRQ_TYPE_LEVEL_LOW)>, > + + IRQ_TYPE_LEVEL_LOW)>; > + }; > + soc { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + interrupt-parent = <>; > + ranges; > + > + gic: gic@44101000 { > + compatible = "arm,cortex-a7-gic", "arm,gic-400"; > + interrupt-controller; > + #interrupt-cells = <3>; > + reg = <0x44101000 0x1000>, /* Distributer */ > + <0x44102000 0x2000>, /* CPU interface */ > + <0x44104000 0x2000>, /* Virt interface control */ > + <0x44106000 0x2000>; /* Virt CPU interface */ > + interrupts = > + + IRQ_TYPE_LEVEL_HIGH)>; > + }; > + sysctrl: sysctrl@4000c000 { > + compatible = "renesas,rzn1-sysctrl", "syscon", > + "simple-mfd"; > + reg = <0x4000c000 0x1000>; > + > + reboot {
Re: [PATCH v3 6/8] DT: arm: Add Renesas RZ/N1 SoC base device tree file
Hi Michel The subject of all your patches for arch/arm should start with: ARM: dts: A git log on that directory clearly shows that's the preferred one. I would also say that you are missing a symbol definition in arch/arm/mach-shmobile/Kconfig (even if you got rid of any board file) I would expect a symbol to select in menuconfig, with your dependencies listed there (ie, the serial interface driver) Something like this (I left the 'xx' out from the part name on purpose) diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig index 280e731..9a519330 100644 --- a/arch/arm/mach-shmobile/Kconfig +++ b/arch/arm/mach-shmobile/Kconfig @@ -114,4 +114,8 @@ config ARCH_SH73A0 bool "SH-Mobile AG5 (R8A73A00)" select ARCH_RMOBILE select RENESAS_INTC_IRQPIN + +config ARCH_R9A06G0 + bool "RZ/N1 (R9A06G0)" + select SERIAL_8250_DW endif But please wait for others (preferibly Geert or Simon) to confim this. On Thu, Mar 29, 2018 at 08:47:02AM +0100, Michel Pollet wrote: > This adds the Renesas RZ/N1 Family (Part #R9A06G0xx) SoC > bare bone support. > > This currently only handles generic parts (gic, architected timer) > and a UART. > For simplicity sake, this also relies on the bootloader to set the > pinctrl and clocks. > > Signed-off-by: Michel Pollet > --- > arch/arm/boot/dts/r9a06g0xx.dtsi | 96 > > 1 file changed, 96 insertions(+) > create mode 100644 arch/arm/boot/dts/r9a06g0xx.dtsi > > diff --git a/arch/arm/boot/dts/r9a06g0xx.dtsi > b/arch/arm/boot/dts/r9a06g0xx.dtsi > new file mode 100644 > index 000..c63 > --- /dev/null > +++ b/arch/arm/boot/dts/r9a06g0xx.dtsi > @@ -0,0 +1,96 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Base Device Tree Source for the Renesas RZ/N1 SoC Family of devices > + * > + * Copyright (C) 2018 Renesas Electronics Europe Limited > + * > + */ > + > +#include > + > +/ { > + compatible = "renesas,rzn1"; > + #address-cells = <1>; > + #size-cells = <1>; > + > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + cpu@0 { > + device_type = "cpu"; > + compatible = "arm,cortex-a7"; > + reg = <0>; > + }; > + cpu@1 { > + device_type = "cpu"; > + compatible = "arm,cortex-a7"; > + reg = <1>; > + }; > + }; I see you don't like empy lines, that's fine, it is not a strict requiremen afaik, but I find a few empty lines here and there more redable, expecially if the file is going to grow, as it will be. > + clocks { > + /* > + * this is fixed clock for now, > + * until the clock driver is merged > + */ > + clkuarts: clkuarts { You can remove the node lable if it's the same as the node name afaik > + #clock-cells = <0>; > + compatible = "fixed-clock"; > + clock-frequency = <47619047>; > + }; > + }; Grouping clock nodes under a "clocks" one is now deprecated. Please see, ie. "ARM: dts: r7s72100: stop grouping clocks under a "clocks" subnode" Thanks j > + arch-timer { > + compatible = "arm,cortex-a7-timer", > + "arm,armv7-timer"; > + interrupt-parent = <>; > + arm,cpu-registers-not-fw-configured; > + interrupts = > + + IRQ_TYPE_LEVEL_LOW)>, > + + IRQ_TYPE_LEVEL_LOW)>, > + + IRQ_TYPE_LEVEL_LOW)>, > + + IRQ_TYPE_LEVEL_LOW)>; > + }; > + soc { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + interrupt-parent = <>; > + ranges; > + > + gic: gic@44101000 { > + compatible = "arm,cortex-a7-gic", "arm,gic-400"; > + interrupt-controller; > + #interrupt-cells = <3>; > + reg = <0x44101000 0x1000>, /* Distributer */ > + <0x44102000 0x2000>, /* CPU interface */ > + <0x44104000 0x2000>, /* Virt interface control */ > + <0x44106000 0x2000>; /* Virt CPU interface */ > + interrupts = > + + IRQ_TYPE_LEVEL_HIGH)>; > + }; > + sysctrl: sysctrl@4000c000 { > + compatible = "renesas,rzn1-sysctrl", "syscon", > + "simple-mfd"; > + reg = <0x4000c000 0x1000>; > + > + reboot { > +
[PATCH v3 6/8] DT: arm: Add Renesas RZ/N1 SoC base device tree file
This adds the Renesas RZ/N1 Family (Part #R9A06G0xx) SoC bare bone support. This currently only handles generic parts (gic, architected timer) and a UART. For simplicity sake, this also relies on the bootloader to set the pinctrl and clocks. Signed-off-by: Michel Pollet--- arch/arm/boot/dts/r9a06g0xx.dtsi | 96 1 file changed, 96 insertions(+) create mode 100644 arch/arm/boot/dts/r9a06g0xx.dtsi diff --git a/arch/arm/boot/dts/r9a06g0xx.dtsi b/arch/arm/boot/dts/r9a06g0xx.dtsi new file mode 100644 index 000..c63 --- /dev/null +++ b/arch/arm/boot/dts/r9a06g0xx.dtsi @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Base Device Tree Source for the Renesas RZ/N1 SoC Family of devices + * + * Copyright (C) 2018 Renesas Electronics Europe Limited + * + */ + +#include + +/ { + compatible = "renesas,rzn1"; + #address-cells = <1>; + #size-cells = <1>; + + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu@0 { + device_type = "cpu"; + compatible = "arm,cortex-a7"; + reg = <0>; + }; + cpu@1 { + device_type = "cpu"; + compatible = "arm,cortex-a7"; + reg = <1>; + }; + }; + clocks { + /* +* this is fixed clock for now, +* until the clock driver is merged +*/ + clkuarts: clkuarts { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <47619047>; + }; + }; + arch-timer { + compatible = "arm,cortex-a7-timer", +"arm,armv7-timer"; + interrupt-parent = <>; + arm,cpu-registers-not-fw-configured; + interrupts = + , + , + , + ; + }; + soc { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + interrupt-parent = <>; + ranges; + + gic: gic@44101000 { + compatible = "arm,cortex-a7-gic", "arm,gic-400"; + interrupt-controller; + #interrupt-cells = <3>; + reg = <0x44101000 0x1000>, /* Distributer */ + <0x44102000 0x2000>, /* CPU interface */ + <0x44104000 0x2000>, /* Virt interface control */ + <0x44106000 0x2000>; /* Virt CPU interface */ + interrupts = + ; + }; + sysctrl: sysctrl@4000c000 { + compatible = "renesas,rzn1-sysctrl", "syscon", + "simple-mfd"; + reg = <0x4000c000 0x1000>; + + reboot { + compatible = "renesas,rzn1-reboot"; + }; + }; + uart0: serial@4006 { + compatible = "snps,dw-apb-uart"; + reg = <0x4006 0x400>; + interrupts = ; + reg-shift = <2>; + reg-io-width = <4>; + clocks = <>; + clock-names = "baudclk"; + status = "disabled"; + }; + }; +}; -- 2.7.4
[PATCH v3 6/8] DT: arm: Add Renesas RZ/N1 SoC base device tree file
This adds the Renesas RZ/N1 Family (Part #R9A06G0xx) SoC bare bone support. This currently only handles generic parts (gic, architected timer) and a UART. For simplicity sake, this also relies on the bootloader to set the pinctrl and clocks. Signed-off-by: Michel Pollet --- arch/arm/boot/dts/r9a06g0xx.dtsi | 96 1 file changed, 96 insertions(+) create mode 100644 arch/arm/boot/dts/r9a06g0xx.dtsi diff --git a/arch/arm/boot/dts/r9a06g0xx.dtsi b/arch/arm/boot/dts/r9a06g0xx.dtsi new file mode 100644 index 000..c63 --- /dev/null +++ b/arch/arm/boot/dts/r9a06g0xx.dtsi @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Base Device Tree Source for the Renesas RZ/N1 SoC Family of devices + * + * Copyright (C) 2018 Renesas Electronics Europe Limited + * + */ + +#include + +/ { + compatible = "renesas,rzn1"; + #address-cells = <1>; + #size-cells = <1>; + + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu@0 { + device_type = "cpu"; + compatible = "arm,cortex-a7"; + reg = <0>; + }; + cpu@1 { + device_type = "cpu"; + compatible = "arm,cortex-a7"; + reg = <1>; + }; + }; + clocks { + /* +* this is fixed clock for now, +* until the clock driver is merged +*/ + clkuarts: clkuarts { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <47619047>; + }; + }; + arch-timer { + compatible = "arm,cortex-a7-timer", +"arm,armv7-timer"; + interrupt-parent = <>; + arm,cpu-registers-not-fw-configured; + interrupts = + , + , + , + ; + }; + soc { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + interrupt-parent = <>; + ranges; + + gic: gic@44101000 { + compatible = "arm,cortex-a7-gic", "arm,gic-400"; + interrupt-controller; + #interrupt-cells = <3>; + reg = <0x44101000 0x1000>, /* Distributer */ + <0x44102000 0x2000>, /* CPU interface */ + <0x44104000 0x2000>, /* Virt interface control */ + <0x44106000 0x2000>; /* Virt CPU interface */ + interrupts = + ; + }; + sysctrl: sysctrl@4000c000 { + compatible = "renesas,rzn1-sysctrl", "syscon", + "simple-mfd"; + reg = <0x4000c000 0x1000>; + + reboot { + compatible = "renesas,rzn1-reboot"; + }; + }; + uart0: serial@4006 { + compatible = "snps,dw-apb-uart"; + reg = <0x4006 0x400>; + interrupts = ; + reg-shift = <2>; + reg-io-width = <4>; + clocks = <>; + clock-names = "baudclk"; + status = "disabled"; + }; + }; +}; -- 2.7.4