[questions] : gpiolib and gpioset behaviour

[questions] : gpiolib and gpioset behaviour

[Gilles BULOZ]

Hi Bartosz,

Several years after our discussions about GPIOs, some things are still unclear
to me.

1 - The gpioset command has this in its help : "Note: the state of a GPIO line
controlled over the character device reverts to default when the last process
referencing the file descriptor representing the device file exits. This means
that it's wrong to run gpioset, have it exit and expect the line to continue
being driven high or low. It may happen if given pin is floating but it must
be interpreted as undefined behavior." But up to now I've never seen such
behaviour and I'm glad to have the GPIO set by gpioset keep their state once
the command exits. Is reverting to default an optional behaviour in the GPIO
chip driver, or in the gpiolib stack ?

2 - I've recently wrote a GPIO driver for an I2C FPGA design having ~112 GPIOs
and wanted to use get_multiple() and set_multiple to have more efficent
accesses, but realized that the line number was limited to 63 because of the
unsigned long mask/bits. But I've noticed that working on a line number >= 64
was unexpectedly calling these methods with a mask at 0 instead of calling
get/set methods, and that the only way to have things working was to not
define get_multiple/set_multiple but only get/set. Is it the expected
behaviour ?
At the end I've split the GPIOs into two banks (first with 64 and second with
48 GPIOs) to be able to use get_multiple/set_multiple.

3 - Is there some way to request a GPIO already owned by another process as
input or output, just to get the current level on the input or the level
driven on output ? This would be much more efficient for real-time
applications than asking the owner such information.

Thanks very much for your help.

Gilles Buloz
Kontron Modular Computers

Re: [questions] : gpiolib and gpioset behaviour

[Bartosz Golaszewski]

On Mon, Apr 22, 2024 at 2:44 PM Gilles BULOZ <gilles.buloz@kontron.com> wrote:
>
> Hi Bartosz,
>
> Several years after our discussions about GPIOs, some things are still unclear
> to me.
>
> 1 - The gpioset command has this in its help : "Note: the state of a GPIO line
> controlled over the character device reverts to default when the last process
> referencing the file descriptor representing the device file exits. This means
> that it's wrong to run gpioset, have it exit and expect the line to continue
> being driven high or low. It may happen if given pin is floating but it must
> be interpreted as undefined behavior." But up to now I've never seen such
> behaviour and I'm glad to have the GPIO set by gpioset keep their state once
> the command exits. Is reverting to default an optional behaviour in the GPIO
> chip driver, or in the gpiolib stack ?
>

This behavior is driver-specific. Meaning: you're in-kernel GPIO
driver may actually retain the state.

> 2 - I've recently wrote a GPIO driver for an I2C FPGA design having ~112 GPIOs
> and wanted to use get_multiple() and set_multiple to have more efficent
> accesses, but realized that the line number was limited to 63 because of the
> unsigned long mask/bits. But I've noticed that working on a line number >= 64
> was unexpectedly calling these methods with a mask at 0 instead of calling
> get/set methods, and that the only way to have things working was to not
> define get_multiple/set_multiple but only get/set. Is it the expected
> behaviour ?
> At the end I've split the GPIOs into two banks (first with 64 and second with
> 48 GPIOs) to be able to use get_multiple/set_multiple.
>

Please use libgpiod v2. That won't help you with the max requested
line limit but at least it's more modern API and actively developed.

> 3 - Is there some way to request a GPIO already owned by another process as
> input or output, just to get the current level on the input or the level
> driven on output ? This would be much more efficient for real-time
> applications than asking the owner such information.
>

Ha! Please help me help you. Take a look at the DBus daemon I recently
posted[1]. With the daemon running, the behavior will be exactly what
you expect. You'll be able to get/set values and have the command-line
tool exit while the daemon retains the state.

Bart

[1] https://lore.kernel.org/linux-gpio/20240412122804.109323-1-brgl@bgdev.pl/

Re: [questions] : gpiolib and gpioset behaviour

[Gilles BULOZ]

