Re: [PATCH 1/2] arm: add basic support for Renesas RZ/N1 boards

2018-03-05 Thread Geert Uytterhoeven
Hi Michel,

On Mon, Feb 26, 2018 at 1:18 PM, Michel Pollet
 wrote:
> This adds the Renesas RZ/N1 CPU and bare bone support.
>
> This currently only handles generic parts (gic, architected timer)
> and a UART.
> This also relies on the bootloader to set the pinctrl and clocks.
>
> Signed-off-by: Michel Pollet 

Thanks for your patch!

This should be split in separate patches:

>  Documentation/devicetree/bindings/arm/shmobile.txt |   3 +-

1. DT bindings

>  arch/arm/boot/dts/rzn1.dtsi|  94 +++

2. SoC DTS file

>  arch/arm/mach-shmobile/Kconfig |   5 +

3. Platform Kconfig symbol

>  arch/arm/mach-shmobile/Makefile|   1 +
>  arch/arm/mach-shmobile/setup-r9a06g032.c   |  60 ++

Please no more board files for new platforms (see below).

>  .../dt-bindings/interrupt-controller/rzn1-irq.h| 137 

DTS files are much easier to compare with the datasheet if the interrupt
numbers are present in the DTS files theirselves.

>  include/dt-bindings/soc/renesas,rzn1-map.h | 173 +

Same for base addresses.

> --- a/Documentation/devicetree/bindings/arm/shmobile.txt
> +++ b/Documentation/devicetree/bindings/arm/shmobile.txt
> @@ -47,7 +47,8 @@ SoCs:
>  compatible = "renesas,r8a77980"
>- R-Car D3 (R8A77995)
>  compatible = "renesas,r8a77995"
> -
> +  - RZ/N1D (R9A06G032)
> +compatible = "renesas,r9a06g032"

BTW, are R9A06G032NGBG and R9A06G032VGBA the same SoC,
just in different packages?

> --- /dev/null
> +++ b/arch/arm/boot/dts/rzn1.dtsi

So faw we always named the SoC-specific DTS files after the SoC part
number => r9a06g032.dtsi.

> @@ -0,0 +1,94 @@
> +/*
> + * Base Device Tree Source for the Renesas RZ/N1 SoC
> + *
> + * Copyright (C) 2018 Renesas Electronics Europe Limited
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "skeleton.dtsi"
> +
> +/ {
> +   compatible = "renesas,r9a06g032";
> +   interrupt-parent = <&gic>;
> +   #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>;
> +   };
> +   };
> +   aliases {
> +   serial0 = &uart0;
> +   };
> +   arm_timer: timer {
> +   compatible = "arm,armv7-timer";
> +   arm,cpu-registers-not-fw-configured;
> +   interrupts =
> ++   IRQ_TYPE_LEVEL_LOW)>,
> ++   IRQ_TYPE_LEVEL_LOW)>,
> ++   IRQ_TYPE_LEVEL_LOW)>,
> ++   IRQ_TYPE_LEVEL_LOW)>;
> +   };
> +   gic: interrupt-controller@RZN1_GIC_BASE {

On-SoC devices should be grouped under an "soc" node.
You can move the "interrupt-parent = <&gic>;" there, too.

> +   compatible = "arm,cortex-a7-gic";

As the RZ/N1D's User's Manul refers to the GIC-400 manuals, I assume
this is a GIC-400 => "arm,gic-400".

You can check by reading the GIC_DIST_IIDR register.

> +   reg = <0x44101000 0x1000>,  /* Distributer */
> + <0x44102000 0x1000>,  /* CPU interface */

Shouldn't the size of the second region be 0x2000?

