[PATCH 1/2] hwspinlock/omap: add support for dt nodes

[PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Suman Anna]

HwSpinlock IP is present only on OMAP4 and other newer SoCs,
which are all device-tree boot only. This patch adds the
base support for parsing the DT nodes, and removes the code
dealing with the traditional platform device instantiation.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++++++++++
 arch/arm/mach-omap2/Makefile                       |  3 --
 arch/arm/mach-omap2/hwspinlock.c                   | 60 ----------------------
 drivers/hwspinlock/omap_hwspinlock.c               | 21 ++++++--
 4 files changed, 45 insertions(+), 67 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
 delete mode 100644 arch/arm/mach-omap2/hwspinlock.c

diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
new file mode 100644
index 0000000..adfb8ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
@@ -0,0 +1,28 @@
+OMAP4+ HwSpinlock Driver
+
+Required properties:
+- compatible:		Currently supports only "ti,omap4-hwspinlock" for
+				OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
+- reg:			Contains the hwspinlock register address range (base
+			address and length)
+- ti,hwmods:		Name of the hwmod associated with the hwspinlock device
+
+Optional properties:
+- base_id:		Base Id for the locks for a particular hwspinlock
+			device. If not mentioned, a default value of 0 is used.
+			This property is mandatory ONLY if a SoC has several
+			hwspinlock devices. There are currently no such OMAP
+			SoCs.
+
+			See documentation on struct hwspinlock_pdata in
+			linux/hwspinlock.h for more details.
+
+
+Example:
+
+/* OMAP4 */
+hwspinlock: spinlock@4a0f6000 {
+	compatible = "ti,omap4-hwspinlock";
+	reg = <0x4a0f6000 0x1000>;
+	ti,hwmods = "spinlock";
+};
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index d4f6715..a17b5b8 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -288,9 +288,6 @@ obj-y					+= $(smc91x-m) $(smc91x-y)
 
 smsc911x-$(CONFIG_SMSC911X)		:= gpmc-smsc911x.o
 obj-y					+= $(smsc911x-m) $(smsc911x-y)
-ifneq ($(CONFIG_HWSPINLOCK_OMAP),)
-obj-y					+= hwspinlock.o
-endif
 
 emac-$(CONFIG_TI_DAVINCI_EMAC)		:= am35xx-emac.o
 obj-y					+= $(emac-m) $(emac-y)
diff --git a/arch/arm/mach-omap2/hwspinlock.c b/arch/arm/mach-omap2/hwspinlock.c
deleted file mode 100644
index ef175ac..0000000
--- a/arch/arm/mach-omap2/hwspinlock.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * OMAP hardware spinlock device initialization
- *
- * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
- *
- * Contact: Simon Que <sque@ti.com>
- *          Hari Kanigeri <h-kanigeri2@ti.com>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * General Public License for more details.
- */
-
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/err.h>
-#include <linux/hwspinlock.h>
-
-#include "soc.h"
-#include "omap_hwmod.h"
-#include "omap_device.h"
-
-static struct hwspinlock_pdata omap_hwspinlock_pdata __initdata = {
-	.base_id = 0,
-};
-
-static int __init hwspinlocks_init(void)
-{
-	int retval = 0;
-	struct omap_hwmod *oh;
-	struct platform_device *pdev;
-	const char *oh_name = "spinlock";
-	const char *dev_name = "omap_hwspinlock";
-
-	/*
-	 * Hwmod lookup will fail in case our platform doesn't support the
-	 * hardware spinlock module, so it is safe to run this initcall
-	 * on all omaps
-	 */
-	oh = omap_hwmod_lookup(oh_name);
-	if (oh == NULL)
-		return -EINVAL;
-
-	pdev = omap_device_build(dev_name, 0, oh, &omap_hwspinlock_pdata,
-				sizeof(struct hwspinlock_pdata));
-	if (IS_ERR(pdev)) {
-		pr_err("Can't build omap_device for %s:%s\n", dev_name,
-								oh_name);
-		retval = PTR_ERR(pdev);
-	}
-
-	return retval;
-}
-/* early board code might need to reserve specific hwspinlock instances */
-omap_postcore_initcall(hwspinlocks_init);
diff --git a/drivers/hwspinlock/omap_hwspinlock.c b/drivers/hwspinlock/omap_hwspinlock.c
index 292869c..d928235 100644
--- a/drivers/hwspinlock/omap_hwspinlock.c
+++ b/drivers/hwspinlock/omap_hwspinlock.c
@@ -1,7 +1,7 @@
 /*
  * OMAP hardware spinlock driver
  *
- * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2010-2013 Texas Instruments Incorporated - http://www.ti.com
  *
  * Contact: Simon Que <sque@ti.com>
  *          Hari Kanigeri <h-kanigeri2@ti.com>
@@ -27,6 +27,7 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/hwspinlock.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 
 #include "hwspinlock_internal.h"
@@ -80,14 +81,19 @@ static const struct hwspinlock_ops omap_hwspinlock_ops = {
 
 static int omap_hwspinlock_probe(struct platform_device *pdev)
 {
-	struct hwspinlock_pdata *pdata = pdev->dev.platform_data;
+	struct device_node *node = pdev->dev.of_node;
 	struct hwspinlock_device *bank;
 	struct hwspinlock *hwlock;
 	struct resource *res;
 	void __iomem *io_base;
 	int num_locks, i, ret;
+	int base_id = 0;
 
-	if (!pdata)
+	if (!node)
+		return -ENODEV;
+
+	ret = of_property_read_u32(node, "base_id", &base_id);
+	if (ret && ret != -EINVAL)
 		return -ENODEV;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -128,7 +134,7 @@ static int omap_hwspinlock_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 
 	ret = hwspin_lock_register(bank, &pdev->dev, &omap_hwspinlock_ops,
-						pdata->base_id, num_locks);
+						base_id, num_locks);
 	if (ret)
 		goto reg_fail;
 
@@ -161,12 +167,19 @@ static int omap_hwspinlock_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id omap_hwspinlock_of_match[] = {
+	{ .compatible = "ti,omap4-hwspinlock", },
+	{ /* end */ },
+};
+MODULE_DEVICE_TABLE(of, omap_hwspinlock_of_match);
+
 static struct platform_driver omap_hwspinlock_driver = {
 	.probe		= omap_hwspinlock_probe,
 	.remove		= omap_hwspinlock_remove,
 	.driver		= {
 		.name	= "omap_hwspinlock",
 		.owner	= THIS_MODULE,
+		.of_match_table = omap_hwspinlock_of_match,
 	},
 };
 
