OMAP: Add DT support for early init OMAP4 devices
Subsystems (9)
cover OMAP: Add DT support for early init OMAP4 devices
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 1/10 OMAP2+: l3-noc: Add support for device-tree
Add device-tree support for the l3-noc driver.
Use platform_driver_register to defer the probing at device init time.
--- arch/arm/mach-omap2/omap_l3_noc.c | 16 ++++++++++++++-- 1 files changed, 14 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/omap_l3_noc.c +14 -2
@@ -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);
Signature · 1 line
1.7.0.4
RFC PATCH 1/10 OMAP2+: l3-noc: Add support for device-tree
Quoted 17 lines · Benoit Cousson
Add device-tree support for the l3-noc driver.
Use
platform_driver_registerto defer the probing at device init time.Signed-off-by Benoit Cousson <[email protected]>Cc Tony Lindgren <[email protected]>Cc Santosh Shilimkar <[email protected]>--- arch/arm/mach-omap2/omap_l3_noc.c | 16 ++++++++++++++-- 1 files changed, 14 insertions(+), 2 deletions(-)arch/arm/mach-omap2/omap_l3_noc.c +0
-0
arch/arm/mach-omap2/omap_l3_noc.c static int __exit omap4_l3_remove(struct platform_device *pdev) · line 233
Quoted 6 lines
arch/arm/mach-omap2/omap_l3_noc.c +3
-0@@ -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.
arch/arm/mach-omap2/omap_l3_noc.c static int __exit omap4_l3_remove(struct platform_device *pdev) · line 242
Quoted 9 lines
arch/arm/mach-omap2/omap_l3_noc.c +8
-0+ {}, +} +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.
arch/arm/mach-omap2/omap_l3_noc.c static int __exit omap4_l3_remove(struct platform_device *pdev) · line 243
Quoted 1 line
arch/arm/mach-omap2/omap_l3_noc.c +0
-0.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.
arch/arm/mach-omap2/omap_l3_noc.c static int __exit omap4_l3_remove(struct platform_device *pdev) · line 246
Quoted 4 lines
arch/arm/mach-omap2/omap_l3_noc.c +2
-1.driver = { - .name = "omap_l3_noc", + .name = "omap_l3_noc", + .of_match_table = l3_noc_match,
Looks like ".owner = THIS_MODULE," is missing too.
Quoted 18 lines
arch/arm/mach-omap2/omap_l3_noc.c +1
-1}, };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);Signature · 7 lines
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 1/10 OMAP2+: l3-noc: Add support for device-tree
Quoted 21 lines · Grant Likely
Quoted 20 lines · Benoit Cousson
Add device-tree support for the l3-noc driver.
Use
platform_driver_registerto defer the probing at device init time.Signed-off-by Benoit Cousson<[email protected]>Cc Tony Lindgren<[email protected]>Cc Santosh Shilimkar<[email protected]>--- arch/arm/mach-omap2/omap_l3_noc.c | 16 ++++++++++++++-- 1 files changed, 14 insertions(+), 2 deletions(-)arch/arm/mach-omap2/omap_l3_noc.c +0
-0@@ -228,16 +228,28 @@ static int __exit omap4_l3_remove(struct platform_device *pdev)return 0 ; }
Quoted 5 lines
arch/arm/mach-omap2/omap_l3_noc.c static int __exit omap4_l3_remove(struct platform_device *pdev) · line 233
Quoted 3 lines
arch/arm/mach-omap2/omap_l3_noc.c +3
-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.
Quoted 2 lines
Also, it appears
to be rather on the generic side.
It is indeed a generic IP that will be there on OMAP5 too.
Quoted 17 lines
arch/arm/mach-omap2/omap_l3_noc.c static int __exit omap4_l3_remove(struct platform_device *pdev) · line 242
Quoted 9 lines
arch/arm/mach-omap2/omap_l3_noc.c +8
-0+ {}, +} +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
__devinitsection.arch/arm/mach-omap2/omap_l3_noc.c static int __exit omap4_l3_remove(struct platform_device *pdev) · line 243
Quoted 1 line
arch/arm/mach-omap2/omap_l3_noc.c +0
-0.remove = __exit_p(omap4_l3_remove),Similarly, at the same time the remove hook should be changed to
__devexitand__devexit_p()at the same time.
Quoted 6 lines
arch/arm/mach-omap2/omap_l3_noc.c static int __exit omap4_l3_remove(struct platform_device *pdev) · line 246
Quoted 4 lines
arch/arm/mach-omap2/omap_l3_noc.c +2
-1.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 1/10 OMAP2+: l3-noc: Add support for device-tree
Quoted 28 lines · Benoit
Quoted 21 lines · Grant Likely
Quoted 20 lines · Benoit Cousson
Add device-tree support for the l3-noc driver.
Use
platform_driver_registerto defer the probing at device init time.Signed-off-by Benoit Cousson<[email protected]>Cc Tony Lindgren<[email protected]>Cc Santosh Shilimkar<[email protected]>--- arch/arm/mach-omap2/omap_l3_noc.c | 16 ++++++++++++++-- 1 files changed, 14 insertions(+), 2 deletions(-)arch/arm/mach-omap2/omap_l3_noc.c +0
-0@@ -228,16 +228,28 @@ static int __exit omap4_l3_remove(struct platform_device *pdev)return 0 ; }Quoted 5 lines
arch/arm/mach-omap2/omap_l3_noc.c static int __exit omap4_l3_remove(struct platform_device *pdev) · line 233
Quoted 3 lines
arch/arm/mach-omap2/omap_l3_noc.c +3
-0+#if defined(CONFIG_OF ) +static const struct of_device_id l3_noc_match[] = { + {.compatible = "arteris,noc", },Missing documentation for this compatible property.
Quoted 6 lines
As you already figured out... it will come later.
Quoted 2 lines
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 2/10 arm/dts: OMAP4: Add a main ocp entry bound to l3-noc driver
In reply to Benoit CoussonUsed the main OCP node to add bindings with the l3_noc driver. Remove l3_noc static device creation if CONFIG_OF is defined.
--- arch/arm/boot/dts/omap4.dtsi | 3 ++- arch/arm/mach-omap2/devices.c | 2 ++ 2 files changed, 4 insertions(+), 1 deletions(-)
arch/arm/boot/dts/omap4.dtsi +2 -1
@@ -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";
arch/arm/mach-omap2/devices.c +2 -0
@@ -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 )
Signature · 1 line
1.7.0.4
RFC PATCH 2/10 arm/dts: OMAP4: Add a main ocp entry bound to l3-noc driver
Quoted 41 lines · Benoit Cousson
Used the main OCP node to add bindings with the
l3_nocdriver. Removel3_nocstatic device creation ifCONFIG_OFis defined.Signed-off-by Benoit Cousson <[email protected]>Cc Tony Lindgren <[email protected]>Cc Santosh Shilimkar <[email protected]>--- arch/arm/boot/dts/omap4.dtsi | 3 ++- arch/arm/mach-omap2/devices.c | 2 ++ 2 files changed, 4 insertions(+), 1 deletions(-)arch/arm/boot/dts/omap4.dtsi +2
-1@@ -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";arch/arm/mach-omap2/devices.c +1
-0@@ -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;
arch/arm/mach-omap2/devices.c static int __init omap4_l3_init(void) · line 104
Quoted 4 lines
arch/arm/mach-omap2/devices.c +1
-0@@ -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 2/10 arm/dts: OMAP4: Add a main ocp entry bound to l3-noc driver
Quoted 37 lines · Grant Likely
Quoted 36 lines · Benoit Cousson
Used the main OCP node to add bindings with the
l3_nocdriver. Removel3_nocstatic device creation ifCONFIG_OFis defined.Signed-off-by Benoit Cousson<[email protected]>Cc Tony Lindgren<[email protected]>Cc Santosh Shilimkar<[email protected]>--- arch/arm/boot/dts/omap4.dtsi | 3 ++- arch/arm/mach-omap2/devices.c | 2 ++ 2 files changed, 4 insertions(+), 1 deletions(-)arch/arm/boot/dts/omap4.dtsi +2
-1@@ -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";arch/arm/mach-omap2/devices.c +0
-0@@ -70,6 +70,7 @@ static int __init omap3_l3_init(void) }postcore_initcall (omap3_l3_init);
Quoted 13 lines
arch/arm/mach-omap2/devices.c static int __init omap4_l3_init(void) · line 104
Quoted 9 lines
arch/arm/mach-omap2/devices.c +2
-0+#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_OFmust 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 2/10 arm/dts: OMAP4: Add a main ocp entry bound to l3-noc driver
Quoted 52 lines · Benoit
Quoted 37 lines · Grant Likely
Quoted 36 lines · Benoit Cousson
Used the main OCP node to add bindings with the
l3_nocdriver. Removel3_nocstatic device creation ifCONFIG_OFis defined.Signed-off-by Benoit Cousson<[email protected]>Cc Tony Lindgren<[email protected]>Cc Santosh Shilimkar<[email protected]>--- arch/arm/boot/dts/omap4.dtsi | 3 ++- arch/arm/mach-omap2/devices.c | 2 ++ 2 files changed, 4 insertions(+), 1 deletions(-)arch/arm/boot/dts/omap4.dtsi +2
-1@@ -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";arch/arm/mach-omap2/devices.c +0
-0@@ -70,6 +70,7 @@ static int __init omap3_l3_init(void) }postcore_initcall (omap3_l3_init);Quoted 13 lines
arch/arm/mach-omap2/devices.c static int __init omap4_l3_init(void) · line 104
Quoted 9 lines
arch/arm/mach-omap2/devices.c +2
-0+#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_OFmust not break non-DT booting. Instead, check at runtime if a DT is available, and if it does then don't run the hook.
Quoted 2 lines
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 3/10 documentation/dt: Add l3-noc bindings
In reply to Benoit CoussonAdd documentation for the l3-noc bindings.
--- .../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
Documentation/devicetree/bindings/arm/omap/l3-noc.txt new file +18 -0
@@ -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"; +};
Signature · 1 line
1.7.0.4
RFC PATCH 3/10 documentation/dt: Add l3-noc bindings
Quoted 15 lines · Benoit Cousson
Add documentation for the l3-noc bindings.
Signed-off-by Benoit Cousson <[email protected]>Cc Randy Dunlap <[email protected]>--- .../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.txtDocumentation/devicetree/bindings/arm/omap/l3-noc.txt new file +0
-0
Documentation/devicetree/bindings/arm/omap/l3-noc.txt line 4
Quoted 4 lines
Documentation/devicetree/bindings/arm/omap/l3-noc.txt new file +4
-0@@ -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.
Documentation/devicetree/bindings/arm/omap/l3-noc.txt line 7
Quoted 3 lines
Documentation/devicetree/bindings/arm/omap/l3-noc.txt new file +3
-0+ +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.
Documentation/devicetree/bindings/arm/omap/l3-noc.txt line 8
Quoted 1 line
Documentation/devicetree/bindings/arm/omap/l3-noc.txt new file +1
-0+- 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.
Quoted 13 lines
Documentation/devicetree/bindings/arm/omap/l3-noc.txt new file +10
-0+ +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"; +};Signature · 1 line
1.7.0.4
RFC PATCH 3/10 documentation/dt: Add l3-noc bindings
Quoted 23 lines · Grant Likely
Quoted 15 lines · Benoit Cousson
Add documentation for the l3-noc bindings.
Signed-off-by Benoit Cousson<[email protected]>Cc Randy Dunlap<[email protected]>--- .../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.txtDocumentation/devicetree/bindings/arm/omap/l3-noc.txt new file +0
-0Documentation/devicetree/bindings/arm/omap/l3-noc.txt line 4
Quoted 4 lines
Documentation/devicetree/bindings/arm/omap/l3-noc.txt new file +4
-0@@ -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.
Quoted 8 lines
Documentation/devicetree/bindings/arm/omap/l3-noc.txt line 7
Quoted 3 lines
Documentation/devicetree/bindings/arm/omap/l3-noc.txt new file +3
-0+ +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.
Quoted 5 lines
Documentation/devicetree/bindings/arm/omap/l3-noc.txt line 8
Quoted 2 lines
Documentation/devicetree/bindings/arm/omap/l3-noc.txt new file +1
-0+- 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
Quoted 2 lines
I expect that this should be "ti,hwmods" because this is a TI specific property.
OK.
Benoit
RFC PATCH 4/10 arm/dts: OMAP4: Add mpu, dsp and iva nodes
In reply to Benoit CoussonAdd 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.
--- arch/arm/boot/dts/omap4.dtsi | 28 ++++++++++++++++++++++++++++ arch/arm/mach-omap2/pm.c | 5 ++++- 2 files changed, 32 insertions(+), 1 deletions(-)
arch/arm/boot/dts/omap4.dtsi +28 -0
@@ -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"; + }; }; /*
arch/arm/mach-omap2/pm.c +4 -1
@@ -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" ); } }
Signature · 1 line
1.7.0.4
RFC PATCH 4/10 arm/dts: OMAP4: Add mpu, dsp and iva nodes
Quoted 69 lines · 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_OFis 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 <[email protected]>Cc Grant Likely <[email protected]>Cc Kevin Hilman <[email protected]>--- arch/arm/boot/dts/omap4.dtsi | 28 ++++++++++++++++++++++++++++ arch/arm/mach-omap2/pm.c | 5 ++++- 2 files changed, 32 insertions(+), 1 deletions(-)arch/arm/boot/dts/omap4.dtsi +28
-0@@ -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"; + }; }; /*arch/arm/mach-omap2/pm.c +0
-1@@ -49,15 +49,18 @@ static int _init_omap_device(char *name)*/ static void omap2_init_processor_devices (void ) { -_init_omap_device ("mpu" );
arch/arm/mach-omap2/pm.c static int _init_omap_device(char *name) · line 61
Quoted 10 lines
arch/arm/mach-omap2/pm.c +3
-0if (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.
Quoted 8 lines
arch/arm/mach-omap2/pm.c +1
-0}else { +_init_omap_device ("mpu" );_init_omap_device ("l3_main" ); } }Signature · 1 line
1.7.0.4
RFC PATCH 5/10 documentation/dt: Add mpu, dsp and iva bindings
In reply to Benoit CoussonAdd documentation for the OMAP4 processors bindings.
--- 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
Documentation/devicetree/bindings/arm/omap/dsp.txt new file +14 -0
@@ -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"; +};
Documentation/devicetree/bindings/arm/omap/iva.txt new file +18 -0
@@ -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"; +};
Documentation/devicetree/bindings/arm/omap/mpu.txt new file +35 -0
@@ -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"; + }; +};
Signature · 1 line
1.7.0.4
RFC PATCH 5/10 documentation/dt: Add mpu, dsp and iva bindings
Quoted 39 lines · Benoit Cousson
Add documentation for the OMAP4 processors bindings.
Signed-off-by Benoit Cousson <[email protected]>Cc Randy Dunlap <[email protected]>--- 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.txtDocumentation/devicetree/bindings/arm/omap/dsp.txt new file +14
-0@@ -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"; +};Documentation/devicetree/bindings/arm/omap/iva.txt new file +0
-0
Documentation/devicetree/bindings/arm/omap/iva.txt line 10
Quoted 10 lines
Documentation/devicetree/bindings/arm/omap/iva.txt new file +10
-0@@ -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 5/10 documentation/dt: Add mpu, dsp and iva bindings
Quoted 45 lines · Grant Likely
Quoted 44 lines · Benoit Cousson
Add documentation for the OMAP4 processors bindings.
Signed-off-by Benoit Cousson<[email protected]>Cc Randy Dunlap<[email protected]>--- 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.txtcreate mode 100644Documentation/devicetree/bindings/arm/omap/iva.txtcreate mode 100644Documentation/devicetree/bindings/arm/omap/mpu.txtDocumentation/devicetree/bindings/arm/omap/dsp.txt new file +14
-0@@ -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"; +};Documentation/devicetree/bindings/arm/omap/iva.txt new file +5
-0@@ -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. +
Quoted 8 lines
Documentation/devicetree/bindings/arm/omap/iva.txt line 10
Quoted 5 lines
Documentation/devicetree/bindings/arm/omap/iva.txt new file +5
-0+Required properties: +- compatible : Should be: + - "ti,ivahd", "ti,iva" for OMAP4 + - "ti,iva2", "ti,iva" for OMAP3 + - "ti,iva1", "ti,iva" for OMAP2Again, 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 5/10 documentation/dt: Add mpu, dsp and iva bindings
Quoted 20 lines · Benoit
Quoted 7 lines · Grant Likely
Documentation/devicetree/bindings/arm/omap/iva.txt line 10
Quoted 4 lines
+- compatible : Should be:
Documentation/devicetree/bindings/arm/omap/iva.txt new file +3
-0+ - "ti,ivahd", "ti,iva" for OMAP4 + - "ti,iva2", "ti,iva" for OMAP3 + - "ti,iva1", "ti,iva" for OMAP2Again, 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:(quoted diff) +4
-0+ - "ti,ivahd", for OMAP4 + - "ti,iva2.2", for OMAP3 + - "ti,iva2.1", for OMAP2430 + - "ti,iva1", for OMAP2420
Quoted 1 line
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 6/10 hwspinlock: OMAP4: Add spinlock support in DT
In reply to Benoit CoussonAdd 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.
--- 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(-)
arch/arm/boot/dts/omap4.dtsi +5 -0
@@ -105,5 +105,10 @@ reg = <0x48241000 0x1000>, <0x48240100 0x0200>; }; + + spinlock { + compatible = "ti,omap-spinlock"; + hwmods = "spinlock"; + }; }; };
arch/arm/mach-omap2/Makefile +5 -1
@@ -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
drivers/hwspinlock/omap_hwspinlock.c +11 -0
@@ -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, }, };
Signature · 1 line
1.7.0.4
RFC PATCH 6/10 hwspinlock: OMAP4: Add spinlock support in DT
Quoted 10 lines · Benoit Cousson
Add a device-tree node for the spinlock.
Remove the static device build code ifCONFIG_OFis set.
Update the hwspinlock driver to use theof_matchmethod. Add the information inDocumentation/devicetree.Signed-off-by Benoit Cousson <[email protected]>Cc Grant Likely <[email protected]>Cc Ohad Ben-Cohen <[email protected]>---
...
Quoted 4 lines
+ ? ? ? ? ? ? ? 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 ?
drivers/hwspinlock/omap_hwspinlock.c static int omap_hwspinlock_remove(struct platform_device *pdev) · line 210
Quoted 5 lines
drivers/hwspinlock/omap_hwspinlock.c +5
-0+#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 6/10 hwspinlock: OMAP4: Add spinlock support in DT
Hi Ohad,
Quoted 36 lines · Ohad Ben-Cohen
Quoted 10 lines · Benoit Cousson
Add a device-tree node for the spinlock.
Remove the static device build code ifCONFIG_OFis set.
Update the hwspinlock driver to use theof_matchmethod. Add the information inDocumentation/devicetree.Signed-off-by Benoit Cousson<[email protected]>Cc Grant Likely<[email protected]>Cc Ohad Ben-Cohen<[email protected]>---
...
arch/arm/boot/dts/omap4.dtsi line 112
Quoted 4 lines
arch/arm/boot/dts/omap4.dtsi +4
-0+ 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!).
Quoted 4 lines
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?
Quoted 8 lines
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".
Quoted 7 lines
drivers/hwspinlock/omap_hwspinlock.c static int omap_hwspinlock_remove(struct platform_device *pdev) · line 210
Quoted 5 lines
drivers/hwspinlock/omap_hwspinlock.c +5
-0+#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 6/10 hwspinlock: OMAP4: Add spinlock support in DT
Hi Benoit,
Quoted 4 lines · Benoit
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.
Quoted 4 lines
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 6/10 hwspinlock: OMAP4: Add spinlock support in DT
Quoted 8 lines · Ohad Ben-Cohen
Hi Benoit,
Quoted 4 lines · Benoit
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?
Quoted 7 lines
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.
Quoted 10 lines
Quoted 4 lines
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 6/10 hwspinlock: OMAP4: Add spinlock support in DT
Quoted 6 lines · Benoit
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 6/10 hwspinlock: OMAP4: Add spinlock support in DT
Quoted 9 lines · Ohad Ben-Cohen
Quoted 6 lines · Benoit
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 6/10 hwspinlock: OMAP4: Add spinlock support in DT
Quoted 17 lines · Arnd Bergmann
Quoted 9 lines · Ohad Ben-Cohen
Quoted 6 lines · Benoit
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.
Quoted 13 lines
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 6/10 hwspinlock: OMAP4: Add spinlock support in DT
Quoted 8 lines · Benoit
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 6/10 hwspinlock: OMAP4: Add spinlock support in DT
In reply to Arnd BergmannQuoted 5 lines · Arnd Bergmann
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 6/10 hwspinlock: OMAP4: Add spinlock support in DT
Quoted 14 lines · Ohad Ben-Cohen
Quoted 5 lines · Arnd Bergmann
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.
Quoted 5 lines
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
Quoted 5 lines
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).
Quoted 7 lines
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 6/10 hwspinlock: OMAP4: Add spinlock support in DT
Quoted 2 lines · Arnd Bergmann
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.
Quoted 2 lines
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 6/10 hwspinlock: OMAP4: Add spinlock support in DT
Quoted 6 lines · Ohad Ben-Cohen
Quoted 2 lines
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.
Quoted 12 lines
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 callhwspin_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).
Quoted 2 lines
Vast majority of hwspinlocks are not statically allocated, so this would keep the DT minimal, and IMHO, cleaner.
Ok.
Arnd
RFC PATCH 7/10 documentation/dt: Add spinlock bindings
In reply to Benoit CoussonAdd documentation for the HW spinlock in OMAP4.
--- .../bindings/hwspinlock/omap-spinlock.txt | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt
Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt new file +5 -0
@@ -0,0 +1,5 @@ +HW spinlock on OMAP platform: + +Required properties: +- compatible : Must be "ti,omap-spinlock"; +- hwmods : "spinlock"
Signature · 1 line
1.7.0.4
RFC PATCH 7/10 documentation/dt: Add spinlock bindings
Quoted 15 lines · Benoit Cousson
Add documentation for the HW spinlock in OMAP4.
Signed-off-by Benoit Cousson <[email protected]>Cc Randy Dunlap <[email protected]>--- .../bindings/hwspinlock/omap-spinlock.txt | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txtDocumentation/devicetree/bindings/hwspinlock/omap-spinlock.txt new file +0
-0
Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt line 4
Quoted 4 lines
Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt new file +4
-0@@ -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.
Quoted 4 lines
Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt new file +1
-0+- hwmods : "spinlock"Signature · 1 line
1.7.0.4
RFC PATCH 7/10 documentation/dt: Add spinlock bindings
Quoted 10 lines · Grant Likely
Quoted 9 lines · Benoit Cousson
Add documentation for the HW spinlock in OMAP4.
Signed-off-by Benoit Cousson<[email protected]>Cc Randy Dunlap<[email protected]>--- .../bindings/hwspinlock/omap-spinlock.txt | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)create mode 100644
Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt
Quoted 13 lines
Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt line 4
Quoted 10 lines
Documentation/devicetree/bindings/hwspinlock/omap-spinlock.txt new file +4
-0@@ -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 8/10 gpio/omap: Adapt GPIO driver to DT
In reply to Benoit CoussonAdapt 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.
--- drivers/gpio/gpio-omap.c | 103 ++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 94 insertions(+), 9 deletions(-)
drivers/gpio/gpio-omap.c +94 -9
@@ -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, }, };
Signature · 1 line
1.7.0.4
RFC PATCH 8/10 gpio/omap: Adapt GPIO driver to DT
Quoted 8 lines · 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 <[email protected]>Cc Grant Likely <[email protected]>Cc Charulatha V <[email protected]>Cc Tarun Kanti DebBarma <[email protected]>
Mostly looks good. Comments below.
Quoted 17 lines
--- drivers/gpio/gpio-omap.c | 103 ++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 94 insertions(+), 9 deletions(-)drivers/gpio/gpio-omap.c +1
-0@@ -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>
drivers/gpio/gpio-omap.c static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable) · line 525
Quoted 5 lines
drivers/gpio/gpio-omap.c +1
-1@@ -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
Quoted 14 lines
drivers/gpio/gpio-omap.c +2
-0"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;
drivers/gpio/gpio-omap.c static int __devinit omap_gpio_probe(struct platform_device *pdev) · line 1170
Quoted 10 lines
drivers/gpio/gpio-omap.c +8
-0int 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.
drivers/gpio/gpio-omap.c static int __devinit omap_gpio_probe(struct platform_device *pdev) · line 1174
Quoted 4 lines
drivers/gpio/gpio-omap.c +4
-0+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.
Quoted 121 lines
drivers/gpio/gpio-omap.c +78
-8+ }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, }, };Signature · 1 line
1.7.0.4
RFC PATCH 8/10 gpio/omap: Adapt GPIO driver to DT
Quoted 64 lines · Grant Likely
Quoted 8 lines · 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<[email protected]>Cc Grant Likely<[email protected]>Cc Charulatha V<[email protected]>Cc Tarun Kanti DebBarma<[email protected]>Mostly looks good. Comments below.
Quoted 17 lines
--- drivers/gpio/gpio-omap.c | 103 ++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 94 insertions(+), 9 deletions(-)drivers/gpio/gpio-omap.c +1
-0@@ -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> drivers/gpio/gpio-omap.c static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable) · line 525
Quoted 5 lines
drivers/gpio/gpio-omap.c +1
-1@@ -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
Quoted 14 lines
drivers/gpio/gpio-omap.c +2
-0"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;drivers/gpio/gpio-omap.c static int __devinit omap_gpio_probe(struct platform_device *pdev) · line 1170
Quoted 10 lines
drivers/gpio/gpio-omap.c +8
-0int 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.
Quoted 9 lines
drivers/gpio/gpio-omap.c static int __devinit omap_gpio_probe(struct platform_device *pdev) · line 1174
Quoted 4 lines
drivers/gpio/gpio-omap.c +4
-0+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 9/10 arm/dts: OMAP4: Add gpio nodes
In reply to Benoit CoussonAdd 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.
--- arch/arm/boot/dts/omap4.dtsi | 69 ++++++++++++++++++++++++++++++++++++++++++ arch/arm/mach-omap2/gpio.c | 2 + 2 files changed, 71 insertions(+), 0 deletions(-)
arch/arm/boot/dts/omap4.dtsi +69 -0
@@ -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; + }; }; };
arch/arm/mach-omap2/gpio.c +2 -0
@@ -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
Signature · 1 line
1.7.0.4
RFC PATCH 9/10 arm/dts: OMAP4: Add gpio nodes
Quoted 107 lines · 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_OFis defined.Signed-off-by Benoit Cousson <[email protected]>Cc Grant Likely <[email protected]>Cc Charulatha V <[email protected]>--- arch/arm/boot/dts/omap4.dtsi | 69 ++++++++++++++++++++++++++++++++++++++++++ arch/arm/mach-omap2/gpio.c | 2 + 2 files changed, 71 insertions(+), 0 deletions(-)arch/arm/boot/dts/omap4.dtsi +69
-0@@ -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; + }; }; };arch/arm/mach-omap2/gpio.c +1
-0@@ -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>
arch/arm/mach-omap2/gpio.c static int __init omap2_gpio_init(void) · line 130
Quoted 4 lines
arch/arm/mach-omap2/gpio.c +1
-0@@ -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.
Quoted 3 lines
Signature · 1 line
1.7.0.4
RFC PATCH 10/10 documentation/dt: Add OMAP GPIO properties
In reply to Benoit CoussonAdd documentation for GPIO properties specific to OMAP.
--- .../devicetree/bindings/gpio/gpio-omap.txt | 33 ++++++++++++++++++++ 1 files changed, 33 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-omap.txt
Documentation/devicetree/bindings/gpio/gpio-omap.txt new file +33 -0
@@ -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; +}; +
Signature · 1 line
1.7.0.4
RFC PATCH 10/10 documentation/dt: Add OMAP GPIO properties
Quoted 19 lines · Benoit Cousson
Add documentation for GPIO properties specific to OMAP.
Signed-off-by Benoit Cousson <[email protected]>Cc Randy Dunlap <[email protected]>--- .../devicetree/bindings/gpio/gpio-omap.txt | 33 ++++++++++++++++++++ 1 files changed, 33 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-omap.txtDocumentation/devicetree/bindings/gpio/gpio-omap.txt new file +4
-0@@ -0,0 +1,33 @@ +OMAP GPIO controller + +Required properties: +- compatible:
Documentation/devicetree/bindings/gpio/gpio-omap.txt line 14
Quoted 10 lines
Documentation/devicetree/bindings/gpio/gpio-omap.txt new file +10
-0+ - "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.
Documentation/devicetree/bindings/gpio/gpio-omap.txt line 18
Quoted 4 lines
Documentation/devicetree/bindings/gpio/gpio-omap.txt new file +4
-0+- 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
Quoted 18 lines
Documentation/devicetree/bindings/gpio/gpio-omap.txt new file +15
-0+ + +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; +}; +Signature · 1 line
1.7.0.4
RFC PATCH 10/10 documentation/dt: Add OMAP GPIO properties
Quoted 34 lines · Grant Likely
Quoted 19 lines · Benoit Cousson
Add documentation for GPIO properties specific to OMAP.
Signed-off-by Benoit Cousson<[email protected]>Cc Randy Dunlap<[email protected]>--- .../devicetree/bindings/gpio/gpio-omap.txt | 33 ++++++++++++++++++++ 1 files changed, 33 insertions(+), 0 deletions(-)create mode 100644
Documentation/devicetree/bindings/gpio/gpio-omap.txtDocumentation/devicetree/bindings/gpio/gpio-omap.txt new file +4
-0@@ -0,0 +1,33 @@ +OMAP GPIO controller + +Required properties: +- compatible:Documentation/devicetree/bindings/gpio/gpio-omap.txt line 14
Quoted 10 lines
Documentation/devicetree/bindings/gpio/gpio-omap.txt new file +10
-0+ - "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.
Quoted 7 lines
Documentation/devicetree/bindings/gpio/gpio-omap.txt line 18
Quoted 4 lines
Documentation/devicetree/bindings/gpio/gpio-omap.txt new file +4
-0+- 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