Re: [PATCH v3 8/8] dt-bindings: sound: lpass-cpu: Move to yaml format

From: Rob Herring
Date: Mon Jul 13 2020 - 18:53:34 EST

Next message: Rob Herring: "Re: [PATCH v3 01/16] dt-bindings: clk: sunxi-ccu: add compatible string for A100 CCU and R-CCU"
Previous message: Tom Roeder: "Re: [PATCH v2] Makefile: Add clang-tidy and static analyzer support to makefile"
In reply to: Rohit kumar: "[PATCH v3 8/8] dt-bindings: sound: lpass-cpu: Move to yaml format"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Jul 08, 2020 at 10:38:16AM +0530, Rohit kumar wrote:
> From: Ajit Pandey <ajitp@xxxxxxxxxxxxxx>
>
> Update lpass-cpu binding with yaml formats.
>
> Signed-off-by: Ajit Pandey <ajitp@xxxxxxxxxxxxxx>
> Signed-off-by: Rohit kumar <rohitkr@xxxxxxxxxxxxxx>
> ---
> .../devicetree/bindings/sound/qcom,lpass-cpu.txt | 80 -----------
> .../devicetree/bindings/sound/qcom,lpass-cpu.yaml | 154 +++++++++++++++++++++
> 2 files changed, 154 insertions(+), 80 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
> create mode 100644 Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
>
> diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
> deleted file mode 100644
> index 04e34cc..00000000
> --- a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
> +++ /dev/null
> @@ -1,80 +0,0 @@
> -* Qualcomm Technologies LPASS CPU DAI
> -
> -This node models the Qualcomm Technologies Low-Power Audio SubSystem (LPASS).
> -
> -Required properties:
> -
> -- compatible : "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu" or
> - "qcom,lpass-cpu-sc7180"
> -- clocks : Must contain an entry for each entry in clock-names.
> -- clock-names : A list which must include the following entries:
> - * "ahbix-clk"
> - * "mi2s-osr-clk"
> - * "mi2s-bit-clk"
> - : required clocks for "qcom,lpass-cpu-apq8016"
> - * "ahbix-clk"
> - * "mi2s-bit-clk0"
> - * "mi2s-bit-clk1"
> - * "mi2s-bit-clk2"
> - * "mi2s-bit-clk3"
> - * "pcnoc-mport-clk"
> - * "pcnoc-sway-clk"
> -
> -- interrupts : Must contain an entry for each entry in
> - interrupt-names.
> -- interrupt-names : A list which must include the following entries:
> - * "lpass-irq-lpaif"
> -- pinctrl-N : One property must exist for each entry in
> - pinctrl-names. See ../pinctrl/pinctrl-bindings.txt
> - for details of the property values.
> -- pinctrl-names : Must contain a "default" entry.
> -- reg : Must contain an address for each entry in reg-names.
> -- reg-names : A list which must include the following entries:
> - * "lpass-lpaif"
> -- #address-cells : Must be 1
> -- #size-cells : Must be 0
> -
> -
> -
> -Optional properties:
> -
> -- qcom,adsp : Phandle for the audio DSP node
> -
> -By default, the driver uses up to 4 MI2S SD lines, for a total of 8 channels.
> -The SD lines to use can be configured by adding subnodes for each of the DAIs.
> -
> -Required properties for each DAI (represented by a subnode):
> -- reg : Must be one of the DAI IDs
> - (usually part of dt-bindings header)
> -- qcom,playback-sd-lines: List of serial data lines to use for playback
> - Each SD line should be represented by a number from 0-3.
> -- qcom,capture-sd-lines : List of serial data lines to use for capture
> - Each SD line should be represented by a number from 0-3.
> -
> -Note that adding a subnode changes the default to "no lines configured",
> -so both playback and capture lines should be configured when a subnode is added.
> -
> -Example:
> -
> -lpass@28100000 {
> - compatible = "qcom,lpass-cpu";
> - clocks = <&lcc AHBIX_CLK>, <&lcc MI2S_OSR_CLK>, <&lcc MI2S_BIT_CLK>;
> - clock-names = "ahbix-clk", "mi2s-osr-clk", "mi2s-bit-clk";
> - interrupts = <0 85 1>;
> - interrupt-names = "lpass-irq-lpaif";
> - pinctrl-names = "default", "idle";
> - pinctrl-0 = <&mi2s_default>;
> - pinctrl-1 = <&mi2s_idle>;
> - reg = <0x28100000 0x10000>;
> - reg-names = "lpass-lpaif";
> - qcom,adsp = <&adsp>;
> -
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - /* Optional to set different MI2S SD lines */
> - dai@3 {
> - reg = <MI2S_QUATERNARY>;
> - qcom,playback-sd-lines = <0 1>;
> - };
> -};
> diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
> new file mode 100644
> index 00000000..9c350bc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
> @@ -0,0 +1,154 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/qcom,lpass-cpu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm LPASS CPU dai driver bindings
> +
> +maintainers:
> + - Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> + - Rohit kumar <rohitkr@xxxxxxxxxxxxxx>
> +
> +description:
> + Qualcomm SOC Low-Power Audio SubSystem (LPASS) that consist of MI2S interface
> + for audio data transfer on external codecs. LPASS cpu driver is a module to
> + configure Low-Power Audio Interface(LPAIF) core registers across different
> + IP versions.
> +
> +properties:
> + compatible:
> + enum:
> + - qcom,lpass-cpu
> + - qcom,apq8016-lpass-cpu
> + - qcom,lpass-cpu-sc7180
> +
> + reg:
> + items:
> + - description: LPAIF core registers

