Re: [PATCH 6/9] ASoC: dt-bindings: meson: convert axg fifo to schema

On Fri 03 Feb 2023 at 09:02, Krzysztof Kozlowski krzysztof.kozlowski@linaro.org wrote:

On 02/02/2023 19:36, Jerome Brunet wrote:

...

Convert the DT binding documentation for the Amlogic axg audio FIFOs to schema.

Signed-off-by: Jerome Brunet jbrunet@baylibre.com

.../bindings/sound/amlogic,axg-fifo.txt | 34 ----- .../bindings/sound/amlogic,axg-fifo.yaml | 116 ++++++++++++++++++ 2 files changed, 116 insertions(+), 34 deletions(-) delete mode 100644 Documentation/devicetree/bindings/sound/amlogic,axg-fifo.txt create mode 100644 Documentation/devicetree/bindings/sound/amlogic,axg-fifo.yaml

diff --git a/Documentation/devicetree/bindings/sound/amlogic,axg-fifo.txt b/Documentation/devicetree/bindings/sound/amlogic,axg-fifo.txt deleted file mode 100644 index fa4545ed81ca..000000000000 --- a/Documentation/devicetree/bindings/sound/amlogic,axg-fifo.txt +++ /dev/null @@ -1,34 +0,0 @@ -* Amlogic Audio FIFO controllers

-Required properties: -- compatible: 'amlogic,axg-toddr' or

•      'amlogic,axg-toddr' or
  

•      'amlogic,g12a-frddr' or
  

•      'amlogic,g12a-toddr' or
  

•      'amlogic,sm1-frddr' or
  

•      'amlogic,sm1-toddr'
  

-- reg: physical base address of the controller and length of memory

•   mapped region.
  

-- interrupts: interrupt specifier for the fifo. -- clocks: phandle to the fifo peripheral clock provided by the audio

•  clock controller.
  

-- resets: list of reset phandle, one for each entry reset-names. -- reset-names: should contain the following:

• • "arb" : memory ARB line (required)
• • "rst" : dedicated device reset line (optional)

-- #sound-dai-cells: must be 0. -- amlogic,fifo-depth: The size of the controller's fifo in bytes. This

•       is useful for determining certain configuration such
  

•       as the flush threshold of the fifo
  

-Example of FRDDR A on the A113 SoC:

-frddr_a: audio-controller@1c0 {

• compatible = "amlogic,axg-frddr";
• reg = <0x0 0x1c0 0x0 0x1c>;
• #sound-dai-cells = <0>;
• interrupts = <GIC_SPI 88 IRQ_TYPE_EDGE_RISING>;
• clocks = <&clkc_audio AUD_CLKID_FRDDR_A>;
• resets = <&arb AXG_ARB_FRDDR_A>;
• fifo-depth = <512>;

-}; diff --git a/Documentation/devicetree/bindings/sound/amlogic,axg-fifo.yaml b/Documentation/devicetree/bindings/sound/amlogic,axg-fifo.yaml new file mode 100644 index 000000000000..f6222ad08880 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/amlogic,axg-fifo.yaml @@ -0,0 +1,116 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/amlogic,axg-fifo.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#

+title: Amlogic AXG Audio FIFO controllers

+maintainers:

• • Jerome Brunet jbrunet@baylibre.com

+allOf:

• • $ref: dai-common.yaml#

+properties:

• $nodename:
• pattern: "^audio-controller@.*"
• compatible:
• oneOf:

•  - items:
  

•      - enum:
  

•          - amlogic,axg-toddr
  

•          - amlogic,axg-frddr
  

•  - items:
  

•      - enum:
  

•          - amlogic,g12a-toddr
  

•          - amlogic,sm1-toddr
  

•      - const:
  

•          amlogic,axg-toddr
  

•  - items:
  

•      - enum:
  

•          - amlogic,g12a-frddr
  

•          - amlogic,sm1-frddr
  

•      - const:
  

•          amlogic,axg-frddr
  

All usual comments apply.

...

• reg:
• maxItems: 1
• "#sound-dai-cells":
• const: 0
• clocks:
• items:

•  - description: Peripheral clock
  

• interrupts:
• maxItems: 1
• resets:
• items:

•  - description: Memory ARB line
  

•  - description: Dedicated device reset line
  

This won't work without minItems and you should see errors on your DTS or in dt_binding_check

The example provided here worked but there is indeed a warning with the axg-frddr variant.

I'm adding a 2nd example so it does not happen again.

...

• reset-names: true

minItems maxItems

Adding this causes troubles with the reset-names definitions in the 'if' clause. If I put min: 1, max: 2 and min: 2 in the 'then' clause I get:

Documentation/devicetree/bindings/sound/amlogic,axg-fifo.yaml: allOf:1:then:properties:reset-names: 'oneOf' conditional failed, one must be fixed: [{'const': 'arb'}, {'const': 'rst'}] is too long [{'const': 'arb'}, {'const': 'rst'}] is too short False schema does not allow 2 1 was expected hint: "minItems" is only needed if less than the "items" list length from schema $id: http://devicetree.org/meta-schemas/items.yaml#

The older devices just have the 'arb' reset. Newer devices have a 2nd reset line (called rst here)

If I just restrict the min and max, it would be valid for the older devices to have 'rst' only - but it is not valid.

With just 'true', it works as expected (throw errors if an incorrect name or number of names is passed). Min and Max comes from the items list.

Any suggestions ?

...

• amlogic,fifo-depth:
• $ref: /schemas/types.yaml#/definitions/uint32
• description: Size of the controller's fifo in bytes

+required:

Best regards, Krzysztof