Re: [PATCHv2 1/9] hwspinlock/core: add common dt bindings and OF helpers
On Thu, Oct 03, 2013 at 05:04:15AM +0100, Suman Anna wrote: Hi Mark, On 10/01/2013 03:36 AM, Mark Rutland wrote: Hi Suman, Apologies for replying to a subthread, due to an earlier mistake on my part I don't have the original to hand. No issues, thanks for your review. On Fri, Sep 27, 2013 at 05:04:22PM +0100, Kumar Gala wrote: On Sep 17, 2013, at 2:30 PM, Suman Anna wrote: All the platform-specific hwlock driver implementations need the number of locks and the associated base id for registering the locks present within a hwspinlock device with the driver core. These two variables are represented by 'hwlock-num-locks' and 'hwlock-base-id' properties. The documentation and OF helpers to retrieve these common properties have been added to the driver core. Signed-off-by: Suman Anna s-a...@ti.com --- .../devicetree/bindings/hwlock/hwlock.txt | 26 + drivers/hwspinlock/hwspinlock_core.c | 61 +- include/linux/hwspinlock.h | 11 ++-- 3 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt new file mode 100644 index 000..789930e --- /dev/null +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt @@ -0,0 +1,26 @@ +Generic hwlock bindings +=== + +Generic bindings that are common to all the hwlock platform specific driver +implementations, the retrieved values are used for registering the device +specific parameters with the hwspinlock core. + +The validity and need of these common properties may vary from one driver +implementation to another. Look through the individual hwlock driver +binding documentations for identifying which are mandatory and which are +optional for that specific driver. + +Common properties: +- hwlock-base-id:Base Id for the locks for a particular hwlock device. + This property is used for representing a set of locks + present in a hwlock device with a unique base id in + the driver core. This property is mandatory ONLY if a + SoC has several hwlock devices. + + See documentation on struct hwspinlock_pdata in + linux/hwspinlock.h for more details. I don't like this, as it seems to be encoding a Linux implementation detail (the ID of the lock, which means that we have to have a numeric representation of each hwlock) in the device tree. I don't see why the ID within Linux should be a concern of the device tree binding. I think that whatever internal identifier we have in Linux (be it an integer or struct) should be allocated by Linux. If a driver needs to request specific hwlocks, then we should have a phandle + args representation for referring to a specific hwlock that bidnings can use, and some code for parsing that and returning a Linux-internal identifier/struct as we do with interrupts and clocks. This is based on gathering the information required by the platform implementation drivers for registering with the driver core. The driver core currently maintains all the locks from different instances as a radix tree, as it is simpler to manage when granting locks to users. The current version is based on allowing the platform implementation drivers to retrieve the required data for registering with the hwspinlock driver core. Ok. I don't see why this implementation detail needs to leak into the dt. The users would either get a lock dynamically by requesting any free one (and extract the id thereafter to share with others), or a specific one which is currently by id. I agree that the phandle + args approach is best suited for requesting a specific one through DT, but I would think that the args would still have to be a relative lock number from 0 w.r.t a phandle. I will look into the feasibility of such an approach retaining the radix tree implementation, as this requires new OF friendly registration and request functions. The value in the args would be a unique identifier within the unit pointed to be the phandle, but the mechanism by which it would refer to a particular lock would be binding-specific. It's perfectly fine for this to be an zero-based index in most bindings, but it should not be a global namespace as with the hwlock-base-id property approach. + +- hwlock-num-locks: Number of locks present in a hwlock device. This + property is needed on hwlock devices, where the number + of supported locks within a hwlock device cannot be + read from a register. Hmm... this seems generic, but it doesn't allow for sparse ID spaces on the hwlock module. It should
Re: [PATCHv2 1/9] hwspinlock/core: add common dt bindings and OF helpers
Hi Mark, On 10/01/2013 03:36 AM, Mark Rutland wrote: Hi Suman, Apologies for replying to a subthread, due to an earlier mistake on my part I don't have the original to hand. No issues, thanks for your review. On Fri, Sep 27, 2013 at 05:04:22PM +0100, Kumar Gala wrote: On Sep 17, 2013, at 2:30 PM, Suman Anna wrote: All the platform-specific hwlock driver implementations need the number of locks and the associated base id for registering the locks present within a hwspinlock device with the driver core. These two variables are represented by 'hwlock-num-locks' and 'hwlock-base-id' properties. The documentation and OF helpers to retrieve these common properties have been added to the driver core. Signed-off-by: Suman Anna s-a...@ti.com --- .../devicetree/bindings/hwlock/hwlock.txt | 26 + drivers/hwspinlock/hwspinlock_core.c | 61 +- include/linux/hwspinlock.h | 11 ++-- 3 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt new file mode 100644 index 000..789930e --- /dev/null +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt @@ -0,0 +1,26 @@ +Generic hwlock bindings +=== + +Generic bindings that are common to all the hwlock platform specific driver +implementations, the retrieved values are used for registering the device +specific parameters with the hwspinlock core. + +The validity and need of these common properties may vary from one driver +implementation to another. Look through the individual hwlock driver +binding documentations for identifying which are mandatory and which are +optional for that specific driver. + +Common properties: +- hwlock-base-id: Base Id for the locks for a particular hwlock device. + This property is used for representing a set of locks + present in a hwlock device with a unique base id in + the driver core. This property is mandatory ONLY if a + SoC has several hwlock devices. + + See documentation on struct hwspinlock_pdata in + linux/hwspinlock.h for more details. I don't like this, as it seems to be encoding a Linux implementation detail (the ID of the lock, which means that we have to have a numeric representation of each hwlock) in the device tree. I don't see why the ID within Linux should be a concern of the device tree binding. I think that whatever internal identifier we have in Linux (be it an integer or struct) should be allocated by Linux. If a driver needs to request specific hwlocks, then we should have a phandle + args representation for referring to a specific hwlock that bidnings can use, and some code for parsing that and returning a Linux-internal identifier/struct as we do with interrupts and clocks. This is based on gathering the information required by the platform implementation drivers for registering with the driver core. The driver core currently maintains all the locks from different instances as a radix tree, as it is simpler to manage when granting locks to users. The current version is based on allowing the platform implementation drivers to retrieve the required data for registering with the hwspinlock driver core. The users would either get a lock dynamically by requesting any free one (and extract the id thereafter to share with others), or a specific one which is currently by id. I agree that the phandle + args approach is best suited for requesting a specific one through DT, but I would think that the args would still have to be a relative lock number from 0 w.r.t a phandle. I will look into the feasibility of such an approach retaining the radix tree implementation, as this requires new OF friendly registration and request functions. + +- hwlock-num-locks:Number of locks present in a hwlock device. This + property is needed on hwlock devices, where the number + of supported locks within a hwlock device cannot be + read from a register. Hmm... this seems generic, but it doesn't allow for sparse ID spaces on the hwlock module. It should probably be common for the moment, but if we encounter a hwlock module with a sparse ID space, we'll need to come up with a way of handling sparse IDs (that might be device-specific). Agreed. diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c index 461a0d7..aec32e7 100644 --- a/drivers/hwspinlock/hwspinlock_core.c +++ b/drivers/hwspinlock/hwspinlock_core.c @@ -1,7 +1,7 @@ /* * Hardware spinlock framework * - * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com + * Copyright (C) 2010-2013 Texas
Re: [PATCHv2 1/9] hwspinlock/core: add common dt bindings and OF helpers
Hi Suman,
Apologies for replying to a subthread, due to an earlier mistake on my
part I don't have the original to hand.
On Fri, Sep 27, 2013 at 05:04:22PM +0100, Kumar Gala wrote:
On Sep 17, 2013, at 2:30 PM, Suman Anna wrote:
All the platform-specific hwlock driver implementations need the
number of locks and the associated base id for registering the
locks present within a hwspinlock device with the driver core.
These two variables are represented by 'hwlock-num-locks' and
'hwlock-base-id' properties. The documentation and OF helpers to
retrieve these common properties have been added to the driver core.
Signed-off-by: Suman Anna s-a...@ti.com
---
.../devicetree/bindings/hwlock/hwlock.txt | 26 +
drivers/hwspinlock/hwspinlock_core.c | 61
+-
include/linux/hwspinlock.h | 11 ++--
3 files changed, 93 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt
diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt
b/Documentation/devicetree/bindings/hwlock/hwlock.txt
new file mode 100644
index 000..789930e
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt
@@ -0,0 +1,26 @@
+Generic hwlock bindings
+===
+
+Generic bindings that are common to all the hwlock platform specific driver
+implementations, the retrieved values are used for registering the device
+specific parameters with the hwspinlock core.
+
+The validity and need of these common properties may vary from one driver
+implementation to another. Look through the individual hwlock driver
+binding documentations for identifying which are mandatory and which are
+optional for that specific driver.
+
+Common properties:
+- hwlock-base-id: Base Id for the locks for a particular hwlock device.
+ This property is used for representing a set of locks
+ present in a hwlock device with a unique base id in
+ the driver core. This property is mandatory ONLY if a
+ SoC has several hwlock devices.
+
+ See documentation on struct hwspinlock_pdata in
+ linux/hwspinlock.h for more details.
I don't like this, as it seems to be encoding a Linux implementation
detail (the ID of the lock, which means that we have to have a numeric
representation of each hwlock) in the device tree.
I don't see why the ID within Linux should be a concern of the device
tree binding. I think that whatever internal identifier we have in Linux
(be it an integer or struct) should be allocated by Linux. If a driver
needs to request specific hwlocks, then we should have a phandle + args
representation for referring to a specific hwlock that bidnings can use,
and some code for parsing that and returning a Linux-internal
identifier/struct as we do with interrupts and clocks.
+
+- hwlock-num-locks:Number of locks present in a hwlock device. This
+ property is needed on hwlock devices, where the number
+ of supported locks within a hwlock device cannot be
+ read from a register.
Hmm... this seems generic, but it doesn't allow for sparse ID spaces on
the hwlock module. It should probably be common for the moment, but if
we encounter a hwlock module with a sparse ID space, we'll need to come
up with a way of handling sparse IDs (that might be device-specific).
diff --git a/drivers/hwspinlock/hwspinlock_core.c
b/drivers/hwspinlock/hwspinlock_core.c
index 461a0d7..aec32e7 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -1,7 +1,7 @@
/*
* Hardware spinlock framework
*
- * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2010-2013 Texas Instruments Incorporated -
http://www.ti.com
*
* Contact: Ohad Ben-Cohen o...@wizery.com
*
@@ -27,6 +27,7 @@
#include linux/hwspinlock.h
#include linux/pm_runtime.h
#include linux/mutex.h
+#include linux/of.h
#include hwspinlock_internal.h
@@ -308,6 +309,64 @@ out:
}
/**
+ * of_hwspin_lock_get_base_id() - OF helper to retrieve base id
+ * @dn: device node pointer
+ *
+ * This is an OF helper function that can be called by the underlying
+ * platform-specific implementations, to retrieve the base id for the
+ * set of locks present within a hwspinlock device instance.
+ *
+ * Returns the base id value on success, -ENODEV on generic failure or
+ * an appropriate error code as returned by the OF layer
+ */
+int of_hwspin_lock_get_base_id(struct device_node *dn)
+{
+ unsigned int val;
+ int ret = -ENODEV;
+
+#ifdef CONFIG_OF
+ if (!dn)
+ return -ENODEV;
Checking !dn is done in of_property_read_u32, so you don't need to duplicate
Re: [PATCHv2 1/9] hwspinlock/core: add common dt bindings and OF helpers
On Sep 17, 2013, at 2:30 PM, Suman Anna wrote:
All the platform-specific hwlock driver implementations need the
number of locks and the associated base id for registering the
locks present within a hwspinlock device with the driver core.
These two variables are represented by 'hwlock-num-locks' and
'hwlock-base-id' properties. The documentation and OF helpers to
retrieve these common properties have been added to the driver core.
Signed-off-by: Suman Anna s-a...@ti.com
---
.../devicetree/bindings/hwlock/hwlock.txt | 26 +
drivers/hwspinlock/hwspinlock_core.c | 61 +-
include/linux/hwspinlock.h | 11 ++--
3 files changed, 93 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt
diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt
b/Documentation/devicetree/bindings/hwlock/hwlock.txt
new file mode 100644
index 000..789930e
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt
@@ -0,0 +1,26 @@
+Generic hwlock bindings
+===
+
+Generic bindings that are common to all the hwlock platform specific driver
+implementations, the retrieved values are used for registering the device
+specific parameters with the hwspinlock core.
+
+The validity and need of these common properties may vary from one driver
+implementation to another. Look through the individual hwlock driver
+binding documentations for identifying which are mandatory and which are
+optional for that specific driver.
+
+Common properties:
+- hwlock-base-id:Base Id for the locks for a particular hwlock device.
+ This property is used for representing a set of locks
+ present in a hwlock device with a unique base id in
+ the driver core. This property is mandatory ONLY if a
+ SoC has several hwlock devices.
+
+ See documentation on struct hwspinlock_pdata in
+ linux/hwspinlock.h for more details.
+
+- hwlock-num-locks: Number of locks present in a hwlock device. This
+ property is needed on hwlock devices, where the number
+ of supported locks within a hwlock device cannot be
+ read from a register.
diff --git a/drivers/hwspinlock/hwspinlock_core.c
b/drivers/hwspinlock/hwspinlock_core.c
index 461a0d7..aec32e7 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -1,7 +1,7 @@
/*
* Hardware spinlock framework
*
- * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2010-2013 Texas Instruments Incorporated - http://www.ti.com
*
* Contact: Ohad Ben-Cohen o...@wizery.com
*
@@ -27,6 +27,7 @@
#include linux/hwspinlock.h
#include linux/pm_runtime.h
#include linux/mutex.h
+#include linux/of.h
#include hwspinlock_internal.h
@@ -308,6 +309,64 @@ out:
}
/**
+ * of_hwspin_lock_get_base_id() - OF helper to retrieve base id
+ * @dn: device node pointer
+ *
+ * This is an OF helper function that can be called by the underlying
+ * platform-specific implementations, to retrieve the base id for the
+ * set of locks present within a hwspinlock device instance.
+ *
+ * Returns the base id value on success, -ENODEV on generic failure or
+ * an appropriate error code as returned by the OF layer
+ */
+int of_hwspin_lock_get_base_id(struct device_node *dn)
+{
+ unsigned int val;
+ int ret = -ENODEV;
+
+#ifdef CONFIG_OF
+ if (!dn)
+ return -ENODEV;
Checking !dn is done in of_property_read_u32, so you don't need to duplicate
+
+ ret = of_property_read_u32(dn, hwlock-base-id, val);
+ if (!ret)
+ ret = val;
+#endif
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(of_hwspin_lock_get_base_id);
+
+/**
+ * of_hwspin_lock_get_num_locks() - OF helper to retrieve number of locks
+ * @dn: device node pointer
+ *
+ * This is an OF helper function that can be called by the underlying
+ * platform-specific implementations, to retrieve the number of locks
+ * present within a hwspinlock device instance.
+ *
+ * Returns a positive number of locks on success, -ENODEV on generic
+ * failure or an appropriate error code as returned by the OF layer
+ */
+int of_hwspin_lock_get_num_locks(struct device_node *dn)
+{
+ unsigned int val;
+ int ret = -ENODEV;
+
+#ifdef CONFIG_OF
+ if (!dn)
+ return -ENODEV;
Checking !dn is done in of_property_read_u32, so you don't need to duplicate
+
+ ret = of_property_read_u32(dn, hwlock-num-locks, val);
+ if (!ret)
+ ret = val ? val : -ENODEV;
+#endif
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(of_hwspin_lock_get_num_locks);
+
+/**
* hwspin_lock_register() - register a new hw spinlock device
* @bank: the
Re: [PATCHv2 1/9] hwspinlock/core: add common dt bindings and OF helpers
Kumar,
On 09/27/2013 11:04 AM, Kumar Gala wrote:
On Sep 17, 2013, at 2:30 PM, Suman Anna wrote:
All the platform-specific hwlock driver implementations need the
number of locks and the associated base id for registering the
locks present within a hwspinlock device with the driver core.
These two variables are represented by 'hwlock-num-locks' and
'hwlock-base-id' properties. The documentation and OF helpers to
retrieve these common properties have been added to the driver core.
Signed-off-by: Suman Anna s-a...@ti.com
---
.../devicetree/bindings/hwlock/hwlock.txt | 26 +
drivers/hwspinlock/hwspinlock_core.c | 61
+-
include/linux/hwspinlock.h | 11 ++--
3 files changed, 93 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt
diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt
b/Documentation/devicetree/bindings/hwlock/hwlock.txt
new file mode 100644
index 000..789930e
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt
@@ -0,0 +1,26 @@
+Generic hwlock bindings
+===
+
+Generic bindings that are common to all the hwlock platform specific driver
+implementations, the retrieved values are used for registering the device
+specific parameters with the hwspinlock core.
+
+The validity and need of these common properties may vary from one driver
+implementation to another. Look through the individual hwlock driver
+binding documentations for identifying which are mandatory and which are
+optional for that specific driver.
+
+Common properties:
+- hwlock-base-id: Base Id for the locks for a particular hwlock device.
+This property is used for representing a set of locks
+present in a hwlock device with a unique base id in
+the driver core. This property is mandatory ONLY if a
+SoC has several hwlock devices.
+
+See documentation on struct hwspinlock_pdata in
+linux/hwspinlock.h for more details.
+
+- hwlock-num-locks: Number of locks present in a hwlock device. This
+property is needed on hwlock devices, where the number
+of supported locks within a hwlock device cannot be
+read from a register.
diff --git a/drivers/hwspinlock/hwspinlock_core.c
b/drivers/hwspinlock/hwspinlock_core.c
index 461a0d7..aec32e7 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -1,7 +1,7 @@
/*
* Hardware spinlock framework
*
- * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2010-2013 Texas Instruments Incorporated -
http://www.ti.com
*
* Contact: Ohad Ben-Cohen o...@wizery.com
*
@@ -27,6 +27,7 @@
#include linux/hwspinlock.h
#include linux/pm_runtime.h
#include linux/mutex.h
+#include linux/of.h
#include hwspinlock_internal.h
@@ -308,6 +309,64 @@ out:
}
/**
+ * of_hwspin_lock_get_base_id() - OF helper to retrieve base id
+ * @dn: device node pointer
+ *
+ * This is an OF helper function that can be called by the underlying
+ * platform-specific implementations, to retrieve the base id for the
+ * set of locks present within a hwspinlock device instance.
+ *
+ * Returns the base id value on success, -ENODEV on generic failure or
+ * an appropriate error code as returned by the OF layer
+ */
+int of_hwspin_lock_get_base_id(struct device_node *dn)
+{
+unsigned int val;
+int ret = -ENODEV;
+
+#ifdef CONFIG_OF
+if (!dn)
+return -ENODEV;
Checking !dn is done in of_property_read_u32, so you don't need to duplicate
I have added this specifically since my intention is to differentiate
the validity of the node vs the presence of the property within a node.
This property may be optional for some platforms and a must for some
others, and differentiating would allow the individual driver
implementations to make the distinction.
+
+ret = of_property_read_u32(dn, hwlock-base-id, val);
+if (!ret)
+ret = val;
+#endif
+
+return ret;
+}
+EXPORT_SYMBOL_GPL(of_hwspin_lock_get_base_id);
+
+/**
+ * of_hwspin_lock_get_num_locks() - OF helper to retrieve number of locks
+ * @dn: device node pointer
+ *
+ * This is an OF helper function that can be called by the underlying
+ * platform-specific implementations, to retrieve the number of locks
+ * present within a hwspinlock device instance.
+ *
+ * Returns a positive number of locks on success, -ENODEV on generic
+ * failure or an appropriate error code as returned by the OF layer
+ */
+int of_hwspin_lock_get_num_locks(struct device_node *dn)
+{
+unsigned int val;
+int ret = -ENODEV;
+
+#ifdef CONFIG_OF
+if (!dn)
+return -ENODEV;
Checking !dn is done in
Re: [PATCHv2 1/9] hwspinlock/core: add common dt bindings and OF helpers
On Sep 27, 2013, at 11:48 AM, Suman Anna wrote:
Kumar,
On 09/27/2013 11:04 AM, Kumar Gala wrote:
On Sep 17, 2013, at 2:30 PM, Suman Anna wrote:
All the platform-specific hwlock driver implementations need the
number of locks and the associated base id for registering the
locks present within a hwspinlock device with the driver core.
These two variables are represented by 'hwlock-num-locks' and
'hwlock-base-id' properties. The documentation and OF helpers to
retrieve these common properties have been added to the driver core.
Signed-off-by: Suman Anna s-a...@ti.com
---
.../devicetree/bindings/hwlock/hwlock.txt | 26 +
drivers/hwspinlock/hwspinlock_core.c | 61
+-
include/linux/hwspinlock.h | 11 ++--
3 files changed, 93 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt
diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt
b/Documentation/devicetree/bindings/hwlock/hwlock.txt
new file mode 100644
index 000..789930e
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt
@@ -0,0 +1,26 @@
+Generic hwlock bindings
+===
+
+Generic bindings that are common to all the hwlock platform specific driver
+implementations, the retrieved values are used for registering the device
+specific parameters with the hwspinlock core.
+
+The validity and need of these common properties may vary from one driver
+implementation to another. Look through the individual hwlock driver
+binding documentations for identifying which are mandatory and which are
+optional for that specific driver.
+
+Common properties:
+- hwlock-base-id: Base Id for the locks for a particular hwlock device.
+ This property is used for representing a set of locks
+ present in a hwlock device with a unique base id in
+ the driver core. This property is mandatory ONLY if a
+ SoC has several hwlock devices.
+
+ See documentation on struct hwspinlock_pdata in
+ linux/hwspinlock.h for more details.
+
+- hwlock-num-locks:Number of locks present in a hwlock device. This
+ property is needed on hwlock devices, where the number
+ of supported locks within a hwlock device cannot be
+ read from a register.
Was meaning to say this before, another reason for hwlock-num-locks is to limit
the # of locks available to the processors running linux vs what other
processors in the SoC might be using.
diff --git a/drivers/hwspinlock/hwspinlock_core.c
b/drivers/hwspinlock/hwspinlock_core.c
index 461a0d7..aec32e7 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -1,7 +1,7 @@
/*
* Hardware spinlock framework
*
- * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2010-2013 Texas Instruments Incorporated -
http://www.ti.com
*
* Contact: Ohad Ben-Cohen o...@wizery.com
*
@@ -27,6 +27,7 @@
#include linux/hwspinlock.h
#include linux/pm_runtime.h
#include linux/mutex.h
+#include linux/of.h
#include hwspinlock_internal.h
@@ -308,6 +309,64 @@ out:
}
/**
+ * of_hwspin_lock_get_base_id() - OF helper to retrieve base id
+ * @dn: device node pointer
+ *
+ * This is an OF helper function that can be called by the underlying
+ * platform-specific implementations, to retrieve the base id for the
+ * set of locks present within a hwspinlock device instance.
+ *
+ * Returns the base id value on success, -ENODEV on generic failure or
+ * an appropriate error code as returned by the OF layer
+ */
+int of_hwspin_lock_get_base_id(struct device_node *dn)
+{
+ unsigned int val;
+ int ret = -ENODEV;
+
+#ifdef CONFIG_OF
+ if (!dn)
+ return -ENODEV;
Checking !dn is done in of_property_read_u32, so you don't need to duplicate
I have added this specifically since my intention is to differentiate
the validity of the node vs the presence of the property within a node.
This property may be optional for some platforms and a must for some
others, and differentiating would allow the individual driver
implementations to make the distinction.
Maybe add a comment for both cases.
+
+ ret = of_property_read_u32(dn, hwlock-base-id, val);
+ if (!ret)
+ ret = val;
+#endif
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(of_hwspin_lock_get_base_id);
+
+/**
+ * of_hwspin_lock_get_num_locks() - OF helper to retrieve number of locks
+ * @dn: device node pointer
+ *
+ * This is an OF helper function that can be called by the underlying
+ * platform-specific implementations, to retrieve the number of locks
+ * present within a hwspinlock device instance.
+ *
+ * Returns a positive number of locks on success, -ENODEV on generic
+ *
Re: [PATCHv2 1/9] hwspinlock/core: add common dt bindings and OF helpers
Kumar,
On 09/27/2013 11:04 AM, Kumar Gala wrote:
On Sep 17, 2013, at 2:30 PM, Suman Anna wrote:
All the platform-specific hwlock driver implementations need the
number of locks and the associated base id for registering the
locks present within a hwspinlock device with the driver core.
These two variables are represented by 'hwlock-num-locks' and
'hwlock-base-id' properties. The documentation and OF helpers to
retrieve these common properties have been added to the driver core.
Signed-off-by: Suman Anna s-a...@ti.com
---
.../devicetree/bindings/hwlock/hwlock.txt | 26 +
drivers/hwspinlock/hwspinlock_core.c | 61
+-
include/linux/hwspinlock.h | 11 ++--
3 files changed, 93 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwlock/hwlock.txt
diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt
b/Documentation/devicetree/bindings/hwlock/hwlock.txt
new file mode 100644
index 000..789930e
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt
@@ -0,0 +1,26 @@
+Generic hwlock bindings
+===
+
+Generic bindings that are common to all the hwlock platform specific
driver
+implementations, the retrieved values are used for registering the device
+specific parameters with the hwspinlock core.
+
+The validity and need of these common properties may vary from one driver
+implementation to another. Look through the individual hwlock driver
+binding documentations for identifying which are mandatory and which are
+optional for that specific driver.
+
+Common properties:
+- hwlock-base-id: Base Id for the locks for a particular hwlock device.
+ This property is used for representing a set of locks
+ present in a hwlock device with a unique base id in
+ the driver core. This property is mandatory ONLY if a
+ SoC has several hwlock devices.
+
+ See documentation on struct hwspinlock_pdata in
+ linux/hwspinlock.h for more details.
+
+- hwlock-num-locks: Number of locks present in a hwlock device. This
+ property is needed on hwlock devices, where the number
+ of supported locks within a hwlock device cannot be
+ read from a register.
Was meaning to say this before, another reason for hwlock-num-locks is to
limit the # of locks available to the processors running linux vs what other
processors in the SoC might be using.
Yes, I understand the usecase/scenario since we had a similar need on
our product. That said, I guess this should be left to the individual
driver implementation/integration/documentation since this can be
achieved in different ways and depends on how you partition the
resources on your system (static partition vs resource reservation at
linux init time). Mentioning that here begs the question if you are
gonna use the starting 'n' locks, ending 'n' locks or range of 'n' locks
beginning in the middle. It would also imply it has to work together
with the hwlock-base-id as well in the last case. I would prefer to keep
the documentation generic here.
diff --git a/drivers/hwspinlock/hwspinlock_core.c
b/drivers/hwspinlock/hwspinlock_core.c
index 461a0d7..aec32e7 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -1,7 +1,7 @@
/*
* Hardware spinlock framework
*
- * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2010-2013 Texas Instruments Incorporated -
http://www.ti.com
*
* Contact: Ohad Ben-Cohen o...@wizery.com
*
@@ -27,6 +27,7 @@
#include linux/hwspinlock.h
#include linux/pm_runtime.h
#include linux/mutex.h
+#include linux/of.h
#include hwspinlock_internal.h
@@ -308,6 +309,64 @@ out:
}
/**
+ * of_hwspin_lock_get_base_id() - OF helper to retrieve base id
+ * @dn: device node pointer
+ *
+ * This is an OF helper function that can be called by the underlying
+ * platform-specific implementations, to retrieve the base id for the
+ * set of locks present within a hwspinlock device instance.
+ *
+ * Returns the base id value on success, -ENODEV on generic failure or
+ * an appropriate error code as returned by the OF layer
+ */
+int of_hwspin_lock_get_base_id(struct device_node *dn)
+{
+ unsigned int val;
+ int ret = -ENODEV;
+
+#ifdef CONFIG_OF
+ if (!dn)
+ return -ENODEV;
Checking !dn is done in of_property_read_u32, so you don't need to duplicate
I have added this specifically since my intention is to differentiate
the validity of the node vs the presence of the property within a node.
This property may be optional for some platforms and a must for some
others, and differentiating would allow the individual driver
implementations to make the distinction.
Maybe add a comment for both
