[PATCH v2] platform: finally disallow IRQ0 in platform_get_irq() and its ilk

[PATCH v2] platform: finally disallow IRQ0 in platform_get_irq() and its ilk

[Sergey Shtylyov]

The commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is
invalid") only calls WARN() when IRQ0 is about to be returned, however
using IRQ0 is considered invalid (according to Linus) outside the arch/
code where it's used by the i8253 drivers. Many driver subsystems treat
0 specially (e.g. as an indication of the polling mode by libata), so
the users of platform_get_irq[_byname]() in them would have to filter
out IRQ0 explicitly and this (quite obviously) doesn't scale...
Let's finally get this straight and return -EINVAL instead of IRQ0!

Fixes: a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid")
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Acked-by: Marc Zyngier <maz@kernel.org>

---
The patch is against the 'driver-core-next' branch of Greg Kroah-Hartman's
'driver-core.git' repo.

Changes in version 2:
- added Marc's ACK.

 drivers/base/platform.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Index: driver-core/drivers/base/platform.c
===================================================================
--- driver-core.orig/drivers/base/platform.c
+++ driver-core/drivers/base/platform.c
@@ -231,7 +231,8 @@ int platform_get_irq_optional(struct pla
 out_not_found:
 	ret = -ENXIO;
 out:
-	WARN(ret == 0, "0 is an invalid IRQ number\n");
+	if (WARN(!ret, "0 is an invalid IRQ number\n"))
+		return -EINVAL;
 	return ret;
 }
 EXPORT_SYMBOL_GPL(platform_get_irq_optional);
@@ -446,7 +447,8 @@ static int __platform_get_irq_byname(str
 
 	r = platform_get_resource_byname(dev, IORESOURCE_IRQ, name);
 	if (r) {
-		WARN(r->start == 0, "0 is an invalid IRQ number\n");
+		if (WARN(!r->start, "0 is an invalid IRQ number\n"))
+			return -EINVAL;
 		return r->start;
 	}
 

Re: [PATCH v2] platform: finally disallow IRQ0 in platform_get_irq() and its ilk

[Sergey Shtylyov]

Hello!

On 3/11/22 10:35 PM, Sergey Shtylyov wrote:

> The commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is
> invalid") only calls WARN() when IRQ0 is about to be returned, however
> using IRQ0 is considered invalid (according to Linus) outside the arch/
> code where it's used by the i8253 drivers. Many driver subsystems treat
> 0 specially (e.g. as an indication of the polling mode by libata), so
> the users of platform_get_irq[_byname]() in them would have to filter
> out IRQ0 explicitly and this (quite obviously) doesn't scale...
> Let's finally get this straight and return -EINVAL instead of IRQ0!
> 
> Fixes: a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid")
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> Acked-by: Marc Zyngier <maz@kernel.org>
> 
> ---
> The patch is against the 'driver-core-next' branch of Greg Kroah-Hartman's
> 'driver-core.git' repo.
> 
> Changes in version 2:
> - added Marc's ACK.

   Greg, are you going to finally merge this?

MBR, Sergey

Re: [PATCH v2] platform: finally disallow IRQ0 in platform_get_irq() and its ilk

[Greg Kroah-Hartman]

On Fri, Mar 11, 2022 at 10:35:29PM +0300, Sergey Shtylyov wrote:
> The commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is
> invalid") only calls WARN() when IRQ0 is about to be returned, however
> using IRQ0 is considered invalid (according to Linus) outside the arch/
> code where it's used by the i8253 drivers. Many driver subsystems treat
> 0 specially (e.g. as an indication of the polling mode by libata), so
> the users of platform_get_irq[_byname]() in them would have to filter
> out IRQ0 explicitly and this (quite obviously) doesn't scale...
> Let's finally get this straight and return -EINVAL instead of IRQ0!
> 
> Fixes: a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid")
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> Acked-by: Marc Zyngier <maz@kernel.org>
> 
> ---
> The patch is against the 'driver-core-next' branch of Greg Kroah-Hartman's
> 'driver-core.git' repo.
> 
> Changes in version 2:
> - added Marc's ACK.
> 
>  drivers/base/platform.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Index: driver-core/drivers/base/platform.c
> ===================================================================
> --- driver-core.orig/drivers/base/platform.c
> +++ driver-core/drivers/base/platform.c
> @@ -231,7 +231,8 @@ int platform_get_irq_optional(struct pla
>  out_not_found:
>  	ret = -ENXIO;
>  out:
> -	WARN(ret == 0, "0 is an invalid IRQ number\n");
> +	if (WARN(!ret, "0 is an invalid IRQ number\n"))
> +		return -EINVAL;
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(platform_get_irq_optional);
> @@ -446,7 +447,8 @@ static int __platform_get_irq_byname(str
>  
>  	r = platform_get_resource_byname(dev, IORESOURCE_IRQ, name);
>  	if (r) {
> -		WARN(r->start == 0, "0 is an invalid IRQ number\n");
> +		if (WARN(!r->start, "0 is an invalid IRQ number\n"))
> +			return -EINVAL;
>  		return r->start;
>  	}
>  

Ok, let's try this now.  Worst case, we revert it later :)

thanks,

greg k-h

Re: [PATCH v2] platform: finally disallow IRQ0 in platform_get_irq() and its ilk

[Sergey Shtylyov]

Hello!

On 4/26/22 1:46 PM, Greg Kroah-Hartman wrote:

>> The commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is
>> invalid") only calls WARN() when IRQ0 is about to be returned, however
>> using IRQ0 is considered invalid (according to Linus) outside the arch/
>> code where it's used by the i8253 drivers. Many driver subsystems treat
>> 0 specially (e.g. as an indication of the polling mode by libata), so
>> the users of platform_get_irq[_byname]() in them would have to filter
>> out IRQ0 explicitly and this (quite obviously) doesn't scale...
>> Let's finally get this straight and return -EINVAL instead of IRQ0!
>>
>> Fixes: a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid")
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>> Acked-by: Marc Zyngier <maz@kernel.org>

[...]

> Ok, let's try this now.

   Well, better late than never! :-)

> Worst case, we revert it later :)

   Please just don't revert it outright on the 1st issue report -- give me time
to look at the issue(s) reported...
   BTW, I've CC'ed you on the SH patch that avoids using IRQ0. Please help to
merge it (v1/v2 were posted on February 11th and there was no motion since then)!

> thanks,
> 
> greg k-h

MBR, Sergey

Re: [PATCH v2] platform: finally disallow IRQ0 in platform_get_irq() and its ilk

[Greg Kroah-Hartman]

On Wed, Apr 27, 2022 at 09:59:41PM +0300, Sergey Shtylyov wrote:
> Hello!
> 
> On 4/26/22 1:46 PM, Greg Kroah-Hartman wrote:
> 
> >> The commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is
> >> invalid") only calls WARN() when IRQ0 is about to be returned, however
> >> using IRQ0 is considered invalid (according to Linus) outside the arch/
> >> code where it's used by the i8253 drivers. Many driver subsystems treat
> >> 0 specially (e.g. as an indication of the polling mode by libata), so
> >> the users of platform_get_irq[_byname]() in them would have to filter
> >> out IRQ0 explicitly and this (quite obviously) doesn't scale...
> >> Let's finally get this straight and return -EINVAL instead of IRQ0!
> >>
> >> Fixes: a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid")
> >> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> >> Acked-by: Marc Zyngier <maz@kernel.org>
> 
> [...]
> 
> > Ok, let's try this now.
> 
>    Well, better late than never! :-)
> 
> > Worst case, we revert it later :)
> 
>    Please just don't revert it outright on the 1st issue report -- give me time
> to look at the issue(s) reported...
>    BTW, I've CC'ed you on the SH patch that avoids using IRQ0. Please help to
> merge it (v1/v2 were posted on February 11th and there was no motion since then)!