-- 
1.8.3.3

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Kumar Gala]

On Sep 3, 2013, at 12:52 PM, Suman Anna wrote:

> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
> which are all device-tree boot only. This patch adds the
> base support for parsing the DT nodes, and removes the code
> dealing with the traditional platform device instantiation.
> 
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
> .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++++++++++
> arch/arm/mach-omap2/Makefile                       |  3 --
> arch/arm/mach-omap2/hwspinlock.c                   | 60 ----------------------
> drivers/hwspinlock/omap_hwspinlock.c               | 21 ++++++--
> 4 files changed, 45 insertions(+), 67 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
> delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
> 
> diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
> new file mode 100644
> index 0000000..adfb8ad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
> @@ -0,0 +1,28 @@
> +OMAP4+ HwSpinlock Driver
> +
> +Required properties:
> +- compatible:		Currently supports only "ti,omap4-hwspinlock" for
> +				OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
> +- reg:			Contains the hwspinlock register address range (base
> +			address and length)
> +- ti,hwmods:		Name of the hwmod associated with the hwspinlock device
> +
> +Optional properties:
> +- base_id:		Base Id for the locks for a particular hwspinlock
> +			device. If not mentioned, a default value of 0 is used.
> +			This property is mandatory ONLY if a SoC has several
> +			hwspinlock devices. There are currently no such OMAP
> +			SoCs.

Should this be ti,base_id ? [ I know its kinda generic in its intent for any SoC w/multiple blocks ]

> +
> +			See documentation on struct hwspinlock_pdata in
> +			linux/hwspinlock.h for more details.
> +
> +
> +Example:
> +
> +/* OMAP4 */
> +hwspinlock: spinlock@4a0f6000 {
> +	compatible = "ti,omap4-hwspinlock";
> +	reg = <0x4a0f6000 0x1000>;
> +	ti,hwmods = "spinlock";
> +};

[ snip ]

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Suman Anna]

Kumar,

On 09/03/2013 01:50 PM, Kumar Gala wrote:
> 
> On Sep 3, 2013, at 12:52 PM, Suman Anna wrote:
> 
>> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
>> which are all device-tree boot only. This patch adds the
>> base support for parsing the DT nodes, and removes the code
>> dealing with the traditional platform device instantiation.
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> ---
>> .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++++++++++
>> arch/arm/mach-omap2/Makefile                       |  3 --
>> arch/arm/mach-omap2/hwspinlock.c                   | 60 ----------------------
>> drivers/hwspinlock/omap_hwspinlock.c               | 21 ++++++--
>> 4 files changed, 45 insertions(+), 67 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>> delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
>>
>> diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>> new file mode 100644
>> index 0000000..adfb8ad
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>> @@ -0,0 +1,28 @@
>> +OMAP4+ HwSpinlock Driver
>> +
>> +Required properties:
>> +- compatible:		Currently supports only "ti,omap4-hwspinlock" for
>> +				OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
>> +- reg:			Contains the hwspinlock register address range (base
>> +			address and length)
>> +- ti,hwmods:		Name of the hwmod associated with the hwspinlock device
>> +
>> +Optional properties:
>> +- base_id:		Base Id for the locks for a particular hwspinlock
>> +			device. If not mentioned, a default value of 0 is used.
>> +			This property is mandatory ONLY if a SoC has several
>> +			hwspinlock devices. There are currently no such OMAP
>> +			SoCs.
> 
> Should this be ti,base_id ? [ I know its kinda generic in its intent for any SoC w/multiple blocks ]

I didn't add the "ti," prefix exactly for the same reason - it is
generic w.r.t the hwspinlock core irrespective of the SoC family, and
there is nothing ti or OMAP specific about it. I have added it to keep
the DT node definition in sync with the driver code. If it is too
generic a name, it can always be renamed as hwlock_base_id. This will be
SoC agnostic property for the hwspinlock driver. What do you think?

regards
Suman

> 
>> +
>> +			See documentation on struct hwspinlock_pdata in
>> +			linux/hwspinlock.h for more details.
>> +
>> +
>> +Example:
>> +
>> +/* OMAP4 */
>> +hwspinlock: spinlock@4a0f6000 {
>> +	compatible = "ti,omap4-hwspinlock";
>> +	reg = <0x4a0f6000 0x1000>;
>> +	ti,hwmods = "spinlock";
>> +};
> 
> [ snip ]
> 
> - k
> 

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Kumar Gala]

On Sep 3, 2013, at 2:10 PM, Suman Anna wrote:

