diff mbox series

[v2,1/4] ASoC: dt-bindings: stm32: i2s: add audio-graph-card port

Message ID 20211125144053.774-2-olivier.moysan@foss.st.com
State Changes Requested, archived
Headers show
Series ARM: dts: stm32: merge spi and i2s nodes | expand

Checks

Context Check Description
robh/checkpatch success
robh/dt-meta-schema success
robh/dtbs-check fail build log

Commit Message

Olivier MOYSAN Nov. 25, 2021, 2:40 p.m. UTC
The STM2 I2S DAI can be connected via the audio-graph-card.
Add port entry into the bindings.

Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
---
 Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Rob Herring Nov. 25, 2021, 9:26 p.m. UTC | #1
On Thu, 25 Nov 2021 15:40:50 +0100, Olivier Moysan wrote:
> The STM2 I2S DAI can be connected via the audio-graph-card.
> Add port entry into the bindings.
> 
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> ---
>  Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/1559750


audio-controller@4000b000: 'port' does not match any of the regexes: '^port@[0-9]', 'pinctrl-[0-9]+'
	arch/arm/boot/dts/stm32mp157a-dk1.dt.yaml
	arch/arm/boot/dts/stm32mp157c-dk2.dt.yaml
Olivier MOYSAN Nov. 26, 2021, 10:25 a.m. UTC | #2
Hi Rob,

On 11/25/21 10:26 PM, Rob Herring wrote:
> On Thu, 25 Nov 2021 15:40:50 +0100, Olivier Moysan wrote:
>> The STM2 I2S DAI can be connected via the audio-graph-card.
>> Add port entry into the bindings.
>>
>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>> ---
>>   Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
> 
> Running 'make dtbs_check' with the schema in this patch gives the
> following warnings. Consider if they are expected or the schema is
> incorrect. These may not be new warnings.
> 
> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> This will change in the future.
> 
> Full log is available here: https://patchwork.ozlabs.org/patch/1559750
> 
> 
> audio-controller@4000b000: 'port' does not match any of the regexes: '^port@[0-9]', 'pinctrl-[0-9]+'
> 	arch/arm/boot/dts/stm32mp157a-dk1.dt.yaml
> 	arch/arm/boot/dts/stm32mp157c-dk2.dt.yaml
> 

This warning is not a new one.

The i2s2 node in stm32mp15xx-dkx.dtsi would require the following binding:
port:
	$ref: audio-graph-port.yaml#
	unevaluatedProperties: false

However the spi binding requires to introduce a unit address:
patternProperties:
   '^port@[0-9]':
     $ref: audio-graph-port.yaml#
     unevaluatedProperties: false

The warning can be removed by re-ordering the bindings patches in the 
serie, as "additionalProperties: true" makes the check more tolerant on 
extra properties.
The patch "ASoC: dt-bindings: stm32: i2s: add audio-graph-card port" can 
even be merely dropped.
So, I suggest to resend the serie without audio-graph-card patch.

Does it sound too permissive to you ?

Thanks
Olivier
Rob Herring Dec. 1, 2021, 10:34 p.m. UTC | #3
On Fri, Nov 26, 2021 at 11:25:27AM +0100, Olivier MOYSAN wrote:
> Hi Rob,
> 
> On 11/25/21 10:26 PM, Rob Herring wrote:
> > On Thu, 25 Nov 2021 15:40:50 +0100, Olivier Moysan wrote:
> > > The STM2 I2S DAI can be connected via the audio-graph-card.
> > > Add port entry into the bindings.
> > > 
> > > Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> > > ---
> > >   Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > > 
> > 
> > Running 'make dtbs_check' with the schema in this patch gives the
> > following warnings. Consider if they are expected or the schema is
> > incorrect. These may not be new warnings.
> > 
> > Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> > This will change in the future.
> > 
> > Full log is available here: https://patchwork.ozlabs.org/patch/1559750
> > 
> > 
> > audio-controller@4000b000: 'port' does not match any of the regexes: '^port@[0-9]', 'pinctrl-[0-9]+'
> > 	arch/arm/boot/dts/stm32mp157a-dk1.dt.yaml
> > 	arch/arm/boot/dts/stm32mp157c-dk2.dt.yaml
> > 
> 
> This warning is not a new one.
> 
> The i2s2 node in stm32mp15xx-dkx.dtsi would require the following binding:
> port:
> 	$ref: audio-graph-port.yaml#
> 	unevaluatedProperties: false
> 
> However the spi binding requires to introduce a unit address:
> patternProperties:
>   '^port@[0-9]':
>     $ref: audio-graph-port.yaml#
>     unevaluatedProperties: false
> 
> The warning can be removed by re-ordering the bindings patches in the serie,
> as "additionalProperties: true" makes the check more tolerant on extra
> properties.

