| Message ID | 20230713121907.3249291-1-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Introduce ndo_hwtstamp_get() and ndo_hwtstamp_set() | expand |
On 7/13/2023 5:18 AM, Vladimir Oltean wrote: > Based on previous RFCs from Maxim Georgiev: > https://lore.kernel.org/netdev/20230502043150.17097-1-glipus@gmail.com/ > > this series attempts to introduce new API for the hardware timestamping > control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling). > > I don't have any board with phylib hardware timestamping, so I would > appreciate testing (especially on lan966x, the most intricate > conversion). I was, however, able to test netdev level timestamping, > because I also have some more unsubmitted conversions in progress: > > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7 > > I hope that the concerns expressed in the comments of previous series > were addressed, and that Köry Maincent's series: > https://lore.kernel.org/netdev/20230406173308.401924-1-kory.maincent@bootlin.com/ > can make progress in parallel with the conversion of the rest of drivers. > This series looks good to me, nice cleanup and reducing some boiler plate code is excellent. I'd like to convert the Intel drivers too, but I am not sure when I can commit to doing that as I have a lot on my plate presently. Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > Maxim Georgiev (5): > net: add NDOs for configuring hardware timestamping > net: add hwtstamping helpers for stackable net devices > net: vlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set() > net: macvlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set() > net: bonding: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set() > > Vladimir Oltean (5): > net: fec: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() > net: fec: delete fec_ptp_disable_hwts() > net: sparx5: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() > net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() > net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from > converted drivers > > drivers/net/bonding/bond_main.c | 105 ++++++---- > drivers/net/ethernet/freescale/fec.h | 6 +- > drivers/net/ethernet/freescale/fec_main.c | 41 ++-- > drivers/net/ethernet/freescale/fec_ptp.c | 43 ++-- > .../ethernet/microchip/lan966x/lan966x_main.c | 61 ++++-- > .../ethernet/microchip/lan966x/lan966x_main.h | 12 +- > .../ethernet/microchip/lan966x/lan966x_ptp.c | 34 ++-- > .../ethernet/microchip/sparx5/sparx5_main.h | 9 +- > .../ethernet/microchip/sparx5/sparx5_netdev.c | 35 +++- > .../ethernet/microchip/sparx5/sparx5_ptp.c | 24 ++- > drivers/net/macvlan.c | 34 ++-- > include/linux/net_tstamp.h | 28 +++ > include/linux/netdevice.h | 25 +++ > net/8021q/vlan_dev.c | 27 ++- > net/core/dev_ioctl.c | 183 +++++++++++++++++- > 15 files changed, 480 insertions(+), 187 deletions(-) >
On Thu, Jul 13, 2023 at 02:50:39PM -0700, Jacob Keller wrote: > On 7/13/2023 5:18 AM, Vladimir Oltean wrote: > > Based on previous RFCs from Maxim Georgiev: > > https://lore.kernel.org/netdev/20230502043150.17097-1-glipus@gmail.com/ > > > > this series attempts to introduce new API for the hardware timestamping > > control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling). > > > > I don't have any board with phylib hardware timestamping, so I would > > appreciate testing (especially on lan966x, the most intricate > > conversion). I was, however, able to test netdev level timestamping, > > because I also have some more unsubmitted conversions in progress: > > > > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7 > > > > I hope that the concerns expressed in the comments of previous series > > were addressed, and that Köry Maincent's series: > > https://lore.kernel.org/netdev/20230406173308.401924-1-kory.maincent@bootlin.com/ > > can make progress in parallel with the conversion of the rest of drivers. > > > > This series looks good to me, nice cleanup and reducing some boiler > plate code is excellent. > > I'd like to convert the Intel drivers too, but I am not sure when I can > commit to doing that as I have a lot on my plate presently. > > Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Thanks for the review. The conversion of Intel drivers is in the Github link I had posted.
On 7/13/2023 3:33 PM, Vladimir Oltean wrote: > On Thu, Jul 13, 2023 at 02:50:39PM -0700, Jacob Keller wrote: >> On 7/13/2023 5:18 AM, Vladimir Oltean wrote: >>> Based on previous RFCs from Maxim Georgiev: >>> https://lore.kernel.org/netdev/20230502043150.17097-1-glipus@gmail.com/ >>> >>> this series attempts to introduce new API for the hardware timestamping >>> control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling). >>> >>> I don't have any board with phylib hardware timestamping, so I would >>> appreciate testing (especially on lan966x, the most intricate >>> conversion). I was, however, able to test netdev level timestamping, >>> because I also have some more unsubmitted conversions in progress: >>> >>> https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7 >>> >>> I hope that the concerns expressed in the comments of previous series >>> were addressed, and that Köry Maincent's series: >>> https://lore.kernel.org/netdev/20230406173308.401924-1-kory.maincent@bootlin.com/ >>> can make progress in parallel with the conversion of the rest of drivers. >>> >> >> This series looks good to me, nice cleanup and reducing some boiler >> plate code is excellent. >> >> I'd like to convert the Intel drivers too, but I am not sure when I can >> commit to doing that as I have a lot on my plate presently. >> >> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > > Thanks for the review. The conversion of Intel drivers is in the Github > link I had posted. Oh nice!
The 07/13/2023 15:18, Vladimir Oltean wrote: Hi Vladimir, > > Based on previous RFCs from Maxim Georgiev: > https://lore.kernel.org/netdev/20230502043150.17097-1-glipus@gmail.com/ > > this series attempts to introduce new API for the hardware timestamping > control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling). > > I don't have any board with phylib hardware timestamping, so I would > appreciate testing (especially on lan966x, the most intricate > conversion). I was, however, able to test netdev level timestamping, > because I also have some more unsubmitted conversions in progress: > > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7 I have tested this patch series on lan966x. In both cases, when there is a PHY that supports HW timestamping and when the isn't a PHY that supports HW timestamping. In both cases it behaves as expected. Thanks! Tested-by: Horatiu Vultur <horatiu.vultur@microchip.com> > > I hope that the concerns expressed in the comments of previous series > were addressed, and that Köry Maincent's series: > https://lore.kernel.org/netdev/20230406173308.401924-1-kory.maincent@bootlin.com/ > can make progress in parallel with the conversion of the rest of drivers. > > Maxim Georgiev (5): > net: add NDOs for configuring hardware timestamping > net: add hwtstamping helpers for stackable net devices > net: vlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set() > net: macvlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set() > net: bonding: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set() > > Vladimir Oltean (5): > net: fec: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() > net: fec: delete fec_ptp_disable_hwts() > net: sparx5: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() > net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() > net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from > converted drivers > > drivers/net/bonding/bond_main.c | 105 ++++++---- > drivers/net/ethernet/freescale/fec.h | 6 +- > drivers/net/ethernet/freescale/fec_main.c | 41 ++-- > drivers/net/ethernet/freescale/fec_ptp.c | 43 ++-- > .../ethernet/microchip/lan966x/lan966x_main.c | 61 ++++-- > .../ethernet/microchip/lan966x/lan966x_main.h | 12 +- > .../ethernet/microchip/lan966x/lan966x_ptp.c | 34 ++-- > .../ethernet/microchip/sparx5/sparx5_main.h | 9 +- > .../ethernet/microchip/sparx5/sparx5_netdev.c | 35 +++- > .../ethernet/microchip/sparx5/sparx5_ptp.c | 24 ++- > drivers/net/macvlan.c | 34 ++-- > include/linux/net_tstamp.h | 28 +++ > include/linux/netdevice.h | 25 +++ > net/8021q/vlan_dev.c | 27 ++- > net/core/dev_ioctl.c | 183 +++++++++++++++++- > 15 files changed, 480 insertions(+), 187 deletions(-) > > -- > 2.34.1 >
On Thu, Jul 13, 2023 at 6:19 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > Based on previous RFCs from Maxim Georgiev: > https://lore.kernel.org/netdev/20230502043150.17097-1-glipus@gmail.com/ > > this series attempts to introduce new API for the hardware timestamping > control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling). > > I don't have any board with phylib hardware timestamping, so I would > appreciate testing (especially on lan966x, the most intricate > conversion). I was, however, able to test netdev level timestamping, > because I also have some more unsubmitted conversions in progress: > > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7 > > I hope that the concerns expressed in the comments of previous series > were addressed, and that Köry Maincent's series: > https://lore.kernel.org/netdev/20230406173308.401924-1-kory.maincent@bootlin.com/ > can make progress in parallel with the conversion of the rest of drivers. > > Maxim Georgiev (5): > net: add NDOs for configuring hardware timestamping > net: add hwtstamping helpers for stackable net devices > net: vlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set() > net: macvlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set() > net: bonding: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set() > > Vladimir Oltean (5): > net: fec: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() > net: fec: delete fec_ptp_disable_hwts() > net: sparx5: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() > net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() > net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from > converted drivers > > drivers/net/bonding/bond_main.c | 105 ++++++---- > drivers/net/ethernet/freescale/fec.h | 6 +- > drivers/net/ethernet/freescale/fec_main.c | 41 ++-- > drivers/net/ethernet/freescale/fec_ptp.c | 43 ++-- > .../ethernet/microchip/lan966x/lan966x_main.c | 61 ++++-- > .../ethernet/microchip/lan966x/lan966x_main.h | 12 +- > .../ethernet/microchip/lan966x/lan966x_ptp.c | 34 ++-- > .../ethernet/microchip/sparx5/sparx5_main.h | 9 +- > .../ethernet/microchip/sparx5/sparx5_netdev.c | 35 +++- > .../ethernet/microchip/sparx5/sparx5_ptp.c | 24 ++- > drivers/net/macvlan.c | 34 ++-- > include/linux/net_tstamp.h | 28 +++ > include/linux/netdevice.h | 25 +++ > net/8021q/vlan_dev.c | 27 ++- > net/core/dev_ioctl.c | 183 +++++++++++++++++- > 15 files changed, 480 insertions(+), 187 deletions(-) > > -- > 2.34.1 > Vladimir, thank you for taking over and improving this patch stack! I see you dropped the netdevsim patch: https://www.spinics.net/lists/netdev/msg901378.html Do you believe it's not useful any more since the rest of the patches in the stack were tested through other means?
Hi Maxim, On Sun, Jul 16, 2023 at 07:22:23PM -0600, Max Georgiev wrote: > Vladimir, thank you for taking over and improving this patch stack! > > I see you dropped the netdevsim patch: > https://www.spinics.net/lists/netdev/msg901378.html > Do you believe it's not useful any more since the rest of the > patches in the stack were tested through other means? I just didn't consider that adding mock hardware timestamping support to netdevsim was necessary or useful, considering the number of other driver conversions that will have to be submitted. Just an extra, avoidable effort for me.
On Fri, Jul 14, 2023 at 10:00:59AM +0200, Horatiu Vultur wrote: > The 07/13/2023 15:18, Vladimir Oltean wrote: > > Hi Vladimir, > > > > > Based on previous RFCs from Maxim Georgiev: > > https://lore.kernel.org/netdev/20230502043150.17097-1-glipus@gmail.com/ > > > > this series attempts to introduce new API for the hardware timestamping > > control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling). > > > > I don't have any board with phylib hardware timestamping, so I would > > appreciate testing (especially on lan966x, the most intricate > > conversion). I was, however, able to test netdev level timestamping, > > because I also have some more unsubmitted conversions in progress: > > > > https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7 > > I have tested this patch series on lan966x. In both cases, when there is > a PHY that supports HW timestamping and when the isn't a PHY that > supports HW timestamping. In both cases it behaves as expected. Thanks! > Tested-by: Horatiu Vultur <horatiu.vultur@microchip.com> Thanks for testing! I'll apply this tag to patches: net: add NDOs for configuring hardware timestamping net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from converted drivers
On 7/17/2023 4:25 AM, Vladimir Oltean wrote: > Hi Maxim, > > On Sun, Jul 16, 2023 at 07:22:23PM -0600, Max Georgiev wrote: >> Vladimir, thank you for taking over and improving this patch stack! >> >> I see you dropped the netdevsim patch: >> https://www.spinics.net/lists/netdev/msg901378.html >> Do you believe it's not useful any more since the rest of the >> patches in the stack were tested through other means? > > I just didn't consider that adding mock hardware timestamping support to > netdevsim was necessary or useful, considering the number of other driver > conversions that will have to be submitted. Just an extra, avoidable effort > for me. FWIW I think its unnecessary as well. I read through the implementation and noticed that it also used the .get_ts_info callback by directly reporting whatever type and filter was set via SIOCSHWTSTAMP, rather than reporting some device capability. Obviously as a mock device there is no real capability, and that was likely done for testing purposes. However, it would still leave the kernel with an implementation that does not follow the expected rules for these ioctls. For a mock device thats not really an issue. However, I'd prefer to avoid such in the kernel so that its not available for copying when someone without such knowledge comes along to write a new driver.
On Mon, Jul 17, 2023 at 01:23:02PM -0700, Jacob Keller wrote: > For a mock device thats not really an issue. However, I'd prefer to > avoid such in the kernel so that its not available for copying when > someone without such knowledge comes along to write a new driver. +1
Based on previous RFCs from Maxim Georgiev: https://lore.kernel.org/netdev/20230502043150.17097-1-glipus@gmail.com/ this series attempts to introduce new API for the hardware timestamping control path (SIOCGHWTSTAMP and SIOCSHWTSTAMP handling). I don't have any board with phylib hardware timestamping, so I would appreciate testing (especially on lan966x, the most intricate conversion). I was, however, able to test netdev level timestamping, because I also have some more unsubmitted conversions in progress: https://github.com/vladimiroltean/linux/commits/ndo-hwtstamp-v7 I hope that the concerns expressed in the comments of previous series were addressed, and that Köry Maincent's series: https://lore.kernel.org/netdev/20230406173308.401924-1-kory.maincent@bootlin.com/ can make progress in parallel with the conversion of the rest of drivers. Maxim Georgiev (5): net: add NDOs for configuring hardware timestamping net: add hwtstamping helpers for stackable net devices net: vlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set() net: macvlan: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set() net: bonding: convert to ndo_hwtstamp_get() / ndo_hwtstamp_set() Vladimir Oltean (5): net: fec: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() net: fec: delete fec_ptp_disable_hwts() net: sparx5: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set() net: remove phy_has_hwtstamp() -> phy_mii_ioctl() decision from converted drivers drivers/net/bonding/bond_main.c | 105 ++++++---- drivers/net/ethernet/freescale/fec.h | 6 +- drivers/net/ethernet/freescale/fec_main.c | 41 ++-- drivers/net/ethernet/freescale/fec_ptp.c | 43 ++-- .../ethernet/microchip/lan966x/lan966x_main.c | 61 ++++-- .../ethernet/microchip/lan966x/lan966x_main.h | 12 +- .../ethernet/microchip/lan966x/lan966x_ptp.c | 34 ++-- .../ethernet/microchip/sparx5/sparx5_main.h | 9 +- .../ethernet/microchip/sparx5/sparx5_netdev.c | 35 +++- .../ethernet/microchip/sparx5/sparx5_ptp.c | 24 ++- drivers/net/macvlan.c | 34 ++-- include/linux/net_tstamp.h | 28 +++ include/linux/netdevice.h | 25 +++ net/8021q/vlan_dev.c | 27 ++- net/core/dev_ioctl.c | 183 +++++++++++++++++- 15 files changed, 480 insertions(+), 187 deletions(-)