> +   bus {

Oh, you do have an "soc" node. Please call it "soc".

> --- /dev/null
> +++ b/arch/arm/mach-shmobile/setup-r9a06g032.c
> @@ -0,0 +1,60 @@
> +/*
> + * RZ/N1 processor support file
> + *
> + * Copyright (C) 2018 Renesas Electronics Europe Limited
> + *
> + * Michel Pollet , 
> + *
> + */
> + /* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static void __iomem *sysctrl_base_addr;
> +
> +static void rzn1_sysctrl_init(void)
> +{
> +   if (sysctrl_base_addr)
> +   return;
> +   sysctrl_base_addr = ioremap(RZN1_SYSTEM_CTRL_BASE,
> +   RZN1_SYSTEM_CTRL_SIZE);

These values should be obtained from DT.

> +   BUG_ON(!sysctrl_base_addr);
> +}
> +
> +void __iomem *rzn1_sysctrl_base(void)
> +{
> +   if (!sysctrl_base_addr)
> +   rzn1_sysctrl_init();
> +   return sysctrl_base_addr;
> +}
> +EXPORT_SYMBOL(rzn1_sysctrl_base);

Looks like this is a "system controller", providing a bunch of registers
to a collection of random functionality, to be used by various drivers.

Please see:
  Documentation/devicetree/bindings/mfd/syscon.txt
  include/linux/mfd/syscon.h
  drivers/mfd/syscon.c

> +static void r

Re: [PATCH 1/2] arm: add basic support for Renesas RZ/N1 boards

2018-03-02 Thread Rob Herring
On Mon, Feb 26, 2018 at 12:18:19PM +, Michel Pollet wrote:
> This adds the Renesas RZ/N1 CPU and bare bone support.
> 
> This currently only handles generic parts (gic, architected timer)
> and a UART.
> This also relies on the bootloader to set the pinctrl and clocks.
> 
> Signed-off-by: Michel Pollet 
> ---
>  Documentation/devicetree/bindings/arm/shmobile.txt |   3 +-
>  arch/arm/boot/dts/rzn1.dtsi|  94 +++
>  arch/arm/mach-shmobile/Kconfig |   5 +
>  arch/arm/mach-shmobile/Makefile|   1 +
>  arch/arm/mach-shmobile/setup-r9a06g032.c   |  60 ++
>  .../dt-bindings/interrupt-controller/rzn1-irq.h| 137 
>  include/dt-bindings/soc/renesas,rzn1-map.h | 173 +
>  include/soc/rzn1/sysctrl.h | 736 
> +
>  8 files changed, 1208 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/rzn1.dtsi
>  create mode 100644 arch/arm/mach-shmobile/setup-r9a06g032.c
>  create mode 100644 include/dt-bindings/interrupt-controller/rzn1-irq.h
>  create mode 100644 include/dt-bindings/soc/renesas,rzn1-map.h
>  create mode 100644 include/soc/rzn1/sysctrl.h
> 
> diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt 
> b/Documentation/devicetree/bindings/arm/shmobile.txt
> index 63edc11..153f69bb 100644
> --- a/Documentation/devicetree/bindings/arm/shmobile.txt
> +++ b/Documentation/devicetree/bindings/arm/shmobile.txt
> @@ -47,7 +47,8 @@ SoCs:
>  compatible = "renesas,r8a77980"
>- R-Car D3 (R8A77995)
>  compatible = "renesas,r8a77995"
> -
> +  - RZ/N1D (R9A06G032)
> +compatible = "renesas,r9a06g032"
>  
>  Boards:
>  
> diff --git a/arch/arm/boot/dts/rzn1.dtsi b/arch/arm/boot/dts/rzn1.dtsi
> new file mode 100644
> index 000..bc134b0
> --- /dev/null
> +++ b/arch/arm/boot/dts/rzn1.dtsi
> @@ -0,0 +1,94 @@
> +/*
> + * Base Device Tree Source for the Renesas RZ/N1 SoC
> + *
> + * Copyright (C) 2018 Renesas Electronics Europe Limited
> + *
> + * SPDX-License-Identifier: GPL-2.0

Goes on the first line now.

> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "skeleton.dtsi"

Don't use skeleton.dtsi. We're trying to remove it.

> +
> +/ {
> + compatible = "renesas,r9a06g032";
> + interrupt-parent = <&gic>;
> + #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>;
> + };
> + };
> + aliases {
> + serial0 = &uart0;
> + };
> + arm_timer: timer {
> + compatible = "arm,armv7-timer";
> + arm,cpu-registers-not-fw-configured;
> + interrupts =
> +  + IRQ_TYPE_LEVEL_LOW)>,
> +  + IRQ_TYPE_LEVEL_LOW)>,
> +  + IRQ_TYPE_LEVEL_LOW)>,
> +  + IRQ_TYPE_LEVEL_LOW)>;
> + };
> + gic: interrupt-controller@RZN1_GIC_BASE {

Don't use macros for unit-addresses.

> + compatible = "arm,cortex-a7-gic";
> + reg = <0x44101000 0x1000>,  /* Distributer */
> +   <0x44102000 0x1000>,  /* CPU interface */
> +   <0x44104000 0x2000>,  /* Virt interface control */
> +   <0x44106000 0x2000>;  /* Virt CPU interface */
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + interrupts =
> +  + IRQ_TYPE_LEVEL_HIGH)>;
> + };
> + clocks: clocks@0 {

Build with W=1 and W=12 and fix those warnings.

> + /*
> +  * this is fixed clock for now,
> +  * until the clock driver is merged
> +  */
> + clk_uarts: clk_uarts@0 {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <47619047>;
> + };
> + };
> + bus {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + uart0: serial@RZN1_UART0_BASE {
> + compatible = "snps,dw-apb-uart";
> + reg = ;
> + interrupts =  + IRQ_TYPE_LEVEL_HIGH>;
> + reg-shift = <2>;
> + reg-io-width = <4>;
> + clocks = <&clk_uarts>;
> + clock-nam

[PATCH 1/2] arm: add basic support for Renesas RZ/N1 boards

2018-02-26 Thread Michel Pollet
This adds the Renesas RZ/N1 CPU and bare bone support.

This currently only handles generic parts (gic, architected timer)
and a UART.
This also relies on the bootloader to set the pinctrl and clocks.

Signed-off-by: Michel Pollet 
---
 Documentation/devicetree/bindings/arm/shmobile.txt |   3 +-
 arch/arm/boot/dts/rzn1.dtsi|  94 +++
 arch/arm/mach-shmobile/Kconfig |   5 +
 arch/arm/mach-shmobile/Makefile|   1 +
 arch/arm/mach-shmobile/setup-r9a06g032.c   |  60 ++
 .../dt-bindings/interrupt-controller/rzn1-irq.h| 137 
 include/dt-bindings/soc/renesas,rzn1-map.h | 173 +
 include/soc/rzn1/sysctrl.h | 736 +
 8 files changed, 1208 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/rzn1.dtsi
 create mode 100644 arch/arm/mach-shmobile/setup-r9a06g032.c
 create mode 100644 include/dt-bindings/interrupt-controller/rzn1-irq.h
 create mode 100644 include/dt-bindings/soc/renesas,rzn1-map.h
 create mode 100644 include/soc/rzn1/sysctrl.h

diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt 
b/Documentation/devicetree/bindings/arm/shmobile.txt
index 63edc11..153f69bb 100644
--- a/Documentation/devicetree/bindings/arm/shmobile.txt
+++ b/Documentation/devicetree/bindings/arm/shmobile.txt
@@ -47,7 +47,8 @@ SoCs:
 compatible = "renesas,r8a77980"
   - R-Car D3 (R8A77995)
 compatible = "renesas,r8a77995"
-
+  - RZ/N1D (R9A06G032)
+compatible = "renesas,r9a06g032"
 
 Boards:
 
diff --git a/arch/arm/boot/dts/rzn1.dtsi b/arch/arm/boot/dts/rzn1.dtsi
new file mode 100644
index 000..bc134b0
--- /dev/null
+++ b/arch/arm/boot/dts/rzn1.dtsi
@@ -0,0 +1,94 @@
+/*
+ * Base Device Tree Source for the Renesas RZ/N1 SoC
+ *
+ * Copyright (C) 2018 Renesas Electronics Europe Limited
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "skeleton.dtsi"
+
+/ {
+   compatible = "renesas,r9a06g032";
+   interrupt-parent = <&gic>;
+   #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>;
+   };
+   };
+   aliases {
+   serial0 = &uart0;
+   };
+   arm_timer: timer {
+   compatible = "arm,armv7-timer";
+   arm,cpu-registers-not-fw-configured;
+   interrupts =
+   ,
+   ,
+   ,
+   ;
+   };
+   gic: interrupt-controller@RZN1_GIC_BASE {
+   compatible = "arm,cortex-a7-gic";
+   reg = <0x44101000 0x1000>,  /* Distributer */
+ <0x44102000 0x1000>,  /* CPU interface */
+ <0x44104000 0x2000>,  /* Virt interface control */
+ <0x44106000 0x2000>;  /* Virt CPU interface */
+   interrupt-controller;
+   #interrupt-cells = <3>;
+   interrupts =
+   ;
+   };
+   clocks: clocks@0 {
+   /*
+* this is fixed clock for now,
+* until the clock driver is merged
+*/
+   clk_uarts: clk_uarts@0 {
+   #clock-cells = <0>;
+   compatible = "fixed-clock";
+   clock-frequency = <47619047>;
+   };
+   };
+   bus {
+   compatible = "simple-bus";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+
+   uart0: serial@RZN1_UART0_BASE {
+   compatible = "snps,dw-apb-uart";
+   reg = ;
+   interrupts = ;
+   reg-shift = <2>;
+   reg-io-width = <4>;
+   clocks = <&clk_uarts>;
+   clock-names = "baudclk";
+   status = "disabled";
+   };
+   };
+};
diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index 280e731..e2cd7aa 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -110,6 +110,11 @@ config ARCH_R8A7794
bool "R-Car E2 (R8A77940)"
select ARCH_RCAR_GEN2
 
+config ARCH_R9A06G032
+   bool "RZ/N1D (R9A06G032)"
+   select ARM_AMBA
+   select CPU_V7
+
 config ARCH_SH73A0
bool "SH-Mobile AG5 (R8A73A00)"
select ARCH_RMOBILE
diff --git a/arch/arm/mach-shmobile/Makefile b/arch/arm/mach-shmobile/Makefile
index 1939f52..a63e5c2 1