* [PATCH v3 0/2] Add ADSP PIL driver for SDM845 @ 2018-09-03 11:52 Rohit kumar 2018-09-03 11:52 ` [PATCH v3 1/2] dt-binding: remoteproc: Add QTI ADSP PIL bindings Rohit kumar 2018-09-03 11:52 ` [PATCH v3 2/2] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver Rohit kumar 0 siblings, 2 replies; 13+ messages in thread From: Rohit kumar @ 2018-09-03 11:52 UTC (permalink / raw) To: ohad, bjorn.andersson, robh+dt, mark.rutland, linux-remoteproc, devicetree, linux-kernel, plai, bgoswami, srinivas.kandagatla Cc: Rohit kumar This provides initial patchset for ADSP PIL driver for SDM845. Changes since v2: Separated dt-bindings documentation into separate patch. Used clk_bulk APIs for adsp clocks. Used power domain API and use reference to rpmhd resource. Renamed qcom_nonpas_adsp_pil.c to qcom_adsp_pil.c Removed smd as sdm845 does not support smd. Removed px_supply and aggre2_clk for sdm845 as they are not required. Removed reg-names as there is only one register. This patch is dependent on the rpmh powerdomain driver https://lkml.org/lkml/2018/6/27/7 and renaming of Hexagon v5 PAS driver https://lkml.org/lkml/2018/8/28/129 . Rohit kumar (2): dt-binding: remoteproc: Add QTI ADSP PIL bindings remoteproc: qcom: Introduce Non-PAS ADSP PIL driver .../bindings/remoteproc/qcom,adsp-pil.txt | 123 +++++ drivers/remoteproc/Kconfig | 14 + drivers/remoteproc/Makefile | 1 + drivers/remoteproc/qcom_adsp_pil.c | 500 +++++++++++++++++++++ 4 files changed, 638 insertions(+) create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt create mode 100644 drivers/remoteproc/qcom_adsp_pil.c -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 1/2] dt-binding: remoteproc: Add QTI ADSP PIL bindings 2018-09-03 11:52 [PATCH v3 0/2] Add ADSP PIL driver for SDM845 Rohit kumar @ 2018-09-03 11:52 ` Rohit kumar 2018-09-10 18:27 ` Bjorn Andersson 2018-09-10 20:01 ` Rob Herring 2018-09-03 11:52 ` [PATCH v3 2/2] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver Rohit kumar 1 sibling, 2 replies; 13+ messages in thread From: Rohit kumar @ 2018-09-03 11:52 UTC (permalink / raw) To: ohad, bjorn.andersson, robh+dt, mark.rutland, linux-remoteproc, devicetree, linux-kernel, plai, bgoswami, srinivas.kandagatla Cc: Rohit kumar Add devicetree bindings documentation file for Qualcomm Technolgies Inc ADSP Peripheral Image Loader. Signed-off-by: Rohit kumar <rohitkr@codeaurora.org> --- .../bindings/remoteproc/qcom,adsp-pil.txt | 123 +++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt b/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt new file mode 100644 index 0000000..f1c215a --- /dev/null +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt @@ -0,0 +1,123 @@ +Qualcomm Technology Inc. ADSP Peripheral Image Loader + +This document defines the binding for a component that loads and boots firmware +on the Qualcomm Technology Inc. ADSP Hexagon core. + +- compatible: + Usage: required + Value type: <string> + Definition: must be one of: + "qcom,sdm845-adsp-pil" + +- reg: + Usage: required + Value type: <prop-encoded-array> + Definition: must specify the base address and size of the qdsp6ss register + +- interrupts-extended: + Usage: required + Value type: <prop-encoded-array> + Definition: must list the watchdog, fatal IRQs ready, handover and + stop-ack IRQs + +- interrupt-names: + Usage: required + Value type: <stringlist> + Definition: must be "wdog", "fatal", "ready", "handover", "stop-ack" + +- clocks: + Usage: required + Value type: <prop-encoded-array> + Definition: List of phandle and clock specifier pairs + +- clock-names: + Usage: required + Value type: <stringlist> + Definition: List of clock input name strings sorted in the same + order as the clocks property. + +- power-domains: + Usage: required + Value type: <phandle> + Definition: reference to cx power domain node. + +- resets: + Usage: required + Value type: <phandle> + Definition: reference to the reset-controller for the lpass + +- reset-names: + Usage: required + Value type: <stringlist> + Definition: must be "pdc_sync" and "cc_lpass" + +- qcom,halt-regs: + Usage: required + Value type: <prop-encoded-array> + Definition: a phandle reference to a syscon representing TCSR followed + by the offset within syscon for lpass halt register. + +- memory-region: + Usage: required + Value type: <phandle> + Definition: reference to the reserved-memory for the ADSP + +- qcom,smem-states: + Usage: required + Value type: <phandle> + Definition: reference to the smem state for requesting the ADSP to + shut down + +- qcom,smem-state-names: + Usage: required + Value type: <stringlist> + Definition: must be "stop" + + += SUBNODES +The adsp node may have an subnode named "glink-edge" that describes the +communication edge, channels and devices related to the ADSP. +See ../soc/qcom/qcom,glink.txt for details on how to describe these. + += EXAMPLE +The following example describes the resources needed to boot control the +ADSP, as it is found on SDM845 boards. + adsp-pil { + compatible = "qcom,sdm845-adsp-pil"; + + reg = <0x17300000 0x40c>; + + interrupts-extended = <&intc 0 162 IRQ_TYPE_EDGE_RISING>, + <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>, + <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>, + <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>, + <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>; + interrupt-names = "wdog", "fatal", "ready", + "handover", "stop-ack"; + + clocks = <&rpmhcc RPMH_CXO_CLK>, + <&gcc GCC_LPASS_SWAY_CLK>, + <&lpasscc LPASS_AUDIO_WRAPPER_AON_CLK>, + <&lpasscc LPASS_Q6SS_AHBS_AON_CLK>, + <&lpasscc LPASS_Q6SS_AHBM_AON_CLK>, + <&lpasscc LPASS_QDSP6SS_XO_CLK>, + <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>, + <&lpasscc LPASS_QDSP6SS_CORE_CLK>; + clock-names = "xo", "sway_cbcr", "lpass_aon", + "lpass_ahbs_aon_cbcr", + "lpass_ahbm_aon_cbcr", "qdsp6ss_xo", + "qdsp6ss_sleep", "qdsp6ss_core"; + + power-domains = <&rpmhpd SDM845_CX>; + + resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>, + <&aoss_reset AOSS_CC_LPASS_RESTART>; + reset-names = "pdc_sync", "cc_lpass"; + + qcom,halt-regs = <&tcsr_mutex_regs 0x22000>; + + memory-region = <&pil_adsp_mem>; + + qcom,smem-states = <&adsp_smp2p_out 0>; + qcom,smem-state-names = "stop"; + }; -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/2] dt-binding: remoteproc: Add QTI ADSP PIL bindings 2018-09-03 11:52 ` [PATCH v3 1/2] dt-binding: remoteproc: Add QTI ADSP PIL bindings Rohit kumar @ 2018-09-10 18:27 ` Bjorn Andersson 2018-09-10 20:01 ` Rob Herring 1 sibling, 0 replies; 13+ messages in thread From: Bjorn Andersson @ 2018-09-10 18:27 UTC (permalink / raw) To: Rohit kumar Cc: ohad, robh+dt, mark.rutland, linux-remoteproc, devicetree, linux-kernel, plai, bgoswami, srinivas.kandagatla On Mon 03 Sep 04:52 PDT 2018, Rohit kumar wrote: > Add devicetree bindings documentation file for Qualcomm > Technolgies Inc ADSP Peripheral Image Loader. > > Signed-off-by: Rohit kumar <rohitkr@codeaurora.org> Rob, this revision looks good to me and I would like to move ahead and merge it. Regards, Bjorn > --- > .../bindings/remoteproc/qcom,adsp-pil.txt | 123 +++++++++++++++++++++ > 1 file changed, 123 insertions(+) > create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt > > diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt b/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt > new file mode 100644 > index 0000000..f1c215a > --- /dev/null > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt > @@ -0,0 +1,123 @@ > +Qualcomm Technology Inc. ADSP Peripheral Image Loader > + > +This document defines the binding for a component that loads and boots firmware > +on the Qualcomm Technology Inc. ADSP Hexagon core. > + > +- compatible: > + Usage: required > + Value type: <string> > + Definition: must be one of: > + "qcom,sdm845-adsp-pil" > + > +- reg: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: must specify the base address and size of the qdsp6ss register > + > +- interrupts-extended: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: must list the watchdog, fatal IRQs ready, handover and > + stop-ack IRQs > + > +- interrupt-names: > + Usage: required > + Value type: <stringlist> > + Definition: must be "wdog", "fatal", "ready", "handover", "stop-ack" > + > +- clocks: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: List of phandle and clock specifier pairs > + > +- clock-names: > + Usage: required > + Value type: <stringlist> > + Definition: List of clock input name strings sorted in the same > + order as the clocks property. > + > +- power-domains: > + Usage: required > + Value type: <phandle> > + Definition: reference to cx power domain node. > + > +- resets: > + Usage: required > + Value type: <phandle> > + Definition: reference to the reset-controller for the lpass > + > +- reset-names: > + Usage: required > + Value type: <stringlist> > + Definition: must be "pdc_sync" and "cc_lpass" > + > +- qcom,halt-regs: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: a phandle reference to a syscon representing TCSR followed > + by the offset within syscon for lpass halt register. > + > +- memory-region: > + Usage: required > + Value type: <phandle> > + Definition: reference to the reserved-memory for the ADSP > + > +- qcom,smem-states: > + Usage: required > + Value type: <phandle> > + Definition: reference to the smem state for requesting the ADSP to > + shut down > + > +- qcom,smem-state-names: > + Usage: required > + Value type: <stringlist> > + Definition: must be "stop" > + > + > += SUBNODES > +The adsp node may have an subnode named "glink-edge" that describes the > +communication edge, channels and devices related to the ADSP. > +See ../soc/qcom/qcom,glink.txt for details on how to describe these. > + > += EXAMPLE > +The following example describes the resources needed to boot control the > +ADSP, as it is found on SDM845 boards. > + adsp-pil { > + compatible = "qcom,sdm845-adsp-pil"; > + > + reg = <0x17300000 0x40c>; > + > + interrupts-extended = <&intc 0 162 IRQ_TYPE_EDGE_RISING>, > + <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>, > + <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>, > + <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>, > + <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>; > + interrupt-names = "wdog", "fatal", "ready", > + "handover", "stop-ack"; > + > + clocks = <&rpmhcc RPMH_CXO_CLK>, > + <&gcc GCC_LPASS_SWAY_CLK>, > + <&lpasscc LPASS_AUDIO_WRAPPER_AON_CLK>, > + <&lpasscc LPASS_Q6SS_AHBS_AON_CLK>, > + <&lpasscc LPASS_Q6SS_AHBM_AON_CLK>, > + <&lpasscc LPASS_QDSP6SS_XO_CLK>, > + <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>, > + <&lpasscc LPASS_QDSP6SS_CORE_CLK>; > + clock-names = "xo", "sway_cbcr", "lpass_aon", > + "lpass_ahbs_aon_cbcr", > + "lpass_ahbm_aon_cbcr", "qdsp6ss_xo", > + "qdsp6ss_sleep", "qdsp6ss_core"; > + > + power-domains = <&rpmhpd SDM845_CX>; > + > + resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>, > + <&aoss_reset AOSS_CC_LPASS_RESTART>; > + reset-names = "pdc_sync", "cc_lpass"; > + > + qcom,halt-regs = <&tcsr_mutex_regs 0x22000>; > + > + memory-region = <&pil_adsp_mem>; > + > + qcom,smem-states = <&adsp_smp2p_out 0>; > + qcom,smem-state-names = "stop"; > + }; > -- > Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., > is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/2] dt-binding: remoteproc: Add QTI ADSP PIL bindings 2018-09-03 11:52 ` [PATCH v3 1/2] dt-binding: remoteproc: Add QTI ADSP PIL bindings Rohit kumar 2018-09-10 18:27 ` Bjorn Andersson @ 2018-09-10 20:01 ` Rob Herring 2018-09-11 3:46 ` Rohit Kumar 1 sibling, 1 reply; 13+ messages in thread From: Rob Herring @ 2018-09-10 20:01 UTC (permalink / raw) To: Rohit kumar Cc: ohad, bjorn.andersson, mark.rutland, linux-remoteproc, devicetree, linux-kernel, plai, bgoswami, srinivas.kandagatla On Mon, Sep 03, 2018 at 05:22:39PM +0530, Rohit kumar wrote: > Add devicetree bindings documentation file for Qualcomm > Technolgies Inc ADSP Peripheral Image Loader. > > Signed-off-by: Rohit kumar <rohitkr@codeaurora.org> > --- > .../bindings/remoteproc/qcom,adsp-pil.txt | 123 +++++++++++++++++++++ > 1 file changed, 123 insertions(+) > create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt > > diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt b/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt > new file mode 100644 > index 0000000..f1c215a > --- /dev/null > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt > @@ -0,0 +1,123 @@ > +Qualcomm Technology Inc. ADSP Peripheral Image Loader > + > +This document defines the binding for a component that loads and boots firmware > +on the Qualcomm Technology Inc. ADSP Hexagon core. > + > +- compatible: > + Usage: required > + Value type: <string> > + Definition: must be one of: > + "qcom,sdm845-adsp-pil" > + > +- reg: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: must specify the base address and size of the qdsp6ss register > + > +- interrupts-extended: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: must list the watchdog, fatal IRQs ready, handover and > + stop-ack IRQs > + > +- interrupt-names: > + Usage: required > + Value type: <stringlist> > + Definition: must be "wdog", "fatal", "ready", "handover", "stop-ack" > + > +- clocks: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: List of phandle and clock specifier pairs How many clocks? > + > +- clock-names: > + Usage: required > + Value type: <stringlist> > + Definition: List of clock input name strings sorted in the same > + order as the clocks property. What are the names? > + > +- power-domains: > + Usage: required > + Value type: <phandle> > + Definition: reference to cx power domain node. > + > +- resets: > + Usage: required > + Value type: <phandle> > + Definition: reference to the reset-controller for the lpass How many? > + > +- reset-names: > + Usage: required > + Value type: <stringlist> > + Definition: must be "pdc_sync" and "cc_lpass" > + > +- qcom,halt-regs: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: a phandle reference to a syscon representing TCSR followed > + by the offset within syscon for lpass halt register. > + > +- memory-region: > + Usage: required > + Value type: <phandle> > + Definition: reference to the reserved-memory for the ADSP > + > +- qcom,smem-states: > + Usage: required > + Value type: <phandle> > + Definition: reference to the smem state for requesting the ADSP to > + shut down > + > +- qcom,smem-state-names: > + Usage: required > + Value type: <stringlist> > + Definition: must be "stop" > + > + > += SUBNODES > +The adsp node may have an subnode named "glink-edge" that describes the > +communication edge, channels and devices related to the ADSP. > +See ../soc/qcom/qcom,glink.txt for details on how to describe these. > + > += EXAMPLE > +The following example describes the resources needed to boot control the > +ADSP, as it is found on SDM845 boards. > + adsp-pil { > + compatible = "qcom,sdm845-adsp-pil"; > + > + reg = <0x17300000 0x40c>; > + > + interrupts-extended = <&intc 0 162 IRQ_TYPE_EDGE_RISING>, > + <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>, > + <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>, > + <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>, > + <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>; > + interrupt-names = "wdog", "fatal", "ready", > + "handover", "stop-ack"; > + > + clocks = <&rpmhcc RPMH_CXO_CLK>, > + <&gcc GCC_LPASS_SWAY_CLK>, > + <&lpasscc LPASS_AUDIO_WRAPPER_AON_CLK>, > + <&lpasscc LPASS_Q6SS_AHBS_AON_CLK>, > + <&lpasscc LPASS_Q6SS_AHBM_AON_CLK>, > + <&lpasscc LPASS_QDSP6SS_XO_CLK>, > + <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>, > + <&lpasscc LPASS_QDSP6SS_CORE_CLK>; > + clock-names = "xo", "sway_cbcr", "lpass_aon", > + "lpass_ahbs_aon_cbcr", > + "lpass_ahbm_aon_cbcr", "qdsp6ss_xo", > + "qdsp6ss_sleep", "qdsp6ss_core"; > + > + power-domains = <&rpmhpd SDM845_CX>; > + > + resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>, > + <&aoss_reset AOSS_CC_LPASS_RESTART>; > + reset-names = "pdc_sync", "cc_lpass"; > + > + qcom,halt-regs = <&tcsr_mutex_regs 0x22000>; > + > + memory-region = <&pil_adsp_mem>; > + > + qcom,smem-states = <&adsp_smp2p_out 0>; > + qcom,smem-state-names = "stop"; > + }; > -- > Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., > is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/2] dt-binding: remoteproc: Add QTI ADSP PIL bindings 2018-09-10 20:01 ` Rob Herring @ 2018-09-11 3:46 ` Rohit Kumar 0 siblings, 0 replies; 13+ messages in thread From: Rohit Kumar @ 2018-09-11 3:46 UTC (permalink / raw) To: Rob Herring Cc: ohad, bjorn.andersson, mark.rutland, linux-remoteproc, devicetree, linux-kernel, plai, bgoswami, srinivas.kandagatla Thanks Rob for reviewing. On 9/11/2018 1:31 AM, Rob Herring wrote: > On Mon, Sep 03, 2018 at 05:22:39PM +0530, Rohit kumar wrote: >> Add devicetree bindings documentation file for Qualcomm >> Technolgies Inc ADSP Peripheral Image Loader. >> >> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org> >> --- >> .../bindings/remoteproc/qcom,adsp-pil.txt | 123 +++++++++++++++++++++ >> 1 file changed, 123 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt >> >> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt b/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt >> new file mode 100644 >> index 0000000..f1c215a >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,adsp-pil.txt >> @@ -0,0 +1,123 @@ >> +Qualcomm Technology Inc. ADSP Peripheral Image Loader >> + >> +This document defines the binding for a component that loads and boots firmware >> +on the Qualcomm Technology Inc. ADSP Hexagon core. >> + >> +- compatible: >> + Usage: required >> + Value type: <string> >> + Definition: must be one of: >> + "qcom,sdm845-adsp-pil" >> + >> +- reg: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: must specify the base address and size of the qdsp6ss register >> + >> +- interrupts-extended: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: must list the watchdog, fatal IRQs ready, handover and >> + stop-ack IRQs >> + >> +- interrupt-names: >> + Usage: required >> + Value type: <stringlist> >> + Definition: must be "wdog", "fatal", "ready", "handover", "stop-ack" >> + >> +- clocks: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: List of phandle and clock specifier pairs > How many clocks? > >> + >> +- clock-names: >> + Usage: required >> + Value type: <stringlist> >> + Definition: List of clock input name strings sorted in the same >> + order as the clocks property. > What are the names? I will update these in next spin. >> + >> +- power-domains: >> + Usage: required >> + Value type: <phandle> >> + Definition: reference to cx power domain node. >> + >> +- resets: >> + Usage: required >> + Value type: <phandle> >> + Definition: reference to the reset-controller for the lpass > How many? > >> + >> +- reset-names: >> + Usage: required >> + Value type: <stringlist> >> + Definition: must be "pdc_sync" and "cc_lpass" >> + >> +- qcom,halt-regs: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: a phandle reference to a syscon representing TCSR followed >> + by the offset within syscon for lpass halt register. >> + >> +- memory-region: >> + Usage: required >> + Value type: <phandle> >> + Definition: reference to the reserved-memory for the ADSP >> + >> +- qcom,smem-states: >> + Usage: required >> + Value type: <phandle> >> + Definition: reference to the smem state for requesting the ADSP to >> + shut down >> + >> +- qcom,smem-state-names: >> + Usage: required >> + Value type: <stringlist> >> + Definition: must be "stop" >> + >> + >> += SUBNODES >> +The adsp node may have an subnode named "glink-edge" that describes the >> +communication edge, channels and devices related to the ADSP. >> +See ../soc/qcom/qcom,glink.txt for details on how to describe these. >> + >> += EXAMPLE >> +The following example describes the resources needed to boot control the >> +ADSP, as it is found on SDM845 boards. >> + adsp-pil { >> + compatible = "qcom,sdm845-adsp-pil"; >> + >> + reg = <0x17300000 0x40c>; >> + >> + interrupts-extended = <&intc 0 162 IRQ_TYPE_EDGE_RISING>, >> + <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>, >> + <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>, >> + <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>, >> + <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>; >> + interrupt-names = "wdog", "fatal", "ready", >> + "handover", "stop-ack"; >> + >> + clocks = <&rpmhcc RPMH_CXO_CLK>, >> + <&gcc GCC_LPASS_SWAY_CLK>, >> + <&lpasscc LPASS_AUDIO_WRAPPER_AON_CLK>, >> + <&lpasscc LPASS_Q6SS_AHBS_AON_CLK>, >> + <&lpasscc LPASS_Q6SS_AHBM_AON_CLK>, >> + <&lpasscc LPASS_QDSP6SS_XO_CLK>, >> + <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>, >> + <&lpasscc LPASS_QDSP6SS_CORE_CLK>; >> + clock-names = "xo", "sway_cbcr", "lpass_aon", >> + "lpass_ahbs_aon_cbcr", >> + "lpass_ahbm_aon_cbcr", "qdsp6ss_xo", >> + "qdsp6ss_sleep", "qdsp6ss_core"; >> + >> + power-domains = <&rpmhpd SDM845_CX>; >> + >> + resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>, >> + <&aoss_reset AOSS_CC_LPASS_RESTART>; >> + reset-names = "pdc_sync", "cc_lpass"; >> + >> + qcom,halt-regs = <&tcsr_mutex_regs 0x22000>; >> + >> + memory-region = <&pil_adsp_mem>; >> + >> + qcom,smem-states = <&adsp_smp2p_out 0>; >> + qcom,smem-state-names = "stop"; >> + }; >> -- >> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., >> is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. Thanks, Rohit ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 2/2] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver 2018-09-03 11:52 [PATCH v3 0/2] Add ADSP PIL driver for SDM845 Rohit kumar 2018-09-03 11:52 ` [PATCH v3 1/2] dt-binding: remoteproc: Add QTI ADSP PIL bindings Rohit kumar @ 2018-09-03 11:52 ` Rohit kumar 2018-09-10 18:31 ` Bjorn Andersson 2018-09-21 19:41 ` Sibi Sankar 1 sibling, 2 replies; 13+ messages in thread From: Rohit kumar @ 2018-09-03 11:52 UTC (permalink / raw) To: ohad, bjorn.andersson, robh+dt, mark.rutland, linux-remoteproc, devicetree, linux-kernel, plai, bgoswami, srinivas.kandagatla Cc: Rohit kumar This adds Non PAS ADSP PIL driver for Qualcomm Technologies Inc SoCs. Added initial support for SDM845 with ADSP bootup and shutdown operation handled from Application Processor SubSystem(APSS). Signed-off-by: Rohit kumar <rohitkr@codeaurora.org> --- drivers/remoteproc/Kconfig | 14 ++ drivers/remoteproc/Makefile | 1 + drivers/remoteproc/qcom_adsp_pil.c | 500 +++++++++++++++++++++++++++++++++++++ 3 files changed, 515 insertions(+) create mode 100644 drivers/remoteproc/qcom_adsp_pil.c diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig index c98c0b2..445de2d 100644 --- a/drivers/remoteproc/Kconfig +++ b/drivers/remoteproc/Kconfig @@ -139,6 +139,20 @@ config QCOM_Q6V5_WCSS Say y here to support the Qualcomm Peripheral Image Loader for the Hexagon V5 based WCSS remote processors. +config QCOM_ADSP_PIL + tristate "Qualcomm Technology Inc ADSP Peripheral Image Loader" + depends on OF && ARCH_QCOM + depends on QCOM_SMEM + depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n + depends on QCOM_SYSMON || QCOM_SYSMON=n + select MFD_SYSCON + select QCOM_MDT_LOADER + select QCOM_Q6V5_COMMON + select QCOM_RPROC_COMMON + help + Say y here to support the Peripherial Image Loader + for the Qualcomm Technology Inc. ADSP remote processors. + config QCOM_SYSMON tristate "Qualcomm sysmon driver" depends on RPMSG diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile index eb86c8b..1258519 100644 --- a/drivers/remoteproc/Makefile +++ b/drivers/remoteproc/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_QCOM_Q6V5_COMMON) += qcom_q6v5.o obj-$(CONFIG_QCOM_Q6V5_PAS) += qcom_q6v5_pas.o obj-$(CONFIG_QCOM_Q6V5_PIL) += qcom_q6v5_pil.o obj-$(CONFIG_QCOM_Q6V5_WCSS) += qcom_q6v5_wcss.o +obj-$(CONFIG_QCOM_ADSP_PIL) += qcom_adsp_pil.o obj-$(CONFIG_QCOM_SYSMON) += qcom_sysmon.o obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o qcom_wcnss_pil-y += qcom_wcnss.o diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c new file mode 100644 index 0000000..977e804 --- /dev/null +++ b/drivers/remoteproc/qcom_adsp_pil.c @@ -0,0 +1,500 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Qualcomm Technology Inc. ADSP Peripheral Image Loader for SDM845. + * Copyright (c) 2018, The Linux Foundation. All rights reserved. + */ + +#include <linux/clk.h> +#include <linux/firmware.h> +#include <linux/interrupt.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/delay.h> +#include <linux/io.h> +#include <linux/reset.h> +#include <linux/iopoll.h> +#include <linux/pm_domain.h> +#include <linux/pm_runtime.h> +#include <linux/regmap.h> +#include <linux/mfd/syscon.h> +#include <linux/of_address.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/remoteproc.h> +#include <linux/soc/qcom/mdt_loader.h> +#include <linux/soc/qcom/smem.h> +#include <linux/soc/qcom/smem_state.h> + +#include "qcom_common.h" +#include "qcom_q6v5.h" +#include "remoteproc_internal.h" + +/* time out value */ +#define ACK_TIMEOUT 1000 +#define BOOT_FSM_TIMEOUT 10000 +/* mask values */ +#define EVB_MASK GENMASK(27, 4) +/*QDSP6SS register offsets*/ +#define RST_EVB_REG 0x10 +#define CORE_START_REG 0x400 +#define BOOT_CMD_REG 0x404 +#define BOOT_STATUS_REG 0x408 +#define RET_CFG_REG 0x1C +/*TCSR register offsets*/ +#define LPASS_MASTER_IDLE_REG 0x8 +#define LPASS_HALTACK_REG 0x4 +#define LPASS_PWR_ON_REG 0x10 +#define LPASS_HALTREQ_REG 0x0 + +/* list of clocks required by ADSP PIL */ +static const char * const adsp_clk_id[] = { + "sway_cbcr", "lpass_aon", "lpass_ahbs_aon_cbcr", "lpass_ahbm_aon_cbcr", + "qdsp6ss_xo", "qdsp6ss_sleep", "qdsp6ss_core", +}; + +struct adsp_pil_data { + int crash_reason_smem; + const char *firmware_name; + + const char *ssr_name; + const char *sysmon_name; + int ssctl_id; +}; + +struct qcom_adsp { + struct device *dev; + struct rproc *rproc; + + struct qcom_q6v5 q6v5; + + struct clk *xo; + + int num_clks; + struct clk_bulk_data *clks; + + void __iomem *qdsp6ss_base; + + struct reset_control *pdc_sync_reset; + struct reset_control *cc_lpass_restart; + + struct regmap *halt_map; + unsigned int halt_lpass; + + int crash_reason_smem; + + struct completion start_done; + struct completion stop_done; + + phys_addr_t mem_phys; + phys_addr_t mem_reloc; + void *mem_region; + size_t mem_size; + + struct qcom_rproc_glink glink_subdev; + struct qcom_rproc_ssr ssr_subdev; + struct qcom_sysmon *sysmon; +}; + +static int qcom_adsp_shutdown(struct qcom_adsp *adsp) +{ + unsigned long timeout; + unsigned int val; + int ret; + + /* Reset the retention logic */ + val = readl(adsp->qdsp6ss_base + RET_CFG_REG); + val |= 0x1; + writel(val, adsp->qdsp6ss_base + RET_CFG_REG); + + clk_bulk_disable_unprepare(adsp->num_clks, adsp->clks); + + /* QDSP6 master port needs to be explicitly halted */ + ret = regmap_read(adsp->halt_map, + adsp->halt_lpass + LPASS_PWR_ON_REG, &val); + if (ret || !val) + goto reset; + + ret = regmap_read(adsp->halt_map, + adsp->halt_lpass + LPASS_MASTER_IDLE_REG, + &val); + if (ret || val) + goto reset; + + regmap_write(adsp->halt_map, + adsp->halt_lpass + LPASS_HALTREQ_REG, 1); + + /* Wait for halt ACK from QDSP6 */ + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); + for (;;) { + ret = regmap_read(adsp->halt_map, + adsp->halt_lpass + LPASS_HALTACK_REG, &val); + if (ret || val || time_after(jiffies, timeout)) + break; + + udelay(1000); + } + + ret = regmap_read(adsp->halt_map, + adsp->halt_lpass + LPASS_MASTER_IDLE_REG, &val); + if (ret || !val) + dev_err(adsp->dev, "port failed halt\n"); + +reset: + /* Assert the LPASS PDC Reset */ + reset_control_assert(adsp->pdc_sync_reset); + /* Place the LPASS processor into reset */ + reset_control_assert(adsp->cc_lpass_restart); + /* wait after asserting subsystem restart from AOSS */ + udelay(200); + + /* Clear the halt request for the AXIM and AHBM for Q6 */ + regmap_write(adsp->halt_map, adsp->halt_lpass + LPASS_HALTREQ_REG, 0); + + /* De-assert the LPASS PDC Reset */ + reset_control_deassert(adsp->pdc_sync_reset); + /* Remove the LPASS reset */ + reset_control_deassert(adsp->cc_lpass_restart); + /* wait after de-asserting subsystem restart from AOSS */ + udelay(200); + + return 0; +} + +static int adsp_load(struct rproc *rproc, const struct firmware *fw) +{ + struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; + + return qcom_mdt_load_no_init(adsp->dev, fw, rproc->firmware, 0, + adsp->mem_region, adsp->mem_phys, adsp->mem_size, + &adsp->mem_reloc); +} + +static int adsp_start(struct rproc *rproc) +{ + struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; + int ret; + unsigned int val; + + qcom_q6v5_prepare(&adsp->q6v5); + + ret = clk_prepare_enable(adsp->xo); + if (ret) + return ret; + + dev_pm_genpd_set_performance_state(adsp->dev, INT_MAX); + ret = pm_runtime_get_sync(adsp->dev); + if (ret) + goto disable_xo_clk; + + ret = clk_bulk_prepare_enable(adsp->num_clks, adsp->clks); + if (ret) { + dev_err(adsp->dev, "adsp clk_enable failed\n"); + goto disable_power_domain; + } + + /* Program boot address */ + writel(adsp->mem_phys >> 4, adsp->qdsp6ss_base + RST_EVB_REG); + + /* De-assert QDSP6 stop core. QDSP6 will execute after out of reset */ + writel(0x1, adsp->qdsp6ss_base + CORE_START_REG); + + /* Trigger boot FSM to start QDSP6 */ + writel(0x1, adsp->qdsp6ss_base + BOOT_CMD_REG); + + /* Wait for core to come out of reset */ + ret = readl_poll_timeout(adsp->qdsp6ss_base + BOOT_STATUS_REG, + val, (val & BIT(0)) != 0, 10, BOOT_FSM_TIMEOUT); + if (ret) { + dev_err(adsp->dev, "failed to bootup adsp\n"); + goto disable_adsp_clks; + } + + ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5 * HZ)); + if (ret == -ETIMEDOUT) { + dev_err(adsp->dev, "start timed out\n"); + goto disable_adsp_clks; + } + + return 0; + +disable_adsp_clks: + clk_bulk_disable_unprepare(adsp->num_clks, adsp->clks); +disable_power_domain: + dev_pm_genpd_set_performance_state(adsp->dev, 0); + pm_runtime_put(adsp->dev); +disable_xo_clk: + clk_disable_unprepare(adsp->xo); + + return ret; +} + +static void qcom_adsp_pil_handover(struct qcom_q6v5 *q6v5) +{ + struct qcom_adsp *adsp = container_of(q6v5, struct qcom_adsp, q6v5); + + clk_disable_unprepare(adsp->xo); + dev_pm_genpd_set_performance_state(adsp->dev, 0); + pm_runtime_put(adsp->dev); +} + +static int adsp_stop(struct rproc *rproc) +{ + struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; + int handover; + int ret; + + ret = qcom_q6v5_request_stop(&adsp->q6v5); + if (ret == -ETIMEDOUT) + dev_err(adsp->dev, "timed out on wait\n"); + + ret = qcom_adsp_shutdown(adsp); + if (ret) + dev_err(adsp->dev, "failed to shutdown: %d\n", ret); + + handover = qcom_q6v5_unprepare(&adsp->q6v5); + if (handover) + qcom_adsp_pil_handover(&adsp->q6v5); + + return ret; +} + +static void *adsp_da_to_va(struct rproc *rproc, u64 da, int len) +{ + struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; + int offset; + + offset = da - adsp->mem_reloc; + if (offset < 0 || offset + len > adsp->mem_size) + return NULL; + + return adsp->mem_region + offset; +} + +static const struct rproc_ops adsp_ops = { + .start = adsp_start, + .stop = adsp_stop, + .da_to_va = adsp_da_to_va, + .parse_fw = qcom_register_dump_segments, + .load = adsp_load, +}; + +static int adsp_init_clock(struct qcom_adsp *adsp) +{ + int i, ret; + + adsp->xo = devm_clk_get(adsp->dev, "xo"); + if (IS_ERR(adsp->xo)) { + ret = PTR_ERR(adsp->xo); + if (ret != -EPROBE_DEFER) + dev_err(adsp->dev, "failed to get xo clock"); + return ret; + } + + adsp->num_clks = ARRAY_SIZE(adsp_clk_id); + adsp->clks = devm_kcalloc(adsp->dev, adsp->num_clks, + sizeof(*adsp->clks), GFP_KERNEL); + if (IS_ERR(adsp->clks)) { + ret = PTR_ERR(adsp->clks); + if (ret != -EPROBE_DEFER) + dev_err(adsp->dev, "failed to get adsp clock"); + return ret; + } + + for (i = 0; i < adsp->num_clks; i++) + adsp->clks[i].id = adsp_clk_id[i]; + + return devm_clk_bulk_get(adsp->dev, adsp->num_clks, adsp->clks); +} + +static int adsp_init_reset(struct qcom_adsp *adsp) +{ + adsp->pdc_sync_reset = devm_reset_control_get_exclusive(adsp->dev, + "pdc_sync"); + if (IS_ERR(adsp->pdc_sync_reset)) { + dev_err(adsp->dev, "failed to acquire pdc_sync reset\n"); + return PTR_ERR(adsp->pdc_sync_reset); + } + + adsp->cc_lpass_restart = devm_reset_control_get_exclusive(adsp->dev, + "cc_lpass"); + if (IS_ERR(adsp->cc_lpass_restart)) { + dev_err(adsp->dev, "failed to acquire cc_lpass restart\n"); + return PTR_ERR(adsp->cc_lpass_restart); + } + + return 0; +} + +static int adsp_init_mmio(struct qcom_adsp *adsp, + struct platform_device *pdev) +{ + struct device_node *syscon; + struct resource *res; + int ret; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + adsp->qdsp6ss_base = devm_ioremap(&pdev->dev, res->start, + resource_size(res)); + if (IS_ERR(adsp->qdsp6ss_base)) { + dev_err(adsp->dev, "failed to map QDSP6SS registers\n"); + return PTR_ERR(adsp->qdsp6ss_base); + } + + syscon = of_parse_phandle(pdev->dev.of_node, "qcom,halt-regs", 0); + if (!syscon) { + dev_err(&pdev->dev, "failed to parse qcom,halt-regs\n"); + return -EINVAL; + } + + adsp->halt_map = syscon_node_to_regmap(syscon); + of_node_put(syscon); + if (IS_ERR(adsp->halt_map)) + return PTR_ERR(adsp->halt_map); + + ret = of_property_read_u32_index(pdev->dev.of_node, "qcom,halt-regs", + 1, &adsp->halt_lpass); + if (ret < 0) { + dev_err(&pdev->dev, "no offset in syscon\n"); + return ret; + } + + return 0; +} + +static int adsp_alloc_memory_region(struct qcom_adsp *adsp) +{ + struct device_node *node; + struct resource r; + int ret; + + node = of_parse_phandle(adsp->dev->of_node, "memory-region", 0); + if (!node) { + dev_err(adsp->dev, "no memory-region specified\n"); + return -EINVAL; + } + + ret = of_address_to_resource(node, 0, &r); + if (ret) + return ret; + + adsp->mem_phys = adsp->mem_reloc = r.start; + adsp->mem_size = resource_size(&r); + adsp->mem_region = devm_ioremap_wc(adsp->dev, + adsp->mem_phys, adsp->mem_size); + if (!adsp->mem_region) { + dev_err(adsp->dev, "unable to map memory region: %pa+%zx\n", + &r.start, adsp->mem_size); + return -EBUSY; + } + + return 0; +} + +static int adsp_probe(struct platform_device *pdev) +{ + const struct adsp_pil_data *desc; + struct qcom_adsp *adsp; + struct rproc *rproc; + int ret; + + desc = of_device_get_match_data(&pdev->dev); + if (!desc) + return -EINVAL; + + rproc = rproc_alloc(&pdev->dev, pdev->name, &adsp_ops, + desc->firmware_name, sizeof(*adsp)); + if (!rproc) { + dev_err(&pdev->dev, "unable to allocate remoteproc\n"); + return -ENOMEM; + } + + adsp = (struct qcom_adsp *)rproc->priv; + adsp->dev = &pdev->dev; + adsp->rproc = rproc; + platform_set_drvdata(pdev, adsp); + + ret = adsp_alloc_memory_region(adsp); + if (ret) + goto free_rproc; + + ret = adsp_init_clock(adsp); + if (ret) + goto free_rproc; + + pm_runtime_enable(adsp->dev); + + ret = adsp_init_reset(adsp); + if (ret) + goto disable_pm; + + ret = adsp_init_mmio(adsp, pdev); + if (ret) + goto disable_pm; + + ret = qcom_q6v5_init(&adsp->q6v5, pdev, rproc, desc->crash_reason_smem, + qcom_adsp_pil_handover); + if (ret) + goto disable_pm; + + qcom_add_glink_subdev(rproc, &adsp->glink_subdev); + qcom_add_ssr_subdev(rproc, &adsp->ssr_subdev, desc->ssr_name); + adsp->sysmon = qcom_add_sysmon_subdev(rproc, + desc->sysmon_name, + desc->ssctl_id); + + ret = rproc_add(rproc); + if (ret) + goto disable_pm; + + return 0; + +disable_pm: + pm_runtime_disable(adsp->dev); +free_rproc: + rproc_free(rproc); + + return ret; +} + +static int adsp_remove(struct platform_device *pdev) +{ + struct qcom_adsp *adsp = platform_get_drvdata(pdev); + + rproc_del(adsp->rproc); + + qcom_remove_glink_subdev(adsp->rproc, &adsp->glink_subdev); + qcom_remove_sysmon_subdev(adsp->sysmon); + qcom_remove_ssr_subdev(adsp->rproc, &adsp->ssr_subdev); + rproc_free(adsp->rproc); + pm_runtime_disable(adsp->dev); + + return 0; +} + +static const struct adsp_pil_data adsp_resource_init = { + .crash_reason_smem = 423, + .firmware_name = "adsp.mdt", + .ssr_name = "lpass", + .sysmon_name = "adsp", + .ssctl_id = 0x14, +}; + +static const struct of_device_id adsp_of_match[] = { + { .compatible = "qcom,sdm845-adsp-pil", + .data = &adsp_resource_init}, + { }, +}; +MODULE_DEVICE_TABLE(of, adsp_of_match); + +static struct platform_driver adsp_pil_driver = { + .probe = adsp_probe, + .remove = adsp_remove, + .driver = { + .name = "qcom_adsp_pil", + .of_match_table = adsp_of_match, + }, +}; + +module_platform_driver(adsp_pil_driver); +MODULE_DESCRIPTION("QTi SDM845 ADSP Peripherial Image Loader"); +MODULE_LICENSE("GPL v2"); -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver 2018-09-03 11:52 ` [PATCH v3 2/2] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver Rohit kumar @ 2018-09-10 18:31 ` Bjorn Andersson 2018-09-11 3:49 ` Rohit Kumar 2018-09-21 19:41 ` Sibi Sankar 1 sibling, 1 reply; 13+ messages in thread From: Bjorn Andersson @ 2018-09-10 18:31 UTC (permalink / raw) To: Rohit kumar Cc: ohad, robh+dt, mark.rutland, linux-remoteproc, devicetree, linux-kernel, plai, bgoswami, srinivas.kandagatla On Mon 03 Sep 04:52 PDT 2018, Rohit kumar wrote: > This adds Non PAS ADSP PIL driver for Qualcomm > Technologies Inc SoCs. > Added initial support for SDM845 with ADSP bootup and > shutdown operation handled from Application Processor > SubSystem(APSS). > > Signed-off-by: Rohit kumar <rohitkr@codeaurora.org> Thanks for the changes Rohit, this looks good. Once we hear from DT maintainers that patch 1 can be applied I will update the name of the file and driver as I apply it to match the naming scheme I'm aiming for - no need for you to resend because of this. > --- > drivers/remoteproc/Kconfig | 14 ++ > drivers/remoteproc/Makefile | 1 + > drivers/remoteproc/qcom_adsp_pil.c | 500 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 515 insertions(+) > create mode 100644 drivers/remoteproc/qcom_adsp_pil.c > > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig > index c98c0b2..445de2d 100644 > --- a/drivers/remoteproc/Kconfig > +++ b/drivers/remoteproc/Kconfig > @@ -139,6 +139,20 @@ config QCOM_Q6V5_WCSS > Say y here to support the Qualcomm Peripheral Image Loader for the > Hexagon V5 based WCSS remote processors. > > +config QCOM_ADSP_PIL I will make this QCOM_Q6V5_ADSP [..] > diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c Make this qcom_q6v5_adsp.c [..] > +static struct platform_driver adsp_pil_driver = { > + .probe = adsp_probe, > + .remove = adsp_remove, > + .driver = { > + .name = "qcom_adsp_pil", and this qcom_q6v5_adsp". > + .of_match_table = adsp_of_match, > + }, > +}; Please let me know if you have any objections to this. Regards, Bjorn ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver 2018-09-10 18:31 ` Bjorn Andersson @ 2018-09-11 3:49 ` Rohit Kumar 0 siblings, 0 replies; 13+ messages in thread From: Rohit Kumar @ 2018-09-11 3:49 UTC (permalink / raw) To: Bjorn Andersson Cc: ohad, robh+dt, mark.rutland, linux-remoteproc, devicetree, linux-kernel, plai, bgoswami, srinivas.kandagatla Thanks Bjorn for reviewing. On 9/11/2018 12:01 AM, Bjorn Andersson wrote: > On Mon 03 Sep 04:52 PDT 2018, Rohit kumar wrote: > >> This adds Non PAS ADSP PIL driver for Qualcomm >> Technologies Inc SoCs. >> Added initial support for SDM845 with ADSP bootup and >> shutdown operation handled from Application Processor >> SubSystem(APSS). >> >> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org> > Thanks for the changes Rohit, this looks good. > > Once we hear from DT maintainers that patch 1 can be applied I will > update the name of the file and driver as I apply it to match the naming > scheme I'm aiming for - no need for you to resend because of this. Sure, I will just update dt-bindings with addressing some comments given by Rob. >> --- >> drivers/remoteproc/Kconfig | 14 ++ >> drivers/remoteproc/Makefile | 1 + >> drivers/remoteproc/qcom_adsp_pil.c | 500 +++++++++++++++++++++++++++++++++++++ >> 3 files changed, 515 insertions(+) >> create mode 100644 drivers/remoteproc/qcom_adsp_pil.c >> >> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig >> index c98c0b2..445de2d 100644 >> --- a/drivers/remoteproc/Kconfig >> +++ b/drivers/remoteproc/Kconfig >> @@ -139,6 +139,20 @@ config QCOM_Q6V5_WCSS >> Say y here to support the Qualcomm Peripheral Image Loader for the >> Hexagon V5 based WCSS remote processors. >> >> +config QCOM_ADSP_PIL > I will make this QCOM_Q6V5_ADSP > > [..] >> diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c > Make this qcom_q6v5_adsp.c > > [..] >> +static struct platform_driver adsp_pil_driver = { >> + .probe = adsp_probe, >> + .remove = adsp_remove, >> + .driver = { >> + .name = "qcom_adsp_pil", > and this qcom_q6v5_adsp". > >> + .of_match_table = adsp_of_match, >> + }, >> +}; > Please let me know if you have any objections to this. Naming looks fine. Thanks, Rohit > Regards, > Bjorn ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver 2018-09-03 11:52 ` [PATCH v3 2/2] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver Rohit kumar 2018-09-10 18:31 ` Bjorn Andersson @ 2018-09-21 19:41 ` Sibi Sankar 2018-09-24 6:49 ` Rohit Kumar 1 sibling, 1 reply; 13+ messages in thread From: Sibi Sankar @ 2018-09-21 19:41 UTC (permalink / raw) To: Rohit kumar Cc: ohad, bjorn.andersson, robh+dt, mark.rutland, linux-remoteproc, devicetree, linux-kernel, plai, bgoswami, srinivas.kandagatla, linux-kernel-owner Hi Rohit, On 2018-09-03 17:22, Rohit kumar wrote: > This adds Non PAS ADSP PIL driver for Qualcomm > Technologies Inc SoCs. > Added initial support for SDM845 with ADSP bootup and > shutdown operation handled from Application Processor > SubSystem(APSS). > > Signed-off-by: Rohit kumar <rohitkr@codeaurora.org> > --- .... > + select QCOM_MDT_LOADER > + select QCOM_Q6V5_COMMON > + select QCOM_RPROC_COMMON > + help > + Say y here to support the Peripherial Image Loader replace Peripherial/Peripheral. .... > +#include <linux/clk.h> > +#include <linux/firmware.h> > +#include <linux/interrupt.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/delay.h> > +#include <linux/io.h> > +#include <linux/reset.h> > +#include <linux/iopoll.h> > +#include <linux/pm_domain.h> > +#include <linux/pm_runtime.h> > +#include <linux/regmap.h> > +#include <linux/mfd/syscon.h> > +#include <linux/of_address.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/remoteproc.h> > +#include <linux/soc/qcom/mdt_loader.h> > +#include <linux/soc/qcom/smem.h> > +#include <linux/soc/qcom/smem_state.h> > + The headers should be in alphabetical order. .... > + struct reset_control *pdc_sync_reset; > + struct reset_control *cc_lpass_restart; > + > + struct regmap *halt_map; > + unsigned int halt_lpass; remove the double spaces above. .... > + if (ret || val) > + goto reset; > + > + regmap_write(adsp->halt_map, > + adsp->halt_lpass + LPASS_HALTREQ_REG, 1); > + > + /* Wait for halt ACK from QDSP6 */ Minor nit, please remove the double spaces in the comment above. > + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); > + for (;;) { > + ret = regmap_read(adsp->halt_map, > + adsp->halt_lpass + LPASS_HALTACK_REG, &val); > + if (ret || val || time_after(jiffies, timeout)) > + break; > + > + udelay(1000); According to Documentation/timers/timers-howto.txt please use usleep_range() when the delay range is between(10us - 20ms). > + } > + > + ret = regmap_read(adsp->halt_map, > + adsp->halt_lpass + LPASS_MASTER_IDLE_REG, &val); .... > + /* wait after asserting subsystem restart from AOSS */ > + udelay(200); ditto > + > + /* Clear the halt request for the AXIM and AHBM for Q6 */ > + regmap_write(adsp->halt_map, adsp->halt_lpass + LPASS_HALTREQ_REG, > 0); > + > + /* De-assert the LPASS PDC Reset */ > + reset_control_deassert(adsp->pdc_sync_reset); > + /* Remove the LPASS reset */ > + reset_control_deassert(adsp->cc_lpass_restart); > + /* wait after de-asserting subsystem restart from AOSS */ > + udelay(200); ditto > + > + return 0; > +} .... > +static int adsp_start(struct rproc *rproc) > +{ > + struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; > + int ret; > + unsigned int val; > + > + qcom_q6v5_prepare(&adsp->q6v5); > + > + ret = clk_prepare_enable(adsp->xo); > + if (ret) > + return ret; please call qcom_q6v5_unprepare on clk_prepare_enable failure to disable_irqs > + > + dev_pm_genpd_set_performance_state(adsp->dev, INT_MAX); > + ret = pm_runtime_get_sync(adsp->dev); > + if (ret) > + goto disable_xo_clk; > + .... > +static int adsp_init_reset(struct qcom_adsp *adsp) > +{ > + adsp->pdc_sync_reset = devm_reset_control_get_exclusive(adsp->dev, > + "pdc_sync"); > + if (IS_ERR(adsp->pdc_sync_reset)) { > + dev_err(adsp->dev, "failed to acquire pdc_sync reset\n"); > + return PTR_ERR(adsp->pdc_sync_reset); > + } Bjorn, should we return EPROBE_DEFER here since PDC global reset controller can be a module? > + > + adsp->cc_lpass_restart = devm_reset_control_get_exclusive(adsp->dev, > + "cc_lpass"); > + if (IS_ERR(adsp->cc_lpass_restart)) { > + dev_err(adsp->dev, "failed to acquire cc_lpass restart\n"); > + return PTR_ERR(adsp->cc_lpass_restart); > + } > + > + return 0; .... > +static int adsp_remove(struct platform_device *pdev) > +{ > + struct qcom_adsp *adsp = platform_get_drvdata(pdev); > + > + rproc_del(adsp->rproc); > + > + qcom_remove_glink_subdev(adsp->rproc, &adsp->glink_subdev); > + qcom_remove_sysmon_subdev(adsp->sysmon); > + qcom_remove_ssr_subdev(adsp->rproc, &adsp->ssr_subdev); > + rproc_free(adsp->rproc); > + pm_runtime_disable(adsp->dev); > + rmmod of the driver resulted in the following kernel panic: having a pm_runtime_disable after rproc_free seems to be the cause of the kernel panic. Please call pm_runtime_disable before rproc_free. do_raw_spin_lock+0x28/0x118 __raw_spin_lock_irq+0x30/0x3c __pm_runtime_disable+0x28/0xf4 adsp_remove+0x4c/0x5c [qcom_adsp_pil] platform_drv_remove+0x28/0x50 device_release_driver_internal+0x124/0x1c8 driver_detach+0x44/0x80 bus_remove_driver+0x78/0x9c driver_unregister+0x34/0x54 platform_driver_unregister+0x1c/0x28 cleanup_module+0x14/0x6bc [qcom_adsp_pil] SyS_delete_module+0x1c4/0x214 > + return 0; > +} > + > +static const struct adsp_pil_data adsp_resource_init = { > + .crash_reason_smem = 423, > + .firmware_name = "adsp.mdt", > + .ssr_name = "lpass", > + .sysmon_name = "adsp", > + .ssctl_id = 0x14, > +}; .... > +module_platform_driver(adsp_pil_driver); > +MODULE_DESCRIPTION("QTi SDM845 ADSP Peripherial Image Loader"); replace QTi/QTI and Peripherial/Peripheral. .... Also I see the following warns on stopping the adsp remoteproc, couldn't root cause it though: device_del+0x84/0x29c platform_device_del+0x2c/0x88 platform_device_unregister+0x1c/0x30 of_platform_device_destroy+0x98/0xa8 device_for_each_child+0x54/0xa4 of_platform_depopulate+0x38/0x54 q6asm_remove+0x1c/0x2c apr_device_remove+0x38/0x70 device_release_driver_internal+0x124/0x1c8 device_release_driver+0x24/0x30 bus_remove_device+0xcc/0xe4 device_del+0x1f8/0x29c device_unregister+0x1c/0x30 apr_remove_device+0x1c/0x2c device_for_each_child+0x54/0xa4 apr_remove+0x28/0x34 rpmsg_dev_remove+0x48/0x70 device_release_driver_internal+0x124/0x1c8 device_release_driver+0x24/0x30 bus_remove_device+0xcc/0xe4 device_del+0x1f8/0x29c device_unregister+0x1c/0x30 qcom_glink_remove_device+0x1c/0x2c device_for_each_child+0x54/0xa4 qcom_glink_native_remove+0x54/0x15c qcom_glink_smem_unregister+0x1c/0x30 glink_subdev_stop+0x1c/0x2c [qcom_common] rproc_stop+0x40/0xc0 rproc_shutdown+0x6c/0xc0 rproc_del+0x28/0xa0 adsp_remove+0x20/0x5c [qcom_adsp_pil] platform_drv_remove+0x28/0x50 device_release_driver_internal+0x124/0x1c8 driver_detach+0x44/0x80 bus_remove_driver+0x78/0x9c driver_unregister+0x34/0x54 platform_driver_unregister+0x1c/0x28 cleanup_module+0x14/0x6bc [qcom_adsp_pil] SyS_delete_module+0x1c4/0x214 -- -- Sibi Sankar -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver 2018-09-21 19:41 ` Sibi Sankar @ 2018-09-24 6:49 ` Rohit Kumar 2018-09-24 7:08 ` Rohit Kumar 2018-09-24 12:04 ` Sibi Sankar 0 siblings, 2 replies; 13+ messages in thread From: Rohit Kumar @ 2018-09-24 6:49 UTC (permalink / raw) To: Sibi Sankar Cc: ohad, bjorn.andersson, robh+dt, mark.rutland, linux-remoteproc, devicetree, linux-kernel, plai, bgoswami, srinivas.kandagatla, linux-kernel-owner Thanks Sibi for reviewing. On 9/22/2018 1:11 AM, Sibi Sankar wrote: > Hi Rohit, > > On 2018-09-03 17:22, Rohit kumar wrote: >> This adds Non PAS ADSP PIL driver for Qualcomm >> Technologies Inc SoCs. >> Added initial support for SDM845 with ADSP bootup and >> shutdown operation handled from Application Processor >> SubSystem(APSS). >> >> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org> >> --- > .... >> + select QCOM_MDT_LOADER >> + select QCOM_Q6V5_COMMON >> + select QCOM_RPROC_COMMON >> + help >> + Say y here to support the Peripherial Image Loader > > replace Peripherial/Peripheral. sure > .... >> +#include <linux/clk.h> >> +#include <linux/firmware.h> >> +#include <linux/interrupt.h> >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/delay.h> >> +#include <linux/io.h> >> +#include <linux/reset.h> >> +#include <linux/iopoll.h> >> +#include <linux/pm_domain.h> >> +#include <linux/pm_runtime.h> >> +#include <linux/regmap.h> >> +#include <linux/mfd/syscon.h> >> +#include <linux/of_address.h> >> +#include <linux/of_device.h> >> +#include <linux/platform_device.h> >> +#include <linux/remoteproc.h> >> +#include <linux/soc/qcom/mdt_loader.h> >> +#include <linux/soc/qcom/smem.h> >> +#include <linux/soc/qcom/smem_state.h> >> + > > The headers should be in alphabetical order. > ok, will do > .... >> + struct reset_control *pdc_sync_reset; >> + struct reset_control *cc_lpass_restart; >> + >> + struct regmap *halt_map; >> + unsigned int halt_lpass; > > remove the double spaces above. ok > .... >> + if (ret || val) >> + goto reset; >> + >> + regmap_write(adsp->halt_map, >> + adsp->halt_lpass + LPASS_HALTREQ_REG, 1); >> + >> + /* Wait for halt ACK from QDSP6 */ > > Minor nit, please remove the double spaces in the comment above. ok > >> + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); >> + for (;;) { >> + ret = regmap_read(adsp->halt_map, >> + adsp->halt_lpass + LPASS_HALTACK_REG, &val); >> + if (ret || val || time_after(jiffies, timeout)) >> + break; >> + >> + udelay(1000); > > According to Documentation/timers/timers-howto.txt please use > usleep_range() > when the delay range is between(10us - 20ms). okay, will update in next spin. > >> + } >> + >> + ret = regmap_read(adsp->halt_map, >> + adsp->halt_lpass + LPASS_MASTER_IDLE_REG, &val); > .... >> + /* wait after asserting subsystem restart from AOSS */ >> + udelay(200); > > ditto ok > >> + >> + /* Clear the halt request for the AXIM and AHBM for Q6 */ >> + regmap_write(adsp->halt_map, adsp->halt_lpass + >> LPASS_HALTREQ_REG, 0); >> + >> + /* De-assert the LPASS PDC Reset */ >> + reset_control_deassert(adsp->pdc_sync_reset); >> + /* Remove the LPASS reset */ >> + reset_control_deassert(adsp->cc_lpass_restart); >> + /* wait after de-asserting subsystem restart from AOSS */ >> + udelay(200); > > ditto > >> + >> + return 0; >> +} > .... >> +static int adsp_start(struct rproc *rproc) >> +{ >> + struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; >> + int ret; >> + unsigned int val; >> + >> + qcom_q6v5_prepare(&adsp->q6v5); >> + >> + ret = clk_prepare_enable(adsp->xo); >> + if (ret) >> + return ret; > > please call qcom_q6v5_unprepare on clk_prepare_enable failure to > disable_irqs > sure, will do that >> + >> + dev_pm_genpd_set_performance_state(adsp->dev, INT_MAX); >> + ret = pm_runtime_get_sync(adsp->dev); >> + if (ret) >> + goto disable_xo_clk; >> + > .... >> +static int adsp_init_reset(struct qcom_adsp *adsp) >> +{ >> + adsp->pdc_sync_reset = devm_reset_control_get_exclusive(adsp->dev, >> + "pdc_sync"); >> + if (IS_ERR(adsp->pdc_sync_reset)) { >> + dev_err(adsp->dev, "failed to acquire pdc_sync reset\n"); >> + return PTR_ERR(adsp->pdc_sync_reset); >> + } > > Bjorn, should we return EPROBE_DEFER here since PDC global reset > controller can be a module? > devm_reset_control_get_exclusive itself returns EPROBE_DEFER until PDC reset driver is probed. return PTR_ERR(adsp->pdc_sync_reset) handles this case. >> + >> + adsp->cc_lpass_restart = >> devm_reset_control_get_exclusive(adsp->dev, >> + "cc_lpass"); >> + if (IS_ERR(adsp->cc_lpass_restart)) { >> + dev_err(adsp->dev, "failed to acquire cc_lpass restart\n"); >> + return PTR_ERR(adsp->cc_lpass_restart); >> + } >> + >> + return 0; > .... >> +static int adsp_remove(struct platform_device *pdev) >> +{ >> + struct qcom_adsp *adsp = platform_get_drvdata(pdev); >> + >> + rproc_del(adsp->rproc); >> + >> + qcom_remove_glink_subdev(adsp->rproc, &adsp->glink_subdev); >> + qcom_remove_sysmon_subdev(adsp->sysmon); >> + qcom_remove_ssr_subdev(adsp->rproc, &adsp->ssr_subdev); >> + rproc_free(adsp->rproc); >> + pm_runtime_disable(adsp->dev); >> + > > rmmod of the driver resulted in the following kernel panic: > having a pm_runtime_disable after rproc_free seems to be the cause of > the kernel panic. > Please call pm_runtime_disable before rproc_free. > Thanks for pointing out, will update. > do_raw_spin_lock+0x28/0x118 > __raw_spin_lock_irq+0x30/0x3c > __pm_runtime_disable+0x28/0xf4 > adsp_remove+0x4c/0x5c [qcom_adsp_pil] > platform_drv_remove+0x28/0x50 > device_release_driver_internal+0x124/0x1c8 > driver_detach+0x44/0x80 > bus_remove_driver+0x78/0x9c > driver_unregister+0x34/0x54 > platform_driver_unregister+0x1c/0x28 > cleanup_module+0x14/0x6bc [qcom_adsp_pil] > SyS_delete_module+0x1c4/0x214 > >> + return 0; >> +} >> + >> +static const struct adsp_pil_data adsp_resource_init = { >> + .crash_reason_smem = 423, >> + .firmware_name = "adsp.mdt", >> + .ssr_name = "lpass", >> + .sysmon_name = "adsp", >> + .ssctl_id = 0x14, >> +}; > .... >> +module_platform_driver(adsp_pil_driver); >> +MODULE_DESCRIPTION("QTi SDM845 ADSP Peripherial Image Loader"); > > replace QTi/QTI and Peripherial/Peripheral. > ok > .... > Also I see the following warns on stopping the adsp remoteproc, > couldn't root cause it though: It should be issue in Q6 drivers. I will check and update q6 drivers. Thanks for reporting. > device_del+0x84/0x29c > platform_device_del+0x2c/0x88 > platform_device_unregister+0x1c/0x30 > of_platform_device_destroy+0x98/0xa8 > device_for_each_child+0x54/0xa4 > of_platform_depopulate+0x38/0x54 > q6asm_remove+0x1c/0x2c > apr_device_remove+0x38/0x70 > device_release_driver_internal+0x124/0x1c8 > device_release_driver+0x24/0x30 > bus_remove_device+0xcc/0xe4 > device_del+0x1f8/0x29c > device_unregister+0x1c/0x30 > apr_remove_device+0x1c/0x2c > device_for_each_child+0x54/0xa4 > apr_remove+0x28/0x34 > rpmsg_dev_remove+0x48/0x70 > device_release_driver_internal+0x124/0x1c8 > device_release_driver+0x24/0x30 > bus_remove_device+0xcc/0xe4 > device_del+0x1f8/0x29c > device_unregister+0x1c/0x30 > qcom_glink_remove_device+0x1c/0x2c > device_for_each_child+0x54/0xa4 > qcom_glink_native_remove+0x54/0x15c > qcom_glink_smem_unregister+0x1c/0x30 > glink_subdev_stop+0x1c/0x2c [qcom_common] > rproc_stop+0x40/0xc0 > rproc_shutdown+0x6c/0xc0 > rproc_del+0x28/0xa0 > adsp_remove+0x20/0x5c [qcom_adsp_pil] > platform_drv_remove+0x28/0x50 > device_release_driver_internal+0x124/0x1c8 > driver_detach+0x44/0x80 > bus_remove_driver+0x78/0x9c > driver_unregister+0x34/0x54 > platform_driver_unregister+0x1c/0x28 > cleanup_module+0x14/0x6bc [qcom_adsp_pil] > SyS_delete_module+0x1c4/0x214 > Thanks, Rohit ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver 2018-09-24 6:49 ` Rohit Kumar @ 2018-09-24 7:08 ` Rohit Kumar 2018-09-24 12:02 ` Sibi Sankar 2018-09-24 12:04 ` Sibi Sankar 1 sibling, 1 reply; 13+ messages in thread From: Rohit Kumar @ 2018-09-24 7:08 UTC (permalink / raw) To: Sibi Sankar Cc: ohad, bjorn.andersson, robh+dt, mark.rutland, linux-remoteproc, devicetree, linux-kernel, plai, bgoswami, srinivas.kandagatla, linux-kernel-owner On 9/24/2018 12:19 PM, Rohit Kumar wrote: > Thanks Sibi for reviewing. > > > On 9/22/2018 1:11 AM, Sibi Sankar wrote: >> Hi Rohit, >> >> On 2018-09-03 17:22, Rohit kumar wrote: >>> This adds Non PAS ADSP PIL driver for Qualcomm >>> Technologies Inc SoCs. >>> Added initial support for SDM845 with ADSP bootup and >>> shutdown operation handled from Application Processor >>> SubSystem(APSS). >>> >>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org> >>> --- >> .... ... >> .... >> Also I see the following warns on stopping the adsp remoteproc, >> couldn't root cause it though: > > It should be issue in Q6 drivers. I will check and update q6 drivers. > Thanks for reporting. > Checked this warning. This is a core driver issue fixed with https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/base/core.c?h=next-20180924&id=2ec16150179888b81717d1d3ce84e634f4736af2 >> device_del+0x84/0x29c >> platform_device_del+0x2c/0x88 >> platform_device_unregister+0x1c/0x30 >> of_platform_device_destroy+0x98/0xa8 >> device_for_each_child+0x54/0xa4 >> of_platform_depopulate+0x38/0x54 >> q6asm_remove+0x1c/0x2c >> apr_device_remove+0x38/0x70 >> device_release_driver_internal+0x124/0x1c8 >> device_release_driver+0x24/0x30 >> bus_remove_device+0xcc/0xe4 >> device_del+0x1f8/0x29c >> device_unregister+0x1c/0x30 >> apr_remove_device+0x1c/0x2c >> device_for_each_child+0x54/0xa4 >> apr_remove+0x28/0x34 >> rpmsg_dev_remove+0x48/0x70 >> device_release_driver_internal+0x124/0x1c8 >> device_release_driver+0x24/0x30 >> bus_remove_device+0xcc/0xe4 >> device_del+0x1f8/0x29c >> device_unregister+0x1c/0x30 >> qcom_glink_remove_device+0x1c/0x2c >> device_for_each_child+0x54/0xa4 >> qcom_glink_native_remove+0x54/0x15c >> qcom_glink_smem_unregister+0x1c/0x30 >> glink_subdev_stop+0x1c/0x2c [qcom_common] >> rproc_stop+0x40/0xc0 >> rproc_shutdown+0x6c/0xc0 >> rproc_del+0x28/0xa0 >> adsp_remove+0x20/0x5c [qcom_adsp_pil] >> platform_drv_remove+0x28/0x50 >> device_release_driver_internal+0x124/0x1c8 >> driver_detach+0x44/0x80 >> bus_remove_driver+0x78/0x9c >> driver_unregister+0x34/0x54 >> platform_driver_unregister+0x1c/0x28 >> cleanup_module+0x14/0x6bc [qcom_adsp_pil] >> SyS_delete_module+0x1c4/0x214 >> > > Thanks, > Rohit Thanks ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver 2018-09-24 7:08 ` Rohit Kumar @ 2018-09-24 12:02 ` Sibi Sankar 0 siblings, 0 replies; 13+ messages in thread From: Sibi Sankar @ 2018-09-24 12:02 UTC (permalink / raw) To: Rohit Kumar Cc: ohad, bjorn.andersson, robh+dt, mark.rutland, linux-remoteproc, devicetree, linux-kernel, plai, bgoswami, srinivas.kandagatla, linux-kernel-owner On 2018-09-24 12:38, Rohit Kumar wrote: > On 9/24/2018 12:19 PM, Rohit Kumar wrote: >> Thanks Sibi for reviewing. >> >> >> On 9/22/2018 1:11 AM, Sibi Sankar wrote: >>> Hi Rohit, >>> >>> On 2018-09-03 17:22, Rohit kumar wrote: >>>> This adds Non PAS ADSP PIL driver for Qualcomm >>>> Technologies Inc SoCs. >>>> Added initial support for SDM845 with ADSP bootup and >>>> shutdown operation handled from Application Processor >>>> SubSystem(APSS). >>>> >>>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org> >>>> --- >>> .... > ... >>> .... >>> Also I see the following warns on stopping the adsp remoteproc, >>> couldn't root cause it though: >> >> It should be issue in Q6 drivers. I will check and update q6 drivers. >> Thanks for reporting. >> > > Checked this warning. This is a core driver issue fixed with > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/base/core.c?h=next-20180924&id=2ec16150179888b81717d1d3ce84e634f4736af2 > Thanks for pointing this out. >>> device_del+0x84/0x29c >>> platform_device_del+0x2c/0x88 >>> platform_device_unregister+0x1c/0x30 >>> of_platform_device_destroy+0x98/0xa8 >>> device_for_each_child+0x54/0xa4 >>> of_platform_depopulate+0x38/0x54 >>> q6asm_remove+0x1c/0x2c >>> apr_device_remove+0x38/0x70 >>> device_release_driver_internal+0x124/0x1c8 >>> device_release_driver+0x24/0x30 >>> bus_remove_device+0xcc/0xe4 >>> device_del+0x1f8/0x29c >>> device_unregister+0x1c/0x30 >>> apr_remove_device+0x1c/0x2c >>> device_for_each_child+0x54/0xa4 >>> apr_remove+0x28/0x34 >>> rpmsg_dev_remove+0x48/0x70 >>> device_release_driver_internal+0x124/0x1c8 >>> device_release_driver+0x24/0x30 >>> bus_remove_device+0xcc/0xe4 >>> device_del+0x1f8/0x29c >>> device_unregister+0x1c/0x30 >>> qcom_glink_remove_device+0x1c/0x2c >>> device_for_each_child+0x54/0xa4 >>> qcom_glink_native_remove+0x54/0x15c >>> qcom_glink_smem_unregister+0x1c/0x30 >>> glink_subdev_stop+0x1c/0x2c [qcom_common] >>> rproc_stop+0x40/0xc0 >>> rproc_shutdown+0x6c/0xc0 >>> rproc_del+0x28/0xa0 >>> adsp_remove+0x20/0x5c [qcom_adsp_pil] >>> platform_drv_remove+0x28/0x50 >>> device_release_driver_internal+0x124/0x1c8 >>> driver_detach+0x44/0x80 >>> bus_remove_driver+0x78/0x9c >>> driver_unregister+0x34/0x54 >>> platform_driver_unregister+0x1c/0x28 >>> cleanup_module+0x14/0x6bc [qcom_adsp_pil] >>> SyS_delete_module+0x1c4/0x214 >>> >> >> Thanks, >> Rohit > Thanks -- -- Sibi Sankar -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver 2018-09-24 6:49 ` Rohit Kumar 2018-09-24 7:08 ` Rohit Kumar @ 2018-09-24 12:04 ` Sibi Sankar 1 sibling, 0 replies; 13+ messages in thread From: Sibi Sankar @ 2018-09-24 12:04 UTC (permalink / raw) To: Rohit Kumar Cc: ohad, bjorn.andersson, robh+dt, mark.rutland, linux-remoteproc, devicetree, linux-kernel, plai, bgoswami, srinivas.kandagatla, linux-kernel-owner, linux-remoteproc-owner On 2018-09-24 12:19, Rohit Kumar wrote: > Thanks Sibi for reviewing. > > > On 9/22/2018 1:11 AM, Sibi Sankar wrote: >> Hi Rohit, >> >> On 2018-09-03 17:22, Rohit kumar wrote: >>> This adds Non PAS ADSP PIL driver for Qualcomm >>> Technologies Inc SoCs. >>> Added initial support for SDM845 with ADSP bootup and >>> shutdown operation handled from Application Processor >>> SubSystem(APSS). >>> >>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org> >>> --- >> .... >>> + select QCOM_MDT_LOADER >>> + select QCOM_Q6V5_COMMON >>> + select QCOM_RPROC_COMMON >>> + help >>> + Say y here to support the Peripherial Image Loader >> >> replace Peripherial/Peripheral. > > sure >> .... >>> +#include <linux/clk.h> >>> +#include <linux/firmware.h> >>> +#include <linux/interrupt.h> >>> +#include <linux/kernel.h> >>> +#include <linux/module.h> >>> +#include <linux/delay.h> >>> +#include <linux/io.h> >>> +#include <linux/reset.h> >>> +#include <linux/iopoll.h> >>> +#include <linux/pm_domain.h> >>> +#include <linux/pm_runtime.h> >>> +#include <linux/regmap.h> >>> +#include <linux/mfd/syscon.h> >>> +#include <linux/of_address.h> >>> +#include <linux/of_device.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/remoteproc.h> >>> +#include <linux/soc/qcom/mdt_loader.h> >>> +#include <linux/soc/qcom/smem.h> >>> +#include <linux/soc/qcom/smem_state.h> >>> + >> >> The headers should be in alphabetical order. >> > > ok, will do >> .... >>> + struct reset_control *pdc_sync_reset; >>> + struct reset_control *cc_lpass_restart; >>> + >>> + struct regmap *halt_map; >>> + unsigned int halt_lpass; >> >> remove the double spaces above. > > ok >> .... >>> + if (ret || val) >>> + goto reset; >>> + >>> + regmap_write(adsp->halt_map, >>> + adsp->halt_lpass + LPASS_HALTREQ_REG, 1); >>> + >>> + /* Wait for halt ACK from QDSP6 */ >> >> Minor nit, please remove the double spaces in the comment above. > > ok >> >>> + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT); >>> + for (;;) { >>> + ret = regmap_read(adsp->halt_map, >>> + adsp->halt_lpass + LPASS_HALTACK_REG, &val); >>> + if (ret || val || time_after(jiffies, timeout)) >>> + break; >>> + >>> + udelay(1000); >> >> According to Documentation/timers/timers-howto.txt please use >> usleep_range() >> when the delay range is between(10us - 20ms). > > okay, will update in next spin. >> >>> + } >>> + >>> + ret = regmap_read(adsp->halt_map, >>> + adsp->halt_lpass + LPASS_MASTER_IDLE_REG, &val); >> .... >>> + /* wait after asserting subsystem restart from AOSS */ >>> + udelay(200); >> >> ditto > > ok >> >>> + >>> + /* Clear the halt request for the AXIM and AHBM for Q6 */ >>> + regmap_write(adsp->halt_map, adsp->halt_lpass + >>> LPASS_HALTREQ_REG, 0); >>> + >>> + /* De-assert the LPASS PDC Reset */ >>> + reset_control_deassert(adsp->pdc_sync_reset); >>> + /* Remove the LPASS reset */ >>> + reset_control_deassert(adsp->cc_lpass_restart); >>> + /* wait after de-asserting subsystem restart from AOSS */ >>> + udelay(200); >> >> ditto >> >>> + >>> + return 0; >>> +} >> .... >>> +static int adsp_start(struct rproc *rproc) >>> +{ >>> + struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; >>> + int ret; >>> + unsigned int val; >>> + >>> + qcom_q6v5_prepare(&adsp->q6v5); >>> + >>> + ret = clk_prepare_enable(adsp->xo); >>> + if (ret) >>> + return ret; >> >> please call qcom_q6v5_unprepare on clk_prepare_enable failure to >> disable_irqs >> > > sure, will do that >>> + >>> + dev_pm_genpd_set_performance_state(adsp->dev, INT_MAX); >>> + ret = pm_runtime_get_sync(adsp->dev); >>> + if (ret) >>> + goto disable_xo_clk; >>> + >> .... >>> +static int adsp_init_reset(struct qcom_adsp *adsp) >>> +{ >>> + adsp->pdc_sync_reset = >>> devm_reset_control_get_exclusive(adsp->dev, >>> + "pdc_sync"); >>> + if (IS_ERR(adsp->pdc_sync_reset)) { >>> + dev_err(adsp->dev, "failed to acquire pdc_sync reset\n"); >>> + return PTR_ERR(adsp->pdc_sync_reset); >>> + } >> >> Bjorn, should we return EPROBE_DEFER here since PDC global reset >> controller can be a module? >> > > devm_reset_control_get_exclusive itself returns EPROBE_DEFER until PDC > reset driver is probed. > return PTR_ERR(adsp->pdc_sync_reset) handles this case. > Thanks for pointing this out, missed this. >>> + >>> + adsp->cc_lpass_restart = >>> devm_reset_control_get_exclusive(adsp->dev, >>> + "cc_lpass"); >>> + if (IS_ERR(adsp->cc_lpass_restart)) { >>> + dev_err(adsp->dev, "failed to acquire cc_lpass restart\n"); >>> + return PTR_ERR(adsp->cc_lpass_restart); >>> + } >>> + >>> + return 0; >> .... >>> +static int adsp_remove(struct platform_device *pdev) >>> +{ >>> + struct qcom_adsp *adsp = platform_get_drvdata(pdev); >>> + >>> + rproc_del(adsp->rproc); >>> + >>> + qcom_remove_glink_subdev(adsp->rproc, &adsp->glink_subdev); >>> + qcom_remove_sysmon_subdev(adsp->sysmon); >>> + qcom_remove_ssr_subdev(adsp->rproc, &adsp->ssr_subdev); >>> + rproc_free(adsp->rproc); >>> + pm_runtime_disable(adsp->dev); >>> + >> >> rmmod of the driver resulted in the following kernel panic: >> having a pm_runtime_disable after rproc_free seems to be the cause of >> the kernel panic. >> Please call pm_runtime_disable before rproc_free. >> > > Thanks for pointing out, will update. >> do_raw_spin_lock+0x28/0x118 >> __raw_spin_lock_irq+0x30/0x3c >> __pm_runtime_disable+0x28/0xf4 >> adsp_remove+0x4c/0x5c [qcom_adsp_pil] >> platform_drv_remove+0x28/0x50 >> device_release_driver_internal+0x124/0x1c8 >> driver_detach+0x44/0x80 >> bus_remove_driver+0x78/0x9c >> driver_unregister+0x34/0x54 >> platform_driver_unregister+0x1c/0x28 >> cleanup_module+0x14/0x6bc [qcom_adsp_pil] >> SyS_delete_module+0x1c4/0x214 >> >>> + return 0; >>> +} >>> + >>> +static const struct adsp_pil_data adsp_resource_init = { >>> + .crash_reason_smem = 423, >>> + .firmware_name = "adsp.mdt", >>> + .ssr_name = "lpass", >>> + .sysmon_name = "adsp", >>> + .ssctl_id = 0x14, >>> +}; >> .... >>> +module_platform_driver(adsp_pil_driver); >>> +MODULE_DESCRIPTION("QTi SDM845 ADSP Peripherial Image Loader"); >> >> replace QTi/QTI and Peripherial/Peripheral. >> > ok >> .... >> Also I see the following warns on stopping the adsp remoteproc, >> couldn't root cause it though: > > It should be issue in Q6 drivers. I will check and update q6 drivers. > Thanks for reporting. > >> device_del+0x84/0x29c >> platform_device_del+0x2c/0x88 >> platform_device_unregister+0x1c/0x30 >> of_platform_device_destroy+0x98/0xa8 >> device_for_each_child+0x54/0xa4 >> of_platform_depopulate+0x38/0x54 >> q6asm_remove+0x1c/0x2c >> apr_device_remove+0x38/0x70 >> device_release_driver_internal+0x124/0x1c8 >> device_release_driver+0x24/0x30 >> bus_remove_device+0xcc/0xe4 >> device_del+0x1f8/0x29c >> device_unregister+0x1c/0x30 >> apr_remove_device+0x1c/0x2c >> device_for_each_child+0x54/0xa4 >> apr_remove+0x28/0x34 >> rpmsg_dev_remove+0x48/0x70 >> device_release_driver_internal+0x124/0x1c8 >> device_release_driver+0x24/0x30 >> bus_remove_device+0xcc/0xe4 >> device_del+0x1f8/0x29c >> device_unregister+0x1c/0x30 >> qcom_glink_remove_device+0x1c/0x2c >> device_for_each_child+0x54/0xa4 >> qcom_glink_native_remove+0x54/0x15c >> qcom_glink_smem_unregister+0x1c/0x30 >> glink_subdev_stop+0x1c/0x2c [qcom_common] >> rproc_stop+0x40/0xc0 >> rproc_shutdown+0x6c/0xc0 >> rproc_del+0x28/0xa0 >> adsp_remove+0x20/0x5c [qcom_adsp_pil] >> platform_drv_remove+0x28/0x50 >> device_release_driver_internal+0x124/0x1c8 >> driver_detach+0x44/0x80 >> bus_remove_driver+0x78/0x9c >> driver_unregister+0x34/0x54 >> platform_driver_unregister+0x1c/0x28 >> cleanup_module+0x14/0x6bc [qcom_adsp_pil] >> SyS_delete_module+0x1c4/0x214 >> > > Thanks, > Rohit -- -- Sibi Sankar -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-09-24 12:04 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-03 11:52 [PATCH v3 0/2] Add ADSP PIL driver for SDM845 Rohit kumar 2018-09-03 11:52 ` [PATCH v3 1/2] dt-binding: remoteproc: Add QTI ADSP PIL bindings Rohit kumar 2018-09-10 18:27 ` Bjorn Andersson 2018-09-10 20:01 ` Rob Herring 2018-09-11 3:46 ` Rohit Kumar 2018-09-03 11:52 ` [PATCH v3 2/2] remoteproc: qcom: Introduce Non-PAS ADSP PIL driver Rohit kumar 2018-09-10 18:31 ` Bjorn Andersson 2018-09-11 3:49 ` Rohit Kumar 2018-09-21 19:41 ` Sibi Sankar 2018-09-24 6:49 ` Rohit Kumar 2018-09-24 7:08 ` Rohit Kumar 2018-09-24 12:02 ` Sibi Sankar 2018-09-24 12:04 ` Sibi Sankar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).