| Message ID | 20240416063600.309747-4-quic_mohs@quicinc.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ASoC: codecs: wcd937x: add wcd937x audio codec support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
On Tue, 16 Apr 2024 12:05:55 +0530, Mohammad Rafi Shaik wrote: > From: Prasad Kumpatla <quic_pkumpatl@quicinc.com> > > Qualcomm WCD9370/WCD9375 Codec is a standalone Hi-Fi audio codec IC > connected over SoundWire. This device has two SoundWire devices RX and > TX respectively. > This binding is for those slave devices on WCD9370/WCD9375. > > Co-developed-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> > Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> > Signed-off-by: Prasad Kumpatla <quic_pkumpatl@quicinc.com> > --- > .../bindings/sound/qcom,wcd937x-sdw.yaml | 71 +++++++++++++++++++ > 1 file changed, 71 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/qcom,wcd937x.example.dtb: codec@0,4: 'qcom,port-mapping' is a required property from schema $id: http://devicetree.org/schemas/sound/qcom,wcd937x-sdw.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/qcom,wcd937x.example.dtb: codec@0,3: 'qcom,port-mapping' is a required property from schema $id: http://devicetree.org/schemas/sound/qcom,wcd937x-sdw.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.example.dtb: codec@0,4: 'qcom,port-mapping' is a required property from schema $id: http://devicetree.org/schemas/sound/qcom,wcd937x-sdw.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.example.dtb: codec@0,3: 'qcom,port-mapping' is a required property from schema $id: http://devicetree.org/schemas/sound/qcom,wcd937x-sdw.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240416063600.309747-4-quic_mohs@quicinc.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On Tue, Apr 16, 2024 at 12:05:55PM +0530, Mohammad Rafi Shaik wrote: > From: Prasad Kumpatla <quic_pkumpatl@quicinc.com> > > Qualcomm WCD9370/WCD9375 Codec is a standalone Hi-Fi audio codec IC > connected over SoundWire. This device has two SoundWire devices RX and > TX respectively. > This binding is for those slave devices on WCD9370/WCD9375. > > Co-developed-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> > Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> > Signed-off-by: Prasad Kumpatla <quic_pkumpatl@quicinc.com> > --- > .../bindings/sound/qcom,wcd937x-sdw.yaml | 71 +++++++++++++++++++ > 1 file changed, 71 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml > > diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml > new file mode 100644 > index 000000000000..2b7358e266ba > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml > @@ -0,0 +1,71 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/qcom,wcd937x-sdw.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm SoundWire Slave devices on WCD9370 > + > +maintainers: > + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > + > +description: | Don't need '|' if no formatting. > + Qualcomm WCD9370 Codec is a standalone Hi-Fi audio codec IC. > + It has RX and TX Soundwire slave devices. This bindings is for the > + slave devices. > + > +properties: > + compatible: > + const: sdw20217010a00 > + > + reg: > + maxItems: 1 > + > + qcom,tx-port-mapping: > + description: | > + Specifies static port mapping between slave and master tx ports. > + In the order of slave port index. Are there constraints on the values of the entries? > + $ref: /schemas/types.yaml#/definitions/uint32-array > + minItems: 4 > + maxItems: 4 > + > + qcom,rx-port-mapping: > + description: | > + Specifies static port mapping between slave and master rx ports. > + In the order of slave port index. > + $ref: /schemas/types.yaml#/definitions/uint32-array > + minItems: 5 > + maxItems: 5 > + > +required: > + - compatible > + - reg > + - qcom,port-mapping > + > +additionalProperties: false > + > +examples: > + - | > + soundwire@3210000 { > + #address-cells = <2>; > + #size-cells = <0>; > + reg = <0x03210000 0x2000>; > + wcd937x_rx: codec@0,4 { > + compatible = "sdw20217010a00"; > + reg = <0 4>; > + qcom,rx-port-mapping = <1 2 3 4 5>; > + }; > + }; > + > + soundwire@3230000 { > + #address-cells = <2>; > + #size-cells = <0>; > + reg = <0x03230000 0x2000>; > + wcd937x_tx: codec@0,3 { > + compatible = "sdw20217010a00"; > + reg = <0 3>; > + qcom,tx-port-mapping = <2 3 4 5>; > + }; > + }; > + > +... > -- > 2.25.1 >
On 4/16/2024 8:02 PM, Rob Herring wrote: > On Tue, Apr 16, 2024 at 12:05:55PM +0530, Mohammad Rafi Shaik wrote: >> From: Prasad Kumpatla <quic_pkumpatl@quicinc.com> >> >> Qualcomm WCD9370/WCD9375 Codec is a standalone Hi-Fi audio codec IC >> connected over SoundWire. This device has two SoundWire devices RX and >> TX respectively. >> This binding is for those slave devices on WCD9370/WCD9375. >> >> Co-developed-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> >> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> >> Signed-off-by: Prasad Kumpatla <quic_pkumpatl@quicinc.com> >> --- >> .../bindings/sound/qcom,wcd937x-sdw.yaml | 71 +++++++++++++++++++ >> 1 file changed, 71 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >> >> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >> new file mode 100644 >> index 000000000000..2b7358e266ba >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >> @@ -0,0 +1,71 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/sound/qcom,wcd937x-sdw.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm SoundWire Slave devices on WCD9370 >> + >> +maintainers: >> + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> + >> +description: | > > Don't need '|' if no formatting. > >> + Qualcomm WCD9370 Codec is a standalone Hi-Fi audio codec IC. >> + It has RX and TX Soundwire slave devices. This bindings is for the >> + slave devices. >> + >> +properties: >> + compatible: >> + const: sdw20217010a00 >> + >> + reg: >> + maxItems: 1 >> + >> + qcom,tx-port-mapping: >> + description: | >> + Specifies static port mapping between slave and master tx ports. >> + In the order of slave port index. > > Are there constraints on the values of the entries? The port mapping entries are fixed values. There is no constraints. Thanks Rafi > >> + $ref: /schemas/types.yaml#/definitions/uint32-array >> + minItems: 4 >> + maxItems: 4 >> + >> + qcom,rx-port-mapping: >> + description: | >> + Specifies static port mapping between slave and master rx ports. >> + In the order of slave port index. >> + $ref: /schemas/types.yaml#/definitions/uint32-array >> + minItems: 5 >> + maxItems: 5 >> + >> +required: >> + - compatible >> + - reg >> + - qcom,port-mapping >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + soundwire@3210000 { >> + #address-cells = <2>; >> + #size-cells = <0>; >> + reg = <0x03210000 0x2000>; >> + wcd937x_rx: codec@0,4 { >> + compatible = "sdw20217010a00"; >> + reg = <0 4>; >> + qcom,rx-port-mapping = <1 2 3 4 5>; >> + }; >> + }; >> + >> + soundwire@3230000 { >> + #address-cells = <2>; >> + #size-cells = <0>; >> + reg = <0x03230000 0x2000>; >> + wcd937x_tx: codec@0,3 { >> + compatible = "sdw20217010a00"; >> + reg = <0 3>; >> + qcom,tx-port-mapping = <2 3 4 5>; >> + }; >> + }; >> + >> +... >> -- >> 2.25.1 >>
On 17/04/2024 10:17, Mohammad Rafi Shaik wrote: > On 4/16/2024 8:02 PM, Rob Herring wrote: >> On Tue, Apr 16, 2024 at 12:05:55PM +0530, Mohammad Rafi Shaik wrote: >>> From: Prasad Kumpatla <quic_pkumpatl@quicinc.com> >>> >>> Qualcomm WCD9370/WCD9375 Codec is a standalone Hi-Fi audio codec IC >>> connected over SoundWire. This device has two SoundWire devices RX and >>> TX respectively. >>> This binding is for those slave devices on WCD9370/WCD9375. >>> >>> Co-developed-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> >>> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> >>> Signed-off-by: Prasad Kumpatla <quic_pkumpatl@quicinc.com> >>> --- >>> .../bindings/sound/qcom,wcd937x-sdw.yaml | 71 +++++++++++++++++++ >>> 1 file changed, 71 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >>> new file mode 100644 >>> index 000000000000..2b7358e266ba >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >>> @@ -0,0 +1,71 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/sound/qcom,wcd937x-sdw.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Qualcomm SoundWire Slave devices on WCD9370 >>> + >>> +maintainers: >>> + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>> + >>> +description: | >> >> Don't need '|' if no formatting. >> >>> + Qualcomm WCD9370 Codec is a standalone Hi-Fi audio codec IC. >>> + It has RX and TX Soundwire slave devices. This bindings is for the >>> + slave devices. >>> + >>> +properties: >>> + compatible: >>> + const: sdw20217010a00 >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + qcom,tx-port-mapping: >>> + description: | >>> + Specifies static port mapping between slave and master tx ports. >>> + In the order of slave port index. >> >> Are there constraints on the values of the entries? > > The port mapping entries are fixed values. > There is no constraints. If they are fixed, then for sure you have constraints, because they are known. I really do not understand your response. Best regards, Krzysztof
On 16/04/2024 08:35, Mohammad Rafi Shaik wrote: > From: Prasad Kumpatla <quic_pkumpatl@quicinc.com> > > Qualcomm WCD9370/WCD9375 Codec is a standalone Hi-Fi audio codec IC > connected over SoundWire. This device has two SoundWire devices RX and > TX respectively. > This binding is for those slave devices on WCD9370/WCD9375. > > Co-developed-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> > Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> > Signed-off-by: Prasad Kumpatla <quic_pkumpatl@quicinc.com> > --- > .../bindings/sound/qcom,wcd937x-sdw.yaml | 71 +++++++++++++++++++ > 1 file changed, 71 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml > > diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml > new file mode 100644 > index 000000000000..2b7358e266ba > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml > @@ -0,0 +1,71 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/qcom,wcd937x-sdw.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm SoundWire Slave devices on WCD9370 > + > +maintainers: > + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > + > +description: | Do not need '|' unless you need to preserve formatting. > + Qualcomm WCD9370 Codec is a standalone Hi-Fi audio codec IC. > + It has RX and TX Soundwire slave devices. This bindings is for the > + slave devices. > + > +properties: > + compatible: > + const: sdw20217010a00 > + > + reg: > + maxItems: 1 > + > + qcom,tx-port-mapping: > + description: | > + Specifies static port mapping between slave and master tx ports. > + In the order of slave port index. Use inclusive terminology. Describe what is here - what is the index? What is the value? > + $ref: /schemas/types.yaml#/definitions/uint32-array > + minItems: 4 > + maxItems: 4 Add constraints on values. You have maximum 15 TX ports, don't you? > + > + qcom,rx-port-mapping: > + description: | > + Specifies static port mapping between slave and master rx ports. > + In the order of slave port index. > + $ref: /schemas/types.yaml#/definitions/uint32-array > + minItems: 5 > + maxItems: 5 > + > +required: > + - compatible > + - reg > + - qcom,port-mapping Test your binding. There is no need to engage reviewers for reviewing simple mistakes which *tools* can point. Respect reviewers time and use the tools first. You need oneOf: with required for TX and RX... or just unify the properties. Why do you need two? > + > +additionalProperties: false > + > +examples: > + - | > + soundwire@3210000 { Drop unit address. > + #address-cells = <2>; > + #size-cells = <0>; > + reg = <0x03210000 0x2000>; Drop, not relevant and not placed correctly (see DTS coding style). > + wcd937x_rx: codec@0,4 { Drop label, not used. > + compatible = "sdw20217010a00"; > + reg = <0 4>; > + qcom,rx-port-mapping = <1 2 3 4 5>; > + }; > + }; > + > + soundwire@3230000 { Drop this example, it's almost identical. Best regards, Krzysztof
On 4/17/2024 9:26 PM, Krzysztof Kozlowski wrote: > On 16/04/2024 08:35, Mohammad Rafi Shaik wrote: >> From: Prasad Kumpatla <quic_pkumpatl@quicinc.com> >> >> Qualcomm WCD9370/WCD9375 Codec is a standalone Hi-Fi audio codec IC >> connected over SoundWire. This device has two SoundWire devices RX and >> TX respectively. >> This binding is for those slave devices on WCD9370/WCD9375. >> >> Co-developed-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> >> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com> >> Signed-off-by: Prasad Kumpatla <quic_pkumpatl@quicinc.com> >> --- >> .../bindings/sound/qcom,wcd937x-sdw.yaml | 71 +++++++++++++++++++ >> 1 file changed, 71 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >> >> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >> new file mode 100644 >> index 000000000000..2b7358e266ba >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml >> @@ -0,0 +1,71 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/sound/qcom,wcd937x-sdw.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm SoundWire Slave devices on WCD9370 >> + >> +maintainers: >> + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> + >> +description: | > > Do not need '|' unless you need to preserve formatting. ACK will remove '|' in next version patch set. > >> + Qualcomm WCD9370 Codec is a standalone Hi-Fi audio codec IC. >> + It has RX and TX Soundwire slave devices. This bindings is for the >> + slave devices. >> + >> +properties: >> + compatible: >> + const: sdw20217010a00 >> + >> + reg: >> + maxItems: 1 >> + >> + qcom,tx-port-mapping: >> + description: | >> + Specifies static port mapping between slave and master tx ports. >> + In the order of slave port index. > > Use inclusive terminology. Describe what is here - what is the index? > What is the value? ACK > >> + $ref: /schemas/types.yaml#/definitions/uint32-array >> + minItems: 4 >> + maxItems: 4 > > Add constraints on values. You have maximum 15 TX ports, don't you? > >> + >> + qcom,rx-port-mapping: >> + description: | >> + Specifies static port mapping between slave and master rx ports. >> + In the order of slave port index. >> + $ref: /schemas/types.yaml#/definitions/uint32-array >> + minItems: 5 >> + maxItems: 5 >> + >> +required: >> + - compatible >> + - reg >> + - qcom,port-mapping > > Test your binding. There is no need to engage reviewers for reviewing > simple mistakes which *tools* can point. Respect reviewers time and use > the tools first. > > You need oneOf: with required for TX and RX... or just unify the > properties. Why do you need two? > ACK, will fix the binding errors. > >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + soundwire@3210000 { > Drop unit address. actually took the reference from wcd938x. https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/sound/qcom,wcd938x-sdw.yaml#n48 > >> + #address-cells = <2>; >> + #size-cells = <0>; >> + reg = <0x03210000 0x2000>; > > Drop, not relevant and not placed correctly (see DTS coding style). >> + wcd937x_rx: codec@0,4 { > > Drop label, not used. > >> + compatible = "sdw20217010a00"; >> + reg = <0 4>; >> + qcom,rx-port-mapping = <1 2 3 4 5>; >> + }; >> + }; >> + >> + soundwire@3230000 { > > Drop this example, it's almost identical. > > Best regards, > Krzysztof > Thanks & Regards, Rafi
diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml new file mode 100644 index 000000000000..2b7358e266ba --- /dev/null +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml @@ -0,0 +1,71 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/qcom,wcd937x-sdw.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm SoundWire Slave devices on WCD9370 + +maintainers: + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> + +description: | + Qualcomm WCD9370 Codec is a standalone Hi-Fi audio codec IC. + It has RX and TX Soundwire slave devices. This bindings is for the + slave devices. + +properties: + compatible: + const: sdw20217010a00 + + reg: + maxItems: 1 + + qcom,tx-port-mapping: + description: | + Specifies static port mapping between slave and master tx ports. + In the order of slave port index. + $ref: /schemas/types.yaml#/definitions/uint32-array + minItems: 4 + maxItems: 4 + + qcom,rx-port-mapping: + description: | + Specifies static port mapping between slave and master rx ports. + In the order of slave port index. + $ref: /schemas/types.yaml#/definitions/uint32-array + minItems: 5 + maxItems: 5 + +required: + - compatible + - reg + - qcom,port-mapping + +additionalProperties: false + +examples: + - | + soundwire@3210000 { + #address-cells = <2>; + #size-cells = <0>; + reg = <0x03210000 0x2000>; + wcd937x_rx: codec@0,4 { + compatible = "sdw20217010a00"; + reg = <0 4>; + qcom,rx-port-mapping = <1 2 3 4 5>; + }; + }; + + soundwire@3230000 { + #address-cells = <2>; + #size-cells = <0>; + reg = <0x03230000 0x2000>; + wcd937x_tx: codec@0,3 { + compatible = "sdw20217010a00"; + reg = <0 3>; + qcom,tx-port-mapping = <2 3 4 5>; + }; + }; + +...