> Kumar,
> 
> On 09/03/2013 01:50 PM, Kumar Gala wrote:
>> 
>> On Sep 3, 2013, at 12:52 PM, Suman Anna wrote:
>> 
>>> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
>>> which are all device-tree boot only. This patch adds the
>>> base support for parsing the DT nodes, and removes the code
>>> dealing with the traditional platform device instantiation.
>>> 
>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>> ---
>>> .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++++++++++
>>> arch/arm/mach-omap2/Makefile                       |  3 --
>>> arch/arm/mach-omap2/hwspinlock.c                   | 60 ----------------------
>>> drivers/hwspinlock/omap_hwspinlock.c               | 21 ++++++--
>>> 4 files changed, 45 insertions(+), 67 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>> delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
>>> 
>>> diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>> new file mode 100644
>>> index 0000000..adfb8ad
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>> @@ -0,0 +1,28 @@
>>> +OMAP4+ HwSpinlock Driver
>>> +
>>> +Required properties:
>>> +- compatible:		Currently supports only "ti,omap4-hwspinlock" for
>>> +				OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
>>> +- reg:			Contains the hwspinlock register address range (base
>>> +			address and length)
>>> +- ti,hwmods:		Name of the hwmod associated with the hwspinlock device
>>> +
>>> +Optional properties:
>>> +- base_id:		Base Id for the locks for a particular hwspinlock
>>> +			device. If not mentioned, a default value of 0 is used.
>>> +			This property is mandatory ONLY if a SoC has several
>>> +			hwspinlock devices. There are currently no such OMAP
>>> +			SoCs.
>> 
>> Should this be ti,base_id ? [ I know its kinda generic in its intent for any SoC w/multiple blocks ]
> 
> I didn't add the "ti," prefix exactly for the same reason - it is
> generic w.r.t the hwspinlock core irrespective of the SoC family, and
> there is nothing ti or OMAP specific about it. I have added it to keep
> the DT node definition in sync with the driver code. If it is too
> generic a name, it can always be renamed as hwlock_base_id. This will be
> SoC agnostic property for the hwspinlock driver. What do you think?

I'm wondering if we should use cell-index for this purpose.

- k

> 
> regards
> Suman
> 
>> 
>>> +
>>> +			See documentation on struct hwspinlock_pdata in
>>> +			linux/hwspinlock.h for more details.
>>> +
>>> +
>>> +Example:
>>> +
>>> +/* OMAP4 */
>>> +hwspinlock: spinlock@4a0f6000 {
>>> +	compatible = "ti,omap4-hwspinlock";
>>> +	reg = <0x4a0f6000 0x1000>;
>>> +	ti,hwmods = "spinlock";
>>> +};
>> 
>> [ snip ]
>> 
>> - k
>> 
> 

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Suman Anna]

Kumar,

>>
>> On 09/03/2013 01:50 PM, Kumar Gala wrote:
>>>
>>> On Sep 3, 2013, at 12:52 PM, Suman Anna wrote:
>>>
>>>> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
>>>> which are all device-tree boot only. This patch adds the
>>>> base support for parsing the DT nodes, and removes the code
>>>> dealing with the traditional platform device instantiation.
>>>>
>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>> ---
>>>> .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++++++++++
>>>> arch/arm/mach-omap2/Makefile                       |  3 --
>>>> arch/arm/mach-omap2/hwspinlock.c                   | 60 ----------------------
>>>> drivers/hwspinlock/omap_hwspinlock.c               | 21 ++++++--
>>>> 4 files changed, 45 insertions(+), 67 deletions(-)
>>>> create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>> delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>> new file mode 100644
>>>> index 0000000..adfb8ad
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>> @@ -0,0 +1,28 @@
>>>> +OMAP4+ HwSpinlock Driver
>>>> +
>>>> +Required properties:
>>>> +- compatible:		Currently supports only "ti,omap4-hwspinlock" for
>>>> +				OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
>>>> +- reg:			Contains the hwspinlock register address range (base
>>>> +			address and length)
>>>> +- ti,hwmods:		Name of the hwmod associated with the hwspinlock device
>>>> +
>>>> +Optional properties:
>>>> +- base_id:		Base Id for the locks for a particular hwspinlock
>>>> +			device. If not mentioned, a default value of 0 is used.
>>>> +			This property is mandatory ONLY if a SoC has several
>>>> +			hwspinlock devices. There are currently no such OMAP
>>>> +			SoCs.
>>>
>>> Should this be ti,base_id ? [ I know its kinda generic in its intent for any SoC w/multiple blocks ]
>>
>> I didn't add the "ti," prefix exactly for the same reason - it is
>> generic w.r.t the hwspinlock core irrespective of the SoC family, and
>> there is nothing ti or OMAP specific about it. I have added it to keep
>> the DT node definition in sync with the driver code. If it is too
>> generic a name, it can always be renamed as hwlock_base_id. This will be
>> SoC agnostic property for the hwspinlock driver. What do you think?
> 
> I'm wondering if we should use cell-index for this purpose.

I didn't get you completely. Do you intend to compute the base_id using
cell-index and number of locks (which may be a separate field altogether
if this information cannot be read from the h/w)? My understanding is
that cell-index is primarily used for identifying the h/w instance number.

regards
Suman

>>
>>>
>>>> +
>>>> +			See documentation on struct hwspinlock_pdata in
>>>> +			linux/hwspinlock.h for more details.
>>>> +
>>>> +
>>>> +Example:
>>>> +
>>>> +/* OMAP4 */
>>>> +hwspinlock: spinlock@4a0f6000 {
>>>> +	compatible = "ti,omap4-hwspinlock";
>>>> +	reg = <0x4a0f6000 0x1000>;
>>>> +	ti,hwmods = "spinlock";
>>>> +};
>>>
>>> [ snip ]
>>>
>>> - k
>>>
>>
> 

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Stanimir Varbanov]