On Mon, Apr 22, 2024 at 3:55 PM Bartosz Golaszewski wrote :
> On Mon, Apr 22, 2024 at 2:44 PM Gilles BULOZ <gilles.buloz@kontron.com> wrote:
>>
>> Hi Bartosz,
>>
>> Several years after our discussions about GPIOs, some things are still unclear
>> to me.
>>
>> 1 - The gpioset command has this in its help : "Note: the state of a GPIO line
>> controlled over the character device reverts to default when the last process
>> referencing the file descriptor representing the device file exits. This means
>> that it's wrong to run gpioset, have it exit and expect the line to continue
>> being driven high or low. It may happen if given pin is floating but it must
>> be interpreted as undefined behavior." But up to now I've never seen such
>> behaviour and I'm glad to have the GPIO set by gpioset keep their state once
>> the command exits. Is reverting to default an optional behaviour in the GPIO
>> chip driver, or in the gpiolib stack ?
>>
>
> This behavior is driver-specific. Meaning: you're in-kernel GPIO
> driver may actually retain the state.
>

Which method should the driver implement to restore the state on GPIO when the
last process referencing the character device exits ?

>> 2 - I've recently wrote a GPIO driver for an I2C FPGA design having ~112 GPIOs
>> and wanted to use get_multiple() and set_multiple to have more efficent
>> accesses, but realized that the line number was limited to 63 because of the
>> unsigned long mask/bits. But I've noticed that working on a line number >= 64
>> was unexpectedly calling these methods with a mask at 0 instead of calling
>> get/set methods, and that the only way to have things working was to not
>> define get_multiple/set_multiple but only get/set. Is it the expected
>> behaviour ?
>> At the end I've split the GPIOs into two banks (first with 64 and second with
>> 48 GPIOs) to be able to use get_multiple/set_multiple.
>>
>
> Please use libgpiod v2. That won't help you with the max requested
> line limit but at least it's more modern API and actively developed.
>

OK

>> 3 - Is there some way to request a GPIO already owned by another process as
>> input or output, just to get the current level on the input or the level
>> driven on output ? This would be much more efficient for real-time
>> applications than asking the owner such information.
>>
>
> Ha! Please help me help you. Take a look at the DBus daemon I recently
> posted[1]. With the daemon running, the behavior will be exactly what
> you expect. You'll be able to get/set values and have the command-line
> tool exit while the daemon retains the state.
>

I was thinking about some specific "watcher" ioctl to do so, not a DBus
daemon because this is not welcome in the real-time and embedded world.
The only workaround I've found is to directly read the GPIO chip registers
but this is bad to do so.

> Bart
>
> [1] https://lore.kernel.org/linux-gpio/20240412122804.109323-1-brgl@bgdev.pl/
> .

Re: [questions] : gpiolib and gpioset behaviour

[Bartosz Golaszewski]

On Mon, Apr 22, 2024 at 6:49 PM Gilles BULOZ <gilles.buloz@kontron.com> wrote:
>
> On Mon, Apr 22, 2024 at 3:55 PM Bartosz Golaszewski wrote :
> >
> > This behavior is driver-specific. Meaning: you're in-kernel GPIO
> > driver may actually retain the state.
> >
>
> Which method should the driver implement to restore the state on GPIO when the
> last process referencing the character device exits ?
>

The free callback in struct gpio_chip.

Bart

Re: [questions] : gpiolib and gpioset behaviour

[Kent Gibson]

