Thanks Bjorn for reviewing the patch.
On 8/28/2018 11:39 AM, Bjorn Andersson wrote:
On Fri 29 Jun 02:20 PDT 2018, Rohit kumar wrote:
This adds APSS based ADSP PIL driver for QCOM SoCs.
Added initial support for SDM845 with ADSP bootup and
shutdown operation handled from Application Processor
SubSystem(APSS).
Hi Rohit,
I've submitted a patch that renames the PAS based adsp driver, please
rebase your patch upon this.
Sure.
Signed-off-by: Rohit kumar <rohitkr@xxxxxxxxxxxxxx>
---
Changes since v1:
- Used APIs from qcom_q6v5.c
- Use clock, reset and regmap driver APIs instead of
directly writing into the LPASS registers.
- Created new file for non PAS ADSP PIL instead of extending
existing ADSP PIL driver.
- cleanups as suggested by Bjorn and Rob.
.../bindings/remoteproc/qcom,non-pas-adsp.txt | 138 +++++
drivers/remoteproc/Kconfig | 18 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/qcom_nonpas_adsp_pil.c | 667 +++++++++++++++++++++
4 files changed, 824 insertions(+)
create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,non-pas-adsp.txt
create mode 100644 drivers/remoteproc/qcom_nonpas_adsp_pil.c
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,non-pas-adsp.txt b/Documentation/devicetree/bindings/remoteproc/qcom,non-pas-adsp.txt
new file mode 100644
index 0000000..0581aaa
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,non-pas-adsp.txt
qcom,adsp-pil.txt
@@ -0,0 +1,138 @@
+Qualcomm Technology Inc. Non PAS 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-apss-adsp-pil"
"qcom,sdm845-adsp-pil"
ok
+
+- reg:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: must specify the base address and size of the qdsp6ss
+
+- reg-names:
+ Usage: required
+ Value type: <stringlist>
+ Definition: must be "qdsp6ss"
No need for reg-names when we have a single reg.
ok
+
+- interrupts-extended:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: must list the watchdog, fatal IRQs ready, handover and
+ stop-ack IRQs
+
[..]
+
+= 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-apss-adsp-pil";
+
+ reg = <0x17300000 0x40c>;
+ reg-names = "qdsp6ss";
+
+ 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 = <&clock_rpmh RPMH_CXO_CLK>,
+ <&lpasscc 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";
+
+ cx-supply = <&pm8998_s9_level>;
If this is a corner you should use the power-domain reference to the
appropriate rpmhpd resource.
ok, will update it in next patchset
+
+ 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";
+ };
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 0dde375..9de0a53 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -100,6 +100,24 @@ config QCOM_ADSP_PIL
Say y here to support the TrustZone based Peripherial Image Loader
for the Qualcomm ADSP remote processors.
+config QCOM_NON_PAS_ADSP_PIL
QCOM_ADSP_PIL
ok
[..]
diff --git a/drivers/remoteproc/qcom_nonpas_adsp_pil.c b/drivers/remoteproc/qcom_nonpas_adsp_pil.c
new file mode 100644
index 0000000..826d3d4
--- /dev/null
+++ b/drivers/remoteproc/qcom_nonpas_adsp_pil.c
Now qcom_adsp_pil.c
Yup
[..]
+struct qcom_adsp {
[..]
+
+ struct qcom_rproc_glink glink_subdev;
+ struct qcom_rproc_subdev smd_subdev;
We don't use smd on sdm845, so omit this until we have a user.
sure
+ struct qcom_rproc_ssr ssr_subdev;
+ struct qcom_sysmon *sysmon;
+};
+
+static int adsp_clk_enable(struct qcom_adsp *adsp)
+{
+ int ret;
+
+ /* Enable SWAY clock */
+ ret = clk_prepare_enable(adsp->gcc_sway_cbcr);
+ if (ret)
+ return ret;
+
+ /* Enable LPASS AHB AON Bus */
+ ret = clk_prepare_enable(adsp->lpass_audio_aon_clk);
+ if (ret)
+ return ret;
+
+ /* Enable the QDSP6SS AHBM and AHBS clocks */
+ ret = clk_prepare_enable(adsp->lpass_ahbs_aon_cbcr);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(adsp->lpass_ahbm_aon_cbcr);
+ if (ret)
+ return ret;
+
+ /* Turn on the XO clock, required to boot FSM */
+ ret = clk_prepare_enable(adsp->qdsp6ss_xo_cbcr);
+ if (ret)
+ return ret;
+
+ /* Enable the QDSP6SS sleep clock for the QDSP6 watchdog enablement */
+ ret = clk_prepare_enable(adsp->qdsp6ss_sleep_cbcr);
+ if (ret)
+ return ret;
+
+ /* Configure QDSP6 core CBC to enable clock */
+ ret = clk_prepare_enable(adsp->qdsp6ss_core_cbcr);
+ if (ret)
+ return ret;
Can we use the clk_bulk interface instead of using this expanded form?
Yup, will update in next spin.
+
+ return ret;
+}
+
+static int adsp_reset(struct qcom_adsp *adsp)
+{
+ unsigned int val;
+ int ret = 0;
No need to initialize ret.
ok
+
+ /* 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, "Boot FSM failed to complete.\n");
+
+ return ret;
+}
+
+static int qcom_adsp_start(struct qcom_adsp *adsp)
I don't see the reason why adsp_reset() is split out from this function,
or why qcom_adsp_start() is split out from adsp_start.
Will combine them in next spin
+{
+ int ret;
+
+ ret = adsp_clk_enable(adsp);
+ if (ret) {
+ dev_err(adsp->dev, "adsp clk_enable failed\n");
+ return ret;
+ }
+
+ /* Program boot address */
+ writel(adsp->mem_phys >> 4, adsp->qdsp6ss_base + RST_EVB_REG);
+
+ ret = adsp_reset(adsp);
+ if (ret)
+ dev_err(adsp->dev, "De-assert QDSP6 out of reset failed\n");
+
+ return ret;
+}
+
+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);
+
+ /* Disable QDSP6 core CBCR clock */
+ clk_disable_unprepare(adsp->qdsp6ss_core_cbcr);
+
+ /* Disable the QDSP6SS sleep clock */
+ clk_disable_unprepare(adsp->qdsp6ss_sleep_cbcr);
+
+ /* Turn off the XO clock */
+ clk_disable_unprepare(adsp->qdsp6ss_xo_cbcr);
+
+ /* Disable the QDSP6SS AHBM and AHBS clocks */
+ clk_disable_unprepare(adsp->lpass_ahbs_aon_cbcr);
+
+ clk_disable_unprepare(adsp->lpass_ahbm_aon_cbcr);
+
+ /* Disable LPASS AHB AON Bus */
+ clk_disable_unprepare(adsp->lpass_audio_aon_clk);
+
+ /* Disable the slave way clock to LPASS */
+ clk_disable_unprepare(adsp->gcc_sway_cbcr);
+
+ /* 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);
Is the core halted at this point?
Core is halted when we request for it by writing 1 to LPASS_HALTREQ_REG.
After halt, we need to cleanup for next start.
+
+ 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;
+
+ qcom_q6v5_prepare(&adsp->q6v5);
+
+ ret = clk_prepare_enable(adsp->xo);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(adsp->aggre2_clk);
+ if (ret)
+ goto disable_xo_clk;
Just skip aggre2_clk for now, unless there's going to be some other
subsystem added soon that uses it.
Ok
+
+ ret = regulator_enable(adsp->cx_supply);
+ if (ret)
+ goto disable_aggre2_clk;
+
+ ret = regulator_enable(adsp->px_supply);
+ if (ret)
+ goto disable_cx_supply;
Skip px_supply for now.
ok
+
+ ret = qcom_adsp_start(adsp);
+ if (ret) {
+ dev_err(adsp->dev,
+ "failed to bootup adsp\n");
+ goto disable_px_supply;
+ }
+
+ ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5000));
Use 5 * HZ
sure
+ if (ret == -ETIMEDOUT) {
+ dev_err(adsp->dev, "start timed out\n");
+ qcom_adsp_shutdown(adsp);
+ goto disable_px_supply;
+ }
+
+ return 0;
+
+disable_px_supply:
+ regulator_disable(adsp->px_supply);
+disable_cx_supply:
+ regulator_disable(adsp->cx_supply);
+disable_aggre2_clk:
+ clk_disable_unprepare(adsp->aggre2_clk);
+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);
+
+ regulator_disable(adsp->px_supply);
+ regulator_disable(adsp->cx_supply);
+ clk_disable_unprepare(adsp->aggre2_clk);
+ clk_disable_unprepare(adsp->xo);
Omit px_supply and aggre2_clk for now.
ok
+}
[..]
+static int adsp_init_clock(struct qcom_adsp *adsp)
+{
+ int 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;
+ }
+
+ if (adsp->has_aggre2_clk) {
Omit this for now.
ok
+ adsp->aggre2_clk = devm_clk_get(adsp->dev, "aggre2");
+ if (IS_ERR(adsp->aggre2_clk)) {
+ ret = PTR_ERR(adsp->aggre2_clk);
+ if (ret != -EPROBE_DEFER)
+ dev_err(adsp->dev,
+ "failed to get aggre2 clock");
+ return ret;
+ }
+ }
+
+ adsp->gcc_sway_cbcr = devm_clk_get(adsp->dev, "sway_cbcr");
Use the clk_bulk api to acquire the rest of these clocks.
okay
+ if (IS_ERR(adsp->gcc_sway_cbcr)) {
+ ret = PTR_ERR(adsp->xo);
+ if (ret != -EPROBE_DEFER)
+ dev_err(adsp->dev, "failed to get gcc_sway clock\n");
+ return PTR_ERR(adsp->gcc_sway_cbcr);
+ }
+
[..]
+
+static const struct non_pas_adsp_data adsp_resource_init = {
+ .crash_reason_smem = 423,
+ .firmware_name = "adsp.mdt",
+ .has_aggre2_clk = false,
+ .ssr_name = "lpass",
+ .sysmon_name = "adsp",
+ .ssctl_id = 0x14,
Please don't inherit the broken indentation.
Yup, will update in next spin.
+};
Regards,
Bjorn
Thanks,
Rohit
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
