[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <97221658-49a9-a61f-75ea-41559222bb38@codeaurora.org>
Date: Tue, 22 May 2018 14:20:14 +0530
From: Sricharan R <sricharan@...eaurora.org>
To: Vinod <vkoul@...nel.org>
Cc: bjorn.andersson@...aro.org, ohad@...ery.com, robh+dt@...nel.org,
mark.rutland@....com, andy.gross@...aro.org,
david.brown@...aro.org, linux-remoteproc@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org, linux-soc@...r.kernel.org,
sibis@...eaurora.org
Subject: Re: [PATCH V6 3/5] remoteproc: qcom: Split the head and tail of the
q5v5-pil rproc reset function
Hi Vinod,
Thanks for the review.
On 5/18/2018 5:52 PM, Vinod wrote:
> On 14-05-18, 16:16, Sricharan R wrote:
>
>> +static int q6v5_reset(struct q6v5 *qproc)
>> +{
>> + u32 ret;
>> + int val, i;
>> +
>> + /* Assert resets, stop core */
>> + val = readl(qproc->reg_base + QDSP6SS_RESET_REG);
>> + val |= Q6SS_CORE_ARES | Q6SS_BUS_ARES_ENABLE | Q6SS_STOP_CORE;
>> + writel(val, qproc->reg_base + QDSP6SS_RESET_REG);
>> +
>> + /* BHS require xo cbcr to be enabled */
>> + val = readl(qproc->reg_base + QDSP6SS_XO_CBCR);
>> + val |= 0x1;
>> + writel(val, qproc->reg_base + QDSP6SS_XO_CBCR);
>
> consider adding a updatel macro which does read, update and write for you...
>
ok.
>> +
>> + /* Read CLKOFF bit to go low indicating CLK is enabled */
>> + ret = readl_poll_timeout(qproc->reg_base + QDSP6SS_XO_CBCR,
>> + val, !(val & BIT(31)), 1,
>> + HALT_CHECK_MAX_LOOPS);
>> + if (ret) {
>> + dev_err(qproc->dev,
>> + "xo cbcr enabling timed out (rc:%d)\n", ret);
>> + return ret;
>> + }
>> + /* Enable power block headswitch and wait for it to stabilize */
>> + val = readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>> + val |= QDSP6v56_BHS_ON;
>> + writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>> + val |= readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>
> why is this read required
>
It was a ditto of what was given in the programming sequence from HW folks.
yeah, logically the readl does not look needed. Will remove and update.
>> + udelay(1);
>> +
>> + /* Put LDO in bypass mode */
>> + val |= QDSP6v56_LDO_BYP;
>> + writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>> +
>> + /* Deassert QDSP6 compiler memory clamp */
>> + val = readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>> + val &= ~QDSP6v56_CLAMP_QMC_MEM;
>> + writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>> +
>> + /* Deassert memory peripheral sleep and L2 memory standby */
>> + val |= Q6SS_L2DATA_STBY_N | Q6SS_SLP_RET_N;
>> + writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>> +
>> + /* Turn on L1, L2, ETB and JU memories 1 at a time */
>> + val = readl(qproc->reg_base + QDSP6SS_MEM_PWR_CTL);
>> + for (i = 19; i >= 0; i--) {
>
> where is the magic number 19 coming from?
>
Its the total number of Q6's memory head/foot switch banks. Infact
the magic was there even before my patch. But will add a Macro to fix it.
Regards,
Sricharan
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Powered by blists - more mailing lists