On Mon, Apr 22, 2024 at 06:49:05PM +0200, Gilles BULOZ wrote:
> On Mon, Apr 22, 2024 at 3:55 PM Bartosz Golaszewski wrote :
> > On Mon, Apr 22, 2024 at 2:44 PM Gilles BULOZ <gilles.buloz@kontron.com> wrote:
> >>
> >> Hi Bartosz,
> >>
> >> Several years after our discussions about GPIOs, some things are still unclear
> >> to me.
> >>
> >> 1 - The gpioset command has this in its help : "Note: the state of a GPIO line
> >> controlled over the character device reverts to default when the last process
> >> referencing the file descriptor representing the device file exits. This means
> >> that it's wrong to run gpioset, have it exit and expect the line to continue
> >> being driven high or low. It may happen if given pin is floating but it must
> >> be interpreted as undefined behavior." But up to now I've never seen such
> >> behaviour and I'm glad to have the GPIO set by gpioset keep their state once
> >> the command exits. Is reverting to default an optional behaviour in the GPIO
> >> chip driver, or in the gpiolib stack ?
> >>
> >
> > This behavior is driver-specific. Meaning: you're in-kernel GPIO
> > driver may actually retain the state.
> >
>
> Which method should the driver implement to restore the state on GPIO when the
> last process referencing the character device exits ?
>
> >> 2 - I've recently wrote a GPIO driver for an I2C FPGA design having ~112 GPIOs
> >> and wanted to use get_multiple() and set_multiple to have more efficent
> >> accesses, but realized that the line number was limited to 63 because of the
> >> unsigned long mask/bits. But I've noticed that working on a line number >= 64
> >> was unexpectedly calling these methods with a mask at 0 instead of calling
> >> get/set methods, and that the only way to have things working was to not
> >> define get_multiple/set_multiple but only get/set. Is it the expected
> >> behaviour ?
> >> At the end I've split the GPIOs into two banks (first with 64 and second with
> >> 48 GPIOs) to be able to use get_multiple/set_multiple.
> >>
> >
> > Please use libgpiod v2. That won't help you with the max requested
> > line limit but at least it's more modern API and actively developed.
> >
>

I suspect you are referring to gpiolib here - the mask in gc->get_multiple()
being unsigned long*.

The uAPI that libgpiod uses is limited to 64 lines per request, but that is
only relevant if you want to request more than 64 lines at once from userspace
(you would have to break that into two requests to access all 112 lines).

Note that the mask in gc->get_multiple() is unsigned long*, so it is a
pointer to an array of unsigned long.  Its width is not limited by
unsigned long, but by the bits parameter.  In your case the mask you pass
should contain multiple unsigned longs to achieve 112 bits.
Refer to gpiod_get_array_value_complex() for an example of building bitmap
masks to pass to gc->get_multiple(), in that case via
gpio_chip_get_multiple().

> OK
>
> >> 3 - Is there some way to request a GPIO already owned by another process as
> >> input or output, just to get the current level on the input or the level
> >> driven on output ? This would be much more efficient for real-time
> >> applications than asking the owner such information.
> >>
> >
> > Ha! Please help me help you. Take a look at the DBus daemon I recently
> > posted[1]. With the daemon running, the behavior will be exactly what
> > you expect. You'll be able to get/set values and have the command-line
> > tool exit while the daemon retains the state.
> >
>
> I was thinking about some specific "watcher" ioctl to do so, not a DBus
> daemon because this is not welcome in the real-time and embedded world.
> The only workaround I've found is to directly read the GPIO chip registers
> but this is bad to do so.
>

No there isn't, and I can't say I'm a fan of using GPIOs as shared memory,
though it does seems to be a common use case for those accustomed to accessing
hardware registers directly.

I would question whether "much more efficient" is true, as going through all
the gpiolib machinery, including the device driver, to perform the get could
even turn out to be slower than some IPC options - such as actual shared
memory.

Cheers,
Kent.

Re: [questions] : gpiolib and gpioset behaviour

[Gilles BULOZ]

