Initial Device Tree support for S3C64xx

This patch extends vic_of_init to parse valid interrupt sources and resume sources masks from device tree.

If mask values are not specified in device tree, all sources are assumed to be valid, as before this patch.

Signed-off-by Tomasz Figa <[email protected]>

---
 Documentation/devicetree/bindings/arm/vic.txt | 6 ++++++
 arch/arm/common/vic.c                         | 7 ++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/arm/vic.txt +0 -0

index 266716b..bb7137c 100644

Documentation/devicetree/bindings/arm/vic.txt +3 -0

 Example:
 

@@ -26,4 +29,7 @@ Example:
 		interrupt-controller;
 		#interrupt-cells = <1>;
 		reg = <0x60000 0x1000>;
+
+		interrupt-mask = <0xffffff7f>;
+		wakeup-mask = <0x0000ff7f>;
 	};

arch/arm/common/vic.c +6 -1

index e4df17c..c2889da 100644
@@ -407,6 +407,8 @@ void __init vic_init(void __iomem *base, unsigned int irq_start,
 int __init vic_of_init(struct device_node *node, struct device_node *parent)
 {
 	void __iomem *regs;
+	u32 interrupt_mask = ~0;
+	u32 wakeup_mask = ~0;
 
 	if (WARN(parent, "non-root VICs are not supported"))
 		return -EINVAL;

@@ -415,10 +417,13 @@ int __init vic_of_init(struct device_node *node, struct device_node *parent)
 	if (WARN_ON(!regs))
 		return -EIO;
 
+	of_property_read_u32(node, "interrupt-mask", &interrupt_mask);
+	of_property_read_u32(node, "wakeup-mask", &wakeup_mask);
+
 	/*
 	 * Passing -1 as first IRQ makes the simple domain allocate descriptors
 	 */
-	__vic_init(regs, -1, ~0, ~0, node);
+	__vic_init(regs, -1, interrupt_mask, wakeup_mask, node);
 
 	return 0;
 }

Quoted 19 lines

This patch extends vic_of_init to parse valid interrupt sources and resume sources masks from device tree.

If mask values are not specified in device tree, all sources are assumed to be valid, as before this patch.

Signed-off-by Tomasz Figa <[email protected]>

---
 Documentation/devicetree/bindings/arm/vic.txt | 6 ++++++
 arch/arm/common/vic.c                         | 7 ++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/arm/vic.txt +0 -0

index 266716b..bb7137c 100644
@@ -18,6 +18,9 @@ Required properties:
 Optional properties:
 

Documentation/devicetree/bindings/arm/vic.txt Required properties: · line 21

Quoted 2 lines

Documentation/devicetree/bindings/arm/vic.txt +1 -0

 - interrupts : Interrupt source for parent controllers if the VIC is nested.
+- interrupt-mask : Bit mask of valid interrupt sources (defaults to all valid)

Can you explain why this is needed and is not just the OR of all interrupts described in the DT?

Documentation/devicetree/bindings/arm/vic.txt Required properties: · line 23

Quoted 2 lines

Documentation/devicetree/bindings/arm/vic.txt +2 -0

+- wakeup-mask : Bit mask of interrupt sources that can wake up the system
+  (defaults to all allowed)

Seems like this would be all VIC interrupts unless the wake-up handling is done in some shadow controller. If the former is true, then wake-up capability is really a property of individual devices. If the later, then this property would belong in that shadow controller.

Rob

Quoted 40 lines

Documentation/devicetree/bindings/arm/vic.txt +3 -0

 Example:
 

@@ -26,4 +29,7 @@ Example:
              interrupt-controller;
              #interrupt-cells = <1>;
              reg = <0x60000 0x1000>;
+
+             interrupt-mask = <0xffffff7f>;
+             wakeup-mask = <0x0000ff7f>;
      };

arch/arm/common/vic.c +6 -1

