[PATCH net-next 0/4] net: macb: Rework hardware timestamp configuration

[PATCH net-next 0/4] net: macb: Rework hardware timestamp configuration

[Kim Wooseok via B4 Relay]

This started with two problems I found while using PTP hardware
timestamping on a Raspberry Pi 5. A rejected RX filter could still
change the TX one-step setting, and the PTPv1 Sync and Delay_Req filters
returned success without enabling RX timestamps.

This series reworks gem_set_hwtst() to validate the whole request before
programming the hardware. It calculates the settings locally, then
updates the registers and saved configuration under bp->lock. With the
register writes now in the setter, the two helpers are no longer needed.

The remaining patches use the existing ALL fallback for the specific
PTPv1 filters, turn off one-step mode for TX_OFF, and clear SRTSM when
switching away from a PTPv2 filter. Each change is kept in its own patch.

I compared the old and new behavior on the Pi 5. When I requested a TX
mode change together with an invalid RX filter, the patched driver
rejected the request without changing the saved settings or NCR. I
also switched from one-step TX and PTPv2 RX to other settings and back.
Only the relevant bits changed, and restoring the original settings
restored NCR.

For the PTPv1 filters, I checked the effect by sending Sync and Delay_Req
packets over Ethernet. Before the change, the packets arrived without
hardware timestamps. With the patches, the driver returned ALL and I
could read the hardware timestamps from the received packets. PTPv2
multicast reception continued to provide hardware timestamps, and
selecting NONE disabled timestamping. I ran these tests with the changes
backported to the Pi's existing Linux 6.18.46 RT kernel.

On net-next, I built the series with ARM64 allyesconfig and allmodconfig,
both with W=1. Sparse reported no diagnostics in macb_ptp.c.

Assisted-by: GPT-6 Astra
Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
---
Kim Wooseok (4):
      net: macb: Preserve timestamp settings on rejected requests
      net: macb: Enable RX timestamping for specific PTPv1 filters
      net: macb: Disable one-step mode when TX timestamping is off
      net: macb: Clear SRTSM outside PTPv2 receive filters

 drivers/net/ethernet/cadence/macb_ptp.c | 63 ++++++++++++---------------------
 1 file changed, 23 insertions(+), 40 deletions(-)
---
base-commit: 8830e65ed46de41f849eefb8ba227d4852c460f6
change-id: 20260922-codex-macb-hwtstamp-submit-25ab4a2e6b94

Best regards,
--  
Kim Wooseok <5mghybrid@khu.ac.kr>

[PATCH net-next 1/4] net: macb: Preserve timestamp settings on rejected requests

[Kim Wooseok via B4 Relay]

From: Kim Wooseok <5mghybrid@khu.ac.kr>

gem_set_hwtst() can reject a request after changing the TX one-step
setting, because it programs the TX mode before checking the RX
filter. The call returns -ERANGE, but the hardware may no longer match
the cached configuration.

Validate both settings first and keep the adjusted RX filter local
until validation succeeds. Then apply the register settings and update
the configuration. A rejected request now leaves the hardware, the
caller's settings and the cached configuration unchanged.

Protect the NCR read-modify-write with bp->lock, keeping the descriptor
writes and cache update in the same section. With the register writes
now in the setter, remove gem_ptp_set_one_step_sync() and
gem_ptp_set_ts_mode().

Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support")
Assisted-by: GPT-6 Astra
Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
---
 drivers/net/ethernet/cadence/macb_ptp.c | 61 ++++++++++++++-------------------
 1 file changed, 25 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
index 14ae57fa0..4dbb6daa6 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -374,16 +374,6 @@ void gem_ptp_remove(struct net_device *netdev)
 		 GEM_PTP_TIMER_NAME);
 }
 
