gpio: gpio-pci-idio-16 regression after LTS upgrade

gpio: gpio-pci-idio-16 regression after LTS upgrade

[Mark Cave-Ayland]

Hi all,

As part of our internal vfio testing here at Nutanix we make use of an 
emulated gpio-pci-idio-16 device. Recently we've upgraded our LTS kernel 
and found that the device stops working which I've bisected down to this 
commit:

$ git bisect bad
73d8f3efc5c2b757ab06685741df01eaed8090c4 is the first bad commit
commit 73d8f3efc5c2b757ab06685741df01eaed8090c4
Author: William Breathitt Gray <william.gray@linaro.org>
Date:   Thu Aug 10 18:00:40 2023 -0400

     gpio: pci-idio-16: Migrate to the regmap API

     The regmap API supports IO port accessors so we can take advantage of
     regmap abstractions rather than handling access to the device registers
     directly in the driver. Migrate the pci-idio-16 module to the new
     idio-16 library interface leveraging the gpio-regmap API.

     Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
     Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
     Link: 
https://lore.kernel.org/r/5ba5405c64aca984d5cf3bdbdffa04c325e5a147.1680618405.git.william.gray@linaro.org/
     Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
     Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
     Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

  drivers/gpio/Kconfig            |   2 +-
  drivers/gpio/gpio-pci-idio-16.c | 294 
+++++++++-------------------------------
  2 files changed, 62 insertions(+), 234 deletions(-)


With this commit when the gpio-pci-idio16 module fails upon load as below:

[    0.266606] pci-idio-16 0000:00:03.0: error -EINVAL: Unable to 
initialize register map
[    0.266867] pci-idio-16: probe of 0000:00:03.0 failed with error -22

After some more debugging I was able to determine that the failure was 
due to the regmap cache failing initialisation in 
drivers/base/regmap/regcache-flat.c::regcache_flat_init() because 
max_register wasn't set on the regmap. I was able to fix that fairly 
easily with this:


