All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] ARM: dts: AM4372: cpu(s) node per latest binding
@ 2013-08-02 13:46 ` Afzal Mohammed
  0 siblings, 0 replies; 20+ messages in thread
From: Afzal Mohammed @ 2013-08-02 13:46 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, devicetree
  Cc: Mark Rutland, Paul Walmsley, Russell King, Ian Campbell,
	Pawel Moll, Stephen Warren, Tony Lindgren, benoit.cousson,
	Rob Herring, Benoit Cousson

Update AM4372 cpu node to the latest cpus/cpu bindings for ARM.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 arch/arm/boot/dts/am4372.dtsi |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index ddc1df7..4635e7f 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -22,8 +22,12 @@
 	};
 
 	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
 		cpu@0 {
 			compatible = "arm,cortex-a9";
+			device_type = "cpu";
+			reg = <0>;
 		};
 	};
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 1/2] ARM: dts: AM4372: cpu(s) node per latest binding
@ 2013-08-02 13:46 ` Afzal Mohammed
  0 siblings, 0 replies; 20+ messages in thread
From: Afzal Mohammed @ 2013-08-02 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

Update AM4372 cpu node to the latest cpus/cpu bindings for ARM.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 arch/arm/boot/dts/am4372.dtsi |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index ddc1df7..4635e7f 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -22,8 +22,12 @@
 	};
 
 	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
 		cpu at 0 {
 			compatible = "arm,cortex-a9";
+			device_type = "cpu";
+			reg = <0>;
 		};
 	};
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/2] ARM: dts: AM4372: add few nodes
  2013-08-02 13:46 ` Afzal Mohammed
@ 2013-08-02 13:46   ` Afzal Mohammed
  -1 siblings, 0 replies; 20+ messages in thread
From: Afzal Mohammed @ 2013-08-02 13:46 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, devicetree
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Russell King, Benoit Cousson, benoit.cousson,
	Paul Walmsley, Tony Lindgren

Populate uarts, timers, rtc, wdt, gpio, i2c, spi, cpsw & pwm nodes.

Reason for adding these nodes early - hwmod code required address
space of peripherals corresponding to these nodes (as address space
details are removed from hwmod database).

uart0, timers - 1 & 2 and synctimer were already present, so here the
remaining uarts & timers are added.

All properties as per the existing binding has been added for uart,
timer, rtc, wdt & gpio. Even though that was not the current scope
of work, felt adding those would reduce or require no effort later
to get these peripherals working.

For i2c, spi, cpsw & pwm - only the properties that were sure to be
correct has been added (main intention is to make hwmod happy and
avoid any later modification to here added properties).

While at it add "ti,hwmod" property to already existing nodes.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 arch/arm/boot/dts/am4372.dtsi |  343 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 343 insertions(+)

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index 4635e7f..0fe393a 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -49,6 +49,47 @@
 			compatible = "ti,am4372-uart","ti,omap2-uart";
 			reg = <0x44e09000 0x2000>;
 			interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "uart1";
