Message ID | 20240417103829.2324960-1-andriy.shevchenko@linux.intel.com |
---|---|
Headers | show |
Series | gpiolib: acpi: Improve IRQ labeling | expand |
On Wed, Apr 17, 2024 at 01:37:27PM +0300, Andy Shevchenko wrote: > It's ambiguous to have a device-related index in the GPIO interrupt > label as most of the devices will have it the same or very similar. > Extend label with fwnode name for better granularity. It significantly > reduces the scope of searching among devices. Can you add an example here how it looks like before and after the patch? > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/gpio/gpiolib-acpi.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c > index 909113312a1b..0b0c8729fc6e 100644 > --- a/drivers/gpio/gpiolib-acpi.c > +++ b/drivers/gpio/gpiolib-acpi.c > @@ -1035,6 +1035,7 @@ struct gpio_desc *acpi_find_gpio(struct fwnode_handle *fwnode, > int acpi_dev_gpio_irq_wake_get_by(struct acpi_device *adev, const char *con_id, int index, > bool *wake_capable) > { > + struct fwnode_handle *fwnode = acpi_fwnode_handle(adev); > int idx, i; > unsigned int irq_flags; > int ret; > @@ -1044,7 +1045,7 @@ int acpi_dev_gpio_irq_wake_get_by(struct acpi_device *adev, const char *con_id, > struct gpio_desc *desc; > > /* Ignore -EPROBE_DEFER, it only matters if idx matches */ > - desc = __acpi_find_gpio(acpi_fwnode_handle(adev), con_id, i, true, &info); > + desc = __acpi_find_gpio(fwnode, con_id, i, true, &info); > if (IS_ERR(desc) && PTR_ERR(desc) != -EPROBE_DEFER) > return PTR_ERR(desc); > > @@ -1064,7 +1065,7 @@ int acpi_dev_gpio_irq_wake_get_by(struct acpi_device *adev, const char *con_id, > acpi_gpio_update_gpiod_flags(&dflags, &info); > acpi_gpio_update_gpiod_lookup_flags(&lflags, &info); > > - snprintf(label, sizeof(label), "GpioInt() %d", index); > + snprintf(label, sizeof(label), "%pfwP GpioInt(%d)", fwnode, index); > ret = gpiod_configure_flags(desc, label, lflags, dflags); > if (ret < 0) > return ret; > -- > 2.43.0.rc1.1336.g36b5255a03ac
On Thu, Apr 18, 2024 at 07:49:07AM +0300, Mika Westerberg wrote: > On Wed, Apr 17, 2024 at 01:37:27PM +0300, Andy Shevchenko wrote: > > It's ambiguous to have a device-related index in the GPIO interrupt > > label as most of the devices will have it the same or very similar. > > Extend label with fwnode name for better granularity. It significantly > > reduces the scope of searching among devices. > > Can you add an example here how it looks like before and after the > patch? Sure: Before: GpioInt() 0 GpioInt() 0 After: NIO1 GpioInt(0) URT0 GpioInt(0) Assuming I update this when applying, can you give your tag?
On Thu, Apr 18, 2024 at 12:23:45PM +0300, Andy Shevchenko wrote: > On Thu, Apr 18, 2024 at 07:49:07AM +0300, Mika Westerberg wrote: > > On Wed, Apr 17, 2024 at 01:37:27PM +0300, Andy Shevchenko wrote: > > > It's ambiguous to have a device-related index in the GPIO interrupt > > > label as most of the devices will have it the same or very similar. > > > Extend label with fwnode name for better granularity. It significantly > > > reduces the scope of searching among devices. > > > > Can you add an example here how it looks like before and after the > > patch? > > Sure: > > Before: > > GpioInt() 0 > GpioInt() 0 > > After: > > NIO1 GpioInt(0) > URT0 GpioInt(0) > > Assuming I update this when applying, can you give your tag? Sure. For both, Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
On Thu, Apr 18, 2024 at 12:33:59PM +0300, Mika Westerberg wrote: > On Thu, Apr 18, 2024 at 12:23:45PM +0300, Andy Shevchenko wrote: > > On Thu, Apr 18, 2024 at 07:49:07AM +0300, Mika Westerberg wrote: > > > On Wed, Apr 17, 2024 at 01:37:27PM +0300, Andy Shevchenko wrote: > > > > It's ambiguous to have a device-related index in the GPIO interrupt > > > > label as most of the devices will have it the same or very similar. > > > > Extend label with fwnode name for better granularity. It significantly > > > > reduces the scope of searching among devices. > > > > > > Can you add an example here how it looks like before and after the > > > patch? > > > > Sure: > > > > Before: > > > > GpioInt() 0 > > GpioInt() 0 > > > > After: > > > > NIO1 GpioInt(0) > > URT0 GpioInt(0) > > > > Assuming I update this when applying, can you give your tag? > > Sure. For both, > > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> Pushed to my review and testing queue, thanks!