Hi Suman,

Thanks for the patch.

On 09/03/2013 08:52 PM, Suman Anna wrote:
> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
> which are all device-tree boot only. This patch adds the
> base support for parsing the DT nodes, and removes the code
> dealing with the traditional platform device instantiation.
> 
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
>  .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++++++++++
>  arch/arm/mach-omap2/Makefile                       |  3 --
>  arch/arm/mach-omap2/hwspinlock.c                   | 60 ----------------------
>  drivers/hwspinlock/omap_hwspinlock.c               | 21 ++++++--
>  4 files changed, 45 insertions(+), 67 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>  delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
> 
> diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
> new file mode 100644
> index 0000000..adfb8ad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
> @@ -0,0 +1,28 @@
> +OMAP4+ HwSpinlock Driver
> +
> +Required properties:
> +- compatible:		Currently supports only "ti,omap4-hwspinlock" for
> +				OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
> +- reg:			Contains the hwspinlock register address range (base
> +			address and length)
> +- ti,hwmods:		Name of the hwmod associated with the hwspinlock device
> +
> +Optional properties:
> +- base_id:		Base Id for the locks for a particular hwspinlock

Could you rename base_id to base-id. The property name convention is to
use dashes in DT world.

regards,
Stan

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Kumar Gala]

On Sep 3, 2013, at 5:34 PM, Suman Anna wrote:

> Kumar,
> 
>>> 
>>> On 09/03/2013 01:50 PM, Kumar Gala wrote:
>>>> 
>>>> On Sep 3, 2013, at 12:52 PM, Suman Anna wrote:
>>>> 
>>>>> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
>>>>> which are all device-tree boot only. This patch adds the
>>>>> base support for parsing the DT nodes, and removes the code
>>>>> dealing with the traditional platform device instantiation.
>>>>> 
>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>>> ---
>>>>> .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++++++++++
>>>>> arch/arm/mach-omap2/Makefile                       |  3 --
>>>>> arch/arm/mach-omap2/hwspinlock.c                   | 60 ----------------------
>>>>> drivers/hwspinlock/omap_hwspinlock.c               | 21 ++++++--
>>>>> 4 files changed, 45 insertions(+), 67 deletions(-)
>>>>> create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>> delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
>>>>> 
>>>>> diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>> new file mode 100644
>>>>> index 0000000..adfb8ad
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>> @@ -0,0 +1,28 @@
>>>>> +OMAP4+ HwSpinlock Driver
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible:		Currently supports only "ti,omap4-hwspinlock" for
>>>>> +				OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
>>>>> +- reg:			Contains the hwspinlock register address range (base
>>>>> +			address and length)
>>>>> +- ti,hwmods:		Name of the hwmod associated with the hwspinlock device
>>>>> +
>>>>> +Optional properties:
>>>>> +- base_id:		Base Id for the locks for a particular hwspinlock
>>>>> +			device. If not mentioned, a default value of 0 is used.
>>>>> +			This property is mandatory ONLY if a SoC has several
>>>>> +			hwspinlock devices. There are currently no such OMAP
>>>>> +			SoCs.
>>>> 
>>>> Should this be ti,base_id ? [ I know its kinda generic in its intent for any SoC w/multiple blocks ]
>>> 
>>> I didn't add the "ti," prefix exactly for the same reason - it is
>>> generic w.r.t the hwspinlock core irrespective of the SoC family, and
>>> there is nothing ti or OMAP specific about it. I have added it to keep
>>> the DT node definition in sync with the driver code. If it is too
>>> generic a name, it can always be renamed as hwlock_base_id. This will be
>>> SoC agnostic property for the hwspinlock driver. What do you think?
>> 
>> I'm wondering if we should use cell-index for this purpose.
> 
> I didn't get you completely. Do you intend to compute the base_id using
> cell-index and number of locks (which may be a separate field altogether
> if this information cannot be read from the h/w)? My understanding is
> that cell-index is primarily used for identifying the h/w instance number.

I was suggesting using cell-index instead of base_id.  What we should probably due is have a devicetree/bindings/hwlock/hwlock.txt that would describe generic properties like this and just reference that in the omap binding spec.

I'm thinking if we dont use cell-index, that it should probably be hwlock-base-id

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Suman Anna]

Kumar,

