Re: [PATCH v3 6/6] gpiolib: notify user-space about in-kernel line state changes

[Kent Gibson <warthog618@xxxxxxxxx>]

On Wed, Oct 16, 2024 at 01:19:44PM +0800, Kent Gibson wrote:
> On Tue, Oct 15, 2024 at 12:56:18PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> >
> > -	return gpio_do_set_config(guard.gc, gpio_chip_hwgpio(desc), config);
> > +	ret = gpio_do_set_config(guard.gc, gpio_chip_hwgpio(desc), config);
> > +	if (ret == 0) {
> > +		/* These are the only options we notify the userspace about. */
> > +		switch (pinconf_to_config_param(config)) {
> > +		case PIN_CONFIG_BIAS_DISABLE:
> > +		case PIN_CONFIG_BIAS_PULL_DOWN:
> > +		case PIN_CONFIG_BIAS_PULL_UP:
> > +		case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> > +		case PIN_CONFIG_DRIVE_OPEN_SOURCE:
> > +		case PIN_CONFIG_DRIVE_PUSH_PULL:
> > +		case PIN_CONFIG_INPUT_DEBOUNCE:
> > +			gpiod_line_state_notify(desc,
> > +						GPIO_V2_LINE_CHANGED_CONFIG);
> > +			break;
> > +		default:
> > +			break;
> > +		}
> > +	}
> > +
> > +	return ret;
> >  }
>
> Ah, the debounce - I forgot about that, and other features that cdev
> might emulate.
>
> What happens if userspace requests a line with debounce that is
> supported by hardware?  Seems to me we'll see both a LINE_REQUESTED and a
> LINE_CONFIG_CHANGED when the line is requested.
>

This is problematic for me to test at the moment, as gpiosim doesn't support
debounce. Any chance we could make that configurable?  Similarly drive.

> Conversely, what if a config change impacts features that don't result in a
> notification from gpiod_set_config(), like active low, or emulated
> drive or debounce?
>

Bah, drive is emulated in gpiolib itself, so that should be fine.

When changing config cdev always calls gpiod_direction_input/output(), so I
think that covers the active low case.

But I have a test taking a line from input to output|open_drain and I
get two change events.  The first is the most interesting as it reports
input|open_drain, the second then reports output|open_drain.
That is due to gpiod_direction_output() calling gpiod_set_config() to
set the drive, and later to set the direction, in that order.
Given it will be setting the direction, it should inhibit the event from
the drive setting?

Still haven't tested any debounce changes...

Cheers,
Kent.