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@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> ---
[...]
>  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@xxxxxxxxxxx>

Thanks,

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