From: "Théo Lebrun" <theo.lebrun@bootlin.com>
To: "James Clark" <jjc@jclark.com>
Cc: "David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@kernel.org>,
"Jakub Kicinski" <kuba@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Andrew Lunn" <andrew+netdev@lunn.ch>,
"Heiner Kallweit" <hkallweit1@gmail.com>,
"Richard Cochran" <richardcochran@gmail.com>,
"Florian Fainelli" <florian.fainelli@broadcom.com>,
"Doug Berger" <opendmb@gmail.com>,
"Nicolai Buchwitz" <nb@tipi-net.de>,
"Russell King" <linux@armlinux.org.uk>,
"Conor Dooley" <conor.dooley@microchip.com>,
"Broadcom internal kernel review list"
<bcm-kernel-feedback-list@broadcom.com>,
"Thomas Gleixner" <tglx@kernel.org>,
"Miroslav Lichvar" <mlichvar@redhat.com>,
<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
"Maxime Chevallier" <maxime.chevallier@bootlin.com>,
"Brian Masney" <bmasney+clk@redhat.com>,
"Stephen Boyd" <sboyd@kernel.org>,
"Jerome Brunet" <jbrunet+clk@baylibre.com>
Subject: Re: [PATCH net-next 4/5] net: macb: implement timestamped MDIO writes
Date: Fri, 16 Oct 2026 19:51:54 +0200 [thread overview]
Message-ID: <DM6GCIT1KQXJ.2174FA0BOBXSG@bootlin.com> (raw)
In-Reply-To: <CANz3_EZ7SZau+xL8CaaNAHMvMia+_t1oVGiA-aksPCDUWkadHg@mail.gmail.com>
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@bootlin.com> 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
next prev parent reply other threads:[~2026-10-16 17:52 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-09 14:35 [PATCH net-next 0/5] net: mdio: add timestamped MDIO writes for PHY gettimex64 James Clark
2026-10-09 14:35 ` [PATCH net-next 1/5] net: mdio: add timestamped write operation James Clark
2026-10-10 15:10 ` netdev-bot+sashiko
2026-10-11 14:03 ` Andrew Lunn
2026-10-11 14:33 ` James Clark
2026-10-09 14:35 ` [PATCH net-next 2/5] net: phy: broadcom: use timestamped MDIO writes in gettimex64 James Clark
2026-10-10 15:10 ` netdev-bot+sashiko
2026-10-09 14:35 ` [PATCH net-next 3/5] ptp: add functions to adjust system timestamps James Clark
2026-10-10 15:10 ` netdev-bot+sashiko
2026-10-09 14:35 ` [PATCH net-next 4/5] net: macb: implement timestamped MDIO writes James Clark
2026-10-10 15:10 ` netdev-bot+sashiko
2026-10-11 9:23 ` Théo Lebrun
2026-10-11 10:02 ` Théo Lebrun
2026-10-11 13:01 ` James Clark
2026-10-15 15:52 ` Théo Lebrun
2026-10-15 16:02 ` Andrew Lunn
2026-10-16 4:36 ` James Clark
2026-10-16 17:51 ` Théo Lebrun [this message]
2026-10-11 21:10 ` Andrew Lunn
2026-10-09 14:35 ` [PATCH net-next 5/5] net: mdio: bcm-unimac: " James Clark
2026-10-09 16:04 ` Florian Fainelli
2026-10-10 1:25 ` James Clark
2026-10-10 15:18 ` Nicolai Buchwitz
2026-10-11 7:20 ` James Clark
2026-10-11 13:22 ` Nicolai Buchwitz
2026-10-12 0:54 ` James Clark
2026-10-12 13:18 ` Nicolai Buchwitz
2026-10-12 11:51 ` Andrew Lunn
2026-10-12 13:21 ` Nicolai Buchwitz
2026-10-15 22:12 ` Florian Fainelli
2026-10-10 15:11 ` netdev-bot+sashiko
2026-10-12 12:03 ` Andrew Lunn
2026-10-10 4:55 ` [PATCH net-next 0/5] net: mdio: add timestamped MDIO writes for PHY gettimex64 James Clark
2026-10-11 13:50 ` Andrew Lunn
2026-10-11 14:00 ` James Clark
2026-10-11 18:07 ` Nicolai Buchwitz
2026-10-12 13:18 ` Andrew Lunn
2026-10-12 13:20 ` Nicolai Buchwitz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6GCIT1KQXJ.2174FA0BOBXSG@bootlin.com \
--to=theo.lebrun@bootlin.com \
--cc=andrew+netdev@lunn.ch \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=bmasney+clk@redhat.com \
--cc=conor.dooley@microchip.com \
--cc=davem@davemloft.net \
--cc=edumazet@kernel.org \
--cc=florian.fainelli@broadcom.com \
--cc=hkallweit1@gmail.com \
--cc=jbrunet+clk@baylibre.com \
--cc=jjc@jclark.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=maxime.chevallier@bootlin.com \
--cc=mlichvar@redhat.com \
--cc=nb@tipi-net.de \
--cc=netdev@vger.kernel.org \
--cc=opendmb@gmail.com \
--cc=pabeni@redhat.com \
--cc=richardcochran@gmail.com \
--cc=sboyd@kernel.org \
--cc=tglx@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.