On 08/11/2022 07:42, Srinivasa Rao Mandadapu wrote: > > On 11/7/2022 8:41 PM, Krzysztof Kozlowski wrote: > Thanks for Your time Krzysztof!!! >> On 07/11/2022 15:20, Srinivasa Rao Mandadapu wrote: >>> Convert soundwire text bindings to DT Schema format. >>> >>> Update interrupt property items as per device tree, >>> as it is not appropriately described in text file. >>> >>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@xxxxxxxxxxx> >>> Co-developed-by: Ratna Deepthi Kudaravalli <quic_rkudarav@xxxxxxxxxxx> >>> Signed-off-by: Ratna Deepthi Kudaravalli <quic_rkudarav@xxxxxxxxxxx> >>> --- >>> This patch depends on: >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=42801e6185290d63691bd39cf8a3bba6cd5fe520 >> Hi, >> >> This is a bit surprising. You pointed to Linus' repo, so what does this >> dependency mean? The Linus' repo is the mainline, there is nothing else, >> so all its commits are already in every tree... unless this is an RC >> fix? If so, you should rather say which RC is needed. > > Yeah. Will remove it. Actually in initial version, when used different > repo, got bot errors. > > So to fix that, mentioned this dependency and continued the same in next > versions. > >> >>> Changes since V3: >>> -- Remove subnode description and add appropriate pattern properties. >>> -- Add interrput names in example. >>> -- update some properties description. >>> -- Revert minIteams change in previous version. >>> -- Rebase to latest code base. >>> Changes since V2: >>> -- Update commit message. >>> -- Add child node property. >>> -- Change file name. >>> -- Remove minIteams for few properties. >>> -- Remove redundant required properties. >>> -- Remove redundant description for wakeup-source property. >>> -- Update interrupt property items. >>> Changes since V1: >>> -- Remove the status field in example. >>> -- Remove interrupt-names property in the required list. >>> -- Add the wakeup-source property. >>> >>> .../devicetree/bindings/soundwire/qcom,sdw.txt | 215 ------------------ >>> .../bindings/soundwire/qcom,soundwire.yaml | 241 +++++++++++++++++++++ >>> 2 files changed, 241 insertions(+), 215 deletions(-) >>> delete mode 100644 Documentation/devicetree/bindings/soundwire/qcom,sdw.txt >>> create mode 100644 Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt >>> deleted file mode 100644 >>> index e0faed8..0000000 >>> --- a/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt >>> +++ /dev/null >>> @@ -1,215 +0,0 @@ >>> -Qualcomm SoundWire Controller Bindings >>> - >>> - >>> -This binding describes the Qualcomm SoundWire Controller along with its >>> -board specific bus parameters. >>> - >>> -- compatible: >>> - Usage: required >>> - Value type: <stringlist> >>> - Definition: must be "qcom,soundwire-v<MAJOR>.<MINOR>.<STEP>", >>> - Example: >>> - "qcom,soundwire-v1.3.0" >>> - "qcom,soundwire-v1.5.0" >>> - "qcom,soundwire-v1.5.1" >>> - "qcom,soundwire-v1.6.0" >>> - "qcom,soundwire-v1.7.0" >>> -- reg: >>> - Usage: required >>> - Value type: <prop-encoded-array> >>> - Definition: the base address and size of SoundWire controller >>> - address space. >>> - >>> -- interrupts: >>> - Usage: required >>> - Value type: <prop-encoded-array> >>> - Definition: should specify the SoundWire Controller core and optional >>> - wake IRQ >>> - >>> -- interrupt-names: >>> - Usage: Optional >>> - Value type: boolean >>> - Value type: <stringlist> >>> - Definition: should be "core" for core and "wakeup" for wake interrupt. >>> - >>> -- wakeup-source: >>> - Usage: Optional >>> - Value type: boolean >>> - Definition: should specify if SoundWire Controller is wake up capable. >>> - >>> -- clock-names: >>> - Usage: required >>> - Value type: <stringlist> >>> - Definition: should be "iface" for SoundWire Controller interface clock >>> - >>> -- clocks: >>> - Usage: required >>> - Value type: <prop-encoded-array> >>> - Definition: should specify the SoundWire Controller interface clock >>> - >>> -- #sound-dai-cells: >>> - Usage: required >>> - Value type: <u32> >>> - Definition: must be 1 for digital audio interfaces on the controller. >>> - >>> -- qcom,dout-ports: >>> - Usage: required >>> - Value type: <u32> >>> - Definition: must be count of data out ports >>> - >>> -- qcom,din-ports: >>> - Usage: required >>> - Value type: <u32> >>> - Definition: must be count of data in ports >>> - >>> -- qcom,ports-offset1: >>> - Usage: required >>> - Value type: <prop-encoded-array> >>> - Definition: should specify payload transport window offset1 of each >>> - data port. Out ports followed by In ports. >>> - Value of 0xFF indicates that this option is not implemented >>> - or applicable for the respective data port. >>> - More info in MIPI Alliance SoundWire 1.0 Specifications. >>> - >>> -- qcom,ports-offset2: >>> - Usage: required >>> - Value type: <prop-encoded-array> >>> - Definition: should specify payload transport window offset2 of each >>> - data port. Out ports followed by In ports. >>> - Value of 0xFF indicates that this option is not implemented >>> - or applicable for the respective data port. >>> - More info in MIPI Alliance SoundWire 1.0 Specifications. >>> - >>> -- qcom,ports-sinterval-low: >>> - Usage: required >>> - Value type: <prop-encoded-array> >>> - Definition: should be sample interval low of each data port. >>> - Out ports followed by In ports. Used for Sample Interval >>> - calculation. >>> - Value of 0xFF indicates that this option is not implemented >>> - or applicable for the respective data port. >>> - More info in MIPI Alliance SoundWire 1.0 Specifications. >>> - >>> -- qcom,ports-word-length: >>> - Usage: optional >>> - Value type: <prop-encoded-array> >>> - Definition: should be size of payload channel sample. >>> - Value of 0xFF indicates that this option is not implemented >>> - or applicable for the respective data port. >>> - More info in MIPI Alliance SoundWire 1.0 Specifications. >>> - >>> -- qcom,ports-block-pack-mode: >>> - Usage: optional >>> - Value type: <prop-encoded-array> >>> - Definition: should be 0 or 1 to indicate the block packing mode. >>> - 0 to indicate Blocks are per Channel >>> - 1 to indicate Blocks are per Port. >>> - Out ports followed by In ports. >>> - Value of 0xFF indicates that this option is not implemented >>> - or applicable for the respective data port. >>> - More info in MIPI Alliance SoundWire 1.0 Specifications. >>> - >>> -- qcom,ports-block-group-count: >>> - Usage: optional >>> - Value type: <prop-encoded-array> >>> - Definition: should be in range 1 to 4 to indicate how many sample >>> - intervals are combined into a payload. >>> - Out ports followed by In ports. >>> - Value of 0xFF indicates that this option is not implemented >>> - or applicable for the respective data port. >>> - More info in MIPI Alliance SoundWire 1.0 Specifications. >>> - >>> -- qcom,ports-lane-control: >>> - Usage: optional >>> - Value type: <prop-encoded-array> >>> - Definition: should be in range 0 to 7 to identify which data lane >>> - the data port uses. >>> - Out ports followed by In ports. >>> - Value of 0xFF indicates that this option is not implemented >>> - or applicable for the respective data port. >>> - More info in MIPI Alliance SoundWire 1.0 Specifications. >>> - >>> -- qcom,ports-hstart: >>> - Usage: optional >>> - Value type: <prop-encoded-array> >>> - Definition: should be number identifying lowerst numbered coloum in >>> - SoundWire Frame, i.e. left edge of the Transport sub-frame >>> - for each port. Values between 0 and 15 are valid. >>> - Out ports followed by In ports. >>> - Value of 0xFF indicates that this option is not implemented >>> - or applicable for the respective data port. >>> - More info in MIPI Alliance SoundWire 1.0 Specifications. >>> - >>> -- qcom,ports-hstop: >>> - Usage: optional >>> - Value type: <prop-encoded-array> >>> - Definition: should be number identifying highest numbered coloum in >>> - SoundWire Frame, i.e. the right edge of the Transport >>> - sub-frame for each port. Values between 0 and 15 are valid. >>> - Out ports followed by In ports. >>> - Value of 0xFF indicates that this option is not implemented >>> - or applicable for the respective data port. >>> - More info in MIPI Alliance SoundWire 1.0 Specifications. >>> - >>> -- qcom,dports-type: >>> - Usage: optional >>> - Value type: <prop-encoded-array> >>> - Definition: should be one of the following types >>> - 0 for reduced port >>> - 1 for simple ports >>> - 2 for full port >>> - Out ports followed by In ports. >>> - Value of 0xFF indicates that this option is not implemented >>> - or applicable for the respective data port. >>> - More info in MIPI Alliance SoundWire 1.0 Specifications. >>> - >>> -- reset: >>> - Usage: optional >>> - Value type: <prop-encoded-array> >>> - Definition: Should specify the SoundWire audio CSR reset controller interface, >>> - which is required for SoundWire version 1.6.0 and above. >>> - >>> -- reset-names: >>> - Usage: optional >>> - Value type: <stringlist> >>> - Definition: should be "swr_audio_cgcr" for SoundWire audio CSR reset >>> - controller interface. >>> - >>> -Note: >>> - More Information on detail of encoding of these fields can be >>> -found in MIPI Alliance SoundWire 1.0 Specifications. >>> - >>> -= SoundWire devices >>> -Each subnode of the bus represents SoundWire device attached to it. >>> -The properties of these nodes are defined by the individual bindings. >>> - >>> -= EXAMPLE >>> -The following example represents a SoundWire controller on DB845c board >>> -which has controller integrated inside WCD934x codec on SDM845 SoC. >>> - >>> -soundwire: soundwire@c85 { >>> - compatible = "qcom,soundwire-v1.3.0"; >>> - reg = <0xc85 0x20>; >>> - interrupts = <20 IRQ_TYPE_EDGE_RISING>; >>> - clocks = <&wcc>; >>> - clock-names = "iface"; >>> - resets = <&lpass_audiocc LPASS_AUDIO_SWR_TX_CGCR>; >>> - reset-names = "swr_audio_cgcr"; >>> - #sound-dai-cells = <1>; >>> - qcom,dports-type = <0>; >>> - qcom,dout-ports = <6>; >>> - qcom,din-ports = <2>; >>> - qcom,ports-sinterval-low = /bits/ 8 <0x07 0x1F 0x3F 0x7 0x1F 0x3F 0x0F 0x0F>; >>> - qcom,ports-offset1 = /bits/ 8 <0x01 0x02 0x0C 0x6 0x12 0x0D 0x07 0x0A >; >>> - qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x1F 0x00 0x00 0x1F 0x00 0x00>; >>> - >>> - /* Left Speaker */ >>> - left{ >>> - .... >>> - }; >>> - >>> - /* Right Speaker */ >>> - right{ >>> - .... >>> - }; >>> -}; >>> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml >>> new file mode 100644 >>> index 0000000..b0590cb >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/soundwire/qcom,soundwire.yaml >>> @@ -0,0 +1,241 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/soundwire/qcom,soundwire.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Qualcomm SoundWire Controller >>> + >>> +maintainers: >>> + - Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> >>> + - Srinivasa Rao Mandadapu <quic_srivasam@xxxxxxxxxxx> >>> + >>> +description: >>> + The Qualcomm SoundWire controller along with its board specific bus parameters. >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - qcom,soundwire-v1.3.0 >>> + - qcom,soundwire-v1.5.0 >>> + - qcom,soundwire-v1.5.1 >>> + - qcom,soundwire-v1.6.0 >>> + - qcom,soundwire-v1.7.0 >> Missing blank line. > Okay. will add it. >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: >>> + minItems: 1 >>> + items: >>> + - description: specify the SoundWire controller core. >>> + - description: specify the Soundwire controller wake IRQ. >>> + >>> + interrupt-names: >>> + minItems: 1 >>> + items: >>> + - const: core >>> + - const: wakeup >>> + >>> + clocks: >>> + items: >>> + - description: iface clock >>> + >>> + clock-names: >>> + items: >>> + - const: iface >>> + >>> + resets: >>> + items: >>> + - description: SWR_AUDIO_CGCR RESET >>> + >>> + reset-names: >>> + items: >>> + - const: swr_audio_cgcr >>> + >>> + '#sound-dai-cells': >>> + const: 1 >>> + >>> + '#address-cells': >>> + const: 2 >>> + >>> + '#size-cells': >>> + const: 0 >>> + >>> + wakeup-source: true >>> + >>> + qcom,din-ports: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: count of data in ports >>> + >>> + qcom,dout-ports: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: count of data out ports >>> + >>> + qcom,ports-word-length: >>> + $ref: /schemas/types.yaml#/definitions/uint8-array >>> + description: Size of payload channel sample. >>> + Value of 0xFF indicates that this option is not implemented >>> + or applicable for the respective data port. >>> + More info in MIPI Alliance SoundWire 1.0 Specifications. >>> + minItems: 3 >>> + maxItems: 5 >>> + >>> + qcom,ports-sinterval-low: >>> + $ref: /schemas/types.yaml#/definitions/uint8-array >>> + description: Sample interval low of each data port. >>> + Out ports followed by In ports. Used for Sample Interval >>> + calculation. >>> + Value of 0xFF indicates that this option is not implemented >>> + or applicable for the respective data port. >>> + More info in MIPI Alliance SoundWire 1.0 Specifications. >>> + minItems: 3 >>> + maxItems: 8 >>> + >>> + qcom,ports-offset1: >>> + $ref: /schemas/types.yaml#/definitions/uint8-array >>> + description: Payload transport window offset1 of each >>> + data port. Out ports followed by In ports. >>> + Value of 0xFF indicates that this option is not implemented >>> + or applicable for the respective data port. >>> + More info in MIPI Alliance SoundWire 1.0 Specifications. >>> + minItems: 3 >>> + maxItems: 8 >>> + >>> + qcom,ports-offset2: >>> + $ref: /schemas/types.yaml#/definitions/uint8-array >>> + description: Payload transport window offset2 of each >>> + data port. Out ports followed by In ports. >>> + Value of 0xFF indicates that this option is not implemented >>> + or applicable for the respective data port. >>> + More info in MIPI Alliance SoundWire 1.0 Specifications. >>> + minItems: 3 >>> + maxItems: 8 >>> + >>> + qcom,ports-lane-control: >>> + $ref: /schemas/types.yaml#/definitions/uint8-array >>> + description: Identify which data lane the data port uses. >>> + Out ports followed by In ports. >>> + Value of 0xFF indicates that this option is not implemented >>> + or applicable for the respective data port. >>> + More info in MIPI Alliance SoundWire 1.0 Specifications. >>> + minItems: 3 >>> + maxItems: 5 >>> + >>> + qcom,ports-block-pack-mode: >>> + $ref: /schemas/types.yaml#/definitions/uint8-array >>> + description: Indicate the block packing mode. >>> + 0 to indicate Blocks are per Channel >>> + 1 to indicate Blocks are per Port. >>> + Out ports followed by In ports. >>> + Value of 0xFF indicates that this option is not implemented >>> + or applicable for the respective data port. >>> + More info in MIPI Alliance SoundWire 1.0 Specifications. >>> + minItems: 3 >>> + maxItems: 8 >>> + >>> + qcom,ports-hstart: >>> + $ref: /schemas/types.yaml#/definitions/uint8-array >>> + description: Identifying lowerst numbered coloum in >>> + SoundWire Frame, i.e. left edge of the Transport sub-frame >>> + for each port. Values between 0 and 15 are valid. >>> + Out ports followed by In ports. >>> + Value of 0xFF indicates that this option is not implemented >>> + or applicable for the respective data port. >>> + More info in MIPI Alliance SoundWire 1.0 Specifications. >>> + minItems: 3 >>> + maxItems: 5 >>> + >>> + qcom,ports-hstop: >>> + $ref: /schemas/types.yaml#/definitions/uint8-array >>> + description: Identifying highest numbered coloum in >>> + SoundWire Frame, i.e. the right edge of the Transport >>> + sub-frame for each port. Values between 0 and 15 are valid. >>> + Out ports followed by In ports. >>> + Value of 0xFF indicates that this option is not implemented >>> + or applicable for the respective data port. >>> + More info in MIPI Alliance SoundWire 1.0 Specifications. >>> + minItems: 3 >>> + maxItems: 5 >>> + >>> + qcom,ports-block-group-count: >>> + $ref: /schemas/types.yaml#/definitions/uint8-array >>> + description: In range 1 to 4 to indicate how many sample >>> + intervals are combined into a payload. >>> + Out ports followed by In ports. >>> + Value of 0xFF indicates that this option is not implemented >>> + or applicable for the respective data port. >>> + More info in MIPI Alliance SoundWire 1.0 Specifications. >>> + minItems: 3 >>> + maxItems: 5 >>> + >>> + label: >>> + maxItems: 1 >>> + >>> +patternProperties: >>> + "^.*@[0-9a-f],[0-9a-f]$": >>> + type: object >> You should have here a description. > > Bit confusion Here. In any of examples there is no description for > patternProperties. That's interesting, let's check: $ git grep -C 4 patternProperties -- Documentation/devicetree/bindings/ First has description, second does not have, third has, 4th has, 5th has, 6th has, 7th has, 8th has. So from all first 8 entries, only one did not have. When the objects are obvious or are a standard properties, the description could be skipped, but in regular cases description is expected. Just like for every custom property. > You mean, description for below compatible property? No, description on the level of type: object. > >> >>> + properties: >>> + compatible: >>> + pattern: "^sdw[0-9a-f]{1}[0-9a-f]{4}[0-9a-f]{4}[0-9a-f]{2}$" >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - interrupts >>> + - clocks >>> + - clock-names >>> + - '#sound-dai-cells' >>> + - '#address-cells' >>> + - '#size-cells' >>> + - qcom,dout-ports >>> + - qcom,din-ports >>> + - qcom,ports-sinterval-low >>> + - qcom,ports-offset1 >>> + - qcom,ports-offset2 >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + #include <dt-bindings/interrupt-controller/arm-gic.h> >>> + #include <dt-bindings/interrupt-controller/irq.h> >>> + #include <dt-bindings/clock/qcom,lpassaudiocc-sc7280.h> >>> + >>> + soundwire@3210000 { >>> + compatible = "qcom,soundwire-v1.6.0"; >>> + reg = <0x03210000 0x2000>; >>> + >>> + interrupts = <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>, >>> + <&pdc 130 IRQ_TYPE_LEVEL_HIGH>; >>> + >>> + interrupt-names = "core"; >> Does not look like you tested the bindings. Please run `make >> dt_binding_check` (see >> Documentation/devicetree/bindings/writing-schema.rst for instructions). > Tested bindings and didn't see any issues on my setup. will do cross check. You have two interrupts but only one name. Best regards, Krzysztof
