Message ID | 1663938340-24345-1-git-send-email-quic_srivasam@quicinc.com |
---|---|
Headers | show |
Series | Update ADSP pil loader for SC7280 platform | expand |
On 23/09/2022 15:05, Srinivasa Rao Mandadapu wrote: > Add efuse evb selection control and enable it for starting ADSP. > > Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com> > Reviewed-by: Stephen Boyd <swboyd@chromium.org> > Reviewed-by: Sibi Sankar <quic_sibis@quicinc.com> > --- Thank you for your patch. There is something to discuss/improve. > @@ -543,6 +549,17 @@ static int adsp_init_mmio(struct qcom_adsp *adsp, > return PTR_ERR(adsp->qdsp6ss_base); > } > > + efuse_region = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + if (!efuse_region) { > + adsp->lpass_efuse = NULL; > + dev_dbg(adsp->dev, "failed to get efuse memory region\n"); > + } else { This needs bindings updates in all users. Best regards, Krzysztof
On 9/23/2022 10:53 PM, Krzysztof Kozlowski wrote: Thanks for Your time Krzyszto!!! > On 23/09/2022 15:05, Srinivasa Rao Mandadapu wrote: >> Add ADSP PIL loading support for SC7280 SoCs. >> >> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com> >> Reviewed-by: Rob Herring <robh@kernel.org> >> Reviewed-by: Stephen Boyd <swboyd@chromium.org> >> --- >> Changes since V7: >> -- Remove redundant clocks in dt bindings. >> -- Fix dt compilation error in dt bindings. >> Changes since V6: >> -- Update glink-edge property. >> -- Add qcom,qmp property. >> Changes since V5: >> -- Remove qcom,adsp-memory-regions property. >> Changes since V4: >> -- Update halt registers description in dt bindings. >> >> .../bindings/remoteproc/qcom,sc7280-adsp-pil.yaml | 207 +++++++++++++++++++++ >> 1 file changed, 207 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml >> >> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml >> new file mode 100644 >> index 0000000..79ef3c0 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml >> @@ -0,0 +1,207 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/remoteproc/qcom,sc7280-adsp-pil.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm SC7280 ADSP Peripheral Image Loader >> + >> +maintainers: >> + - Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com> >> + >> +description: >> + This document describes the hardware for a component that loads and boots firmware >> + on the Qualcomm Technology Inc. ADSP. >> + >> +properties: >> + compatible: >> + enum: >> + - qcom,sc7280-adsp-pil >> + >> + reg: >> + minItems: 1 >> + items: >> + - description: qdsp6ss register >> + - description: efuse q6ss register > Why second IO address space is optional? Yeah, both are required for SC7280. will remove minItems. > >> + >> + interrupts: >> + items: >> + - description: Watchdog interrupt >> + - description: Fatal interrupt >> + - description: Ready interrupt >> + - description: Handover interrupt >> + - description: Stop acknowledge interrupt >> + - description: Shutdown acknowledge interrupt >> + >> + interrupt-names: >> + items: >> + - const: wdog >> + - const: fatal >> + - const: ready >> + - const: handover >> + - const: stop-ack >> + - const: shutdown-ack >> + >> + clocks: >> + items: >> + - description: XO clock >> + - description: GCC CFG NOC LPASS clock >> + >> + clock-names: >> + items: >> + - const: xo >> + - const: gcc_cfg_noc_lpass >> + >> + power-domains: >> + items: >> + - description: LCX power domain >> + >> + resets: >> + items: >> + - description: PDC AUDIO SYNC RESET >> + - description: CC LPASS restart >> + >> + reset-names: >> + items: >> + - const: pdc_sync >> + - const: cc_lpass >> + >> + memory-region: >> + maxItems: 1 >> + description: Reference to the reserved-memory for the Hexagon core >> + >> + qcom,halt-regs: >> + $ref: /schemas/types.yaml#/definitions/phandle-array >> + description: >> + Phandle reference to a syscon representing TCSR followed by the >> + four offsets within syscon for q6, CE, AXI and qv6 halt registers. >> + items: >> + items: > This has to be strictly defined, IOW, the second items must be already > an item of previous list. Look at the other mss-pil. Okay. Will modify as per mss-pil. > >> + - description: phandle to TCSR MUTEX >> + - description: offset to q6 halt registers >> + - description: offset to CE halt registers >> + - description: offset to AXI halt registers >> + - description: offset to qv6 halt registers >> + >> + qcom,smem-states: >> + $ref: /schemas/types.yaml#/definitions/phandle-array >> + description: States used by the AP to signal the Hexagon core >> + items: >> + - description: Stop the modem >> + >> + qcom,smem-state-names: >> + description: The names of the state bits used for SMP2P output >> + const: stop >> + >> + qcom,qmp: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: Reference to the AOSS side-channel message RAM. >> + >> + glink-edge: >> + type: object > Missing ref to glink-edge and unevaluatedProperties:false. Please take a > look at recent > Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml Okay. Will add reference to glink-edge. > >> + description: | >> + Qualcomm G-Link subnode which represents communication edge, channels >> + and devices related to the ADSP. >> + >> + properties: >> + interrupts: >> + items: >> + - description: IRQ from ADSP to GLINK > Skip interrupts and mboxes - both are coming from glink-edge. Okay. Will remove it. But Same is followed in mss-pil > >> + >> + mboxes: >> + items: >> + - description: Mailbox for communication between APPS and ADSP >> + >> + label: >> + description: The names of the state bits used for SMP2P output > Skip description Okay. > >> + items: >> + - const: lpass > No items, just const: lpass Okay. > >> + >> + qcom,remote-pid: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: ID of the shared memory used by GLINK for communication with ADSP > This can be dropped. Okay. Will remove qcom,remote-pid. > >> + >> + gpr: true >> + apr: false >> + fastrpc: false > > BTW, all these three do not make sense without ref to glink-edge. After > adding ref, these seem reasonable. Okay. will add glink-edge reference. > >> + >> + required: >> + - interrupts >> + - mboxes >> + - label >> + - qcom,remote-pid >> + >> +required: >> + - compatible >> + - reg >> + - interrupts >> + - interrupt-names >> + - clocks >> + - clock-names >> + - power-domains >> + - resets >> + - reset-names >> + - qcom,halt-regs >> + - memory-region >> + - qcom,smem-states >> + - qcom,smem-state-names >> + - qcom,qmp >> + >> +additionalProperties: false > Best regards, > Krzysztof >
On 9/23/2022 10:55 PM, Krzysztof Kozlowski wrote: Thanks for Your time Krzyszto!!! > On 23/09/2022 15:05, Srinivasa Rao Mandadapu wrote: >> Add efuse evb selection control and enable it for starting ADSP. >> >> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com> >> Reviewed-by: Stephen Boyd <swboyd@chromium.org> >> Reviewed-by: Sibi Sankar <quic_sibis@quicinc.com> >> --- > Thank you for your patch. There is something to discuss/improve. > >> @@ -543,6 +549,17 @@ static int adsp_init_mmio(struct qcom_adsp *adsp, >> return PTR_ERR(adsp->qdsp6ss_base); >> } >> >> + efuse_region = platform_get_resource(pdev, IORESOURCE_MEM, 1); >> + if (!efuse_region) { >> + adsp->lpass_efuse = NULL; >> + dev_dbg(adsp->dev, "failed to get efuse memory region\n"); >> + } else { > This needs bindings updates in all users. Actually this is being used in SC7280 platform only. I am not sure if it applicable for other platforms. If required, it can be posted as new series after this series got main lined. > > Best regards, > Krzysztof >