>>>>>
>>>>>> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
>>>>>> which are all device-tree boot only. This patch adds the
>>>>>> base support for parsing the DT nodes, and removes the code
>>>>>> dealing with the traditional platform device instantiation.
>>>>>>
>>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>>>> ---
>>>>>> .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++++++++++
>>>>>> arch/arm/mach-omap2/Makefile                       |  3 --
>>>>>> arch/arm/mach-omap2/hwspinlock.c                   | 60 ----------------------
>>>>>> drivers/hwspinlock/omap_hwspinlock.c               | 21 ++++++--
>>>>>> 4 files changed, 45 insertions(+), 67 deletions(-)
>>>>>> create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>> delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>> new file mode 100644
>>>>>> index 0000000..adfb8ad
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>> @@ -0,0 +1,28 @@
>>>>>> +OMAP4+ HwSpinlock Driver
>>>>>> +
>>>>>> +Required properties:
>>>>>> +- compatible:		Currently supports only "ti,omap4-hwspinlock" for
>>>>>> +				OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
>>>>>> +- reg:			Contains the hwspinlock register address range (base
>>>>>> +			address and length)
>>>>>> +- ti,hwmods:		Name of the hwmod associated with the hwspinlock device
>>>>>> +
>>>>>> +Optional properties:
>>>>>> +- base_id:		Base Id for the locks for a particular hwspinlock
>>>>>> +			device. If not mentioned, a default value of 0 is used.
>>>>>> +			This property is mandatory ONLY if a SoC has several
>>>>>> +			hwspinlock devices. There are currently no such OMAP
>>>>>> +			SoCs.
>>>>>
>>>>> Should this be ti,base_id ? [ I know its kinda generic in its intent for any SoC w/multiple blocks ]
>>>>
>>>> I didn't add the "ti," prefix exactly for the same reason - it is
>>>> generic w.r.t the hwspinlock core irrespective of the SoC family, and
>>>> there is nothing ti or OMAP specific about it. I have added it to keep
>>>> the DT node definition in sync with the driver code. If it is too
>>>> generic a name, it can always be renamed as hwlock_base_id. This will be
>>>> SoC agnostic property for the hwspinlock driver. What do you think?
>>>
>>> I'm wondering if we should use cell-index for this purpose.
>>
>> I didn't get you completely. Do you intend to compute the base_id using
>> cell-index and number of locks (which may be a separate field altogether
>> if this information cannot be read from the h/w)? My understanding is
>> that cell-index is primarily used for identifying the h/w instance number.
> 
> I was suggesting using cell-index instead of base_id.  What we should probably due is have a devicetree/bindings/hwlock/hwlock.txt that would describe generic properties like this and just reference that in the omap binding spec.

Common hwlock.txt sounds good. Will make the change.

> 
> I'm thinking if we dont use cell-index, that it should probably be hwlock-base-id
> 

I prefer to use hwlock-base-id. I think we should also be defining a
common property name for number of locks, say hwlock-num-locks.

regards
Suman

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Suman Anna]

On 09/04/2013 03:09 AM, Stanimir Varbanov wrote:
> Hi Suman,
> 
> Thanks for the patch.
> 
> On 09/03/2013 08:52 PM, Suman Anna wrote:
>> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
>> which are all device-tree boot only. This patch adds the
>> base support for parsing the DT nodes, and removes the code
>> dealing with the traditional platform device instantiation.
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> ---
>>  .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++++++++++
>>  arch/arm/mach-omap2/Makefile                       |  3 --
>>  arch/arm/mach-omap2/hwspinlock.c                   | 60 ----------------------
>>  drivers/hwspinlock/omap_hwspinlock.c               | 21 ++++++--
>>  4 files changed, 45 insertions(+), 67 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>  delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
>>
>> diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>> new file mode 100644
>> index 0000000..adfb8ad
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>> @@ -0,0 +1,28 @@
>> +OMAP4+ HwSpinlock Driver
>> +
>> +Required properties:
>> +- compatible:		Currently supports only "ti,omap4-hwspinlock" for
>> +				OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
>> +- reg:			Contains the hwspinlock register address range (base
>> +			address and length)
>> +- ti,hwmods:		Name of the hwmod associated with the hwspinlock device
>> +
>> +Optional properties:
>> +- base_id:		Base Id for the locks for a particular hwspinlock
> 
> Could you rename base_id to base-id. The property name convention is to
> use dashes in DT world.

Yep, will change to use dashes.

Thanks
Suman

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Kumar Gala]

On Sep 4, 2013, at 11:27 AM, Suman Anna wrote:

> Kumar,
> 
>>>>>> 
>>>>>>> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
>>>>>>> which are all device-tree boot only. This patch adds the
>>>>>>> base support for parsing the DT nodes, and removes the code
>>>>>>> dealing with the traditional platform device instantiation.
>>>>>>> 
>>>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>>>>> ---
>>>>>>> .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++++++++++
>>>>>>> arch/arm/mach-omap2/Makefile                       |  3 --
>>>>>>> arch/arm/mach-omap2/hwspinlock.c                   | 60 ----------------------
>>>>>>> drivers/hwspinlock/omap_hwspinlock.c               | 21 ++++++--
>>>>>>> 4 files changed, 45 insertions(+), 67 deletions(-)
>>>>>>> create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>> delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
>>>>>>> 
>>>>>>> diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>> new file mode 100644
>>>>>>> index 0000000..adfb8ad
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>> @@ -0,0 +1,28 @@
>>>>>>> +OMAP4+ HwSpinlock Driver
>>>>>>> +
>>>>>>> +Required properties:
>>>>>>> +- compatible:		Currently supports only "ti,omap4-hwspinlock" for
>>>>>>> +				OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
>>>>>>> +- reg:			Contains the hwspinlock register address range (base
>>>>>>> +			address and length)
>>>>>>> +- ti,hwmods:		Name of the hwmod associated with the hwspinlock device
>>>>>>> +
>>>>>>> +Optional properties:
>>>>>>> +- base_id:		Base Id for the locks for a particular hwspinlock
>>>>>>> +			device. If not mentioned, a default value of 0 is used.
>>>>>>> +			This property is mandatory ONLY if a SoC has several
>>>>>>> +			hwspinlock devices. There are currently no such OMAP
>>>>>>> +			SoCs.
>>>>>> 
>>>>>> Should this be ti,base_id ? [ I know its kinda generic in its intent for any SoC w/multiple blocks ]
>>>>> 
>>>>> I didn't add the "ti," prefix exactly for the same reason - it is
>>>>> generic w.r.t the hwspinlock core irrespective of the SoC family, and
>>>>> there is nothing ti or OMAP specific about it. I have added it to keep
>>>>> the DT node definition in sync with the driver code. If it is too
>>>>> generic a name, it can always be renamed as hwlock_base_id. This will be
>>>>> SoC agnostic property for the hwspinlock driver. What do you think?
>>>> 
>>>> I'm wondering if we should use cell-index for this purpose.
>>> 
>>> I didn't get you completely. Do you intend to compute the base_id using
>>> cell-index and number of locks (which may be a separate field altogether
>>> if this information cannot be read from the h/w)? My understanding is
>>> that cell-index is primarily used for identifying the h/w instance number.
>> 
>> I was suggesting using cell-index instead of base_id.  What we should probably due is have a devicetree/bindings/hwlock/hwlock.txt that would describe generic properties like this and just reference that in the omap binding spec.
> 
> Common hwlock.txt sounds good. Will make the change.
> 
>> 
>> I'm thinking if we dont use cell-index, that it should probably be hwlock-base-id
>> 
> 
> I prefer to use hwlock-base-id. I think we should also be defining a
> common property name for number of locks, say hwlock-num-locks.

