Re: [PATCH] gpio: pl061: Warn when IRQ line has not been configured

[Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>]

Hi!

On 04/03/2020 15:21, Bartosz Golaszewski wrote:
> wt., 3 mar 2020 o 10:29 Alexander A Sverdlin
> <alexander.sverdlin@xxxxxxxxx> napisał(a):
>> From: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>
>>
>> Existing (irq < 0) condition is always false because adev->irq has unsigned
>> type and contains 0 in case of failed irq_of_parse_and_map(). Up to now all
>> the mapping errors were silently ignored.
>>
>> Seems that repairing this check would be backwards-incompatible and might
>> break the probe() for the implementations without IRQ support. Therefore
>> warn the user instead.
>>
>> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>
>> ---
>>  drivers/gpio/gpio-pl061.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
>> index 5df7782..3439120 100644
>> --- a/drivers/gpio/gpio-pl061.c
>> +++ b/drivers/gpio/gpio-pl061.c
>> @@ -326,10 +326,8 @@ static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
>>
>>         writeb(0, pl061->base + GPIOIE); /* disable irqs */
>>         irq = adev->irq[0];
>> -       if (irq < 0) {
>> -               dev_err(&adev->dev, "invalid IRQ\n");
>> -               return -ENODEV;
>> -       }
>> +       if (!irq)
>> +               dev_warn(&adev->dev, "IRQ support disabled\n");
>>         pl061->parent_irq = irq;
>>
>>         girq = &pl061->gc.irq;
>> --
>> 2.4.6
>>
> What happens later on if irq == 0? Does irq_set_irq_wake() fail?

Yes, would fail if IRQs would be requested from PL061:

int irq_set_irq_wake(unsigned int irq, unsigned int on)                                                                                                                                                            
{                                                                                                                                                                                                                  
        unsigned long flags;                                                                                                                                                                                       
        struct irq_desc *desc = irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL);                                                                                                                      
        int ret = 0;                                                                                                                                                                                               
                                                                                                                                                                                                                   
        if (!desc)                                                                                                                                                                                                 
                return -EINVAL;                                                                                                                                                                                    


-- 
Best regards,
Alexander Sverdlin.