Re: [PATCH net] net: bcmgenet: add hwtstamp callbacks to restore PHY timestamping

[Kory Maincent]

On 9/18/26 10:20, Nicolai Buchwitz wrote:
> Hi James
>
> On 18.9.2026 05:01, James Clark wrote:
> > Hardware timestamp configuration fails with EOPNOTSUPP on BCMGENET
> > interfaces connected to a timestamp-capable PHY. On a Raspberry Pi
> > CM4, "hwstamp_ctl -i eth0 -t 1 -r 12" fails with "SIOCSHWTSTAMP failed:
> > Operation not supported", preventing ptp4l from using hardware
> > timestamping.
> >
> > Commit 5062245a5a7f ("net: remove legacy way to get/set HW timestamp
> > config") removed the legacy ioctl fallback, assuming that all MAC
> > drivers supporting hardware timestamping implement ndo_hwtstamp_get()
> > and ndo_hwtstamp_set(), even when timestamping is provided only by
> > an attached PHY. BCMGENET does not implement these callbacks, so the
> > core rejects timestamp configuration requests before they reach
> > the PHY.
> >
> > Add ndo_hwtstamp_get() and ndo_hwtstamp_set() callbacks so the core
> > can dispatch timestamp configuration requests through its phylib
> > helpers. Both callbacks return -EOPNOTSUPP because the MAC itself
> > does not support hardware timestamping; requests for the PHY are
> > handled by the core without invoking these callbacks.
>
> I've tested with and without your patch and can reproduce the issue / fix.
> IMHO the commit message should mention that 'get' isn't restored - it was
> already broken as the legacy path through phy_mii_ioctl() only handled
> SIOCSHWTSTAMP.
>
> > Fixes: 5062245a5a7f ("net: remove legacy way to get/set HW timestamp 
> > config")
> > Assisted-by: LLM
> > Signed-off-by: James Clark <jjc@xxxxxxxxxx>
> > ---
> >  drivers/net/ethernet/broadcom/genet/bcmgenet.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/ 
> > net/ethernet/broadcom/genet/bcmgenet.c
> > index a2305e642..8ead37ff9 100644
> > --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> > +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> > @@ -3712,6 +3712,19 @@ static int bcmgenet_change_carrier(struct 
> > net_device *dev, bool new_carrier)
> >      return 0;
> >  }
> >
> > +static int bcmgenet_hwtstamp_get(struct net_device *dev,
> > +                 struct kernel_hwtstamp_config *cfg)
> > +{
> > +    return -EOPNOTSUPP;
> > +}
> > +
> > +static int bcmgenet_hwtstamp_set(struct net_device *dev,
> > +                 struct kernel_hwtstamp_config *cfg,
> > +                 struct netlink_ext_ack *extack)
> > +{
> > +    return -EOPNOTSUPP;
> > +}
>
> The check that rejects the request is in dev_set_hwtstamp():
>
>          if (!ops->ndo_hwtstamp_set)
>                  return -EOPNOTSUPP;
>
> It runs before dev_set_hwtstamp_phylib() checks phy_is_default_hwtstamp()
> and nothing about this is specific to bcmgenet. Personally, I think this 
> should
> be better handled by the core than adding -EOPNOTSUPP stubs to the drivers.
>
> I will send a patch for this.

I agree. Now that the legacy ioctl fallback has been removed from all 
NIC drivers we don't need this check to verify its support anymore.

Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com