Messages in this thread | | | Date | Fri, 28 Jul 2023 20:33:38 +0100 | From | "Russell King (Oracle)" <> | Subject | Re: [PATCH v2 net 2/2] net: stmmac: dwmac-imx: pause the TXC clock in fixed-link |
| |
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!
| |