| Message ID | 20220718140344.1831731-3-stephan.gerhold@kernkonzept.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | remoteproc: qcom_q6v5_mss: Add MSM8909 | expand |
On Mon, 18 Jul 2022 16:03:41 +0200, Stephan Gerhold wrote: > qcom,q6v5.txt covers multiple SoCs with quite different binding > requirements. Converting this into one DT schema would require > several if statements, making the DT schema overall harder to > read and understand. > > To avoid this, follow the example of SC7180/SC7280 and split > "qcom,msm8916-mss-pil" (and the equivalent deprecated "qcom,q6v5-pil" > compatible) into a separate DT schema. The schema is somewhat based > on the one for SC7180/SC7280 but adjusted for the old platforms. > > Compared to the old plain text bindings, add missing documentation for > the "bam-dmux" subnode and recommend one particular approach to specify > the MBA/MPSS "memory-region" (the other one is marked as deprecated). > > Cc: Sireesh Kodali <sireeshkodali1@gmail.com> > Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com> > --- > Changes in v2: > - Add blank lines between top-level properties > - Drop "deprecated" in "oneOf" list, it is not clear if this is valid > and it should be redundant since the properties itself are already > marked as "deprecated" > --- > Like Sibi's patch series for SC7180/SC7820 [1] this is somewhat related > to Sireesh's series that converts all of qcom,q6v5.txt [2] (with a lot > of if statements). However, this series focuses on MSM8916/MSM8974 (or > actually MSM8909) only. > > [1]: https://lore.kernel.org/linux-arm-msm/1657020721-24939-1-git-send-email-quic_sibis@quicinc.com/ > [2]: https://lore.kernel.org/linux-arm-msm/20220511161602.117772-7-sireeshkodali1@gmail.com/ > --- > .../remoteproc/qcom,msm8916-mss-pil.yaml | 247 ++++++++++++++++++ > .../bindings/remoteproc/qcom,q6v5.txt | 19 -- > 2 files changed, 247 insertions(+), 19 deletions(-) > create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: ./Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/remoteproc/qcom,smd-edge.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.example.dtb: remoteproc@4080000: smd-edge: False schema does not allow {'interrupts': [[0, 25, 1]], 'qcom,smd-edge': [[0]], 'qcom,ipc': [[4294967295, 8, 12]], 'qcom,remote-pid': [[1]], 'label': ['hexagon']} From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. 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.
On Mon, Jul 18, 2022 at 04:03:41PM +0200, Stephan Gerhold wrote: > qcom,q6v5.txt covers multiple SoCs with quite different binding > requirements. Converting this into one DT schema would require > several if statements, making the DT schema overall harder to > read and understand. > > To avoid this, follow the example of SC7180/SC7280 and split > "qcom,msm8916-mss-pil" (and the equivalent deprecated "qcom,q6v5-pil" > compatible) into a separate DT schema. The schema is somewhat based > on the one for SC7180/SC7280 but adjusted for the old platforms. > > Compared to the old plain text bindings, add missing documentation for > the "bam-dmux" subnode and recommend one particular approach to specify > the MBA/MPSS "memory-region" (the other one is marked as deprecated). > > Cc: Sireesh Kodali <sireeshkodali1@gmail.com> > Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com> > --- > Changes in v2: > - Add blank lines between top-level properties > - Drop "deprecated" in "oneOf" list, it is not clear if this is valid > and it should be redundant since the properties itself are already > marked as "deprecated" > --- > Like Sibi's patch series for SC7180/SC7820 [1] this is somewhat related > to Sireesh's series that converts all of qcom,q6v5.txt [2] (with a lot > of if statements). However, this series focuses on MSM8916/MSM8974 (or > actually MSM8909) only. > > [1]: https://lore.kernel.org/linux-arm-msm/1657020721-24939-1-git-send-email-quic_sibis@quicinc.com/ > [2]: https://lore.kernel.org/linux-arm-msm/20220511161602.117772-7-sireeshkodali1@gmail.com/ Is that one abandoned or do we just get to review both approaches without coordination? I think you need a common q6v5 schema here with all the common properties. Having the same property name with the type defined multiple times is not great. In fact, I'm working on a check for finding those. Rob
On Wed, Jul 20, 2022 at 04:46:08PM -0600, Rob Herring wrote: > On Mon, Jul 18, 2022 at 04:03:41PM +0200, Stephan Gerhold wrote: > > qcom,q6v5.txt covers multiple SoCs with quite different binding > > requirements. Converting this into one DT schema would require > > several if statements, making the DT schema overall harder to > > read and understand. > > > > To avoid this, follow the example of SC7180/SC7280 and split > > "qcom,msm8916-mss-pil" (and the equivalent deprecated "qcom,q6v5-pil" > > compatible) into a separate DT schema. The schema is somewhat based > > on the one for SC7180/SC7280 but adjusted for the old platforms. > > > > Compared to the old plain text bindings, add missing documentation for > > the "bam-dmux" subnode and recommend one particular approach to specify > > the MBA/MPSS "memory-region" (the other one is marked as deprecated). > > > > Cc: Sireesh Kodali <sireeshkodali1@gmail.com> > > Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com> > > --- > > Changes in v2: > > - Add blank lines between top-level properties > > - Drop "deprecated" in "oneOf" list, it is not clear if this is valid > > and it should be redundant since the properties itself are already > > marked as "deprecated" > > --- > > Like Sibi's patch series for SC7180/SC7820 [1] this is somewhat related > > to Sireesh's series that converts all of qcom,q6v5.txt [2] (with a lot > > of if statements). However, this series focuses on MSM8916/MSM8974 (or > > actually MSM8909) only. > > > > [1]: https://lore.kernel.org/linux-arm-msm/1657020721-24939-1-git-send-email-quic_sibis@quicinc.com/ > > [2]: https://lore.kernel.org/linux-arm-msm/20220511161602.117772-7-sireeshkodali1@gmail.com/ > > Is that one abandoned or do we just get to review both approaches > without coordination? > I assumed the decision to make separate schemas rather than a big one was already made, since Sibi's series was applied and has already moved parts of qcom,q6v5.txt into separate schemas. Still, I did coordinate with Sireesh before submitting this patch and suggested that he can likely just add the new "qcom,msm8953-mss-pil" compatible from his series to the DT schema in this patch (the setup is also very similar). > I think you need a common q6v5 schema here with all the common > properties. Having the same property name with the type defined multiple > times is not great. In fact, I'm working on a check for finding those. Which properties would you move to a common schema? Most of the schema is just listing items for generic properties (interrupts, clocks, power domains, supplies, resets, memory-region, ...) and having them separated is intended to avoid lots of if statements in a common schema. What remains is maybe: - "qcom,smem-states", which is already used in several other schemas and could be possibly defined together with #qcom,smem-state-cells in some generic schema(?) - "qcom,halt-regs", "firmware-name", "smd-edge" are used by different Qualcomm remoteproc drivers, so they could possibly be defined in some common "qcom-remoteproc.yaml" schema(?) Thanks, Stephan
On 21/07/2022 22:02, Stephan Gerhold wrote: > What remains is maybe: > > - "qcom,smem-states", which is already used in several other schemas > and could be possibly defined together with #qcom,smem-state-cells > in some generic schema(?) > > - "qcom,halt-regs", "firmware-name", "smd-edge" are used by different > Qualcomm remoteproc drivers, so they could possibly be defined in > some common "qcom-remoteproc.yaml" schema(?) smd-edge and glink is already in remoteproc/qcom,smd-edge.yaml qcom,glink-edge.yaml Best regards, Krzysztof
On Sat, Jul 23, 2022 at 10:41:53PM +0200, Krzysztof Kozlowski wrote: > On 21/07/2022 22:02, Stephan Gerhold wrote: > > What remains is maybe: > > > > - "qcom,smem-states", which is already used in several other schemas > > and could be possibly defined together with #qcom,smem-state-cells > > in some generic schema(?) > > > > - "qcom,halt-regs", "firmware-name", "smd-edge" are used by different > > Qualcomm remoteproc drivers, so they could possibly be defined in > > some common "qcom-remoteproc.yaml" schema(?) > > smd-edge and glink is already in remoteproc/qcom,smd-edge.yaml > qcom,glink-edge.yaml > Yep, I'm actually using it already in this patch. I think what I meant is that the smd-edge property $ref could be already defined in a common "qcom-remoteproc.yaml" schema and inherited via allOf, since all the qcom remoteprocs should have either a smd-edge or glink-edge. But all in all I'm still unsure which "common properties" Rob is referring to here, in which file to place them, and if this is worth it at all for the few I have listed above... :-) Thanks, Stephan
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml new file mode 100644 index 000000000000..e4ac4889fc92 --- /dev/null +++ b/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml @@ -0,0 +1,247 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/remoteproc/qcom,msm8916-mss-pil.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm MSM8916 MSS Peripheral Image Loader (and similar) + +maintainers: + - Stephan Gerhold <stephan@gerhold.net> + +description: + This document describes the hardware for a component that loads and boots + firmware on the Qualcomm MSM8916 Modem Hexagon Core (and similar). + +properties: + compatible: + oneOf: + - enum: + - qcom,msm8916-mss-pil + + - const: qcom,q6v5-pil + description: Deprecated, prefer using qcom,msm8916-mss-pil + deprecated: true + + reg: + items: + - description: MSS QDSP6 registers + - description: RMB registers + + reg-names: + items: + - const: qdsp6 + - const: rmb + + interrupts: + items: + - description: Watchdog interrupt + - description: Fatal interrupt + - description: Ready interrupt + - description: Handover interrupt + - description: Stop acknowledge interrupt + + interrupt-names: + items: + - const: wdog + - const: fatal + - const: ready + - const: handover + - const: stop-ack + + clocks: + items: + - description: Configuration interface (AXI) clock + - description: Configuration bus (AHB) clock + - description: Boot ROM (AHB) clock + - description: XO proxy clock (control handed over after startup) + + clock-names: + items: + - const: iface + - const: bus + - const: mem + - const: xo + + power-domains: + items: + - description: CX proxy power domain (control handed over after startup) + - description: MX proxy power domain (control handed over after startup) + + power-domain-names: + items: + - const: cx + - const: mx + + pll-supply: + description: PLL proxy supply (control handed over after startup) + + resets: + items: + - description: MSS restart control + + reset-names: + items: + - const: mss_restart + + qcom,smem-states: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: States used by the AP to signal the Hexagon core + items: + - description: Stop modem + + qcom,smem-state-names: + description: Names of the states used by the AP to signal the Hexagon core + items: + - const: stop + + qcom,halt-regs: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: + Halt registers are used to halt transactions of various sub-components + within MSS. + items: + - items: + - description: phandle to TCSR syscon region + - description: offset to the Q6 halt register + - description: offset to the modem halt register + - description: offset to the nc halt register + + memory-region: + items: + - description: MBA reserved region + - description: MPSS reserved region + + firmware-name: + $ref: /schemas/types.yaml#/definitions/string-array + items: + - description: Name of MBA firmware + - description: Name of modem firmware + + bam-dmux: + $ref: /schemas/net/qcom,bam-dmux.yaml# + description: + Qualcomm BAM Data Multiplexer (provides network interface to the modem) + + smd-edge: + $ref: qcom,smd-edge.yaml# + description: + Qualcomm SMD subnode which represents communication edge, channels + and devices related to the DSP. + + properties: + label: + enum: + - modem + - hexagon + + # Deprecated properties + cx-supply: + description: CX power domain regulator supply (prefer using power-domains) + deprecated: true + + mx-supply: + description: MX power domain regulator supply (prefer using power-domains) + deprecated: true + + mba: + type: object + description: + MBA reserved region (prefer using memory-region with two items) + properties: + memory-region: true + required: + - memory-region + deprecated: true + + mpss: + type: object + description: + MPSS reserved region (prefer using memory-region with two items) + properties: + memory-region: true + required: + - memory-region + deprecated: true + +required: + - compatible + - reg + - reg-names + - interrupts + - interrupt-names + - clocks + - clock-names + - pll-supply + - resets + - reset-names + - qcom,halt-regs + - qcom,smem-states + - qcom,smem-state-names + - smd-edge + +allOf: + # Fallbacks for deprecated properties + - oneOf: + - required: + - memory-region + - required: + - mba + - mpss + - oneOf: + - required: + - power-domains + - power-domain-names + - required: + - cx-supply + - mx-supply + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/qcom,gcc-msm8916.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/power/qcom-rpmpd.h> + + remoteproc_mpss: remoteproc@4080000 { + compatible = "qcom,msm8916-mss-pil"; + reg = <0x04080000 0x100>, <0x04020000 0x40>; + reg-names = "qdsp6", "rmb"; + + interrupts-extended = <&intc GIC_SPI 24 IRQ_TYPE_EDGE_RISING>, + <&hexagon_smp2p_in 0 IRQ_TYPE_EDGE_RISING>, + <&hexagon_smp2p_in 1 IRQ_TYPE_EDGE_RISING>, + <&hexagon_smp2p_in 2 IRQ_TYPE_EDGE_RISING>, + <&hexagon_smp2p_in 3 IRQ_TYPE_EDGE_RISING>; + interrupt-names = "wdog", "fatal", "ready", "handover", "stop-ack"; + + qcom,smem-states = <&hexagon_smp2p_out 0>; + qcom,smem-state-names = "stop"; + qcom,halt-regs = <&tcsr 0x18000 0x19000 0x1a000>; + + clocks = <&gcc GCC_MSS_CFG_AHB_CLK>, + <&gcc GCC_MSS_Q6_BIMC_AXI_CLK>, + <&gcc GCC_BOOT_ROM_AHB_CLK>, + <&xo_board>; + clock-names = "iface", "bus", "mem", "xo"; + + power-domains = <&rpmpd MSM8916_VDDCX>, <&rpmpd MSM8916_VDDMX>; + power-domain-names = "cx", "mx"; + pll-supply = <&pm8916_l7>; + + resets = <&scm 0>; + reset-names = "mss_restart"; + + memory-region = <&mba_mem>, <&mpss_mem>; + + smd-edge { + interrupts = <GIC_SPI 25 IRQ_TYPE_EDGE_RISING>; + + qcom,smd-edge = <0>; + qcom,ipc = <&apcs 8 12>; + qcom,remote-pid = <1>; + + label = "hexagon"; + }; + }; diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt index d0a75443e27e..3beb01da134b 100644 --- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt @@ -7,10 +7,8 @@ on the Qualcomm Hexagon core. Usage: required Value type: <string> Definition: must be one of: - "qcom,q6v5-pil", "qcom,ipq8074-wcss-pil" "qcom,qcs404-wcss-pil" - "qcom,msm8916-mss-pil", "qcom,msm8974-mss-pil" "qcom,msm8996-mss-pil" "qcom,msm8998-mss-pil" @@ -37,10 +35,8 @@ on the Qualcomm Hexagon core. Value type: <stringlist> Definition: The interrupts needed depends on the the compatible string: - qcom,q6v5-pil: qcom,ipq8074-wcss-pil: qcom,qcs404-wcss-pil: - qcom,msm8916-mss-pil: qcom,msm8974-mss-pil: must be "wdog", "fatal", "ready", "handover", "stop-ack" qcom,msm8996-mss-pil: @@ -72,8 +68,6 @@ on the Qualcomm Hexagon core. "gcc_axim_cbcr", "lcc_ahbfabric_cbc", "tcsr_lcc_cbc", "lcc_abhs_cbc", "lcc_tcm_slave_cbc", "lcc_abhm_cbc", "lcc_axim_cbc", "lcc_bcr_sleep" - qcom,q6v5-pil: - qcom,msm8916-mss-pil: qcom,msm8974-mss-pil: must be "iface", "bus", "mem", "xo" qcom,msm8996-mss-pil: @@ -112,17 +106,6 @@ should be referenced as follows: Definition: reference to the reserved-memory for the mba region followed by the mpss region -For the compatible strings below the following supplies are required: - "qcom,q6v5-pil" - "qcom,msm8916-mss-pil", -- cx-supply: (deprecated, use power domain instead) -- mx-supply: (deprecated, use power domain instead) -- pll-supply: - Usage: required - Value type: <phandle> - Definition: reference to the regulators to be held on behalf of the - booting of the Hexagon core - For the compatible string below the following supplies are required: "qcom,msm8974-mss-pil" - cx-supply: (deprecated, use power domain instead) @@ -161,8 +144,6 @@ For the compatible string below the following supplies are required: Definition: The power-domains needed depend on the compatible string: qcom,ipq8074-wcss-pil: no power-domain names required - qcom,q6v5-pil: - qcom,msm8916-mss-pil: qcom,msm8974-mss-pil: qcom,msm8996-mss-pil: qcom,msm8998-mss-pil:
qcom,q6v5.txt covers multiple SoCs with quite different binding requirements. Converting this into one DT schema would require several if statements, making the DT schema overall harder to read and understand. To avoid this, follow the example of SC7180/SC7280 and split "qcom,msm8916-mss-pil" (and the equivalent deprecated "qcom,q6v5-pil" compatible) into a separate DT schema. The schema is somewhat based on the one for SC7180/SC7280 but adjusted for the old platforms. Compared to the old plain text bindings, add missing documentation for the "bam-dmux" subnode and recommend one particular approach to specify the MBA/MPSS "memory-region" (the other one is marked as deprecated). Cc: Sireesh Kodali <sireeshkodali1@gmail.com> Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com> --- Changes in v2: - Add blank lines between top-level properties - Drop "deprecated" in "oneOf" list, it is not clear if this is valid and it should be redundant since the properties itself are already marked as "deprecated" --- Like Sibi's patch series for SC7180/SC7820 [1] this is somewhat related to Sireesh's series that converts all of qcom,q6v5.txt [2] (with a lot of if statements). However, this series focuses on MSM8916/MSM8974 (or actually MSM8909) only. [1]: https://lore.kernel.org/linux-arm-msm/1657020721-24939-1-git-send-email-quic_sibis@quicinc.com/ [2]: https://lore.kernel.org/linux-arm-msm/20220511161602.117772-7-sireeshkodali1@gmail.com/ --- .../remoteproc/qcom,msm8916-mss-pil.yaml | 247 ++++++++++++++++++ .../bindings/remoteproc/qcom,q6v5.txt | 19 -- 2 files changed, 247 insertions(+), 19 deletions(-) create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml