[RFC PATCH 00/10] OMAP: Add DT support for early init OMAP4 devices

[RFC PATCH 00/10] OMAP: Add DT support for early init OMAP4 devices

[Benoit Cousson]

Hi Grant, Tony,

Here are a couple of drivers / devices adaptated to DT on OMAP4.
I focused first on devices that used to be initialized before machine_init
time (postcore_initcall most of the time).

The GPIO adaptation is not taking into account the current cleanup done
by Charu / Tarun. It will have to be revisited when the cleanup will
be completed.

Patches are based on for_3.2/3_omap4_dt_base + devicetree/test
and are available here:
git://gitorious.org/omap-pm/linux.git for_3.2/4_omap4_dt_early_devices

Comments are welcome.

Regards,
Benoit


Benoit Cousson (10):
  OMAP2+: l3-noc: Add support for device-tree
  arm/dts: OMAP4: Add a main ocp entry bound to l3-noc driver
  documentation/dt: Add l3-noc bindings
  arm/dts: OMAP4: Add mpu, dsp and iva nodes
  documentation/dt: Add mpu, dsp and iva bindings
  hwspinlock: OMAP4: Add spinlock support in DT
  documentation/dt: Add spinlock bindings
  gpio/omap: Adapt GPIO driver to DT
  arm/dts: OMAP4: Add gpio nodes
  documentation/dt: Add OMAP GPIO properties

 Documentation/devicetree/bindings/arm/omap/dsp.txt |   14 +++
 Documentation/devicetree/bindings/arm/omap/iva.txt |   18 ++++
 .../devicetree/bindings/arm/omap/l3-noc.txt        |   18 ++++
 Documentation/devicetree/bindings/arm/omap/mpu.txt |   35 +++++++
 .../devicetree/bindings/gpio/gpio-omap.txt         |   33 ++++++
 .../bindings/hwspinlock/omap-spinlock.txt          |    5 +
 arch/arm/boot/dts/omap4.dtsi                       |  105 +++++++++++++++++++-
 arch/arm/mach-omap2/Makefile                       |    6 +-
 arch/arm/mach-omap2/devices.c                      |    2 +
 arch/arm/mach-omap2/gpio.c                         |    2 +
 arch/arm/mach-omap2/omap_l3_noc.c                  |   16 +++-
 arch/arm/mach-omap2/pm.c                           |    5 +-
 drivers/gpio/gpio-omap.c                           |  103 +++++++++++++++++--
 drivers/hwspinlock/omap_hwspinlock.c               |   11 ++
 14 files changed, 359 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/omap/dsp.txt
 create mode 100644 Documentation/devicetree/bindings/arm/omap/iva.txt
 create mode 100644 Documentation/devicetree/bindings/arm/omap/l3-noc.txt
 create mode 100644 Documentation/devicetree/bindings/arm/omap/mpu.txt
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-omap.txt
 create mode 100644 Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt

[RFC PATCH 01/10] OMAP2+: l3-noc: Add support for device-tree

[Benoit Cousson]

Add device-tree support for the l3-noc driver.

Use platform_driver_register to defer the probing at device init
time.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/omap_l3_noc.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_l3_noc.c b/arch/arm/mach-omap2/omap_l3_noc.c
index 7b9f190..4630703 100644
--- a/arch/arm/mach-omap2/omap_l3_noc.c
+++ b/arch/arm/mach-omap2/omap_l3_noc.c
@@ -228,16 +228,28 @@ static int __exit omap4_l3_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#if defined(CONFIG_OF)
+static const struct of_device_id l3_noc_match[] = {
+	{.compatible = "arteris,noc", },
+	{},
+}
+MODULE_DEVICE_TABLE(of, l3_noc_match);
+#else
+#define l3_noc_match NULL
+#endif
+
 static struct platform_driver omap4_l3_driver = {
+	.probe		= omap4_l3_probe,
 	.remove		= __exit_p(omap4_l3_remove),
 	.driver		= {
-	.name		= "omap_l3_noc",
+		.name		= "omap_l3_noc",
+		.of_match_table = l3_noc_match,
 	},
 };
 
 static int __init omap4_l3_init(void)
 {
-	return platform_driver_probe(&omap4_l3_driver, omap4_l3_probe);
+	return platform_driver_register(&omap4_l3_driver);
 }
 postcore_initcall_sync(omap4_l3_init);
 
-- 
1.7.0.4

[RFC PATCH 01/10] OMAP2+: l3-noc: Add support for device-tree

[Grant Likely]

On Wed, Aug 24, 2011 at 03:09:07PM +0200, Benoit Cousson wrote:
> Add device-tree support for the l3-noc driver.
> 
> Use platform_driver_register to defer the probing at device init
> time.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>  arch/arm/mach-omap2/omap_l3_noc.c |   16 ++++++++++++++--
>  1 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_l3_noc.c b/arch/arm/mach-omap2/omap_l3_noc.c
> index 7b9f190..4630703 100644
> --- a/arch/arm/mach-omap2/omap_l3_noc.c
> +++ b/arch/arm/mach-omap2/omap_l3_noc.c
> @@ -228,16 +228,28 @@ static int __exit omap4_l3_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#if defined(CONFIG_OF)
> +static const struct of_device_id l3_noc_match[] = {
> +	{.compatible = "arteris,noc", },

Missing documentation for this compatible property.  Also, it appears
to be rather on the generic side.

> +	{},
> +}
> +MODULE_DEVICE_TABLE(of, l3_noc_match);
> +#else
> +#define l3_noc_match NULL
> +#endif
> +
>  static struct platform_driver omap4_l3_driver = {
> +	.probe		= omap4_l3_probe,

.probe needs to be put into the __devinit section.

>  	.remove		= __exit_p(omap4_l3_remove),

Similarly, at the same time the remove hook should be changed to
__devexit and __devexit_p() at the same time.

>  	.driver		= {
> -	.name		= "omap_l3_noc",
> +		.name		= "omap_l3_noc",
> +		.of_match_table = l3_noc_match,

Looks like ".owner = THIS_MODULE," is missing too.

>  	},
>  };
>  
>  static int __init omap4_l3_init(void)
>  {
> -	return platform_driver_probe(&omap4_l3_driver, omap4_l3_probe);
> +	return platform_driver_register(&omap4_l3_driver);
>  }
>  postcore_initcall_sync(omap4_l3_init);
>  
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC PATCH 01/10] OMAP2+: l3-noc: Add support for device-tree

[Cousson, Benoit]

On 9/8/2011 8:01 PM, Grant Likely wrote:
> On Wed, Aug 24, 2011 at 03:09:07PM +0200, Benoit Cousson wrote:
>> Add device-tree support for the l3-noc driver.
>>
>> Use platform_driver_register to defer the probing at device init
>> time.
>>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Tony Lindgren<tony@atomide.com>
>> Cc: Santosh Shilimkar<santosh.shilimkar@ti.com>
>> ---
>>   arch/arm/mach-omap2/omap_l3_noc.c |   16 ++++++++++++++--
>>   1 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_l3_noc.c b/arch/arm/mach-omap2/omap_l3_noc.c
>> index 7b9f190..4630703 100644
>> --- a/arch/arm/mach-omap2/omap_l3_noc.c
>> +++ b/arch/arm/mach-omap2/omap_l3_noc.c
>> @@ -228,16 +228,28 @@ static int __exit omap4_l3_remove(struct platform_device *pdev)
>>   	return 0;
>>   }
>>
>> +#if defined(CONFIG_OF)
>> +static const struct of_device_id l3_noc_match[] = {
>> +	{.compatible = "arteris,noc", },
>
> Missing documentation for this compatible property.

As you already figured out... it will come later.

> Also, it appears
> to be rather on the generic side.

It is indeed a generic IP that will be there on OMAP5 too.

>> +	{},
>> +}
>> +MODULE_DEVICE_TABLE(of, l3_noc_match);
>> +#else
>> +#define l3_noc_match NULL
>> +#endif
>> +
>>   static struct platform_driver omap4_l3_driver = {
>> +	.probe		= omap4_l3_probe,
>
> .probe needs to be put into the __devinit section.
>
>>   	.remove		= __exit_p(omap4_l3_remove),
>
> Similarly, at the same time the remove hook should be changed to
> __devexit and __devexit_p() at the same time.
>
>>   	.driver		= {
>> -	.name		= "omap_l3_noc",
>> +		.name		= "omap_l3_noc",
>> +		.of_match_table = l3_noc_match,
>
> Looks like ".owner = THIS_MODULE," is missing too.

Well, yeah, that driver was supposed to be started really early to catch 
any potential bus violation. So it was clearly not targeted to be a 
loadable module.
Anyway, it will not hurt, so I'll fix that.

Thanks for the review,
Benoit

[RFC PATCH 01/10] OMAP2+: l3-noc: Add support for device-tree

[Grant Likely]

On Thu, Sep 08, 2011 at 11:59:18PM +0200, Cousson, Benoit wrote:
> On 9/8/2011 8:01 PM, Grant Likely wrote:
> >On Wed, Aug 24, 2011 at 03:09:07PM +0200, Benoit Cousson wrote:
> >>Add device-tree support for the l3-noc driver.
> >>
> >>Use platform_driver_register to defer the probing at device init
> >>time.
> >>
> >>Signed-off-by: Benoit Cousson<b-cousson@ti.com>
> >>Cc: Tony Lindgren<tony@atomide.com>
> >>Cc: Santosh Shilimkar<santosh.shilimkar@ti.com>
> >>---
> >>  arch/arm/mach-omap2/omap_l3_noc.c |   16 ++++++++++++++--
> >>  1 files changed, 14 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/arch/arm/mach-omap2/omap_l3_noc.c b/arch/arm/mach-omap2/omap_l3_noc.c
> >>index 7b9f190..4630703 100644
> >>--- a/arch/arm/mach-omap2/omap_l3_noc.c
> >>+++ b/arch/arm/mach-omap2/omap_l3_noc.c
> >>@@ -228,16 +228,28 @@ static int __exit omap4_l3_remove(struct platform_device *pdev)
> >>  	return 0;
> >>  }
> >>
> >>+#if defined(CONFIG_OF)
> >>+static const struct of_device_id l3_noc_match[] = {
> >>+	{.compatible = "arteris,noc", },
> >
> >Missing documentation for this compatible property.
> 
> As you already figured out... it will come later.
> 
> >Also, it appears
> >to be rather on the generic side.
> 
> It is indeed a generic IP that will be there on OMAP5 too.

Even for common ip blocks, it is good practice to be specific about
the implementation.  IP block details tend to change from one silicon
design to the next.  Overly generic (optimistic) compatible value names
generally should be avoided.  Instead, backwards compatibility is
achieved by newer devices claiming compatibility with older
implementations in the compatible list.

g.

[RFC PATCH 02/10] arm/dts: OMAP4: Add a main ocp entry bound to l3-noc driver

[Benoit Cousson]

Used the main OCP node to add bindings with the l3_noc driver.
Remove l3_noc static device creation if CONFIG_OF is defined.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/boot/dts/omap4.dtsi  |    3 ++-
 arch/arm/mach-omap2/devices.c |    2 ++
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 97a3ea7..928a74c 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -64,10 +64,11 @@
 	 * hierarchy.
 	 */
 	ocp {
-		compatible = "simple-bus";
+		compatible = "ti,l3-noc", "arteris,noc", "simple-bus";
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges;
+		hwmods = "l3_main_1", "l3_main_2", "l3_main_3";
 
 		gic: interrupt-controller at 48241000 {
 			compatible = "ti,omap4-gic", "arm,gic";
diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 2d4a199..5964650 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -70,6 +70,7 @@ static int __init omap3_l3_init(void)
 }
 postcore_initcall(omap3_l3_init);
 
+#ifndef CONFIG_OF
 static int __init omap4_l3_init(void)
 {
 	int l, i;
@@ -100,6 +101,7 @@ static int __init omap4_l3_init(void)
 	return IS_ERR(pdev) ? PTR_ERR(pdev) : 0;
 }
 postcore_initcall(omap4_l3_init);
+#endif
 
 #if defined(CONFIG_VIDEO_OMAP2) || defined(CONFIG_VIDEO_OMAP2_MODULE)
 
-- 
1.7.0.4

[RFC PATCH 02/10] arm/dts: OMAP4: Add a main ocp entry bound to l3-noc driver

[Grant Likely]

On Wed, Aug 24, 2011 at 03:09:08PM +0200, Benoit Cousson wrote:
> Used the main OCP node to add bindings with the l3_noc driver.
> Remove l3_noc static device creation if CONFIG_OF is defined.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>  arch/arm/boot/dts/omap4.dtsi  |    3 ++-
>  arch/arm/mach-omap2/devices.c |    2 ++
>  2 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 97a3ea7..928a74c 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -64,10 +64,11 @@
>  	 * hierarchy.
>  	 */
>  	ocp {
> -		compatible = "simple-bus";
> +		compatible = "ti,l3-noc", "arteris,noc", "simple-bus";
>  		#address-cells = <1>;
>  		#size-cells = <1>;
>  		ranges;
> +		hwmods = "l3_main_1", "l3_main_2", "l3_main_3";
>  
>  		gic: interrupt-controller at 48241000 {
>  			compatible = "ti,omap4-gic", "arm,gic";
> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index 2d4a199..5964650 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -70,6 +70,7 @@ static int __init omap3_l3_init(void)
>  }
>  postcore_initcall(omap3_l3_init);
>  
> +#ifndef CONFIG_OF
>  static int __init omap4_l3_init(void)
>  {
>  	int l, i;
> @@ -100,6 +101,7 @@ static int __init omap4_l3_init(void)
>  	return IS_ERR(pdev) ? PTR_ERR(pdev) : 0;
>  }
>  postcore_initcall(omap4_l3_init);
> +#endif

Don't do this.  Turning on CONFIG_OF must not break non-DT booting.
Instead, check at runtime if a DT is available, and if it does then
don't run the hook.

g.

[RFC PATCH 02/10] arm/dts: OMAP4: Add a main ocp entry bound to l3-noc driver

[Cousson, Benoit]

On 9/8/2011 8:03 PM, Grant Likely wrote:
> On Wed, Aug 24, 2011 at 03:09:08PM +0200, Benoit Cousson wrote:
>> Used the main OCP node to add bindings with the l3_noc driver.
>> Remove l3_noc static device creation if CONFIG_OF is defined.
>>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Tony Lindgren<tony@atomide.com>
>> Cc: Santosh Shilimkar<santosh.shilimkar@ti.com>
>> ---
>>   arch/arm/boot/dts/omap4.dtsi  |    3 ++-
>>   arch/arm/mach-omap2/devices.c |    2 ++
>>   2 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>> index 97a3ea7..928a74c 100644
>> --- a/arch/arm/boot/dts/omap4.dtsi
>> +++ b/arch/arm/boot/dts/omap4.dtsi
>> @@ -64,10 +64,11 @@
>>   	 * hierarchy.
>>   	 */
>>   	ocp {
>> -		compatible = "simple-bus";
>> +		compatible = "ti,l3-noc", "arteris,noc", "simple-bus";
>>   		#address-cells =<1>;
>>   		#size-cells =<1>;
>>   		ranges;
>> +		hwmods = "l3_main_1", "l3_main_2", "l3_main_3";
>>
>>   		gic: interrupt-controller at 48241000 {
>>   			compatible = "ti,omap4-gic", "arm,gic";
>> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
>> index 2d4a199..5964650 100644
>> --- a/arch/arm/mach-omap2/devices.c
>> +++ b/arch/arm/mach-omap2/devices.c
>> @@ -70,6 +70,7 @@ static int __init omap3_l3_init(void)
>>   }
>>   postcore_initcall(omap3_l3_init);
>>
>> +#ifndef CONFIG_OF
>>   static int __init omap4_l3_init(void)
>>   {
>>   	int l, i;
>> @@ -100,6 +101,7 @@ static int __init omap4_l3_init(void)
>>   	return IS_ERR(pdev) ? PTR_ERR(pdev) : 0;
>>   }
>>   postcore_initcall(omap4_l3_init);
>> +#endif
>
> Don't do this.  Turning on CONFIG_OF must not break non-DT booting.
> Instead, check at runtime if a DT is available, and if it does then
> don't run the hook.

Do you want to check only the availability of the DT or the availability 
of that specific node inside the DT?

Benoit

[RFC PATCH 02/10] arm/dts: OMAP4: Add a main ocp entry bound to l3-noc driver

[Grant Likely]

On Fri, Sep 09, 2011 at 02:10:08AM +0200, Cousson, Benoit wrote:
> On 9/8/2011 8:03 PM, Grant Likely wrote:
> >On Wed, Aug 24, 2011 at 03:09:08PM +0200, Benoit Cousson wrote:
> >>Used the main OCP node to add bindings with the l3_noc driver.
> >>Remove l3_noc static device creation if CONFIG_OF is defined.
> >>
> >>Signed-off-by: Benoit Cousson<b-cousson@ti.com>
> >>Cc: Tony Lindgren<tony@atomide.com>
> >>Cc: Santosh Shilimkar<santosh.shilimkar@ti.com>
> >>---
> >>  arch/arm/boot/dts/omap4.dtsi  |    3 ++-
> >>  arch/arm/mach-omap2/devices.c |    2 ++
> >>  2 files changed, 4 insertions(+), 1 deletions(-)
> >>
> >>diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> >>index 97a3ea7..928a74c 100644
> >>--- a/arch/arm/boot/dts/omap4.dtsi
> >>+++ b/arch/arm/boot/dts/omap4.dtsi
> >>@@ -64,10 +64,11 @@
> >>  	 * hierarchy.
> >>  	 */
> >>  	ocp {
> >>-		compatible = "simple-bus";
> >>+		compatible = "ti,l3-noc", "arteris,noc", "simple-bus";
> >>  		#address-cells =<1>;
> >>  		#size-cells =<1>;
> >>  		ranges;
> >>+		hwmods = "l3_main_1", "l3_main_2", "l3_main_3";
> >>
> >>  		gic: interrupt-controller at 48241000 {
> >>  			compatible = "ti,omap4-gic", "arm,gic";
> >>diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> >>index 2d4a199..5964650 100644
> >>--- a/arch/arm/mach-omap2/devices.c
> >>+++ b/arch/arm/mach-omap2/devices.c
> >>@@ -70,6 +70,7 @@ static int __init omap3_l3_init(void)
> >>  }
> >>  postcore_initcall(omap3_l3_init);
> >>
> >>+#ifndef CONFIG_OF
> >>  static int __init omap4_l3_init(void)
> >>  {
> >>  	int l, i;
> >>@@ -100,6 +101,7 @@ static int __init omap4_l3_init(void)
> >>  	return IS_ERR(pdev) ? PTR_ERR(pdev) : 0;
> >>  }
> >>  postcore_initcall(omap4_l3_init);
> >>+#endif
> >
> >Don't do this.  Turning on CONFIG_OF must not break non-DT booting.
> >Instead, check at runtime if a DT is available, and if it does then
> >don't run the hook.
> 
> Do you want to check only the availability of the DT or the
> availability of that specific node inside the DT?

Up to you.  What makes the most sense?

g.

[RFC PATCH 03/10] documentation/dt: Add l3-noc bindings

[Benoit Cousson]

Add documentation for the l3-noc bindings.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Randy Dunlap <rdunlap@xenotime.net>
---
 .../devicetree/bindings/arm/omap/l3-noc.txt        |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/omap/l3-noc.txt

diff --git a/Documentation/devicetree/bindings/arm/omap/l3-noc.txt b/Documentation/devicetree/bindings/arm/omap/l3-noc.txt
new file mode 100644
index 0000000..dbfa878
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/omap/l3-noc.txt
@@ -0,0 +1,18 @@
+* TI - L3 Network On Chip (NoC)
+
+This version is an implementation of the generic NoC IP
+provided by Arteris.
+
+Required properties:
+- compatible : Should be "ti,l3-noc", "arteris,noc"
+- hwmods: "l3_main_1", ... One hwmod for each noc domain.
+
+Examples:
+
+ocp {
+	compatible = "ti,l3-noc", "arteris,noc", "simple-bus";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+	hwmods = "l3_main_1", "l3_main_2", "l3_main_3";
+};
-- 
1.7.0.4

[RFC PATCH 03/10] documentation/dt: Add l3-noc bindings

[Grant Likely]

On Wed, Aug 24, 2011 at 03:09:09PM +0200, Benoit Cousson wrote:
> Add documentation for the l3-noc bindings.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Randy Dunlap <rdunlap@xenotime.net>
> ---
>  .../devicetree/bindings/arm/omap/l3-noc.txt        |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/omap/l3-noc.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/omap/l3-noc.txt b/Documentation/devicetree/bindings/arm/omap/l3-noc.txt
> new file mode 100644
> index 0000000..dbfa878
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/omap/l3-noc.txt
> @@ -0,0 +1,18 @@
> +* TI - L3 Network On Chip (NoC)
> +
> +This version is an implementation of the generic NoC IP
> +provided by Arteris.

Hahaha.  Here's the documentation.  Okay.

> +
> +Required properties:
> +- compatible : Should be "ti,l3-noc", "arteris,noc"

Should probably be "ti,omap4-l3-noc", and it isn't necessary to have a
value for the IP core vendor... or at least if an arteris value is
provided, then it should have some form of ip-core version
information.

> +- hwmods: "l3_main_1", ... One hwmod for each noc domain.

Is there some documentation on how the "hwmods" property is to be
used?  I expect that this should be "ti,hwmods" because this is a TI
specific property.

> +
> +Examples:
> +
> +ocp {
> +	compatible = "ti,l3-noc", "arteris,noc", "simple-bus";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	ranges;
> +	hwmods = "l3_main_1", "l3_main_2", "l3_main_3";
> +};
> -- 
> 1.7.0.4
> 

[RFC PATCH 03/10] documentation/dt: Add l3-noc bindings

[Cousson, Benoit]

On 9/8/2011 8:06 PM, Grant Likely wrote:
> On Wed, Aug 24, 2011 at 03:09:09PM +0200, Benoit Cousson wrote:
>> Add documentation for the l3-noc bindings.
>>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Randy Dunlap<rdunlap@xenotime.net>
>> ---
>>   .../devicetree/bindings/arm/omap/l3-noc.txt        |   18 ++++++++++++++++++
>>   1 files changed, 18 insertions(+), 0 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/arm/omap/l3-noc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/omap/l3-noc.txt b/Documentation/devicetree/bindings/arm/omap/l3-noc.txt
>> new file mode 100644
>> index 0000000..dbfa878
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/omap/l3-noc.txt
>> @@ -0,0 +1,18 @@
>> +* TI - L3 Network On Chip (NoC)
>> +
>> +This version is an implementation of the generic NoC IP
>> +provided by Arteris.
>
> Hahaha.  Here's the documentation.  Okay.
>
>> +
>> +Required properties:
>> +- compatible : Should be "ti,l3-noc", "arteris,noc"
>
> Should probably be "ti,omap4-l3-noc", and it isn't necessary to have a
> value for the IP core vendor... or at least if an arteris value is
> provided, then it should have some form of ip-core version
> information.

OK, in that case, the noc version is clearly not obvious to get, so I'll 
get rid of that compatible value.

>
>> +- hwmods: "l3_main_1", ... One hwmod for each noc domain.
>
> Is there some documentation on how the "hwmods" property is to be
> used?

Yes, it is part of the generic OMAP bindings documentation:
http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-August/007621.html

> I expect that this should be "ti,hwmods" because this is a TI
> specific property.

OK.

Benoit

[RFC PATCH 04/10] arm/dts: OMAP4: Add mpu, dsp and iva nodes

[Benoit Cousson]

Add nodes for devices used by PM code (mpu, dsp, iva).

Add an empty cpus node as well as recommended in the DT spec.

Remove mpu, dsp, iva devices init if CONFIG_OF is defined.
Ideally the whole function should be removed. It will be doable as
soon as the OMAP3 DT support will be added.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Kevin Hilman <khilman@ti.com>
---
 arch/arm/boot/dts/omap4.dtsi |   28 ++++++++++++++++++++++++++++
 arch/arm/mach-omap2/pm.c     |    5 ++++-
 2 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 928a74c..a3efe76 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -25,11 +25,39 @@
 	};
 
 	/*
+	 * XXX: The cpus node is mandatory, but since the CPUs are as well part
+	 * of the mpu subsystem below, it is not clear where the information
+	 * should be. Maybe here with a phandle inside the mpu?
+	 */
+	cpus {
+	};
+
+	/*
 	 * The soc node represents the soc top level view. It is uses for IPs
 	 * that are not memory mapped in the MPU view or for the MPU itself.
 	 */
 	soc {
 		compatible = "ti,omap-infra";
+		mpu {
+			compatible = "ti,omap4-mpu";
+			hwmods = "mpu";
+			cpu at 0 {
+				compatible = "arm,cortex-a9";
+			};
+			cpu at 1 {
+				compatible = "arm,cortex-a9";
+			};
+		};
+
+		dsp {
+			compatible = "ti,omap4-c64", "ti,c64";
+			hwmods = "dsp";
+		};
+
+		iva {
+			compatible = "ti,ivahd", "ti,iva";
+			hwmods = "iva";
+		};
 	};
 
 	/*
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 832577a..ba4d187 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -49,15 +49,18 @@ static int _init_omap_device(char *name)
  */
 static void omap2_init_processor_devices(void)
 {
-	_init_omap_device("mpu");
 	if (omap3_has_iva())
 		_init_omap_device("iva");
 
 	if (cpu_is_omap44xx()) {
+#ifndef CONFIG_OF
+		_init_omap_device("mpu");
 		_init_omap_device("l3_main_1");
 		_init_omap_device("dsp");
 		_init_omap_device("iva");
+#endif
 	} else {
+		_init_omap_device("mpu");
 		_init_omap_device("l3_main");
 	}
 }
-- 
1.7.0.4

[RFC PATCH 04/10] arm/dts: OMAP4: Add mpu, dsp and iva nodes

[Grant Likely]

On Wed, Aug 24, 2011 at 03:09:10PM +0200, Benoit Cousson wrote:
> Add nodes for devices used by PM code (mpu, dsp, iva).
> 
> Add an empty cpus node as well as recommended in the DT spec.
> 
> Remove mpu, dsp, iva devices init if CONFIG_OF is defined.
> Ideally the whole function should be removed. It will be doable as
> soon as the OMAP3 DT support will be added.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Kevin Hilman <khilman@ti.com>
> ---
>  arch/arm/boot/dts/omap4.dtsi |   28 ++++++++++++++++++++++++++++
>  arch/arm/mach-omap2/pm.c     |    5 ++++-
>  2 files changed, 32 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 928a74c..a3efe76 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -25,11 +25,39 @@
>  	};
>  
>  	/*
> +	 * XXX: The cpus node is mandatory, but since the CPUs are as well part
> +	 * of the mpu subsystem below, it is not clear where the information
> +	 * should be. Maybe here with a phandle inside the mpu?
> +	 */
> +	cpus {
> +	};
> +
> +	/*
>  	 * The soc node represents the soc top level view. It is uses for IPs
>  	 * that are not memory mapped in the MPU view or for the MPU itself.
>  	 */
>  	soc {
>  		compatible = "ti,omap-infra";
> +		mpu {
> +			compatible = "ti,omap4-mpu";
> +			hwmods = "mpu";
> +			cpu at 0 {
> +				compatible = "arm,cortex-a9";
> +			};
> +			cpu at 1 {
> +				compatible = "arm,cortex-a9";
> +			};
> +		};
> +
> +		dsp {
> +			compatible = "ti,omap4-c64", "ti,c64";
> +			hwmods = "dsp";
> +		};
> +
> +		iva {
> +			compatible = "ti,ivahd", "ti,iva";
> +			hwmods = "iva";
> +		};
>  	};
>  
>  	/*
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index 832577a..ba4d187 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -49,15 +49,18 @@ static int _init_omap_device(char *name)
>   */
>  static void omap2_init_processor_devices(void)
>  {
> -	_init_omap_device("mpu");
>  	if (omap3_has_iva())
>  		_init_omap_device("iva");
>  
>  	if (cpu_is_omap44xx()) {
> +#ifndef CONFIG_OF
> +		_init_omap_device("mpu");
>  		_init_omap_device("l3_main_1");
>  		_init_omap_device("dsp");
>  		_init_omap_device("iva");
> +#endif

Ditto here, turning on CONFIG_OF should not be an either/or option
with ATAGs support.

>  	} else {
> +		_init_omap_device("mpu");
>  		_init_omap_device("l3_main");
>  	}
>  }
> -- 
> 1.7.0.4
> 

[RFC PATCH 05/10] documentation/dt: Add mpu, dsp and iva bindings

[Benoit Cousson]

Add documentation for the OMAP4 processors bindings.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Randy Dunlap <rdunlap@xenotime.net>
---
 Documentation/devicetree/bindings/arm/omap/dsp.txt |   14 ++++++++
 Documentation/devicetree/bindings/arm/omap/iva.txt |   18 ++++++++++
 Documentation/devicetree/bindings/arm/omap/mpu.txt |   35 ++++++++++++++++++++
 3 files changed, 67 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/omap/dsp.txt
 create mode 100644 Documentation/devicetree/bindings/arm/omap/iva.txt
 create mode 100644 Documentation/devicetree/bindings/arm/omap/mpu.txt

diff --git a/Documentation/devicetree/bindings/arm/omap/dsp.txt b/Documentation/devicetree/bindings/arm/omap/dsp.txt
new file mode 100644
index 0000000..8fcd82c
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/omap/dsp.txt
@@ -0,0 +1,14 @@
+* TI - DSP (Digital Signal Processor)
+
+TI DSP included in OMAP SoC
+
+Required properties:
+- compatible : Should be "ti,c64" for OMAP3 & 4
+- hwmods: "dsp"
+
+Examples:
+
+dsp {
+    compatible = "ti,omap4-c64", "ti,c64";
+    hwmods = "dsp";
+};
diff --git a/Documentation/devicetree/bindings/arm/omap/iva.txt b/Documentation/devicetree/bindings/arm/omap/iva.txt
new file mode 100644
index 0000000..f428e88
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/omap/iva.txt
@@ -0,0 +1,18 @@
+* TI - IVA (Imaging and Video Accelerator) subsystem
+
+The IVA contain various audio, video or imaging HW accelerator
+depending of the version.
+
+Required properties:
+- compatible : Should be:
+  - "ti,ivahd", "ti,iva" for OMAP4
+  - "ti,iva2", "ti,iva" for OMAP3
+  - "ti,iva1", "ti,iva" for OMAP2
+- hwmods: "iva"
+
+Examples:
+
+iva {
+    compatible = "ti,ivahd", "ti,iva";
+    hwmods = "iva";
+};
diff --git a/Documentation/devicetree/bindings/arm/omap/mpu.txt b/Documentation/devicetree/bindings/arm/omap/mpu.txt
new file mode 100644
index 0000000..eadab5a
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/omap/mpu.txt
@@ -0,0 +1,35 @@
+* TI - MPU (Main Processor Unit) subsystem
+
+The MPU subsystem contain one or several ARM cores
+depending of the version.
+The MPU contain CPUs, GIC, L2 cache and a local PRCM.
+
+Required properties:
+- compatible : Should be "ti,omap4-mpu" for OMAP4
+- hwmods: "mpu"
+
+Examples:
+
+- For an OMAP4 SMP system:
+
+mpu {
+    compatible = "ti,omap4-mpu";
+    hwmods = "mpu";
+    cpu at 0 {
+        compatible = "arm,cortex-a9";
+    };
+    cpu at 1 {
+        compatible = "arm,cortex-a9";
+    };
+};
+
+
+- For an OMAP3 monocore system:
+
+mpu {
+    compatible = "ti,omap3-mpu";
+    hwmods = "mpu";
+    cpu at 0 {
+        compatible = "arm,cortex-a8";
+    };
+};
-- 
1.7.0.4

[RFC PATCH 05/10] documentation/dt: Add mpu, dsp and iva bindings

[Grant Likely]

On Wed, Aug 24, 2011 at 03:09:11PM +0200, Benoit Cousson wrote:
> Add documentation for the OMAP4 processors bindings.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Randy Dunlap <rdunlap@xenotime.net>
> ---
>  Documentation/devicetree/bindings/arm/omap/dsp.txt |   14 ++++++++
>  Documentation/devicetree/bindings/arm/omap/iva.txt |   18 ++++++++++
>  Documentation/devicetree/bindings/arm/omap/mpu.txt |   35 ++++++++++++++++++++
>  3 files changed, 67 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/omap/dsp.txt
>  create mode 100644 Documentation/devicetree/bindings/arm/omap/iva.txt
>  create mode 100644 Documentation/devicetree/bindings/arm/omap/mpu.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/omap/dsp.txt b/Documentation/devicetree/bindings/arm/omap/dsp.txt
> new file mode 100644
> index 0000000..8fcd82c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/omap/dsp.txt
> @@ -0,0 +1,14 @@
> +* TI - DSP (Digital Signal Processor)
> +
> +TI DSP included in OMAP SoC
> +
> +Required properties:
> +- compatible : Should be "ti,c64" for OMAP3 & 4
> +- hwmods: "dsp"
> +
> +Examples:
> +
> +dsp {
> +    compatible = "ti,omap4-c64", "ti,c64";
> +    hwmods = "dsp";
> +};
> diff --git a/Documentation/devicetree/bindings/arm/omap/iva.txt b/Documentation/devicetree/bindings/arm/omap/iva.txt
> new file mode 100644
> index 0000000..f428e88
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/omap/iva.txt
> @@ -0,0 +1,18 @@
> +* TI - IVA (Imaging and Video Accelerator) subsystem
> +
> +The IVA contain various audio, video or imaging HW accelerator
> +depending of the version.
> +
> +Required properties:
> +- compatible : Should be:
> +  - "ti,ivahd", "ti,iva" for OMAP4
> +  - "ti,iva2", "ti,iva" for OMAP3
> +  - "ti,iva1", "ti,iva" for OMAP2

Again, be specific to the instantiation and encode the omap version
into the compatible value.  Use the form "ti,omap4-*", etc.

g.

[RFC PATCH 05/10] documentation/dt: Add mpu, dsp and iva bindings

[Cousson, Benoit]

On 9/8/2011 8:09 PM, Grant Likely wrote:
> On Wed, Aug 24, 2011 at 03:09:11PM +0200, Benoit Cousson wrote:
>> Add documentation for the OMAP4 processors bindings.
>>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Randy Dunlap<rdunlap@xenotime.net>
>> ---
>>   Documentation/devicetree/bindings/arm/omap/dsp.txt |   14 ++++++++
>>   Documentation/devicetree/bindings/arm/omap/iva.txt |   18 ++++++++++
>>   Documentation/devicetree/bindings/arm/omap/mpu.txt |   35 ++++++++++++++++++++
>>   3 files changed, 67 insertions(+), 0 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/arm/omap/dsp.txt
>>   create mode 100644 Documentation/devicetree/bindings/arm/omap/iva.txt
>>   create mode 100644 Documentation/devicetree/bindings/arm/omap/mpu.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/omap/dsp.txt b/Documentation/devicetree/bindings/arm/omap/dsp.txt
>> new file mode 100644
>> index 0000000..8fcd82c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/omap/dsp.txt
>> @@ -0,0 +1,14 @@
>> +* TI - DSP (Digital Signal Processor)
>> +
>> +TI DSP included in OMAP SoC
>> +
>> +Required properties:
>> +- compatible : Should be "ti,c64" for OMAP3&  4
>> +- hwmods: "dsp"
>> +
>> +Examples:
>> +
>> +dsp {
>> +    compatible = "ti,omap4-c64", "ti,c64";
>> +    hwmods = "dsp";
>> +};
>> diff --git a/Documentation/devicetree/bindings/arm/omap/iva.txt b/Documentation/devicetree/bindings/arm/omap/iva.txt
>> new file mode 100644
>> index 0000000..f428e88
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/omap/iva.txt
>> @@ -0,0 +1,18 @@
>> +* TI - IVA (Imaging and Video Accelerator) subsystem
>> +
>> +The IVA contain various audio, video or imaging HW accelerator
>> +depending of the version.
>> +
>> +Required properties:
>> +- compatible : Should be:
>> +  - "ti,ivahd", "ti,iva" for OMAP4
>> +  - "ti,iva2", "ti,iva" for OMAP3
>> +  - "ti,iva1", "ti,iva" for OMAP2
>
> Again, be specific to the instantiation and encode the omap version
> into the compatible value.  Use the form "ti,omap4-*", etc.

In that case, I do not necessarily agree. The IVA is a generic TI IP 
used in various chips. The IP version is well documented in the TRM.

OK, in fact I didn't pick the proper version:-(
It should be:

+  - "ti,ivahd", for OMAP4
+  - "ti,iva2.2", for OMAP3
+  - "ti,iva2.1", for OMAP2430
+  - "ti,iva1", for OMAP2420

But still, no need to rely on OMAP version to identify the IVA version.

Benoit

[RFC PATCH 05/10] documentation/dt: Add mpu, dsp and iva bindings

[Grant Likely]

On Fri, Sep 09, 2011 at 02:30:24AM +0200, Cousson, Benoit wrote:
> On 9/8/2011 8:09 PM, Grant Likely wrote:
> >>+- compatible : Should be:
> >>+  - "ti,ivahd", "ti,iva" for OMAP4
> >>+  - "ti,iva2", "ti,iva" for OMAP3
> >>+  - "ti,iva1", "ti,iva" for OMAP2
> >
> >Again, be specific to the instantiation and encode the omap version
> >into the compatible value.  Use the form "ti,omap4-*", etc.
> 
> In that case, I do not necessarily agree. The IVA is a generic TI IP
> used in various chips. The IP version is well documented in the TRM.
> 
> OK, in fact I didn't pick the proper version:-(
> It should be:
> 
> +  - "ti,ivahd", for OMAP4
> +  - "ti,iva2.2", for OMAP3
> +  - "ti,iva2.1", for OMAP2430
> +  - "ti,iva1", for OMAP2420
> 
> But still, no need to rely on OMAP version to identify the IVA version.

Fair enough, providing that the "ti,iva" value is dropped.

g.

[RFC PATCH 06/10] hwspinlock: OMAP4: Add spinlock support in DT

[Benoit Cousson]

Add a device-tree node for the spinlock.
Remove the static device build code if CONFIG_OF
is set.
Update the hwspinlock driver to use the of_match method.
Add the information in Documentation/devicetree.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Ohad Ben-Cohen <ohad@wizery.com>
---
 arch/arm/boot/dts/omap4.dtsi         |    5 +++++
 arch/arm/mach-omap2/Makefile         |    6 +++++-
 drivers/hwspinlock/omap_hwspinlock.c |   11 +++++++++++
 3 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index a3efe76..231d7b4 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -105,5 +105,10 @@
 			reg = <0x48241000 0x1000>,
 			      <0x48240100 0x0200>;
 		};
+
+		spinlock {
+			compatible = "ti,omap-spinlock";
+			hwmods = "spinlock";
+		};
 	};
 };
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 79e42a1..6ab9116 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -263,9 +263,13 @@ obj-y					+= $(smc91x-m) $(smc91x-y)
 
 smsc911x-$(CONFIG_SMSC911X)		:= gpmc-smsc911x.o
 obj-y					+= $(smsc911x-m) $(smsc911x-y)
-obj-$(CONFIG_ARCH_OMAP4)		+= hwspinlock.o
 
 disp-$(CONFIG_OMAP2_DSS)		:= display.o
 obj-y					+= $(disp-m) $(disp-y)
 
 obj-y					+= common-board-devices.o twl-common.o
+
+# Do not build static device init code in case of DT build
+ifneq ($(CONFIG_OF),y)
+obj-$(CONFIG_ARCH_OMAP4)		+= hwspinlock.o
+endif
diff --git a/drivers/hwspinlock/omap_hwspinlock.c b/drivers/hwspinlock/omap_hwspinlock.c
index a8f0273..09dd132 100644
--- a/drivers/hwspinlock/omap_hwspinlock.c
+++ b/drivers/hwspinlock/omap_hwspinlock.c
@@ -203,11 +203,22 @@ static int omap_hwspinlock_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#if defined(CONFIG_OF)
+static const struct of_device_id spinlock_match[] = {
+	{.compatible = "ti,omap-spinlock", },
+	{},
+}
+MODULE_DEVICE_TABLE(of, spinlock_match);
+#else
+#define spinlock_match NULL
+#endif
+
 static struct platform_driver omap_hwspinlock_driver = {
 	.probe		= omap_hwspinlock_probe,
 	.remove		= omap_hwspinlock_remove,
 	.driver		= {
 		.name	= "omap_hwspinlock",
+		.of_match_table = spinlock_match,
 	},
 };
 
-- 
1.7.0.4

[RFC PATCH 06/10] hwspinlock: OMAP4: Add spinlock support in DT

[Ohad Ben-Cohen]

On Wed, Aug 24, 2011 at 4:09 PM, Benoit Cousson <b-cousson@ti.com> wrote:
> Add a device-tree node for the spinlock.
> Remove the static device build code if CONFIG_OF
> is set.
> Update the hwspinlock driver to use the of_match method.
> Add the information in Documentation/devicetree.
>
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Ohad Ben-Cohen <ohad@wizery.com>
> ---
...
> + ? ? ? ? ? ? ? spinlock {
> + ? ? ? ? ? ? ? ? ? ? ? compatible = "ti,omap-spinlock";
> + ? ? ? ? ? ? ? ? ? ? ? hwmods = "spinlock";
> + ? ? ? ? ? ? ? };

This seem to satisfy the current hwspinlock driver, but I'm wondering
about an issue which was discussed awhile ago by Arnd and Mathieu:

Hwspinlock devices provide system-wide hardware locks that are used by
remote processors that have no other way to achieve synchronization.

For that to work, each physical lock must have a system-wide unique id
number that all processors are familiar with, otherwise they can't
possibly assume they're using the same hardware lock.

Usually SoC have a single hwspinlock device, which provides several
hardware spinlocks, and in this case, the locks can be trivially
numbered 0 to (num-of-locks - 1).

In case boards have several hwspinlocks devices (each of which
providing numerous hardware spinlocks) a different base id should be
used for each hwspinlock device (they can't all use 0 as a starting
id!).

While this is certainly not common, it's just plain wrong for the
hwspinlock driver to silently use 0 as a base id whenever it is probed
with a device (and by that implicitly assume there will always be only
one device).

So we need to couple an hwspinlock device with a base id (which is
trivially zero when there's only a single hwspinlock device). This can
be easily achieved today using platform data, which boards will use to
set a different base id for each of the hwspinlock devices they have
(i'll send a patch demonstrating this soon), but I'm wondering how to
specify this hwspinlock-specific data with DT: is there an existing
binding we can use for this ? or should we create something like a
"baseid" one especially for the hwspinlock driver ?

> +#if defined(CONFIG_OF)
> +static const struct of_device_id spinlock_match[] = {
> + ? ? ? {.compatible = "ti,omap-spinlock", },
> + ? ? ? {},
> +}

you're missing a semicolon there (yeah I actually tried to build this ;)

[RFC PATCH 06/10] hwspinlock: OMAP4: Add spinlock support in DT

[Cousson, Benoit]

Hi Ohad,

On 9/7/2011 9:58 PM, Ohad Ben-Cohen wrote:
> On Wed, Aug 24, 2011 at 4:09 PM, Benoit Cousson<b-cousson@ti.com>  wrote:
>> Add a device-tree node for the spinlock.
>> Remove the static device build code if CONFIG_OF
>> is set.
>> Update the hwspinlock driver to use the of_match method.
>> Add the information in Documentation/devicetree.
>>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Grant Likely<grant.likely@secretlab.ca>
>> Cc: Ohad Ben-Cohen<ohad@wizery.com>
>> ---
> ...
>> +               spinlock {
>> +                       compatible = "ti,omap-spinlock";
>> +                       hwmods = "spinlock";
>> +               };
>
> This seem to satisfy the current hwspinlock driver, but I'm wondering
> about an issue which was discussed awhile ago by Arnd and Mathieu:
>
> Hwspinlock devices provide system-wide hardware locks that are used by
> remote processors that have no other way to achieve synchronization.
>
> For that to work, each physical lock must have a system-wide unique id
> number that all processors are familiar with, otherwise they can't
> possibly assume they're using the same hardware lock.
>
> Usually SoC have a single hwspinlock device, which provides several
> hardware spinlocks, and in this case, the locks can be trivially
> numbered 0 to (num-of-locks - 1).
>
> In case boards have several hwspinlocks devices (each of which
> providing numerous hardware spinlocks) a different base id should be
> used for each hwspinlock device (they can't all use 0 as a starting
> id!).
>
> While this is certainly not common, it's just plain wrong for the
> hwspinlock driver to silently use 0 as a base id whenever it is probed
> with a device (and by that implicitly assume there will always be only
> one device).

Hehe, I'm not the one who wrote that driver :-)

This is not wrong for the current HW. The point is do we want to 
anticipate potential HW evolution that might never happen on that IP?

> So we need to couple an hwspinlock device with a base id (which is
> trivially zero when there's only a single hwspinlock device). This can
> be easily achieved today using platform data, which boards will use to
> set a different base id for each of the hwspinlock devices they have
> (i'll send a patch demonstrating this soon), but I'm wondering how to
> specify this hwspinlock-specific data with DT: is there an existing
> binding we can use for this ? or should we create something like a
> "baseid" one especially for the hwspinlock driver ?

This is no different than the multiple GPIO controllers we have today.
Since we cannot rely on the DT nodes order, I added an explicit "id" 
attribute to provide that information to the driver. And then the baseid 
is "id * #gpios".

>> +#if defined(CONFIG_OF)
>> +static const struct of_device_id spinlock_match[] = {
>> +       {.compatible = "ti,omap-spinlock", },
>> +       {},
>> +}
>
> you're missing a semicolon there (yeah I actually tried to build this ;)

That was a test :-)
In fact it looks like this driver is not built with a default 
omap2plus_defconfig :-(
I'll fix that.

Thanks for the review,
Benoit

[RFC PATCH 06/10] hwspinlock: OMAP4: Add spinlock support in DT

[Ohad Ben-Cohen]

Hi Benoit,

On Thu, Sep 8, 2011 at 10:14 AM, Cousson, Benoit <b-cousson@ti.com> wrote:
> Hehe, I'm not the one who wrote that driver :-)
>
> This is not wrong for the current HW. The point is do we want to anticipate
> potential HW evolution that might never happen on that IP?

I originally really thought we can ignore those cases (hence the 0
base id ;), and personally I still think the scenario is a bit
fictional, and wouldn't even mind just having omap_hwspinlock_probe()
return an error if it is unexpectedly probed with a second device.

But if fixing this entirely only means doing a small change, then it's
surely nicer.

> This is no different than the multiple GPIO controllers we have today.
> Since we cannot rely on the DT nodes order, I added an explicit "id"
> attribute to provide that information to the driver. And then the baseid is
> "id * #gpios".

That would work if #hwspinlock is a fixed number, but a "baseid"
attribute would allow supporting devices with different #hwspinlocks
per device. Since I am not aware of any real hardware that does this
kind of blasphemy, I can't say if the latter is really necessary :) If
you prefer the former, I'm entirely fine with it.

Thanks,
Ohad.

[RFC PATCH 06/10] hwspinlock: OMAP4: Add spinlock support in DT

[Cousson, Benoit]

On 9/8/2011 9:56 AM, Ohad Ben-Cohen wrote:
> Hi Benoit,
>
> On Thu, Sep 8, 2011 at 10:14 AM, Cousson, Benoit<b-cousson@ti.com>  wrote:
>> Hehe, I'm not the one who wrote that driver :-)
>>
>> This is not wrong for the current HW. The point is do we want to anticipate
>> potential HW evolution that might never happen on that IP?
>
> I originally really thought we can ignore those cases (hence the 0
> base id ;), and personally I still think the scenario is a bit
> fictional, and wouldn't even mind just having omap_hwspinlock_probe()
> return an error if it is unexpectedly probed with a second device.
>
> But if fixing this entirely only means doing a small change, then it's
> surely nicer.

That should not be a big deal to add that kind of support.

>> This is no different than the multiple GPIO controllers we have today.
>> Since we cannot rely on the DT nodes order, I added an explicit "id"
>> attribute to provide that information to the driver. And then the baseid is
>> "id * #gpios".
>
> That would work if #hwspinlock is a fixed number, but a "baseid"
> attribute would allow supporting devices with different #hwspinlocks
> per device. Since I am not aware of any real hardware that does this
> kind of blasphemy, I can't say if the latter is really necessary :) If
> you prefer the former, I'm entirely fine with it.

The (small) issue for my point of view is that the #hwspinlock is 
already encoded in the IP itself. So adding a baseid directly in DT will 
look like duplicating indirectly something that is already there in the HW.
That being said, since we cannot rely on the order, we will not be able 
to get the proper baseid until the driver probe every hwspinlock devices 
:-(
So baseid might be a easier choice.

Regards,
Benoit

[RFC PATCH 06/10] hwspinlock: OMAP4: Add spinlock support in DT

[Ohad Ben-Cohen]

On Thu, Sep 8, 2011 at 11:07 AM, Cousson, Benoit <b-cousson@ti.com> wrote:
> The (small) issue for my point of view is that the #hwspinlock is already
> encoded in the IP itself. So adding a baseid directly in DT will look like
> duplicating indirectly something that is already there in the HW.
> That being said, since we cannot rely on the order, we will not be able to
> get the proper baseid until the driver probe every hwspinlock devices :-(
> So baseid might be a easier choice.

Sounds good. Thanks a lot !

[RFC PATCH 06/10] hwspinlock: OMAP4: Add spinlock support in DT

[Arnd Bergmann]

On Thursday 08 September 2011, Ohad Ben-Cohen wrote:
> On Thu, Sep 8, 2011 at 11:07 AM, Cousson, Benoit <b-cousson@ti.com> wrote:
> > The (small) issue for my point of view is that the #hwspinlock is already
> > encoded in the IP itself. So adding a baseid directly in DT will look like
> > duplicating indirectly something that is already there in the HW.
> > That being said, since we cannot rely on the order, we will not be able to
> > get the proper baseid until the driver probe every hwspinlock devices :-(
> > So baseid might be a easier choice.
> 
> Sounds good. Thanks a lot !

I think a number would work here but is not optimal for the device tree
representation. I think a better binding would be to encode it like
interrupt numbers, where every device that uses a hwspinlock will describe
that as a combination of phandle to the hwspinlock controller and
identifier to be used by that controller, e.g.

	spinlock1 {
		compatible = "ti,omap-spinlock";
		regs = ...
		interrupts = <42>;
		interrupt-parent = &irq-controller;
	};

	dsp {
		compatible = ...
		regs = ...
		spinlocks = <23>; // local number withing &spinlock1;
		spinlock-controller = &spinlock1;
	};

or possibly shorter

		spinlocks = <&spinlock1 23>;

	Arnd

[RFC PATCH 06/10] hwspinlock: OMAP4: Add spinlock support in DT

[Cousson, Benoit]

On 9/8/2011 4:47 PM, Arnd Bergmann wrote:
> On Thursday 08 September 2011, Ohad Ben-Cohen wrote:
>> On Thu, Sep 8, 2011 at 11:07 AM, Cousson, Benoit<b-cousson@ti.com>  wrote:
>>> The (small) issue for my point of view is that the #hwspinlock is already
>>> encoded in the IP itself. So adding a baseid directly in DT will look like
>>> duplicating indirectly something that is already there in the HW.
>>> That being said, since we cannot rely on the order, we will not be able to
>>> get the proper baseid until the driver probe every hwspinlock devices :-(
>>> So baseid might be a easier choice.
>>
>> Sounds good. Thanks a lot !
>
> I think a number would work here but is not optimal for the device tree
> representation. I think a better binding would be to encode it like
> interrupt numbers, where every device that uses a hwspinlock will describe
> that as a combination of phandle to the hwspinlock controller and
> identifier to be used by that controller, e.g.
>
> 	spinlock1 {
> 		compatible = "ti,omap-spinlock";
> 		regs = ...
> 		interrupts =<42>;
> 		interrupt-parent =&irq-controller;
> 	};
>
> 	dsp {
> 		compatible = ...
> 		regs = ...
> 		spinlocks =<23>; // local number withing&spinlock1;
> 		spinlock-controller =&spinlock1;
> 	};

OK, this is indeed much more aligned with the current practice, and what 
DMA should do as well.

Practically speaking, that change will go beyond the original scope of 
that patch that was just adding the DT support based on the existing 
functionality.

Is it OK to handle that improvement in a further patch / series?

Thanks,
Benoit

[RFC PATCH 06/10] hwspinlock: OMAP4: Add spinlock support in DT

[Arnd Bergmann]

On Thursday 08 September 2011, Cousson, Benoit wrote:
> OK, this is indeed much more aligned with the current practice, and what 
> DMA should do as well.
> 
> Practically speaking, that change will go beyond the original scope of 
> that patch that was just adding the DT support based on the existing 
> functionality.
> 
> Is it OK to handle that improvement in a further patch / series?

I think we should be very careful to get the device tree syntax right
at first. So if you have a good representation of the data in the device
tree but don't handle all possible cases in the kernel yet, that's ok.

E.g. the kernel code can assume for now that the device tree has only
one spinlock controller node. We can fix that later when we actually
need more, but we would not easily be able to fix the device trees
after the fact.

	Arnd

[RFC PATCH 06/10] hwspinlock: OMAP4: Add spinlock support in DT

[Ohad Ben-Cohen]

On Thu, Sep 8, 2011 at 5:47 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> I think a number would work here but is not optimal for the device tree
> representation. I think a better binding would be to encode it like
> interrupt numbers, where every device that uses a hwspinlock will describe
> that as a combination of phandle to the hwspinlock controller and
> identifier to be used by that controller

I'm not sure.

This implies that devices are hardwired to the specific
controller+identifier, which is true for interrupts, but not for
hwspinlocks. As a result the hardware depicted by such representation
would be imprecise and might unnecessarily limit the software.

hwspinlock devices are really just a pool of locks, which are not
inherently bound to any device: any master that can initiate
read/write bus access can use any of the locks (hardware-wise). One
just needs to make sure that ownership of the locks, even if
determined dynamically at runtime, is managed correctly.

I think the phandle+identifier approach is very elegant to achieve
static allocation of the hwspinlocks, and some boards will definitely
need it (e.g. those unlucky designs which arbiter i2c access using an
hwspinlock).

But wouldn't that limit us from providing dynamic allocation of
hwspinlocks ? I was told about teams working on complex multimedia use
cases which involves numerous object sharing and require actually more
hwspinlocks than exist (so they're planning on multiplexing several
logical locks on a single hwspinlock). They use dynamic allocation of
hwspinlocks in order to allow different hwspinlocks users to co-exist
(but naturally not run together at the same time).

[RFC PATCH 06/10] hwspinlock: OMAP4: Add spinlock support in DT

[Arnd Bergmann]

On Thursday 08 September 2011, Ohad Ben-Cohen wrote:
> On Thu, Sep 8, 2011 at 5:47 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > I think a number would work here but is not optimal for the device tree
> > representation. I think a better binding would be to encode it like
> > interrupt numbers, where every device that uses a hwspinlock will describe
> > that as a combination of phandle to the hwspinlock controller and
> > identifier to be used by that controller
> 
> I'm not sure.
> 
> This implies that devices are hardwired to the specific
> controller+identifier, which is true for interrupts, but not for
> hwspinlocks. As a result the hardware depicted by such representation
> would be imprecise and might unnecessarily limit the software.
> 
> hwspinlock devices are really just a pool of locks, which are not
> inherently bound to any device: any master that can initiate
> read/write bus access can use any of the locks (hardware-wise). One
> just needs to make sure that ownership of the locks, even if
> determined dynamically at runtime, is managed correctly.

ok

> I think the phandle+identifier approach is very elegant to achieve
> static allocation of the hwspinlocks, and some boards will definitely
> need it (e.g. those unlucky designs which arbiter i2c access using an
> hwspinlock).
> 
> But wouldn't that limit us from providing dynamic allocation of
> hwspinlocks ? I was told about teams working on complex multimedia use
> cases which involves numerous object sharing and require actually more
> hwspinlocks than exist (so they're planning on multiplexing several
> logical locks on a single hwspinlock). They use dynamic allocation of
> hwspinlocks in order to allow different hwspinlocks users to co-exist
> (but naturally not run together at the same time).

Good point. I guess we will need both static and dynamic allocation
then. You need the static allocation for cases where the other
user of the spinlock is not under the control of Linux, but is known
at boot time. For dynamic allocation, my impression is that we don't
need any link from the spinlock user device to the controller at all,
but instead the controller should have a list of the available
spinlocks.

In a case where you have three operating systems running independently
on the same hardware and each of the shares one spinlock with the
other two, you want to be able to model in the device tree which
spinlocks are already known to the other instances and therefore
fixed, which ones are available to the local OS and which ones
are not available.

	Arnd

[RFC PATCH 06/10] hwspinlock: OMAP4: Add spinlock support in DT

[Ohad Ben-Cohen]

On Fri, Sep 9, 2011 at 3:58 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> For dynamic allocation, my impression is that we don't
> need any link from the spinlock user device to the controller at all,

I agree.

> but instead the controller should have a list of the available
> spinlocks.

Might make more sense to give it the list of reserved (i.e. those that
were statically allocated) spinlocks, and then let it treat the rest
as available.

hwspinlock drivers will tell the core which of their spinlocks are
reserved, so it can make sure not to allocate them when someone calls
hwspin_lock_request(). To use those reserved spinlocks, users will
explicitly have to call hwspin_lock_request_specific().

The controller's node should still have something like a "baseid"
attribute, and possibly also the number of available spinlocks. The
latter is a bit redundant though, as drivers already know how many
spinlocks are available (at least the OMAP driver reads it from an
hardware register. The U8500 one seem just to have it hardcoded in the
driver).

Vast majority of hwspinlocks are not statically allocated, so this
would keep the DT minimal, and IMHO, cleaner.

[RFC PATCH 06/10] hwspinlock: OMAP4: Add spinlock support in DT

[Arnd Bergmann]

On Sunday 11 September 2011, Ohad Ben-Cohen wrote:
> > but instead the controller should have a list of the available
> > spinlocks.
> 
> Might make more sense to give it the list of reserved (i.e. those that
> were statically allocated) spinlocks, and then let it treat the rest
> as available.

Fair enough. Whatever you expect to be a shorter list, I guess.

> hwspinlock drivers will tell the core which of their spinlocks are
> reserved, so it can make sure not to allocate them when someone calls
> hwspin_lock_request(). To use those reserved spinlocks, users will
> explicitly have to call hwspin_lock_request_specific().
> 
> The controller's node should still have something like a "baseid"
> attribute, and possibly also the number of available spinlocks. The
> latter is a bit redundant though, as drivers already know how many
> spinlocks are available (at least the OMAP driver reads it from an
> hardware register. The U8500 one seem just to have it hardcoded in the
> driver).
> 
> Vast majority of hwspinlocks are not statically allocated, so this
> would keep the DT minimal, and IMHO, cleaner.

Ok.

	Arnd

[RFC PATCH 07/10] documentation/dt: Add spinlock bindings

[Benoit Cousson]

Add documentation for the HW spinlock in OMAP4.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Randy Dunlap <rdunlap@xenotime.net>
---
 .../bindings/hwspinlock/omap-spinlock.txt          |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt

diff --git a/Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt b/Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt
new file mode 100644
index 0000000..36ac074
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt
@@ -0,0 +1,5 @@
+HW spinlock on OMAP platform:
+
+Required properties:
+- compatible : Must be "ti,omap-spinlock";
+- hwmods : "spinlock"
-- 
1.7.0.4

[RFC PATCH 07/10] documentation/dt: Add spinlock bindings

[Grant Likely]

On Wed, Aug 24, 2011 at 03:09:13PM +0200, Benoit Cousson wrote:
> Add documentation for the HW spinlock in OMAP4.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Randy Dunlap <rdunlap@xenotime.net>
> ---
>  .../bindings/hwspinlock/omap-spinlock.txt          |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt
> 
> diff --git a/Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt b/Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt
> new file mode 100644
> index 0000000..36ac074
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt
> @@ -0,0 +1,5 @@
> +HW spinlock on OMAP platform:
> +
> +Required properties:
> +- compatible : Must be "ti,omap-spinlock";

"ti,omap-*" is probably too generic. omap3-spinlock or omap4-spinlock
would be better.

> +- hwmods : "spinlock"
> -- 
> 1.7.0.4
> 

[RFC PATCH 07/10] documentation/dt: Add spinlock bindings

[Cousson, Benoit]

On 9/8/2011 8:10 PM, Grant Likely wrote:
> On Wed, Aug 24, 2011 at 03:09:13PM +0200, Benoit Cousson wrote:
>> Add documentation for the HW spinlock in OMAP4.
>>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Randy Dunlap<rdunlap@xenotime.net>
>> ---
>>   .../bindings/hwspinlock/omap-spinlock.txt          |    5 +++++
>>   1 files changed, 5 insertions(+), 0 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt
>>
>> diff --git a/Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt b/Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt
>> new file mode 100644
>> index 0000000..36ac074
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt
>> @@ -0,0 +1,5 @@
>> +HW spinlock on OMAP platform:
>> +
>> +Required properties:
>> +- compatible : Must be "ti,omap-spinlock";
>
> "ti,omap-*" is probably too generic. omap3-spinlock or omap4-spinlock
> would be better.

For that IP, we do not have any real IP version documented, so I do agree.
Since it is a new OMAP4 IP, I'll use omap4-spinlock.

Benoit

[RFC PATCH 08/10] gpio/omap: Adapt GPIO driver to DT

[Benoit Cousson]

Adapt the GPIO driver to retrieve information from a DT file.
Note that since the driver is currently under cleanup, some hacks
will have to be removed after.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Charulatha V <charu@ti.com>
Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
---
 drivers/gpio/gpio-omap.c |  103 ++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 94 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 0599854..96d19d7 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -21,6 +21,7 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/pm_runtime.h>
+#include <linux/of_platform.h>
 
 #include <mach/hardware.h>
 #include <asm/irq.h>
@@ -521,7 +522,7 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable)
 	unsigned long flags;
 
 	if (bank->non_wakeup_gpios & gpio_bit) {
-		dev_err(bank->dev, 
+		dev_err(bank->dev,
 			"Unable to modify wakeup on non-wakeup GPIO%d\n", gpio);
 		return -EINVAL;
 	}
@@ -1150,6 +1151,8 @@ static void __devinit omap_gpio_chip_init(struct gpio_bank *bank)
 	irq_set_handler_data(bank->irq, bank);
 }
 
+static const struct of_device_id omap_gpio_match[];
+
 static int __devinit omap_gpio_probe(struct platform_device *pdev)
 {
 	static int gpio_init_done;
@@ -1157,11 +1160,25 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
 	struct resource *res;
 	int id;
 	struct gpio_bank *bank;
+	struct device_node *node = pdev->dev.of_node;
+	const struct of_device_id *match;
+
+	match = of_match_device(omap_gpio_match, &pdev->dev);
+	if (match) {
+		pdata = match->data;
+		/* XXX: big hack until the bank_count is removed */
+		of_property_read_u32(node, "bank_count", &gpio_bank_count);
+		if (of_property_read_u32(node, "id", &id))
+			return -EINVAL;
+		/* XXX: maybe the id in DT should be zero based to avoid that */
+		id -= 1;
+	} else {
+		if (!pdev->dev.platform_data)
+			return -EINVAL;
 
-	if (!pdev->dev.platform_data)
-		return -EINVAL;
-
-	pdata = pdev->dev.platform_data;
+		pdata = pdev->dev.platform_data;
+		id = pdev->id;
+	}
 
 	if (!gpio_init_done) {
 		int ret;
@@ -1171,7 +1188,6 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	id = pdev->id;
 	bank = &gpio_bank[id];
 
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -1181,12 +1197,19 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
 	}
 
 	bank->irq = res->start;
-	bank->virtual_irq_start = pdata->virtual_irq_start;
 	bank->method = pdata->bank_type;
 	bank->dev = &pdev->dev;
-	bank->dbck_flag = pdata->dbck_flag;
 	bank->stride = pdata->bank_stride;
-	bank->width = pdata->bank_width;
+	if (match) {
+		of_property_read_u32(node, "bank_width", &bank->width);
+		if (of_get_property(node, "debounce", NULL))
+			bank->dbck_flag = true;
+		bank->virtual_irq_start = IH_GPIO_BASE + 32 * id;
+	} else {
+		bank->width = pdata->bank_width;
+		bank->dbck_flag = pdata->dbck_flag;
+		bank->virtual_irq_start = pdata->virtual_irq_start;
+	}
 
 	bank->regs = pdata->regs;
 
@@ -1559,10 +1582,72 @@ void omap_gpio_restore_context(void)
 }
 #endif
 
+
+static struct omap_gpio_reg_offs omap2_gpio_regs = {
+	.revision =		OMAP24XX_GPIO_REVISION,
+	.direction =		OMAP24XX_GPIO_OE,
+	.datain =		OMAP24XX_GPIO_DATAIN,
+	.dataout =		OMAP24XX_GPIO_DATAOUT,
+	.set_dataout =		OMAP24XX_GPIO_SETDATAOUT,
+	.clr_dataout =		OMAP24XX_GPIO_CLEARDATAOUT,
+	.irqstatus =		OMAP24XX_GPIO_IRQSTATUS1,
+	.irqstatus2 =		OMAP24XX_GPIO_IRQSTATUS2,
+	.irqenable =		OMAP24XX_GPIO_IRQENABLE1,
+	.set_irqenable =	OMAP24XX_GPIO_SETIRQENABLE1,
+	.clr_irqenable =	OMAP24XX_GPIO_CLEARIRQENABLE1,
+	.debounce =		OMAP24XX_GPIO_DEBOUNCE_VAL,
+	.debounce_en =		OMAP24XX_GPIO_DEBOUNCE_EN,
+};
+
+static struct omap_gpio_platform_data omap2_pdata = {
+	.bank_type = METHOD_GPIO_24XX,
+	.regs = &omap2_gpio_regs,
+};
+
+static struct omap_gpio_reg_offs omap4_gpio_regs = {
+	.revision =		OMAP4_GPIO_REVISION,
+	.direction =		OMAP4_GPIO_OE,
+	.datain =		OMAP4_GPIO_DATAIN,
+	.dataout =		OMAP4_GPIO_DATAOUT,
+	.set_dataout =		OMAP4_GPIO_SETDATAOUT,
+	.clr_dataout =		OMAP4_GPIO_CLEARDATAOUT,
+	.irqstatus =		OMAP4_GPIO_IRQSTATUS1,
+	.irqstatus2 =		OMAP4_GPIO_IRQSTATUS2,
+	.irqenable =		OMAP4_GPIO_IRQENABLE1,
+	.set_irqenable =	OMAP4_GPIO_SETIRQENABLE1,
+	.clr_irqenable =	OMAP4_GPIO_CLEARIRQENABLE1,
+	.debounce =		OMAP4_GPIO_DEBOUNCINGTIME,
+	.debounce_en =		OMAP4_GPIO_DEBOUNCENABLE,
+};
+
+static struct omap_gpio_platform_data omap4_pdata = {
+	.bank_type = METHOD_GPIO_44XX,
+	.regs = &omap4_gpio_regs,
+};
+
+
+#if defined(CONFIG_OF)
+	static const struct of_device_id omap_gpio_match[] = {
+	{
+		.compatible = "ti,omap4-gpio",
+		.data = &omap4_pdata,
+	},
+	{
+		.compatible = "ti,omap2-gpio",
+		.data = &omap2_pdata,
+	},
+	{},
+}
+MODULE_DEVICE_TABLE(of, omap_gpio_match);
+#else
+#define omap_gpio_match NULL
+#endif
+
 static struct platform_driver omap_gpio_driver = {
 	.probe		= omap_gpio_probe,
 	.driver		= {
 		.name	= "omap_gpio",
+		.of_match_table = omap_gpio_match,
 	},
 };
 
-- 
1.7.0.4

[RFC PATCH 08/10] gpio/omap: Adapt GPIO driver to DT

[Grant Likely]

On Wed, Aug 24, 2011 at 03:09:14PM +0200, Benoit Cousson wrote:
> Adapt the GPIO driver to retrieve information from a DT file.
> Note that since the driver is currently under cleanup, some hacks
> will have to be removed after.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Charulatha V <charu@ti.com>
> Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>

Mostly looks good.  Comments below.

> ---
>  drivers/gpio/gpio-omap.c |  103 ++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 94 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 0599854..96d19d7 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -21,6 +21,7 @@
>  #include <linux/io.h>
>  #include <linux/slab.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/of_platform.h>
>  
>  #include <mach/hardware.h>
>  #include <asm/irq.h>
> @@ -521,7 +522,7 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable)
>  	unsigned long flags;
>  
>  	if (bank->non_wakeup_gpios & gpio_bit) {
> -		dev_err(bank->dev, 
> +		dev_err(bank->dev,

nit: unrelated whitespace change

>  			"Unable to modify wakeup on non-wakeup GPIO%d\n", gpio);
>  		return -EINVAL;
>  	}
> @@ -1150,6 +1151,8 @@ static void __devinit omap_gpio_chip_init(struct gpio_bank *bank)
>  	irq_set_handler_data(bank->irq, bank);
>  }
>  
> +static const struct of_device_id omap_gpio_match[];
> +
>  static int __devinit omap_gpio_probe(struct platform_device *pdev)
>  {
>  	static int gpio_init_done;
> @@ -1157,11 +1160,25 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	int id;
>  	struct gpio_bank *bank;
> +	struct device_node *node = pdev->dev.of_node;
> +	const struct of_device_id *match;
> +
> +	match = of_match_device(omap_gpio_match, &pdev->dev);
> +	if (match) {
> +		pdata = match->data;
> +		/* XXX: big hack until the bank_count is removed */
> +		of_property_read_u32(node, "bank_count", &gpio_bank_count);

Nit: by convention, '-' is preferred over '_' in DT property names.

> +		if (of_property_read_u32(node, "id", &id))
> +			return -EINVAL;
> +		/* XXX: maybe the id in DT should be zero based to avoid that */
> +		id -= 1;

Actually, the reason it is -1 based, is that when using the DT, gpio
number are dynamically assigned.  Since the gpio numbers are resolved
entirely within the core DT gpio support code, the number used by
linux isn't actually important for building a .dts file.

> +	} else {
> +		if (!pdev->dev.platform_data)
> +			return -EINVAL;
>  
> -	if (!pdev->dev.platform_data)
> -		return -EINVAL;
> -
> -	pdata = pdev->dev.platform_data;
> +		pdata = pdev->dev.platform_data;
> +		id = pdev->id;
> +	}
>  
>  	if (!gpio_init_done) {
>  		int ret;
> @@ -1171,7 +1188,6 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
>  			return ret;
>  	}
>  
> -	id = pdev->id;
>  	bank = &gpio_bank[id];
>  
>  	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> @@ -1181,12 +1197,19 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
>  	}
>  
>  	bank->irq = res->start;
> -	bank->virtual_irq_start = pdata->virtual_irq_start;
>  	bank->method = pdata->bank_type;
>  	bank->dev = &pdev->dev;
> -	bank->dbck_flag = pdata->dbck_flag;
>  	bank->stride = pdata->bank_stride;
> -	bank->width = pdata->bank_width;
> +	if (match) {
> +		of_property_read_u32(node, "bank_width", &bank->width);
> +		if (of_get_property(node, "debounce", NULL))
> +			bank->dbck_flag = true;
> +		bank->virtual_irq_start = IH_GPIO_BASE + 32 * id;
> +	} else {
> +		bank->width = pdata->bank_width;
> +		bank->dbck_flag = pdata->dbck_flag;
> +		bank->virtual_irq_start = pdata->virtual_irq_start;
> +	}
>  
>  	bank->regs = pdata->regs;
>  
> @@ -1559,10 +1582,72 @@ void omap_gpio_restore_context(void)
>  }
>  #endif
>  
> +
> +static struct omap_gpio_reg_offs omap2_gpio_regs = {
> +	.revision =		OMAP24XX_GPIO_REVISION,
> +	.direction =		OMAP24XX_GPIO_OE,
> +	.datain =		OMAP24XX_GPIO_DATAIN,
> +	.dataout =		OMAP24XX_GPIO_DATAOUT,
> +	.set_dataout =		OMAP24XX_GPIO_SETDATAOUT,
> +	.clr_dataout =		OMAP24XX_GPIO_CLEARDATAOUT,
> +	.irqstatus =		OMAP24XX_GPIO_IRQSTATUS1,
> +	.irqstatus2 =		OMAP24XX_GPIO_IRQSTATUS2,
> +	.irqenable =		OMAP24XX_GPIO_IRQENABLE1,
> +	.set_irqenable =	OMAP24XX_GPIO_SETIRQENABLE1,
> +	.clr_irqenable =	OMAP24XX_GPIO_CLEARIRQENABLE1,
> +	.debounce =		OMAP24XX_GPIO_DEBOUNCE_VAL,
> +	.debounce_en =		OMAP24XX_GPIO_DEBOUNCE_EN,
> +};
> +
> +static struct omap_gpio_platform_data omap2_pdata = {
> +	.bank_type = METHOD_GPIO_24XX,
> +	.regs = &omap2_gpio_regs,
> +};
> +
> +static struct omap_gpio_reg_offs omap4_gpio_regs = {
> +	.revision =		OMAP4_GPIO_REVISION,
> +	.direction =		OMAP4_GPIO_OE,
> +	.datain =		OMAP4_GPIO_DATAIN,
> +	.dataout =		OMAP4_GPIO_DATAOUT,
> +	.set_dataout =		OMAP4_GPIO_SETDATAOUT,
> +	.clr_dataout =		OMAP4_GPIO_CLEARDATAOUT,
> +	.irqstatus =		OMAP4_GPIO_IRQSTATUS1,
> +	.irqstatus2 =		OMAP4_GPIO_IRQSTATUS2,
> +	.irqenable =		OMAP4_GPIO_IRQENABLE1,
> +	.set_irqenable =	OMAP4_GPIO_SETIRQENABLE1,
> +	.clr_irqenable =	OMAP4_GPIO_CLEARIRQENABLE1,
> +	.debounce =		OMAP4_GPIO_DEBOUNCINGTIME,
> +	.debounce_en =		OMAP4_GPIO_DEBOUNCENABLE,
> +};
> +
> +static struct omap_gpio_platform_data omap4_pdata = {
> +	.bank_type = METHOD_GPIO_44XX,
> +	.regs = &omap4_gpio_regs,
> +};
> +
> +
> +#if defined(CONFIG_OF)
> +	static const struct of_device_id omap_gpio_match[] = {
> +	{
> +		.compatible = "ti,omap4-gpio",
> +		.data = &omap4_pdata,
> +	},
> +	{
> +		.compatible = "ti,omap2-gpio",
> +		.data = &omap2_pdata,
> +	},
> +	{},
> +}
> +MODULE_DEVICE_TABLE(of, omap_gpio_match);
> +#else
> +#define omap_gpio_match NULL
> +#endif
> +
>  static struct platform_driver omap_gpio_driver = {
>  	.probe		= omap_gpio_probe,
>  	.driver		= {
>  		.name	= "omap_gpio",
> +		.of_match_table = omap_gpio_match,
>  	},
>  };
>  
> -- 
> 1.7.0.4
> 

[RFC PATCH 08/10] gpio/omap: Adapt GPIO driver to DT

[Cousson, Benoit]

On 9/8/2011 8:15 PM, Grant Likely wrote:
> On Wed, Aug 24, 2011 at 03:09:14PM +0200, Benoit Cousson wrote:
>> Adapt the GPIO driver to retrieve information from a DT file.
>> Note that since the driver is currently under cleanup, some hacks
>> will have to be removed after.
>>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Grant Likely<grant.likely@secretlab.ca>
>> Cc: Charulatha V<charu@ti.com>
>> Cc: Tarun Kanti DebBarma<tarun.kanti@ti.com>
>
> Mostly looks good.  Comments below.
>
>> ---
>>   drivers/gpio/gpio-omap.c |  103 ++++++++++++++++++++++++++++++++++++++++++----
>>   1 files changed, 94 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> index 0599854..96d19d7 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -21,6 +21,7 @@
>>   #include<linux/io.h>
>>   #include<linux/slab.h>
>>   #include<linux/pm_runtime.h>
>> +#include<linux/of_platform.h>
>>
>>   #include<mach/hardware.h>
>>   #include<asm/irq.h>
>> @@ -521,7 +522,7 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable)
>>   	unsigned long flags;
>>
>>   	if (bank->non_wakeup_gpios&  gpio_bit) {
>> -		dev_err(bank->dev,
>> +		dev_err(bank->dev,
>
> nit: unrelated whitespace change
>
>>   			"Unable to modify wakeup on non-wakeup GPIO%d\n", gpio);
>>   		return -EINVAL;
>>   	}
>> @@ -1150,6 +1151,8 @@ static void __devinit omap_gpio_chip_init(struct gpio_bank *bank)
>>   	irq_set_handler_data(bank->irq, bank);
>>   }
>>
>> +static const struct of_device_id omap_gpio_match[];
>> +
>>   static int __devinit omap_gpio_probe(struct platform_device *pdev)
>>   {
>>   	static int gpio_init_done;
>> @@ -1157,11 +1160,25 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
>>   	struct resource *res;
>>   	int id;
>>   	struct gpio_bank *bank;
>> +	struct device_node *node = pdev->dev.of_node;
>> +	const struct of_device_id *match;
>> +
>> +	match = of_match_device(omap_gpio_match,&pdev->dev);
>> +	if (match) {
>> +		pdata = match->data;
>> +		/* XXX: big hack until the bank_count is removed */
>> +		of_property_read_u32(node, "bank_count",&gpio_bank_count);
>
> Nit: by convention, '-' is preferred over '_' in DT property names.
>
>> +		if (of_property_read_u32(node, "id",&id))
>> +			return -EINVAL;
>> +		/* XXX: maybe the id in DT should be zero based to avoid that */
>> +		id -= 1;
>
> Actually, the reason it is -1 based, is that when using the DT, gpio
> number are dynamically assigned.  Since the gpio numbers are resolved
> entirely within the core DT gpio support code, the number used by
> linux isn't actually important for building a .dts file.

I'm not sure about that. The six controllers are handled as banks, so 
the order does matters. In fact it should be considered as a big GPIO 
controller with 192 entries. And this is that global number that the HW 
documentation, board definition and even pin mux use.
The user does not even have a clue about the controller used without 
doing a little bit of math.

Here is for example how a beagle board is using the gpio global number.

static struct gpio omap3_beagle_rev_gpios[] __initdata = {
	{ 171, GPIOF_IN, "rev_id_0"    },
	{ 172, GPIOF_IN, "rev_id_1" },
	{ 173, GPIOF_IN, "rev_id_2"    },
};

The current GPIO usage will force us doing that:

node {
	gpios = <&gpio6 11 0>,
		<&gpio6 12 0>,
		<&gpio6 13 0>;
};

It looks to me that this kind of conversion tends to be error prone.

I guess that this is a quite standard organization, so is there a way to 
handle that global number? Like that for example:

node {
	gpios = <&omap_gpio 171 0>,
		<&omap_gpio 172 0>,
		<&omap_gpio 173 0>;
};


Benoit

[RFC PATCH 09/10] arm/dts: OMAP4: Add gpio nodes

[Benoit Cousson]

Add the 6 GPIOs controller nodes.
Since the GPIO driver is still under cleanup, a couple of temp
hacks are needed. They will be removed as soon as the driver will
be cleaned.

Remove gpio static device initialisation if CONFIG_OF is defined.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Charulatha V <charu@ti.com>
---
 arch/arm/boot/dts/omap4.dtsi |   69 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/gpio.c   |    2 +
 2 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 231d7b4..f672927 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -110,5 +110,74 @@
 			compatible = "ti,omap-spinlock";
 			hwmods = "spinlock";
 		};
+
+		gpio1: gpio at 1 {
+			compatible = "ti,omap4-gpio", "ti,omap-gpio";
+			hwmods = "gpio1";
+			/* id should not be needed with a global GPIO parent */
+			id = <1>;
+			bank_width = <32>;
+			debounce;
+			/* XXX: big hack until the bank_count is removed */
+			bank_count = <6>;
+			no_idle_on_suspend;
+			#gpio-cells = <2>;
+			gpio-controller;
+		};
+
+		gpio2: gpio at 2 {
+			compatible = "ti,omap4-gpio", "ti,omap-gpio";
+			hwmods = "gpio2";
+			id = <2>;
+			bank_width = <32>;
+			debounce;
+			no_idle_on_suspend;
+			#gpio-cells = <2>;
+			gpio-controller;
+		};
+
+		gpio3: gpio at 3 {
+			compatible = "ti,omap4-gpio", "ti,omap-gpio";
+			hwmods = "gpio3";
+			id = <3>;
+			bank_width = <32>;
+			debounce;
+			no_idle_on_suspend;
+			#gpio-cells = <2>;
+			gpio-controller;
+		};
+
+		gpio4: gpio at 4 {
+			compatible = "ti,omap4-gpio", "ti,omap-gpio";
+			hwmods = "gpio4";
+			id = <4>;
+			bank_width = <32>;
+			debounce;
+			no_idle_on_suspend;
+			#gpio-cells = <2>;
+			gpio-controller;
+		};
+
+		gpio5: gpio at 5 {
+			compatible = "ti,omap4-gpio", "ti,omap-gpio";
+			hwmods = "gpio5";
+			id = <5>;
+			bank_width = <32>;
+			debounce;
+			no_idle_on_suspend;
+			#gpio-cells = <2>;
+			gpio-controller;
+		};
+
+		gpio6: gpio at 6 {
+			compatible = "ti,omap4-gpio", "ti,omap-gpio";
+			hwmods = "gpio6";
+			id = <6>;
+			bank_width = <32>;
+			debounce;
+			no_idle_on_suspend;
+			#gpio-cells = <2>;
+			gpio-controller;
+		};
 	};
 };
diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
index 8cbfbc2..c51e952 100644
--- a/arch/arm/mach-omap2/gpio.c
+++ b/arch/arm/mach-omap2/gpio.c
@@ -16,6 +16,7 @@
  * GNU General Public License for more details.
  */
 
+#ifndef CONFIG_OF
 #include <linux/gpio.h>
 #include <linux/err.h>
 #include <linux/slab.h>
@@ -126,3 +127,4 @@ static int __init omap2_gpio_init(void)
 						NULL);
 }
 postcore_initcall(omap2_gpio_init);
+#endif
-- 
1.7.0.4

[RFC PATCH 09/10] arm/dts: OMAP4: Add gpio nodes

[Grant Likely]

On Wed, Aug 24, 2011 at 03:09:15PM +0200, Benoit Cousson wrote:
> Add the 6 GPIOs controller nodes.
> Since the GPIO driver is still under cleanup, a couple of temp
> hacks are needed. They will be removed as soon as the driver will
> be cleaned.
> 
> Remove gpio static device initialisation if CONFIG_OF is defined.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Charulatha V <charu@ti.com>
> ---
>  arch/arm/boot/dts/omap4.dtsi |   69 ++++++++++++++++++++++++++++++++++++++++++
>  arch/arm/mach-omap2/gpio.c   |    2 +
>  2 files changed, 71 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 231d7b4..f672927 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -110,5 +110,74 @@
>  			compatible = "ti,omap-spinlock";
>  			hwmods = "spinlock";
>  		};
> +
> +		gpio1: gpio at 1 {
> +			compatible = "ti,omap4-gpio", "ti,omap-gpio";
> +			hwmods = "gpio1";
> +			/* id should not be needed with a global GPIO parent */
> +			id = <1>;
> +			bank_width = <32>;
> +			debounce;
> +			/* XXX: big hack until the bank_count is removed */
> +			bank_count = <6>;
> +			no_idle_on_suspend;
> +			#gpio-cells = <2>;
> +			gpio-controller;
> +		};
> +
> +		gpio2: gpio at 2 {
> +			compatible = "ti,omap4-gpio", "ti,omap-gpio";
> +			hwmods = "gpio2";
> +			id = <2>;
> +			bank_width = <32>;
> +			debounce;
> +			no_idle_on_suspend;
> +			#gpio-cells = <2>;
> +			gpio-controller;
> +		};
> +
> +		gpio3: gpio at 3 {
> +			compatible = "ti,omap4-gpio", "ti,omap-gpio";
> +			hwmods = "gpio3";
> +			id = <3>;
> +			bank_width = <32>;
> +			debounce;
> +			no_idle_on_suspend;
> +			#gpio-cells = <2>;
> +			gpio-controller;
> +		};
> +
> +		gpio4: gpio at 4 {
> +			compatible = "ti,omap4-gpio", "ti,omap-gpio";
> +			hwmods = "gpio4";
> +			id = <4>;
> +			bank_width = <32>;
> +			debounce;
> +			no_idle_on_suspend;
> +			#gpio-cells = <2>;
> +			gpio-controller;
> +		};
> +
> +		gpio5: gpio at 5 {
> +			compatible = "ti,omap4-gpio", "ti,omap-gpio";
> +			hwmods = "gpio5";
> +			id = <5>;
> +			bank_width = <32>;
> +			debounce;
> +			no_idle_on_suspend;
> +			#gpio-cells = <2>;
> +			gpio-controller;
> +		};
> +
> +		gpio6: gpio at 6 {
> +			compatible = "ti,omap4-gpio", "ti,omap-gpio";
> +			hwmods = "gpio6";
> +			id = <6>;
> +			bank_width = <32>;
> +			debounce;
> +			no_idle_on_suspend;
> +			#gpio-cells = <2>;
> +			gpio-controller;
> +		};
>  	};
>  };
> diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
> index 8cbfbc2..c51e952 100644
> --- a/arch/arm/mach-omap2/gpio.c
> +++ b/arch/arm/mach-omap2/gpio.c
> @@ -16,6 +16,7 @@
>   * GNU General Public License for more details.
>   */
>  
> +#ifndef CONFIG_OF
>  #include <linux/gpio.h>
>  #include <linux/err.h>
>  #include <linux/slab.h>
> @@ -126,3 +127,4 @@ static int __init omap2_gpio_init(void)
>  						NULL);
>  }
>  postcore_initcall(omap2_gpio_init);
> +#endif

Same as on earlier patch, don't #ifndef CONFIG_OF out non-DT code.

g.

> -- 
> 1.7.0.4
> 

[RFC PATCH 10/10] documentation/dt: Add OMAP GPIO properties

[Benoit Cousson]

Add documentation for GPIO properties specific to OMAP.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Randy Dunlap <rdunlap@xenotime.net>
---
 .../devicetree/bindings/gpio/gpio-omap.txt         |   33 ++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-omap.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-omap.txt b/Documentation/devicetree/bindings/gpio/gpio-omap.txt
new file mode 100644
index 0000000..692b1c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-omap.txt
@@ -0,0 +1,33 @@
+OMAP GPIO controller
+
+Required properties:
+- compatible:
+  - "ti,omap2-gpio" for OMAP2 and OMAP3 controllers
+  - "ti,omap4-gpio" for OMAP4 controller
+- #gpio-cells : Should be two.
+  - first cell is the pin number
+  - second cell is used to specify optional parameters (unused)
+- gpio-controller : Marks the device node as a GPIO controller.
+
+OMAP specific properties:
+- hwmods: Name of the hwmod associated to the GPIO
+- id: 32 bits to identify the id (1 based index)
+- bank_width: number of pin supported by the controller (16 or 32)
+- debounce: set if the controller support the debouce funtionnality
+- bank_count: number of controller support by the SoC. This is a temporary
+  hack until the bank_count is removed from the driver.
+
+
+Example:
+
+gpio4: gpio4 {
+    compatible = "ti,omap4-gpio", "ti,omap-gpio";
+    hwmods = "gpio4";
+    id = <4>;
+    bank_width = <32>;
+    debounce;
+    no_idle_on_suspend;
+    #gpio-cells = <2>;
+    gpio-controller;
+};
+
-- 
1.7.0.4

[RFC PATCH 10/10] documentation/dt: Add OMAP GPIO properties

[Grant Likely]

On Wed, Aug 24, 2011 at 03:09:16PM +0200, Benoit Cousson wrote:
> Add documentation for GPIO properties specific to OMAP.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Randy Dunlap <rdunlap@xenotime.net>
> ---
>  .../devicetree/bindings/gpio/gpio-omap.txt         |   33 ++++++++++++++++++++
>  1 files changed, 33 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-omap.txt
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-omap.txt b/Documentation/devicetree/bindings/gpio/gpio-omap.txt
> new file mode 100644
> index 0000000..692b1c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-omap.txt
> @@ -0,0 +1,33 @@
> +OMAP GPIO controller
> +
> +Required properties:
> +- compatible:
> +  - "ti,omap2-gpio" for OMAP2 and OMAP3 controllers
> +  - "ti,omap4-gpio" for OMAP4 controller
> +- #gpio-cells : Should be two.
> +  - first cell is the pin number
> +  - second cell is used to specify optional parameters (unused)
> +- gpio-controller : Marks the device node as a GPIO controller.
> +
> +OMAP specific properties:
> +- hwmods: Name of the hwmod associated to the GPIO
> +- id: 32 bits to identify the id (1 based index)

id should not be necessary.  Linux GPIO numbers are to be dynamically
assigned when using DT.

> +- bank_width: number of pin supported by the controller (16 or 32)
> +- debounce: set if the controller support the debouce funtionnality
> +- bank_count: number of controller support by the SoC. This is a temporary
> +  hack until the bank_count is removed from the driver.

All these properties should be prefixed with "ti,".
Use '-' instead of '_' in property names

> +
> +
> +Example:
> +
> +gpio4: gpio4 {
> +    compatible = "ti,omap4-gpio", "ti,omap-gpio";
> +    hwmods = "gpio4";
> +    id = <4>;
> +    bank_width = <32>;
> +    debounce;
> +    no_idle_on_suspend;
> +    #gpio-cells = <2>;
> +    gpio-controller;
> +};
> +
> -- 
> 1.7.0.4
> 

[RFC PATCH 10/10] documentation/dt: Add OMAP GPIO properties

[Cousson, Benoit]

On 9/8/2011 8:18 PM, Grant Likely wrote:
> On Wed, Aug 24, 2011 at 03:09:16PM +0200, Benoit Cousson wrote:
>> Add documentation for GPIO properties specific to OMAP.
>>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Randy Dunlap<rdunlap@xenotime.net>
>> ---
>>   .../devicetree/bindings/gpio/gpio-omap.txt         |   33 ++++++++++++++++++++
>>   1 files changed, 33 insertions(+), 0 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/gpio/gpio-omap.txt
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-omap.txt b/Documentation/devicetree/bindings/gpio/gpio-omap.txt
>> new file mode 100644
>> index 0000000..692b1c7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-omap.txt
>> @@ -0,0 +1,33 @@
>> +OMAP GPIO controller
>> +
>> +Required properties:
>> +- compatible:
>> +  - "ti,omap2-gpio" for OMAP2 and OMAP3 controllers
>> +  - "ti,omap4-gpio" for OMAP4 controller
>> +- #gpio-cells : Should be two.
>> +  - first cell is the pin number
>> +  - second cell is used to specify optional parameters (unused)
>> +- gpio-controller : Marks the device node as a GPIO controller.
>> +
>> +OMAP specific properties:
>> +- hwmods: Name of the hwmod associated to the GPIO
>> +- id: 32 bits to identify the id (1 based index)
>
> id should not be necessary.  Linux GPIO numbers are to be dynamically
> assigned when using DT.
>
>> +- bank_width: number of pin supported by the controller (16 or 32)
>> +- debounce: set if the controller support the debouce funtionnality
>> +- bank_count: number of controller support by the SoC. This is a temporary
>> +  hack until the bank_count is removed from the driver.
>
> All these properties should be prefixed with "ti,".
> Use '-' instead of '_' in property names

I'll fix that.

Thanks,
Benoit