On Fri, Apr 26, 2024 at 04:07 AM, Kent Gibson wrote :
> On Mon, Apr 22, 2024 at 06:49:05PM +0200, Gilles BULOZ wrote:
>> On Mon, Apr 22, 2024 at 3:55 PM Bartosz Golaszewski wrote :
>>> On Mon, Apr 22, 2024 at 2:44 PM Gilles BULOZ <gilles.buloz@kontron.com> wrote:
>>>>
>>>> Hi Bartosz,
>>>>
>>>> Several years after our discussions about GPIOs, some things are still unclear
>>>> to me.
>>>>
>>>> 1 - The gpioset command has this in its help : "Note: the state of a GPIO line
>>>> controlled over the character device reverts to default when the last process
>>>> referencing the file descriptor representing the device file exits. This means
>>>> that it's wrong to run gpioset, have it exit and expect the line to continue
>>>> being driven high or low. It may happen if given pin is floating but it must
>>>> be interpreted as undefined behavior." But up to now I've never seen such
>>>> behaviour and I'm glad to have the GPIO set by gpioset keep their state once
>>>> the command exits. Is reverting to default an optional behaviour in the GPIO
>>>> chip driver, or in the gpiolib stack ?
>>>>
>>>
>>> This behavior is driver-specific. Meaning: you're in-kernel GPIO
>>> driver may actually retain the state.
>>>
>>
>> Which method should the driver implement to restore the state on GPIO when the
>> last process referencing the character device exits ?
>>
>>>> 2 - I've recently wrote a GPIO driver for an I2C FPGA design having ~112 GPIOs
>>>> and wanted to use get_multiple() and set_multiple to have more efficent
>>>> accesses, but realized that the line number was limited to 63 because of the
>>>> unsigned long mask/bits. But I've noticed that working on a line number >= 64
>>>> was unexpectedly calling these methods with a mask at 0 instead of calling
>>>> get/set methods, and that the only way to have things working was to not
>>>> define get_multiple/set_multiple but only get/set. Is it the expected
>>>> behaviour ?
>>>> At the end I've split the GPIOs into two banks (first with 64 and second with
>>>> 48 GPIOs) to be able to use get_multiple/set_multiple.
>>>>
>>>
>>> Please use libgpiod v2. That won't help you with the max requested
>>> line limit but at least it's more modern API and actively developed.
>>>
>>
>
> I suspect you are referring to gpiolib here - the mask in gc->get_multiple()
> being unsigned long*.
>
> The uAPI that libgpiod uses is limited to 64 lines per request, but that is
> only relevant if you want to request more than 64 lines at once from userspace
> (you would have to break that into two requests to access all 112 lines).
>
> Note that the mask in gc->get_multiple() is unsigned long*, so it is a
> pointer to an array of unsigned long.  Its width is not limited by
> unsigned long, but by the bits parameter.  In your case the mask you pass
> should contain multiple unsigned longs to achieve 112 bits.
> Refer to gpiod_get_array_value_complex() for an example of building bitmap
> masks to pass to gc->get_multiple(), in that case via
> gpio_chip_get_multiple().
>

I was refering the get_multiple/set_multiple callbacks in struct gpio_chip
that are defined like this :
 int (*get_multiple)(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits);
 void (*set_multiple)(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits);
With some debug in my GPIO chip driver implementing these functions, I saw that
the bits set in "mask" and the ones used in "bits" are the ones whose bit
numbers are directly matching the GPIO offset/line numbers of the chip. But I
only used unsigned long, not arrays, so I thought I was limited to offset/line
31 on 32bit arch, and 63 on 64bit arch.
As you suggested, I'm currently having a look to gpiod_get_array_value_complex()
but I must admit I'm currently a little bit lost. I've never thought GPIO
implementation could become so complex for my brain :-)

>> OK
>>
>>>> 3 - Is there some way to request a GPIO already owned by another process as
>>>> input or output, just to get the current level on the input or the level
>>>> driven on output ? This would be much more efficient for real-time
>>>> applications than asking the owner such information.
>>>>
>>>
>>> Ha! Please help me help you. Take a look at the DBus daemon I recently
>>> posted[1]. With the daemon running, the behavior will be exactly what
>>> you expect. You'll be able to get/set values and have the command-line
>>> tool exit while the daemon retains the state.
>>>
>>
>> I was thinking about some specific "watcher" ioctl to do so, not a DBus
>> daemon because this is not welcome in the real-time and embedded world.
>> The only workaround I've found is to directly read the GPIO chip registers
>> but this is bad to do so.
>>
>
> No there isn't, and I can't say I'm a fan of using GPIOs as shared memory,
> though it does seems to be a common use case for those accustomed to accessing
> hardware registers directly.
>

As a workaround, using debugfs in the driver to report the current state of
GPIOs to all processes requiring them is probably cleaner that doing a direct
access to the register. But this is not as clean as a specific "watcher"
ioctl could be.

> I would question whether "much more efficient" is true, as going through all
> the gpiolib machinery, including the device driver, to perform the get could
> even turn out to be slower than some IPC options - such as actual shared
> memory.
>

