Re: [RESEND PATCH v4 5/7] remoteproc: qcom: Modify reset sequence for hexagon to support v56 1.5.0

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Mon 12 Dec 04:45 PST 2016, Dwivedi, Avaneesh Kumar (avani) wrote:
> On 12/9/2016 10:05 AM, Bjorn Andersson wrote:
> >On Thu 24 Nov 02:00 PST 2016, Avaneesh Kumar Dwivedi wrote:
[..]
> >>+
> >>+		/* 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?
> As in comment, order of turning need to be serialized so this memory
> barrier.
> Do u think its not required?

The problem is that mb() don't actually wait for the write to finish, it
simply makes sure that any subsequent writes will come after this one.

If we want to make sure the write actually hits the hardware before the
delay we should read the register back after the write - as that would
stall execution until the write is available.

Either way, using the non-_relaxed version of writel() will be
equivalent to what you have now.

> >>+			udelay(1);
> >>+		}

Regards,
Bjorn