[PATCH] ASoC: dt-bindings: davinci-mcasp: Fix interrupts property

[PATCH] ASoC: dt-bindings: davinci-mcasp: Fix interrupts property

[Miquel Raynal]

My understanding of the interrupts property is that it can either be:
1/ - TX
2/ - TX
   - RX
3/ - Common/combined.

There are very little chances that either:
   - TX
   - Common/combined
or even
   - TX
   - RX
   - Common/combined
could be a thing.

Looking at the interrupt-names definition (which uses oneOf instead of
anyOf), it makes indeed little sense to use anyOf in the interrupts
definition. I believe this is just a mistake, hence let's fix it.

Fixes: 8be90641a0bb ("ASoC: dt-bindings: davinci-mcasp: convert McASP bindings to yaml schema")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
---
 .../devicetree/bindings/sound/davinci-mcasp-audio.yaml          | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
index 7735e08d35ba..ab3206ffa4af 100644
--- a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
+++ b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
@@ -102,7 +102,7 @@ properties:
     default: 2
 
   interrupts:
-    anyOf:
+    oneOf:
       - minItems: 1
         items:
           - description: TX interrupt
-- 
2.43.0

Re: [PATCH] ASoC: dt-bindings: davinci-mcasp: Fix interrupts property

[Krzysztof Kozlowski]

On Tue, Oct 01, 2024 at 10:47:49PM +0200, Miquel Raynal wrote:
> My understanding of the interrupts property is that it can either be:
> 1/ - TX
> 2/ - TX
>    - RX
> 3/ - Common/combined.
> 
> There are very little chances that either:
>    - TX
>    - Common/combined
> or even
>    - TX
>    - RX
>    - Common/combined
> could be a thing.
> 
> Looking at the interrupt-names definition (which uses oneOf instead of
> anyOf), it makes indeed little sense to use anyOf in the interrupts
> definition. I believe this is just a mistake, hence let's fix it.
> 
> Fixes: 8be90641a0bb ("ASoC: dt-bindings: davinci-mcasp: convert McASP bindings to yaml schema")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> ---
>  .../devicetree/bindings/sound/davinci-mcasp-audio.yaml          | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
> index 7735e08d35ba..ab3206ffa4af 100644
> --- a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
> +++ b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
> @@ -102,7 +102,7 @@ properties:
>      default: 2
>  
>    interrupts:
> -    anyOf:
> +    oneOf:


You need to change interrupt-names as well.

Best regards,
Krzysztof

Re: [PATCH] ASoC: dt-bindings: davinci-mcasp: Fix interrupts property

[Miquel Raynal]

Hi Krzysztof,

krzk@kernel.org wrote on Wed, 2 Oct 2024 08:34:44 +0200:

> On Tue, Oct 01, 2024 at 10:47:49PM +0200, Miquel Raynal wrote:
> > My understanding of the interrupts property is that it can either be:
> > 1/ - TX
> > 2/ - TX
> >    - RX
> > 3/ - Common/combined.
> > 
> > There are very little chances that either:
> >    - TX
> >    - Common/combined
> > or even
> >    - TX
> >    - RX
> >    - Common/combined
> > could be a thing.
> > 
> > Looking at the interrupt-names definition (which uses oneOf instead of
> > anyOf), it makes indeed little sense to use anyOf in the interrupts
> > definition. I believe this is just a mistake, hence let's fix it.
> > 
> > Fixes: 8be90641a0bb ("ASoC: dt-bindings: davinci-mcasp: convert McASP bindings to yaml schema")
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > ---
> >  .../devicetree/bindings/sound/davinci-mcasp-audio.yaml          | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
> > index 7735e08d35ba..ab3206ffa4af 100644
> > --- a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
> > +++ b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
> > @@ -102,7 +102,7 @@ properties:
> >      default: 2
> >  
> >    interrupts:
> > -    anyOf:
> > +    oneOf:  
> 
> 
> You need to change interrupt-names as well.

interrupt-names is already using 'oneOf'!

The extended diff looks like that:

   interrupts:
-    anyOf:
+    oneOf:
       - minItems: 1
         items:
           - description: TX interrupt
           - description: RX interrupt
       - items:
           - description: common/combined interrupt
 
   interrupt-names:
     oneOf:
       - minItems: 1
         items:
           - const: tx
           - const: rx
       - const: common

Thanks,
Miquèl

Re: [PATCH] ASoC: dt-bindings: davinci-mcasp: Fix interrupts property

[Krzysztof Kozlowski]

On 02/10/2024 08:56, Miquel Raynal wrote:
> Hi Krzysztof,
> 
> krzk@kernel.org wrote on Wed, 2 Oct 2024 08:34:44 +0200:
> 
>> On Tue, Oct 01, 2024 at 10:47:49PM +0200, Miquel Raynal wrote:
>>> My understanding of the interrupts property is that it can either be:
>>> 1/ - TX
>>> 2/ - TX
>>>    - RX
>>> 3/ - Common/combined.
>>>
>>> There are very little chances that either:
>>>    - TX
>>>    - Common/combined
>>> or even
>>>    - TX
>>>    - RX
>>>    - Common/combined
>>> could be a thing.
>>>
>>> Looking at the interrupt-names definition (which uses oneOf instead of
>>> anyOf), it makes indeed little sense to use anyOf in the interrupts
>>> definition. I believe this is just a mistake, hence let's fix it.
>>>
>>> Fixes: 8be90641a0bb ("ASoC: dt-bindings: davinci-mcasp: convert McASP bindings to yaml schema")
>>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>>> ---
>>> ---
>>>  .../devicetree/bindings/sound/davinci-mcasp-audio.yaml          | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
>>> index 7735e08d35ba..ab3206ffa4af 100644
>>> --- a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
>>> +++ b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
>>> @@ -102,7 +102,7 @@ properties:
>>>      default: 2
>>>  
>>>    interrupts:
>>> -    anyOf:
>>> +    oneOf:  
>>
>>
>> You need to change interrupt-names as well.
> 
> interrupt-names is already using 'oneOf'!
> 
> The extended diff looks like that:

Indeed.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH] ASoC: dt-bindings: davinci-mcasp: Fix interrupts property

[Péter Ujfalusi]

Hi,

On 01/10/2024 23:47, Miquel Raynal wrote:
> My understanding of the interrupts property is that it can either be:
> 1/ - TX
> 2/ - TX
>    - RX
> 3/ - Common/combined.
> 
> There are very little chances that either:
>    - TX
>    - Common/combined
> or even
>    - TX
>    - RX
>    - Common/combined
> could be a thing.

For interrupt these are the valid onesÉ
- Common only
- TX and RX
- TX only
- RX only

The driver cuts this through by trying to request all and leaves it for
DT to specify the correct irqs.

Note: in case of common only, we still have RX+TX, TX only, RX only
operation, but that is just a side note.

> 
> Looking at the interrupt-names definition (which uses oneOf instead of
> anyOf), it makes indeed little sense to use anyOf in the interrupts
> definition. I believe this is just a mistake, hence let's fix it.
> 
> Fixes: 8be90641a0bb ("ASoC: dt-bindings: davinci-mcasp: convert McASP bindings to yaml schema")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> ---
>  .../devicetree/bindings/sound/davinci-mcasp-audio.yaml          | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
> index 7735e08d35ba..ab3206ffa4af 100644
> --- a/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
> +++ b/Documentation/devicetree/bindings/sound/davinci-mcasp-audio.yaml
> @@ -102,7 +102,7 @@ properties:
>      default: 2
>  
>    interrupts:
> -    anyOf:
> +    oneOf:
>        - minItems: 1
>          items:
>            - description: TX interrupt

-- 
Péter

Re: [PATCH] ASoC: dt-bindings: davinci-mcasp: Fix interrupts property

[Miquel Raynal]

Hi Péter,

peter.ujfalusi@gmail.com wrote on Wed, 2 Oct 2024 11:43:56 +0300:

> Hi,
> 
> On 01/10/2024 23:47, Miquel Raynal wrote:
> > My understanding of the interrupts property is that it can either be:
> > 1/ - TX
> > 2/ - TX
> >    - RX
> > 3/ - Common/combined.
> > 
> > There are very little chances that either:
> >    - TX
> >    - Common/combined
> > or even
> >    - TX
> >    - RX
> >    - Common/combined
> > could be a thing.  
> 
> For interrupt these are the valid onesÉ
> - Common only
> - TX and RX
> - TX only
> - RX only

Thanks for the input!

AFAIU, Rx only is currently not a valid description. As you are
providing a description list with minItems = <1>, I think it expects
either the first item or nothing. When I change the example in the yaml
to only give the "rx" interrupt, make dt_binding_check errors out.

I will propose an update.

Thanks,
Miquèl

Re: [PATCH] ASoC: dt-bindings: davinci-mcasp: Fix interrupts property

[Mark Brown]

On Tue, 01 Oct 2024 22:47:49 +0200, Miquel Raynal wrote:
> My understanding of the interrupts property is that it can either be:
> 1/ - TX
> 2/ - TX
>    - RX
> 3/ - Common/combined.
> 
> There are very little chances that either:
>    - TX
>    - Common/combined
> or even
>    - TX
>    - RX
>    - Common/combined
> could be a thing.
> 
> [...]

Applied to

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

Thanks!

[1/1] ASoC: dt-bindings: davinci-mcasp: Fix interrupts property
      commit: 17d8adc4cd5181c13c1041b197b76efc09eaf8a8

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

Re: [PATCH] ASoC: dt-bindings: davinci-mcasp: Fix interrupts property

[Miquel Raynal]

Hi Mark,

broonie@kernel.org wrote on Wed, 02 Oct 2024 18:37:51 +0100:

> On Tue, 01 Oct 2024 22:47:49 +0200, Miquel Raynal wrote:
> > My understanding of the interrupts property is that it can either be:
> > 1/ - TX
> > 2/ - TX
> >    - RX
> > 3/ - Common/combined.
> > 
> > There are very little chances that either:
> >    - TX
> >    - Common/combined
> > or even
> >    - TX
> >    - RX
> >    - Common/combined
> > could be a thing.
> > 
> > [...]  
> 
> Applied to
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

I didn't read your e-mail in time, there is apparently more to fix if
Péter is right, as the current binding still does not allow the "rx"
interrupt alone, while apparently it should. I prepared a second fix.

Thanks,
Miquèl