On Mon, May 25, 2020 at 03:21:36PM +0300, Mika Westerberg wrote: > On Mon, May 25, 2020 at 02:35:51PM +0300, Andy Shevchenko wrote: > > On Mon, May 25, 2020 at 02:05:56PM +0300, Mika Westerberg wrote: > > > On Mon, May 25, 2020 at 01:13:35PM +0300, Andy Shevchenko wrote: > > > > Due to parsing of ACPI tables. I don't want to copy'n'paste 25% of > > > > gpiolib-acpi.c in here. I think provided solution is cleaner and (more) > > > > flexible in terms of maintenance. > > > > > > Hmm, you seem to pass a hard-coded pin number (1) to the core that then > > > passes it back to the driver. Why you can't simple use that number here > > > directly? You don't need to parse anything. What I'm missing? :-) > > > > Okay, so, AFAIU you are proposing something like this: > > > > 1) find a GPIO controller by the ACPI path (somehow, I guess by finding a > > handle followed by physical device behind it); 2) somehow to request a > > pin from that device by number; > > 3) convert to IRQ and use. > > > > Is it correct? > > Well, no. In the first patch you do this: > > pin = lookup->info.quirks_data; > > and this essentially becomes 1 so you know the pin number upfront in the > driver. Why not simply get GPIO number 1 in the driver and use it as an > interrupt? You know already that this particular board with the matching > DMI identifier always uses the this number anyway. But GPIO (relative!) number is not enough. We need to understand more, i.e.: 1) from which GPIO controller it comes from (okay, for this particular platform I know it); 2) which expander does have this resource (they all have same ACPI HID). So, second one means to count GpioInt() resources (I don't remember if we have helper for that, probably we can add one). For the first we need to get a GPIO controller something (gpiochip?) And this first one I have no idea how we can perform without talking to the core. Basically, it may be done by reimplementing acpi_dev_gpio_irq_get(), followed by acpi_get_gpiod_by_index(), followed by acpi_gpio_resource_lookup(), followed by acpi_populate_gpio_lookup(), where at last this quirk should be applied. It seems for me like an over duplicated solution. I really don't understand how we can shortcut all these. What am I missing? -- With Best Regards, Andy Shevchenko
