lkml.org 
[lkml]   [2020]   [Jul]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 8/8] dt-bindings: sound: lpass-cpu: Move to yaml format
    On Wed, Jul 08, 2020 at 10:38:16AM +0530, Rohit kumar wrote:
    > From: Ajit Pandey <ajitp@codeaurora.org>
    >
    > Update lpass-cpu binding with yaml formats.
    >
    > Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
    > Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
    > ---
    > .../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@linaro.org>
    > + - Rohit kumar <rohitkr@codeaurora.org>
    > +
    > +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.
    >

    \
     
     \ /
      Last update: 2020-07-14 00:55    [W:4.449 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site