That's never right.

> The patch "ASoC: dt-bindings: stm32: i2s: add audio-graph-card port" can
> even be merely dropped.
> So, I suggest to resend the serie without audio-graph-card patch.

Only if you aren't using audio-graph-card.

> 
> Does it sound too permissive to you ?

I think perhaps you need to combine the schemas into 1. Or you need to 
restructure your dtsi files such that you only add spi specific 
properties when spi mode is enabled and only add i2s specific properties 
when i2s mode is enabled. Or use the /delete-property/ directive.

Rob
Alexandre TORGUE Dec. 7, 2021, 1:52 p.m. UTC | #4
Hi Rob

On 12/1/21 11:34 PM, Rob Herring wrote:
> On Fri, Nov 26, 2021 at 11:25:27AM +0100, Olivier MOYSAN wrote:
>> Hi Rob,
>>
>> On 11/25/21 10:26 PM, Rob Herring wrote:
>>> On Thu, 25 Nov 2021 15:40:50 +0100, Olivier Moysan wrote:
>>>> The STM2 I2S DAI can be connected via the audio-graph-card.
>>>> Add port entry into the bindings.
>>>>
>>>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>>>> ---
>>>>    Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml | 5 +++++
>>>>    1 file changed, 5 insertions(+)
>>>>
>>>
>>> Running 'make dtbs_check' with the schema in this patch gives the
>>> following warnings. Consider if they are expected or the schema is
>>> incorrect. These may not be new warnings.
>>>
>>> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
>>> This will change in the future.
>>>
>>> Full log is available here: https://patchwork.ozlabs.org/patch/1559750
>>>
>>>
>>> audio-controller@4000b000: 'port' does not match any of the regexes: '^port@[0-9]', 'pinctrl-[0-9]+'
>>> 	arch/arm/boot/dts/stm32mp157a-dk1.dt.yaml
>>> 	arch/arm/boot/dts/stm32mp157c-dk2.dt.yaml
>>>
>>
>> This warning is not a new one.
>>
>> The i2s2 node in stm32mp15xx-dkx.dtsi would require the following binding:
>> port:
>> 	$ref: audio-graph-port.yaml#
>> 	unevaluatedProperties: false
>>
>> However the spi binding requires to introduce a unit address:
>> patternProperties:
>>    '^port@[0-9]':
>>      $ref: audio-graph-port.yaml#
>>      unevaluatedProperties: false
>>
>> The warning can be removed by re-ordering the bindings patches in the serie,
>> as "additionalProperties: true" makes the check more tolerant on extra
>> properties.
> 
> That's never right.
> 
>> The patch "ASoC: dt-bindings: stm32: i2s: add audio-graph-card port" can
>> even be merely dropped.
>> So, I suggest to resend the serie without audio-graph-card patch.
> 
> Only if you aren't using audio-graph-card.
> 
>>
>> Does it sound too permissive to you ?
> 
> I think perhaps you need to combine the schemas into 1. Or you need to
> restructure your dtsi files such that you only add spi specific
> properties when spi mode is enabled and only add i2s specific properties
> when i2s mode is enabled. Or use the /delete-property/ directive.

Initially the aim of this series was to fix a "make W=1" warnings seen 
on spi and i2s nodes (duplicate unit-address). Moving both nodes in a 
common node + using a different compatible depending on SPI or I2S usage 
sounded good) but it is not enough. In this series the common node is 
named as following: "spi2s2: spi@4000b000". It is fine for a spi usage 
but if we want to use this "common node" with I2S compatible and 
specific bindings, the node name remains spi@... and then specific spi 
checks are done. For this with this series applied we got this issue 
reported by spi-controller.yaml:

spi@4000b000: port@0: 'compatible' is a required property

So, if we use two separates nodes we got W=1 warning and if we use a 
common node we got yaml check issue. One possibility would be to use a 
common node with a new node name (for example i2spi@...) but I think it 
is not acceptable.

How to progress ?

Thanks
Alex


> Rob
>
Ahmad Fatoum Dec. 7, 2021, 1:59 p.m. UTC | #5
Hello Alex,

