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

From: Alexandre TORGUE
Date: Mon Dec 13 2021 - 11:27:06 EST


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@xxxxxxxxxxx> 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@xxxxxxxxxxx>
---
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@xxxxxxxxxx/

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

Regards
Alex