| Message ID | 4df03ac30dcc79132e3acc2e586d0ca99b431258.1379445653.git.s-anna@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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-anna@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 0000000..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 <ohad@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 hwspinlock device, which usually provides numerous hw locks > * @dev: the backing device > diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h > index 3343298..1f6a5b8 100644 > --- a/include/linux/hwspinlock.h > +++ b/include/linux/hwspinlock.h > @@ -1,7 +1,7 @@ > /* > * Hardware spinlock public header > * > - * 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 <ohad@wizery.com> > * > @@ -26,6 +26,7 @@ > #define HWLOCK_IRQ 0x02 /* Disable interrupts, don't save state */ > > struct device; > +struct device_node; > struct hwspinlock; > struct hwspinlock_device; > struct hwspinlock_ops; > @@ -60,6 +61,8 @@ struct hwspinlock_pdata { > > #if defined(CONFIG_HWSPINLOCK) || defined(CONFIG_HWSPINLOCK_MODULE) > > +int of_hwspin_lock_get_base_id(struct device_node *dn); > +int of_hwspin_lock_get_num_locks(struct device_node *dn); > int hwspin_lock_register(struct hwspinlock_device *bank, struct device *dev, > const struct hwspinlock_ops *ops, int base_id, int num_locks); > int hwspin_lock_unregister(struct hwspinlock_device *bank); > @@ -80,9 +83,9 @@ void __hwspin_unlock(struct hwspinlock *, int, unsigned long *); > * code path get compiled away. This way, if CONFIG_HWSPINLOCK is not > * required on a given setup, users will still work. > * > - * The only exception is hwspin_lock_register/hwspin_lock_unregister, with which > - * we _do_ want users to fail (no point in registering hwspinlock instances if > - * the framework is not available). > + * The only exception is hwspin_lock_register/hwspin_lock_unregister and > + * associated OF helpers, with which we _do_ want users to fail (no point > + * in registering hwspinlock instances if the framework is not available). > * > * Note: ERR_PTR(-ENODEV) will still be considered a success for NULL-checking > * users. Others, which care, can still check this with IS_ERR. > -- > 1.8.3.3 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
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-anna@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 0000000..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 <ohad@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 of_property_read_u32, so you don't need to duplicate Same comment as above. regards Suman > >> + >> + 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 hwspinlock device, which usually provides numerous hw locks >> * @dev: the backing device >> diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h >> index 3343298..1f6a5b8 100644 >> --- a/include/linux/hwspinlock.h >> +++ b/include/linux/hwspinlock.h >> @@ -1,7 +1,7 @@ >> /* >> * Hardware spinlock public header >> * >> - * 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 <ohad@wizery.com> >> * >> @@ -26,6 +26,7 @@ >> #define HWLOCK_IRQ 0x02 /* Disable interrupts, don't save state */ >> >> struct device; >> +struct device_node; >> struct hwspinlock; >> struct hwspinlock_device; >> struct hwspinlock_ops; >> @@ -60,6 +61,8 @@ struct hwspinlock_pdata { >> >> #if defined(CONFIG_HWSPINLOCK) || defined(CONFIG_HWSPINLOCK_MODULE) >> >> +int of_hwspin_lock_get_base_id(struct device_node *dn); >> +int of_hwspin_lock_get_num_locks(struct device_node *dn); >> int hwspin_lock_register(struct hwspinlock_device *bank, struct device *dev, >> const struct hwspinlock_ops *ops, int base_id, int num_locks); >> int hwspin_lock_unregister(struct hwspinlock_device *bank); >> @@ -80,9 +83,9 @@ void __hwspin_unlock(struct hwspinlock *, int, unsigned long *); >> * code path get compiled away. This way, if CONFIG_HWSPINLOCK is not >> * required on a given setup, users will still work. >> * >> - * The only exception is hwspin_lock_register/hwspin_lock_unregister, with which >> - * we _do_ want users to fail (no point in registering hwspinlock instances if >> - * the framework is not available). >> + * The only exception is hwspin_lock_register/hwspin_lock_unregister and >> + * associated OF helpers, with which we _do_ want users to fail (no point >> + * in registering hwspinlock instances if the framework is not available). >> * >> * Note: ERR_PTR(-ENODEV) will still be considered a success for NULL-checking >> * users. Others, which care, can still check this with IS_ERR. >> -- >> 1.8.3.3 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe devicetree" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-anna@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 0000000..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 <ohad@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 >>> + * 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 > > Same comment as above. > > regards > Suman > >> >>> + >>> + 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 hwspinlock device, which usually provides numerous hw locks >>> * @dev: the backing device >>> diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h >>> index 3343298..1f6a5b8 100644 >>> --- a/include/linux/hwspinlock.h >>> +++ b/include/linux/hwspinlock.h >>> @@ -1,7 +1,7 @@ >>> /* >>> * Hardware spinlock public header >>> * >>> - * 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 <ohad@wizery.com> >>> * >>> @@ -26,6 +26,7 @@ >>> #define HWLOCK_IRQ 0x02 /* Disable interrupts, don't save state */ >>> >>> struct device; >>> +struct device_node; >>> struct hwspinlock; >>> struct hwspinlock_device; >>> struct hwspinlock_ops; >>> @@ -60,6 +61,8 @@ struct hwspinlock_pdata { >>> >>> #if defined(CONFIG_HWSPINLOCK) || defined(CONFIG_HWSPINLOCK_MODULE) >>> >>> +int of_hwspin_lock_get_base_id(struct device_node *dn); >>> +int of_hwspin_lock_get_num_locks(struct device_node *dn); >>> int hwspin_lock_register(struct hwspinlock_device *bank, struct device *dev, >>> const struct hwspinlock_ops *ops, int base_id, int num_locks); >>> int hwspin_lock_unregister(struct hwspinlock_device *bank); >>> @@ -80,9 +83,9 @@ void __hwspin_unlock(struct hwspinlock *, int, unsigned long *); >>> * code path get compiled away. This way, if CONFIG_HWSPINLOCK is not >>> * required on a given setup, users will still work. >>> * >>> - * The only exception is hwspin_lock_register/hwspin_lock_unregister, with which >>> - * we _do_ want users to fail (no point in registering hwspinlock instances if >>> - * the framework is not available). >>> + * The only exception is hwspin_lock_register/hwspin_lock_unregister and >>> + * associated OF helpers, with which we _do_ want users to fail (no point >>> + * in registering hwspinlock instances if the framework is not available). >>> * >>> * Note: ERR_PTR(-ENODEV) will still be considered a success for NULL-checking >>> * users. Others, which care, can still check this with IS_ERR. >>> -- >>> 1.8.3.3 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe devicetree" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
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-anna@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 0000000..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 <ohad@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. OK, will do. regards Suman -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-anna@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 0000000..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 <ohad@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 Do we need the CONFIG_OF check? of_property_read_u32 is defined to return -ENOSYS if CONFIG_OF isn't defined. Or would that confuse a higher layer? Similarly elsewhere in the file. Cheers, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-anna@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 0000000..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 Instruments Incorporated - http://www.ti.com >>> * >>> * Contact: Ohad Ben-Cohen <ohad@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 > > Do we need the CONFIG_OF check? of_property_read_u32 is defined to > return -ENOSYS if CONFIG_OF isn't defined. Or would that confuse a > higher layer? These are primarily OF helpers and provided for the SoC implementation drivers, and I have used the CONFIG_OF check within the function to streamline the function prototypes and behavior in the common hwspinlock.h header file between combinations of CONFIG_HWSPINLOCK and CONFIG_OF. The check for !dn is done deliberately to help the implementation drivers. regards Suman -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-anna@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 0000000..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 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 Instruments Incorporated - http://www.ti.com > >>> * > >>> * Contact: Ohad Ben-Cohen <ohad@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 > > > > Do we need the CONFIG_OF check? of_property_read_u32 is defined to > > return -ENOSYS if CONFIG_OF isn't defined. Or would that confuse a > > higher layer? > > These are primarily OF helpers and provided for the SoC implementation > drivers, and I have used the CONFIG_OF check within the function to > streamline the function prototypes and behavior in the common > hwspinlock.h header file between combinations of CONFIG_HWSPINLOCK and > CONFIG_OF. Ok. Due to the !CONFIG_OF stub for of_property_read_u32, and the check for !dn done in of_property_read_u32, you could just have: int of_hwspin_lock_get_base_id(struct device_node *dn) { u32 val; if (of_property_read_u32(dn, "hwlock-base-id", &val) != 0) return -ENODEV; return val; } Which would work regardless of CONFIG_OF. That said, I don't think this function is necessary because a phandle + args approach would be fundamantally better. > > The check for !dn is done deliberately to help the implementation drivers. As I mentioned above, of_property_read_u32 already checks for !dn, so the check here is redundant. Cheers, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt new file mode 100644 index 0000000..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 <ohad@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; + + 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; + + 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 hwspinlock device, which usually provides numerous hw locks * @dev: the backing device diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h index 3343298..1f6a5b8 100644 --- a/include/linux/hwspinlock.h +++ b/include/linux/hwspinlock.h @@ -1,7 +1,7 @@ /* * Hardware spinlock public header * - * 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 <ohad@wizery.com> * @@ -26,6 +26,7 @@ #define HWLOCK_IRQ 0x02 /* Disable interrupts, don't save state */ struct device; +struct device_node; struct hwspinlock; struct hwspinlock_device; struct hwspinlock_ops; @@ -60,6 +61,8 @@ struct hwspinlock_pdata { #if defined(CONFIG_HWSPINLOCK) || defined(CONFIG_HWSPINLOCK_MODULE) +int of_hwspin_lock_get_base_id(struct device_node *dn); +int of_hwspin_lock_get_num_locks(struct device_node *dn); int hwspin_lock_register(struct hwspinlock_device *bank, struct device *dev, const struct hwspinlock_ops *ops, int base_id, int num_locks); int hwspin_lock_unregister(struct hwspinlock_device *bank); @@ -80,9 +83,9 @@ void __hwspin_unlock(struct hwspinlock *, int, unsigned long *); * code path get compiled away. This way, if CONFIG_HWSPINLOCK is not * required on a given setup, users will still work. * - * The only exception is hwspin_lock_register/hwspin_lock_unregister, with which - * we _do_ want users to fail (no point in registering hwspinlock instances if - * the framework is not available). + * The only exception is hwspin_lock_register/hwspin_lock_unregister and + * associated OF helpers, with which we _do_ want users to fail (no point + * in registering hwspinlock instances if the framework is not available). * * Note: ERR_PTR(-ENODEV) will still be considered a success for NULL-checking * users. Others, which care, can still check this with IS_ERR.
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-anna@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