On 07.12.21 14:52, Alexandre TORGUE wrote:
> Hi Rob
> 
> On 12/1/21 11:34 PM, Rob Herring wrote:
>> On Fri, Nov 26, 2021 at 11:25:27AM +0100, Olivier MOYSAN wrote:
>>> Hi Rob,
>>>
>>> On 11/25/21 10:26 PM, Rob Herring wrote:
>>>> On Thu, 25 Nov 2021 15:40:50 +0100, Olivier Moysan wrote:
>>>>> The STM2 I2S DAI can be connected via the audio-graph-card.
>>>>> Add port entry into the bindings.
>>>>>
>>>>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>>>>> ---
>>>>>    Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml | 5 +++++
>>>>>    1 file changed, 5 insertions(+)
>>>>>
>>>>
>>>> Running 'make dtbs_check' with the schema in this patch gives the
>>>> following warnings. Consider if they are expected or the schema is
>>>> incorrect. These may not be new warnings.
>>>>
>>>> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
>>>> This will change in the future.
>>>>
>>>> Full log is available here: https://patchwork.ozlabs.org/patch/1559750
>>>>
>>>>
>>>> audio-controller@4000b000: 'port' does not match any of the regexes: '^port@[0-9]', 'pinctrl-[0-9]+'
>>>>     arch/arm/boot/dts/stm32mp157a-dk1.dt.yaml
>>>>     arch/arm/boot/dts/stm32mp157c-dk2.dt.yaml
>>>>
>>>
>>> This warning is not a new one.
>>>
>>> The i2s2 node in stm32mp15xx-dkx.dtsi would require the following binding:
>>> port:
>>>     $ref: audio-graph-port.yaml#
>>>     unevaluatedProperties: false
>>>
>>> However the spi binding requires to introduce a unit address:
>>> patternProperties:
>>>    '^port@[0-9]':
>>>      $ref: audio-graph-port.yaml#
>>>      unevaluatedProperties: false
>>>
>>> The warning can be removed by re-ordering the bindings patches in the serie,
>>> as "additionalProperties: true" makes the check more tolerant on extra
>>> properties.
>>
>> That's never right.
>>
>>> The patch "ASoC: dt-bindings: stm32: i2s: add audio-graph-card port" can
>>> even be merely dropped.
>>> So, I suggest to resend the serie without audio-graph-card patch.
>>
>> Only if you aren't using audio-graph-card.
>>
>>>
>>> Does it sound too permissive to you ?
>>
>> I think perhaps you need to combine the schemas into 1. Or you need to
>> restructure your dtsi files such that you only add spi specific
>> properties when spi mode is enabled and only add i2s specific properties
>> when i2s mode is enabled. Or use the /delete-property/ directive.
> 
> Initially the aim of this series was to fix a "make W=1" warnings seen on spi and i2s nodes (duplicate unit-address). Moving both nodes in a common node + using a different compatible depending on SPI or I2S usage sounded good) but it is not enough. In this series the common node is named as following: "spi2s2: spi@4000b000". It is fine for a spi usage but if we want to use this "common node" with I2S compatible and specific bindings, the node name remains spi@... and then specific spi checks are done. For this with this series applied we got this issue reported by spi-controller.yaml:
> 
> spi@4000b000: port@0: 'compatible' is a required property
> 
> So, if we use two separates nodes we got W=1 warning and if we use a common node we got yaml check issue. One possibility would be to use a common node with a new node name (for example i2spi@...) but I think it is not acceptable.
> 
> How to progress ?

Atmel Flexcom can be configured to be either UART, SPI or i2c. Functions
are child nodes of the flexcom node and the MFD driver matching against it,
just configure the operating mode and then calls of_platform_populate.

Would something along these lines fit here as well?

Cheers,
Ahmad

> 
> Thanks
> Alex
> 
> 
>> Rob
>>
> 
> _______________________________________________
> Linux-stm32 mailing list
> Linux-stm32@st-md-mailman.stormreply.com
> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32
>
Alexandre TORGUE Dec. 8, 2021, 9:55 a.m. UTC | #6
Hi Ahmad

