Re: [PATCH v2 2/2] irqchip: Add Qualcomm MPM controller driver

On Tue, 30 Nov 2021 08:31:44 +0000,
Shawn Guo [off-list ref] wrote:

On Tue, Nov 30, 2021 at 01:19:48PM +0530, Maulik Shah wrote:

quoted

   Hi Shawn,

   On 11/30/2021 8:01 AM, Shawn Guo wrote:

+       do {
+               r_val = readl(priv->base + offset);
+               udelay(5);
+       } while (r_val != val);

What? Is this waiting for a bit to clear? Why isn't this one of the
read*_poll_timeout*() function instead? Surely you can't wait forever
here.

This is taken from downstream, and it seems to double check the written
value by reading it back.  But to be honest, I'm not really this is
necessary.  I will do some testing with the read-back check dropped.

How about asking for specs instead? There are QC people on Cc, and
many more reading the list. Hopefully they can explain what this is
all about.

Maulik,

If you have some information about this, that would be great.

   This can be converted to read poll_timeout(). This was introduced in
   place of wmb() to make sure writes are completed.

Hmm, in this case, writel() will just do the right thing, as it wraps
wmb() there.  Or am I missing something?

writel() places the wmb() *before* the MMIO access. This is use for
ordering with RAM access if the device is DMA capable, for example. I
seriously doubt this is the case.

My understanding of Maulik's comment is that there is a requirement
for the MMIO access to complete. And for that, a barrier *after* the
write is the right tool for the job.

	M.

-- 
Without deviation from the norm, progress is not possible.