| Message ID | 20220127102333.987195-5-horatiu.vultur@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: lan966x: Add PTP Hardward Clock support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 160 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Thu, Jan 27, 2022 at 11:23:30AM +0100, Horatiu Vultur wrote: > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c b/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c > index 69d8f43e2b1b..9ff4d3fca5a1 100644 > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c > @@ -35,6 +35,90 @@ static u64 lan966x_ptp_get_nominal_value(void) > return res; > } > > +int lan966x_ptp_hwtstamp_set(struct lan966x_port *port, struct ifreq *ifr) > +{ > + struct lan966x *lan966x = port->lan966x; > + bool l2 = false, l4 = false; > + struct hwtstamp_config cfg; > + struct lan966x_phc *phc; > + > + /* For now don't allow to run ptp on ports that are part of a bridge, > + * because in case of transparent clock the HW will still forward the > + * frames, so there would be duplicate frames > + */ > + if (lan966x->bridge_mask & BIT(port->chip_port)) > + return -EINVAL; > + > + if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg))) > + return -EFAULT; > + > + switch (cfg.tx_type) { > + case HWTSTAMP_TX_ON: > + port->ptp_cmd = IFH_REW_OP_TWO_STEP_PTP; > + break; > + case HWTSTAMP_TX_ONESTEP_SYNC: > + port->ptp_cmd = IFH_REW_OP_ONE_STEP_PTP; > + break; > + case HWTSTAMP_TX_OFF: > + port->ptp_cmd = IFH_REW_OP_NOOP; > + break; > + default: > + return -ERANGE; > + } > + > + mutex_lock(&lan966x->ptp_lock); No need to lock stack variables. Move locking down to ... > + switch (cfg.rx_filter) { > + case HWTSTAMP_FILTER_NONE: > + break; > + case HWTSTAMP_FILTER_PTP_V2_L4_EVENT: > + case HWTSTAMP_FILTER_PTP_V2_L4_SYNC: > + case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ: > + l4 = true; > + break; > + case HWTSTAMP_FILTER_PTP_V2_L2_EVENT: > + case HWTSTAMP_FILTER_PTP_V2_L2_SYNC: > + case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ: > + l2 = true; > + break; > + case HWTSTAMP_FILTER_PTP_V2_EVENT: > + case HWTSTAMP_FILTER_PTP_V2_SYNC: > + case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ: > + l2 = true; > + l4 = true; > + break; > + default: > + mutex_unlock(&lan966x->ptp_lock); > + return -ERANGE; > + } > + > + if (l2 && l4) > + cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT; > + else if (l2) > + cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT; > + else if (l4) > + cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT; > + else > + cfg.rx_filter = HWTSTAMP_FILTER_NONE; > + > + /* Commit back the result & save it */ ... here > + phc = &lan966x->phc[LAN966X_PHC_PORT]; > + memcpy(&phc->hwtstamp_config, &cfg, sizeof(cfg)); > + mutex_unlock(&lan966x->ptp_lock); > + > + return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0; > +} Thanks, Richard
The 01/27/2022 13:55, Richard Cochran wrote: > > On Thu, Jan 27, 2022 at 11:23:30AM +0100, Horatiu Vultur wrote: > > > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c b/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c > > index 69d8f43e2b1b..9ff4d3fca5a1 100644 > > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c > > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c > > @@ -35,6 +35,90 @@ static u64 lan966x_ptp_get_nominal_value(void) > > return res; > > } > > > > +int lan966x_ptp_hwtstamp_set(struct lan966x_port *port, struct ifreq *ifr) > > +{ > > + struct lan966x *lan966x = port->lan966x; > > + bool l2 = false, l4 = false; > > + struct hwtstamp_config cfg; > > + struct lan966x_phc *phc; > > + > > + /* For now don't allow to run ptp on ports that are part of a bridge, > > + * because in case of transparent clock the HW will still forward the > > + * frames, so there would be duplicate frames > > + */ > > + if (lan966x->bridge_mask & BIT(port->chip_port)) > > + return -EINVAL; > > + > > + if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg))) > > + return -EFAULT; > > + > > + switch (cfg.tx_type) { > > + case HWTSTAMP_TX_ON: > > + port->ptp_cmd = IFH_REW_OP_TWO_STEP_PTP; > > + break; > > + case HWTSTAMP_TX_ONESTEP_SYNC: > > + port->ptp_cmd = IFH_REW_OP_ONE_STEP_PTP; > > + break; > > + case HWTSTAMP_TX_OFF: > > + port->ptp_cmd = IFH_REW_OP_NOOP; > > + break; > > + default: > > + return -ERANGE; > > + } > > + > > + mutex_lock(&lan966x->ptp_lock); > > No need to lock stack variables. Move locking down to ... Good catch, will do that. > > > + switch (cfg.rx_filter) { > > + case HWTSTAMP_FILTER_NONE: > > + break; > > + case HWTSTAMP_FILTER_PTP_V2_L4_EVENT: > > + case HWTSTAMP_FILTER_PTP_V2_L4_SYNC: > > + case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ: > > + l4 = true; > > + break; > > + case HWTSTAMP_FILTER_PTP_V2_L2_EVENT: > > + case HWTSTAMP_FILTER_PTP_V2_L2_SYNC: > > + case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ: > > + l2 = true; > > + break; > > + case HWTSTAMP_FILTER_PTP_V2_EVENT: > > + case HWTSTAMP_FILTER_PTP_V2_SYNC: > > + case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ: > > + l2 = true; > > + l4 = true; > > + break; > > + default: > > + mutex_unlock(&lan966x->ptp_lock); > > + return -ERANGE; > > + } > > + > > + if (l2 && l4) > > + cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT; > > + else if (l2) > > + cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT; > > + else if (l4) > > + cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT; > > + else > > + cfg.rx_filter = HWTSTAMP_FILTER_NONE; > > + > > + /* Commit back the result & save it */ > > ... here > > > + phc = &lan966x->phc[LAN966X_PHC_PORT]; > > + memcpy(&phc->hwtstamp_config, &cfg, sizeof(cfg)); > > + mutex_unlock(&lan966x->ptp_lock); > > + > > + return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0; > > +} > > Thanks, > Richard
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c index ee3505318c5c..c62615b9d101 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c @@ -351,6 +351,23 @@ static int lan966x_port_get_parent_id(struct net_device *dev, return 0; } +static int lan966x_port_ioctl(struct net_device *dev, struct ifreq *ifr, + int cmd) +{ + struct lan966x_port *port = netdev_priv(dev); + + if (!phy_has_hwtstamp(dev->phydev) && port->lan966x->ptp) { + switch (cmd) { + case SIOCSHWTSTAMP: + return lan966x_ptp_hwtstamp_set(port, ifr); + case SIOCGHWTSTAMP: + return lan966x_ptp_hwtstamp_get(port, ifr); + } + } + + return phy_mii_ioctl(dev->phydev, ifr, cmd); +} + static const struct net_device_ops lan966x_port_netdev_ops = { .ndo_open = lan966x_port_open, .ndo_stop = lan966x_port_stop, @@ -361,6 +378,7 @@ static const struct net_device_ops lan966x_port_netdev_ops = { .ndo_get_stats64 = lan966x_stats_get, .ndo_set_mac_address = lan966x_port_set_mac_address, .ndo_get_port_parent_id = lan966x_port_get_parent_id, + .ndo_eth_ioctl = lan966x_port_ioctl, }; bool lan966x_netdevice_check(const struct net_device *dev) diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h index c77a91aa24e7..55fa5e56b8d1 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h @@ -54,6 +54,10 @@ #define LAN966X_PHC_COUNT 3 #define LAN966X_PHC_PORT 0 +#define IFH_REW_OP_NOOP 0x0 +#define IFH_REW_OP_ONE_STEP_PTP 0x3 +#define IFH_REW_OP_TWO_STEP_PTP 0x4 + /* MAC table entry types. * ENTRYTYPE_NORMAL is subject to aging. * ENTRYTYPE_LOCKED is not subject to aging. @@ -130,6 +134,7 @@ struct lan966x { bool ptp; struct lan966x_phc phc[LAN966X_PHC_COUNT]; spinlock_t ptp_clock_lock; /* lock for phc */ + struct mutex ptp_lock; /* lock for ptp interface state */ }; struct lan966x_port_config { @@ -159,6 +164,8 @@ struct lan966x_port { struct phylink *phylink; struct phy *serdes; struct fwnode_handle *fwnode; + + u8 ptp_cmd; }; extern const struct phylink_mac_ops lan966x_phylink_mac_ops; @@ -247,6 +254,8 @@ void lan966x_mdb_write_entries(struct lan966x *lan966x, u16 vid); int lan966x_ptp_init(struct lan966x *lan966x); void lan966x_ptp_deinit(struct lan966x *lan966x); +int lan966x_ptp_hwtstamp_set(struct lan966x_port *port, struct ifreq *ifr); +int lan966x_ptp_hwtstamp_get(struct lan966x_port *port, struct ifreq *ifr); static inline void __iomem *lan_addr(void __iomem *base[], int id, int tinst, int tcnt, diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c b/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c index 69d8f43e2b1b..9ff4d3fca5a1 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_ptp.c @@ -35,6 +35,90 @@ static u64 lan966x_ptp_get_nominal_value(void) return res; } +int lan966x_ptp_hwtstamp_set(struct lan966x_port *port, struct ifreq *ifr) +{ + struct lan966x *lan966x = port->lan966x; + bool l2 = false, l4 = false; + struct hwtstamp_config cfg; + struct lan966x_phc *phc; + + /* For now don't allow to run ptp on ports that are part of a bridge, + * because in case of transparent clock the HW will still forward the + * frames, so there would be duplicate frames + */ + if (lan966x->bridge_mask & BIT(port->chip_port)) + return -EINVAL; + + if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg))) + return -EFAULT; + + switch (cfg.tx_type) { + case HWTSTAMP_TX_ON: + port->ptp_cmd = IFH_REW_OP_TWO_STEP_PTP; + break; + case HWTSTAMP_TX_ONESTEP_SYNC: + port->ptp_cmd = IFH_REW_OP_ONE_STEP_PTP; + break; + case HWTSTAMP_TX_OFF: + port->ptp_cmd = IFH_REW_OP_NOOP; + break; + default: + return -ERANGE; + } + + mutex_lock(&lan966x->ptp_lock); + + switch (cfg.rx_filter) { + case HWTSTAMP_FILTER_NONE: + break; + case HWTSTAMP_FILTER_PTP_V2_L4_EVENT: + case HWTSTAMP_FILTER_PTP_V2_L4_SYNC: + case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ: + l4 = true; + break; + case HWTSTAMP_FILTER_PTP_V2_L2_EVENT: + case HWTSTAMP_FILTER_PTP_V2_L2_SYNC: + case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ: + l2 = true; + break; + case HWTSTAMP_FILTER_PTP_V2_EVENT: + case HWTSTAMP_FILTER_PTP_V2_SYNC: + case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ: + l2 = true; + l4 = true; + break; + default: + mutex_unlock(&lan966x->ptp_lock); + return -ERANGE; + } + + if (l2 && l4) + cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT; + else if (l2) + cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT; + else if (l4) + cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT; + else + cfg.rx_filter = HWTSTAMP_FILTER_NONE; + + /* Commit back the result & save it */ + phc = &lan966x->phc[LAN966X_PHC_PORT]; + memcpy(&phc->hwtstamp_config, &cfg, sizeof(cfg)); + mutex_unlock(&lan966x->ptp_lock); + + return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0; +} + +int lan966x_ptp_hwtstamp_get(struct lan966x_port *port, struct ifreq *ifr) +{ + struct lan966x *lan966x = port->lan966x; + struct lan966x_phc *phc; + + phc = &lan966x->phc[LAN966X_PHC_PORT]; + return copy_to_user(ifr->ifr_data, &phc->hwtstamp_config, + sizeof(phc->hwtstamp_config)) ? -EFAULT : 0; +} + static int lan966x_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm) { struct lan966x_phc *phc = container_of(ptp, struct lan966x_phc, info); @@ -252,6 +336,7 @@ int lan966x_ptp_init(struct lan966x *lan966x) } spin_lock_init(&lan966x->ptp_clock_lock); + mutex_init(&lan966x->ptp_lock); /* Disable master counters */ lan_wr(PTP_DOM_CFG_ENA_SET(0), lan966x, PTP_DOM_CFG);
Implement the ioctl callbacks SIOCSHWTSTAMP and SIOCGHWTSTAMP to allow to configure the ports to enable/disable timestamping. The HW is capable to run both 1-step timestamping and 2-step timestamping. Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> --- .../ethernet/microchip/lan966x/lan966x_main.c | 18 ++++ .../ethernet/microchip/lan966x/lan966x_main.h | 9 ++ .../ethernet/microchip/lan966x/lan966x_ptp.c | 85 +++++++++++++++++++ 3 files changed, 112 insertions(+)