| Message ID | 20221125181158.67265-2-andriy.shevchenko@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1,1/2] gpiolib: Provide to_gpio_device() helper | expand |
On Fri, Nov 25, 2022 at 7:11 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > Introduce gpio_device_get() and gpio_device_put() helpers > and convert existing users. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> That's nice coding taste. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Tue, Nov 29, 2022 at 02:54:10PM +0100, Bartosz Golaszewski wrote: > On Fri, Nov 25, 2022 at 7:11 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > Introduce gpio_device_get() and gpio_device_put() helpers > > and convert existing users. > This doesn't apply to my for-next branch, can you rebase and resend > (just this one, the other is applied). The problem is that you don't merge or cherry-pick fixes into your for-next branch and they are getting diverged. In PDx86 subsystem we decided to cherry-pick the fixes into for-next. Some other subsystems are doing back-merges (but I remember that Linus T. complained about back merges, although I dunno if it's still the case). Some subsystems merges their fixes into for-next, dunno if it's the best practice either. That said, this can be submitted after v6.2-rc1 is out.
On Tue, Nov 29, 2022 at 3:47 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Nov 29, 2022 at 02:54:10PM +0100, Bartosz Golaszewski wrote: > > On Fri, Nov 25, 2022 at 7:11 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > Introduce gpio_device_get() and gpio_device_put() helpers > > > and convert existing users. > > > This doesn't apply to my for-next branch, can you rebase and resend > > (just this one, the other is applied). > > The problem is that you don't merge or cherry-pick fixes into your > for-next branch and they are getting diverged. > > In PDx86 subsystem we decided to cherry-pick the fixes into for-next. > Some other subsystems are doing back-merges (but I remember that Linus > T. complained about back merges, although I dunno if it's still > the case). Some subsystems merges their fixes into for-next, dunno > if it's the best practice either. > > That said, this can be submitted after v6.2-rc1 is out. > I do merge tags if I need to. Normally you'd mention any requirements for the series in the cover letter. I have not sent any fixes so far in this release cycle BTW. Bart
On Tue, Nov 29, 2022 at 04:49:34PM +0100, Bartosz Golaszewski wrote: > On Tue, Nov 29, 2022 at 3:47 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Nov 29, 2022 at 02:54:10PM +0100, Bartosz Golaszewski wrote: > > > On Fri, Nov 25, 2022 at 7:11 PM Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > > > Introduce gpio_device_get() and gpio_device_put() helpers > > > > and convert existing users. > > > > > This doesn't apply to my for-next branch, can you rebase and resend > > > (just this one, the other is applied). > > > > The problem is that you don't merge or cherry-pick fixes into your > > for-next branch and they are getting diverged. > > > > In PDx86 subsystem we decided to cherry-pick the fixes into for-next. > > Some other subsystems are doing back-merges (but I remember that Linus > > T. complained about back merges, although I dunno if it's still > > the case). Some subsystems merges their fixes into for-next, dunno > > if it's the best practice either. > > > > That said, this can be submitted after v6.2-rc1 is out. > > I do merge tags if I need to. Normally you'd mention any requirements > for the series in the cover letter. It's my fault. But thanks for clarifying the process. > I have not sent any fixes so far in this release cycle BTW. Yes, that's now obvious...
On Tue, Nov 29, 2022 at 06:21:28PM +0200, Andy Shevchenko wrote: > On Tue, Nov 29, 2022 at 04:49:34PM +0100, Bartosz Golaszewski wrote: > > On Tue, Nov 29, 2022 at 3:47 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Tue, Nov 29, 2022 at 02:54:10PM +0100, Bartosz Golaszewski wrote: ... > > > That said, this can be submitted after v6.2-rc1 is out. Btw, since you are trying to fix a user space race, I would like to postpone this since it will conflict with that one anyway.
On Tue, Nov 29, 2022 at 3:47 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > In PDx86 subsystem we decided to cherry-pick the fixes into for-next. > Some other subsystems are doing back-merges (but I remember that Linus > T. complained about back merges, although I dunno if it's still > the case). Some subsystems merges their fixes into for-next, dunno > if it's the best practice either. I usually (A) let it conflict (Torvalds will solve) if it is small and Rothwell fixed it up in next (B) back-merge some -rcN if is is big and annoying or as last resort (C) apply the patch to for-next (git will cope, but not elegant). Yours, Linus Walleij
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c index f9288e41e3a7..acfb779767ed 100644 --- a/drivers/gpio/gpiolib-cdev.c +++ b/drivers/gpio/gpiolib-cdev.c @@ -265,7 +265,7 @@ static void linehandle_free(struct linehandle_state *lh) if (lh->descs[i]) gpiod_free(lh->descs[i]); kfree(lh->label); - put_device(&lh->gdev->dev); + gpio_device_put(lh->gdev); kfree(lh); } @@ -307,8 +307,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip) lh = kzalloc(sizeof(*lh), GFP_KERNEL); if (!lh) return -ENOMEM; - lh->gdev = gdev; - get_device(&gdev->dev); + lh->gdev = gpio_device_get(gdev); if (handlereq.consumer_label[0] != '\0') { /* label is only initialized if consumer_label is set */ @@ -1480,7 +1479,7 @@ static void linereq_free(struct linereq *lr) } kfifo_free(&lr->events); kfree(lr->label); - put_device(&lr->gdev->dev); + gpio_device_put(lr->gdev); kfree(lr); } @@ -1550,8 +1549,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip) if (!lr) return -ENOMEM; - lr->gdev = gdev; - get_device(&gdev->dev); + lr->gdev = gpio_device_get(gdev); for (i = 0; i < ulr.num_lines; i++) { lr->lines[i].req = lr; @@ -1799,7 +1797,7 @@ static void lineevent_free(struct lineevent_state *le) if (le->desc) gpiod_free(le->desc); kfree(le->label); - put_device(&le->gdev->dev); + gpio_device_put(le->gdev); kfree(le); } @@ -1965,8 +1963,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip) le = kzalloc(sizeof(*le), GFP_KERNEL); if (!le) return -ENOMEM; - le->gdev = gdev; - get_device(&gdev->dev); + le->gdev = gpio_device_get(gdev); if (eventreq.consumer_label[0] != '\0') { /* label is only initialized if consumer_label is set */ @@ -2514,7 +2511,7 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file) init_waitqueue_head(&cdev->wait); INIT_KFIFO(cdev->events); - cdev->gdev = gdev; + cdev->gdev = gpio_device_get(gdev); cdev->lineinfo_changed_nb.notifier_call = lineinfo_changed_notify; ret = blocking_notifier_chain_register(&gdev->notifier, @@ -2522,7 +2519,6 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file) if (ret) goto out_free_bitmap; - get_device(&gdev->dev); file->private_data = cdev; ret = nonseekable_open(inode, file); @@ -2535,6 +2531,7 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file) blocking_notifier_chain_unregister(&gdev->notifier, &cdev->lineinfo_changed_nb); out_free_bitmap: + gpio_device_put(gdev); bitmap_free(cdev->watched_lines); out_free_cdev: kfree(cdev); @@ -2555,7 +2552,7 @@ static int gpio_chrdev_release(struct inode *inode, struct file *file) bitmap_free(cdev->watched_lines); blocking_notifier_chain_unregister(&gdev->notifier, &cdev->lineinfo_changed_nb); - put_device(&gdev->dev); + gpio_device_put(gdev); kfree(cdev); return 0; diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 9b935288db9d..52a1b03987dc 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -874,7 +874,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, gpiochip_free_valid_mask(gc); if (gdev->dev.release) { /* release() has been registered by gpiochip_setup_dev() */ - put_device(&gdev->dev); + gpio_device_put(gdev); goto err_print_message; } err_remove_from_list: @@ -961,7 +961,7 @@ void gpiochip_remove(struct gpio_chip *gc) * gone. */ gcdev_unregister(gdev); - put_device(&gdev->dev); + gpio_device_put(gdev); } EXPORT_SYMBOL_GPL(gpiochip_remove); @@ -2052,17 +2052,15 @@ static int validate_desc(const struct gpio_desc *desc, const char *func) int gpiod_request(struct gpio_desc *desc, const char *label) { int ret = -EPROBE_DEFER; - struct gpio_device *gdev; VALIDATE_DESC(desc); - gdev = desc->gdev; - if (try_module_get(gdev->owner)) { + if (try_module_get(desc->gdev->owner)) { ret = gpiod_request_commit(desc, label); if (ret) - module_put(gdev->owner); + module_put(desc->gdev->owner); else - get_device(&gdev->dev); + gpio_device_get(desc->gdev); } if (ret) @@ -2123,7 +2121,7 @@ void gpiod_free(struct gpio_desc *desc) { if (desc && desc->gdev && gpiod_free_commit(desc)) { module_put(desc->gdev->owner); - put_device(&desc->gdev->dev); + gpio_device_put(desc->gdev); } else { WARN_ON(extra_checks); } diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h index 027674025561..13b2c02ec328 100644 --- a/drivers/gpio/gpiolib.h +++ b/drivers/gpio/gpiolib.h @@ -74,6 +74,16 @@ struct gpio_device { #define to_gpio_device(_dev_) container_of(_dev_, struct gpio_device, dev) +static inline struct gpio_device *gpio_device_get(struct gpio_device *gdev) +{ + return to_gpio_device(get_device(&gdev->dev)); +} + +static inline void gpio_device_put(struct gpio_device *gdev) +{ + put_device(&gdev->dev); +} + /* gpio suffixes used for ACPI and device tree lookup */ static __maybe_unused const char * const gpio_suffixes[] = { "gpios", "gpio" };
Introduce gpio_device_get() and gpio_device_put() helpers and convert existing users. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/gpio/gpiolib-cdev.c | 21 +++++++++------------ drivers/gpio/gpiolib.c | 14 ++++++-------- drivers/gpio/gpiolib.h | 10 ++++++++++ 3 files changed, 25 insertions(+), 20 deletions(-)