diff --git a/drivers/gpio/gpio-pci-idio-16.c 
b/drivers/gpio/gpio-pci-idio-16.c
index 44c0a21b1d1d..55be571b5cca 100644
--- a/drivers/gpio/gpio-pci-idio-16.c
+++ b/drivers/gpio/gpio-pci-idio-16.c
@@ -41,6 +41,7 @@ static const struct regmap_config 
idio_16_regmap_config = {
         .reg_stride = 1,
         .val_bits = 8,
         .io_port = true,
+  .max_register = 0x7,
         .wr_table = &idio_16_wr_table,
         .rd_table = &idio_16_rd_table,
         .volatile_table = &idio_16_rd_table,


Okay so the module now loads, but all of my gpios are now inputs!

root@debian12:~# gpioinfo
gpiochip0 - 32 lines:
     line   0:       "OUT0"       unused   input  active-high
     line   1:       "OUT1"       unused   input  active-high
     line   2:       "OUT2"       unused   input  active-high
     line   3:       "OUT3"       unused   input  active-high
     line   4:       "OUT4"       unused   input  active-high
     line   5:       "OUT5"       unused   input  active-high
     line   6:       "OUT6"       unused   input  active-high
     line   7:       "OUT7"       unused   input  active-high
     line   8:       "OUT8"       unused   input  active-high
     line   9:       "OUT9"       unused   input  active-high
     line  10:      "OUT10"       unused   input  active-high
     line  11:      "OUT11"       unused   input  active-high
     line  12:      "OUT12"       unused   input  active-high
     line  13:      "OUT13"       unused   input  active-high
     line  14:      "OUT14"       unused   input  active-high
     line  15:      "OUT15"       unused   input  active-high
     line  16:       "IIN0"       unused   input  active-high
     line  17:       "IIN1"       unused   input  active-high
     line  18:       "IIN2"       unused   input  active-high
     line  19:       "IIN3"       unused   input  active-high
     line  20:       "IIN4"       unused   input  active-high
     line  21:       "IIN5"       unused   input  active-high
     line  22:       "IIN6"       unused   input  active-high
     line  23:       "IIN7"       unused   input  active-high
     line  24:       "IIN8"       unused   input  active-high
     line  25:       "IIN9"       unused   input  active-high
     line  26:      "IIN10"       unused   input  active-high
     line  27:      "IIN11"       unused   input  active-high
     line  28:      "IIN12"       unused   input  active-high
     line  29:      "IIN13"       unused   input  active-high
     line  30:      "IIN14"       unused   input  active-high
     line  31:      "IIN15"       unused   input  active-high

root@debian12:~# gpioget 0 0
gpioget: error reading GPIO values: Input/output error

which also output:

[  329.529321] gpio-512 (gpioget): gpiod_direction_input: missing 
direction_input() operation and line is output

My guess is that this is because 
drivers/gpio/gpio-regmap.c::gpio_regmap_get_direction() isn't able to 
can't handle the situation where lines 0-15 are outputs and lines 16-31 
are inputs, compared with the old idio_16_gpio_get_direction() function 
it replaced.

What would be the best way forward? Possibly add the .get_direction 
callback to the gpio_regmap_config? Or is there another way to have 
mixed inputs and outputs with the gpio_regmap API?


Many thanks,

Mark.

Re: gpio: gpio-pci-idio-16 regression after LTS upgrade

[William Breathitt Gray]

On Mon, Oct 06, 2025 at 09:37:14AM +0100, Mark Cave-Ayland wrote:
> root@debian12:~# gpioget 0 0
> gpioget: error reading GPIO values: Input/output error
> 
> which also output:
> 
> [  329.529321] gpio-512 (gpioget): gpiod_direction_input: missing
> direction_input() operation and line is output
> 
> My guess is that this is because
> drivers/gpio/gpio-regmap.c::gpio_regmap_get_direction() isn't able to
> can't handle the situation where lines 0-15 are outputs and lines 16-31
> are inputs, compared with the old idio_16_gpio_get_direction() function
> it replaced.
> 
> What would be the best way forward? Possibly add the .get_direction
> callback to the gpio_regmap_config? Or is there another way to have
> mixed inputs and outputs with the gpio_regmap API?

So the intention I had with gpio-idio-16 was to provide reg_dat_base and
reg_set_base to define the input and output bases, and then
reg_mask_xlate would do the translation between input and outputs. I
think this design is allowed by gpio-regmap, is it not Michael?

In theory, gpio_regmap_get_direction should call gpio->reg_mask_xlate()
which is mapped to idio_16_reg_mask_xlate(), and thus set reg and mask
which then is evaluated at the end of gpio_regmap_get_direction() to
determine which direction to return.

Is it possible idio_16_reg_mask_xlate() is returning the wrong values
for reg and mask?

William Breathitt Gray

Re: gpio: gpio-pci-idio-16 regression after LTS upgrade

[Mark Cave-Ayland]

On 07/10/2025 09:16, William Breathitt Gray wrote:

> On Mon, Oct 06, 2025 at 09:37:14AM +0100, Mark Cave-Ayland wrote:
>> root@debian12:~# gpioget 0 0
>> gpioget: error reading GPIO values: Input/output error
>>
>> which also output:
>>
>> [  329.529321] gpio-512 (gpioget): gpiod_direction_input: missing
>> direction_input() operation and line is output
>>
>> My guess is that this is because
>> drivers/gpio/gpio-regmap.c::gpio_regmap_get_direction() isn't able to
>> can't handle the situation where lines 0-15 are outputs and lines 16-31
>> are inputs, compared with the old idio_16_gpio_get_direction() function
>> it replaced.
>>
>> What would be the best way forward? Possibly add the .get_direction
>> callback to the gpio_regmap_config? Or is there another way to have
>> mixed inputs and outputs with the gpio_regmap API?
> 
> So the intention I had with gpio-idio-16 was to provide reg_dat_base and
> reg_set_base to define the input and output bases, and then
> reg_mask_xlate would do the translation between input and outputs. I
> think this design is allowed by gpio-regmap, is it not Michael?
> 
> In theory, gpio_regmap_get_direction should call gpio->reg_mask_xlate()
> which is mapped to idio_16_reg_mask_xlate(), and thus set reg and mask
> which then is evaluated at the end of gpio_regmap_get_direction() to
> determine which direction to return.
> 
> Is it possible idio_16_reg_mask_xlate() is returning the wrong values
> for reg and mask?
> 
> William Breathitt Gray

The only logic around .reg_dat_base and .reg_set_base in 
gpio_regmap_get_direction() is this:

	if (gpio->reg_dat_base && !gpio->reg_set_base)
		return GPIO_LINE_DIRECTION_IN;
	if (gpio->reg_set_base && !gpio->reg_dat_base)
		return GPIO_LINE_DIRECTION_OUT;

Otherwise it attempts to use .reg_dir_out_base and .reg_dir_in_base 
which are not set for gpio-idio-16 because the GPIO directions are fixed 
and not controlled via a data-direction register. And as these are not 
set, gpio_regmap_get_direction() returns -ENOTSUPP.

Were you thinking that gpio_regmap_get_direction() should have some 
additional direction logic if both .reg_dat_base and .reg_set_base are 
set, based upon their comparative values?


ATB,

Mark.

Re: gpio: gpio-pci-idio-16 regression after LTS upgrade

[William Breathitt Gray]

On Thu, Oct 09, 2025 at 10:05:58AM +0100, Mark Cave-Ayland wrote:
> On 07/10/2025 09:16, William Breathitt Gray wrote:
> > So the intention I had with gpio-idio-16 was to provide reg_dat_base and
> > reg_set_base to define the input and output bases, and then
> > reg_mask_xlate would do the translation between input and outputs. I
> > think this design is allowed by gpio-regmap, is it not Michael?
> >
> > In theory, gpio_regmap_get_direction should call gpio->reg_mask_xlate()
> > which is mapped to idio_16_reg_mask_xlate(), and thus set reg and mask
> > which then is evaluated at the end of gpio_regmap_get_direction() to
> > determine which direction to return.
> >
> > Is it possible idio_16_reg_mask_xlate() is returning the wrong values
> > for reg and mask?
> >
> > William Breathitt Gray
> 
> The only logic around .reg_dat_base and .reg_set_base in
> gpio_regmap_get_direction() is this:
> 
> 	if (gpio->reg_dat_base && !gpio->reg_set_base)
> 		return GPIO_LINE_DIRECTION_IN;
> 	if (gpio->reg_set_base && !gpio->reg_dat_base)
> 		return GPIO_LINE_DIRECTION_OUT;
> 
> Otherwise it attempts to use .reg_dir_out_base and .reg_dir_in_base
> which are not set for gpio-idio-16 because the GPIO directions are fixed
> and not controlled via a data-direction register. And as these are not
> set, gpio_regmap_get_direction() returns -ENOTSUPP.
> 
> Were you thinking that gpio_regmap_get_direction() should have some
> additional direction logic if both .reg_dat_base and .reg_set_base are
> set, based upon their comparative values?

Ah you're right, I misunderstood the logic in gpio_regmap_get_direction.
So essentially the problem is that gpio-idio-16 has no way of indicating
the direction of its I/O because it's mixed.

The IDIO-16 series lacks a direction setting register, so I think the
proper solution is as you suggest: add support for a get_direction
callback to struct gpio_regmap_config in the same vein as the existing
reg_mask_xlate callback. Then in gpio_regmap_register you can set
gpio->get_direction = config->get_direction in the same way
config->reg_mask_xlate is handled.

William Breathitt Gray

Re: gpio: gpio-pci-idio-16 regression after LTS upgrade

[William Breathitt Gray]

On Mon, Oct 06, 2025 at 09:37:14AM +0100, Mark Cave-Ayland wrote:
> After some more debugging I was able to determine that the failure was
> due to the regmap cache failing initialisation in
> drivers/base/regmap/regcache-flat.c::regcache_flat_init() because
> max_register wasn't set on the regmap. I was able to fix that fairly
> easily with this:
> 
> 
> diff --git a/drivers/gpio/gpio-pci-idio-16.c
> b/drivers/gpio/gpio-pci-idio-16.c
> index 44c0a21b1d1d..55be571b5cca 100644
> --- a/drivers/gpio/gpio-pci-idio-16.c
> +++ b/drivers/gpio/gpio-pci-idio-16.c
> @@ -41,6 +41,7 @@ static const struct regmap_config
> idio_16_regmap_config = {
>          .reg_stride = 1,
>          .val_bits = 8,
>          .io_port = true,
> +  .max_register = 0x7,
>          .wr_table = &idio_16_wr_table,
>          .rd_table = &idio_16_rd_table,
>          .volatile_table = &idio_16_rd_table,

This particular failure is separate from the get_direction issue
discovered after. It would be good to have this fix as its own patch so
we can keep each solution dedicated to their respective failures and
streamline any necessary backports to the stable trees.

I have some travel scheduled in the next couple days, but I can
probably get a patch series addressing both issues ready by the end of
the week. I'll also CC the stable mailing list so we can get the fixes
picked up for the LTS kernel they are affecting.

William Breathitt Gray

Re: gpio: gpio-pci-idio-16 regression after LTS upgrade

[Mark Cave-Ayland]

On 12/10/2025 15:22, William Breathitt Gray wrote:

> On Mon, Oct 06, 2025 at 09:37:14AM +0100, Mark Cave-Ayland wrote:
>> After some more debugging I was able to determine that the failure was
>> due to the regmap cache failing initialisation in
>> drivers/base/regmap/regcache-flat.c::regcache_flat_init() because
>> max_register wasn't set on the regmap. I was able to fix that fairly
>> easily with this:
>>
>>
>> diff --git a/drivers/gpio/gpio-pci-idio-16.c
>> b/drivers/gpio/gpio-pci-idio-16.c
>> index 44c0a21b1d1d..55be571b5cca 100644
>> --- a/drivers/gpio/gpio-pci-idio-16.c
>> +++ b/drivers/gpio/gpio-pci-idio-16.c
>> @@ -41,6 +41,7 @@ static const struct regmap_config
>> idio_16_regmap_config = {
>>           .reg_stride = 1,
>>           .val_bits = 8,
>>           .io_port = true,
>> +  .max_register = 0x7,
>>           .wr_table = &idio_16_wr_table,
>>           .rd_table = &idio_16_rd_table,
>>           .volatile_table = &idio_16_rd_table,
> 
> This particular failure is separate from the get_direction issue
> discovered after. It would be good to have this fix as its own patch so
> we can keep each solution dedicated to their respective failures and
> streamline any necessary backports to the stable trees.

Agreed.

> I have some travel scheduled in the next couple days, but I can
> probably get a patch series addressing both issues ready by the end of
> the week. I'll also CC the stable mailing list so we can get the fixes
> picked up for the LTS kernel they are affecting.

That would be fantastic! I was hoping to try and look at this over the 
past week, but it got delayed due to other work. Regardless of this I've 
still got access to the debugging environment to test any patches if you 
add me on CC.

If it helps the implementation we're using is part of the libvfio-user 
test suite (see https://github.com/nutanix/libvfio-user and 
samples/gpio-pci-idio-16.c). Internally we run a script that launches a 
QEMU VM, reads and writes a few sample gpios and then checks the logging 
to ensure that the basic vfio-user protocol is working as expected.


ATB,

Mark.

Re: gpio: gpio-pci-idio-16 regression after LTS upgrade

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 2671 bytes --]

Hi,

On Thu Oct 9, 2025 at 11:49 AM CEST, William Breathitt Gray wrote:
> On Thu, Oct 09, 2025 at 10:05:58AM +0100, Mark Cave-Ayland wrote:
>> On 07/10/2025 09:16, William Breathitt Gray wrote:
>> > So the intention I had with gpio-idio-16 was to provide reg_dat_base and
>> > reg_set_base to define the input and output bases, and then
>> > reg_mask_xlate would do the translation between input and outputs. I
>> > think this design is allowed by gpio-regmap, is it not Michael?
>> >
>> > In theory, gpio_regmap_get_direction should call gpio->reg_mask_xlate()
>> > which is mapped to idio_16_reg_mask_xlate(), and thus set reg and mask
>> > which then is evaluated at the end of gpio_regmap_get_direction() to
>> > determine which direction to return.
>> >
>> > Is it possible idio_16_reg_mask_xlate() is returning the wrong values
>> > for reg and mask?
>> >
>> > William Breathitt Gray
>> 
>> The only logic around .reg_dat_base and .reg_set_base in
>> gpio_regmap_get_direction() is this:
>> 
>> 	if (gpio->reg_dat_base && !gpio->reg_set_base)
>> 		return GPIO_LINE_DIRECTION_IN;
>> 	if (gpio->reg_set_base && !gpio->reg_dat_base)
>> 		return GPIO_LINE_DIRECTION_OUT;
>> 
>> Otherwise it attempts to use .reg_dir_out_base and .reg_dir_in_base
>> which are not set for gpio-idio-16 because the GPIO directions are fixed
>> and not controlled via a data-direction register. And as these are not
>> set, gpio_regmap_get_direction() returns -ENOTSUPP.
>> 
>> Were you thinking that gpio_regmap_get_direction() should have some
>> additional direction logic if both .reg_dat_base and .reg_set_base are
>> set, based upon their comparative values?
>
> Ah you're right, I misunderstood the logic in gpio_regmap_get_direction.
> So essentially the problem is that gpio-idio-16 has no way of indicating
> the direction of its I/O because it's mixed.
>
> The IDIO-16 series lacks a direction setting register, so I think the
> proper solution is as you suggest: add support for a get_direction
> callback to struct gpio_regmap_config in the same vein as the existing
> reg_mask_xlate callback. Then in gpio_regmap_register you can set
> gpio->get_direction = config->get_direction in the same way
> config->reg_mask_xlate is handled.

IIUC the chip has fixed input and outputs. In that case this should
help:
https://lore.kernel.org/all/20250922142427.3310221-8-ioana.ciornei@nxp.com/

I guess Ioana will resubmit the series for this cycle. To fix the
regression, I guess the patch can then be picked up by the stable
team along with the driver patch which will set the
.fixed_direction_output config field.

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

Re: gpio: gpio-pci-idio-16 regression after LTS upgrade

[Ioana Ciornei]

On Tue, Oct 14, 2025 at 11:03:25AM +0200, Michael Walle wrote:
> Hi,
> 
> On Thu Oct 9, 2025 at 11:49 AM CEST, William Breathitt Gray wrote:
> > On Thu, Oct 09, 2025 at 10:05:58AM +0100, Mark Cave-Ayland wrote:
> >> On 07/10/2025 09:16, William Breathitt Gray wrote:
> >> > So the intention I had with gpio-idio-16 was to provide reg_dat_base and
> >> > reg_set_base to define the input and output bases, and then
> >> > reg_mask_xlate would do the translation between input and outputs. I
> >> > think this design is allowed by gpio-regmap, is it not Michael?
> >> >
> >> > In theory, gpio_regmap_get_direction should call gpio->reg_mask_xlate()
> >> > which is mapped to idio_16_reg_mask_xlate(), and thus set reg and mask
> >> > which then is evaluated at the end of gpio_regmap_get_direction() to
> >> > determine which direction to return.
> >> >
> >> > Is it possible idio_16_reg_mask_xlate() is returning the wrong values
> >> > for reg and mask?
> >> >
> >> > William Breathitt Gray
> >> 
> >> The only logic around .reg_dat_base and .reg_set_base in
> >> gpio_regmap_get_direction() is this:
> >> 
> >> 	if (gpio->reg_dat_base && !gpio->reg_set_base)
> >> 		return GPIO_LINE_DIRECTION_IN;
> >> 	if (gpio->reg_set_base && !gpio->reg_dat_base)
> >> 		return GPIO_LINE_DIRECTION_OUT;
> >> 
> >> Otherwise it attempts to use .reg_dir_out_base and .reg_dir_in_base
> >> which are not set for gpio-idio-16 because the GPIO directions are fixed
> >> and not controlled via a data-direction register. And as these are not
> >> set, gpio_regmap_get_direction() returns -ENOTSUPP.
> >> 
> >> Were you thinking that gpio_regmap_get_direction() should have some
> >> additional direction logic if both .reg_dat_base and .reg_set_base are
> >> set, based upon their comparative values?
> >
> > Ah you're right, I misunderstood the logic in gpio_regmap_get_direction.
> > So essentially the problem is that gpio-idio-16 has no way of indicating
> > the direction of its I/O because it's mixed.
> >
> > The IDIO-16 series lacks a direction setting register, so I think the
> > proper solution is as you suggest: add support for a get_direction
> > callback to struct gpio_regmap_config in the same vein as the existing
> > reg_mask_xlate callback. Then in gpio_regmap_register you can set
> > gpio->get_direction = config->get_direction in the same way
> > config->reg_mask_xlate is handled.
> 
> IIUC the chip has fixed input and outputs. In that case this should
> help:
> https://lore.kernel.org/all/20250922142427.3310221-8-ioana.ciornei@nxp.com/
> 
> I guess Ioana will resubmit the series for this cycle.

Yes, I will resubmit shortly.

> To fix the regression, I guess the patch can then be picked up by the
> stable team along with the driver patch which will set the
> .fixed_direction_output config field.

Just to make sure that I understand, do you want me to add another patch
that adds the new driver user or that will be handled separately?

Ioana

Re: gpio: gpio-pci-idio-16 regression after LTS upgrade

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

On Tue Oct 14, 2025 at 1:21 PM CEST, Ioana Ciornei wrote:
>> To fix the regression, I guess the patch can then be picked up by the
>> stable team along with the driver patch which will set the
>> .fixed_direction_output config field.
>
> Just to make sure that I understand, do you want me to add another patch
> that adds the new driver user or that will be handled separately?

No, no. No new patch. It's just that your patch (just the one which
has that property added to gpio-regmap) has to be submitted to the stable
kernels (by the one who is fixing the regression) I guess.

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

Re: gpio: gpio-pci-idio-16 regression after LTS upgrade

[Ioana Ciornei]

On Tue, Oct 14, 2025 at 01:25:24PM +0200, Michael Walle wrote:
> On Tue Oct 14, 2025 at 1:21 PM CEST, Ioana Ciornei wrote:
> >> To fix the regression, I guess the patch can then be picked up by the
> >> stable team along with the driver patch which will set the
> >> .fixed_direction_output config field.
> >
> > Just to make sure that I understand, do you want me to add another patch
> > that adds the new driver user or that will be handled separately?
> 
> No, no. No new patch. It's just that your patch (just the one which
> has that property added to gpio-regmap) has to be submitted to the stable
> kernels (by the one who is fixing the regression) I guess.

Ok, understood.

Ioana