Re: [PATCH net-next 4/5] net: macb: implement timestamped MDIO writes

From: Théo Lebrun

Date: Fri Oct 16 2026 - 13:58:29 EST


On Fri Oct 16, 2026 at 6:36 AM CEST, James Clark wrote:
> On Thu, Oct 15, 2026 at 10:52 PM Théo Lebrun <theo.lebrun@xxxxxxxxxxx> wrote:
>> >> > + if (sts) {
>> >> > + delay_ns = macb_mdio_sts_delay(bp);
>> >> > + local_irq_save(flags);
>> >> > + ptp_read_system_prets(sts);
>> >> > + /* macb_writel() is relaxed; order it after the timestamp. */
>> >> > + mb();
>> >> > + }
>> >> > + macb_writel(bp, MAN, cmd);
>> >> > + if (sts) {
>> >> > + /* Flush the posted write before taking the upper bound. */
>> >> > + macb_readl(bp, NSR);
>> >>
>> >> An unconditional readl for flushing the posted write won't do it.
>> >> MACB is over PCI only on very few platforms. I guess the straightforward
>> >> solution is a POSTED_WRITES flag that is then read from here.
>> >
>> > This is only under sts. Assuming write_sts becomes RP1 only, then it
>> > is only over PCIe. I am not sure if I have understood what you are
>> > getting at here.
>>
>> On any platform but RP1, such a readl() is useless. My remark was that
>> the readl()-for-posted-writes must be made conditional, and I don't
>> think we should make the whole of write_sts conditional.
>
> I may be missing something, but I don't think the read-back is
> specific to PCIe. device-io.rst says that on many platforms and buses
> writes through ioremap() are posted, so the CPU doesn't wait for the
> write to reach the device, and it gives a dummy read as the way to
> wait for it that works on all platforms. On arm64 the mapping is
> Device-nGnRE, where the E means an intermediate point in the fabric
> may acknowledge the write before it reaches the MAC. The unimac patch
> in this series does the same read-back, on an on-chip controller.

Well today I learnt about 'Early Write Acknowledgement' on ARM, which
indeed is the default on ARM64 ioremap() mappings. I've only ever seen
readl() to flush posted writes on PCI busses, never on platform
devices. Thanks!

https://support.arm.com/documentation/102376/0200/Device-memory/Sub-types-of-Device

But then I'm curious: what timing would we prefer? Here adding a readl
means we measure an upper bound. If we dropped it, we would lose that
upper bound but we might get a more accurate measurement (by guessing
that if the request is in the write buffer of the interconnect, it will
soon reach the peripheral). After all a MMIO register read is pretty
slow and might be in the range of an MDC period.

Could that explain why you landed on 63.5 MDC periods delay instead of a
round 64 periods? Could that be compensation for the overhead MMIO
register read?

Just guessing here; no strong opinions.

Another random idea: could we grab a non-posted mapping using
ioremap_np() and then only use that for your usecase? An LLM tells me
only dwc3 acquires both mappings. They use the NP one for the xHCI
registers on some platforms. So we'd be the first to grab both and use
one for a specific register to get non-posted behavior; this is
probably a sign this idea is a bad one.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com