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

[Russell King (Oracle)]

On Fri, Jul 28, 2023 at 12:29:46PM -0400, Frank Li wrote:
> On Fri, Jul 28, 2023 at 04:36:12PM +0100, Will Deacon wrote:
> > 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()

Yes, it can be a problem - because the first write may take a while
to hit the hardware. It's been this way ever since PCI became a thing,
even on x86 hardware.

PCI posting rules are that writes can be posted into the various
bridges in the bus structure and forwarded on at some point later.
However, reads are not allowed to bypass writes - which means that if
one reads from a PCI device, the preceeding writes need to be flushed
out of the bridges _in the path to the device being read_.

So, if we take an example and apply it to PCI:

	writel()
	udelay(100)
	writel()
	readl()

The device could well see nothing for a while, and then two consecutive
writes and a read in quick succession.

> 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.

There are no guarantees, even on x86, that udelay() offers anything to
space device writes apart.

If this pattern is popular in drivers, and it's critical to the
drivers operation, then it's technically buggy - and it's been that way
for at least a couple of decades! One might get away with it (maybe the
hardware isn't delaying the writes?) but the kernel has never
guaranteed that writel(), udelay(), writel() will space the two writes
apart by the specified delay.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!