On Thu 24 Nov 02:00 PST 2016, Avaneesh Kumar Dwivedi wrote:
> This change introduces appropriate additional steps in reset sequence so
> that hexagon v56 1.5.0 is brough out of reset.
>
You should use the non-_relaxed version throughout this patch, unless
you have good reason not to.
> Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@xxxxxxxxxxxxxx>
> ---
> drivers/remoteproc/qcom_q6v5_pil.c | 125 ++++++++++++++++++++++++++++++-------
> 1 file changed, 104 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
> index c55dc9a..7ea2f53 100644
> --- a/drivers/remoteproc/qcom_q6v5_pil.c
> +++ b/drivers/remoteproc/qcom_q6v5_pil.c
> @@ -65,6 +65,8 @@
> #define QDSP6SS_RESET_REG 0x014
> #define QDSP6SS_GFMUX_CTL_REG 0x020
> #define QDSP6SS_PWR_CTL_REG 0x030
> +#define QDSP6SS_MEM_PWR_CTL 0x0B0
> +#define QDSP6SS_STRAP_ACC 0x110
>
> /* AXI Halt Register Offsets */
> #define AXI_HALTREQ_REG 0x0
> @@ -93,6 +95,15 @@
> #define QDSS_BHS_ON BIT(21)
> #define QDSS_LDO_BYP BIT(22)
>
> +/* QDSP6v56 parameters */
> +#define QDSP6v56_LDO_BYP BIT(25)
> +#define QDSP6v56_BHS_ON BIT(24)
> +#define QDSP6v56_CLAMP_WL BIT(21)
> +#define QDSP6v56_CLAMP_QMC_MEM BIT(22)
> +#define HALT_CHECK_MAX_LOOPS (200)
> +#define QDSP6SS_XO_CBCR (0x0038)
Please drop these parenthesis.
> +#define QDSP6SS_ACC_OVERRIDE_VAL 0x20
> +
> struct rproc_hexagon_res {
> char *hexagon_mba_image;
> char **proxy_reg_string;
> @@ -147,6 +158,8 @@ struct q6v5 {
> phys_addr_t mpss_reloc;
> void *mpss_region;
> size_t mpss_size;
> +
> + int hexagon_ver;
> };
>
> enum {
> @@ -388,35 +401,103 @@ static int q6v5_rmb_mba_wait(struct q6v5 *qproc, u32 status, int ms)
>
> static int q6v5proc_reset(struct q6v5 *qproc)
> {
> - u32 val;
> - int ret;
> + u64 val;
> + int ret, i, count;
One variable per line, sorted descending by length, please.
> +
> + /* Override the ACC value if required */
> + if (qproc->hexagon_ver & 0x2)
This will break when we reach the 6th version, compare (==) with the
related enum.
> + writel_relaxed(QDSP6SS_ACC_OVERRIDE_VAL,
> + qproc->reg_base + QDSP6SS_STRAP_ACC);
>
> /* 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 */
This comment goes inside the if-statement.
> + if (qproc->hexagon_ver & 0x2) {
==
> + val = readl_relaxed(qproc->reg_base + QDSP6SS_XO_CBCR);
> + val |= 0x1;
> + writel_relaxed(val, qproc->reg_base + QDSP6SS_XO_CBCR);
Replace the rest of this block with:
ret = readl_poll_timeout(qproc->reg_base + QDSP6SS_XO_CBCR,
val, !(val & BIT(31)), 1, 200);
> + for (count = HALT_CHECK_MAX_LOOPS; count > 0; count--) {
> + val = readl_relaxed(qproc->reg_base + QDSP6SS_XO_CBCR);
> + if (!(val & BIT(31)))
> + break;
> + udelay(1);
> + }
> +
> + val = readl_relaxed(qproc->reg_base + QDSP6SS_XO_CBCR);
> + if ((val & BIT(31)))
> + dev_err(qproc->dev, "Failed to enable xo branch clock.\n");
> + }
> +
> + if (qproc->hexagon_ver & 0x2) {
==
> /* Enable power block headswitch, and wait for it to stabilize */
> - val = readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> - val |= QDSS_BHS_ON | QDSS_LDO_BYP;
> - writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> - udelay(1);
> -
> - /*
> - * Turn on memories. L2 banks should be done individually
> - * to minimize inrush current.
> - */
> - val = readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> - val |= Q6SS_SLP_RET_N | Q6SS_L2TAG_SLP_NRET_N |
> - Q6SS_ETB_SLP_NRET_N | Q6SS_L2DATA_STBY_N;
> - writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> - val |= Q6SS_L2DATA_SLP_NRET_N_2;
> - writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> - val |= Q6SS_L2DATA_SLP_NRET_N_1;
> - writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> - val |= Q6SS_L2DATA_SLP_NRET_N_0;
> - writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> + val = readl_relaxed(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
Use non-relaxed version of readl and writel, please.
> + val |= QDSP6v56_BHS_ON;
> + writel_relaxed(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> + udelay(1);
Please add a short comment on why this delay is needed.
>
> + /* Put LDO in bypass mode */
> + val |= QDSP6v56_LDO_BYP;
> + writel_relaxed(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> +
Remove empty line
> + } else {
> + /*
> + * Enable power block headswitch,
> + * and wait for it to stabilize
> + */
> + val = readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> + val |= QDSS_BHS_ON | QDSS_LDO_BYP;
> + writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> + udelay(1);
> + }
> +
> + if (qproc->hexagon_ver & 0x2) {
Why do you have:
if (ver == 2) {
...
}
if (ver == 2) {
...
} else {
...
}
if (ver == 2) {
...
} else {
...
}
As far as I can see you should be able to merge those into one if/else.
> + /*
> + * Deassert QDSP6 compiler memory clamp
> + */
> + val = readl_relaxed(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> + val &= ~QDSP6v56_CLAMP_QMC_MEM;
> + writel_relaxed(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_relaxed(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> +
> + /* Turn on L1, L2, ETB and JU memories 1 at a time */
> + val = readl_relaxed(qproc->reg_base + QDSP6SS_MEM_PWR_CTL);
> + for (i = 19; i >= 0; i--) {
> + val |= BIT(i);
> + writel_relaxed(val, qproc->reg_base +
> + QDSP6SS_MEM_PWR_CTL);
> + /*
> + * Wait for 1us for both memory peripheral and
> + * data array to turn on.
> + */
> + mb();
mb() ensures that your writes are ordered, it does not ensure that the
write is done before the sleep. What is the actual requirement here?
> + udelay(1);
> + }
> + /* Remove word line clamp */
> + val = readl_relaxed(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> + val &= ~QDSP6v56_CLAMP_WL;
> + writel_relaxed(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> + } else {
> + /*
> + * Turn on memories. L2 banks should be done individually
> + * to minimize inrush current.
> + */
> + val = readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> + val |= Q6SS_SLP_RET_N | Q6SS_L2TAG_SLP_NRET_N |
> + Q6SS_ETB_SLP_NRET_N | Q6SS_L2DATA_STBY_N;
> + writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> + val |= Q6SS_L2DATA_SLP_NRET_N_2;
> + writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> + val |= Q6SS_L2DATA_SLP_NRET_N_1;
> + writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> + val |= Q6SS_L2DATA_SLP_NRET_N_0;
> + writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> + }
> /* Remove IO clamp */
> val &= ~Q6SS_CLAMP_IO;
> writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