+		};
+
+		uart1: serial@48022000 {
+			compatible = "ti,am4372-uart","ti,omap2-uart";
+			reg = <0x48022000 0x2000>;
+			interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "uart2";
+			status = "disabled";
+		};
+
+		uart2: serial@48024000 {
+			compatible = "ti,am4372-uart","ti,omap2-uart";
+			reg = <0x48024000 0x2000>;
+			interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "uart3";
+			status = "disabled";
+		};
+
+		uart3: serial@481a6000 {
+			compatible = "ti,am4372-uart","ti,omap2-uart";
+			reg = <0x481a6000 0x2000>;
+			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "uart4";
+			status = "disabled";
+		};
+
+		uart4: serial@481a8000 {
+			compatible = "ti,am4372-uart","ti,omap2-uart";
+			reg = <0x481a8000 0x2000>;
+			interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "uart5";
+			status = "disabled";
+		};
+
+		uart5: serial@481aa000 {
+			compatible = "ti,am4372-uart","ti,omap2-uart";
+			reg = <0x481aa000 0x2000>;
+			interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "uart6";
+			status = "disabled";
 		};
 
 		timer1: timer@44e31000 {
@@ -56,17 +97,319 @@
 			reg = <0x44e31000 0x400>;
 			interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
 			ti,timer-alwon;
+			ti,hwmods = "timer1";
 		};
 
 		timer2: timer@48040000  {
 			compatible = "ti,am4372-timer","ti,am335x-timer";
 			reg = <0x48040000  0x400>;
 			interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "timer2";
+		};
+
+		timer3: timer@48042000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x48042000 0x400>;
+			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "timer3";
+			status = "disabled";
+		};
+
+		timer4: timer@48044000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x48044000 0x400>;
+			interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
+			ti,timer-pwm;
+			ti,hwmods = "timer4";
+			status = "disabled";
+		};
+
+		timer5: timer@48046000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x48046000 0x400>;
+			interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
+			ti,timer-pwm;
+			ti,hwmods = "timer5";
+			status = "disabled";
+		};
+
+		timer6: timer@48048000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x48048000 0x400>;
+			interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
+			ti,timer-pwm;
+			ti,hwmods = "timer6";
+			status = "disabled";
+		};
+
+		timer7: timer@4804a000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x4804a000 0x400>;
+			interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
+			ti,timer-pwm;
+			ti,hwmods = "timer7";
+			status = "disabled";
+		};
+
+		timer8: timer@481c1000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x481c1000 0x400>;
+			interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "timer8";
+			status = "disabled";
+		};
+
+		timer9: timer@4833d000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x4833d000 0x400>;
+			interrupts = <GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "timer9";
+			status = "disabled";
+		};
+
+		timer10: timer@4833f000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x4833f000 0x400>;
+			interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "timer10";
+			status = "disabled";
+		};
+
+		timer11: timer@48341000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x48341000 0x400>;
+			interrupts = <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "timer11";
+			status = "disabled";
 		};
 
 		counter32k: counter@44e86000 {
 			compatible = "ti,am4372-counter32k","ti,omap-counter32k";
 			reg = <0x44e86000 0x40>;
+			ti,hwmods = "counter_32k";
+		};
+
+		rtc@44e3e000 {
+			compatible = "ti,am4372-rtc","ti,da830-rtc";
+			reg = <0x44e3e000 0x1000>;
+			interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH
+				      GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "rtc";
+			status = "disabled";
+		};
+
+		wdt@44e35000 {
+			compatible = "ti,am4372-wdt","ti,omap3-wdt";
+			reg = <0x44e35000 0x1000>;
+			interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "wd_timer2";
+			status = "disabled";
+		};
+
+		gpio0: gpio@44e07000 {
+			compatible = "ti,am4372-gpio","ti,omap4-gpio";
+			reg = <0x44e07000 0x1000>;
+			interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			ti,hwmods = "gpio1";
+			status = "disabled";
+		};
+
+		gpio1: gpio@4804c000 {
+			compatible = "ti,am4372-gpio","ti,omap4-gpio";
+			reg = <0x4804c000 0x1000>;
+			interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			ti,hwmods = "gpio2";
+			status = "disabled";
+		};
+
+		gpio2: gpio@481ac000 {
+			compatible = "ti,am4372-gpio","ti,omap4-gpio";
+			reg = <0x481ac000 0x1000>;
+			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			ti,hwmods = "gpio3";
+			status = "disabled";
+		};
+
+		gpio3: gpio@481ae000 {
+			compatible = "ti,am4372-gpio","ti,omap4-gpio";
+			reg = <0x481ae000 0x1000>;
+			interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			ti,hwmods = "gpio4";
+			status = "disabled";
+		};
+
+		gpio4: gpio@48320000 {
+			compatible = "ti,am4372-gpio","ti,omap4-gpio";
+			reg = <0x48320000 0x1000>;
+			interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			ti,hwmods = "gpio5";
+			status = "disabled";
+		};
+
+		gpio5: gpio@48322000 {
+			compatible = "ti,am4372-gpio","ti,omap4-gpio";
+			reg = <0x48322000 0x1000>;
+			interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			ti,hwmods = "gpio6";
+			status = "disabled";
+		};
+
+		i2c0: i2c@44e0b000 {
+			compatible = "ti,am4372-i2c","ti,omap4-i2c";
+			reg = <0x44e0b000 0x1000>;
+			interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "i2c1";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c1: i2c@4802a000 {
+			compatible = "ti,am4372-i2c","ti,omap4-i2c";
+			reg = <0x4802a000 0x1000>;
+			interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "i2c2";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c2: i2c@4819c000 {
+			compatible = "ti,am4372-i2c","ti,omap4-i2c";
+			reg = <0x4819c000 0x1000>;
+			interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "i2c3";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi0: spi@48030000 {
+			compatible = "ti,am4372-mcspi","ti,omap4-mcspi";
+			reg = <0x48030000 0x400>;
+			interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "spi0";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi1: spi@481a0000 {
+			compatible = "ti,am4372-mcspi","ti,omap4-mcspi";
+			reg = <0x481a0000 0x400>;
+			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "spi1";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi2: spi@481a2000 {
+			compatible = "ti,am4372-mcspi","ti,omap4-mcspi";
+			reg = <0x481a2000 0x400>;
+			interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "spi2";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi3: spi@481a4000 {
+			compatible = "ti,am4372-mcspi","ti,omap4-mcspi";
+			reg = <0x481a4000 0x400>;
+			interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "spi3";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi4: spi@48345000 {
+			compatible = "ti,am4372-mcspi","ti,omap4-mcspi";
+			reg = <0x48345000 0x400>;
+			interrupts = <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "spi4";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		mac: ethernet@4a100000 {
+			compatible = "ti,am4372-cpsw","ti,cpsw";
+			reg = <0x4a100000 0x800
+			       0x4a101200 0x100>;
+			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH
+				      GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH
+				      GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH
+				      GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "cpgmac0";
+			status = "disabled";
+		};
+
+		epwmss0: epwmss@48300000 {
+			compatible = "ti,am4372-pwmss","ti,am33xx-pwmss";
+			reg = <0x48300000 0x10>;
+			ti,hwmods = "epwmss0";
+			status = "disabled";
+		};
+
+		epwmss1: epwmss@48302000 {
+			compatible = "ti,am4372-pwmss","ti,am33xx-pwmss";
+			reg = <0x48302000 0x10>;
+			ti,hwmods = "epwmss1";
+			status = "disabled";
+		};
+
+		epwmss2: epwmss@48304000 {
+			compatible = "ti,am4372-pwmss","ti,am33xx-pwmss";
+			reg = <0x48304000 0x10>;
+			ti,hwmods = "epwmss2";
+			status = "disabled";
+		};
+
+		epwmss3: epwmss@48306000 {
+			compatible = "ti,am4372-pwmss","ti,am33xx-pwmss";
+			reg = <0x48306000 0x10>;
+			ti,hwmods = "epwmss3";
+			status = "disabled";
+		};
+
+		epwmss4: epwmss@48308000 {
+			compatible = "ti,am4372-pwmss","ti,am33xx-pwmss";
+			reg = <0x48308000 0x10>;
+			ti,hwmods = "epwmss4";
+			status = "disabled";
+		};
+
+		epwmss5: epwmss@4830a000 {
+			compatible = "ti,am4372-pwmss","ti,am33xx-pwmss";
+			reg = <0x4830a000 0x10>;
+			ti,hwmods = "epwmss5";
+			status = "disabled";
 		};
 	};
 };
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/2] ARM: dts: AM4372: add few nodes
@ 2013-08-02 13:46   ` Afzal Mohammed
  0 siblings, 0 replies; 20+ messages in thread
From: Afzal Mohammed @ 2013-08-02 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

Populate uarts, timers, rtc, wdt, gpio, i2c, spi, cpsw & pwm nodes.

Reason for adding these nodes early - hwmod code required address
space of peripherals corresponding to these nodes (as address space
details are removed from hwmod database).

uart0, timers - 1 & 2 and synctimer were already present, so here the
remaining uarts & timers are added.

All properties as per the existing binding has been added for uart,
timer, rtc, wdt & gpio. Even though that was not the current scope
of work, felt adding those would reduce or require no effort later
to get these peripherals working.

For i2c, spi, cpsw & pwm - only the properties that were sure to be
correct has been added (main intention is to make hwmod happy and
avoid any later modification to here added properties).

While at it add "ti,hwmod" property to already existing nodes.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 arch/arm/boot/dts/am4372.dtsi |  343 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 343 insertions(+)

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index 4635e7f..0fe393a 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -49,6 +49,47 @@
 			compatible = "ti,am4372-uart","ti,omap2-uart";
 			reg = <0x44e09000 0x2000>;
 			interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "uart1";
+		};
+
+		uart1: serial at 48022000 {
+			compatible = "ti,am4372-uart","ti,omap2-uart";
+			reg = <0x48022000 0x2000>;
+			interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "uart2";
+			status = "disabled";
+		};
+
+		uart2: serial at 48024000 {
+			compatible = "ti,am4372-uart","ti,omap2-uart";
+			reg = <0x48024000 0x2000>;
+			interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "uart3";
+			status = "disabled";
+		};
+
+		uart3: serial at 481a6000 {
+			compatible = "ti,am4372-uart","ti,omap2-uart";
+			reg = <0x481a6000 0x2000>;
+			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "uart4";
+			status = "disabled";
+		};
+
+		uart4: serial at 481a8000 {
+			compatible = "ti,am4372-uart","ti,omap2-uart";
+			reg = <0x481a8000 0x2000>;
+			interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "uart5";
+			status = "disabled";
+		};
+
+		uart5: serial at 481aa000 {
+			compatible = "ti,am4372-uart","ti,omap2-uart";
+			reg = <0x481aa000 0x2000>;
+			interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "uart6";
+			status = "disabled";
 		};
 
 		timer1: timer at 44e31000 {
@@ -56,17 +97,319 @@
 			reg = <0x44e31000 0x400>;
 			interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
 			ti,timer-alwon;
+			ti,hwmods = "timer1";
 		};
 
 		timer2: timer at 48040000  {
 			compatible = "ti,am4372-timer","ti,am335x-timer";
 			reg = <0x48040000  0x400>;
 			interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "timer2";
+		};
+
+		timer3: timer at 48042000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x48042000 0x400>;
+			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "timer3";
+			status = "disabled";
+		};
+
+		timer4: timer at 48044000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x48044000 0x400>;
+			interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
+			ti,timer-pwm;
+			ti,hwmods = "timer4";
+			status = "disabled";
+		};
+
+		timer5: timer at 48046000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x48046000 0x400>;
+			interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
+			ti,timer-pwm;
+			ti,hwmods = "timer5";
+			status = "disabled";
+		};
+
+		timer6: timer at 48048000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x48048000 0x400>;
+			interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>;
+			ti,timer-pwm;
+			ti,hwmods = "timer6";
+			status = "disabled";
+		};
+
+		timer7: timer at 4804a000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x4804a000 0x400>;
+			interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
+			ti,timer-pwm;
+			ti,hwmods = "timer7";
+			status = "disabled";
+		};
+
+		timer8: timer at 481c1000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x481c1000 0x400>;
+			interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "timer8";
+			status = "disabled";
+		};
+
+		timer9: timer at 4833d000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x4833d000 0x400>;
+			interrupts = <GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "timer9";
+			status = "disabled";
+		};
+
+		timer10: timer at 4833f000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x4833f000 0x400>;
+			interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "timer10";
+			status = "disabled";
+		};
+
+		timer11: timer at 48341000 {
+			compatible = "ti,am4372-timer","ti,am335x-timer";
+			reg = <0x48341000 0x400>;
+			interrupts = <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "timer11";
+			status = "disabled";
 		};
 
 		counter32k: counter at 44e86000 {
 			compatible = "ti,am4372-counter32k","ti,omap-counter32k";
 			reg = <0x44e86000 0x40>;
+			ti,hwmods = "counter_32k";
+		};
+
+		rtc at 44e3e000 {
+			compatible = "ti,am4372-rtc","ti,da830-rtc";
+			reg = <0x44e3e000 0x1000>;
+			interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH
+				      GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "rtc";
+			status = "disabled";
+		};
+
+		wdt at 44e35000 {
+			compatible = "ti,am4372-wdt","ti,omap3-wdt";
+			reg = <0x44e35000 0x1000>;
+			interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "wd_timer2";
+			status = "disabled";
+		};
+
+		gpio0: gpio at 44e07000 {
+			compatible = "ti,am4372-gpio","ti,omap4-gpio";
+			reg = <0x44e07000 0x1000>;
+			interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			ti,hwmods = "gpio1";
+			status = "disabled";
+		};
+
+		gpio1: gpio at 4804c000 {
+			compatible = "ti,am4372-gpio","ti,omap4-gpio";
+			reg = <0x4804c000 0x1000>;
+			interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			ti,hwmods = "gpio2";
+			status = "disabled";
+		};
+
+		gpio2: gpio at 481ac000 {
+			compatible = "ti,am4372-gpio","ti,omap4-gpio";
+			reg = <0x481ac000 0x1000>;
+			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			ti,hwmods = "gpio3";
+			status = "disabled";
+		};
+
+		gpio3: gpio at 481ae000 {
+			compatible = "ti,am4372-gpio","ti,omap4-gpio";
+			reg = <0x481ae000 0x1000>;
+			interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			ti,hwmods = "gpio4";
+			status = "disabled";
+		};
+
+		gpio4: gpio at 48320000 {
+			compatible = "ti,am4372-gpio","ti,omap4-gpio";
+			reg = <0x48320000 0x1000>;
+			interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			ti,hwmods = "gpio5";
+			status = "disabled";
+		};
+
+		gpio5: gpio at 48322000 {
+			compatible = "ti,am4372-gpio","ti,omap4-gpio";
+			reg = <0x48322000 0x1000>;
+			interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			ti,hwmods = "gpio6";
+			status = "disabled";
+		};
+
+		i2c0: i2c at 44e0b000 {
+			compatible = "ti,am4372-i2c","ti,omap4-i2c";
+			reg = <0x44e0b000 0x1000>;
+			interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "i2c1";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c1: i2c at 4802a000 {
+			compatible = "ti,am4372-i2c","ti,omap4-i2c";
+			reg = <0x4802a000 0x1000>;
+			interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "i2c2";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c2: i2c at 4819c000 {
+			compatible = "ti,am4372-i2c","ti,omap4-i2c";
+			reg = <0x4819c000 0x1000>;
+			interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "i2c3";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi0: spi at 48030000 {
+			compatible = "ti,am4372-mcspi","ti,omap4-mcspi";
+			reg = <0x48030000 0x400>;
+			interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "spi0";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi1: spi at 481a0000 {
+			compatible = "ti,am4372-mcspi","ti,omap4-mcspi";
+			reg = <0x481a0000 0x400>;
+			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "spi1";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi2: spi at 481a2000 {
+			compatible = "ti,am4372-mcspi","ti,omap4-mcspi";
+			reg = <0x481a2000 0x400>;
+			interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "spi2";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi3: spi at 481a4000 {
+			compatible = "ti,am4372-mcspi","ti,omap4-mcspi";
+			reg = <0x481a4000 0x400>;
+			interrupts = <GIC_SPI 136 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "spi3";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi4: spi at 48345000 {
+			compatible = "ti,am4372-mcspi","ti,omap4-mcspi";
+			reg = <0x48345000 0x400>;
+			interrupts = <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "spi4";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		mac: ethernet at 4a100000 {
+			compatible = "ti,am4372-cpsw","ti,cpsw";
+			reg = <0x4a100000 0x800
+			       0x4a101200 0x100>;
+			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH
+				      GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH
+				      GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH
+				      GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "cpgmac0";
+			status = "disabled";
+		};
+
+		epwmss0: epwmss at 48300000 {
+			compatible = "ti,am4372-pwmss","ti,am33xx-pwmss";
+			reg = <0x48300000 0x10>;
+			ti,hwmods = "epwmss0";
+			status = "disabled";
+		};
+
+		epwmss1: epwmss at 48302000 {
+			compatible = "ti,am4372-pwmss","ti,am33xx-pwmss";
+			reg = <0x48302000 0x10>;
+			ti,hwmods = "epwmss1";
+			status = "disabled";
+		};
+
+		epwmss2: epwmss at 48304000 {
+			compatible = "ti,am4372-pwmss","ti,am33xx-pwmss";
+			reg = <0x48304000 0x10>;
+			ti,hwmods = "epwmss2";
+			status = "disabled";
+		};
+
+		epwmss3: epwmss at 48306000 {
+			compatible = "ti,am4372-pwmss","ti,am33xx-pwmss";
+			reg = <0x48306000 0x10>;
+			ti,hwmods = "epwmss3";
+			status = "disabled";
+		};
+
+		epwmss4: epwmss at 48308000 {
+			compatible = "ti,am4372-pwmss","ti,am33xx-pwmss";
+			reg = <0x48308000 0x10>;
+			ti,hwmods = "epwmss4";
+			status = "disabled";
+		};
+
+		epwmss5: epwmss at 4830a000 {
+			compatible = "ti,am4372-pwmss","ti,am33xx-pwmss";
+			reg = <0x4830a000 0x10>;
+			ti,hwmods = "epwmss5";
+			status = "disabled";
 		};
 	};
 };
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ARM: dts: AM4372: add few nodes
  2013-08-02 13:46   ` Afzal Mohammed
@ 2013-08-03 11:49     ` Mugunthan V N
  -1 siblings, 0 replies; 20+ messages in thread
From: Mugunthan V N @ 2013-08-03 11:49 UTC (permalink / raw)
  To: Afzal Mohammed
  Cc: linux-omap, linux-arm-kernel, devicetree, Rob Herring,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	Russell King, Benoit Cousson, benoit.cousson, Paul Walmsley,
	Tony Lindgren

On 8/2/2013 7:16 PM, Afzal Mohammed wrote:
> +		mac: ethernet@4a100000 {
> +			compatible = "ti,am4372-cpsw","ti,cpsw";
compatibility "ti,am4372-cpsw" is not needed as driver has only "ti,cpsw"
compatibility
> +			reg = <0x4a100000 0x800
> +			       0x4a101200 0x100>;
> +			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH
> +				      GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH
> +				      GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH
> +				      GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> +			ti,hwmods = "cpgmac0";
> +			status = "disabled";
> +		};
There are many other parameters which are missed here.

Regards
Mugunthan V N

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/2] ARM: dts: AM4372: add few nodes
@ 2013-08-03 11:49     ` Mugunthan V N
  0 siblings, 0 replies; 20+ messages in thread
From: Mugunthan V N @ 2013-08-03 11:49 UTC (permalink / raw)
  To: linux-arm-kernel

On 8/2/2013 7:16 PM, Afzal Mohammed wrote:
> +		mac: ethernet at 4a100000 {
> +			compatible = "ti,am4372-cpsw","ti,cpsw";
compatibility "ti,am4372-cpsw" is not needed as driver has only "ti,cpsw"
compatibility
> +			reg = <0x4a100000 0x800
> +			       0x4a101200 0x100>;
> +			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH
> +				      GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH
> +				      GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH
> +				      GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> +			ti,hwmods = "cpgmac0";
> +			status = "disabled";
> +		};
There are many other parameters which are missed here.

Regards
Mugunthan V N

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ARM: dts: AM4372: add few nodes
  2013-08-03 11:49     ` Mugunthan V N
@ 2013-08-05  5:08       ` Afzal Mohammed
  -1 siblings, 0 replies; 20+ messages in thread
From: Afzal Mohammed @ 2013-08-05  5:08 UTC (permalink / raw)
  To: Mugunthan V N
  Cc: linux-omap, linux-arm-kernel, devicetree, Rob Herring,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	Russell King, Benoit Cousson, benoit.cousson, Paul Walmsley,
	Tony Lindgren

Hi Muguthan,

On Saturday 03 August 2013 05:19 PM, Mugunthan V N wrote:
> On 8/2/2013 7:16 PM, Afzal Mohammed wrote:

>> +		mac: ethernet@4a100000 {
>> +			compatible = "ti,am4372-cpsw","ti,cpsw";

> compatibility "ti,am4372-cpsw" is not needed as driver has only "ti,cpsw"
> compatibility

No, please read device tree documentation [1].

DT is a pure hardware description, it does not depend on driver, 
dependency is only vice versa.

>> +			reg = <0x4a100000 0x800
>> +			       0x4a101200 0x100>;
>> +			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH
>> +				      GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH
>> +				      GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH
>> +				      GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
>> +			ti,hwmods = "cpgmac0";
>> +			status = "disabled";
>> +		};

> There are many other parameters which are missed here.

Reason has been mentioned in the commit message, quoting relevant here 
again,

 >> For i2c, spi, cpsw & pwm - only the properties that were sure to be
 >> correct has been added (main intention is to make hwmod happy and
 >> avoid any later modification to here added properties).

I really wanted to avoid a later patch that has a line starting with 
minus on DTS.

Since you are working on cpsw support, can you help here with a patch 
for other properties.

Regards
Afzal

[1] 
http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/2] ARM: dts: AM4372: add few nodes
@ 2013-08-05  5:08       ` Afzal Mohammed
  0 siblings, 0 replies; 20+ messages in thread
From: Afzal Mohammed @ 2013-08-05  5:08 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Muguthan,

On Saturday 03 August 2013 05:19 PM, Mugunthan V N wrote:
> On 8/2/2013 7:16 PM, Afzal Mohammed wrote:

>> +		mac: ethernet at 4a100000 {
>> +			compatible = "ti,am4372-cpsw","ti,cpsw";

> compatibility "ti,am4372-cpsw" is not needed as driver has only "ti,cpsw"
> compatibility

No, please read device tree documentation [1].

DT is a pure hardware description, it does not depend on driver, 
dependency is only vice versa.

>> +			reg = <0x4a100000 0x800
>> +			       0x4a101200 0x100>;
>> +			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH
>> +				      GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH
>> +				      GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH
>> +				      GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
>> +			ti,hwmods = "cpgmac0";
>> +			status = "disabled";
>> +		};

> There are many other parameters which are missed here.

Reason has been mentioned in the commit message, quoting relevant here 
again,

 >> For i2c, spi, cpsw & pwm - only the properties that were sure to be
 >> correct has been added (main intention is to make hwmod happy and
 >> avoid any later modification to here added properties).

I really wanted to avoid a later patch that has a line starting with 
minus on DTS.

Since you are working on cpsw support, can you help here with a patch 
for other properties.

Regards
Afzal

[1] 
http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ARM: dts: AM4372: add few nodes
  2013-08-05  5:08       ` Afzal Mohammed
@ 2013-08-05  6:06         ` Mugunthan V N
  -1 siblings, 0 replies; 20+ messages in thread
From: Mugunthan V N @ 2013-08-05  6:06 UTC (permalink / raw)
  To: Afzal Mohammed
  Cc: Mark Rutland, devicetree, Paul Walmsley, Russell King,
	Ian Campbell, Pawel Moll, Stephen Warren, Tony Lindgren,
	benoit.cousson, Rob Herring, Benoit Cousson, linux-omap,
	linux-arm-kernel

On Monday 05 August 2013 10:38 AM, Afzal Mohammed wrote:
> Hi Muguthan,
>
> On Saturday 03 August 2013 05:19 PM, Mugunthan V N wrote:
>> On 8/2/2013 7:16 PM, Afzal Mohammed wrote:
>
>>> +        mac: ethernet@4a100000 {
>>> +            compatible = "ti,am4372-cpsw","ti,cpsw";
>
>> compatibility "ti,am4372-cpsw" is not needed as driver has only
>> "ti,cpsw"
>> compatibility
>
> No, please read device tree documentation [1].
>
> DT is a pure hardware description, it does not depend on driver,
> dependency is only vice versa.

Thanks for the clarification

>
>>> +            reg = <0x4a100000 0x800
>>> +                   0x4a101200 0x100>;
>>> +            interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH
>>> +                      GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH
>>> +                      GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH
>>> +                      GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
>>> +            ti,hwmods = "cpgmac0";
>>> +            status = "disabled";
>>> +        };
>
>> There are many other parameters which are missed here.
>
> Reason has been mentioned in the commit message, quoting relevant here
> again,
>
> >> For i2c, spi, cpsw & pwm - only the properties that were sure to be
> >> correct has been added (main intention is to make hwmod happy and
> >> avoid any later modification to here added properties).
>
> I really wanted to avoid a later patch that has a line starting with
> minus on DTS.
>
> Since you are working on cpsw support, can you help here with a patch
> for other properties.

Sure I can help you to add more DT entries which will not change
according to hardware

Regards
Mugunthan V N

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/2] ARM: dts: AM4372: add few nodes
@ 2013-08-05  6:06         ` Mugunthan V N
  0 siblings, 0 replies; 20+ messages in thread
From: Mugunthan V N @ 2013-08-05  6:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Monday 05 August 2013 10:38 AM, Afzal Mohammed wrote:
> Hi Muguthan,
>
> On Saturday 03 August 2013 05:19 PM, Mugunthan V N wrote:
>> On 8/2/2013 7:16 PM, Afzal Mohammed wrote:
>
>>> +        mac: ethernet at 4a100000 {
>>> +            compatible = "ti,am4372-cpsw","ti,cpsw";
>
>> compatibility "ti,am4372-cpsw" is not needed as driver has only
>> "ti,cpsw"
>> compatibility
>
> No, please read device tree documentation [1].
>
> DT is a pure hardware description, it does not depend on driver,
> dependency is only vice versa.

Thanks for the clarification

>
>>> +            reg = <0x4a100000 0x800
>>> +                   0x4a101200 0x100>;
>>> +            interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH
>>> +                      GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH
>>> +                      GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH
>>> +                      GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
>>> +            ti,hwmods = "cpgmac0";
>>> +            status = "disabled";
>>> +        };
>
>> There are many other parameters which are missed here.
>
> Reason has been mentioned in the commit message, quoting relevant here
> again,
>
> >> For i2c, spi, cpsw & pwm - only the properties that were sure to be
> >> correct has been added (main intention is to make hwmod happy and
> >> avoid any later modification to here added properties).
>
> I really wanted to avoid a later patch that has a line starting with
> minus on DTS.
>
> Since you are working on cpsw support, can you help here with a patch
> for other properties.

Sure I can help you to add more DT entries which will not change
according to hardware

Regards
Mugunthan V N

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] ARM: dts: AM4372: cpu(s) node per latest binding
  2013-08-02 13:46 ` Afzal Mohammed
@ 2013-08-10 14:13   ` Mark Rutland
  -1 siblings, 0 replies; 20+ messages in thread
From: Mark Rutland @ 2013-08-10 14:13 UTC (permalink / raw)
  To: Afzal Mohammed
  Cc: devicetree, Paul Walmsley, Russell King, Ian Campbell,
	Pawel Moll, Stephen Warren, Tony Lindgren, benoit.cousson,
	rob.herring, Benoit Cousson, linux-omap, linux-arm-kernel

On Fri, Aug 02, 2013 at 02:46:13PM +0100, Afzal Mohammed wrote:
> Update AM4372 cpu node to the latest cpus/cpu bindings for ARM.
> 
> Signed-off-by: Afzal Mohammed <afzal@ti.com>
> ---
>  arch/arm/boot/dts/am4372.dtsi |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
> index ddc1df7..4635e7f 100644
> --- a/arch/arm/boot/dts/am4372.dtsi
> +++ b/arch/arm/boot/dts/am4372.dtsi
> @@ -22,8 +22,12 @@
>  	};
>  
>  	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
>  		cpu@0 {
>  			compatible = "arm,cortex-a9";
> +			device_type = "cpu";
> +			reg = <0>;
>  		};
>  	};
>  

This looks sane to me:

Acked-by: Mark Rutland <mark.rutland@arm.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/2] ARM: dts: AM4372: cpu(s) node per latest binding
@ 2013-08-10 14:13   ` Mark Rutland
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Rutland @ 2013-08-10 14:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Aug 02, 2013 at 02:46:13PM +0100, Afzal Mohammed wrote:
> Update AM4372 cpu node to the latest cpus/cpu bindings for ARM.
> 
> Signed-off-by: Afzal Mohammed <afzal@ti.com>
> ---
>  arch/arm/boot/dts/am4372.dtsi |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
> index ddc1df7..4635e7f 100644
> --- a/arch/arm/boot/dts/am4372.dtsi
> +++ b/arch/arm/boot/dts/am4372.dtsi
> @@ -22,8 +22,12 @@
>  	};
>  
>  	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
>  		cpu at 0 {
>  			compatible = "arm,cortex-a9";
> +			device_type = "cpu";
> +			reg = <0>;
>  		};
>  	};
>  

This looks sane to me:

Acked-by: Mark Rutland <mark.rutland@arm.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ARM: dts: AM4372: add few nodes
  2013-08-05  5:08       ` Afzal Mohammed
