[6/9] ASoC: dt-bindings: meson: convert axg fifo to schema
Commit Message
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
Comments
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
> +
> + reset-names: true
minItems
maxItems
> +
> + amlogic,fifo-depth:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Size of the controller's fifo in bytes
> +
> +required:
Best regards,
Krzysztof
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
On 03/02/2023 14:27, Jerome Brunet wrote:
>
>>> + 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.
If the difference is only in one property, no need. If the difference is
in more properties - then could be. We do not keep examples for every
trivial change, because the assumption is that submitter tests DTS as well.
>
>>> +
>>> + 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#
Probably because rest of binding does not match. One way is like this:
https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57
>
> 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.
How? Why would you define for old devices "rst" as one name if this is
not correct?
>
> 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.
Because the rest is not in recommended way. Once you implement it in
recommended way, there will be no such...
>
> Any suggestions ?
Implement rest of comments.
Best regards,
Krzysztof
deleted file mode 100644
@@ -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>;
-};
new file mode 100644
@@ -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
+
+ 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
+
+ reset-names: true
+
+ amlogic,fifo-depth:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Size of the controller's fifo in bytes
+
+required:
+ - compatible
+ - reg
+ - "#sound-dai-cells"
+ - clocks
+ - interrupts
+ - resets
+ - amlogic,fifo-depth
+
+if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - amlogic,g12a-toddr
+ - amlogic,sm1-toddr
+ - amlogic,g12a-frddr
+ - amlogic,sm1-frddr
+then:
+ properties:
+ resets:
+ minItems: 2
+ reset-names:
+ items:
+ - const: arb
+ - const: rst
+ required:
+ - reset-names
+else:
+ properties:
+ resets:
+ maxItems: 1
+ reset-names:
+ const: arb
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/axg-audio-clkc.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/reset/amlogic,meson-axg-audio-arb.h>
+ #include <dt-bindings/reset/amlogic,meson-g12a-audio-reset.h>
+
+ frddr_a: audio-controller@1c0 {
+ compatible = "amlogic,g12a-frddr", "amlogic,axg-frddr";
+ reg = <0x1c0 0x1c>;
+ #sound-dai-cells = <0>;
+ clocks = <&clkc_audio AUD_CLKID_FRDDR_A>;
+ interrupts = <GIC_SPI 88 IRQ_TYPE_EDGE_RISING>;
+ resets = <&arb>, <&clkc_audio AUD_RESET_FRDDR_A>;
+ reset-names = "arb", "rst";
+ amlogic,fifo-depth = <512>;
+ };