| Message ID | 87y0w21a4h.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: add Renesas MSIOF sound driver | expand |
Hi Morimoto-san, On Tue, 15 Apr 2025 at 03:33, Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote: > Renesas MSIOF (Clock-Synchronized Serial Interface with FIFO) can work as > both SPI and I2S. MSIOF-I2S will use Audio Graph Card/Card2 driver which > uses Of-Graph in DT. > > MSIOF-SPI/I2S are using same DT compatible properties. > MSIOF-I2S uses Of-Graph for Audio-Graph-Card/Card2, > MSIOF-SPI doesn't use Of-Graph. > > Adds schema for MSIOF-I2S (= Sound). > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Thanks for the update! > --- a/Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml > +++ b/Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml > @@ -4,14 +4,11 @@ > $id: http://devicetree.org/schemas/spi/renesas,sh-msiof.yaml# > $schema: http://devicetree.org/meta-schemas/core.yaml# > > -title: Renesas MSIOF SPI controller > +title: Renesas MSIOF SPI / I2S controller > > maintainers: > - Geert Uytterhoeven <geert+renesas@glider.be> > > -allOf: > - - $ref: spi-controller.yaml# > - > properties: > compatible: > oneOf: > @@ -146,24 +143,38 @@ properties: > $ref: /schemas/types.yaml#/definitions/uint32 > default: 64 > > + # for MSIOF-I2S > + port: > + $ref: ../sound/audio-graph-port.yaml# > + unevaluatedProperties: false > + > required: > - compatible > - reg > - interrupts > - clocks > - power-domains > - - '#address-cells' > - - '#size-cells' > - > -if: > - not: > - properties: > - compatible: > - contains: > - const: renesas,sh-mobile-msiof > -then: > - required: > - - resets > + > +allOf: > + # additional "required"" > + - if: > + not: > + properties: > + compatible: > + contains: > + const: renesas,sh-mobile-msiof > + then: > + required: > + - resets > + > + # "MSIOF-SPI" specific > + - if: > + properties: > + $nodename: > + pattern: '^spi@' This condition does not match what you wrote in the cover letter: the controller is used in I2S mode when a port(s) subnode is present, and in SPI mode when no port(s) subnode is present. > + then: > + allOf: > + - $ref: spi-controller.yaml# Documentation/devicetree/bindings/spi/spi-controller.yaml indeed requires that the node-name matches "^spi(@.*|-([0-9]|[1-9][0-9]+))?$". The controller's node is located in the SoC-specific .dtsi, where its intended use case is not yet known, and its node name cannot easily be overridden in the board .dts that specifies the use case. Hence the node name must always be "spi" (and cannot be e.g. "serial-engine"). Let's hope there is no other use case for MSIOF that requires using a different node name... > > unevaluatedProperties: false Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert Thank you for your review > > + # "MSIOF-SPI" specific > > + - if: > > + properties: > > + $nodename: > > + pattern: '^spi@' > > This condition does not match what you wrote in the cover letter: > the controller is used in I2S mode when a port(s) subnode is present, > and in SPI mode when no port(s) subnode is present. > > > + then: > > + allOf: > > + - $ref: spi-controller.yaml# > > Documentation/devicetree/bindings/spi/spi-controller.yaml indeed > requires that the node-name matches "^spi(@.*|-([0-9]|[1-9][0-9]+))?$". > The controller's node is located in the SoC-specific .dtsi, where its > intended use case is not yet known, and its node name cannot easily be > overridden in the board .dts that specifies the use case. Hence the > node name must always be "spi" (and cannot be e.g. "serial-engine"). > Let's hope there is no other use case for MSIOF that requires using > a different node name... Hmm... OK So what we can do is keep spi@xxx node name, and check whether it has Of-Graph, and select spi-controller.yaml Thank you for your help !! Best regards --- Kuninori Morimoto
Hi Geert (as Renesas SoC Maintainer), Rob (as DT Maintainer), Mark (as SPI Maintainer) > > > + # "MSIOF-SPI" specific > > > + - if: > > > + properties: > > > + $nodename: > > > + pattern: '^spi@' > > > > This condition does not match what you wrote in the cover letter: > > the controller is used in I2S mode when a port(s) subnode is present, > > and in SPI mode when no port(s) subnode is present. > > > > > + then: > > > + allOf: > > > + - $ref: spi-controller.yaml# > > > > Documentation/devicetree/bindings/spi/spi-controller.yaml indeed > > requires that the node-name matches "^spi(@.*|-([0-9]|[1-9][0-9]+))?$". > > The controller's node is located in the SoC-specific .dtsi, where its > > intended use case is not yet known, and its node name cannot easily be > > overridden in the board .dts that specifies the use case. Hence the > > node name must always be "spi" (and cannot be e.g. "serial-engine"). > > Let's hope there is no other use case for MSIOF that requires using > > a different node name... Hmm... Now, MSIOF node has "spi@xxxx". SoC file indicates MSIOF-SPI as default, so it has below lines --- SoC file ---- msiof1: spi@xxxx { ... #address-cells = <1>; #size-cells = <0>; ... }; These are not needed for MSIOF-I2S, so removes these --- Board file ---- &msiof1 { ... /delete-property/#address-cells; /delete-property/#size-cells; ... }; Now, my dt-bindings doesn't load spi-controller.yaml (as sample), but I got [SoC file]: Warning (spi_bus_bridge): /soc/spi@xxxx: incorrect #address-cells for SPI bus also defined at [Board file] [SoC file]: Warning (spi_bus_bridge): /soc/spi@xxxx: incorrect #size-cells for SPI bus also defined at [Board file] MSIOF dt-bindings doesn't load spi-controller.yaml, but why I got "spi_bus_bridge" warning ?? I wonder dt compiler (?) automatically check "spi" node ? I have tryed some code, my expectation seems correct (In case of node name was "spi@xxx", I got many SPI related warnings even though I didn't load spi-controller). Best regards --- Kuninori Morimoto
Hi Morimoto-san, On Thu, 17 Apr 2025 at 01:52, Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote: > > > > + # "MSIOF-SPI" specific > > > > + - if: > > > > + properties: > > > > + $nodename: > > > > + pattern: '^spi@' > > > > > > This condition does not match what you wrote in the cover letter: > > > the controller is used in I2S mode when a port(s) subnode is present, > > > and in SPI mode when no port(s) subnode is present. > > > > > > > + then: > > > > + allOf: > > > > + - $ref: spi-controller.yaml# > > > > > > Documentation/devicetree/bindings/spi/spi-controller.yaml indeed > > > requires that the node-name matches "^spi(@.*|-([0-9]|[1-9][0-9]+))?$". > > > The controller's node is located in the SoC-specific .dtsi, where its > > > intended use case is not yet known, and its node name cannot easily be > > > overridden in the board .dts that specifies the use case. Hence the > > > node name must always be "spi" (and cannot be e.g. "serial-engine"). > > > Let's hope there is no other use case for MSIOF that requires using > > > a different node name... > > Hmm... > > Now, MSIOF node has "spi@xxxx". > SoC file indicates MSIOF-SPI as default, so it has below lines > > --- SoC file ---- > msiof1: spi@xxxx { > ... > #address-cells = <1>; > #size-cells = <0>; > ... > }; > > These are not needed for MSIOF-I2S, so removes these > > --- Board file ---- > &msiof1 { > ... > /delete-property/#address-cells; > /delete-property/#size-cells; > ... > }; > > Now, my dt-bindings doesn't load spi-controller.yaml (as sample), but I got > > [SoC file]: Warning (spi_bus_bridge): /soc/spi@xxxx: incorrect #address-cells for SPI bus > also defined at [Board file] > [SoC file]: Warning (spi_bus_bridge): /soc/spi@xxxx: incorrect #size-cells for SPI bus > also defined at [Board file] > > MSIOF dt-bindings doesn't load spi-controller.yaml, but why I got "spi_bus_bridge" > warning ?? I wonder dt compiler (?) automatically check "spi" node ? > I have tryed some code, my expectation seems correct (In case of node name was "spi@xxx", > I got many SPI related warnings even though I didn't load spi-controller). These come from dtc, which makes its own assumptions: $ git grep spi_bus_bridge scripts/dtc/checks.c:static void check_spi_bus_bridge(struct check *c, struct dt_info *dti, struct node *node) scripts/dtc/checks.c:WARNING(spi_bus_bridge, check_spi_bus_bridge, NULL, &addr_size_cells); scripts/dtc/checks.c:WARNING(spi_bus_reg, check_spi_bus_reg, NULL, ®_format, &spi_bus_bridge); scripts/dtc/checks.c: &spi_bus_bridge, Perhaps we do need to extend the use of role-specifying properties like "interrupt-controller" (in Device Tree Specification v0.4 and in dt-schema) and the few others in Documentation/devicetree/bindings: gpio-controller mctp-controller msi-controller system-power-controller Gr{oetje,eeting}s, Geert
Hi Geert, Rob, Mark > > [SoC file]: Warning (spi_bus_bridge): /soc/spi@xxxx: incorrect #address-cells for SPI bus > > also defined at [Board file] > > [SoC file]: Warning (spi_bus_bridge): /soc/spi@xxxx: incorrect #size-cells for SPI bus > > also defined at [Board file] > > > > MSIOF dt-bindings doesn't load spi-controller.yaml, but why I got "spi_bus_bridge" > > warning ?? I wonder dt compiler (?) automatically check "spi" node ? > > I have tryed some code, my expectation seems correct (In case of node name was "spi@xxx", > > I got many SPI related warnings even though I didn't load spi-controller). > > These come from dtc, which makes its own assumptions: > > $ git grep spi_bus_bridge > scripts/dtc/checks.c:static void check_spi_bus_bridge(struct check > *c, struct dt_info *dti, struct node *node) > scripts/dtc/checks.c:WARNING(spi_bus_bridge, check_spi_bus_bridge, > NULL, &addr_size_cells); > scripts/dtc/checks.c:WARNING(spi_bus_reg, check_spi_bus_reg, NULL, > ®_format, &spi_bus_bridge); > scripts/dtc/checks.c: &spi_bus_bridge, > > Perhaps we do need to extend the use of role-specifying properties > like "interrupt-controller" (in Device Tree Specification v0.4 and in > dt-schema) and the few others in Documentation/devicetree/bindings: > > gpio-controller > mctp-controller > msi-controller > system-power-controller Hmm... but I'm not familiar with DT. Should I do it ?? Except from SPI warning, and focus to MSIOF-I2S, my patch itself is not so bad, right ? I will post v4 patch-set, with comment above. Thank you for your help !! Best regards --- Kuninori Morimoto
diff --git a/Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml b/Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml index 49649fc3f95a..880bd854d196 100644 --- a/Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml +++ b/Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml @@ -4,14 +4,11 @@ $id: http://devicetree.org/schemas/spi/renesas,sh-msiof.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: Renesas MSIOF SPI controller +title: Renesas MSIOF SPI / I2S controller maintainers: - Geert Uytterhoeven <geert+renesas@glider.be> -allOf: - - $ref: spi-controller.yaml# - properties: compatible: oneOf: @@ -146,24 +143,38 @@ properties: $ref: /schemas/types.yaml#/definitions/uint32 default: 64 + # for MSIOF-I2S + port: + $ref: ../sound/audio-graph-port.yaml# + unevaluatedProperties: false + required: - compatible - reg - interrupts - clocks - power-domains - - '#address-cells' - - '#size-cells' - -if: - not: - properties: - compatible: - contains: - const: renesas,sh-mobile-msiof -then: - required: - - resets + +allOf: + # additional "required"" + - if: + not: + properties: + compatible: + contains: + const: renesas,sh-mobile-msiof + then: + required: + - resets + + # "MSIOF-SPI" specific + - if: + properties: + $nodename: + pattern: '^spi@' + then: + allOf: + - $ref: spi-controller.yaml# unevaluatedProperties: false
Renesas MSIOF (Clock-Synchronized Serial Interface with FIFO) can work as both SPI and I2S. MSIOF-I2S will use Audio Graph Card/Card2 driver which uses Of-Graph in DT. MSIOF-SPI/I2S are using same DT compatible properties. MSIOF-I2S uses Of-Graph for Audio-Graph-Card/Card2, MSIOF-SPI doesn't use Of-Graph. Adds schema for MSIOF-I2S (= Sound). Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- .../bindings/spi/renesas,sh-msiof.yaml | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-)