@ 2013-08-10 14:23         ` Mark Rutland
  -1 siblings, 0 replies; 20+ messages in thread
From: Mark Rutland @ 2013-08-10 14:23 UTC (permalink / raw)
  To: Afzal Mohammed
  Cc: Mugunthan V N, linux-omap, linux-arm-kernel, devicetree,
	rob.herring, Pawel Moll, Stephen Warren, Ian Campbell,
	Russell King, Benoit Cousson, benoit.cousson, Paul Walmsley,
	Tony Lindgren

On Mon, Aug 05, 2013 at 06:08:45AM +0100, Afzal Mohammed wrote:
> Hi Muguthan,
> 
> On Saturday 03 August 2013 05:19 PM, Mugunthan V N wrote:
> > On 8/2/2013 7:16 PM, Afzal Mohammed wrote:
> 
> >> +		mac: ethernet@4a100000 {
> >> +			compatible = "ti,am4372-cpsw","ti,cpsw";
> 
> > compatibility "ti,am4372-cpsw" is not needed as driver has only "ti,cpsw"
> > compatibility
> 
> No, please read device tree documentation [1].
> 
> DT is a pure hardware description, it does not depend on driver, 
> dependency is only vice versa.

One point worth mentioning is that the "ti,am4372-cpsw" string isn't
documented. Will the "ti,am4372-cpsw" binding definitely be a superset
of the "ti,cpsw" binding, and if you were to take the DT as of this
patch, and attempt to use it with a future kernel, can you guarantee
it'll work? 

> 
> >> +			reg = <0x4a100000 0x800
> >> +			       0x4a101200 0x100>;
> >> +			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH
> >> +				      GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH
> >> +				      GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH
> >> +				      GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> >> +			ti,hwmods = "cpgmac0";
> >> +			status = "disabled";
> >> +		};
> 
> > There are many other parameters which are missed here.
> 
> Reason has been mentioned in the commit message, quoting relevant here 
> again,

Actually, as you've marked the nodes disabled, it's probably fine. But
worth considering as properties are added...

Thanks,
Mark.

> 
>  >> For i2c, spi, cpsw & pwm - only the properties that were sure to be
>  >> correct has been added (main intention is to make hwmod happy and
>  >> avoid any later modification to here added properties).
> 
> I really wanted to avoid a later patch that has a line starting with 
> minus on DTS.
> 
> Since you are working on cpsw support, can you help here with a patch 
> for other properties.
> 
> Regards
> Afzal
> 
> [1] 
> http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/2] ARM: dts: AM4372: add few nodes
@ 2013-08-10 14:23         ` Mark Rutland
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Rutland @ 2013-08-10 14:23 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Aug 05, 2013 at 06:08:45AM +0100, Afzal Mohammed wrote:
> Hi Muguthan,
> 
> On Saturday 03 August 2013 05:19 PM, Mugunthan V N wrote:
> > On 8/2/2013 7:16 PM, Afzal Mohammed wrote:
> 
> >> +		mac: ethernet at 4a100000 {
> >> +			compatible = "ti,am4372-cpsw","ti,cpsw";
> 
> > compatibility "ti,am4372-cpsw" is not needed as driver has only "ti,cpsw"
> > compatibility
> 
> No, please read device tree documentation [1].
> 
> DT is a pure hardware description, it does not depend on driver, 
> dependency is only vice versa.

One point worth mentioning is that the "ti,am4372-cpsw" string isn't
documented. Will the "ti,am4372-cpsw" binding definitely be a superset
of the "ti,cpsw" binding, and if you were to take the DT as of this
patch, and attempt to use it with a future kernel, can you guarantee
it'll work? 

> 
> >> +			reg = <0x4a100000 0x800
> >> +			       0x4a101200 0x100>;
> >> +			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH
> >> +				      GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH
> >> +				      GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH
> >> +				      GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> >> +			ti,hwmods = "cpgmac0";
> >> +			status = "disabled";
> >> +		};
> 
> > There are many other parameters which are missed here.
> 
> Reason has been mentioned in the commit message, quoting relevant here 
> again,