Yes, you're probably right. "more efficient" was refering to the less you have
layers the more is is supposed to be fast, but there's still some machinery
to go through.

> Cheers,
> Kent.
> .

Thanks a lot,
Gilles
.

Re: [questions] : gpiolib and gpioset behaviour

[Kent Gibson]

On Fri, Apr 26, 2024 at 03:08:33PM +0200, Gilles BULOZ wrote:
> On Fri, Apr 26, 2024 at 04:07 AM, Kent Gibson wrote :
> > On Mon, Apr 22, 2024 at 06:49:05PM +0200, Gilles BULOZ wrote:
> >> On Mon, Apr 22, 2024 at 3:55 PM Bartosz Golaszewski wrote :
> >>> On Mon, Apr 22, 2024 at 2:44 PM Gilles BULOZ <gilles.buloz@kontron.com> wrote:
> >>>>

> >>
> >
> > I suspect you are referring to gpiolib here - the mask in gc->get_multiple()
> > being unsigned long*.
> >
> > The uAPI that libgpiod uses is limited to 64 lines per request, but that is
> > only relevant if you want to request more than 64 lines at once from userspace
> > (you would have to break that into two requests to access all 112 lines).
> >
> > Note that the mask in gc->get_multiple() is unsigned long*, so it is a
> > pointer to an array of unsigned long.  Its width is not limited by
> > unsigned long, but by the bits parameter.  In your case the mask you pass
> > should contain multiple unsigned longs to achieve 112 bits.
> > Refer to gpiod_get_array_value_complex() for an example of building bitmap
> > masks to pass to gc->get_multiple(), in that case via
> > gpio_chip_get_multiple().
> >
>
> I was refering the get_multiple/set_multiple callbacks in struct gpio_chip
> that are defined like this :
>  int (*get_multiple)(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits);
>  void (*set_multiple)(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits);
> With some debug in my GPIO chip driver implementing these functions, I saw that
> the bits set in "mask" and the ones used in "bits" are the ones whose bit
> numbers are directly matching the GPIO offset/line numbers of the chip. But I
> only used unsigned long, not arrays, so I thought I was limited to offset/line
> 31 on 32bit arch, and 63 on 64bit arch.
> As you suggested, I'm currently having a look to gpiod_get_array_value_complex()
> but I must admit I'm currently a little bit lost. I've never thought GPIO
> implementation could become so complex for my brain :-)
>

The bit of primary interest that I was referring to was the DECLARE_BITMAP()
as used for the fastpath mask:

		DECLARE_BITMAP(fastpath_mask, FASTPATH_NGPIO);

That does the sizing math for you.  In your case you would use 112 for
the NGPIO.  There are also examples of using __set_bit() to set bits in
the mask.  Take a look in linux/bitmaps.h and linux/bitops.h for the
relevant definitions if you want to dig deeper.

And, yeah, the amount of work that goes into just driving physical
lines up and down, fundamentally just toggling bits, frequently makes my brain
hurt too ;-).

Cheers,
Kent.

Re: [questions] : gpiolib and gpioset behaviour

[Gilles BULOZ]

