Messages in this thread | | | Subject | Re: [PATCH V6 3/5] remoteproc: qcom: Split the head and tail of the q5v5-pil rproc reset function | From | Sricharan R <> | Date | Tue, 22 May 2018 14:20:14 +0530 |
| |
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
| |