Actually, as you've marked the nodes disabled, it's probably fine. But
worth considering as properties are added...

Thanks,
Mark.

> 
>  >> For i2c, spi, cpsw & pwm - only the properties that were sure to be
>  >> correct has been added (main intention is to make hwmod happy and
>  >> avoid any later modification to here added properties).
> 
> I really wanted to avoid a later patch that has a line starting with 
> minus on DTS.
> 
> Since you are working on cpsw support, can you help here with a patch 
> for other properties.
> 
> Regards
> Afzal
> 
> [1] 
> http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ARM: dts: AM4372: add few nodes
  2013-08-10 14:23         ` Mark Rutland
@ 2013-08-12  6:48           ` Afzal Mohammed
  -1 siblings, 0 replies; 20+ messages in thread
From: Afzal Mohammed @ 2013-08-12  6:48 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Mugunthan V N, linux-omap, linux-arm-kernel, devicetree,
	rob.herring, Pawel Moll, Stephen Warren, Ian Campbell,
	Russell King, Benoit Cousson, benoit.cousson, Paul Walmsley,
	Tony Lindgren

Hi Mark,

On Saturday 10 August 2013 07:53 PM, Mark Rutland wrote:

>>>> +		mac: ethernet@4a100000 {
>>>> +			compatible = "ti,am4372-cpsw","ti,cpsw";

> One point worth mentioning is that the "ti,am4372-cpsw" string isn't
> documented. Will the "ti,am4372-cpsw" binding definitely be a superset
> of the "ti,cpsw" binding, and if you were to take the DT as of this
> patch, and attempt to use it with a future kernel, can you guarantee
> it'll work?

"ti,am4372-cpsw" was not documented as OMAP DT maintainer didn't prefer 
documenting only for a new compatible.

For the only patch on this file that went into mainline, in that series 
actually I had posted patches to document "ti,am4372-*", but as 
maintainer didn't agree, it was discarded [A].

Regarding whether "ti,am4372-cpsw" would be a superset of "ti,cpsw", 
information I have (am4372 is in pre silicon stage) is that it is a 
reuse from am335x ("ti,cpsw" should have been named "ti,am3352-cpsw" or 
something like that, but nothing can be done now) with minor changes and 
all existing functionalities available, Mugunthan can shed more light on 
this, Mugunthan ?

As mentioned at other places in the thread, for cpsw, only a few 
properties to prevent hwmod code crash was added. I am sure that 
currently added properties for cpsw will not make driver functional this 
Kernel version (if this goes in) or future versions. To make driver work 
additional properties are required.

>>> There are many other parameters which are missed here.
>>
>> Reason has been mentioned in the commit message, quoting relevant here
>> again,
>
> Actually, as you've marked the nodes disabled, it's probably fine. But
> worth considering as properties are added...

There were two factors that was considered while adding cpsw node
1. DT as an ABI
2. While adding DT node, whether all existing required properties has to 
be added

Regarding 1 - DT would not make driver work for this Kernel version and 
also for not any newer Kernel version, I believe this does not go 
against DT an an ABI, although in a negative sense. To make driver work 
more DT properties would have to be added.

Regarding 2 - Currently, I believe most required & optional properties 
in bindings are defined w.r.t driver (bringing in a Linux attachment). 
If DT is only a hardware description, required & optional properties 
should correspond to something that are a must for hardware to work and 
additional features that can be used respectively. In that sense 
interrupts property for many of the peripherals would have to be 
considered optional, as it is possible to work in polling mode. And 
would it be practical for DT in most of the cases to be a complete 
hardware description ?, as to completely describe hardware, we may need 
to have a large amount of properties that may not be relevant to 
software. If requirement is only that DT should describe hardware that 
is relevant for software (this would bring in a software dependency for 
DT), required property for one software may not be required for another 
piece of software or may be an optional property (like in the case of 
interrupts as explained above).

So conclusion arrived within me was that as long as properties are not 
modified, but only added and as long as it does not go against DT as an 
ABI, it is ok.

I would like to hear from DT maintainers what the approach should be.

Regards
Afzal

[A] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg89408.html


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/2] ARM: dts: AM4372: add few nodes
@ 2013-08-12  6:48           ` Afzal Mohammed
  0 siblings, 0 replies; 20+ messages in thread
From: Afzal Mohammed @ 2013-08-12  6:48 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Mark,

On Saturday 10 August 2013 07:53 PM, Mark Rutland wrote:

>>>> +		mac: ethernet at 4a100000 {
>>>> +			compatible = "ti,am4372-cpsw","ti,cpsw";

> One point worth mentioning is that the "ti,am4372-cpsw" string isn't
> documented. Will the "ti,am4372-cpsw" binding definitely be a superset
> of the "ti,cpsw" binding, and if you were to take the DT as of this
> patch, and attempt to use it with a future kernel, can you guarantee
> it'll work?

"ti,am4372-cpsw" was not documented as OMAP DT maintainer didn't prefer 
documenting only for a new compatible.

For the only patch on this file that went into mainline, in that series 
actually I had posted patches to document "ti,am4372-*", but as 
maintainer didn't agree, it was discarded [A].

Regarding whether "ti,am4372-cpsw" would be a superset of "ti,cpsw", 
information I have (am4372 is in pre silicon stage) is that it is a 
reuse from am335x ("ti,cpsw" should have been named "ti,am3352-cpsw" or 
something like that, but nothing can be done now) with minor changes and 
all existing functionalities available, Mugunthan can shed more light on 
this, Mugunthan ?

As mentioned at other places in the thread, for cpsw, only a few 
properties to prevent hwmod code crash was added. I am sure that 
currently added properties for cpsw will not make driver functional this 
Kernel version (if this goes in) or future versions. To make driver work 
additional properties are required.

>>> There are many other parameters which are missed here.
>>
>> Reason has been mentioned in the commit message, quoting relevant here
>> again,
>
> Actually, as you've marked the nodes disabled, it's probably fine. But
> worth considering as properties are added...

There were two factors that was considered while adding cpsw node
1. DT as an ABI
2. While adding DT node, whether all existing required properties has to 
be added

Regarding 1 - DT would not make driver work for this Kernel version and 
also for not any newer Kernel version, I believe this does not go 
against DT an an ABI, although in a negative sense. To make driver work 
more DT properties would have to be added.

Regarding 2 - Currently, I believe most required & optional properties 
in bindings are defined w.r.t driver (bringing in a Linux attachment). 
If DT is only a hardware description, required & optional properties 
should correspond to something that are a must for hardware to work and 
additional features that can be used respectively. In that sense 
interrupts property for many of the peripherals would have to be 
considered optional, as it is possible to work in polling mode. And 
would it be practical for DT in most of the cases to be a complete 
hardware description ?, as to completely describe hardware, we may need 
to have a large amount of properties that may not be relevant to 
software. If requirement is only that DT should describe hardware that 
is relevant for software (this would bring in a software dependency for 
DT), required property for one software may not be required for another 
piece of software or may be an optional property (like in the case of 
interrupts as explained above).

So conclusion arrived within me was that as long as properties are not 
modified, but only added and as long as it does not go against DT as an 
ABI, it is ok.

I would like to hear from DT maintainers what the approach should be.

Regards
Afzal

[A] http://www.mail-archive.com/linux-omap at vger.kernel.org/msg89408.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ARM: dts: AM4372: add few nodes
  2013-08-12  6:48           ` Afzal Mohammed
