| Message ID | 20250203031213.399914-5-koichiro.den@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce configfs-based interface for gpio-aggregator | expand |
Hi Den, On Mon, 3 Feb 2025 at 04:12, Koichiro Den <koichiro.den@canonical.com> wrote: > Previously, there is no way to assign names to GPIO lines exported > through an aggregator. > > Allow users to set custom line names via a 'name' attribute. > > Signed-off-by: Koichiro Den <koichiro.den@canonical.com> Thanks for your patch! > --- a/drivers/gpio/gpio-aggregator.c > +++ b/drivers/gpio/gpio-aggregator.c > @@ -63,6 +63,8 @@ struct gpio_aggregator_line { > /* Line index within the aggregator device */ > int idx; > > + /* Custom name for the virtual line */ > + char *name; This can be const. > /* GPIO chip label or line name */ > char *key; Actually this can be const, too. > /* Can be negative to indicate lookup by line name */ > @@ -678,6 +680,44 @@ gpio_aggr_line_key_store(struct config_item *item, const char *page, > > CONFIGFS_ATTR(gpio_aggr_line_, key); > > +static ssize_t > +gpio_aggr_line_name_show(struct config_item *item, char *page) > +{ > + struct gpio_aggregator_line *line = to_gpio_aggregator_line(item); > + struct gpio_aggregator *aggr = line->parent; > + > + guard(mutex)(&aggr->lock); > + > + return sprintf(page, "%s\n", line->name ?: ""); sysfs_emit() > +} Gr{oetje,eeting}s, Geert
On Wed, Feb 12, 2025 at 03:27:50PM GMT, Geert Uytterhoeven wrote: > Hi Den, > > On Mon, 3 Feb 2025 at 04:12, Koichiro Den <koichiro.den@canonical.com> wrote: > > Previously, there is no way to assign names to GPIO lines exported > > through an aggregator. > > > > Allow users to set custom line names via a 'name' attribute. > > > > Signed-off-by: Koichiro Den <koichiro.den@canonical.com> > > Thanks for your patch! > > > --- a/drivers/gpio/gpio-aggregator.c > > +++ b/drivers/gpio/gpio-aggregator.c > > @@ -63,6 +63,8 @@ struct gpio_aggregator_line { > > /* Line index within the aggregator device */ > > int idx; > > > > + /* Custom name for the virtual line */ > > + char *name; > > This can be const. Will fix in v3. > > > /* GPIO chip label or line name */ > > char *key; > > Actually this can be const, too. Will fix in v3. > > > /* Can be negative to indicate lookup by line name */ > > @@ -678,6 +680,44 @@ gpio_aggr_line_key_store(struct config_item *item, const char *page, > > > > CONFIGFS_ATTR(gpio_aggr_line_, key); > > > > +static ssize_t > > +gpio_aggr_line_name_show(struct config_item *item, char *page) > > +{ > > + struct gpio_aggregator_line *line = to_gpio_aggregator_line(item); > > + struct gpio_aggregator *aggr = line->parent; > > + > > + guard(mutex)(&aggr->lock); > > + > > + return sprintf(page, "%s\n", line->name ?: ""); > > sysfs_emit() Will fix in v3. Thanks! Koichiro > > > +} > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c index 76d3a8677308..3263d99bfe69 100644 --- a/drivers/gpio/gpio-aggregator.c +++ b/drivers/gpio/gpio-aggregator.c @@ -63,6 +63,8 @@ struct gpio_aggregator_line { /* Line index within the aggregator device */ int idx; + /* Custom name for the virtual line */ + char *name; /* GPIO chip label or line name */ char *key; /* Can be negative to indicate lookup by line name */ @@ -678,6 +680,44 @@ gpio_aggr_line_key_store(struct config_item *item, const char *page, CONFIGFS_ATTR(gpio_aggr_line_, key); +static ssize_t +gpio_aggr_line_name_show(struct config_item *item, char *page) +{ + struct gpio_aggregator_line *line = to_gpio_aggregator_line(item); + struct gpio_aggregator *aggr = line->parent; + + guard(mutex)(&aggr->lock); + + return sprintf(page, "%s\n", line->name ?: ""); +} + +static ssize_t +gpio_aggr_line_name_store(struct config_item *item, const char *page, + size_t count) +{ + struct gpio_aggregator_line *line = to_gpio_aggregator_line(item); + struct gpio_aggregator *aggr = line->parent; + + char *name __free(kfree) = kstrndup(skip_spaces(page), count, + GFP_KERNEL); + if (!name) + return -ENOMEM; + + strim(name); + + guard(mutex)(&aggr->lock); + + if (aggr_is_active(aggr)) + return -EBUSY; + + kfree(line->name); + line->name = no_free_ptr(name); + + return count; +} + +CONFIGFS_ATTR(gpio_aggr_line_, name); + static ssize_t gpio_aggr_line_offset_show(struct config_item *item, char *page) { @@ -728,6 +768,7 @@ CONFIGFS_ATTR(gpio_aggr_line_, offset); static struct configfs_attribute *gpio_aggr_line_attrs[] = { &gpio_aggr_line_attr_key, + &gpio_aggr_line_attr_name, &gpio_aggr_line_attr_offset, NULL }; @@ -813,6 +854,7 @@ gpio_aggr_line_release(struct config_item *item) aggr_line_del(aggr, line); kfree(line->key); + kfree(line->name); kfree(line); }
Previously, there is no way to assign names to GPIO lines exported through an aggregator. Allow users to set custom line names via a 'name' attribute. Signed-off-by: Koichiro Den <koichiro.den@canonical.com> --- drivers/gpio/gpio-aggregator.c | 42 ++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)