gpiolib: acpi: Print pin number on acpi_gpiochip_alloc_event errors
diff mbox series

Message ID 20191018195208.94405-1-hdegoede@redhat.com
State New
Headers show
Series
  • gpiolib: acpi: Print pin number on acpi_gpiochip_alloc_event errors
Related show

Commit Message

Hans de Goede Oct. 18, 2019, 7:52 p.m. UTC
Print pin number on acpi_gpiochip_alloc_event errors, to help debugging
these.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpio/gpiolib-acpi.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Andy Shevchenko Oct. 21, 2019, 9:11 a.m. UTC | #1
On Fri, Oct 18, 2019 at 09:52:08PM +0200, Hans de Goede wrote:
> Print pin number on acpi_gpiochip_alloc_event errors, to help debugging
> these.

I'm not sure which one is better decimal or hex, perhaps Mika can help me, in any case
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpio/gpiolib-acpi.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 609ed16ae933..2911dd6f2625 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -230,19 +230,22 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
>  	desc = gpiochip_request_own_desc(chip, pin, "ACPI:Event",
>  					 GPIO_ACTIVE_HIGH, GPIOD_IN);
>  	if (IS_ERR(desc)) {
> -		dev_err(chip->parent, "Failed to request GPIO\n");
> +		dev_err(chip->parent,
> +			"Failed to request GPIO for pin 0x%02X\n", pin);
>  		return AE_ERROR;
>  	}
>  
>  	ret = gpiochip_lock_as_irq(chip, pin);
>  	if (ret) {
> -		dev_err(chip->parent, "Failed to lock GPIO as interrupt\n");
> +		dev_err(chip->parent,
> +			"Failed to lock GPIO pin 0x%02X as interrupt\n", pin);
>  		goto fail_free_desc;
>  	}
>  
>  	irq = gpiod_to_irq(desc);
>  	if (irq < 0) {
> -		dev_err(chip->parent, "Failed to translate GPIO to IRQ\n");
> +		dev_err(chip->parent,
> +			"Failed to translate GPIO pin 0x%02X to IRQ\n", pin);
>  		goto fail_unlock_irq;
>  	}
>  
> -- 
> 2.23.0
>
Hans de Goede Oct. 21, 2019, 12:52 p.m. UTC | #2
Hi,

On 21-10-2019 11:11, Andy Shevchenko wrote:
> On Fri, Oct 18, 2019 at 09:52:08PM +0200, Hans de Goede wrote:
>> Print pin number on acpi_gpiochip_alloc_event errors, to help debugging
>> these.
> 
> I'm not sure which one is better decimal or hex, perhaps Mika can help me, in any case

They are listed as hex in the dis-assembled DSTD.

Regards,

Hans




> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/gpio/gpiolib-acpi.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
>> index 609ed16ae933..2911dd6f2625 100644
>> --- a/drivers/gpio/gpiolib-acpi.c
>> +++ b/drivers/gpio/gpiolib-acpi.c
>> @@ -230,19 +230,22 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
>>   	desc = gpiochip_request_own_desc(chip, pin, "ACPI:Event",
>>   					 GPIO_ACTIVE_HIGH, GPIOD_IN);
>>   	if (IS_ERR(desc)) {
>> -		dev_err(chip->parent, "Failed to request GPIO\n");
>> +		dev_err(chip->parent,
>> +			"Failed to request GPIO for pin 0x%02X\n", pin);
>>   		return AE_ERROR;
>>   	}
>>   
>>   	ret = gpiochip_lock_as_irq(chip, pin);
>>   	if (ret) {
>> -		dev_err(chip->parent, "Failed to lock GPIO as interrupt\n");
>> +		dev_err(chip->parent,
>> +			"Failed to lock GPIO pin 0x%02X as interrupt\n", pin);
>>   		goto fail_free_desc;
>>   	}
>>   
>>   	irq = gpiod_to_irq(desc);
>>   	if (irq < 0) {
>> -		dev_err(chip->parent, "Failed to translate GPIO to IRQ\n");
>> +		dev_err(chip->parent,
>> +			"Failed to translate GPIO pin 0x%02X to IRQ\n", pin);
>>   		goto fail_unlock_irq;
>>   	}
>>   
>> -- 
>> 2.23.0
>>
>
Andy Shevchenko Oct. 21, 2019, 1:23 p.m. UTC | #3
On Mon, Oct 21, 2019 at 02:52:13PM +0200, Hans de Goede wrote:
> On 21-10-2019 11:11, Andy Shevchenko wrote:
> > On Fri, Oct 18, 2019 at 09:52:08PM +0200, Hans de Goede wrote:
> > > Print pin number on acpi_gpiochip_alloc_event errors, to help debugging
> > > these.
> > 
> > I'm not sure which one is better decimal or hex, perhaps Mika can help me, in any case
> 
> They are listed as hex in the dis-assembled DSTD.