@ 2013-08-16 10:17             ` Benoit Cousson
  -1 siblings, 0 replies; 20+ messages in thread
From: Benoit Cousson @ 2013-08-16 10:17 UTC (permalink / raw)
  To: Afzal Mohammed
  Cc: Mark Rutland, Mugunthan V N, linux-omap, linux-arm-kernel,
	devicetree, rob.herring, Pawel Moll, Stephen Warren,
	Ian Campbell, Russell King, Paul Walmsley, Tony Lindgren

Hi Afzal,

On 12/08/2013 08:48, Afzal Mohammed wrote:
> Hi Mark,
>
> On Saturday 10 August 2013 07:53 PM, Mark Rutland wrote:
>
>>>>> +        mac: ethernet@4a100000 {
>>>>> +            compatible = "ti,am4372-cpsw","ti,cpsw";
>
>> One point worth mentioning is that the "ti,am4372-cpsw" string isn't
>> documented. Will the "ti,am4372-cpsw" binding definitely be a superset
>> of the "ti,cpsw" binding, and if you were to take the DT as of this
>> patch, and attempt to use it with a future kernel, can you guarantee
>> it'll work?
>
> "ti,am4372-cpsw" was not documented as OMAP DT maintainer didn't prefer
> documenting only for a new compatible.

My point was more that creating a new compatible for the exact same 
version of the IP is pointeless and could lead to tons of compatible 
strings that would never be used since this is always the exact same IP.

AFAIK, the rational is that we never know what will happen in the future 
and maybe, when a HW bug specific for that SoC will appear, we will have 
a way to differentiate that version.
Sure we never know what will be the future, but with rational like that 
we can quiclky clutter the kernel or the DTS with tons of useless data...

> For the only patch on this file that went into mainline, in that series
> actually I had posted patches to document "ti,am4372-*", but as
> maintainer didn't agree, it was discarded [A].
>
> Regarding whether "ti,am4372-cpsw" would be a superset of "ti,cpsw",
> information I have (am4372 is in pre silicon stage) is that it is a
> reuse from am335x ("ti,cpsw" should have been named "ti,am3352-cpsw" or
> something like that, but nothing can be done now) with minor changes and
> all existing functionalities available, Mugunthan can shed more light on
> this, Mugunthan ?

My second point is, even if we want to differentiate the IPs in various 
SoCs, using the name of the SoC in the IP compatible string is not a 
very good practice anyway.
What is relevant at driver level is most of the time the version of the 
IP. The fact that this is an am3352 or am4372 is irrelevant for the 
driver. With the exception of a different integration of the IP that 
might lead to some functionality change and thus could impact the driver.
But the fact that the ti,cpsw is integrated inside a SoC is already 
describe in the SoC DTS tree. A quick look of the hierarchy root will 
already provide the SoC name.

The current rule is thus enforcing the duplication in every nodes of a 
the exact same SoC name string whereas the information could be 
retrieved potentially from the root node.
This lead us to a bunch of useless strings in the DTS and a bunch of 
useless text in the bindings.

Bottom-line, I'm not challenging your patch, which is applying the 
current rules correctly, I'm challenging this rule that for my point of 
view is a little bit outdated. It was maybe good enough for small DTS 
file in the PPC world but is a source of unneeded overhead in our case 
for my point of view.

> As mentioned at other places in the thread, for cpsw, only a few
> properties to prevent hwmod code crash was added. I am sure that
> currently added properties for cpsw will not make driver functional this
> Kernel version (if this goes in) or future versions. To make driver work
> additional properties are required.
>
>>>> There are many other parameters which are missed here.
>>>
>>> Reason has been mentioned in the commit message, quoting relevant here
>>> again,
>>
>> Actually, as you've marked the nodes disabled, it's probably fine. But
>> worth considering as properties are added...
>
> There were two factors that was considered while adding cpsw node
> 1. DT as an ABI
> 2. While adding DT node, whether all existing required properties has to
> be added
>
> Regarding 1 - DT would not make driver work for this Kernel version and
> also for not any newer Kernel version, I believe this does not go
> against DT an an ABI, although in a negative sense. To make driver work
> more DT properties would have to be added.
>
> Regarding 2 - Currently, I believe most required & optional properties
> in bindings are defined w.r.t driver (bringing in a Linux attachment).
> If DT is only a hardware description, required & optional properties
> should correspond to something that are a must for hardware to work and
> additional features that can be used respectively. In that sense
> interrupts property for many of the peripherals would have to be
> considered optional, as it is possible to work in polling mode. And
> would it be practical for DT in most of the cases to be a complete
> hardware description ?, as to completely describe hardware, we may need
> to have a large amount of properties that may not be relevant to
> software. If requirement is only that DT should describe hardware that
> is relevant for software (this would bring in a software dependency for
> DT), required property for one software may not be required for another
> piece of software or may be an optional property (like in the case of
> interrupts as explained above).
>
> So conclusion arrived within me was that as long as properties are not
> modified, but only added and as long as it does not go against DT as an
> ABI, it is ok.
>
> I would like to hear from DT maintainers what the approach should be.

DT is clearly an ABI. It is there to provide HW information that are 
needed by the SW but cannot be extracted from the HW registers of from 
some ACPI table.

It could as well provide information about the HW hierarchy assuming 
this is relevant for the SW. That's typically why you do not have the 
full accurate description of the OMAP interconnects in DTS. That is just 
pointless for the SW point of view.

The goal is thus not to describe the full HW, which is anyway not 
realistic, but to provide the HW information needed by the SW.

And the driver is really the first one to know what is needed or not for 
a certain IP. Hence a dependency with the driver in term if binding 
definition.

Regards,
Benoit


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/2] ARM: dts: AM4372: add few nodes
@ 2013-08-16 10:17             ` Benoit Cousson
  0 siblings, 0 replies; 20+ messages in thread
