| Message ID | 20221110124345.3901389-1-festevam@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dsa: mv88e6xxx: Allow hwstamping on the master port | expand |
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.
Hi Vladimir, On Thu, Nov 10, 2022 at 9:53 AM Vladimir Oltean <olteanv@gmail.com> wrote: > 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. I tried to implement what you suggested in the attached patch, but I am not sure it is correct. If it doesn't work, please cook a patch so we can try it. I am not familiar with the DSA code, sorry. Thanks
On Fri, Nov 11, 2022 at 08:13:56AM -0300, Fabio Estevam wrote: > Hi Vladimir, > > On Thu, Nov 10, 2022 at 9:53 AM Vladimir Oltean <olteanv@gmail.com> wrote: > > > 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. > > I tried to implement what you suggested in the attached patch, but I > am not sure it is correct. > > If it doesn't work, please cook a patch so we can try it. Yeah, this is not what I meant. I posted a patch illustrating what I meant here: https://patchwork.kernel.org/project/netdevbpf/patch/20221111211020.543540-1-vladimir.oltean@nxp.com/
Hi Vladimir, On Fri, Nov 11, 2022 at 6:41 PM Vladimir Oltean <olteanv@gmail.com> wrote: > Yeah, this is not what I meant. I posted a patch illustrating what I > meant here: > https://patchwork.kernel.org/project/netdevbpf/patch/20221111211020.543540-1-vladimir.oltean@nxp.com/ Thanks for your patch, appreciated. I tested your patch and it works fine. Cheers
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index bf34c942db99..cfa0168cab03 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -6925,8 +6925,10 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = { .port_mirror_del = mv88e6xxx_port_mirror_del, .crosschip_bridge_join = mv88e6xxx_crosschip_bridge_join, .crosschip_bridge_leave = mv88e6xxx_crosschip_bridge_leave, +#ifdef CONFIG_NET_DSA_MV88E6XXX_PTP .port_hwtstamp_set = mv88e6xxx_port_hwtstamp_set, .port_hwtstamp_get = mv88e6xxx_port_hwtstamp_get, +#endif .port_txtstamp = mv88e6xxx_port_txtstamp, .port_rxtstamp = mv88e6xxx_port_rxtstamp, .get_ts_info = mv88e6xxx_get_ts_info, diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.h b/drivers/net/dsa/mv88e6xxx/hwtstamp.h index cf7fb6d660b1..86c6213c2ae8 100644 --- a/drivers/net/dsa/mv88e6xxx/hwtstamp.h +++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.h @@ -132,18 +132,6 @@ int mv88e6165_global_disable(struct mv88e6xxx_chip *chip); #else /* !CONFIG_NET_DSA_MV88E6XXX_PTP */ -static inline int mv88e6xxx_port_hwtstamp_set(struct dsa_switch *ds, - int port, struct ifreq *ifr) -{ - return -EOPNOTSUPP; -} - -static inline int mv88e6xxx_port_hwtstamp_get(struct dsa_switch *ds, - int port, struct ifreq *ifr) -{ - return -EOPNOTSUPP; -} - static inline bool mv88e6xxx_port_rxtstamp(struct dsa_switch *ds, int port, struct sk_buff *clone, unsigned int type)