index e4df17c..c2889da 100644
@@ -407,6 +407,8 @@ void __init vic_init(void __iomem *base, unsigned int irq_start,
 int __init vic_of_init(struct device_node *node, struct device_node *parent)
 {
      void __iomem *regs;
+     u32 interrupt_mask = ~0;
+     u32 wakeup_mask = ~0;
 
      if (WARN(parent, "non-root VICs are not supported"))
              return -EINVAL;

@@ -415,10 +417,13 @@ int __init vic_of_init(struct device_node *node, struct device_node *parent)
      if (WARN_ON(!regs))
              return -EIO;
 
+     of_property_read_u32(node, "interrupt-mask", &interrupt_mask);
+     of_property_read_u32(node, "wakeup-mask", &wakeup_mask);
+
      /*
       * Passing -1 as first IRQ makes the simple domain allocate descriptors
       */
-     __vic_init(regs, -1, ~0, ~0, node);
+     __vic_init(regs, -1, interrupt_mask, wakeup_mask, node);
 
      return 0;
 }

This patch adds basic device tree definitions for Samsung S3C64xx SoCs.

Since all the SoCs in the series are very similar, the files are created hierarchically - one file for the whole series and then separate files for particular SoCs including the common one.

Signed-off-by Tomasz Figa <[email protected]>

arch/arm/boot/dts/s3c64xx.dtsi new file +16 -0

new file mode 100644
index 0000000..55d6e08
@@ -0,0 +1,97 @@
+/*
+ * Samsung's S3C64xx SoC series common device tree source
+ *
+ * Copyright (c) 2013 Tomasz Figa <[email protected]>
+ *
+ * Samsung's S3C64xx SoC series device nodes are listed in this file.
+ * Particular SoCs from S3C64xx series can include this file and provide
+ * values for SoCs specfic bindings.
+ *
+ * Note: This file does not include device nodes for all the controllers in
+ * S3C64xx SoCs. As device tree coverage for S3C64xx increases, additional
+ * nodes can be added to this file.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.

Hello,

This all looks good. I just have a couple of comments about the cpus node.

Quoted 7 lines · Tomasz Figa

This patch adds basic device tree definitions for Samsung S3C64xx SoCs.

Since all the SoCs in the series are very similar, the files are created hierarchically - one file for the whole series and then separate files for particular SoCs including the common one.

Signed-off-by Tomasz Figa <[email protected]>

[...]

Quoted 28 lines

arch/arm/boot/dts/s3c64xx.dtsi new file +22 -0

new file mode 100644
index 0000000..55d6e08
@@ -0,0 +1,97 @@
+/*
+ * Samsung's S3C64xx SoC series common device tree source
+ *
+ * Copyright (c) 2013 Tomasz Figa <[email protected]>
+ *
+ * Samsung's S3C64xx SoC series device nodes are listed in this file.
+ * Particular SoCs from S3C64xx series can include this file and provide
+ * values for SoCs specfic bindings.
+ *
+ * Note: This file does not include device nodes for all the controllers in
+ * S3C64xx SoCs. As device tree coverage for S3C64xx increases, additional
+ * nodes can be added to this file.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/include/ "skeleton.dtsi"
+
+/ {
+	cpus {

arch/arm/boot/dts/s3c64xx.dtsi line 26

Quoted 4 lines

arch/arm/boot/dts/s3c64xx.dtsi new file +4 -0

+		cpu at 0 {
+			compatible = "arm,arm1176jzf-s";
+		};
+	};

You can drop the unit address from the cpu node - it's meant to be there to differentiate multiple nodes (and is supposed to match the reg property, which the 1176jzf-s can't have, as it doesn't have an MPIDR).

Also, "arm,arm1176jzf-s" isn't listed in the binding doc. There was a question about how to maintain this list [1], but I can't seem to find a conclusion, if any were reached. It might be worth appending "arm,arm1176" to the compatible list for the cpu node in case we want to enable something via dt for all 1176 variations.

Dave, Lorenzo, any thoughts?

Quoted 43 lines · Mark Rutland

Hello,

This all looks good. I just have a couple of comments about the cpus node.

Quoted 7 lines · Tomasz Figa

This patch adds basic device tree definitions for Samsung S3C64xx SoCs.

Since all the SoCs in the series are very similar, the files are created hierarchically - one file for the whole series and then separate files for particular SoCs including the common one.

Signed-off-by Tomasz Figa <[email protected]>

[...]

Quoted 28 lines

arch/arm/boot/dts/s3c64xx.dtsi new file +22 -0

new file mode 100644
index 0000000..55d6e08
@@ -0,0 +1,97 @@
+/*
+ * Samsung's S3C64xx SoC series common device tree source
+ *
+ * Copyright (c) 2013 Tomasz Figa <[email protected]>
+ *
+ * Samsung's S3C64xx SoC series device nodes are listed in this file.
+ * Particular SoCs from S3C64xx series can include this file and provide
+ * values for SoCs specfic bindings.
+ *
+ * Note: This file does not include device nodes for all the controllers in
+ * S3C64xx SoCs. As device tree coverage for S3C64xx increases, additional
+ * nodes can be added to this file.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/include/ "skeleton.dtsi"
+
+/ {
+	cpus {

Quoted 8 lines

arch/arm/boot/dts/s3c64xx.dtsi line 26

Quoted 4 lines

arch/arm/boot/dts/s3c64xx.dtsi new file +4 -0

+		cpu at 0 {
+			compatible = "arm,arm1176jzf-s";
+		};
+	};

You can drop the unit address from the cpu node - it's meant to be there to differentiate multiple nodes (and is supposed to match the reg property, which the 1176jzf-s can't have, as it doesn't have an MPIDR).

Well, this is a point that I should consider since the kernel docs I wrote are misleading, they require the reg property that can not be there on UP. True, MPIDR does not exist in this case, but I have to document this in the bindings since it is unclear.

Quoted 7 lines

Also, "arm,arm1176jzf-s" isn't listed in the binding doc. There was a question about how to maintain this list [1], but I can't seem to find a conclusion, if any were reached. It might be worth appending "arm,arm1176" to the compatible list for the cpu node in case we want to enable something via dt for all 1176 variations.

Quoted 1 line

Dave, Lorenzo, any thoughts?

Eh, frankly I do not know how to handle this. Either we add a compatible string to the bindings anytime a DT gets merged in the kernel but how to maintain it, it has to be defined. Happy to hear some feedback on this.

Lorenzo

Quoted 68 lines · Lorenzo Pieralisi

Quoted 47 lines · Mark Rutland

Hello,

This all looks good. I just have a couple of comments about the cpus node.> >

Quoted 8 lines · Tomasz Figa

This patch adds basic device tree definitions for Samsung S3C64xx SoCs.

Since all the SoCs in the series are very similar, the files are created hierarchically - one file for the whole series and then separate files for particular SoCs including the common one.

Signed-off-by Tomasz Figa <[email protected]>

[...]

Quoted 31 lines

(quoted diff) +0 -0

b/arch/arm/boot/dts/s3c64xx.dtsi new file mode 100644 index 0000000..55d6e08

arch/arm/boot/dts/s3c64xx.dtsi new file +6 -0

@@ -0,0 +1,97 @@
+/*
+ * Samsung's S3C64xx SoC series common device tree source
+ *
+ * Copyright (c) 2013 Tomasz Figa <[email protected]>
+ *
+ * Samsung's S3C64xx SoC series device nodes are listed in this

file.
+ * Particular SoCs from S3C64xx series can include this file and provide + * values for SoCs specfic bindings.
+ *
+ * Note: This file does not include device nodes for all the controllers in + * S3C64xx SoCs. As device tree coverage for S3C64xx increases, additional + * nodes can be added to this file.
+ *
+ * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation.
+ */

arch/arm/boot/dts/s3c64xx.dtsi new file +6 -0

+
+/include/ "skeleton.dtsi"
+
+/ {
+	cpus {
+		cpu at 0 {

Quoted 8 lines

arch/arm/boot/dts/s3c64xx.dtsi line 26

Quoted 3 lines

arch/arm/boot/dts/s3c64xx.dtsi new file +3 -0

+			compatible = "arm,arm1176jzf-s";
+		};
+	};

You can drop the unit address from the cpu node - it's meant to be there to differentiate multiple nodes (and is supposed to match the reg property, which the 1176jzf-s can't have, as it doesn't have an MPIDR).

Well, this is a point that I should consider since the kernel docs I wrote are misleading, they require the reg property that can not be there on UP. True, MPIDR does not exist in this case, but I have to document this in the bindings since it is unclear.

Quoted 6 lines

Also, "arm,arm1176jzf-s" isn't listed in the binding doc. There was a question about how to maintain this list [1], but I can't seem to find a conclusion, if any were reached. It might be worth appending "arm,arm1176" to the compatible list for the cpu node in case we want to enable something via dt for all 1176 variations.

Quoted 6 lines

Quoted 1 line

Dave, Lorenzo, any thoughts?

Eh, frankly I do not know how to handle this. Either we add a compatible string to the bindings anytime a DT gets merged in the kernel but how to maintain it, it has to be defined. Happy to hear some feedback on this.

Well, the number of CPU types does not grow rapidly. It will be much less than one per SoC -- so keeping the list up to date shouldn't be that much effort.

For ARM1176JZF-S, it could make sense for the comatible list to be

	"arm,arm1176jzf-s", "arm,arm1176"

...since the differences between 1176 variants are software probeable (i.e., whether there is an FPU or not). AFAIK the J, Z apply to all ARM1176, and the -S (synthesisable RTL) is nothing to do with software. The kernel probably only really needs to know "arm,arm1176".

Quoted 4 lines

Well, the number of CPU types does not grow rapidly. It will be much less than one per SoC -- so keeping the list up to date shouldn't be that much effort.

Quoted 8 lines

For ARM1176JZF-S, it could make sense for the comatible list to be

	"arm,arm1176jzf-s", "arm,arm1176"

...since the differences between 1176 variants are software probeable (i.e., whether there is an FPU or not). AFAIK the J, Z apply to all ARM1176, and the -S (synthesisable RTL) is nothing to do with software. The kernel probably only really needs to know "arm,arm1176".

OK. So the conclusion is that I should change the cpus node to following:

       cpus {
               cpu {

                       compatible = "arm,arm1176jzf-s", "arm,arm1176";
               };
       };

Am I right?

Tomasz Figa wrote:

[...]

Quoted 13 lines

Quoted 12 lines

Well, the number of CPU types does not grow rapidly. It will be much less than one per SoC -- so keeping the list up to date shouldn't be that much effort.

For ARM1176JZF-S, it could make sense for the comatible list to be

	"arm,arm1176jzf-s", "arm,arm1176"

...since the differences between 1176 variants are software probeable (i.e., whether there is an FPU or not). AFAIK the J, Z apply to all ARM1176, and the -S (synthesisable RTL) is nothing to do with software. The kernel probably only really needs to know "arm,arm1176".

Quoted 4 lines

OK. So the conclusion is that I should change the cpus node to following:

       cpus {
               cpu {

cpu at 0 { ?

Quoted 7 lines

                       compatible = "arm,arm1176jzf-s", "arm,arm1176";
               };
       };

Am I right?

I think so :-)

- Kukjin

Quoted 17 lines · Kukjin Kim

Tomasz Figa wrote:

[...]

Quoted 13 lines

Quoted 12 lines

Well, the number of CPU types does not grow rapidly. It will be much less than one per SoC -- so keeping the list up to date shouldn't be that much effort.

For ARM1176JZF-S, it could make sense for the comatible list to be

	"arm,arm1176jzf-s", "arm,arm1176"

...since the differences between 1176 variants are software probeable (i.e., whether there is an FPU or not). AFAIK the J, Z apply to all ARM1176, and the -S (synthesisable RTL) is nothing to do with software. The kernel probably only really needs to know "arm,arm1176".

Quoted 6 lines

Quoted 4 lines

OK. So the conclusion is that I should change the cpus node to following:

       cpus {
               cpu {

cpu at 0 { ?

As the CPU has no MPIDR, and thus doesn't have a reg property, there's no reason to have a unit address. Just "cpu" should be fine.

Quoted 7 lines

Quoted 7 lines

                       compatible = "arm,arm1176jzf-s", "arm,arm1176";
               };
       };

Am I right?

Quoted 4 lines

I think so :-)

- Kukjin

As Dave pointed out, the "jzf-s" portion might be superfluous. It's worth having the "arm,arm1176", though.

This series is adds Device Tree support for Samsung S3C64xx SoC series.

It fixes several problems preventing from booting an S3C64xx-based system using Device Tree, adds all the infrastructure for Device Tree-based board support, including mach-s3c64xx-dt and dts include files for S3C64xx SoCs, and (very) basic device tree source for FriendlyARM Mini6410 board.

Current support is very limited and allows only basic bootup with UART and SDHCI, but should be fine as a start and will be extended in future, hopefully with help of S3C64xx board maintainers.

Tomasz Figa (6):
ARM: common: vic: Parse interrupt and resume masks from device tree
ARM: common: vic: Fix invalid first IRQ number in OF-based
registration
ARM: s3c64xx: Add support for OF-based VIC initialization
ARM: s3c64xx: Add board file for boot using Device Tree
ARM: dts: Add basic dts include files for Samsung S3C64xx SoCs
ARM: dts: Add dts file for S3C6410-based Mini6410 board

 Documentation/devicetree/bindings/arm/vic.txt |  6 ++
 arch/arm/boot/dts/Makefile                    |  1 +
 arch/arm/boot/dts/s3c6400.dtsi                | 33 +++++++++
 arch/arm/boot/dts/s3c6410-mini6410.dts        | 50 ++++++++++++++
 arch/arm/boot/dts/s3c6410.dtsi                | 33 +++++++++
 arch/arm/boot/dts/s3c64xx.dtsi                | 97
+++++++++++++++++++++++++++ arch/arm/common/vic.c                      
  |  9 ++-
 arch/arm/mach-s3c64xx/Kconfig                 | 13 ++++
 arch/arm/mach-s3c64xx/Makefile                |  1 +
 arch/arm/mach-s3c64xx/common.c                | 16 +++++
 arch/arm/mach-s3c64xx/common.h                |  4 ++

 arch/arm/mach-s3c64xx/mach-s3c64xx-dt.c       | 84
+++++++++++++++++++++++ 12 files changed, 345 insertions(+), 2
deletions(-)
 create mode 100644 arch/arm/boot/dts/s3c6400.dtsi
 create mode 100644 arch/arm/boot/dts/s3c6410-mini6410.dts
 create mode 100644 arch/arm/boot/dts/s3c6410.dtsi
 create mode 100644 arch/arm/boot/dts/s3c64xx.dtsi
 create mode 100644 arch/arm/mach-s3c64xx/mach-s3c64xx-dt.c

Quoted 33 lines · On Sunday 13 of January 2013 02:10:52 Tomasz Figa wrote:

This series is adds Device Tree support for Samsung S3C64xx SoC series.

It fixes several problems preventing from booting an S3C64xx-based system using Device Tree, adds all the infrastructure for Device Tree-based board support, including mach-s3c64xx-dt and dts include files for S3C64xx SoCs, and (very) basic device tree source for FriendlyARM Mini6410 board.

Current support is very limited and allows only basic bootup with UART and SDHCI, but should be fine as a start and will be extended in future, hopefully with help of S3C64xx board maintainers.

Tomasz Figa (6):
ARM: common: vic: Parse interrupt and resume masks from device tree
ARM: common: vic: Fix invalid first IRQ number in OF-based
registration
ARM: s3c64xx: Add support for OF-based VIC initialization
ARM: s3c64xx: Add board file for boot using Device Tree
ARM: dts: Add basic dts include files for Samsung S3C64xx SoCs
ARM: dts: Add dts file for S3C6410-based Mini6410 board

 Documentation/devicetree/bindings/arm/vic.txt |  6 ++
 arch/arm/boot/dts/Makefile                    |  1 +
 arch/arm/boot/dts/s3c6400.dtsi                | 33 +++++++++
 arch/arm/boot/dts/s3c6410-mini6410.dts        | 50 ++++++++++++++
 arch/arm/boot/dts/s3c6410.dtsi                | 33 +++++++++
 arch/arm/boot/dts/s3c64xx.dtsi                | 97
+++++++++++++++++++++++++++ arch/arm/common/vic.c
  |  9 ++-
 arch/arm/mach-s3c64xx/Kconfig                 | 13 ++++
 arch/arm/mach-s3c64xx/Makefile                |  1 +
 arch/arm/mach-s3c64xx/common.c                | 16 +++++
 arch/arm/mach-s3c64xx/common.h                |  4 ++

Quoted 8 lines

 arch/arm/mach-s3c64xx/mach-s3c64xx-dt.c       | 84
+++++++++++++++++++++++ 12 files changed, 345 insertions(+), 2
deletions(-)
 create mode 100644 arch/arm/boot/dts/s3c6400.dtsi
 create mode 100644 arch/arm/boot/dts/s3c6410-mini6410.dts
 create mode 100644 arch/arm/boot/dts/s3c6410.dtsi
 create mode 100644 arch/arm/boot/dts/s3c64xx.dtsi
 create mode 100644 arch/arm/mach-s3c64xx/mach-s3c64xx-dt.c

Any other comments on this series?