On Fri, Apr 26, 2024 at 03:37 PM, Kent Gibson wrote :
> On Fri, Apr 26, 2024 at 03:08:33PM +0200, Gilles BULOZ wrote:
>> On Fri, Apr 26, 2024 at 04:07 AM, Kent Gibson wrote :
>>> On Mon, Apr 22, 2024 at 06:49:05PM +0200, Gilles BULOZ wrote:
>>>> On Mon, Apr 22, 2024 at 3:55 PM Bartosz Golaszewski wrote :
>>>>> On Mon, Apr 22, 2024 at 2:44 PM Gilles BULOZ <gilles.buloz@kontron.com> wrote:
>>>>>>
>
>>>>
>>>
>>> I suspect you are referring to gpiolib here - the mask in gc->get_multiple()
>>> being unsigned long*.
>>>
>>> The uAPI that libgpiod uses is limited to 64 lines per request, but that is
>>> only relevant if you want to request more than 64 lines at once from userspace
>>> (you would have to break that into two requests to access all 112 lines).
>>>
>>> Note that the mask in gc->get_multiple() is unsigned long*, so it is a
>>> pointer to an array of unsigned long.  Its width is not limited by
>>> unsigned long, but by the bits parameter.  In your case the mask you pass
>>> should contain multiple unsigned longs to achieve 112 bits.
>>> Refer to gpiod_get_array_value_complex() for an example of building bitmap
>>> masks to pass to gc->get_multiple(), in that case via
>>> gpio_chip_get_multiple().
>>>
>>
>> I was refering the get_multiple/set_multiple callbacks in struct gpio_chip
>> that are defined like this :
>>  int (*get_multiple)(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits);
>>  void (*set_multiple)(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits);
>> With some debug in my GPIO chip driver implementing these functions, I saw that
>> the bits set in "mask" and the ones used in "bits" are the ones whose bit
>> numbers are directly matching the GPIO offset/line numbers of the chip. But I
>> only used unsigned long, not arrays, so I thought I was limited to offset/line
>> 31 on 32bit arch, and 63 on 64bit arch.
>> As you suggested, I'm currently having a look to gpiod_get_array_value_complex()
>> but I must admit I'm currently a little bit lost. I've never thought GPIO
>> implementation could become so complex for my brain :-)
>>
>
> The bit of primary interest that I was referring to was the DECLARE_BITMAP()
> as used for the fastpath mask:
>
>                 DECLARE_BITMAP(fastpath_mask, FASTPATH_NGPIO);
>
> That does the sizing math for you.  In your case you would use 112 for
> the NGPIO.  There are also examples of using __set_bit() to set bits in
> the mask.  Take a look in linux/bitmaps.h and linux/bitops.h for the
> relevant definitions if you want to dig deeper.
>
> And, yeah, the amount of work that goes into just driving physical
> lines up and down, fundamentally just toggling bits, frequently makes my brain
> hurt too ;-).
>

Thanks very much for the tips !
I've added some debug in my GPIO chip driver in get_multiple()/set_multiple()
and clearly see that when using an unsigned long array for "mask" and "bits",
the second unsigned long in array is used starting from GPIO offset/line 64,
starting from bit 0, then 1 for offset/line 65...
For instance when I run "gpioget 0 65", get_multiple() is called with bit 1 of
mask[1] set and I return the level on bit 1 of bits[1] that is correctly
reported by gpioget.

But get_multiple is called by gpio_chip_get_multiple() than is called by
gpiod_get_array_value_complex() that is called either from 
 linehandle_ioctl() for GPIOHANDLE_GET_LINE_VALUES_IOCTL where we have DECLARE_BITMAP(vals, GPIOHANDLES_MAX);
 linereq_get_values() for GPIO_V2_LINE_GET_VALUES_IOCTL  where we have DECLARE_BITMAP(vals, GPIO_V2_LINES_MAX);
so both vals with 64bits (GPIOHANDLES_MAX=GPIO_V2_LINES_MAX=64) so only one unsigned long.
But vals is passed as last argument unsigned long *value_bitmap to
gpiod_get_array_value_complex() that is passed as last argument
unsigned long *bits to gpio_chip_get_multiple() that
is passed as last argument unsigned long *bits to get_multiple() of my driver
where I'm supposed to fill data in the second unsigned logn of array for GPIO
offset/line >= 64 where we have only allocated one.
I'm probably wrong somewhere.

> Cheers,
> Kent.
> .

Re: [questions] : gpiolib and gpioset behaviour

[Kent Gibson]

