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 Tue 13 Dec 11:45 PST 2016, Dwivedi, Avaneesh Kumar (avani) wrote:
> On 12/13/2016 11:39 PM, Bjorn Andersson wrote:
[..]
> >Either way, using the non-_relaxed version of writel() will be
> >equivalent to what you have now.
> Do you mean if writel is used , udelay()  should be removed?

No, I mean that looping writel_relaxed() + wmb() is roughly
equivalent to writel(). So with the overall comment of you replacing
readl_relaxed() and writel_relaxed() with their plain readl/writel
counterparts takes care of the wmb().

> i understand
> writel will not return before register write operation is actually done.
> udelay() is to give enough time so that after writel , there is some time
> available to turn on mem peripheral and data array.

As far as I understand, wmb() will ensure that any cache coherent or
write-back buffered writes are committed before any subsequent writes.
But that this is not the same as the write has finished.

As far as I can see, the downstream code (msm-3.18) do:

for (i = 19; i >= 0; i--) {
	val |= BIT(i);
	writel_relaxed(val, MEM_PWR_CTL);
	val |= readl_relaxed(MEM_PWR_CTL);
	udelay(1);
}

I.e. for this particular version it actually does read back the value,
which will cause a wait for the write to be propagated. But I'm not sure
why this is the only version doing this.

Regards,
Bjorn