I'm good with that, cell-index is always funny so might as well be explicit.

I'm also good with hwlock-num-locks, I'll update the msm spinlock driver to use this.

Can you also maybe add some helper functions into the hwspinlock core to return these values so we both don't duplicate code in drivers and maintain consistency.

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Suman Anna]

>>>>>>>
>>>>>>>> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
>>>>>>>> which are all device-tree boot only. This patch adds the
>>>>>>>> base support for parsing the DT nodes, and removes the code
>>>>>>>> dealing with the traditional platform device instantiation.
>>>>>>>>
>>>>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>>>>>> ---
>>>>>>>> .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++++++++++
>>>>>>>> arch/arm/mach-omap2/Makefile                       |  3 --
>>>>>>>> arch/arm/mach-omap2/hwspinlock.c                   | 60 ----------------------
>>>>>>>> drivers/hwspinlock/omap_hwspinlock.c               | 21 ++++++--
>>>>>>>> 4 files changed, 45 insertions(+), 67 deletions(-)
>>>>>>>> create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>>> delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>>> new file mode 100644
>>>>>>>> index 0000000..adfb8ad
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>>> @@ -0,0 +1,28 @@
>>>>>>>> +OMAP4+ HwSpinlock Driver
>>>>>>>> +
>>>>>>>> +Required properties:
>>>>>>>> +- compatible:		Currently supports only "ti,omap4-hwspinlock" for
>>>>>>>> +				OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
>>>>>>>> +- reg:			Contains the hwspinlock register address range (base
>>>>>>>> +			address and length)
>>>>>>>> +- ti,hwmods:		Name of the hwmod associated with the hwspinlock device
>>>>>>>> +
>>>>>>>> +Optional properties:
>>>>>>>> +- base_id:		Base Id for the locks for a particular hwspinlock
>>>>>>>> +			device. If not mentioned, a default value of 0 is used.
>>>>>>>> +			This property is mandatory ONLY if a SoC has several
>>>>>>>> +			hwspinlock devices. There are currently no such OMAP
>>>>>>>> +			SoCs.
>>>>>>>
>>>>>>> Should this be ti,base_id ? [ I know its kinda generic in its intent for any SoC w/multiple blocks ]
>>>>>>
>>>>>> I didn't add the "ti," prefix exactly for the same reason - it is
>>>>>> generic w.r.t the hwspinlock core irrespective of the SoC family, and
>>>>>> there is nothing ti or OMAP specific about it. I have added it to keep
>>>>>> the DT node definition in sync with the driver code. If it is too
>>>>>> generic a name, it can always be renamed as hwlock_base_id. This will be
>>>>>> SoC agnostic property for the hwspinlock driver. What do you think?
>>>>>
>>>>> I'm wondering if we should use cell-index for this purpose.
>>>>
>>>> I didn't get you completely. Do you intend to compute the base_id using
>>>> cell-index and number of locks (which may be a separate field altogether
>>>> if this information cannot be read from the h/w)? My understanding is
>>>> that cell-index is primarily used for identifying the h/w instance number.
>>>
>>> I was suggesting using cell-index instead of base_id.  What we should probably due is have a devicetree/bindings/hwlock/hwlock.txt that would describe generic properties like this and just reference that in the omap binding spec.
>>
>> Common hwlock.txt sounds good. Will make the change.
>>
>>>
>>> I'm thinking if we dont use cell-index, that it should probably be hwlock-base-id
>>>
>>
>> I prefer to use hwlock-base-id. I think we should also be defining a
>> common property name for number of locks, say hwlock-num-locks.
> 
> I'm good with that, cell-index is always funny so might as well be explicit.
> 
> I'm also good with hwlock-num-locks, I'll update the msm spinlock driver to use this.
> 
> Can you also maybe add some helper functions into the hwspinlock core to return these values so we both don't duplicate code in drivers and maintain consistency.

I am trying to understand what you would need these for. Your driver
would already know the base_id and num_locks, since these are used in
the registration function.

regards
Suman

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Kumar Gala]

On Sep 4, 2013, at 12:03 PM, Suman Anna wrote:

