From: Sibi Sankar <sibis@codeaurora.org>
To: Matthias Kaehlcke <mka@chromium.org>
Cc: bjorn.andersson@linaro.org, robh+dt@kernel.org, will@kernel.org,
saiprakash.ranjan@codeaurora.org, ohad@wizery.com,
agross@kernel.org, mathieu.poirier@linaro.org,
robin.murphy@arm.com, joro@8bytes.org, p.zabel@pengutronix.de,
linux-arm-msm@vger.kernel.org, linux-remoteproc@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, evgreen@chromium.org,
dianders@chromium.org, swboyd@chromium.org
Subject: Re: [PATCH 5/9] remoteproc: mss: q6v5-mss: Add modem support on SC7280
Date: Fri, 25 Jun 2021 19:51:38 +0530 [thread overview]
Message-ID: <73f9814fb4f3aa2abeee0ece3aa26312@codeaurora.org> (raw)
In-Reply-To: <YNUkw5GDrHwTVcC5@google.com>
Hey Matthias,
Thanks for taking time to review the patch
series.
On 2021-06-25 06:05, Matthias Kaehlcke wrote:
> 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@codeaurora.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);
Sure I'll add more comments to this func.
After lowering the request ^^ we wait
for deny to go high or accept to go low.
If it's the former then we do a request
high and repeat the entire process again.
If it's the latter then its considered
that the takedown is success. Let me know
if you feel any other parts of the patch
requires more comments as well.
>> +
>> + 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");
>> +}
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
next prev parent reply other threads:[~2021-06-25 14:21 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-24 19:47 [PATCH 0/9] Add Modem support on SC7280 SoCs Sibi Sankar
2021-06-24 19:47 ` [PATCH 1/9] dt-bindings: remoteproc: qcom: pas: Add SC7280 MPSS support Sibi Sankar
2021-06-25 17:12 ` Matthias Kaehlcke
2021-06-25 17:12 ` Matthias Kaehlcke
2021-06-25 17:28 ` Sibi Sankar
2021-06-25 23:20 ` Matthias Kaehlcke
2021-06-25 23:20 ` Matthias Kaehlcke
2021-07-14 19:36 ` Rob Herring
2021-07-14 19:36 ` Rob Herring
2021-06-24 19:47 ` [PATCH 2/9] remoteproc: qcom: pas: Add SC7280 Modem support Sibi Sankar
2021-06-25 23:23 ` Matthias Kaehlcke
2021-06-25 23:23 ` Matthias Kaehlcke
2021-06-24 19:47 ` [PATCH 3/9] dt-bindings: remoteproc: qcom: Add Q6V5 Modem PIL binding Sibi Sankar
2021-06-25 23:43 ` Matthias Kaehlcke
2021-06-25 23:43 ` Matthias Kaehlcke
2021-06-30 19:57 ` Sibi Sankar
2021-07-14 19:37 ` Rob Herring
2021-07-14 19:37 ` Rob Herring
2021-06-24 19:47 ` [PATCH 4/9] iommu/arm-smmu-qcom: Request direct mapping for modem device Sibi Sankar
2021-06-25 4:27 ` Sai Prakash Ranjan
2021-06-24 19:47 ` [PATCH 5/9] remoteproc: mss: q6v5-mss: Add modem support on SC7280 Sibi Sankar
2021-06-25 0:35 ` Matthias Kaehlcke
2021-06-25 0:35 ` Matthias Kaehlcke
2021-06-25 14:21 ` Sibi Sankar [this message]
2021-06-25 16:39 ` Matthias Kaehlcke
2021-06-25 16:39 ` Matthias Kaehlcke
2021-06-24 19:47 ` [PATCH 6/9] arm64: dts: qcom: sc7280: Update reserved memory map Sibi Sankar
2021-06-28 18:11 ` Matthias Kaehlcke
2021-06-28 18:11 ` Matthias Kaehlcke
2021-06-30 20:02 ` Sibi Sankar
2021-07-30 18:24 ` Bjorn Andersson
2021-07-30 18:24 ` Bjorn Andersson
2021-06-24 19:47 ` [PATCH 7/9] arm64: dts: qcom: sc7280: Add nodes to boot modem Sibi Sankar
2021-07-30 18:00 ` Bjorn Andersson
2021-07-30 18:00 ` Bjorn Andersson
2021-06-24 19:47 ` [PATCH 8/9] arm64: dts: qcom: sc7280: Add Q6V5 MSS node Sibi Sankar
2021-06-28 18:39 ` Matthias Kaehlcke
2021-06-28 18:39 ` Matthias Kaehlcke
2021-06-30 20:03 ` Sibi Sankar
2021-07-30 18:01 ` Bjorn Andersson
2021-07-30 18:01 ` Bjorn Andersson
2021-06-24 19:47 ` [PATCH 9/9] arm64: dts: qcom: sc7280: Update " Sibi Sankar
2021-06-28 19:05 ` Matthias Kaehlcke
2021-06-28 19:05 ` Matthias Kaehlcke
2021-06-30 20:08 ` Sibi Sankar
2021-07-30 18:14 ` Bjorn Andersson
2021-07-30 18:14 ` Bjorn Andersson
2021-07-01 20:02 ` [PATCH 0/9] Add Modem support on SC7280 SoCs Pavel Machek
2021-07-01 20:02 ` Pavel Machek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=73f9814fb4f3aa2abeee0ece3aa26312@codeaurora.org \
--to=sibis@codeaurora.org \
--cc=agross@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=dianders@chromium.org \
--cc=evgreen@chromium.org \
--cc=joro@8bytes.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=mathieu.poirier@linaro.org \
--cc=mka@chromium.org \
--cc=ohad@wizery.com \
--cc=p.zabel@pengutronix.de \
--cc=robh+dt@kernel.org \
--cc=robin.murphy@arm.com \
--cc=saiprakash.ranjan@codeaurora.org \
--cc=swboyd@chromium.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.