Re: [PATCH v5 09/12] gpio: aggregator: handle runtime registration of gpio_desc in gpiochip_fwd

[Thomas Richard <thomas.richard@bootlin.com>]

Hi Andy,

Thanks again for the review.

On 5/7/25 08:34, Andy Shevchenko wrote:
> On Tue, May 6, 2025 at 6:21 PM Thomas Richard
> <thomas.richard@bootlin.com> wrote:
>>
>> Add request() callback to check if the GPIO descriptor was well registered
>> in the gpiochip_fwd before using it. This is done to handle the case where
>> GPIO descriptor is added at runtime in the forwarder.
>>
>> If at least one GPIO descriptor was not added before the forwarder
>> registration, we assume the forwarder can sleep as if a GPIO is added at
>> runtime it may sleep.
> 
> ...
> 
>>  {
>>         struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
>>
>> +       /*
>> +        * get_direction() is called during gpiochip registration, return input
>> +        * direction if there is no descriptor for the line.
>> +        */
>> +       if (!test_bit(offset, fwd->valid_mask))
>> +               return GPIO_LINE_DIRECTION_IN;
> 
> Can you remind me why we choose a valid return for invalid line? From
> a pure code perspective this should return an error.

I reproduced gpiolib behavior. During gpiochip registration, we get the
direction of all lines. In the case the line is not valid, it is marked
as input if direction_input operation exists, otherwise it is marked as
output. [1]

But in fact we could return an error and the core will mark the line as
input. Maybe ENODEV ?

[1]
https://elixir.bootlin.com/linux/v6.15-rc5/source/drivers/gpio/gpiolib.c#L1105-L1123

Regards,

Thomas