[PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[Simon Trimmer]

IRQ lookup functions such as those in ACPI can return error values when
an IRQ is not defined. The i2c core driver converts the error codes to a
value of 0 and the SPI bus driver passes them unaltered to client device
drivers.

The cs35l56 driver should only accept positive non-zero values as IRQ
numbers.

Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
Fixes: 8a731fd37f8b ("ASoC: cs35l56: Move utility functions to shared file")
---
 sound/soc/codecs/cs35l56-shared.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/cs35l56-shared.c b/sound/soc/codecs/cs35l56-shared.c
index 27869e14e9c8..880228f89baf 100644
--- a/sound/soc/codecs/cs35l56-shared.c
+++ b/sound/soc/codecs/cs35l56-shared.c
@@ -397,7 +397,7 @@ int cs35l56_irq_request(struct cs35l56_base *cs35l56_base, int irq)
 {
 	int ret;
 
-	if (!irq)
+	if (irq < 1)
 		return 0;
 
 	ret = devm_request_threaded_irq(cs35l56_base->dev, irq, NULL, cs35l56_irq,
-- 
2.34.1

Re: [PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 444 bytes --]

On Mon, Jun 17, 2024 at 02:53:38PM +0100, Simon Trimmer wrote:
> IRQ lookup functions such as those in ACPI can return error values when
> an IRQ is not defined. The i2c core driver converts the error codes to a
> value of 0 and the SPI bus driver passes them unaltered to client device
> drivers.
> 
> The cs35l56 driver should only accept positive non-zero values as IRQ
> numbers.

Have all architectures removed 0 as a valid IRQ?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[Richard Fitzgerald]

On 17/06/2024 15:04, Mark Brown wrote:
> On Mon, Jun 17, 2024 at 02:53:38PM +0100, Simon Trimmer wrote:
>> IRQ lookup functions such as those in ACPI can return error values when
>> an IRQ is not defined. The i2c core driver converts the error codes to a
>> value of 0 and the SPI bus driver passes them unaltered to client device
>> drivers.
>>
>> The cs35l56 driver should only accept positive non-zero values as IRQ
>> numbers.
> 
> Have all architectures removed 0 as a valid IRQ?

 From discussion threads we can find 0 might still used on x86 for a
legacy device.
But the conversations we can find on this don't seem to exclude passing
a negative error number, just that 0 can normally be assumed invalid.

The kerneldoc for SPI says:

  * @irq: Negative, or the number passed to request_irq() to receive
  *	interrupts from this device.

RE: [PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[Simon Trimmer]

> From: Richard Fitzgerald <rf@opensource.cirrus.com>
> Sent: Monday, June 17, 2024 3:34 PM
> On 17/06/2024 15:04, Mark Brown wrote:
> > On Mon, Jun 17, 2024 at 02:53:38PM +0100, Simon Trimmer wrote:
> >> IRQ lookup functions such as those in ACPI can return error values when
> >> an IRQ is not defined. The i2c core driver converts the error codes to
a
> >> value of 0 and the SPI bus driver passes them unaltered to client
device
> >> drivers.
> >>
> >> The cs35l56 driver should only accept positive non-zero values as IRQ
> >> numbers.
> >
> > Have all architectures removed 0 as a valid IRQ?
> 
>  From discussion threads we can find 0 might still used on x86 for a
> legacy device.
> But the conversations we can find on this don't seem to exclude passing
> a negative error number, just that 0 can normally be assumed invalid.
> 
> The kerneldoc for SPI says:
> 
>   * @irq: Negative, or the number passed to request_irq() to receive
>   *	interrupts from this device.

Yes and the threads of these lore links in these commits are rather feisty

ce753ad1549c platform: finally disallow IRQ0 in platform_get_irq() and its
ilk
a85a6c86c25b driver core: platform: Clarify that IRQ 0 is invalid

Re: [PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[Richard Fitzgerald]

On 17/06/2024 15:48, Simon Trimmer wrote:
>> From: Richard Fitzgerald <rf@opensource.cirrus.com>
>> Sent: Monday, June 17, 2024 3:34 PM
>> On 17/06/2024 15:04, Mark Brown wrote:
>>> On Mon, Jun 17, 2024 at 02:53:38PM +0100, Simon Trimmer wrote:
>>>> IRQ lookup functions such as those in ACPI can return error values when
>>>> an IRQ is not defined. The i2c core driver converts the error codes to
> a
>>>> value of 0 and the SPI bus driver passes them unaltered to client
> device
>>>> drivers.
>>>>
>>>> The cs35l56 driver should only accept positive non-zero values as IRQ
>>>> numbers.
>>>
>>> Have all architectures removed 0 as a valid IRQ?
>>
>>   From discussion threads we can find 0 might still used on x86 for a
>> legacy device.
>> But the conversations we can find on this don't seem to exclude passing
>> a negative error number, just that 0 can normally be assumed invalid.
>>
>> The kerneldoc for SPI says:
>>
>>    * @irq: Negative, or the number passed to request_irq() to receive
>>    *	interrupts from this device.
> 
> Yes and the threads of these lore links in these commits are rather feisty
> 
> ce753ad1549c platform: finally disallow IRQ0 in platform_get_irq() and its
> ilk
> a85a6c86c25b driver core: platform: Clarify that IRQ 0 is invalid
> 
> 
So 0 is invalid. Question is: is it also valid to pass -ve errors, or is
0 the _only_ invalid value?

Re: [PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 207 bytes --]

On Mon, Jun 17, 2024 at 03:54:04PM +0100, Richard Fitzgerald wrote:

> So 0 is invalid. Question is: is it also valid to pass -ve errors, or is
> 0 the _only_ invalid value?

Negative values should be fine.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[Richard Fitzgerald]

On 18/06/2024 17:00, Mark Brown wrote:
> On Mon, Jun 17, 2024 at 03:54:04PM +0100, Richard Fitzgerald wrote:
> 
>> So 0 is invalid. Question is: is it also valid to pass -ve errors, or is
>> 0 the _only_ invalid value?
> 
> Negative values should be fine.
In that case this patch is necessary so we reject negative values
as not an IRQ. Otherwise we'll try to request a non-existant IRQ and
fail with an error.

Re: [PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

On Mon, Jun 17, 2024 at 03:33:59PM +0100, Richard Fitzgerald wrote:
> On 17/06/2024 15:04, Mark Brown wrote:

> > Have all architectures removed 0 as a valid IRQ?

> From discussion threads we can find 0 might still used on x86 for a
> legacy device.

Some of the arm platforms were also an issue in the past, though
possibly they've all been modernised by now.  Don't know about other
older architectures.

> But the conversations we can find on this don't seem to exclude passing
> a negative error number, just that 0 can normally be assumed invalid.

Yes, the question was specifically about the assumption that 0 is
invalid.  The status of 0 is kind of a mess, people keep assuming that
it isn't valid and it just depends if users of platforms which try to
use 0 trip up over it.  Sometimes people work on trying to eliminate
uses of 0 but it tends to get you into older code nobody wants to touch.

> The kerneldoc for SPI says:

>  * @irq: Negative, or the number passed to request_irq() to receive
>  *	interrupts from this device.

Which includes the 0 as valid thing...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[Richard Fitzgerald]

On 18/06/2024 16:58, Mark Brown wrote:
> On Mon, Jun 17, 2024 at 03:33:59PM +0100, Richard Fitzgerald wrote:
>> On 17/06/2024 15:04, Mark Brown wrote:
> 
>>> Have all architectures removed 0 as a valid IRQ?
> 
>>  From discussion threads we can find 0 might still used on x86 for a
>> legacy device.
> 
> Some of the arm platforms were also an issue in the past, though
> possibly they've all been modernised by now.  Don't know about other
> older architectures.
> 
>> But the conversations we can find on this don't seem to exclude passing
>> a negative error number, just that 0 can normally be assumed invalid.
> 
> Yes, the question was specifically about the assumption that 0 is
> invalid.  The status of 0 is kind of a mess, people keep assuming that
> it isn't valid and it just depends if users of platforms which try to
> use 0 trip up over it.  Sometimes people work on trying to eliminate
> uses of 0 but it tends to get you into older code nobody wants to touch.
> 
>> The kerneldoc for SPI says:
> 
>>   * @irq: Negative, or the number passed to request_irq() to receive
>>   *	interrupts from this device.
> 
> Which includes the 0 as valid thing...
The statement of truth from Linus Torvalds et al. seems to be that 0 is
invalid except on x86. And on x86 it is specifically reserved for a
legacy timer IRQ so it can't be anything else.

Re: [PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[Richard Fitzgerald]

On 17/06/2024 14:53, Simon Trimmer wrote:
> IRQ lookup functions such as those in ACPI can return error values when
> an IRQ is not defined. The i2c core driver converts the error codes to a
> value of 0 and the SPI bus driver passes them unaltered to client device
> drivers.
> 
> The cs35l56 driver should only accept positive non-zero values as IRQ
> numbers.
> 
> Signed-off-by: Simon Trimmer <simont@opensource.cirrus.com>
> Fixes: 8a731fd37f8b ("ASoC: cs35l56: Move utility functions to shared file")
> ---
>   sound/soc/codecs/cs35l56-shared.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/cs35l56-shared.c b/sound/soc/codecs/cs35l56-shared.c
> index 27869e14e9c8..880228f89baf 100644
> --- a/sound/soc/codecs/cs35l56-shared.c
> +++ b/sound/soc/codecs/cs35l56-shared.c
> @@ -397,7 +397,7 @@ int cs35l56_irq_request(struct cs35l56_base *cs35l56_base, int irq)
>   {
>   	int ret;
>   
> -	if (!irq)
> +	if (irq < 1)
>   		return 0;
>   
>   	ret = devm_request_threaded_irq(cs35l56_base->dev, irq, NULL, cs35l56_irq,

Mark, I don't understand what your objection is.
What is it you want us to do to get this bugfix accepted?

Re: [PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 196 bytes --]

On Wed, Jun 19, 2024 at 10:44:47AM +0100, Richard Fitzgerald wrote:

> Mark, I don't understand what your objection is.
> What is it you want us to do to get this bugfix accepted?

Have patience.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[Richard Fitzgerald]

On 19/06/2024 11:22, Mark Brown wrote:
> On Wed, Jun 19, 2024 at 10:44:47AM +0100, Richard Fitzgerald wrote:
> 
>> Mark, I don't understand what your objection is.
>> What is it you want us to do to get this bugfix accepted?
> 
> Have patience.
Ah, ok. Sorry, I assumed you were objecting not just overloaded.

Re: [PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

On Wed, Jun 19, 2024 at 11:24:06AM +0100, Richard Fitzgerald wrote:

> Ah, ok. Sorry, I assumed you were objecting not just overloaded.

There's a latency between me deciding to apply a patch and the patch
actually ending up in my tree - I test everything which takes time.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers

[Mark Brown]

On Mon, 17 Jun 2024 14:53:38 +0100, Simon Trimmer wrote:
> IRQ lookup functions such as those in ACPI can return error values when
> an IRQ is not defined. The i2c core driver converts the error codes to a
> value of 0 and the SPI bus driver passes them unaltered to client device
> drivers.
> 
> The cs35l56 driver should only accept positive non-zero values as IRQ
> numbers.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: cs35l56: Accept values greater than 0 as IRQ numbers
      commit: 3ec1428d7b7c519d757a013cef908d7e33dee882

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark