| Message ID | 1410553499-55951-5-git-send-email-s-anna@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Suman, On Fri, Sep 12, 2014 at 11:24 PM, Suman Anna <s-anna@ti.com> wrote: > +int of_hwspin_lock_get_id(struct device_node *np, int index) > +{ > + struct hwspinlock_device *bank; > + struct of_phandle_args args; > + int id; > + int ret; > + > + ret = of_parse_phandle_with_args(np, "hwlocks", "#hwlock-cells", index, > + &args); > + if (ret) > + return ret; > + > + mutex_lock(&hwspinlock_tree_lock); > + list_for_each_entry(bank, &hwspinlock_devices, list) > + if (bank->dev->of_node == args.np) > + break; > + mutex_unlock(&hwspinlock_tree_lock); > + if (&bank->list == &hwspinlock_devices) { > + ret = -EPROBE_DEFER; > + goto out; > + } Is this the validation you mentioned which requires the existence of "hwspinlock/core: maintain a list of registered hwspinlock banks" ? I'm not convinced this is needed for several reasons: - the user isn't using the lock yet at this point, and may only need the id in order to communicate it to a remote processor - if the user will try to use the lock prematurely, hwspin_lock_request_specific should stop her - moreover, hwspin_lock_request_specific must be the one who validates the id, since in heterogeneous systems the user may get the id from a remote processor and not via of_hwspin_lock_get_id "hwspinlock/core: maintain a list of registered hwspinlock banks" adds complexity which must be very strongly justified. If we're not sure there is a strong justification for it, we better not merge it. > +EXPORT_SYMBOL_GPL(of_hwspin_lock_get_base_id); ... > +EXPORT_SYMBOL_GPL(of_hwspin_lock_get_num_locks); Do we really must expose these two helpers globally? Can we instead make these "static inline" methods and embed them in hwspinlock_internal.h ? Thanks, Ohad. -- 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 Ohad, Thanks for the review. On 11/12/2014 01:08 PM, Ohad Ben-Cohen wrote: > Hi Suman, > > On Fri, Sep 12, 2014 at 11:24 PM, Suman Anna <s-anna@ti.com> wrote: >> +int of_hwspin_lock_get_id(struct device_node *np, int index) >> +{ >> + struct hwspinlock_device *bank; >> + struct of_phandle_args args; >> + int id; >> + int ret; >> + >> + ret = of_parse_phandle_with_args(np, "hwlocks", "#hwlock-cells", index, >> + &args); >> + if (ret) >> + return ret; >> + >> + mutex_lock(&hwspinlock_tree_lock); >> + list_for_each_entry(bank, &hwspinlock_devices, list) >> + if (bank->dev->of_node == args.np) >> + break; >> + mutex_unlock(&hwspinlock_tree_lock); >> + if (&bank->list == &hwspinlock_devices) { >> + ret = -EPROBE_DEFER; >> + goto out; >> + } > > Is this the validation you mentioned which requires the existence of > "hwspinlock/core: maintain a list of registered hwspinlock banks" ? Well, not exactly. The validation is on the following segment, + id = of_hwspin_lock_simple_xlate(bank, &args); + if (id < 0 || id >= bank->num_locks) { + ret = -EINVAL; + goto out; + } That said, it is also needed to provide the support for deferred probing without changing the return code conventions on the existing API. > > I'm not convinced this is needed for several reasons: > - the user isn't using the lock yet at this point, and may only need > the id in order to communicate it to a remote processor Yes, and wouldn't that require that the id is validated? It just cannot return any return value, and expect it will be verified somewhere else or in a following API call. Not doing the validation unnecessarily complicates the system usage of a lock as you are sharing an invalid lock to a remote processor and then you have two validation failure paths on different processors. > - if the user will try to use the lock prematurely, > hwspin_lock_request_specific should stop her > - moreover, hwspin_lock_request_specific must be the one who validates > the id, since in heterogeneous systems the user may get the id from a > remote processor and not via of_hwspin_lock_get_id > > "hwspinlock/core: maintain a list of registered hwspinlock banks" > adds complexity which must be very strongly justified. > > If we're not sure there is a strong justification for it, we better > not merge it. > >> +EXPORT_SYMBOL_GPL(of_hwspin_lock_get_base_id); > ... >> +EXPORT_SYMBOL_GPL(of_hwspin_lock_get_num_locks); > > Do we really must expose these two helpers globally? > > Can we instead make these "static inline" methods and embed them in > hwspinlock_internal.h ? Actually, not a bad idea, I will move it, thanks. All the client drivers would need it, and they already have to include the internal header. 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, On Wed, Nov 12, 2014 at 9:32 PM, Suman Anna <s-anna@ti.com> wrote: >> Is this the validation you mentioned which requires the existence of >> "hwspinlock/core: maintain a list of registered hwspinlock banks" ? > > Well, not exactly. The validation is on the following segment, > > + id = of_hwspin_lock_simple_xlate(bank, &args); > + if (id < 0 || id >= bank->num_locks) { > + ret = -EINVAL; > + goto out; > + } I'm not entirely convinced that this justifies adding the proposed linked list. Can't we can get the base_id and number of locks by traversing the DT? > That said, it is also needed to provide the support for deferred probing > without changing the return code conventions on the existing API. Here too, adding a data structure maintaining memory objects and taking care of it life cycle seems a bit overkill for anything to do with return values. > Yes, and wouldn't that require that the id is validated? It just cannot > return any return value, and expect it will be verified somewhere else > or in a following API call. Not doing the validation unnecessarily > complicates the system usage of a lock as you are sharing an invalid > lock to a remote processor and then you have two validation failure > paths on different processors. Validation is great but I'm still not convinced it can't be done without maintaining another data structure. Please show me specific technical issues that can't be resolved without adding the proposed linked list. Thanks, Ohad. -- 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 Ohad, On 11/13/2014 04:03 AM, Ohad Ben-Cohen wrote: > Hi Suman, > > On Wed, Nov 12, 2014 at 9:32 PM, Suman Anna <s-anna@ti.com> wrote: >>> Is this the validation you mentioned which requires the existence of >>> "hwspinlock/core: maintain a list of registered hwspinlock banks" ? >> >> Well, not exactly. The validation is on the following segment, >> >> + id = of_hwspin_lock_simple_xlate(bank, &args); >> + if (id < 0 || id >= bank->num_locks) { >> + ret = -EINVAL; >> + goto out; >> + } > > I'm not entirely convinced that this justifies adding the proposed > linked list. Can't we can get the base_id and number of locks by > traversing the DT? No, not always, because, either of them can be optional between different platform implementations. For example, on OMAP, the number of locks is read from an IP register, and not coded in DT. Similarly, base_id can be optional on SoCs that don't have multiple IP instances. The only place the hwspinlock core knows both of them for sure is at the device registration time, but the core only stores the locks and not the devices at the moment. Any operation on the device is not possible without knowing the exact global lock we are dealing with, and this API is about returning that exact global lock id. > >> That said, it is also needed to provide the support for deferred probing >> without changing the return code conventions on the existing API. > > Here too, adding a data structure maintaining memory objects and > taking care of it life cycle seems a bit overkill for anything to do > with return values. IMHO, this life cycle management is not that complicated, it is managed alongside the addition/removal of the locks during the device registration/unregistration time. > >> Yes, and wouldn't that require that the id is validated? It just cannot >> return any return value, and expect it will be verified somewhere else >> or in a following API call. Not doing the validation unnecessarily >> complicates the system usage of a lock as you are sharing an invalid >> lock to a remote processor and then you have two validation failure >> paths on different processors. > > Validation is great but I'm still not convinced it can't be done > without maintaining another data structure. > > Please show me specific technical issues that can't be resolved > without adding the proposed linked list. Same as above. 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, On Thu, Nov 13, 2014 at 7:38 PM, Suman Anna <s-anna@ti.com> wrote: > No, not always, because, either of them can be optional between > different platform implementations. For example, on OMAP, the number of > locks is read from an IP register, and not coded in DT. Similarly, > base_id can be optional on SoCs that don't have multiple IP instances. It can, but base_id isn't optional today --- it must be provided via platform_data --- and I'm not sure we should implicitly change this semantics while moving to DT. We never guessed the base_id before, even if there was only a single IP instance, and I'm not convinced we should start doing it now. About the number of locks - why do we even need it at this point? the global lock id should just be base_id + the lock index. > IMHO, this life cycle management is not that complicated It's always very easy to add code, really. It is never complicated. But then gradually the code becomes harder to maintain, debug, and change. On the contrary, achieving the same functionality with less code is always harder. But when we get there, the results are pretty satisfying. In this case I still feel the extra linked list is redundant. Please try it: ditch the "hwspinlock/core: maintain a list of registered hwspinlock banks" patch, and replace of_hwspin_lock_get_id with a slim method that just returns the base_id + the lock index. Let me know if it works for you. Thanks, Ohad. -- 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 Ohad, On 11/13/2014 01:45 PM, Ohad Ben-Cohen wrote: > Hi Suman, > > On Thu, Nov 13, 2014 at 7:38 PM, Suman Anna <s-anna@ti.com> wrote: >> No, not always, because, either of them can be optional between >> different platform implementations. For example, on OMAP, the number of >> locks is read from an IP register, and not coded in DT. Similarly, >> base_id can be optional on SoCs that don't have multiple IP instances. > > It can, but base_id isn't optional today --- it must be provided via > platform_data --- That was the case before DT, right. As it is, the hwspinlock core has no knowledge of platform_data, it was used by the individual implementation drivers to supply the base_id in the registration API. The platform_data will/should become obsolete with DT devices. and I'm not sure we should implicitly change this > semantics while moving to DT. We never guessed the base_id before, > even if there was only a single IP instance, and I'm not convinced we > should start doing it now. OK, if we make hwlock-base-id mandatory in DT, we will get past 1 problem (that of looking up base-id for a specific device). > > About the number of locks - why do we even need it at this point? the > global lock id should just be base_id + the lock index. The number of locks would still be needed for validation purposes. OK, lets take an example. I have say 2 device instances, say hwlock1: hwlock@0 { hwlock-num-locks = <32> hwlock-base-id = <0>; #hwlock-cells = <1>; }; hwlock2: hwlock@0 { hwlock-num-locks = <32> hwlock-base-id = <32>; #hwlock-cells = <1>; }; some-client { hwlocks = <&hwlock1 32>, <&hwlock2 0>; }; The first args value is incorrect, and I expect the API to return an error. I don't think the API can make assumptions that all passed in values will be correct. What do you suggest that the API do here? The above locks are equivalent if we forgo checking. > >> IMHO, this life cycle management is not that complicated > > It's always very easy to add code, really. It is never complicated. > But then gradually the code becomes harder to maintain, debug, and > change. > > On the contrary, achieving the same functionality with less code is > always harder. But when we get there, the results are pretty > satisfying. > > In this case I still feel the extra linked list is redundant. > > Please try it: ditch the "hwspinlock/core: maintain a list of > registered hwspinlock banks" patch, and replace of_hwspin_lock_get_id > with a slim method that just returns the base_id + the lock index. Let > me know if it works for you. Yeah, it will work (provided base-id is mandatory) and we drop the support for 1. phandle args-specifier validation 2. Deferred probing One solution to handle #1 is to also make the hwlock-num-locks property also mandatory (even though it could have been read from a register and I wonder what implementations should do if there is a mismatch between DT provided value and the value read from IP register). I am not sure what the DT maintainers' stance is on this, as we are forcing that to solve an implementation detail in the hwspinlock core. And, how do you propose we solve the problem of deferred probing? This was the solution that is resolving both the problems without changing return codes on existing API (that was my first attempt, but you preferred not to change API return convention). Kumar, Mark, Any comments here? 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, On Thu, Nov 13, 2014 at 11:02 PM, Suman Anna <s-anna@ti.com> wrote: > OK, lets take an example. I have say 2 device instances, say > hwlock1: hwlock@0 { > hwlock-num-locks = <32> > hwlock-base-id = <0>; > #hwlock-cells = <1>; > }; > hwlock2: hwlock@0 { > hwlock-num-locks = <32> > hwlock-base-id = <32>; > #hwlock-cells = <1>; > }; > > some-client { > hwlocks = <&hwlock1 32>, <&hwlock2 0>; > }; > > The first args value is incorrect, and I expect the API to return an > error. I don't think the API can make assumptions that all passed in > values will be correct. What do you suggest that the API do here? I think this is just one example of many potential mistakes the DT developer can have, and that there's nothing really special about it. What if the '5' digit below is a typo, and the actual hardware assignment was supposed to be '4' ? some-client { hwlocks = <&hwlock1 5>, <&hwlock2 5>; }; I don't see how much different this mistake is from the one you mentioned above. There's a limit to how much we can really catch DT mistakes in the code, just like we couldn't really catch past mistakes in the platform data structures. If we can easily add some validations then great, but if we have to go to great lengths just to gain very limited validations, then I'd vote against doing so. > One solution to handle #1 is to also make the hwlock-num-locks property > also mandatory (even though it could have been read from a register Yeah, this is awkward. We shouldn't impose this on implementations like OMAP which don't need it. Again I think for the very limited validation this buys us - it's not worth it. > And, how do you propose we solve the problem of deferred probing? It seems to me that hwspin_lock_request_specific failures should be used by clients to defer their probing. Why wouldn't such a simple solution work? Thanks, Ohad. -- 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 Ohad, On 11/14/2014 01:11 AM, Ohad Ben-Cohen wrote: > Hi Suman, > > On Thu, Nov 13, 2014 at 11:02 PM, Suman Anna <s-anna@ti.com> wrote: >> OK, lets take an example. I have say 2 device instances, say >> hwlock1: hwlock@0 { >> hwlock-num-locks = <32> >> hwlock-base-id = <0>; >> #hwlock-cells = <1>; >> }; >> hwlock2: hwlock@0 { >> hwlock-num-locks = <32> >> hwlock-base-id = <32>; >> #hwlock-cells = <1>; >> }; >> >> some-client { >> hwlocks = <&hwlock1 32>, <&hwlock2 0>; >> }; >> >> The first args value is incorrect, and I expect the API to return an >> error. I don't think the API can make assumptions that all passed in >> values will be correct. What do you suggest that the API do here? > > I think this is just one example of many potential mistakes the DT > developer can have, and that there's nothing really special about it. > > What if the '5' digit below is a typo, and the actual hardware > assignment was supposed to be '4' ? > > some-client { > hwlocks = <&hwlock1 5>, <&hwlock2 5>; > }; > > I don't see how much different this mistake is from the one you > mentioned above. There's a limit to how much we can really catch DT > mistakes in the code, just like we couldn't really catch past mistakes > in the platform data structures. > > If we can easily add some validations then great, but if we have to go > to great lengths just to gain very limited validations, then I'd vote > against doing so. OK, personally, I am not too comfortable to not perform any validation on an API. > >> One solution to handle #1 is to also make the hwlock-num-locks property >> also mandatory (even though it could have been read from a register > > Yeah, this is awkward. We shouldn't impose this on implementations > like OMAP which don't need it. > > Again I think for the very limited validation this buys us - it's not worth it. > >> And, how do you propose we solve the problem of deferred probing? > > It seems to me that hwspin_lock_request_specific failures should be > used by clients to defer their probing. Why wouldn't such a simple > solution work? Because the API always returns NULL on failures and there is no way for the clients to figure out if the lock id passed is invalid or the bank containing the lock is not registered. This was an old discussion on v4 [1], and you insisted that we don't change the return code convention on the API. I had couple of patches on v5 that dealt with this, but even that requires the list of registered devices. regards Suman [1] https://patchwork.kernel.org/patch/3481331/ -- 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 Fri, Nov 14, 2014 at 7:09 PM, Suman Anna <s-anna@ti.com> wrote: >> It seems to me that hwspin_lock_request_specific failures should be >> used by clients to defer their probing. Why wouldn't such a simple >> solution work? > Because the API always returns NULL on failures and there is no way for > the clients to figure out if the lock id passed is invalid or the bank > containing the lock is not registered. It seems to me this may always be the case - lock ids may be wrong and there's no way to fully validate them. Let's start with the simpler approach where hwspin_lock_request_specific failures are used by clients to defer their probing. If a real use case will require changes we can always do that later, though it seems to me the only gain by changing this API is to catch a small subset of fatal DT mistakes which will anyway be caught very early in the development. Thanks, Ohad. -- 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/hwspinlock.txt b/Documentation/hwspinlock.txt index 640ae47..c15dc9f 100644 --- a/Documentation/hwspinlock.txt +++ b/Documentation/hwspinlock.txt @@ -48,6 +48,16 @@ independent, drivers. ids for predefined purposes. Should be called from a process context (might sleep). + int of_hwspin_lock_get_id(struct device_node *np, int index); + - retrieve the global lock id for an OF phandle-based specific lock. + This function provides a means for DT users of a hwspinlock module + to get the global lock id of a specific hwspinlock, so that it can + be requested using the normal hwspin_lock_request_specific() API. + The function returns a valid lock id number on success, -EPROBE_DEFER + if the hwspinlock device is not yet registered with the core, or other + error values. + Should be called from a process context (might sleep). + int hwspin_lock_free(struct hwspinlock *hwlock); - free a previously-assigned hwspinlock; returns 0 on success, or an appropriate error code on failure (e.g. -EINVAL if the hwspinlock @@ -243,6 +253,22 @@ int hwspinlock_example2(void) Returns the address of hwspinlock on success, or NULL on error (e.g. if the hwspinlock is still in use). + int of_hwspin_lock_get_num_locks(struct device_node *dn); + - is a common OF helper function that can be used by some underlying + vendor-specific implementations. This can be used by implementations + that require and define the number of locks supported within a hwspinlock + bank as a device tree node property. This function should be called by + needed implementations before registering a hwspinlock device with the + core. + + int of_hwspin_lock_get_base_id(struct device_node *dn); + - is a common OF helper function that can be used by some underlying + vendor-specific implementations. This can be used by implementations + that require and define the base index for a block of locks present + within a hwspinlock bank as a device tree node property. This function + should be called by needed implementations before registering a hwspinlock + device with the core. + 5. Important structs struct hwspinlock_device is a device which usually contains a bank diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c index 48f7866..7d9f749 100644 --- a/drivers/hwspinlock/hwspinlock_core.c +++ b/drivers/hwspinlock/hwspinlock_core.c @@ -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" @@ -262,6 +263,127 @@ void __hwspin_unlock(struct hwspinlock *hwlock, int mode, unsigned long *flags) } EXPORT_SYMBOL_GPL(__hwspin_unlock); +/** + * of_hwspin_lock_simple_xlate - translate hwlock_spec to return a lock id + * @bank: the hwspinlock device bank + * @hwlock_spec: hwlock specifier as found in the device tree + * + * This is a simple translation function, suitable for hwspinlock platform + * drivers that only has a lock specifier length of 1. + * + * Returns a negative value on error, and a relative index of the lock within + * a specified bank on success. + */ +static int of_hwspin_lock_simple_xlate(struct hwspinlock_device *bank, + const struct of_phandle_args *hwlock_spec) +{ + /* sanity check (these shouldn't happen) */ + if (WARN_ON(!bank->dev->of_node)) + return -EINVAL; + + if (WARN_ON(hwlock_spec->args_count != 1)) + return -EINVAL; + + return hwlock_spec->args[0]; +} + +/** + * of_hwspin_lock_get_id() - get lock id for an OF phandle-based specific lock + * @np: device node from which to request the specific hwlock + * @index: index of the hwlock in the list of values + * + * This function provides a means for DT users of the hwspinlock module to + * get the global lock id of a specific hwspinlock using the phandle of the + * hwspinlock device, so that it can be requested using the normal + * hwspin_lock_request_specific() API. + * + * Returns the global lock id number on success, -EPROBE_DEFER if the hwspinlock + * device is not yet registered, -EINVAL on invalid args specifier value or an + * appropriate error as returned from the OF parsing of the DT user node. + */ +int of_hwspin_lock_get_id(struct device_node *np, int index) +{ + struct hwspinlock_device *bank; + struct of_phandle_args args; + int id; + int ret; + + ret = of_parse_phandle_with_args(np, "hwlocks", "#hwlock-cells", index, + &args); + if (ret) + return ret; + + mutex_lock(&hwspinlock_tree_lock); + list_for_each_entry(bank, &hwspinlock_devices, list) + if (bank->dev->of_node == args.np) + break; + mutex_unlock(&hwspinlock_tree_lock); + if (&bank->list == &hwspinlock_devices) { + ret = -EPROBE_DEFER; + goto out; + } + + id = of_hwspin_lock_simple_xlate(bank, &args); + if (id < 0 || id >= bank->num_locks) { + ret = -EINVAL; + goto out; + } + id += bank->base_id; + +out: + of_node_put(args.np); + return ret ? ret : id; +} +EXPORT_SYMBOL_GPL(of_hwspin_lock_get_id); + +/** + * 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, 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; + + ret = of_property_read_u32(dn, "hwlock-base-id", &val); + return ret ? ret : val; +} +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. The hwlock-num-locks + * DT property may be optional for some platforms, while mandatory for + * some others, so this function is typically called only by needed + * platform-specific implementations. + * + * 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; + + ret = of_property_read_u32(dn, "hwlock-num-locks", &val); + if (!ret) + ret = val ? val : -ENODEV; + + return ret; +} +EXPORT_SYMBOL_GPL(of_hwspin_lock_get_num_locks); + static int hwspin_lock_register_single(struct hwspinlock *hwlock, int id) { struct hwspinlock *tmp; diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h index 3343298..a9eeb4f 100644 --- a/include/linux/hwspinlock.h +++ b/include/linux/hwspinlock.h @@ -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,12 +61,15 @@ 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); struct hwspinlock *hwspin_lock_request(void); struct hwspinlock *hwspin_lock_request_specific(unsigned int id); int hwspin_lock_free(struct hwspinlock *hwlock); +int of_hwspin_lock_get_id(struct device_node *np, int index); int hwspin_lock_get_id(struct hwspinlock *hwlock); int __hwspin_lock_timeout(struct hwspinlock *, unsigned int, int, unsigned long *); @@ -80,9 +84,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. @@ -120,6 +124,11 @@ void __hwspin_unlock(struct hwspinlock *hwlock, int mode, unsigned long *flags) { } +static inline int of_hwspin_lock_get_id(struct device_node *np, int index) +{ + return 0; +} + static inline int hwspin_lock_get_id(struct hwspinlock *hwlock) { return 0;
This patch adds three new OF helper functions to use/request locks from a hwspinlock device instantiated through a device-tree blob. 1. The of_hwspin_lock_get_num_locks() is a common OF helper function to read the 'hwlock-num-locks' property. 2. The of_hwspin_lock_get_base_id() is a common OF helper function to read the 'hwlock-base-id' property. 3. The of_hwspin_lock_get_id() API can be used by hwspinlock clients to get the id for a specific lock using the phandle + args specifier, so that it can be requested using the available hwspin_lock_request_specific() API. Signed-off-by: Suman Anna <s-anna@ti.com> --- Documentation/hwspinlock.txt | 26 ++++++++ drivers/hwspinlock/hwspinlock_core.c | 122 +++++++++++++++++++++++++++++++++++ include/linux/hwspinlock.h | 15 ++++- 3 files changed, 160 insertions(+), 3 deletions(-)