| Message ID | 20241001204749.390054-1-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 17d8adc4cd5181c13c1041b197b76efc09eaf8a8 |
| Headers | show |
| Series | ASoC: dt-bindings: davinci-mcasp: Fix interrupts property | expand |
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
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
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
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
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
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
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
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
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(-)