On Fri, Apr 26, 2024 at 06:16:28PM +0200, Gilles BULOZ wrote:
> On Fri, Apr 26, 2024 at 03:37 PM, Kent Gibson wrote :
> > On Fri, Apr 26, 2024 at 03:08:33PM +0200, Gilles BULOZ wrote:
> >> On Fri, Apr 26, 2024 at 04:07 AM, Kent Gibson wrote :
> >>> On Mon, Apr 22, 2024 at 06:49:05PM +0200, Gilles BULOZ wrote:
> >>>> On Mon, Apr 22, 2024 at 3:55 PM Bartosz Golaszewski wrote :
> >>>>> On Mon, Apr 22, 2024 at 2:44 PM Gilles BULOZ <gilles.buloz@kontron.com> wrote:
> >>>>>>
> >
> >>>>
> >>>
> >>> I suspect you are referring to gpiolib here - the mask in gc->get_multiple()
> >>> being unsigned long*.
> >>>
> >>> The uAPI that libgpiod uses is limited to 64 lines per request, but that is
> >>> only relevant if you want to request more than 64 lines at once from userspace
> >>> (you would have to break that into two requests to access all 112 lines).
> >>>
> >>> Note that the mask in gc->get_multiple() is unsigned long*, so it is a
> >>> pointer to an array of unsigned long.  Its width is not limited by
> >>> unsigned long, but by the bits parameter.  In your case the mask you pass
> >>> should contain multiple unsigned longs to achieve 112 bits.
> >>> Refer to gpiod_get_array_value_complex() for an example of building bitmap
> >>> masks to pass to gc->get_multiple(), in that case via
> >>> gpio_chip_get_multiple().
> >>>
> >>
> >> I was refering the get_multiple/set_multiple callbacks in struct gpio_chip
> >> that are defined like this :
> >>  int (*get_multiple)(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits);
> >>  void (*set_multiple)(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits);
> >> With some debug in my GPIO chip driver implementing these functions, I saw that
> >> the bits set in "mask" and the ones used in "bits" are the ones whose bit
> >> numbers are directly matching the GPIO offset/line numbers of the chip. But I
> >> only used unsigned long, not arrays, so I thought I was limited to offset/line
> >> 31 on 32bit arch, and 63 on 64bit arch.
> >> As you suggested, I'm currently having a look to gpiod_get_array_value_complex()
> >> but I must admit I'm currently a little bit lost. I've never thought GPIO
> >> implementation could become so complex for my brain :-)
> >>
> >
> > The bit of primary interest that I was referring to was the DECLARE_BITMAP()
> > as used for the fastpath mask:
> >
> >                 DECLARE_BITMAP(fastpath_mask, FASTPATH_NGPIO);
> >
> > That does the sizing math for you.  In your case you would use 112 for
> > the NGPIO.  There are also examples of using __set_bit() to set bits in
> > the mask.  Take a look in linux/bitmaps.h and linux/bitops.h for the
> > relevant definitions if you want to dig deeper.
> >
> > And, yeah, the amount of work that goes into just driving physical
> > lines up and down, fundamentally just toggling bits, frequently makes my brain
> > hurt too ;-).
> >
>
> Thanks very much for the tips !
> I've added some debug in my GPIO chip driver in get_multiple()/set_multiple()
> and clearly see that when using an unsigned long array for "mask" and "bits",
> the second unsigned long in array is used starting from GPIO offset/line 64,
> starting from bit 0, then 1 for offset/line 65...
> For instance when I run "gpioget 0 65", get_multiple() is called with bit 1 of
> mask[1] set and I return the level on bit 1 of bits[1] that is correctly
> reported by gpioget.
>
> But get_multiple is called by gpio_chip_get_multiple() than is called by
> gpiod_get_array_value_complex() that is called either from
>  linehandle_ioctl() for GPIOHANDLE_GET_LINE_VALUES_IOCTL where we have DECLARE_BITMAP(vals, GPIOHANDLES_MAX);
>  linereq_get_values() for GPIO_V2_LINE_GET_VALUES_IOCTL  where we have DECLARE_BITMAP(vals, GPIO_V2_LINES_MAX);
> so both vals with 64bits (GPIOHANDLES_MAX=GPIO_V2_LINES_MAX=64) so only one unsigned long.
> But vals is passed as last argument unsigned long *value_bitmap to
> gpiod_get_array_value_complex() that is passed as last argument
> unsigned long *bits to gpio_chip_get_multiple() that
> is passed as last argument unsigned long *bits to get_multiple() of my driver
> where I'm supposed to fill data in the second unsigned logn of array for GPIO
> offset/line >= 64 where we have only allocated one.
> I'm probably wrong somewhere.
>

get_multiple() is always passed masks/values that are (at least) ngpios wide.

gpiod_get_array_value_complex() builds a new sparse mask to match the ngpios
for the device, using the condensed array of descs it is passed.
Similarly it packs the values returned by get_multiple() into a bitmap
corresponding to the array of descs.  That is the function of
gpiod_get_array_value_complex() - to translate between a set of
arbitrary lines (descs) to the form expected by the driver.

In the case of cdev, which provides the userspace interface, that set of
arbitrary lines is limited to 64, but that has nothing to do with how many
values can be requested from the device at once from kernel space - you
can request them all.

Cheers,
Kent.

Re: [questions] : gpiolib and gpioset behaviour

[Kent Gibson]

On Fri, Apr 26, 2024 at 10:07:20AM +0800, Kent Gibson wrote:
> On Mon, Apr 22, 2024 at 06:49:05PM +0200, Gilles BULOZ wrote:
> > On Mon, Apr 22, 2024 at 3:55 PM Bartosz Golaszewski wrote :
> > > On Mon, Apr 22, 2024 at 2:44 PM Gilles BULOZ <gilles.buloz@kontron.com> wrote:
> > >>
>
> Note that the mask in gc->get_multiple() is unsigned long*, so it is a
> pointer to an array of unsigned long.  Its width is not limited by
> unsigned long, but by the bits parameter.  In your case the mask you pass
> should contain multiple unsigned longs to achieve 112 bits.
> Refer to gpiod_get_array_value_complex() for an example of building bitmap
> masks to pass to gc->get_multiple(), in that case via
> gpio_chip_get_multiple().
>

Bah, what was I saying here - both the mask AND bits parameters of
get_multiple()/set_multiple() are bitmaps of width gc->ngpio, where the
mask identifies the pins to get/set and bits contains the values.

My bad - must've been before the coffee soaked in.

Cheers,
Kent.

Re: [questions] : gpiolib and gpioset behaviour

[Gilles BULOZ]

On Sat, Apr 27, 2024 at 02:09 PM Kent Gibson wrote:
> On Fri, Apr 26, 2024 at 10:07:20AM +0800, Kent Gibson wrote:
>> On Mon, Apr 22, 2024 at 06:49:05PM +0200, Gilles BULOZ wrote:
>>> On Mon, Apr 22, 2024 at 3:55 PM Bartosz Golaszewski wrote :
>>>> On Mon, Apr 22, 2024 at 2:44 PM Gilles BULOZ <gilles.buloz@kontron.com> wrote:
>>>>>
>>
>> Note that the mask in gc->get_multiple() is unsigned long*, so it is a
>> pointer to an array of unsigned long.  Its width is not limited by
>> unsigned long, but by the bits parameter.  In your case the mask you pass
>> should contain multiple unsigned longs to achieve 112 bits.
>> Refer to gpiod_get_array_value_complex() for an example of building bitmap
>> masks to pass to gc->get_multiple(), in that case via
>> gpio_chip_get_multiple().
>>
>
> Bah, what was I saying here - both the mask AND bits parameters of
> get_multiple()/set_multiple() are bitmaps of width gc->ngpio, where the
> mask identifies the pins to get/set and bits contains the values.
>
> My bad - must've been before the coffee soaked in.
>

Yes, that's clear to me now thanks to your explainations.
I also understand now the allocation of these "mask" and "bits". I was
missing the fact that gpiod_get_array_value_complex() has two calls to
gpio_chip_get_multiple(), the first without new allocation (value_bitmap
directly passed as last parameter to gpio_chip_get_multiple() and the
second allocating "mask" and "bits" (on FASTPATH_NGPIO bits so 512 bits
by default as set by CONFIG_GPIOLIB_FASTPATH_LIMIT). The second call
is the one used in the path from ioctl GPIO_V2_LINE_GET_VALUES_IOCTL to
the driver so OK.
The same occurs for gpiod_set_array_value_complex() having two calls to
gpio_chip_set_multiple() where the second one is used for ioctl
GPIO_V2_LINE_SET_VALUES_IOCTL.

> Cheers,
> Kent.
>
> .