>>>>>>>> 
>>>>>>>>> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
>>>>>>>>> which are all device-tree boot only. This patch adds the
>>>>>>>>> base support for parsing the DT nodes, and removes the code
>>>>>>>>> dealing with the traditional platform device instantiation.
>>>>>>>>> 
>>>>>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>>>>>>> ---
>>>>>>>>> .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++++++++++
>>>>>>>>> arch/arm/mach-omap2/Makefile                       |  3 --
>>>>>>>>> arch/arm/mach-omap2/hwspinlock.c                   | 60 ----------------------
>>>>>>>>> drivers/hwspinlock/omap_hwspinlock.c               | 21 ++++++--
>>>>>>>>> 4 files changed, 45 insertions(+), 67 deletions(-)
>>>>>>>>> create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>>>> delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
>>>>>>>>> 
>>>>>>>>> diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>>>> new file mode 100644
>>>>>>>>> index 0000000..adfb8ad
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>>>> @@ -0,0 +1,28 @@
>>>>>>>>> +OMAP4+ HwSpinlock Driver
>>>>>>>>> +
>>>>>>>>> +Required properties:
>>>>>>>>> +- compatible:		Currently supports only "ti,omap4-hwspinlock" for
>>>>>>>>> +				OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
>>>>>>>>> +- reg:			Contains the hwspinlock register address range (base
>>>>>>>>> +			address and length)
>>>>>>>>> +- ti,hwmods:		Name of the hwmod associated with the hwspinlock device
>>>>>>>>> +
>>>>>>>>> +Optional properties:
>>>>>>>>> +- base_id:		Base Id for the locks for a particular hwspinlock
>>>>>>>>> +			device. If not mentioned, a default value of 0 is used.
>>>>>>>>> +			This property is mandatory ONLY if a SoC has several
>>>>>>>>> +			hwspinlock devices. There are currently no such OMAP
>>>>>>>>> +			SoCs.
>>>>>>>> 
>>>>>>>> Should this be ti,base_id ? [ I know its kinda generic in its intent for any SoC w/multiple blocks ]
>>>>>>> 
>>>>>>> I didn't add the "ti," prefix exactly for the same reason - it is
>>>>>>> generic w.r.t the hwspinlock core irrespective of the SoC family, and
>>>>>>> there is nothing ti or OMAP specific about it. I have added it to keep
>>>>>>> the DT node definition in sync with the driver code. If it is too
>>>>>>> generic a name, it can always be renamed as hwlock_base_id. This will be
>>>>>>> SoC agnostic property for the hwspinlock driver. What do you think?
>>>>>> 
>>>>>> I'm wondering if we should use cell-index for this purpose.
>>>>> 
>>>>> I didn't get you completely. Do you intend to compute the base_id using
>>>>> cell-index and number of locks (which may be a separate field altogether
>>>>> if this information cannot be read from the h/w)? My understanding is
>>>>> that cell-index is primarily used for identifying the h/w instance number.
>>>> 
>>>> I was suggesting using cell-index instead of base_id.  What we should probably due is have a devicetree/bindings/hwlock/hwlock.txt that would describe generic properties like this and just reference that in the omap binding spec.
>>> 
>>> Common hwlock.txt sounds good. Will make the change.
>>> 
>>>> 
>>>> I'm thinking if we dont use cell-index, that it should probably be hwlock-base-id
>>>> 
>>> 
>>> I prefer to use hwlock-base-id. I think we should also be defining a
>>> common property name for number of locks, say hwlock-num-locks.
>> 
>> I'm good with that, cell-index is always funny so might as well be explicit.
>> 
>> I'm also good with hwlock-num-locks, I'll update the msm spinlock driver to use this.
>> 
>> Can you also maybe add some helper functions into the hwspinlock core to return these values so we both don't duplicate code in drivers and maintain consistency.
> 
> I am trying to understand what you would need these for. Your driver
> would already know the base_id and num_locks, since these are used in
> the registration function.


It would be something simple like:

static inline int of_get_hwlock_base_id (struct device_node *dn) {
#ifdef CONFIG_OF
	u32 val;

	if (of_property_read_u32(dn, "hwlock-base-id", &val)
		return val;
#endif
	return 0;
}

make sense?

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Suman Anna]

