Re: [PATCH net-next] net: dsa: mv88e6xxx: Allow hwstamping on the master port

[Vladimir Oltean <olteanv@gmail.com>]

On Thu, Nov 10, 2022 at 09:43:45AM -0300, Fabio Estevam wrote:
> From: Steffen Bätz <steffen@innosonix.de>
> 
> Currently, it is not possible to run SIOCGHWTSTAMP or SIOCSHWTSTAMP
> ioctls on the dsa master interface if the port_hwtstamp_set/get()
> hooks are present, as implemented in net/dsa/master.c:
> 
> static int dsa_master_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
> {
> ...
> 	case SIOCGHWTSTAMP:
> 	case SIOCSHWTSTAMP:
> 		/* Deny PTP operations on master if there is at least one
> 		 * switch in the tree that is PTP capable.
> 		 */
> 		list_for_each_entry(dp, &dst->ports, list)
> 			if (dp->ds->ops->port_hwtstamp_get ||
> 			    dp->ds->ops->port_hwtstamp_set)
> 				return -EBUSY;
> 		break;
> 	}
> 
> Even if the hwtstamping functionality is disabled in the mv88e6xxx driver
> by not setting CONFIG_NET_DSA_MV88E6XXX_PTP, the functions port_hwtstamp_set()
> port_hwtstamp_get() are still present due to their stub declarations.
> 
> Fix this problem, by removing the stub declarations and guarding these
> functions wih CONFIG_NET_DSA_MV88E6XXX_PTP.
> 
> Without this change:
> 
>  # hwstamp_ctl -i eth0
> SIOCGHWTSTAMP failed: Device or resource busy
> 
> With the change applied, it is possible to set and get the timestamping
> options:
> 
>  # hwstamp_ctl -i eth0
> current settings:
> tx_type 0
> rx_filter 0
> 
>  # hwstamp_ctl -i eth0 -r 1 -t 1
> current settings:
> tx_type 0
> rx_filter 0
> new settings:
> tx_type 1
> rx_filter 1
> 
> Tested on a custom i.MX8MN board with a 88E6320 switch.
> 
> Signed-off-by: Steffen Bätz <steffen@innosonix.de>
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> ---

NACK.

Please extend dsa_master_ioctl() to "see through" the dp->ds->ops->port_hwtstamp_get()
pointer, similar to what dsa_port_can_configure_learning() does. Create
a fake struct ifreq, call port_hwtstamp_get(), see if it returned
-EOPNOTSUPP, and wrap that into a dsa_port_supports_hwtstamp() function,
and call that instead of the current simplistic checks for the function
pointers.