Re: [PATCH 0/4] gpiolib: cdev: relocate debounce_period_us

[Bartosz Golaszewski]

On Tue, Dec 12, 2023 at 6:43 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> This series contains minor improvements to gpiolib-cdev.
>
> The banner change is relocating the debounce_period_us from gpiolib's
> struct gpio_desc to cdev's struct line.  The first patch stores the
> field locally in cdev.  The second removes the now unused field from
> gpiolib.
>
> The third patch is somewhat related and removes a FIXME from
> gpio_desc_to_lineinfo().  The FIXME relates to a race condition in
> the calculation of the used  flag, but I would assert that from
> the userspace perspective the read operation itself is inherently racy.
> The line being reported as unused in the info provides no guarantee -
> it just an indicator that requesting the line is likely to succeed -
> assuming the line is not otherwise requested in the meantime.
> Give the overall operation is racy, trying to stamp out an unlikely
> race within the operation is pointless. Accept it as a possibility
> that has negligible side-effects and reduce the number of locks held
> simultaneously and the duration that the gpio_lock is held.
>
> The fourth patch is unrelated to debounce or info, but addresses Andy's
> recent assertion that the linereq get/set values functions are confusing
> and under documented.  Figured I may as well add that while I was in
> there.
>
> Kent Gibson (4):
>   gpiolib: cdev: relocate debounce_period_us from struct gpio_desc
>   gpiolib: remove debounce_period_us from struct gpio_desc
>   gpiolib: cdev: reduce locking in gpio_desc_to_lineinfo()
>   gpiolib: cdev: improve documentation of get/set values
>
>  drivers/gpio/gpiolib-cdev.c | 257 ++++++++++++++++++++++++++++--------
>  drivers/gpio/gpiolib.c      |   3 -
>  drivers/gpio/gpiolib.h      |   5 -
>  3 files changed, 201 insertions(+), 64 deletions(-)
>
> --
> 2.39.2
>

Patches 2-4 look fine, I was about to review patch 1 in detail but I
thought I'd just throw this one in here before we commit to a specific
solution.

For some reason I thought this would not work but I'm now considering
it as an alternative approach: is there anything wrong with adding
struct kref to struct line, allocating it separately per-line when
gpio_chardev_data is created, referencing it from struct linereq when
the line is being requested, and dropping the reference from
gpio_chardev_data and linereq when either is being removed? Other than
the increased number of allocations?

Bartosz