| Message ID | 20230411082806.41361-1-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: gpiolib: Simplify gpiochip_add_data_with_key() fwnode | expand |
On Tue, 11 Apr 2023 at 10:28, Linus Walleij <linus.walleij@linaro.org> wrote: > > The code defaulting to the parents fwnode if no fwnode was assigned > is unnecessarily convoluted, probably due to refactoring. Simplify > it and make it more human-readable. > > Cc: Anders Roxell <anders.roxell@linaro.org> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Reviewed-by: Anders Roxell <anders.roxell@linaro.org> > --- > Anders: you can test this but I don't think it fixes the > regression you have pointing to commit > 24c94060fc9b4e0f19e6e018869db46db21d6bc7 Did not fix the issue though. Cheers, Anders > --- > drivers/gpio/gpiolib.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index 19bd23044b01..5801d682c12b 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -667,7 +667,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, > struct lock_class_key *lock_key, > struct lock_class_key *request_key) > { > - struct fwnode_handle *fwnode = NULL; > struct gpio_device *gdev; > unsigned long flags; > unsigned int i; > @@ -675,12 +674,12 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, > int base = 0; > int ret = 0; > > - /* If the calling driver did not initialize firmware node, do it here */ > - if (gc->fwnode) > - fwnode = gc->fwnode; > - else if (gc->parent) > - fwnode = dev_fwnode(gc->parent); > - gc->fwnode = fwnode; > + /* > + * If the calling driver did not initialize firmware node, do it here > + * using the parent device, if any. > + */ > + if (!gc->fwnode && gc->parent) > + gc->fwnode = dev_fwnode(gc->parent); > > /* > * First: allocate and populate the internal stat container, and > -- > 2.39.2 >
On Tue, Apr 11, 2023 at 10:28:06AM +0200, Linus Walleij wrote: > The code defaulting to the parents fwnode if no fwnode was assigned > is unnecessarily convoluted, probably due to refactoring. Yes, the refactoring patches tried to avoid unneeded churn as you now consolidated into a separate change. > Simplify > it and make it more human-readable. ... > - struct fwnode_handle *fwnode = NULL; > struct gpio_device *gdev; > unsigned long flags; > unsigned int i; > @@ -675,12 +674,12 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, > int base = 0; > int ret = 0; > > - /* If the calling driver did not initialize firmware node, do it here */ > - if (gc->fwnode) > - fwnode = gc->fwnode; > - else if (gc->parent) > - fwnode = dev_fwnode(gc->parent); > - gc->fwnode = fwnode; > + /* > + * If the calling driver did not initialize firmware node, do it here > + * using the parent device, if any. > + */ I would prefer to have this comment either untouched or being changed where it appears to be the same (e.g. in IIO core). > + if (!gc->fwnode && gc->parent) > + gc->fwnode = dev_fwnode(gc->parent); Otherwise fine by me. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
On Tue, Apr 11, 2023 at 10:28 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > The code defaulting to the parents fwnode if no fwnode was assigned > is unnecessarily convoluted, probably due to refactoring. Simplify > it and make it more human-readable. > > Cc: Anders Roxell <anders.roxell@linaro.org> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- Applied, thanks! Bart
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 19bd23044b01..5801d682c12b 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -667,7 +667,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, struct lock_class_key *lock_key, struct lock_class_key *request_key) { - struct fwnode_handle *fwnode = NULL; struct gpio_device *gdev; unsigned long flags; unsigned int i; @@ -675,12 +674,12 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, int base = 0; int ret = 0; - /* If the calling driver did not initialize firmware node, do it here */ - if (gc->fwnode) - fwnode = gc->fwnode; - else if (gc->parent) - fwnode = dev_fwnode(gc->parent); - gc->fwnode = fwnode; + /* + * If the calling driver did not initialize firmware node, do it here + * using the parent device, if any. + */ + if (!gc->fwnode && gc->parent) + gc->fwnode = dev_fwnode(gc->parent); /* * First: allocate and populate the internal stat container, and
The code defaulting to the parents fwnode if no fwnode was assigned is unnecessarily convoluted, probably due to refactoring. Simplify it and make it more human-readable. Cc: Anders Roxell <anders.roxell@linaro.org> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- Anders: you can test this but I don't think it fixes the regression you have pointing to commit 24c94060fc9b4e0f19e6e018869db46db21d6bc7 --- drivers/gpio/gpiolib.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)