Re: [PATCH v2 net 2/2] net: stmmac: dwmac-imx: pause the TXC clock in fixed-link

[Frank Li]

On Fri, Jul 28, 2023 at 04:36:12PM +0100, Will Deacon wrote:
> On Fri, Jul 28, 2023 at 04:22:19PM +0100, Russell King (Oracle) wrote:
> > On Thu, Jul 27, 2023 at 01:36:45PM -0500, Andrew Halaney wrote:
> > > I don't have any documentation for the registers here, and as you can
> > > see I'm an amateur with respect to memory ordering based on my prior
> > > comment.
> > > 
> > > But you:
> > > 
> > >     1. Read intf_reg_off into variable iface
> > >     2. Write the RESET_SPEED for the appropriate mode to MAC_CTRL_REG
> > >     3. wmb() to ensure that write goes through
> > 
> > I wonder about whether that wmb() is required. If the mapping is
> > device-like rather than memory-like, the write should be committed
> > before the read that regmap_update_bits() does according to the ARM
> > memory model. Maybe a bit of information about where this barrier
> > has come from would be good, and maybe getting it reviewed by the
> > arm64 barrier specialist, Will Deacon. :)
> > 
> > wmb() is normally required to be paired with a rmb(), but we're not
> > talking about system memory here, so I also wonder whether wmb() is
> > the correct barrier to use.
> 
> Yes, I don't think wmb() is the right thing here. If you need to ensure
> that the write to MAC_CTRL_REG has taken effect, then you'll need to go
> through some device-specific sequence which probably involves reading
> something back. If you just need things to arrive in order eventually,
> the memory type already gives you that.
> 
> It's also worth pointing out that udelay() isn't necessarily ordered wrt
> MMIO writes, so that usleep_range() might need some help as well.

Hi Deacon:

Does it means below pattern will be problem?

1.writel()
2.udelay()
3.writel()

It may not wait enough time between 1 and 3. I think the above pattern
is quite common in driver code.  I am not sure if usleep_range involve
MMIO to get current counter, ARM may use cp15 to get local timer counter.

In our system, readl() is quite slow because cross some bus bridge.
even readl() can work, we don't know it is because delay by readl() itself.
Or it works logically. Suppose readl() and writel() just guarantee memory
access order. 

Frank

> Non-relaxed MMIO reads, however, _are_ ordered against a subsequent
> udelay(), so if you add the readback then this might all work out.
> 
> I gave a (slightly dated) talk about some of this at ELC a while back:
> 
> https://www.youtube.com/watch?v=i6DayghhA8Q
> 
> which might help.
> 
> Will