[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YNUkw5GDrHwTVcC5@google.com>
Date: Thu, 24 Jun 2021 17:35:15 -0700
From: Matthias Kaehlcke <mka@...omium.org>
To: Sibi Sankar <sibis@...eaurora.org>
Cc: bjorn.andersson@...aro.org, robh+dt@...nel.org, will@...nel.org,
saiprakash.ranjan@...eaurora.org, ohad@...ery.com,
agross@...nel.org, mathieu.poirier@...aro.org,
robin.murphy@....com, joro@...tes.org, p.zabel@...gutronix.de,
linux-arm-msm@...r.kernel.org, linux-remoteproc@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, evgreen@...omium.org,
dianders@...omium.org, swboyd@...omium.org
Subject: Re: [PATCH 5/9] remoteproc: mss: q6v5-mss: Add modem support on
SC7280
Hi Sibi,
On Fri, Jun 25, 2021 at 01:17:34AM +0530, Sibi Sankar wrote:
> Add out of reset sequence support for modem sub-system on SC7280 SoCs.
> It requires access to an additional set of qaccept registers, external
> power/clk control registers and halt vq6 register to put the modem back
> into reset.
>
> Signed-off-by: Sibi Sankar <sibis@...eaurora.org>
> ---
> drivers/remoteproc/qcom_q6v5_mss.c | 245 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 241 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index 5d21084004cb..4e32811e0025 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -77,6 +77,14 @@
>
> #define HALT_ACK_TIMEOUT_US 100000
>
> +/* QACCEPT Register Offsets */
> +#define QACCEPT_ACCEPT_REG 0x0
> +#define QACCEPT_ACTIVE_REG 0x4
> +#define QACCEPT_DENY_REG 0x8
> +#define QACCEPT_REQ_REG 0xC
> +
> +#define QACCEPT_TIMEOUT_US 50
> +
> /* QDSP6SS_RESET */
> #define Q6SS_STOP_CORE BIT(0)
> #define Q6SS_CORE_ARES BIT(1)
> @@ -143,6 +151,9 @@ struct rproc_hexagon_res {
> bool has_alt_reset;
> bool has_mba_logs;
> bool has_spare_reg;
> + bool has_qaccept_regs;
> + bool has_ext_cntl_regs;
> + bool has_vq6;
> };
>
> struct q6v5 {
> @@ -158,8 +169,18 @@ struct q6v5 {
> u32 halt_q6;
> u32 halt_modem;
> u32 halt_nc;
> + u32 halt_vq6;
> u32 conn_box;
>
> + u32 qaccept_mdm;
> + u32 qaccept_cx;
> + u32 qaccept_axi;
> +
> + u32 axim1_clk_off;
> + u32 crypto_clk_off;
> + u32 force_clk_on;
> + u32 rscc_disable;
> +
> struct reset_control *mss_restart;
> struct reset_control *pdc_reset;
>
> @@ -201,6 +222,9 @@ struct q6v5 {
> bool has_alt_reset;
> bool has_mba_logs;
> bool has_spare_reg;
> + bool has_qaccept_regs;
> + bool has_ext_cntl_regs;
> + bool has_vq6;
> int mpss_perm;
> int mba_perm;
> const char *hexagon_mdt_image;
> @@ -213,6 +237,7 @@ enum {
> MSS_MSM8996,
> MSS_MSM8998,
> MSS_SC7180,
> + MSS_SC7280,
> MSS_SDM845,
> };
>
> @@ -473,6 +498,12 @@ static int q6v5_reset_assert(struct q6v5 *qproc)
> regmap_update_bits(qproc->conn_map, qproc->conn_box,
> AXI_GATING_VALID_OVERRIDE, 0);
> ret = reset_control_deassert(qproc->mss_restart);
> + } else if (qproc->has_ext_cntl_regs) {
> + regmap_write(qproc->conn_map, qproc->rscc_disable, 0);
> + reset_control_assert(qproc->pdc_reset);
> + reset_control_assert(qproc->mss_restart);
> + reset_control_deassert(qproc->pdc_reset);
> + ret = reset_control_deassert(qproc->mss_restart);
> } else {
> ret = reset_control_assert(qproc->mss_restart);
> }
> @@ -490,7 +521,7 @@ static int q6v5_reset_deassert(struct q6v5 *qproc)
> ret = reset_control_reset(qproc->mss_restart);
> writel(0, qproc->rmb_base + RMB_MBA_ALT_RESET);
> reset_control_deassert(qproc->pdc_reset);
> - } else if (qproc->has_spare_reg) {
> + } else if (qproc->has_spare_reg || qproc->has_ext_cntl_regs) {
> ret = reset_control_reset(qproc->mss_restart);
> } else {
> ret = reset_control_deassert(qproc->mss_restart);
> @@ -604,7 +635,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
> }
>
> goto pbl_wait;
> - } else if (qproc->version == MSS_SC7180) {
> + } else if (qproc->version == MSS_SC7180 || qproc->version == MSS_SC7280) {
> val = readl(qproc->reg_base + QDSP6SS_SLEEP);
> val |= Q6SS_CBCR_CLKEN;
> writel(val, qproc->reg_base + QDSP6SS_SLEEP);
> @@ -787,6 +818,82 @@ static int q6v5proc_reset(struct q6v5 *qproc)
> return ret;
> }
>
> +static int q6v5proc_enable_qchannel(struct q6v5 *qproc, struct regmap *map, u32 offset)
> +{
> + unsigned int val;
> + int ret;
> +
> + if (!qproc->has_qaccept_regs)
> + return 0;
> +
> + if (qproc->has_ext_cntl_regs) {
> + regmap_write(qproc->conn_map, qproc->rscc_disable, 0);
> + regmap_write(qproc->conn_map, qproc->force_clk_on, 1);
> +
> + ret = regmap_read_poll_timeout(qproc->halt_map, qproc->axim1_clk_off, val,
> + !val, 1, Q6SS_CBCR_TIMEOUT_US);
> + if (ret) {
> + dev_err(qproc->dev, "failed to enable axim1 clock\n");
> + return -ETIMEDOUT;
> + }
> + }
> +
> + regmap_write(map, offset + QACCEPT_REQ_REG, 1);
> +
> + /* Wait for accept */
> + ret = regmap_read_poll_timeout(map, offset + QACCEPT_ACCEPT_REG, val, val, 5,
> + QACCEPT_TIMEOUT_US);
> + if (ret) {
> + dev_err(qproc->dev, "qchannel enable failed\n");
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
> +}
> +
> +static void q6v5proc_disable_qchannel(struct q6v5 *qproc, struct regmap *map, u32 offset)
> +{
> + int ret;
> + unsigned int val, retry;
> + unsigned int nretry = 10;
> + bool takedown_complete = false;
> +
> + if (!qproc->has_qaccept_regs)
> + return;
> +
> + while (!takedown_complete && nretry) {
> + nretry--;
> +
> + regmap_read_poll_timeout(map, offset + QACCEPT_ACTIVE_REG, val, !val, 5,
> + QACCEPT_TIMEOUT_US);
> +
> + regmap_write(map, offset + QACCEPT_REQ_REG, 0);
> +
> + retry = 10;
> + while (retry) {
> + usleep_range(5, 10);
> + retry--;
> + ret = regmap_read(map, offset + QACCEPT_DENY_REG, &val);
> + if (!ret && val) {
> + regmap_write(map, offset + QACCEPT_REQ_REG, 1);
> + break;
> + }
> +
> + ret = regmap_read(map, offset + QACCEPT_ACCEPT_REG, &val);
> + if (!ret && !val) {
> + takedown_complete = true;
> + break;
> + }
A bit of commentary in this branch would do no harm. From the code flow
I can guess that disabling the channel failed when QACCEPT_DENY_REG != 0,
and hence the channel is re-enabled (?) for the next try, and apparently
things are fine when QACCEPT_ACCEPT_REG is 0 after disabling the channel.
Would be good to be a bit more explicit about what all that actually
means.
> + }
> +
> + if (!retry)
> + break;
> + }
> +
> + if (!takedown_complete)
> + dev_err(qproc->dev, "qchannel takedown failed\n");
> +}
Powered by blists - more mailing lists