On 12/7/21 2:59 PM, Ahmad Fatoum wrote:
> Hello Alex,
> 
> On 07.12.21 14:52, Alexandre TORGUE wrote:
>> Hi Rob
>>
>> On 12/1/21 11:34 PM, Rob Herring wrote:
>>> On Fri, Nov 26, 2021 at 11:25:27AM +0100, Olivier MOYSAN wrote:
>>>> Hi Rob,
>>>>
>>>> On 11/25/21 10:26 PM, Rob Herring wrote:
>>>>> On Thu, 25 Nov 2021 15:40:50 +0100, Olivier Moysan wrote:
>>>>>> The STM2 I2S DAI can be connected via the audio-graph-card.
>>>>>> Add port entry into the bindings.
>>>>>>
>>>>>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>>>>>> ---
>>>>>>     Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml | 5 +++++
>>>>>>     1 file changed, 5 insertions(+)
>>>>>>
>>>>>
>>>>> Running 'make dtbs_check' with the schema in this patch gives the
>>>>> following warnings. Consider if they are expected or the schema is
>>>>> incorrect. These may not be new warnings.
>>>>>
>>>>> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
>>>>> This will change in the future.
>>>>>
>>>>> Full log is available here: https://patchwork.ozlabs.org/patch/1559750
>>>>>
>>>>>
>>>>> audio-controller@4000b000: 'port' does not match any of the regexes: '^port@[0-9]', 'pinctrl-[0-9]+'
>>>>>      arch/arm/boot/dts/stm32mp157a-dk1.dt.yaml
>>>>>      arch/arm/boot/dts/stm32mp157c-dk2.dt.yaml
>>>>>
>>>>
>>>> This warning is not a new one.
>>>>
>>>> The i2s2 node in stm32mp15xx-dkx.dtsi would require the following binding:
>>>> port:
>>>>      $ref: audio-graph-port.yaml#
>>>>      unevaluatedProperties: false
>>>>
>>>> However the spi binding requires to introduce a unit address:
>>>> patternProperties:
>>>>     '^port@[0-9]':
>>>>       $ref: audio-graph-port.yaml#
>>>>       unevaluatedProperties: false
>>>>
>>>> The warning can be removed by re-ordering the bindings patches in the serie,
>>>> as "additionalProperties: true" makes the check more tolerant on extra
>>>> properties.
>>>
>>> That's never right.
>>>
>>>> The patch "ASoC: dt-bindings: stm32: i2s: add audio-graph-card port" can
>>>> even be merely dropped.
>>>> So, I suggest to resend the serie without audio-graph-card patch.
>>>
>>> Only if you aren't using audio-graph-card.
>>>
>>>>
>>>> Does it sound too permissive to you ?
>>>
>>> I think perhaps you need to combine the schemas into 1. Or you need to
>>> restructure your dtsi files such that you only add spi specific
>>> properties when spi mode is enabled and only add i2s specific properties
>>> when i2s mode is enabled. Or use the /delete-property/ directive.
>>
>> Initially the aim of this series was to fix a "make W=1" warnings seen on spi and i2s nodes (duplicate unit-address). Moving both nodes in a common node + using a different compatible depending on SPI or I2S usage sounded good) but it is not enough. In this series the common node is named as following: "spi2s2: spi@4000b000". It is fine for a spi usage but if we want to use this "common node" with I2S compatible and specific bindings, the node name remains spi@... and then specific spi checks are done. For this with this series applied we got this issue reported by spi-controller.yaml:
>>
>> spi@4000b000: port@0: 'compatible' is a required property
>>
>> So, if we use two separates nodes we got W=1 warning and if we use a common node we got yaml check issue. One possibility would be to use a common node with a new node name (for example i2spi@...) but I think it is not acceptable.
>>
>> How to progress ?
> 
> Atmel Flexcom can be configured to be either UART, SPI or i2c. Functions
> are child nodes of the flexcom node and the MFD driver matching against it,
> just configure the operating mode and then calls of_platform_populate.
> 
> Would something along these lines fit here as well?

Yes it could but in my mind it was not a MFD as both feature cannot be 
used at the same time: it is either SPI or I2S and choice is done 
"statically" in device tree depending board usage.

Lee, what it is your feeling about that ? Will you accept to add a MFD 
driver for this SPI/I2S peripheral whose prurpose is only to populate 
child node (either SPI or I2S) ?

Cheers
Alex

> 
> Cheers,
> Ahmad
> 
>>
>> Thanks
>> Alex
>>
>>
>>> Rob
>>>
>>
>> _______________________________________________
>> Linux-stm32 mailing list
>> Linux-stm32@st-md-mailman.stormreply.com
>> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32
>>
> 
>
Lee Jones Dec. 8, 2021, 12:08 p.m. UTC | #7
On Wed, 08 Dec 2021, Alexandre TORGUE wrote:

> Hi Ahmad
> 
> On 12/7/21 2:59 PM, Ahmad Fatoum wrote:
> > Hello Alex,
> > 
> > On 07.12.21 14:52, Alexandre TORGUE wrote:
> > > Hi Rob
> > > 
> > > On 12/1/21 11:34 PM, Rob Herring wrote:
> > > > On Fri, Nov 26, 2021 at 11:25:27AM +0100, Olivier MOYSAN wrote:
> > > > > Hi Rob,
> > > > > 
> > > > > On 11/25/21 10:26 PM, Rob Herring wrote:
> > > > > > On Thu, 25 Nov 2021 15:40:50 +0100, Olivier Moysan wrote:
> > > > > > > The STM2 I2S DAI can be connected via the audio-graph-card.
> > > > > > > Add port entry into the bindings.
> > > > > > > 
> > > > > > > Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> > > > > > > ---
> > > > > > >     Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml | 5 +++++
> > > > > > >     1 file changed, 5 insertions(+)
> > > > > > > 
> > > > > > 
> > > > > > Running 'make dtbs_check' with the schema in this patch gives the
> > > > > > following warnings. Consider if they are expected or the schema is
> > > > > > incorrect. These may not be new warnings.
> > > > > > 
> > > > > > Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> > > > > > This will change in the future.
> > > > > > 
> > > > > > Full log is available here: https://patchwork.ozlabs.org/patch/1559750
> > > > > > 
> > > > > > 
> > > > > > audio-controller@4000b000: 'port' does not match any of the regexes: '^port@[0-9]', 'pinctrl-[0-9]+'
> > > > > >      arch/arm/boot/dts/stm32mp157a-dk1.dt.yaml
> > > > > >      arch/arm/boot/dts/stm32mp157c-dk2.dt.yaml
> > > > > > 
> > > > > 
> > > > > This warning is not a new one.
> > > > > 
> > > > > The i2s2 node in stm32mp15xx-dkx.dtsi would require the following binding:
> > > > > port:
> > > > >      $ref: audio-graph-port.yaml#
> > > > >      unevaluatedProperties: false
> > > > > 
> > > > > However the spi binding requires to introduce a unit address:
> > > > > patternProperties:
> > > > >     '^port@[0-9]':
> > > > >       $ref: audio-graph-port.yaml#
> > > > >       unevaluatedProperties: false
> > > > > 
> > > > > The warning can be removed by re-ordering the bindings patches in the serie,
> > > > > as "additionalProperties: true" makes the check more tolerant on extra
> > > > > properties.
> > > > 
> > > > That's never right.
> > > > 
> > > > > The patch "ASoC: dt-bindings: stm32: i2s: add audio-graph-card port" can
> > > > > even be merely dropped.
> > > > > So, I suggest to resend the serie without audio-graph-card patch.
> > > > 
> > > > Only if you aren't using audio-graph-card.
> > > > 
> > > > > 
> > > > > Does it sound too permissive to you ?
> > > > 
> > > > I think perhaps you need to combine the schemas into 1. Or you need to
> > > > restructure your dtsi files such that you only add spi specific
> > > > properties when spi mode is enabled and only add i2s specific properties
> > > > when i2s mode is enabled. Or use the /delete-property/ directive.
> > > 
> > > Initially the aim of this series was to fix a "make W=1" warnings seen on spi and i2s nodes (duplicate unit-address). Moving both nodes in a common node + using a different compatible depending on SPI or I2S usage sounded good) but it is not enough. In this series the common node is named as following: "spi2s2: spi@4000b000". It is fine for a spi usage but if we want to use this "common node" with I2S compatible and specific bindings, the node name remains spi@... and then specific spi checks are done. For this with this series applied we got this issue reported by spi-controller.yaml:
> > > 
> > > spi@4000b000: port@0: 'compatible' is a required property
> > > 
> > > So, if we use two separates nodes we got W=1 warning and if we use a common node we got yaml check issue. One possibility would be to use a common node with a new node name (for example i2spi@...) but I think it is not acceptable.
> > > 
> > > How to progress ?
> > 
> > Atmel Flexcom can be configured to be either UART, SPI or i2c. Functions
> > are child nodes of the flexcom node and the MFD driver matching against it,
> > just configure the operating mode and then calls of_platform_populate.
> > 
> > Would something along these lines fit here as well?
> 
> Yes it could but in my mind it was not a MFD as both feature cannot be used
> at the same time: it is either SPI or I2S and choice is done "statically" in
> device tree depending board usage.
> 
> Lee, what it is your feeling about that ? Will you accept to add a MFD
> driver for this SPI/I2S peripheral whose prurpose is only to populate child
> node (either SPI or I2S) ?

