| Message ID | 1631811353-503-3-git-send-email-pillair@codeaurora.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Add support for sc7280 WPSS PIL loading | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | success | |
| robh/dtbs-check | success |
Quoting Rakesh Pillai (2021-09-16 09:55:52) > @@ -78,6 +84,10 @@ properties: > Phandle reference to a syscon representing TCSR followed by the > three offsets within syscon for q6, modem and nc halt registers. > > + qcom,qmp: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: Reference to the AOSS side-channel message RAM. > + > qcom,smem-states: > $ref: /schemas/types.yaml#/definitions/phandle-array > description: States used by the AP to signal the Hexagon core > @@ -117,6 +127,33 @@ allOf: > compatible: > contains: > enum: > + - qcom,sc7280-wpss-pil > + then: Honestly I find this if/else to be a huge tangle. Why not split the binding so that each compatible is a different file? Then it is easier to read and see what properties to set. > + properties: > + interrupts-extended: > + maxItems: 6 > + items: > + - description: Watchdog interrupt > + - description: Fatal interrupt > + - description: Ready interrupt
> -----Original Message----- > From: Stephen Boyd <swboyd@chromium.org> > Sent: Friday, September 17, 2021 11:56 AM > To: Rakesh Pillai <pillair@codeaurora.org>; agross@kernel.org; > bjorn.andersson@linaro.org; mathieu.poirier@linaro.org; ohad@wizery.com; > p.zabel@pengutronix.de; robh+dt@kernel.org > Cc: linux-arm-msm@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; sibis@codeaurora.org; mpubbise@codeaurora.org; > kuabhs@chromium.org > Subject: Re: [PATCH v3 2/3] dt-bindings: remoteproc: qcom: Add SC7280 > WPSS support > > Quoting Rakesh Pillai (2021-09-16 09:55:52) > > @@ -78,6 +84,10 @@ properties: > > Phandle reference to a syscon representing TCSR followed by the > > three offsets within syscon for q6, modem and nc halt registers. > > > > + qcom,qmp: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: Reference to the AOSS side-channel message RAM. > > + > > qcom,smem-states: > > $ref: /schemas/types.yaml#/definitions/phandle-array > > description: States used by the AP to signal the Hexagon core @@ > > -117,6 +127,33 @@ allOf: > > compatible: > > contains: > > enum: > > + - qcom,sc7280-wpss-pil > > + then: > > Honestly I find this if/else to be a huge tangle. Why not split the binding so > that each compatible is a different file? Then it is easier to read and see what > properties to set. Hi Stephen, I will create a separate dt-bindings yaml file for sc7280-wpss-pil, which will avoid all such if-else conditions. > > > + properties: > > + interrupts-extended: > > + maxItems: 6 > > + items: > > + - description: Watchdog interrupt > > + - description: Fatal interrupt > > + - description: Ready interrupt
On Thu 16 Sep 23:25 PDT 2021, Stephen Boyd wrote: > Quoting Rakesh Pillai (2021-09-16 09:55:52) > > @@ -78,6 +84,10 @@ properties: > > Phandle reference to a syscon representing TCSR followed by the > > three offsets within syscon for q6, modem and nc halt registers. > > > > + qcom,qmp: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: Reference to the AOSS side-channel message RAM. > > + > > qcom,smem-states: > > $ref: /schemas/types.yaml#/definitions/phandle-array > > description: States used by the AP to signal the Hexagon core > > @@ -117,6 +127,33 @@ allOf: > > compatible: > > contains: > > enum: > > + - qcom,sc7280-wpss-pil > > + then: > > Honestly I find this if/else to be a huge tangle. Why not split the > binding so that each compatible is a different file? Then it is easier > to read and see what properties to set. > Further more, the way we express the non-PAS properties in the PAS node in the dtsi and then switch the compatible in the non-PAS devices means that we're causing validation errors. So we should explode this binding to get rid of the conditionals and to describe the "superset" of the PAS and non-PAS compatibles, for platforms where this is applicable. Regards, Bjorn > > + properties: > > + interrupts-extended: > > + maxItems: 6 > > + items: > > + - description: Watchdog interrupt > > + - description: Fatal interrupt > > + - description: Ready interrupt
> -----Original Message----- > From: Bjorn Andersson <bjorn.andersson@linaro.org> > Sent: Wednesday, September 22, 2021 5:08 AM > To: Stephen Boyd <swboyd@chromium.org> > Cc: Rakesh Pillai <pillair@codeaurora.org>; agross@kernel.org; > mathieu.poirier@linaro.org; ohad@wizery.com; p.zabel@pengutronix.de; > robh+dt@kernel.org; linux-arm-msm@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > sibis@codeaurora.org; mpubbise@codeaurora.org; kuabhs@chromium.org > Subject: Re: [PATCH v3 2/3] dt-bindings: remoteproc: qcom: Add SC7280 > WPSS support > > On Thu 16 Sep 23:25 PDT 2021, Stephen Boyd wrote: > > > Quoting Rakesh Pillai (2021-09-16 09:55:52) > > > @@ -78,6 +84,10 @@ properties: > > > Phandle reference to a syscon representing TCSR followed by the > > > three offsets within syscon for q6, modem and nc halt registers. > > > > > > + qcom,qmp: > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > + description: Reference to the AOSS side-channel message RAM. > > > + > > > qcom,smem-states: > > > $ref: /schemas/types.yaml#/definitions/phandle-array > > > description: States used by the AP to signal the Hexagon core > > > @@ -117,6 +127,33 @@ allOf: > > > compatible: > > > contains: > > > enum: > > > + - qcom,sc7280-wpss-pil > > > + then: > > > > Honestly I find this if/else to be a huge tangle. Why not split the > > binding so that each compatible is a different file? Then it is easier > > to read and see what properties to set. > > > > Further more, the way we express the non-PAS properties in the PAS node > in the dtsi and then switch the compatible in the non-PAS devices means that > we're causing validation errors. > > So we should explode this binding to get rid of the conditionals and to > describe the "superset" of the PAS and non-PAS compatibles, for platforms > where this is applicable. > > Regards, > Bjorn Hi Bjorn, I have posted v4 for this patchseries with wpss dt-bindings added as a separate file. Can you please check v4 ? Thanks, Rakesh Pillai. > > > > + properties: > > > + interrupts-extended: > > > + maxItems: 6 > > > + items: > > > + - description: Watchdog interrupt > > > + - description: Fatal interrupt > > > + - description: Ready interrupt
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.yaml index 051da43..5674602 100644 --- a/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.yaml +++ b/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.yaml @@ -17,6 +17,7 @@ properties: compatible: enum: - qcom,qcs404-cdsp-pil + - qcom,sc7280-wpss-pil - qcom,sdm845-adsp-pil reg: @@ -43,14 +44,14 @@ properties: - const: stop-ack clocks: - minItems: 7 + minItems: 3 maxItems: 8 description: List of phandles and clock specifier pairs for the Hexagon, per clock-names below. clock-names: - minItems: 7 + minItems: 3 maxItems: 8 power-domains: @@ -58,6 +59,11 @@ properties: items: - description: CX power domain + power-domain-names: + minItems: 1 + items: + - const: cx + resets: minItems: 1 maxItems: 2 @@ -78,6 +84,10 @@ properties: Phandle reference to a syscon representing TCSR followed by the three offsets within syscon for q6, modem and nc halt registers. + qcom,qmp: + $ref: /schemas/types.yaml#/definitions/phandle + description: Reference to the AOSS side-channel message RAM. + qcom,smem-states: $ref: /schemas/types.yaml#/definitions/phandle-array description: States used by the AP to signal the Hexagon core @@ -117,6 +127,33 @@ allOf: compatible: contains: enum: + - qcom,sc7280-wpss-pil + then: + properties: + interrupts-extended: + maxItems: 6 + items: + - description: Watchdog interrupt + - description: Fatal interrupt + - description: Ready interrupt + - description: Handover interrupt + - description: Stop acknowledge interrupt + - description: Shutdown acknowledge interrupt + interrupt-names: + maxItems: 6 + items: + - const: wdog + - const: fatal + - const: ready + - const: handover + - const: stop-ack + - const: shutdown-ack + + - if: + properties: + compatible: + contains: + enum: - qcom,sdm845-adsp-pil then: properties: @@ -192,6 +229,25 @@ allOf: compatible: contains: enum: + - qcom,sc7280-wpss-pil + then: + properties: + power-domains: + maxItems: 2 + items: + - description: CX power domain + - description: MX power domain + power-domain-names: + maxItems: 2 + items: + - const: cx + - const: mx + + - if: + properties: + compatible: + contains: + enum: - qcom,sdm845-adsp-pil then: properties: @@ -219,6 +275,34 @@ allOf: items: - const: restart + - if: + properties: + compatible: + contains: + enum: + - qcom,sc7280-wpss-pil + then: + properties: + resets: + items: + - description: AOSS restart + - description: PDC SYNC + reset-names: + items: + - const: restart + - const: pdc_sync + + - if: + properties: + compatible: + contains: + enum: + - qcom,sdm845-adsp-pil + - qcom,qcs404-cdsp-pil + then: + properties: + qcom,qmp: false + examples: - | #include <dt-bindings/interrupt-controller/arm-gic.h>
Add WPSS PIL loading support for SC7280 SoCs. Signed-off-by: Rakesh Pillai <pillair@codeaurora.org> --- .../bindings/remoteproc/qcom,hexagon-v56.yaml | 88 +++++++++++++++++++++- 1 file changed, 86 insertions(+), 2 deletions(-)