Just: 'maxItems: 1' for a single entry.

> +
> + reg-names:
> + items:
> + - const: lpass-lpaif

Not all that useful with only 1.

> +
> + clocks:
> + items:
> + - description: AHBIX core clock
> + - description: oscillator clock for MI2S external interfaces
> + - description: Bit clock for single MI2S dai
> + - description: Bit clock for MI2S_PRIMARY dai interface
> + - description: Bit clock for MI2S_SECONDARY dai interface
> + - description: Bit clock for MI2S_TERTIARY dai interface
> + - description: Bit clock for MI2S_QUATERNARY dai interface
> + - description: NOC MPORT clock of LPASS core
> + - description: NOC SWAY clock of LPASS core
> +
> + clock-names:
> + items:
> + - const: ahbix-clk
> + - const: mi2s-osr-clk
> + - const: mi2s-bit-clk
> + - const: mi2s-bit-clk0
> + - const: mi2s-bit-clk1
> + - const: mi2s-bit-clk2
> + - const: mi2s-bit-clk3
> + - const: pcnoc-mport-clk
> + - const: pcnoc-sway-clk
> +
> + interrupts:
> + items:
> + - description: LPAIF DMA buffer interrupt

maxItems: 1

> +
> + interrupt-names:
> + items:
> + - const: lpass-irq-lpaif
> +
> + qcom,adsp:
> + maxItems: 1
> + description: Phandle for the audio DSP node

Needs a type $ref. And if just a phandle, 'maxItems: 1' is not
appropriate.

> +
> + iommus:
> + maxItems: 1
> + description: Phandle to apps_smmu node with sid mask
> +
> + power-domains:
> + maxItems: 1
> + description: Phandle for power domain node

Drop. That's every 'power-domains' property.

> +
> + '#sound-dai-cells':
> + const: 1
> +
> + child-node:

I'm sure I said this on some review recently, but you are defining a
child node named 'child-node'. You need this under patternProperties
with the actual child node name.

> + description: Required properties for each DAI
> + type: object
> + properties:
> + reg:
> + description: Must be one of the DAI ID
> + (Usually part of dtbindings header)

Ideally, you'd define the range of values here.

> + qcom,playback-sd-lines:
> + description: List of serial data lines to use for playback
> + Each SD line should be represented by a number from 0-3.

Needs a type $ref and 0-3 should be expressed as a schema.

'make dt_binding_check' should complain about this. You did run that,
right?

> + qcom,capture-sd-lines :
> + description: List of serial data lines to use for capture
> + Each SD line should be represented by a number from 0-3.

ditto

> + required:
> + -reg

space ^

> + # Note that adding a subnode changes the default to "no lines configured",
> + # so both playback and capture lines should be configured when a subnode
> + # is added.
> +
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - clocks
> + - clock-names
> + - interrupts
> + - interrupt-names
> + - sound-dai-cells

Not a valid property.

> +
> +optional:
> + - qcom,adsp
> +
> +if:
> + properties:
> + compatible:
> + contains:
> + const: qcom,lpass-cpu-sc7180
> +
> +then:
> + required:
> + - iommus
> + - power-domains
> +
> +examples:
> + lpass@28100000 {

This is not valid. 'examples' should be a list.

> + compatible = "qcom,lpass-cpu";
> + clocks = <&lcc AHBIX_CLK>,
> + <&lcc MI2S_OSR_CLK>,
> + <&lcc MI2S_BIT_CLK>;

The example will not build because the includes are missing.

> +
> + clock-names = "ahbix-clk",
> + "mi2s-osr-clk",
> + "mi2s-bit-clk";
> +
> + interrupts = <0 85 1>;
> + interrupt-names = "lpass-irq-lpaif";
> +
> + iommus = <&apps_smmu 0x1020 0>;
> + power-domains = <&lpass_hm LPASS_CORE_HM_GDSCR>;
> +
> + reg = <0x28100000 0x10000>;
> + reg-names = "lpass-lpaif";
> + #sound-dai-cells = <1>;
> + qcom,adsp = <&adsp>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /* Optional to set different MI2S SD lines */
> + mi2s-quaternary@3 {
> + reg = <MI2S_QUATERNARY>;
> + qcom,playback-sd-lines = <0 1>;
> + };
> --
> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
> is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
>

Next message: Rob Herring: "Re: [PATCH v3 01/16] dt-bindings: clk: sunxi-ccu: add compatible string for A100 CCU and R-CCU"
Previous message: Tom Roeder: "Re: [PATCH v2] Makefile: Add clang-tidy and static analyzer support to makefile"
In reply to: Rohit kumar: "[PATCH v3 8/8] dt-bindings: sound: lpass-cpu: Move to yaml format"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]