On Sun, 5 Dec 2021 22:39:07 +0200
Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
> On Sun, Dec 5, 2021 at 9:57 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> >
> > Note the interrupt type should be specified by firmware, not the driver
> > so that is also dropped.
> >
> > Drop previous gpio based retrieval method. Whilst in theory this
> > might cause problems with direction if anyone is using ACPI GioIo().
>
> GpioIo()
My proofreading fails me again...
>
> > As Andy described in v1, such a situation would typically reflect
> > a pin that is actually used in both directions (not true here)
> > or missdesigned ACPI tables.
>
> ...
>
> > - gpio = devm_gpiod_get_index(dev, NULL, i, GPIOD_IN);
> > - if (IS_ERR(gpio)) {
> > - dev_err(dev, "acpi gpio get index failed\n");
> > - return PTR_ERR(gpio);
> > - }
> > -
> > - ret = gpiod_to_irq(gpio);
> > - if (ret < 0)
>
> > + ret = fwnode_irq_get(dev_fwnode(dev), i);
> > + if (ret)
> > return ret;
>
> I don't remember why we decided that this gonna work, because
> fwnode_irq_get() is not an equivalent to the above, more precisely in
> ACPI case it only covers the GSIs (Global System Interrupts) which in
> such case may or may not be GPIOs. On x86 it's usually direct IOxAPIC
> ones.
This isn't accessed as a GPIO, but I kind of assume someone was using
it with one given this code.
I think I misread what you'd written in reply to v1.
Thanks for the explanation.
>
> So, this conversion would probably make it impossible to use this
> device in the ACPI case.
oops :)
>
> See also this discussion:
> https://lore.kernel.org/lkml/20211109200840.135019-1-puranjay12@xxxxxxxxx/T/#u
>
I'd not made the connection that the non named one was also missing most
of the ways it could be specified. Ah well, this will take a little while
to get a test framework in place to poke at it properly. Maybe one for
a bored moment over the festive season.
Jonathan