From your description, this doesn't sound like a good fit for MFD.
Alexandre TORGUE Dec. 8, 2021, 1:34 p.m. UTC | #8
On 12/8/21 1:08 PM, Lee Jones wrote:
> On Wed, 08 Dec 2021, Alexandre TORGUE wrote:
> 
>> Hi Ahmad
>>
>> On 12/7/21 2:59 PM, Ahmad Fatoum wrote:
>>> Hello Alex,
>>>
>>> On 07.12.21 14:52, Alexandre TORGUE wrote:
>>>> Hi Rob
>>>>
>>>> On 12/1/21 11:34 PM, Rob Herring wrote:
>>>>> On Fri, Nov 26, 2021 at 11:25:27AM +0100, Olivier MOYSAN wrote:
>>>>>> Hi Rob,
>>>>>>
>>>>>> On 11/25/21 10:26 PM, Rob Herring wrote:
>>>>>>> On Thu, 25 Nov 2021 15:40:50 +0100, Olivier Moysan wrote:
>>>>>>>> The STM2 I2S DAI can be connected via the audio-graph-card.
>>>>>>>> Add port entry into the bindings.
>>>>>>>>
>>>>>>>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>>>>>>>> ---
>>>>>>>>      Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml | 5 +++++
>>>>>>>>      1 file changed, 5 insertions(+)
>>>>>>>>
>>>>>>>
>>>>>>> Running 'make dtbs_check' with the schema in this patch gives the
>>>>>>> following warnings. Consider if they are expected or the schema is
>>>>>>> incorrect. These may not be new warnings.
>>>>>>>
>>>>>>> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
>>>>>>> This will change in the future.
>>>>>>>
>>>>>>> Full log is available here: https://patchwork.ozlabs.org/patch/1559750
>>>>>>>
>>>>>>>
>>>>>>> audio-controller@4000b000: 'port' does not match any of the regexes: '^port@[0-9]', 'pinctrl-[0-9]+'
>>>>>>>       arch/arm/boot/dts/stm32mp157a-dk1.dt.yaml
>>>>>>>       arch/arm/boot/dts/stm32mp157c-dk2.dt.yaml
>>>>>>>
>>>>>>
>>>>>> This warning is not a new one.
>>>>>>
>>>>>> The i2s2 node in stm32mp15xx-dkx.dtsi would require the following binding:
>>>>>> port:
>>>>>>       $ref: audio-graph-port.yaml#
>>>>>>       unevaluatedProperties: false
>>>>>>
>>>>>> However the spi binding requires to introduce a unit address:
>>>>>> patternProperties:
>>>>>>      '^port@[0-9]':
>>>>>>        $ref: audio-graph-port.yaml#
>>>>>>        unevaluatedProperties: false
>>>>>>
>>>>>> The warning can be removed by re-ordering the bindings patches in the serie,
>>>>>> as "additionalProperties: true" makes the check more tolerant on extra
>>>>>> properties.
>>>>>
>>>>> That's never right.
>>>>>
>>>>>> The patch "ASoC: dt-bindings: stm32: i2s: add audio-graph-card port" can
>>>>>> even be merely dropped.
>>>>>> So, I suggest to resend the serie without audio-graph-card patch.
>>>>>
>>>>> Only if you aren't using audio-graph-card.
>>>>>
>>>>>>
>>>>>> Does it sound too permissive to you ?
>>>>>
>>>>> I think perhaps you need to combine the schemas into 1. Or you need to
>>>>> restructure your dtsi files such that you only add spi specific
>>>>> properties when spi mode is enabled and only add i2s specific properties
>>>>> when i2s mode is enabled. Or use the /delete-property/ directive.
>>>>
>>>> Initially the aim of this series was to fix a "make W=1" warnings seen on spi and i2s nodes (duplicate unit-address). Moving both nodes in a common node + using a different compatible depending on SPI or I2S usage sounded good) but it is not enough. In this series the common node is named as following: "spi2s2: spi@4000b000". It is fine for a spi usage but if we want to use this "common node" with I2S compatible and specific bindings, the node name remains spi@... and then specific spi checks are done. For this with this series applied we got this issue reported by spi-controller.yaml:
>>>>
>>>> spi@4000b000: port@0: 'compatible' is a required property
>>>>
>>>> So, if we use two separates nodes we got W=1 warning and if we use a common node we got yaml check issue. One possibility would be to use a common node with a new node name (for example i2spi@...) but I think it is not acceptable.
>>>>
>>>> How to progress ?
>>>
>>> Atmel Flexcom can be configured to be either UART, SPI or i2c. Functions
>>> are child nodes of the flexcom node and the MFD driver matching against it,
>>> just configure the operating mode and then calls of_platform_populate.
>>>
>>> Would something along these lines fit here as well?
>>
>> Yes it could but in my mind it was not a MFD as both feature cannot be used
>> at the same time: it is either SPI or I2S and choice is done "statically" in
>> device tree depending board usage.
>>
>> Lee, what it is your feeling about that ? Will you accept to add a MFD
>> driver for this SPI/I2S peripheral whose prurpose is only to populate child
>> node (either SPI or I2S) ?
> 
>  From your description, this doesn't sound like a good fit for MFD.

