[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241014020915.GB20620@rigel>
Date: Mon, 14 Oct 2024 10:09:15 +0800
From: Kent Gibson <warthog618@...il.com>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Linus Walleij <linus.walleij@...aro.org>, linux-gpio@...r.kernel.org,
linux-kernel@...r.kernel.org,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v2 4/6] gpio: cdev: put emitting the line state events on
a workqueue
On Thu, Oct 10, 2024 at 11:10:25AM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
>
> In order to allow line state notifications to be emitted from atomic
> context (for instance: from gpiod_direction_input/output()), we must
> stop calling any sleeping functions in lineinfo_changed_notify(). To
> that end let's use the new workqueue.
>
> Let's atomically allocate small structures containing the required data
> and fill it with information immediately upon being notified about the
> change except for the pinctrl state which will be retrieved later from
> process context. We can pretty reliably do this as pin functions are
> typically set once per boot.
>
> Let's make sure to bump the reference count of GPIO device and the GPIO
> character device file descriptor to keep both alive until the event was
> queued.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> ---
> drivers/gpio/gpiolib-cdev.c | 72 +++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 60 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index dec6c9a6005f..2677134b52cd 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -2451,6 +2451,7 @@ struct gpio_chardev_data {
> #ifdef CONFIG_GPIO_CDEV_V1
> atomic_t watch_abi_version;
> #endif
> + struct file *fp;
> };
>
> static int chipinfo_get(struct gpio_chardev_data *cdev, void __user *ip)
> @@ -2621,29 +2622,75 @@ static long gpio_ioctl_compat(struct file *file, unsigned int cmd,
> }
> #endif
>
> +struct lineinfo_changed_ctx {
> + struct work_struct work;
> + struct gpio_v2_line_info_changed chg;
> + struct gpio_device *gdev;
> + struct gpio_chardev_data *cdev;
> +};
> +
> +static void lineinfo_changed_func(struct work_struct *work)
> +{
> + struct lineinfo_changed_ctx *ctx =
> + container_of(work, struct lineinfo_changed_ctx, work);
> + struct gpio_chip *gc;
> + int ret;
> +
> + scoped_guard(srcu, &ctx->gdev->srcu) {
> + gc = srcu_dereference(ctx->gdev->chip, &ctx->gdev->srcu);
> + if (!gc)
> + return;
> +
> + /*
> + * We're doing this late because it's a sleeping function. Pin
> + * functions are in general much more static and while it's not
> + * 100% bullet-proof, it's good enough for most cases.
> + */
> + if (!pinctrl_gpio_can_use_line(gc, ctx->chg.info.offset))
> + ctx->chg.info.flags |= GPIO_V2_LINE_FLAG_USED;
> + }
> +
This block should be conditional on GPIO_V2_LINE_FLAG_USED not already
being set - most of the time it will be and then this is pointless work.
Cheers,
Kent.
Powered by blists - more mailing lists