Re: [PATCH net] net: ethernet: ti: am65-cpsw-nuss: Fix rx_filter value for PTP support

[Chintan Vankar]

Hello Jakub,

On 05/03/26 08:31, Jakub Kicinski wrote:
> On Tue, 3 Mar 2026 13:28:19 +0530 Chintan Vankar wrote:
> > @@ -1888,8 +1894,8 @@ static int am65_cpsw_nuss_hwtstamp_get(struct net_device *ndev,
> >   	cfg->flags = 0;
> >   	cfg->tx_type = port->tx_ts_enabled ?
> >   		      HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF;
> > -	cfg->rx_filter = port->rx_ts_enabled ? HWTSTAMP_FILTER_PTP_V2_EVENT |
> > -			HWTSTAMP_FILTER_PTP_V1_L4_EVENT : HWTSTAMP_FILTER_NONE;
> > +	cfg->rx_filter = port->rx_ts_enabled ?
> > +			 port->rx_ts_filter : HWTSTAMP_FILTER_NONE;
>
> HWTSTAMP_FILTER_NONE == 0
> a lot of code depends on that
> so you can just assign port->rx_ts_filter
>
> >   
> >   	return 0;
> >   }
> > diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.h b/drivers/net/ethernet/ti/am65-cpsw-nuss.h
> > index 917c37e4e89b..595123c155db 100644
> > --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.h
> > +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.h
> > @@ -53,6 +53,7 @@ struct am65_cpsw_port {
> >   	struct am65_cpsw_slave_data	slave;
> >   	bool				tx_ts_enabled;
> >   	bool				rx_ts_enabled;
> > +	int				rx_ts_filter;
>
> given none == 0 you can just turn rx_ts_enabled from a bool into
> an enum hwtstamp_rx_filters and maybe rename it?
> no need to have two members in the struct for rx

Thanks for suggesting this, I have posted a version 2 with updated
changes at:
https://lore.kernel.org/r/20260310160940.109822-1-c-vankar@xxxxxx/

Regards,
Chintan.