Re: [PATCH net-next 1/5] net: mdio: add timestamped write operation

[James Clark]

On Sun, Oct 11, 2026 at 9:03 PM Andrew Lunn <andrew@xxxxxxx> wrote:
>
> > + * If @sts is NULL, perform an ordinary write.
> > + *
>
> Please don't do that. We don't want drivers calling this when they
> don't need to.

Agreed.

> > +             if (!ktime_before(now.monoraw, sts->post_sts.monoraw))
> > +                     return 0;
> > +
> > +             /* Cap the wait at 1 ms, which is more than any single
> > +              * write's delay. This guards against an aux clock being
> > +              * disabled and reenabled, which restarts its raw time.
> > +              */
> > +             if (!ktime_before(ktime_get_raw(), deadline))
> > +                     return -EBUSY;
>
> A bit banging MDIO bus might be slower.

The comment is misleading; I'll fix it. The 1 ms doesn't limit how
long the write takes, but how long the core waits after write_sts()
returns. It allows for a driver that returns before the transfer is
complete, at worst as soon as it has started. A bit-banging driver
returns only after the last bit, so it doesn't wait at all.

> Also, i've not thought about the mux case, where the write gets
> blocked until some other operation is performed on the shared hardware
> of the mux.

In the mux case, mdio_mux_write_sts() takes the parent bus lock and
switches the mux before calling the parent's write_sts(). So any time
spent blocked on the mux comes before the lower bound. Or did you have
another case in mind?

> > +     /**
> > +      * @write_sts: Perform a write transfer on the bus,
> > +      * with system timestamps bounding its completion. Set only
> > +      * if timestamps can always be provided. Must not return
> > +      * -EOPNOTSUPP.
> > +      */
> > +     int (*write_sts)(struct mii_bus *bus, int addr, int regnum, u16 val,
> > +                      struct ptp_system_timestamp *sts);
>
> I suggest putting C22 in the comment. At some point, somebody might
> need C45. And that gets complex because of C45 over C22.

Agreed.

> Is there any need for read_sts?

Not for this series, I think. The BCM54210PE captures its time on a
write, so a timestamped read isn't needed here. I'd leave read_sts
until a driver needs one.

James