The IRQ labeling now is quite ambiguous, improve it here.
Andy Shevchenko (2):
gpiolib: acpi: Add fwnode name to the GPIO interrupt label
gpiolib: acpi: Set label for IRQ only lines
drivers/gpio/gpiolib-acpi.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
--
2.43.0.rc1.1336.g36b5255a03ac
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.
Signed-off-by: Andy Shevchenko <[email protected]>
---
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
When line locked as IRQ it has no label assigned. Assign
the meaningful value to it.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/gpio/gpiolib-acpi.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 0b0c8729fc6e..553a5f94c00a 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1066,6 +1066,10 @@ int acpi_dev_gpio_irq_wake_get_by(struct acpi_device *adev, const char *con_id,
acpi_gpio_update_gpiod_lookup_flags(&lflags, &info);
snprintf(label, sizeof(label), "%pfwP GpioInt(%d)", fwnode, index);
+ ret = gpiod_set_consumer_name(desc, con_id ?: label);
+ if (ret)
+ return ret;
+
ret = gpiod_configure_flags(desc, label, lflags, dflags);
if (ret < 0)
return ret;
--
2.43.0.rc1.1336.g36b5255a03ac
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 <[email protected]>
> ---
> 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 Wed, Apr 17, 2024 at 01:37:28PM +0300, Andy Shevchenko wrote:
> When line locked as IRQ it has no label assigned. Assign
> the meaningful value to it.
Same here.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/gpio/gpiolib-acpi.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 0b0c8729fc6e..553a5f94c00a 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -1066,6 +1066,10 @@ int acpi_dev_gpio_irq_wake_get_by(struct acpi_device *adev, const char *con_id,
> acpi_gpio_update_gpiod_lookup_flags(&lflags, &info);
>
> snprintf(label, sizeof(label), "%pfwP GpioInt(%d)", fwnode, index);
> + ret = gpiod_set_consumer_name(desc, con_id ?: label);
> + if (ret)
> + return ret;
> +
> 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:50AM +0300, Mika Westerberg wrote:
> On Wed, Apr 17, 2024 at 01:37:28PM +0300, Andy Shevchenko wrote:
> > When line locked as IRQ it has no label assigned. Assign
> > the meaningful value to it.
>
> Same here.
Same answer as in the first reply.
--
With Best Regards,
Andy Shevchenko
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 <[email protected]>
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 <[email protected]>
Pushed to my review and testing queue, thanks!
--
With Best Regards,
Andy Shevchenko
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?
--
With Best Regards,
Andy Shevchenko