Oh, now I noticed the potential issue. The pin can be 16-bit, you pring here
only for the case when we have evname. Maybe do something similar in these
messages? Print event name for pin inside byte range and value otherwise?
Andy Shevchenko Oct. 21, 2019, 1:26 p.m. UTC | #4
On Mon, Oct 21, 2019 at 12:11:24PM +0300, Andy Shevchenko wrote:
> On Fri, Oct 18, 2019 at 09:52:08PM +0200, Hans de Goede wrote:
> > Print pin number on acpi_gpiochip_alloc_event errors, to help debugging
> > these.
> 
> I'm not sure which one is better decimal or hex, perhaps Mika can help me, in any case
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

I have to withdraw this due to potential issue found.
Hans de Goede Nov. 14, 2019, 10:13 a.m. UTC | #5
Hi,

On 21-10-2019 15:23, Andy Shevchenko wrote:
> On Mon, Oct 21, 2019 at 02:52:13PM +0200, Hans de Goede wrote:
>> On 21-10-2019 11:11, Andy Shevchenko wrote:
>>> On Fri, Oct 18, 2019 at 09:52:08PM +0200, Hans de Goede wrote:
>>>> Print pin number on acpi_gpiochip_alloc_event errors, to help debugging
>>>> these.
>>>
>>> I'm not sure which one is better decimal or hex, perhaps Mika can help me, in any case
>>
>> They are listed as hex in the dis-assembled DSTD.
> 
> Oh, now I noticed the potential issue. The pin can be 16-bit, you pring here
> only for the case when we have evname. Maybe do something similar in these
> messages? Print event name for pin inside byte range and value otherwise?

I'm preparing a v2 now, together with another related, less cosmetic patch.
I will switch to a 16 bit printf format for v2.

Regards,

Hans

Patch
diff mbox series

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 609ed16ae933..2911dd6f2625 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -230,19 +230,22 @@  static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
 	desc = gpiochip_request_own_desc(chip, pin, "ACPI:Event",
 					 GPIO_ACTIVE_HIGH, GPIOD_IN);
 	if (IS_ERR(desc)) {
-		dev_err(chip->parent, "Failed to request GPIO\n");
+		dev_err(chip->parent,
+			"Failed to request GPIO for pin 0x%02X\n", pin);
 		return AE_ERROR;
 	}
 
 	ret = gpiochip_lock_as_irq(chip, pin);
 	if (ret) {
-		dev_err(chip->parent, "Failed to lock GPIO as interrupt\n");
+		dev_err(chip->parent,
+			"Failed to lock GPIO pin 0x%02X as interrupt\n", pin);
 		goto fail_free_desc;
 	}
 
 	irq = gpiod_to_irq(desc);
 	if (irq < 0) {
-		dev_err(chip->parent, "Failed to translate GPIO to IRQ\n");
+		dev_err(chip->parent,
+			"Failed to translate GPIO pin 0x%02X to IRQ\n", pin);
 		goto fail_unlock_irq;
 	}