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.

Fixes: 5062245a5a7f ("net: remove legacy way to get/set HW timestamp config")
Assisted-by: LLM
Signed-off-by: James Clark <jjc@jclark.com>
---
 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;
+}
+
 static const struct net_device_ops bcmgenet_netdev_ops = {
 	.ndo_open		= bcmgenet_open,
 	.ndo_stop		= bcmgenet_close,
@@ -3723,6 +3736,8 @@ static const struct net_device_ops bcmgenet_netdev_ops = {
 	.ndo_set_features	= bcmgenet_set_features,
 	.ndo_get_stats64	= bcmgenet_get_stats64,
 	.ndo_change_carrier	= bcmgenet_change_carrier,
+	.ndo_hwtstamp_get	= bcmgenet_hwtstamp_get,
+	.ndo_hwtstamp_set	= bcmgenet_hwtstamp_set,
 };
 
 /* GENET hardware parameters/characteristics */
-- 
2.55.0

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Fix applied at the wrong layer: the regression the commit
  message diagnoses ("the core rejects timestamp configuration requests…
- [Medium] Registering ndo_hwtstamp_get/ndo_hwtstamp_set on a MAC that
  has no PHC opens a one-way trap on the ETHTOOL_MSG_TSCONFIG_SET path.

--

net: bcmgenet: add hwtstamp callbacks to restore PHY timestamping

This adds ndo_hwtstamp_get() and ndo_hwtstamp_set() stubs to bcmgenet that
both return -EOPNOTSUPP, so that the core hwtstamp entry points no longer
reject SIOCSHWTSTAMP/SIOCGHWTSTAMP before the phylib helpers get a chance to
dispatch to a timestamp-capable PHY.  No other behaviour is changed.

> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index b916080f4ff1..98d861811328 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -3721,6 +3721,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;
> +}
> +

[Severity: High]

Should this be fixed in net/core instead of per driver?  The commit message
describes the failure as "the core rejects timestamp configuration requests
before they reach the PHY", and the rejecting code is provider independent:

net/core/dev_ioctl.c:dev_get_hwtstamp() {
	if (!ops->ndo_hwtstamp_get)
		return -EOPNOTSUPP;
	...
}

net/core/dev_ioctl.c:dev_set_hwtstamp() {
	...
	if (!ops->ndo_hwtstamp_set)
		return -EOPNOTSUPP;
	...
}

The same presence test exists in generic_hwtstamp_get_lower(),
generic_hwtstamp_set_lower() and in ethnl_set_tsconfig_validate()
(net/ethtool/tsconfig.c):

	if (!ops->ndo_hwtstamp_set || !ops->ndo_hwtstamp_get)
		return -EOPNOTSUPP;

These are the lines 5062245a5a7f turned from a dev_eth_ioctl() fallback into
a hard error, so every MAC whose PTP comes only from the PHY is affected, not
just bcmgenet.

Two drivers in the same directory still have no ndo_hwtstamp_* and rely on
phylib ioctl handling:

drivers/net/ethernet/broadcom/bgmac.c:
	.ndo_eth_ioctl           = phy_do_ioctl_running,

drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:
	.ndo_eth_ioctl		= phy_do_ioctl_running,

arch/arm/boot/dts/broadcom/bcm47189-luxul-xap-1440.dts wires bgmac's gmac0 to
a BCM54210E, and that PHY driver calls bcm_ptp_probe() and sets
phydev->default_timestamp = true, so that board still fails SIOCSHWTSTAMP
after this patch.  Would skipping the NDO presence test when
phy_is_default_hwtstamp(dev->phydev) is true cover all of these at once?

A related question about the stubs themselves: since dev_set_hwtstamp_phylib()
only calls the MAC callback when !phy_ts or dev->see_all_hwtstamp_requests,
and bcmgenet sets neither, are these two functions ever invoked in the PHY
case?  If they exist purely as a capability signal, would a comment saying so
help, so a later "these always fail, remove them" cleanup does not silently
bring the failure back?

>  static const struct net_device_ops bcmgenet_netdev_ops = {
>  	.ndo_open		= bcmgenet_open,
>  	.ndo_stop		= bcmgenet_close,
> @@ -3732,6 +3745,8 @@ static const struct net_device_ops bcmgenet_netdev_ops = {
>  	.ndo_set_features	= bcmgenet_set_features,
>  	.ndo_get_stats64	= bcmgenet_get_stats64,
>  	.ndo_change_carrier	= bcmgenet_change_carrier,
> +	.ndo_hwtstamp_get	= bcmgenet_hwtstamp_get,
> +	.ndo_hwtstamp_set	= bcmgenet_hwtstamp_set,
>  };

[Severity: Medium]

Can advertising these NDOs on a MAC without a PHC make the
ETHTOOL_MSG_TSCONFIG_SET path reachable in a way that cannot be undone?

bcmgenet uses .get_ts_info = ethtool_op_get_ts_info, which reports
info->phc_index = -1.  Before this patch ethnl_set_tsconfig_validate()
returned -EOPNOTSUPP for bcmgenet, so ethnl_set_tsconfig() was never reached.
With both NDOs present, validation passes.

Consider this sequence:

1. TSCONFIG_SET selecting the PHY provider (index = the PHY phc_index), so
   dev->hwprov is non-NULL and PHY timestamping works.

2. TSCONFIG_SET with ETHTOOL_A_TS_HWTSTAMP_PROVIDER_INDEX = 0xFFFFFFFF and
   QUALIFIER = 0.  ts_parse_hwtst_provider() stores that through
   ethnl_update_u32() into the int index field of struct
   hwtstamp_provider_desc, giving -1.

net/ethtool/common.c:ethtool_net_get_ts_info_by_phc() {
	...
	if (info->phc_index == hwprov_desc->index &&
	    net_support_hwtstamp_qualifier(dev, hwprov_desc->qualifier))
		return 0;
	...
}

-1 == -1 matches, and net_support_hwtstamp_qualifier() returns true because
bcmgenet declares no supported_hwtstamp_qualifiers, so the provider becomes
HWTSTAMP_SOURCE_NETDEV with phydev = NULL.

ethnl_set_tsconfig() first disables the working PHY timestamper with
dev_set_hwtstamp_phylib(zero_config) and then installs that provider:

net/ethtool/tsconfig.c:ethnl_set_tsconfig() {
	...
		__hwprov = rcu_replace_pointer(dev->hwprov, hwprov,
					       netdev_is_locked_ops_compat(dev));
	...
}

>From there dev_get_hwtstamp_phylib() and dev_set_hwtstamp_phylib() take the
HWTSTAMP_SOURCE_NETDEV branch:

net/core/dev_ioctl.c:dev_set_hwtstamp_phylib() {
	...
		} else if (hwprov->source == HWTSTAMP_SOURCE_NETDEV) {
			phy_ts = false;
	...
}

so the PHY is never consulted and bcmgenet_hwtstamp_set() returns
-EOPNOTSUPP.

Does that also block recovery?  Reselecting the PHY provider runs the
zero-config disable step first, which now hits the NETDEV branch and the stub,
so ethnl_set_tsconfig() bails out through err_free_hwprov without replacing
dev->hwprov.  dev->hwprov is only cleared in phy_detach() when
hwprov->phydev matches the detaching phydev, which is NULL here, so ifdown
and ifup would not clear it either.

The permissive -1 comparison itself lives in core and predates this change,
but is this patch what makes it reachable for bcmgenet?  Would declaring a
real .get_ts_info (or rejecting an index of -1) avoid it?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918030149.80398-1-jjc%40jclark.com

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@jclark.com>
> ---
>  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.

> +
>  static const struct net_device_ops bcmgenet_netdev_ops = {
>  	.ndo_open		= bcmgenet_open,
>  	.ndo_stop		= bcmgenet_close,
> @@ -3723,6 +3736,8 @@ static const struct net_device_ops 
> bcmgenet_netdev_ops = {
>  	.ndo_set_features	= bcmgenet_set_features,
>  	.ndo_get_stats64	= bcmgenet_get_stats64,
>  	.ndo_change_carrier	= bcmgenet_change_carrier,
> +	.ndo_hwtstamp_get	= bcmgenet_hwtstamp_get,
> +	.ndo_hwtstamp_set	= bcmgenet_hwtstamp_set,
>  };
> 
>  /* GENET hardware parameters/characteristics */

Thanks,
Nicolai

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@jclark.com>
>> ---
>>  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