-static int gem_ptp_set_ts_mode(struct macb *bp,
-			       enum macb_bd_control tx_bd_control,
-			       enum macb_bd_control rx_bd_control)
-{
-	gem_writel(bp, TXBDCTRL, GEM_BF(TXTSMODE, tx_bd_control));
-	gem_writel(bp, RXBDCTRL, GEM_BF(RXTSMODE, rx_bd_control));
-
-	return 0;
-}
-
 int gem_get_hwtst(struct net_device *netdev,
 		  struct kernel_hwtstamp_config *tstamp_config)
 {
@@ -396,25 +386,17 @@ int gem_get_hwtst(struct net_device *netdev,
 	return 0;
 }
 
-static void gem_ptp_set_one_step_sync(struct macb *bp, u8 enable)
-{
-	u32 reg_val;
-
-	reg_val = macb_readl(bp, NCR);
-
-	if (enable)
-		macb_writel(bp, NCR, reg_val | MACB_BIT(OSSMODE));
-	else
-		macb_writel(bp, NCR, reg_val & ~MACB_BIT(OSSMODE));
-}
-
 int gem_set_hwtst(struct net_device *netdev,
 		  struct kernel_hwtstamp_config *tstamp_config,
 		  struct netlink_ext_ack *extack)
 {
+	u32 ncr_mask = 0;
 	enum macb_bd_control tx_bd_control = TSTAMP_DISABLED;
 	enum macb_bd_control rx_bd_control = TSTAMP_DISABLED;
+	int rx_filter = tstamp_config->rx_filter;
 	struct macb *bp = netdev_priv(netdev);
+	unsigned long flags;
+	u32 ncr_bits = 0;
 	u32 regval;
 
 	if (!macb_dma_ptp(bp))
@@ -424,18 +406,17 @@ int gem_set_hwtst(struct net_device *netdev,
 	case HWTSTAMP_TX_OFF:
 		break;
 	case HWTSTAMP_TX_ONESTEP_SYNC:
-		gem_ptp_set_one_step_sync(bp, 1);
-		tx_bd_control = TSTAMP_ALL_FRAMES;
-		break;
+		ncr_bits |= MACB_BIT(OSSMODE);
+		fallthrough;
 	case HWTSTAMP_TX_ON:
-		gem_ptp_set_one_step_sync(bp, 0);
+		ncr_mask |= MACB_BIT(OSSMODE);
 		tx_bd_control = TSTAMP_ALL_FRAMES;
 		break;
 	default:
 		return -ERANGE;
 	}
 
-	switch (tstamp_config->rx_filter) {
+	switch (rx_filter) {
 	case HWTSTAMP_FILTER_NONE:
 		break;
 	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
@@ -451,25 +432,33 @@ int gem_set_hwtst(struct net_device *netdev,
 	case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
 	case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
 	case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
-		rx_bd_control =  TSTAMP_ALL_PTP_FRAMES;
-		tstamp_config->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
-		regval = macb_readl(bp, NCR);
-		macb_writel(bp, NCR, (regval | MACB_BIT(SRTSM)));
+		rx_bd_control = TSTAMP_ALL_PTP_FRAMES;
+		rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
+		ncr_mask |= MACB_BIT(SRTSM);
+		ncr_bits |= MACB_BIT(SRTSM);
 		break;
 	case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
 	case HWTSTAMP_FILTER_ALL:
 		rx_bd_control = TSTAMP_ALL_FRAMES;
-		tstamp_config->rx_filter = HWTSTAMP_FILTER_ALL;
+		rx_filter = HWTSTAMP_FILTER_ALL;
 		break;
 	default:
-		tstamp_config->rx_filter = HWTSTAMP_FILTER_NONE;
 		return -ERANGE;
 	}
 
-	bp->tstamp_config = *tstamp_config;
+	spin_lock_irqsave(&bp->lock, flags);
+	if (ncr_mask) {
+		regval = macb_readl(bp, NCR);
+		regval = (regval & ~ncr_mask) | ncr_bits;
+		macb_writel(bp, NCR, regval);
+	}
 
-	if (gem_ptp_set_ts_mode(bp, tx_bd_control, rx_bd_control) != 0)
-		return -ERANGE;
+	gem_writel(bp, TXBDCTRL, GEM_BF(TXTSMODE, tx_bd_control));
+	gem_writel(bp, RXBDCTRL, GEM_BF(RXTSMODE, rx_bd_control));
+
+	tstamp_config->rx_filter = rx_filter;
+	bp->tstamp_config = *tstamp_config;
+	spin_unlock_irqrestore(&bp->lock, flags);
 
 	return 0;
 }

-- 
2.53.0

Re: [PATCH net-next 1/4] net: macb: Preserve timestamp settings on rejected requests

[Nicolai Buchwitz]

On 22.9.2026 11:10, Kim Wooseok via B4 Relay wrote:
> From: Kim Wooseok <5mghybrid@khu.ac.kr>
> 
> gem_set_hwtst() can reject a request after changing the TX one-step
> setting, because it programs the TX mode before checking the RX
> filter. The call returns -ERANGE, but the hardware may no longer match
> the cached configuration.
> 
> Validate both settings first and keep the adjusted RX filter local
> until validation succeeds. Then apply the register settings and update
> the configuration. A rejected request now leaves the hardware, the
> caller's settings and the cached configuration unchanged.
> 
> Protect the NCR read-modify-write with bp->lock, keeping the descriptor
> writes and cache update in the same section. With the register writes
> now in the setter, remove gem_ptp_set_one_step_sync() and
> gem_ptp_set_ts_mode().
> 
> Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support")
> Assisted-by: GPT-6 Astra
> Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
> ---
>  drivers/net/ethernet/cadence/macb_ptp.c | 61 
> ++++++++++++++-------------------
>  1 file changed, 25 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_ptp.c 
> b/drivers/net/ethernet/cadence/macb_ptp.c
> index 14ae57fa0..4dbb6daa6 100644
> --- a/drivers/net/ethernet/cadence/macb_ptp.c
> +++ b/drivers/net/ethernet/cadence/macb_ptp.c

> [...]

>  int gem_set_hwtst(struct net_device *netdev,
>  		  struct kernel_hwtstamp_config *tstamp_config,
>  		  struct netlink_ext_ack *extack)
>  {
> +	u32 ncr_mask = 0;
>  	enum macb_bd_control tx_bd_control = TSTAMP_DISABLED;
>  	enum macb_bd_control rx_bd_control = TSTAMP_DISABLED;
> +	int rx_filter = tstamp_config->rx_filter;
>  	struct macb *bp = netdev_priv(netdev);
> +	unsigned long flags;
> +	u32 ncr_bits = 0;
>  	u32 regval;

Please re-order to keep RCS [1]

> [...]

[1] 
https://www.kernel.org/doc/html/v6.2/process/maintainer-netdev.html#local-variable-ordering-reverse-xmas-tree-rcs

With the above fixed:

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai

Re: [PATCH net-next 1/4] net: macb: Preserve timestamp settings on rejected requests

[Théo Lebrun]

Hello Kim,

On Tue Sep 22, 2026 at 11:10 AM CEST, Kim Wooseok via B4 Relay wrote:
> From: Kim Wooseok <5mghybrid@khu.ac.kr>
>
> gem_set_hwtst() can reject a request after changing the TX one-step
> setting, because it programs the TX mode before checking the RX
> filter. The call returns -ERANGE, but the hardware may no longer match
> the cached configuration.
>
> Validate both settings first and keep the adjusted RX filter local
> until validation succeeds. Then apply the register settings and update
> the configuration. A rejected request now leaves the hardware, the
> caller's settings and the cached configuration unchanged.
>
> Protect the NCR read-modify-write with bp->lock, keeping the descriptor
> writes and cache update in the same section. With the register writes
> now in the setter, remove gem_ptp_set_one_step_sync() and
> gem_ptp_set_ts_mode().
>
> Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support")
> Assisted-by: GPT-6 Astra
> Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
> ---
[...]
>  int gem_set_hwtst(struct net_device *netdev,
>  		  struct kernel_hwtstamp_config *tstamp_config,
>  		  struct netlink_ext_ack *extack)
>  {
> +	u32 ncr_mask = 0;
>  	enum macb_bd_control tx_bd_control = TSTAMP_DISABLED;
>  	enum macb_bd_control rx_bd_control = TSTAMP_DISABLED;
> +	int rx_filter = tstamp_config->rx_filter;
>  	struct macb *bp = netdev_priv(netdev);
> +	unsigned long flags;
> +	u32 ncr_bits = 0;
>  	u32 regval;

Same remark as Nicolai (no surprise there). With that

Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>

Thanks,

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

Re: [PATCH net-next 1/4] net: macb: Preserve timestamp settings on rejected requests

[Théo Lebrun]

Hello Théo,

On Tue Sep 22, 2026 at 9:01 PM CEST, Théo Lebrun wrote:
> Hello Kim,
>
> On Tue Sep 22, 2026 at 11:10 AM CEST, Kim Wooseok via B4 Relay wrote:
>> From: Kim Wooseok <5mghybrid@khu.ac.kr>
>>
>> gem_set_hwtst() can reject a request after changing the TX one-step
>> setting, because it programs the TX mode before checking the RX
>> filter. The call returns -ERANGE, but the hardware may no longer match
>> the cached configuration.
>>
>> Validate both settings first and keep the adjusted RX filter local
>> until validation succeeds. Then apply the register settings and update
>> the configuration. A rejected request now leaves the hardware, the
>> caller's settings and the cached configuration unchanged.
>>
>> Protect the NCR read-modify-write with bp->lock, keeping the descriptor
>> writes and cache update in the same section. With the register writes
>> now in the setter, remove gem_ptp_set_one_step_sync() and
>> gem_ptp_set_ts_mode().
>>
>> Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support")
>> Assisted-by: GPT-6 Astra
>> Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
>> ---
> [...]
>>  int gem_set_hwtst(struct net_device *netdev,
>>  		  struct kernel_hwtstamp_config *tstamp_config,
>>  		  struct netlink_ext_ack *extack)
>>  {
>> +	u32 ncr_mask = 0;
>>  	enum macb_bd_control tx_bd_control = TSTAMP_DISABLED;
>>  	enum macb_bd_control rx_bd_control = TSTAMP_DISABLED;
>> +	int rx_filter = tstamp_config->rx_filter;
>>  	struct macb *bp = netdev_priv(netdev);
>> +	unsigned long flags;
>> +	u32 ncr_bits = 0;
>>  	u32 regval;
>
> Same remark as Nicolai (no surprise there). With that
>
> Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>

Sorry for the two stage review, I had forgotten that aspect on first
review. I would prefer simpler code:

> @@ -424,18 +406,17 @@ int gem_set_hwtst(struct net_device *netdev,
>     case HWTSTAMP_TX_OFF:
>        break;
>     case HWTSTAMP_TX_ONESTEP_SYNC:
> -      gem_ptp_set_one_step_sync(bp, 1);
> -      tx_bd_control = TSTAMP_ALL_FRAMES;
> -      break;
> +      ncr_bits |= MACB_BIT(OSSMODE);
> +      fallthrough;
>     case HWTSTAMP_TX_ON:
> -      gem_ptp_set_one_step_sync(bp, 0);
> +      ncr_mask |= MACB_BIT(OSSMODE);
>        tx_bd_control = TSTAMP_ALL_FRAMES;
>        break;
>     default:
>        return -ERANGE;
>     }

Let's avoid the fallthrough. Simple & easy to read:

	case HWTSTAMP_TX_ONESTEP_SYNC:
		ncr_bits |= MACB_BIT(OSSMODE);
		ncr_mask |= MACB_BIT(OSSMODE);
		tx_bd_control = TSTAMP_ALL_FRAMES;
		break;
	case HWTSTAMP_TX_ON:
		ncr_mask |= MACB_BIT(OSSMODE);
		tx_bd_control = TSTAMP_ALL_FRAMES;
		break;

Thanks,

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

[PATCH net-next 2/4] net: macb: Enable RX timestamping for specific PTPv1 filters

[Kim Wooseok via B4 Relay]

From: Kim Wooseok <5mghybrid@khu.ac.kr>

Selecting a PTPv1 Sync or Delay_Req filter in gem_set_hwtst() returns
success, but leaves rx_bd_control at its initial value of
TSTAMP_DISABLED. The requested filter therefore appears to have been
applied, even though received packets have no hardware timestamps.

The PTPv1 event case already enables timestamping for all frames, so
use that path for Sync and Delay_Req as well. This enables timestamping
for both requests and returns HWTSTAMP_FILTER_ALL to tell the caller
which filter was actually applied.

Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support")
Assisted-by: GPT-6 Astra
Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
---
 drivers/net/ethernet/cadence/macb_ptp.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
index 4dbb6daa6..b6d17fef4 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -419,10 +419,6 @@ int gem_set_hwtst(struct net_device *netdev,
 	switch (rx_filter) {
 	case HWTSTAMP_FILTER_NONE:
 		break;
-	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
-		break;
-	case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
-		break;
 	case HWTSTAMP_FILTER_PTP_V2_EVENT:
 	case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
 	case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
@@ -437,6 +433,8 @@ int gem_set_hwtst(struct net_device *netdev,
 		ncr_mask |= MACB_BIT(SRTSM);
 		ncr_bits |= MACB_BIT(SRTSM);
 		break;
+	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
+	case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
 	case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
 	case HWTSTAMP_FILTER_ALL:
 		rx_bd_control = TSTAMP_ALL_FRAMES;

-- 
2.53.0

Re: [PATCH net-next 2/4] net: macb: Enable RX timestamping for specific PTPv1 filters

[Nicolai Buchwitz]

On 22.9.2026 11:10, Kim Wooseok via B4 Relay wrote:
> From: Kim Wooseok <5mghybrid@khu.ac.kr>
> 
> Selecting a PTPv1 Sync or Delay_Req filter in gem_set_hwtst() returns
> success, but leaves rx_bd_control at its initial value of
> TSTAMP_DISABLED. The requested filter therefore appears to have been
> applied, even though received packets have no hardware timestamps.
> 
> The PTPv1 event case already enables timestamping for all frames, so
> use that path for Sync and Delay_Req as well. This enables timestamping
> for both requests and returns HWTSTAMP_FILTER_ALL to tell the caller
> which filter was actually applied.
> 
> Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support")
> Assisted-by: GPT-6 Astra
> Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
> ---
>  drivers/net/ethernet/cadence/macb_ptp.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_ptp.c 
> b/drivers/net/ethernet/cadence/macb_ptp.c
> index 4dbb6daa6..b6d17fef4 100644
> --- a/drivers/net/ethernet/cadence/macb_ptp.c
> +++ b/drivers/net/ethernet/cadence/macb_ptp.c
> @@ -419,10 +419,6 @@ int gem_set_hwtst(struct net_device *netdev,
>  	switch (rx_filter) {
>  	case HWTSTAMP_FILTER_NONE:
>  		break;
> -	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
> -		break;
> -	case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
> -		break;
>  	case HWTSTAMP_FILTER_PTP_V2_EVENT:
>  	case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
>  	case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
> @@ -437,6 +433,8 @@ int gem_set_hwtst(struct net_device *netdev,
>  		ncr_mask |= MACB_BIT(SRTSM);
>  		ncr_bits |= MACB_BIT(SRTSM);
>  		break;
> +	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
> +	case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
>  	case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
>  	case HWTSTAMP_FILTER_ALL:
>  		rx_bd_control = TSTAMP_ALL_FRAMES;

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai

Re: [PATCH net-next 2/4] net: macb: Enable RX timestamping for specific PTPv1 filters

[Théo Lebrun]

Hello Kim,

On Tue Sep 22, 2026 at 11:10 AM CEST, Kim Wooseok via B4 Relay wrote:
> From: Kim Wooseok <5mghybrid@khu.ac.kr>
>
> Selecting a PTPv1 Sync or Delay_Req filter in gem_set_hwtst() returns
> success, but leaves rx_bd_control at its initial value of
> TSTAMP_DISABLED. The requested filter therefore appears to have been
> applied, even though received packets have no hardware timestamps.
>
> The PTPv1 event case already enables timestamping for all frames, so
> use that path for Sync and Delay_Req as well. This enables timestamping
> for both requests and returns HWTSTAMP_FILTER_ALL to tell the caller
> which filter was actually applied.
>
> Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support")
> Assisted-by: GPT-6 Astra
> Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>

Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>

Thanks,

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

[PATCH net-next 3/4] net: macb: Disable one-step mode when TX timestamping is off

[Kim Wooseok via B4 Relay]

From: Kim Wooseok <5mghybrid@khu.ac.kr>

Switching from HWTSTAMP_TX_ONESTEP_SYNC to HWTSTAMP_TX_OFF turns off
descriptor timestamping, but leaves NCR.OSSMODE set, so one-step mode
remains enabled.

Update OSSMODE for every accepted TX mode and set it only for
HWTSTAMP_TX_ONESTEP_SYNC. This also clears the previous one-step setting
when switching to OFF. Since the update mask now always includes
OSSMODE, drop the check for an empty mask.

Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support")
Assisted-by: GPT-6 Astra
Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
---
 drivers/net/ethernet/cadence/macb_ptp.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
index b6d17fef4..6376b6631 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -390,7 +390,7 @@ int gem_set_hwtst(struct net_device *netdev,
 		  struct kernel_hwtstamp_config *tstamp_config,
 		  struct netlink_ext_ack *extack)
 {
-	u32 ncr_mask = 0;
+	u32 ncr_mask = MACB_BIT(OSSMODE);
 	enum macb_bd_control tx_bd_control = TSTAMP_DISABLED;
 	enum macb_bd_control rx_bd_control = TSTAMP_DISABLED;
 	int rx_filter = tstamp_config->rx_filter;
@@ -409,7 +409,6 @@ int gem_set_hwtst(struct net_device *netdev,
 		ncr_bits |= MACB_BIT(OSSMODE);
 		fallthrough;
 	case HWTSTAMP_TX_ON:
-		ncr_mask |= MACB_BIT(OSSMODE);
 		tx_bd_control = TSTAMP_ALL_FRAMES;
 		break;
 	default:
@@ -445,11 +444,9 @@ int gem_set_hwtst(struct net_device *netdev,
 	}
 
 	spin_lock_irqsave(&bp->lock, flags);
-	if (ncr_mask) {
-		regval = macb_readl(bp, NCR);
-		regval = (regval & ~ncr_mask) | ncr_bits;
-		macb_writel(bp, NCR, regval);
-	}
+	regval = macb_readl(bp, NCR);
+	regval = (regval & ~ncr_mask) | ncr_bits;
+	macb_writel(bp, NCR, regval);
 
 	gem_writel(bp, TXBDCTRL, GEM_BF(TXTSMODE, tx_bd_control));
 	gem_writel(bp, RXBDCTRL, GEM_BF(RXTSMODE, rx_bd_control));

-- 
2.53.0

Re: [PATCH net-next 3/4] net: macb: Disable one-step mode when TX timestamping is off

[Nicolai Buchwitz]

On 22.9.2026 11:10, Kim Wooseok via B4 Relay wrote:
> From: Kim Wooseok <5mghybrid@khu.ac.kr>
> 
> Switching from HWTSTAMP_TX_ONESTEP_SYNC to HWTSTAMP_TX_OFF turns off
> descriptor timestamping, but leaves NCR.OSSMODE set, so one-step mode
> remains enabled.
> 
> Update OSSMODE for every accepted TX mode and set it only for
> HWTSTAMP_TX_ONESTEP_SYNC. This also clears the previous one-step 
> setting
> when switching to OFF. Since the update mask now always includes
> OSSMODE, drop the check for an empty mask.
> 
> Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support")
> Assisted-by: GPT-6 Astra
> Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
> ---
>  drivers/net/ethernet/cadence/macb_ptp.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_ptp.c 
> b/drivers/net/ethernet/cadence/macb_ptp.c
> index b6d17fef4..6376b6631 100644
> --- a/drivers/net/ethernet/cadence/macb_ptp.c
> +++ b/drivers/net/ethernet/cadence/macb_ptp.c
> @@ -390,7 +390,7 @@ int gem_set_hwtst(struct net_device *netdev,
>  		  struct kernel_hwtstamp_config *tstamp_config,
>  		  struct netlink_ext_ack *extack)
>  {
> -	u32 ncr_mask = 0;
> +	u32 ncr_mask = MACB_BIT(OSSMODE);
>  	enum macb_bd_control tx_bd_control = TSTAMP_DISABLED;
>  	enum macb_bd_control rx_bd_control = TSTAMP_DISABLED;
>  	int rx_filter = tstamp_config->rx_filter;
> @@ -409,7 +409,6 @@ int gem_set_hwtst(struct net_device *netdev,
>  		ncr_bits |= MACB_BIT(OSSMODE);
>  		fallthrough;
>  	case HWTSTAMP_TX_ON:
> -		ncr_mask |= MACB_BIT(OSSMODE);
>  		tx_bd_control = TSTAMP_ALL_FRAMES;
>  		break;
>  	default:
> @@ -445,11 +444,9 @@ int gem_set_hwtst(struct net_device *netdev,
>  	}
> 
>  	spin_lock_irqsave(&bp->lock, flags);
> -	if (ncr_mask) {
> -		regval = macb_readl(bp, NCR);
> -		regval = (regval & ~ncr_mask) | ncr_bits;
> -		macb_writel(bp, NCR, regval);
> -	}
> +	regval = macb_readl(bp, NCR);
> +	regval = (regval & ~ncr_mask) | ncr_bits;
> +	macb_writel(bp, NCR, regval);
> 
>  	gem_writel(bp, TXBDCTRL, GEM_BF(TXTSMODE, tx_bd_control));
>  	gem_writel(bp, RXBDCTRL, GEM_BF(RXTSMODE, rx_bd_control));

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai

Re: [PATCH net-next 3/4] net: macb: Disable one-step mode when TX timestamping is off

[Théo Lebrun]

Hello Kim,

On Tue Sep 22, 2026 at 11:10 AM CEST, Kim Wooseok via B4 Relay wrote:
> From: Kim Wooseok <5mghybrid@khu.ac.kr>
>
> Switching from HWTSTAMP_TX_ONESTEP_SYNC to HWTSTAMP_TX_OFF turns off
> descriptor timestamping, but leaves NCR.OSSMODE set, so one-step mode
> remains enabled.
>
> Update OSSMODE for every accepted TX mode and set it only for
> HWTSTAMP_TX_ONESTEP_SYNC. This also clears the previous one-step setting
> when switching to OFF. Since the update mask now always includes
> OSSMODE, drop the check for an empty mask.
>
> Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support")
> Assisted-by: GPT-6 Astra
> Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>

I don't see the point in this commit. NCR.OSSMODE defines what value we
put in TX Sync messages. This register can be left alone when
HWTSTAMP_TX_OFF.

Thanks,

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

[PATCH net-next 4/4] net: macb: Clear SRTSM outside PTPv2 receive filters

[Kim Wooseok via B4 Relay]

From: Kim Wooseok <5mghybrid@khu.ac.kr>

A PTPv2 receive filter sets NCR.SRTSM, but switching to NONE, ALL or a
PTPv1 filter leaves it set. As a result, selecting the same filter can
produce a different register setting depending on whether PTPv2 was
used before.

Update SRTSM whenever the RX filter changes, setting it only for PTPv2.
Switching away from PTPv2 then clears the old setting, so the bit follows
the currently selected filter.

Assisted-by: GPT-6 Astra
Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
---
 drivers/net/ethernet/cadence/macb_ptp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
index 6376b6631..637d0da32 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -390,7 +390,7 @@ int gem_set_hwtst(struct net_device *netdev,
 		  struct kernel_hwtstamp_config *tstamp_config,
 		  struct netlink_ext_ack *extack)
 {
-	u32 ncr_mask = MACB_BIT(OSSMODE);
+	u32 ncr_mask = MACB_BIT(OSSMODE) | MACB_BIT(SRTSM);
 	enum macb_bd_control tx_bd_control = TSTAMP_DISABLED;
 	enum macb_bd_control rx_bd_control = TSTAMP_DISABLED;
 	int rx_filter = tstamp_config->rx_filter;
@@ -429,7 +429,6 @@ int gem_set_hwtst(struct net_device *netdev,
 	case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
 		rx_bd_control = TSTAMP_ALL_PTP_FRAMES;
 		rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
-		ncr_mask |= MACB_BIT(SRTSM);
 		ncr_bits |= MACB_BIT(SRTSM);
 		break;
 	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:

-- 
2.53.0

Re: [PATCH net-next 4/4] net: macb: Clear SRTSM outside PTPv2 receive filters

[Nicolai Buchwitz]

On 22.9.2026 11:10, Kim Wooseok via B4 Relay wrote:
> From: Kim Wooseok <5mghybrid@khu.ac.kr>
> 
> A PTPv2 receive filter sets NCR.SRTSM, but switching to NONE, ALL or a
> PTPv1 filter leaves it set. As a result, selecting the same filter can
> produce a different register setting depending on whether PTPv2 was
> used before.
> 
> Update SRTSM whenever the RX filter changes, setting it only for PTPv2.
> Switching away from PTPv2 then clears the old setting, so the bit 
> follows
> the currently selected filter.
> 
> Assisted-by: GPT-6 Astra
> Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
> ---
>  drivers/net/ethernet/cadence/macb_ptp.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_ptp.c 
> b/drivers/net/ethernet/cadence/macb_ptp.c
> index 6376b6631..637d0da32 100644
> --- a/drivers/net/ethernet/cadence/macb_ptp.c
> +++ b/drivers/net/ethernet/cadence/macb_ptp.c
> @@ -390,7 +390,7 @@ int gem_set_hwtst(struct net_device *netdev,
>  		  struct kernel_hwtstamp_config *tstamp_config,
>  		  struct netlink_ext_ack *extack)
>  {
> -	u32 ncr_mask = MACB_BIT(OSSMODE);
> +	u32 ncr_mask = MACB_BIT(OSSMODE) | MACB_BIT(SRTSM);
>  	enum macb_bd_control tx_bd_control = TSTAMP_DISABLED;
>  	enum macb_bd_control rx_bd_control = TSTAMP_DISABLED;
>  	int rx_filter = tstamp_config->rx_filter;
> @@ -429,7 +429,6 @@ int gem_set_hwtst(struct net_device *netdev,
>  	case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
>  		rx_bd_control = TSTAMP_ALL_PTP_FRAMES;
>  		rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
> -		ncr_mask |= MACB_BIT(SRTSM);
>  		ncr_bits |= MACB_BIT(SRTSM);
>  		break;
>  	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai

Re: [PATCH net-next 4/4] net: macb: Clear SRTSM outside PTPv2 receive filters

[Théo Lebrun]

Hello Kim,

On Tue Sep 22, 2026 at 11:10 AM CEST, Kim Wooseok via B4 Relay wrote:
> From: Kim Wooseok <5mghybrid@khu.ac.kr>
>
> A PTPv2 receive filter sets NCR.SRTSM, but switching to NONE, ALL or a
> PTPv1 filter leaves it set. As a result, selecting the same filter can
> produce a different register setting depending on whether PTPv2 was
> used before.
>
> Update SRTSM whenever the RX filter changes, setting it only for PTPv2.
> Switching away from PTPv2 then clears the old setting, so the bit follows
> the currently selected filter.
>
> Assisted-by: GPT-6 Astra
> Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>

This one sounds more relevant to keep (compared to patch 3). We have
NCFGR.DRFCS always set so the FCS is always discarded so I don't think
we fix any bug here. But we avoid a footgun that could hit later if we
add Rx FCS support.

Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>

Thanks,

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

Re: [PATCH net-next 0/4] net: macb: Rework hardware timestamp configuration

[Nicolai Buchwitz]

Hi Kim

On 22.9.2026 11:10, Kim Wooseok via B4 Relay wrote:
> This started with two problems I found while using PTP hardware
> timestamping on a Raspberry Pi 5. A rejected RX filter could still
> change the TX one-step setting, and the PTPv1 Sync and Delay_Req 
> filters
> returned success without enabling RX timestamps.
> 
> This series reworks gem_set_hwtst() to validate the whole request 
> before
> programming the hardware. It calculates the settings locally, then
> updates the registers and saved configuration under bp->lock. With the
> register writes now in the setter, the two helpers are no longer 
> needed.
> 
> The remaining patches use the existing ALL fallback for the specific
> PTPv1 filters, turn off one-step mode for TX_OFF, and clear SRTSM when
> switching away from a PTPv2 filter. Each change is kept in its own 
> patch.
> 
> I compared the old and new behavior on the Pi 5. When I requested a TX
> mode change together with an invalid RX filter, the patched driver
> rejected the request without changing the saved settings or NCR. I
> also switched from one-step TX and PTPv2 RX to other settings and back.
> Only the relevant bits changed, and restoring the original settings
> restored NCR.
> 
> For the PTPv1 filters, I checked the effect by sending Sync and 
> Delay_Req
> packets over Ethernet. Before the change, the packets arrived without
> hardware timestamps. With the patches, the driver returned ALL and I
> could read the hardware timestamps from the received packets. PTPv2
> multicast reception continued to provide hardware timestamps, and
> selecting NONE disabled timestamping. I ran these tests with the 
> changes
> backported to the Pi's existing Linux 6.18.46 RT kernel.
> 
> On net-next, I built the series with ARM64 allyesconfig and 
> allmodconfig,
> both with W=1. Sparse reported no diagnostics in macb_ptp.c.
> 
> Assisted-by: GPT-6 Astra
> Signed-off-by: Kim Wooseok <5mghybrid@khu.ac.kr>
> ---
> Kim Wooseok (4):
>       net: macb: Preserve timestamp settings on rejected requests
>       net: macb: Enable RX timestamping for specific PTPv1 filters
>       net: macb: Disable one-step mode when TX timestamping is off
>       net: macb: Clear SRTSM outside PTPv2 receive filters
> 
>  drivers/net/ethernet/cadence/macb_ptp.c | 63 
> ++++++++++++---------------------
>  1 file changed, 23 insertions(+), 40 deletions(-)
> ---
> base-commit: 8830e65ed46de41f849eefb8ba227d4852c460f6
> change-id: 20260922-codex-macb-hwtstamp-submit-25ab4a2e6b94
> 
> Best regards,
> --
> Kim Wooseok <5mghybrid@khu.ac.kr>

For the series:

Tested-by: Nicolai Buchwitz <nb@tipi-net.de> # Raspberry Pi CM5 (macb 
rp1 with BCM54210PE)

Regards
Nicolai