I can't merge that until a sh maintainer acks it.  Is that arch even
still alive?

thanks,

greg k-h

Re: [PATCH v2] platform: finally disallow IRQ0 in platform_get_irq() and its ilk

[Sergey Shtylyov]

On 4/28/22 9:13 AM, Greg Kroah-Hartman wrote:

[...]
>>>> The commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is
>>>> invalid") only calls WARN() when IRQ0 is about to be returned, however
>>>> using IRQ0 is considered invalid (according to Linus) outside the arch/
>>>> code where it's used by the i8253 drivers. Many driver subsystems treat
>>>> 0 specially (e.g. as an indication of the polling mode by libata), so
>>>> the users of platform_get_irq[_byname]() in them would have to filter
>>>> out IRQ0 explicitly and this (quite obviously) doesn't scale...
>>>> Let's finally get this straight and return -EINVAL instead of IRQ0!
>>>>
>>>> Fixes: a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid")
>>>> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
>>>> Acked-by: Marc Zyngier <maz@kernel.org>
>>
>> [...]
>>
>>> Ok, let's try this now.
>>
>>    Well, better late than never! :-)
>>
>>> Worst case, we revert it later :)
>>
>>    Please just don't revert it outright on the 1st issue report -- give me time
>> to look at the issue(s) reported...
>>    BTW, I've CC'ed you on the SH patch that avoids using IRQ0. Please help to
>> merge it (v1/v2 were posted on February 11th and there was no motion since then)!
> 
> I can't merge that until a sh maintainer acks it.

   Hm, looking at the arch/sh/ commit history, you've recently merged couple patches
without any ACKs from the SH people. ;-) What concerns you in this case, me touching
the assembly code? Who's worth looping in on that patch, do you think, maybe Andrew
Morton or Linus himself?

> Is that arch even still alive?

   Well, considering J-core should be still alive (there's #jcore channel on Libera.chat,
where you can find Rich Felker as dalias), arch/sh/ must be still alive too...
   It probably needs the better maintainers though... :-)

> thanks,
> 
> greg k-h

MBR, Sergey