lkml.org 
[lkml]   [2023]   [Aug]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2] gpiolib: notify user-space about line state changes triggered by kernel
    On Thu, Aug 24, 2023 at 11:08 AM Kent Gibson <warthog618@gmail.com> wrote:
    >
    > On Thu, Aug 24, 2023 at 10:55:44AM +0200, Bartosz Golaszewski wrote:
    > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
    > >
    > > We currently only emit CHANGED_CONFIG events when the user-space changes
    > > GPIO config. We won't be notified if changes come from in-kernel. Let's
    > > call the notifier chain whenever kernel users change direction or any of
    > > the active-low, debounce or consumer name settings. We don't notify the
    > > user-space about the persistence as the uAPI has no notion of it.
    > >
    > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
    > > ---
    > > v1 -> v2:
    > > - use the gpiod_line_state_notify() helper
    > > - reorder the code in gpiod_set_debounce() for better readability
    > >
    > > drivers/gpio/gpiolib.c | 16 ++++++++++++++--
    > > 1 file changed, 14 insertions(+), 2 deletions(-)
    > >
    > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
    > > index 40a0022ea719..1cb7731550ca 100644
    > > --- a/drivers/gpio/gpiolib.c
    > > +++ b/drivers/gpio/gpiolib.c
    > > @@ -2439,6 +2439,7 @@ int gpiod_direction_input(struct gpio_desc *desc)
    > > }
    > > if (ret == 0) {
    > > clear_bit(FLAG_IS_OUT, &desc->flags);
    > > + gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG);
    > > ret = gpio_set_bias(desc);
    > > }
    > >
    >
    > gpiod_direction_input() is called by cdev when a line is requested.
    > So requesting a line now creates two events - REQUESTED and
    > HANGED_CONFIG? Even worse - it calls gpiod_direction_input() first, so we
    > will get CHANGED_CONFIG then REQUESTED??
    >
    > And a config change from cdev can call this and then generate a
    > CHANGED_CONFIG event itself, so again double events.
    >
    > Same for output and probably debounce too (that one is a bit more
    > convoluted).

    Ah, should have tested it with user-space too...

    Back to the drawing board I guess. May be the reason why we're not
    doing it in the first place yet.

    Bart

    >
    > Cheers,
    > Kent.
    >
    > > @@ -2484,8 +2485,10 @@ static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
    > > gc->set(gc, gpio_chip_hwgpio(desc), val);
    > > }
    > >
    > > - if (!ret)
    > > + if (!ret) {
    > > set_bit(FLAG_IS_OUT, &desc->flags);
    > > + gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG);
    > > + }
    > > trace_gpio_value(desc_to_gpio(desc), 0, val);
    > > trace_gpio_direction(desc_to_gpio(desc), 0, ret);
    > > return ret;
    > > @@ -2672,9 +2675,16 @@ EXPORT_SYMBOL_GPL(gpiod_set_config);
    > > int gpiod_set_debounce(struct gpio_desc *desc, unsigned int debounce)
    > > {
    > > unsigned long config;
    > > + int ret;
    > >
    > > config = pinconf_to_config_packed(PIN_CONFIG_INPUT_DEBOUNCE, debounce);
    > > - return gpiod_set_config(desc, config);
    > > + ret = gpiod_set_config(desc, config);
    > > + if (ret)
    > > + return ret;
    > > +
    > > + gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG);
    > > +
    > > + return 0;
    > > }
    > > EXPORT_SYMBOL_GPL(gpiod_set_debounce);
    > >
    > > @@ -2723,6 +2733,7 @@ void gpiod_toggle_active_low(struct gpio_desc *desc)
    > > {
    > > VALIDATE_DESC_VOID(desc);
    > > change_bit(FLAG_ACTIVE_LOW, &desc->flags);
    > > + gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG);
    > > }
    > > EXPORT_SYMBOL_GPL(gpiod_toggle_active_low);
    > >
    > > @@ -3330,6 +3341,7 @@ int gpiod_set_consumer_name(struct gpio_desc *desc, const char *name)
    > >
    > > kfree_const(desc->label);
    > > desc_set_label(desc, name);
    > > + gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG);
    > >
    > > return 0;
    > > }
    > > --
    > > 2.39.2
    > >

    \
     
     \ /
      Last update: 2023-08-24 11:12    [W:3.246 / U:0.100 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site