| Message ID | 1434548543-22949-9-git-send-email-tomeu.vizoso@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 17, 2015 at 03:42:18PM +0200, Tomeu Vizoso wrote: > +static bool strends(const char *str, const char *postfix) > +{ > + if (strlen(str) < strlen(postfix)) > + return false; > + > + return strcmp(str + strlen(str) - strlen(postfix), postfix) == 0; > +} This is named like (and looks like) a generic fuction, shouldn't it be in string.h or something? > +static void add_dependency(struct fwnode_handle *fwnode, > + struct list_head *list) > +{ > + struct fwnode_dependency *dep; > + > + dep = kzalloc(sizeof(*dep), GFP_KERNEL); > + if (!dep) > + return; > + > + INIT_LIST_HEAD(&dep->dependency); > + dep->fwnode = fwnode; > + > + list_add_tail(&dep->dependency, list); > +} Might be worth putting this in generic code, it looks pretty generic? I have to say I'm unclear what frees the returned list. > + if (!is_of_node(fwnode)) > + return NULL; > + > + np = of_node(fwnode); > + if (!np) > + return NULL; Presumably the first check could be dropped? > + list = kzalloc(sizeof(*list), GFP_KERNEL); > + if (!list) > + return NULL; Might it make sense for the core to allocate the head of the list and just ask the classes to add to the list? We're going to want to merge the dependencies from multiple subsystems and that saves allocating heads that may never get anything added to them.
On 17 June 2015 at 19:40, Mark Brown <broonie@kernel.org> wrote: > On Wed, Jun 17, 2015 at 03:42:18PM +0200, Tomeu Vizoso wrote: > >> +static bool strends(const char *str, const char *postfix) >> +{ >> + if (strlen(str) < strlen(postfix)) >> + return false; >> + >> + return strcmp(str + strlen(str) - strlen(postfix), postfix) == 0; >> +} > > This is named like (and looks like) a generic fuction, shouldn't it be > in string.h or something? Yeah, will put it there. >> +static void add_dependency(struct fwnode_handle *fwnode, >> + struct list_head *list) >> +{ >> + struct fwnode_dependency *dep; >> + >> + dep = kzalloc(sizeof(*dep), GFP_KERNEL); >> + if (!dep) >> + return; >> + >> + INIT_LIST_HEAD(&dep->dependency); >> + dep->fwnode = fwnode; >> + >> + list_add_tail(&dep->dependency, list); >> +} > > Might be worth putting this in generic code, it looks pretty generic? I > have to say I'm unclear what frees the returned list. Agreed. >> + if (!is_of_node(fwnode)) >> + return NULL; >> + >> + np = of_node(fwnode); >> + if (!np) >> + return NULL; > > Presumably the first check could be dropped? That's right. >> + list = kzalloc(sizeof(*list), GFP_KERNEL); >> + if (!list) >> + return NULL; > > Might it make sense for the core to allocate the head of the list and > just ask the classes to add to the list? We're going to want to merge > the dependencies from multiple subsystems and that saves allocating > heads that may never get anything added to them. Yes, I have gone with that advice and it looks better that way. Thanks, Tomeu
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c index b57ed8e..d0a7fb1 100644 --- a/drivers/gpio/gpiolib-sysfs.c +++ b/drivers/gpio/gpiolib-sysfs.c @@ -7,6 +7,7 @@ #include <linux/interrupt.h> #include <linux/kdev_t.h> #include <linux/slab.h> +#include <linux/of.h> #include "gpiolib.h" @@ -515,6 +516,85 @@ done: return status ? : len; } +static bool strends(const char *str, const char *postfix) +{ + if (strlen(str) < strlen(postfix)) + return false; + + return strcmp(str + strlen(str) - strlen(postfix), postfix) == 0; +} + +static void add_dependency(struct fwnode_handle *fwnode, + struct list_head *list) +{ + struct fwnode_dependency *dep; + + dep = kzalloc(sizeof(*dep), GFP_KERNEL); + if (!dep) + return; + + INIT_LIST_HEAD(&dep->dependency); + dep->fwnode = fwnode; + + list_add_tail(&dep->dependency, list); +} + +struct list_head *gpio_get_dependencies(struct fwnode_handle *fwnode) +{ + struct device_node *np = of_node(fwnode); + struct list_head *list = NULL; + struct property *pp; + struct of_phandle_args pspec; + int count, i, ret; + + if (!is_of_node(fwnode)) + return NULL; + + np = of_node(fwnode); + if (!np) + return NULL; + + list = kzalloc(sizeof(*list), GFP_KERNEL); + if (!list) + return NULL; + + INIT_LIST_HEAD(list); + + for_each_property_of_node(np, pp) { + if (strcmp(pp->name, "gpio") && + !strends(pp->name, "-gpios") && + !strends(pp->name, "-gpio")) + continue; + + count = of_count_phandle_with_args(np, pp->name, + "#gpio-cells"); + for (i = 0; i < count; i++) { + ret = of_parse_phandle_with_args(np, pp->name, + "#gpio-cells", i, + &pspec); + if (ret || !pspec.np) + continue; + + add_dependency(&pspec.np->fwnode, list); + + of_node_put(pspec.np); + } + } + + for (i = 0;; i++) { + ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3, + i, &pspec); + if (ret) + break; + + add_dependency(&pspec.np->fwnode, list); + + of_node_put(pspec.np); + } + + return list; +} + static struct class_attribute gpio_class_attrs[] = { __ATTR(export, 0200, NULL, export_store), __ATTR(unexport, 0200, NULL, unexport_store), @@ -526,6 +606,7 @@ static struct class gpio_class = { .owner = THIS_MODULE, .class_attrs = gpio_class_attrs, + .get_dependencies = gpio_get_dependencies, };
So the GPIO subsystem can be queried about the dependencies of nodes that consume GPIOs, as specified in bindings/gpio/gpio.txt. Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> --- drivers/gpio/gpiolib-sysfs.c | 81 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+)