On 09/04/2013 12:51 PM, Kumar Gala wrote:
> 
> On Sep 4, 2013, at 12:03 PM, Suman Anna wrote:
> 
>>>>>>>>>
>>>>>>>>>> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
>>>>>>>>>> which are all device-tree boot only. This patch adds the
>>>>>>>>>> base support for parsing the DT nodes, and removes the code
>>>>>>>>>> dealing with the traditional platform device instantiation.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>>>>>>>> ---
>>>>>>>>>> .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++++++++++
>>>>>>>>>> arch/arm/mach-omap2/Makefile                       |  3 --
>>>>>>>>>> arch/arm/mach-omap2/hwspinlock.c                   | 60 ----------------------
>>>>>>>>>> drivers/hwspinlock/omap_hwspinlock.c               | 21 ++++++--
>>>>>>>>>> 4 files changed, 45 insertions(+), 67 deletions(-)
>>>>>>>>>> create mode 100644 Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>>>>> delete mode 100644 arch/arm/mach-omap2/hwspinlock.c
>>>>>>>>>>
>>>>>>>>>> diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>>>>> new file mode 100644
>>>>>>>>>> index 0000000..adfb8ad
>>>>>>>>>> --- /dev/null
>>>>>>>>>> +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt
>>>>>>>>>> @@ -0,0 +1,28 @@
>>>>>>>>>> +OMAP4+ HwSpinlock Driver
>>>>>>>>>> +
>>>>>>>>>> +Required properties:
>>>>>>>>>> +- compatible:		Currently supports only "ti,omap4-hwspinlock" for
>>>>>>>>>> +				OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs
>>>>>>>>>> +- reg:			Contains the hwspinlock register address range (base
>>>>>>>>>> +			address and length)
>>>>>>>>>> +- ti,hwmods:		Name of the hwmod associated with the hwspinlock device
>>>>>>>>>> +
>>>>>>>>>> +Optional properties:
>>>>>>>>>> +- base_id:		Base Id for the locks for a particular hwspinlock
>>>>>>>>>> +			device. If not mentioned, a default value of 0 is used.
>>>>>>>>>> +			This property is mandatory ONLY if a SoC has several
>>>>>>>>>> +			hwspinlock devices. There are currently no such OMAP
>>>>>>>>>> +			SoCs.
>>>>>>>>>
>>>>>>>>> Should this be ti,base_id ? [ I know its kinda generic in its intent for any SoC w/multiple blocks ]
>>>>>>>>
>>>>>>>> I didn't add the "ti," prefix exactly for the same reason - it is
>>>>>>>> generic w.r.t the hwspinlock core irrespective of the SoC family, and
>>>>>>>> there is nothing ti or OMAP specific about it. I have added it to keep
>>>>>>>> the DT node definition in sync with the driver code. If it is too
>>>>>>>> generic a name, it can always be renamed as hwlock_base_id. This will be
>>>>>>>> SoC agnostic property for the hwspinlock driver. What do you think?
>>>>>>>
>>>>>>> I'm wondering if we should use cell-index for this purpose.
>>>>>>
>>>>>> I didn't get you completely. Do you intend to compute the base_id using
>>>>>> cell-index and number of locks (which may be a separate field altogether
>>>>>> if this information cannot be read from the h/w)? My understanding is
>>>>>> that cell-index is primarily used for identifying the h/w instance number.
>>>>>
>>>>> I was suggesting using cell-index instead of base_id.  What we should probably due is have a devicetree/bindings/hwlock/hwlock.txt that would describe generic properties like this and just reference that in the omap binding spec.
>>>>
>>>> Common hwlock.txt sounds good. Will make the change.
>>>>
>>>>>
>>>>> I'm thinking if we dont use cell-index, that it should probably be hwlock-base-id
>>>>>
>>>>
>>>> I prefer to use hwlock-base-id. I think we should also be defining a
>>>> common property name for number of locks, say hwlock-num-locks.
>>>
>>> I'm good with that, cell-index is always funny so might as well be explicit.
>>>
>>> I'm also good with hwlock-num-locks, I'll update the msm spinlock driver to use this.
>>>
>>> Can you also maybe add some helper functions into the hwspinlock core to return these values so we both don't duplicate code in drivers and maintain consistency.
>>
>> I am trying to understand what you would need these for. Your driver
>> would already know the base_id and num_locks, since these are used in
>> the registration function.
> 
> 
> It would be something simple like:
> 
> static inline int of_get_hwlock_base_id (struct device_node *dn) {
> #ifdef CONFIG_OF
> 	u32 val;
> 
> 	if (of_property_read_u32(dn, "hwlock-base-id", &val)
> 		return val;
> #endif
> 	return 0;
> }
> 

Sorry I misinterpreted, thought you were asking for accessor functions
after, and not of_ helpers. Yes, will add these as well.

regards
Suman

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Tony Lindgren]

* Suman Anna <s-anna@ti.com> [130903 11:00]:
> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
> which are all device-tree boot only. This patch adds the
> base support for parsing the DT nodes, and removes the code
> dealing with the traditional platform device instantiation.

Great, this should be safe for Ohad to queue:

Acked-by: Tony Lindgren <tony@atomide.com>

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Suman Anna]

Tony,

On 10/03/2013 01:05 PM, Tony Lindgren wrote:
> * Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [130903 11:00]:
>> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
>> which are all device-tree boot only. This patch adds the
>> base support for parsing the DT nodes, and removes the code
>> dealing with the traditional platform device instantiation.
> 
> Great, this should be safe for Ohad to queue:
> 
> Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> 

There is a v2 of this series [1] that adds additional SoC support as
well. It currently has some comments [2] & [3] from Mark Rutland on the
DT bindings. I am looking into that at the moment and it may change this
patch as well (the DT parse portion of it in
drivers/hwspinlock/omap_hwspinlock).

regards
Suman

[1] http://marc.info/?l=linux-omap&m=137944644112727&w=2
[2] http://marc.info/?t=137944636400005&r=1&w=2
[3] http://marc.info/?t=137944636400004&r=1&w=2


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes

[Tony Lindgren]

* Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [131003 11:25]:
> Tony,
> 
> On 10/03/2013 01:05 PM, Tony Lindgren wrote:
> > * Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [130903 11:00]:
> >> HwSpinlock IP is present only on OMAP4 and other newer SoCs,
> >> which are all device-tree boot only. This patch adds the
> >> base support for parsing the DT nodes, and removes the code
> >> dealing with the traditional platform device instantiation.
> > 
> > Great, this should be safe for Ohad to queue:
> > 
> > Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> > 
> 
> There is a v2 of this series [1] that adds additional SoC support as
> well. It currently has some comments [2] & [3] from Mark Rutland on the
> DT bindings. I am looking into that at the moment and it may change this
> patch as well (the DT parse portion of it in
> drivers/hwspinlock/omap_hwspinlock).

OK, yeah I noticed there's some pending comments. Feel free to keep my
ack for the arch/arm/mach-omap2 removal parts, those won't likely
change.

Regards,

Tony
 
> [1] http://marc.info/?l=linux-omap&m=137944644112727&w=2
> [2] http://marc.info/?t=137944636400005&r=1&w=2
> [3] http://marc.info/?t=137944636400004&r=1&w=2
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html