From: Benoit Cousson @ 2013-08-16 10:17 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Afzal,

On 12/08/2013 08:48, Afzal Mohammed wrote:
> Hi Mark,
>
> On Saturday 10 August 2013 07:53 PM, Mark Rutland wrote:
>
>>>>> +        mac: ethernet at 4a100000 {
>>>>> +            compatible = "ti,am4372-cpsw","ti,cpsw";
>
>> One point worth mentioning is that the "ti,am4372-cpsw" string isn't
>> documented. Will the "ti,am4372-cpsw" binding definitely be a superset
>> of the "ti,cpsw" binding, and if you were to take the DT as of this
>> patch, and attempt to use it with a future kernel, can you guarantee
>> it'll work?
>
> "ti,am4372-cpsw" was not documented as OMAP DT maintainer didn't prefer
> documenting only for a new compatible.

My point was more that creating a new compatible for the exact same 
version of the IP is pointeless and could lead to tons of compatible 
strings that would never be used since this is always the exact same IP.

AFAIK, the rational is that we never know what will happen in the future 
and maybe, when a HW bug specific for that SoC will appear, we will have 
a way to differentiate that version.
Sure we never know what will be the future, but with rational like that 
we can quiclky clutter the kernel or the DTS with tons of useless data...

> For the only patch on this file that went into mainline, in that series
> actually I had posted patches to document "ti,am4372-*", but as
> maintainer didn't agree, it was discarded [A].
>
> Regarding whether "ti,am4372-cpsw" would be a superset of "ti,cpsw",
> information I have (am4372 is in pre silicon stage) is that it is a
> reuse from am335x ("ti,cpsw" should have been named "ti,am3352-cpsw" or
> something like that, but nothing can be done now) with minor changes and
> all existing functionalities available, Mugunthan can shed more light on
> this, Mugunthan ?

My second point is, even if we want to differentiate the IPs in various 
SoCs, using the name of the SoC in the IP compatible string is not a 
very good practice anyway.
What is relevant at driver level is most of the time the version of the 
IP. The fact that this is an am3352 or am4372 is irrelevant for the 
driver. With the exception of a different integration of the IP that 
might lead to some functionality change and thus could impact the driver.
But the fact that the ti,cpsw is integrated inside a SoC is already 
describe in the SoC DTS tree. A quick look of the hierarchy root will 
already provide the SoC name.

The current rule is thus enforcing the duplication in every nodes of a 
the exact same SoC name string whereas the information could be 
retrieved potentially from the root node.
This lead us to a bunch of useless strings in the DTS and a bunch of 
useless text in the bindings.

Bottom-line, I'm not challenging your patch, which is applying the 
current rules correctly, I'm challenging this rule that for my point of 
view is a little bit outdated. It was maybe good enough for small DTS 
file in the PPC world but is a source of unneeded overhead in our case 
for my point of view.

> As mentioned at other places in the thread, for cpsw, only a few
> properties to prevent hwmod code crash was added. I am sure that
> currently added properties for cpsw will not make driver functional this
> Kernel version (if this goes in) or future versions. To make driver work
> additional properties are required.
>
>>>> There are many other parameters which are missed here.
>>>
>>> Reason has been mentioned in the commit message, quoting relevant here
>>> again,
>>
>> Actually, as you've marked the nodes disabled, it's probably fine. But
>> worth considering as properties are added...
>
> There were two factors that was considered while adding cpsw node
> 1. DT as an ABI
> 2. While adding DT node, whether all existing required properties has to
> be added
>
> Regarding 1 - DT would not make driver work for this Kernel version and
> also for not any newer Kernel version, I believe this does not go
> against DT an an ABI, although in a negative sense. To make driver work
> more DT properties would have to be added.
>
> Regarding 2 - Currently, I believe most required & optional properties
> in bindings are defined w.r.t driver (bringing in a Linux attachment).
> If DT is only a hardware description, required & optional properties
> should correspond to something that are a must for hardware to work and
> additional features that can be used respectively. In that sense
> interrupts property for many of the peripherals would have to be
> considered optional, as it is possible to work in polling mode. And
> would it be practical for DT in most of the cases to be a complete
> hardware description ?, as to completely describe hardware, we may need
> to have a large amount of properties that may not be relevant to
> software. If requirement is only that DT should describe hardware that
> is relevant for software (this would bring in a software dependency for
> DT), required property for one software may not be required for another
> piece of software or may be an optional property (like in the case of
> interrupts as explained above).
>
> So conclusion arrived within me was that as long as properties are not
> modified, but only added and as long as it does not go against DT as an
> ABI, it is ok.
>
> I would like to hear from DT maintainers what the approach should be.

DT is clearly an ABI. It is there to provide HW information that are 
needed by the SW but cannot be extracted from the HW registers of from 
some ACPI table.

It could as well provide information about the HW hierarchy assuming 
this is relevant for the SW. That's typically why you do not have the 
full accurate description of the OMAP interconnects in DTS. That is just 
pointless for the SW point of view.

The goal is thus not to describe the full HW, which is anyway not 
realistic, but to provide the HW information needed by the SW.

And the driver is really the first one to know what is needed or not for 
a certain IP. Hence a dependency with the driver in term if binding 
definition.

Regards,
Benoit

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ARM: dts: AM4372: add few nodes
  2013-08-16 10:17             ` Benoit Cousson
@ 2013-08-16 23:11               ` Stephen Warren
  -1 siblings, 0 replies; 20+ messages in thread
From: Stephen Warren @ 2013-08-16 23:11 UTC (permalink / raw)
  To: Benoit Cousson
  Cc: Mark Rutland, Mugunthan V N, Afzal Mohammed, Russell King,
	Ian Campbell, Pawel Moll, devicetree, Tony Lindgren,
	Paul Walmsley, rob.herring, linux-omap, linux-arm-kernel

On 08/16/2013 04:17 AM, Benoit Cousson wrote:
> Hi Afzal,
> 
> On 12/08/2013 08:48, Afzal Mohammed wrote:
>> Hi Mark,
>>
>> On Saturday 10 August 2013 07:53 PM, Mark Rutland wrote:
>>
>>>>>> +        mac: ethernet@4a100000 {
>>>>>> +            compatible = "ti,am4372-cpsw","ti,cpsw";
>>
>>> One point worth mentioning is that the "ti,am4372-cpsw" string isn't
>>> documented. Will the "ti,am4372-cpsw" binding definitely be a superset
>>> of the "ti,cpsw" binding, and if you were to take the DT as of this
>>> patch, and attempt to use it with a future kernel, can you guarantee
>>> it'll work?
>>
>> "ti,am4372-cpsw" was not documented as OMAP DT maintainer didn't prefer
>> documenting only for a new compatible.
> 
> My point was more that creating a new compatible for the exact same
> version of the IP is pointeless and could lead to tons of compatible
> strings that would never be used since this is always the exact same IP.

Well, there are two aspects: Version of the IP block, and the
integration into the SoC. We need an entry in compatible for any/all of
these. If the IP block doesn't have some standalone versioning scheme
because the design flow isn't IP-block-centric, the SoC name makes a
reasonable substitute.

...
> My second point is, even if we want to differentiate the IPs in various
> SoCs, using the name of the SoC in the IP compatible string is not a
> very good practice anyway.

There may be integration-specific issues, so even if SoC A and B use the
exact same IP block version, there should still be compatible values for
the SoC as well as the IP block version, so you can quirk on those later.

... although I suppose it might not be too bad if the IP block
compatible value only contained the IP block version, and if any
integration-specific quirks were needed, they could be triggered off
entries in the top-level node's compatible value? That only works for
on-Soc modules, and not complex MFD-like modules, unless you have an
easy way to find the DT node for the top-level of the MFD...

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/2] ARM: dts: AM4372: add few nodes
@ 2013-08-16 23:11               ` Stephen Warren
  0 siblings, 0 replies; 20+ messages in thread
From: Stephen Warren @ 2013-08-16 23:11 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/16/2013 04:17 AM, Benoit Cousson wrote:
> Hi Afzal,
> 
> On 12/08/2013 08:48, Afzal Mohammed wrote:
>> Hi Mark,
>>
>> On Saturday 10 August 2013 07:53 PM, Mark Rutland wrote:
>>
>>>>>> +        mac: ethernet at 4a100000 {
>>>>>> +            compatible = "ti,am4372-cpsw","ti,cpsw";
>>
>>> One point worth mentioning is that the "ti,am4372-cpsw" string isn't
>>> documented. Will the "ti,am4372-cpsw" binding definitely be a superset
>>> of the "ti,cpsw" binding, and if you were to take the DT as of this
>>> patch, and attempt to use it with a future kernel, can you guarantee
>>> it'll work?
>>
>> "ti,am4372-cpsw" was not documented as OMAP DT maintainer didn't prefer
>> documenting only for a new compatible.
> 
> My point was more that creating a new compatible for the exact same
> version of the IP is pointeless and could lead to tons of compatible
> strings that would never be used since this is always the exact same IP.

Well, there are two aspects: Version of the IP block, and the
integration into the SoC. We need an entry in compatible for any/all of
these. If the IP block doesn't have some standalone versioning scheme
because the design flow isn't IP-block-centric, the SoC name makes a
reasonable substitute.

...
> My second point is, even if we want to differentiate the IPs in various
> SoCs, using the name of the SoC in the IP compatible string is not a
> very good practice anyway.

There may be integration-specific issues, so even if SoC A and B use the
exact same IP block version, there should still be compatible values for
the SoC as well as the IP block version, so you can quirk on those later.

... although I suppose it might not be too bad if the IP block
compatible value only contained the IP block version, and if any
integration-specific quirks were needed, they could be triggered off
entries in the top-level node's compatible value? That only works for
on-Soc modules, and not complex MFD-like modules, unless you have an
easy way to find the DT node for the top-level of the MFD...

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2013-08-16 23:11 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-02 13:46 [PATCH 1/2] ARM: dts: AM4372: cpu(s) node per latest binding Afzal Mohammed
2013-08-02 13:46 ` Afzal Mohammed
2013-08-02 13:46 ` [PATCH 2/2] ARM: dts: AM4372: add few nodes Afzal Mohammed
2013-08-02 13:46   ` Afzal Mohammed
2013-08-03 11:49   ` Mugunthan V N
2013-08-03 11:49     ` Mugunthan V N
2013-08-05  5:08     ` Afzal Mohammed
2013-08-05  5:08       ` Afzal Mohammed
2013-08-05  6:06       ` Mugunthan V N
2013-08-05  6:06         ` Mugunthan V N
2013-08-10 14:23       ` Mark Rutland
2013-08-10 14:23         ` Mark Rutland
2013-08-12  6:48         ` Afzal Mohammed
2013-08-12  6:48           ` Afzal Mohammed
2013-08-16 10:17           ` Benoit Cousson
2013-08-16 10:17             ` Benoit Cousson
2013-08-16 23:11             ` Stephen Warren
2013-08-16 23:11               ` Stephen Warren
2013-08-10 14:13 ` [PATCH 1/2] ARM: dts: AM4372: cpu(s) node per latest binding Mark Rutland
2013-08-10 14:13   ` Mark Rutland

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.