Thanks Lee for your quick answer. So rename the node frop spi@... to 
i2spi@... (or something else) looks like to be the only solution. 
Depending the compatible used the well schema will be used (if well 
referenced in each stm32 spi and i2s yaml files):

--> spi-controller.yaml in case of "st,stm32h7-spi"

-->audio-controller in case of "st,stm32h7-i2s"

Rob, do you agree?

regards
alex

>
Rob Herring Dec. 11, 2021, 8:05 p.m. UTC | #9
On Tue, Dec 7, 2021 at 7:52 AM Alexandre TORGUE
<alexandre.torgue@foss.st.com> wrote:
>
> Hi Rob
>
> On 12/1/21 11:34 PM, Rob Herring wrote:
> > On Fri, Nov 26, 2021 at 11:25:27AM +0100, Olivier MOYSAN wrote:
> >> Hi Rob,
> >>
> >> On 11/25/21 10:26 PM, Rob Herring wrote:
> >>> On Thu, 25 Nov 2021 15:40:50 +0100, Olivier Moysan wrote:
> >>>> The STM2 I2S DAI can be connected via the audio-graph-card.
> >>>> Add port entry into the bindings.
> >>>>
> >>>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> >>>> ---
> >>>>    Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml | 5 +++++
> >>>>    1 file changed, 5 insertions(+)
> >>>>
> >>>
> >>> Running 'make dtbs_check' with the schema in this patch gives the
> >>> following warnings. Consider if they are expected or the schema is
> >>> incorrect. These may not be new warnings.
> >>>
> >>> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> >>> This will change in the future.
> >>>
> >>> Full log is available here: https://patchwork.ozlabs.org/patch/1559750
> >>>
> >>>
> >>> audio-controller@4000b000: 'port' does not match any of the regexes: '^port@[0-9]', 'pinctrl-[0-9]+'
> >>>     arch/arm/boot/dts/stm32mp157a-dk1.dt.yaml
> >>>     arch/arm/boot/dts/stm32mp157c-dk2.dt.yaml
> >>>
> >>
> >> This warning is not a new one.
> >>
> >> The i2s2 node in stm32mp15xx-dkx.dtsi would require the following binding:
> >> port:
> >>      $ref: audio-graph-port.yaml#
> >>      unevaluatedProperties: false
> >>
> >> However the spi binding requires to introduce a unit address:
> >> patternProperties:
> >>    '^port@[0-9]':
> >>      $ref: audio-graph-port.yaml#
> >>      unevaluatedProperties: false
> >>
> >> The warning can be removed by re-ordering the bindings patches in the serie,
> >> as "additionalProperties: true" makes the check more tolerant on extra
> >> properties.
> >
> > That's never right.
> >
> >> The patch "ASoC: dt-bindings: stm32: i2s: add audio-graph-card port" can
> >> even be merely dropped.
> >> So, I suggest to resend the serie without audio-graph-card patch.
> >
> > Only if you aren't using audio-graph-card.
> >
> >>
> >> Does it sound too permissive to you ?
> >
> > I think perhaps you need to combine the schemas into 1. Or you need to
> > restructure your dtsi files such that you only add spi specific
> > properties when spi mode is enabled and only add i2s specific properties
> > when i2s mode is enabled. Or use the /delete-property/ directive.
>
> Initially the aim of this series was to fix a "make W=1" warnings seen
> on spi and i2s nodes (duplicate unit-address). Moving both nodes in a
> common node + using a different compatible depending on SPI or I2S usage
> sounded good) but it is not enough. In this series the common node is
> named as following: "spi2s2: spi@4000b000". It is fine for a spi usage
> but if we want to use this "common node" with I2S compatible and
> specific bindings, the node name remains spi@... and then specific spi
> checks are done. For this with this series applied we got this issue
> reported by spi-controller.yaml:
>
> spi@4000b000: port@0: 'compatible' is a required property
>
> So, if we use two separates nodes we got W=1 warning and if we use a
> common node we got yaml check issue. One possibility would be to use a
> common node with a new node name (for example i2spi@...) but I think it
> is not acceptable.

It is acceptable, see this thread[1].

Rob

[1] https://lore.kernel.org/all/20211203183517.11390-1-semen.protsenko@linaro.org/
Alexandre TORGUE Dec. 13, 2021, 4:26 p.m. UTC | #10
Hi Rob,

