From: Bartosz Golaszewski <[email protected]>
Remove all remaining uses of find_chip_by_name() (and subsequently:
gpiochip_find()) from gpiolib.c and use the new
gpio_device_find_by_label() instead.
Signed-off-by: Bartosz Golaszewski <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
---
drivers/gpio/gpiolib.c | 33 ++++++++++++---------------------
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e26cbd10a246..4c734bfe6d32 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1145,18 +1145,6 @@ struct gpio_device *gpio_device_find_by_label(const char *label)
}
EXPORT_SYMBOL_GPL(gpio_device_find_by_label);
-static int gpiochip_match_name(struct gpio_chip *gc, void *data)
-{
- const char *name = data;
-
- return !strcmp(gc->label, name);
-}
-
-static struct gpio_chip *find_chip_by_name(const char *name)
-{
- return gpiochip_find((void *)name, gpiochip_match_name);
-}
-
/**
* gpio_device_get() - Increase the reference count of this GPIO device
* @gdev: GPIO device to increase the refcount for
@@ -3908,21 +3896,22 @@ EXPORT_SYMBOL_GPL(gpiod_remove_lookup_table);
*/
void gpiod_add_hogs(struct gpiod_hog *hogs)
{
- struct gpio_chip *gc;
struct gpiod_hog *hog;
mutex_lock(&gpio_machine_hogs_mutex);
for (hog = &hogs[0]; hog->chip_label; hog++) {
+ struct gpio_device *gdev __free(gpio_device_put) = NULL;
+
list_add_tail(&hog->list, &gpio_machine_hogs);
/*
* The chip may have been registered earlier, so check if it
* exists and, if so, try to hog the line now.
*/
- gc = find_chip_by_name(hog->chip_label);
- if (gc)
- gpiochip_machine_hog(gc, hog);
+ gdev = gpio_device_find_by_label(hog->chip_label);
+ if (gdev)
+ gpiochip_machine_hog(gpio_device_get_chip(gdev), hog);
}
mutex_unlock(&gpio_machine_hogs_mutex);
@@ -3977,13 +3966,14 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,
struct gpio_desc *desc = ERR_PTR(-ENOENT);
struct gpiod_lookup_table *table;
struct gpiod_lookup *p;
+ struct gpio_chip *gc;
table = gpiod_find_lookup_table(dev);
if (!table)
return desc;
for (p = &table->table[0]; p->key; p++) {
- struct gpio_chip *gc;
+ struct gpio_device *gdev __free(gpio_device_put) = NULL;
/* idx must always match exactly */
if (p->idx != idx)
@@ -4005,9 +3995,8 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,
return ERR_PTR(-EPROBE_DEFER);
}
- gc = find_chip_by_name(p->key);
-
- if (!gc) {
+ gdev = gpio_device_find_by_label(p->key);
+ if (!gdev) {
/*
* As the lookup table indicates a chip with
* p->key should exist, assume it may
@@ -4020,6 +4009,8 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,
return ERR_PTR(-EPROBE_DEFER);
}
+ gc = gpio_device_get_chip(gdev);
+
if (gc->ngpio <= p->chip_hwnum) {
dev_err(dev,
"requested GPIO %u (%u) is out of range [0..%u] for chip %s\n",
@@ -4028,7 +4019,7 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,
return ERR_PTR(-EINVAL);
}
- desc = gpiochip_get_desc(gc, p->chip_hwnum);
+ desc = gpio_device_get_desc(gdev, p->chip_hwnum);
*flags = p->flags;
return desc;
--
2.39.2
On Fri, Sep 15, 2023 at 05:03:22PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> Remove all remaining uses of find_chip_by_name() (and subsequently:
> gpiochip_find()) from gpiolib.c and use the new
> gpio_device_find_by_label() instead.
...
> -static int gpiochip_match_name(struct gpio_chip *gc, void *data)
> -{
> - const char *name = data;
> -
> - return !strcmp(gc->label, name);
And this we had no check for the label being NULL...
...
> for (p = &table->table[0]; p->key; p++) {
> + struct gpio_device *gdev __free(gpio_device_put) = NULL;
> + gdev = gpio_device_find_by_label(p->key);
> + if (!gdev) {
I haven't got the fix for gpio-sim, shouldn't we have the same here, i.e.
definition being done together with the assignment when __free() is in use?
> }
--
With Best Regards,
Andy Shevchenko
On Mon, Sep 18, 2023 at 9:23 AM Andy Shevchenko
<[email protected]> wrote:
>
> On Fri, Sep 15, 2023 at 05:03:22PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <[email protected]>
> >
> > Remove all remaining uses of find_chip_by_name() (and subsequently:
> > gpiochip_find()) from gpiolib.c and use the new
> > gpio_device_find_by_label() instead.
>
> ...
>
> > -static int gpiochip_match_name(struct gpio_chip *gc, void *data)
> > -{
> > - const char *name = data;
> > -
> > - return !strcmp(gc->label, name);
>
> And this we had no check for the label being NULL...
>
Yeah, it was wrong. But maybe all kernel users already do assign it in
which case we should be safe just adding a check in
gpiochip_add_data_with_key() that would return EINVAL if they don't.
> ...
>
> > for (p = &table->table[0]; p->key; p++) {
> > + struct gpio_device *gdev __free(gpio_device_put) = NULL;
>
> > + gdev = gpio_device_find_by_label(p->key);
> > + if (!gdev) {
>
> I haven't got the fix for gpio-sim, shouldn't we have the same here, i.e.
> definition being done together with the assignment when __free() is in use?
>
It should but I only got yelled at by Linus under the gpio-sim patch
after I sent this one.
Bart
> > }
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
On Mon, Sep 18, 2023 at 10:03 AM Bartosz Golaszewski <[email protected]> wrote:
> On Mon, Sep 18, 2023 at 9:23 AM Andy Shevchenko
> <[email protected]> wrote:
> > > for (p = &table->table[0]; p->key; p++) {
> > > + struct gpio_device *gdev __free(gpio_device_put) = NULL;
> >
> > > + gdev = gpio_device_find_by_label(p->key);
> > > + if (!gdev) {
> >
> > I haven't got the fix for gpio-sim, shouldn't we have the same here, i.e.
> > definition being done together with the assignment when __free() is in use?
>
> It should but I only got yelled at by Linus under the gpio-sim patch
> after I sent this one.
That happens, it's all new.
I guess ideally we should patch checkpatch to just moan about
this, I wonder how hard that could be (I've only patched it once in
my life...)
Yours,
Linus Walleij