On 12/11/21 9:05 PM, Rob Herring wrote:
> On Tue, Dec 7, 2021 at 7:52 AM Alexandre TORGUE
> <alexandre.torgue@foss.st.com> wrote:
>>
>> Hi Rob
>>
>> On 12/1/21 11:34 PM, Rob Herring wrote:
>>> On Fri, Nov 26, 2021 at 11:25:27AM +0100, Olivier MOYSAN wrote:
>>>> Hi Rob,
>>>>
>>>> On 11/25/21 10:26 PM, Rob Herring wrote:
>>>>> On Thu, 25 Nov 2021 15:40:50 +0100, Olivier Moysan wrote:
>>>>>> The STM2 I2S DAI can be connected via the audio-graph-card.
>>>>>> Add port entry into the bindings.
>>>>>>
>>>>>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>>>>>> ---
>>>>>>     Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml | 5 +++++
>>>>>>     1 file changed, 5 insertions(+)
>>>>>>
>>>>>
>>>>> Running 'make dtbs_check' with the schema in this patch gives the
>>>>> following warnings. Consider if they are expected or the schema is
>>>>> incorrect. These may not be new warnings.
>>>>>
>>>>> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
>>>>> This will change in the future.
>>>>>
>>>>> Full log is available here: https://patchwork.ozlabs.org/patch/1559750
>>>>>
>>>>>
>>>>> audio-controller@4000b000: 'port' does not match any of the regexes: '^port@[0-9]', 'pinctrl-[0-9]+'
>>>>>      arch/arm/boot/dts/stm32mp157a-dk1.dt.yaml
>>>>>      arch/arm/boot/dts/stm32mp157c-dk2.dt.yaml
>>>>>
>>>>
>>>> This warning is not a new one.
>>>>
>>>> The i2s2 node in stm32mp15xx-dkx.dtsi would require the following binding:
>>>> port:
>>>>       $ref: audio-graph-port.yaml#
>>>>       unevaluatedProperties: false
>>>>
>>>> However the spi binding requires to introduce a unit address:
>>>> patternProperties:
>>>>     '^port@[0-9]':
>>>>       $ref: audio-graph-port.yaml#
>>>>       unevaluatedProperties: false
>>>>
>>>> The warning can be removed by re-ordering the bindings patches in the serie,
>>>> as "additionalProperties: true" makes the check more tolerant on extra
>>>> properties.
>>>
>>> That's never right.
>>>
>>>> The patch "ASoC: dt-bindings: stm32: i2s: add audio-graph-card port" can
>>>> even be merely dropped.
>>>> So, I suggest to resend the serie without audio-graph-card patch.
>>>
>>> Only if you aren't using audio-graph-card.
>>>
>>>>
>>>> Does it sound too permissive to you ?
>>>
>>> I think perhaps you need to combine the schemas into 1. Or you need to
>>> restructure your dtsi files such that you only add spi specific
>>> properties when spi mode is enabled and only add i2s specific properties
>>> when i2s mode is enabled. Or use the /delete-property/ directive.
>>
>> Initially the aim of this series was to fix a "make W=1" warnings seen
>> on spi and i2s nodes (duplicate unit-address). Moving both nodes in a
>> common node + using a different compatible depending on SPI or I2S usage
>> sounded good) but it is not enough. In this series the common node is
>> named as following: "spi2s2: spi@4000b000". It is fine for a spi usage
>> but if we want to use this "common node" with I2S compatible and
>> specific bindings, the node name remains spi@... and then specific spi
>> checks are done. For this with this series applied we got this issue
>> reported by spi-controller.yaml:
>>
>> spi@4000b000: port@0: 'compatible' is a required property
>>
>> So, if we use two separates nodes we got W=1 warning and if we use a
>> common node we got yaml check issue. One possibility would be to use a
>> common node with a new node name (for example i2spi@...) but I think it
>> is not acceptable.
> 
> It is acceptable, see this thread[1].
> 
> Rob
> 
> [1] https://lore.kernel.org/all/20211203183517.11390-1-semen.protsenko@linaro.org/

Perfect! So we can abandon this series. Do you know if a patch has been 
sent to also update scripts/Makefile.lib ?

Regards
Alex


>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml b/Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml
index 6feb5a09c184..64b70ac539f8 100644
--- a/Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml
+++ b/Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml
@@ -58,6 +58,11 @@  properties:
     description: Configure the I2S device as MCLK clock provider.
     const: 0
 
+patternProperties:
+  '^port@[0-9]':
+    $ref: audio-graph-port.yaml#
+    unevaluatedProperties: false
+
 required:
   - compatible
   - "#sound-dai-cells"