| Message ID | 20241203085248.14575-4-divya.koppera@microchip.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add ptp library for Microchip phys | expand |
On Tue, Dec 03, 2024 at 02:22:46PM +0530, Divya Koppera wrote: > Add ptp library support in Kconfig > As some of Microchip T1 phys support ptp, add dependency > of 1588 optional flag in Kconfig > > Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev> > Signed-off-by: Divya Koppera <divya.koppera@microchip.com> > --- > v4 -> v5 > Addressed below review comments. > - Indentation fix > - Changed dependency check to if check for PTP_1588_CLOCK_OPTIONAL > > v1 -> v2 -> v3 -> v4 > - No changes > --- > drivers/net/phy/Kconfig | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index 15828f4710a9..e97d389bb250 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -287,8 +287,15 @@ config MICROCHIP_PHY > > config MICROCHIP_T1_PHY > tristate "Microchip T1 PHYs" > + select MICROCHIP_PHYPTP if NETWORK_PHY_TIMESTAMPING && \ > + PTP_1588_CLOCK_OPTIONAL > help > - Supports the LAN87XX PHYs. > + Supports the LAN8XXX PHYs. > + > +config MICROCHIP_PHYPTP > + tristate "Microchip PHY PTP" > + help > + Currently supports LAN887X T1 PHY How many different PTP implementations does Microchip have? I see mscc_ptp.c, lan743x_ptp.c, lan966x_ptp.c and sparx5_ptp.c. Plus this one. Does Microchip keep reinventing the wheel? Or can this library be used in place of any of these? And how many more ptp implementations will microchip have in the future? Maybe MICROCHIP_PHYPTP is too generic, maybe you should leave space for the next PTP implementation? Andrew
Hi Andrew, > -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Wednesday, December 4, 2024 7:10 AM > To: Divya Koppera - I30481 <Divya.Koppera@microchip.com> > Cc: Arun Ramadoss - I17769 <Arun.Ramadoss@microchip.com>; > UNGLinuxDriver <UNGLinuxDriver@microchip.com>; hkallweit1@gmail.com; > linux@armlinux.org.uk; davem@davemloft.net; edumazet@google.com; > kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; richardcochran@gmail.com; > vadim.fedorenko@linux.dev > Subject: Re: [PATCH net-next v5 3/5] net: phy: Kconfig: Add ptp library support > and 1588 optional flag in Microchip phys > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > On Tue, Dec 03, 2024 at 02:22:46PM +0530, Divya Koppera wrote: > > Add ptp library support in Kconfig > > As some of Microchip T1 phys support ptp, add dependency of 1588 > > optional flag in Kconfig > > > > Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev> > > Signed-off-by: Divya Koppera <divya.koppera@microchip.com> > > --- > > v4 -> v5 > > Addressed below review comments. > > - Indentation fix > > - Changed dependency check to if check for PTP_1588_CLOCK_OPTIONAL > > > > v1 -> v2 -> v3 -> v4 > > - No changes > > --- > > drivers/net/phy/Kconfig | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index > > 15828f4710a9..e97d389bb250 100644 > > --- a/drivers/net/phy/Kconfig > > +++ b/drivers/net/phy/Kconfig > > @@ -287,8 +287,15 @@ config MICROCHIP_PHY > > > > config MICROCHIP_T1_PHY > > tristate "Microchip T1 PHYs" > > + select MICROCHIP_PHYPTP if NETWORK_PHY_TIMESTAMPING && \ > > + PTP_1588_CLOCK_OPTIONAL > > help > > - Supports the LAN87XX PHYs. > > + Supports the LAN8XXX PHYs. > > + > > +config MICROCHIP_PHYPTP > > + tristate "Microchip PHY PTP" > > + help > > + Currently supports LAN887X T1 PHY > > How many different PTP implementations does Microchip have? > > I see mscc_ptp.c, lan743x_ptp.c, lan966x_ptp.c and sparx5_ptp.c. Plus this > one. > These are MAC specific PTP. The library that we implemented is for PHYs. > Does Microchip keep reinventing the wheel? Or can this library be used in > place of any of these? As there are no register similarities between these implementations, we cannot use this library for the above mentioned MAC PTPs. >And how many more ptp implementations will > microchip have in the future? Maybe MICROCHIP_PHYPTP is too generic, > maybe you should leave space for the next PTP implementation? > Microchip plan is to use this PTP IP in future PHYs. Hence this phy library will be reused in future PHYs. > Andrew Thanks, Divya
> > How many different PTP implementations does Microchip have? > > > > I see mscc_ptp.c, lan743x_ptp.c, lan966x_ptp.c and sparx5_ptp.c. Plus this > > one. > > > > These are MAC specific PTP. The library that we implemented is for PHYs. And the difference is? Marvell has one PTP implementation they use in the PHYs and MACs in Ethernet switches. The basic core is the same, with different wrappers around it. > > Does Microchip keep reinventing the wheel? Or can this library be used in > > place of any of these? > > As there are no register similarities between these implementations, > we cannot use this library for the above mentioned MAC PTPs. > > >And how many more ptp implementations will > > microchip have in the future? Maybe MICROCHIP_PHYPTP is too generic, > > maybe you should leave space for the next PTP implementation? > Microchip plan is to use this PTP IP in future PHYs. Hence this phy > library will be reused in future PHYs. And future MACs? And has Microchip finial decided not to keep reinventing the wheel, and there will never be a new PHY implementation? I ask, because what would its KCONFIG symbol be? Andrew
On Wed, Dec 04, 2024 at 05:02:46PM +0100, Andrew Lunn wrote: > > > How many different PTP implementations does Microchip have? > > > > > > I see mscc_ptp.c, lan743x_ptp.c, lan966x_ptp.c and sparx5_ptp.c. Plus this > > > one. > > > > > > > These are MAC specific PTP. The library that we implemented is for PHYs. > > And the difference is? Marvell has one PTP implementation they use in > the PHYs and MACs in Ethernet switches. The basic core is the same, > with different wrappers around it. ... and I have code that implements a library for Marvell PTP, which is dependent on the timestamping stuff getting sorted. I tested it recently, and at least the user API wasn't working as intended in mainline kernels. When we actually get to a point where we can choose between the MAC and the PHY timestamper, I'll go back to working on that library. Sadly, that's not yet.
Hi Andrew, Thanks for your comments. > -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Wednesday, December 4, 2024 9:33 PM > To: Divya Koppera - I30481 <Divya.Koppera@microchip.com> > Cc: Arun Ramadoss - I17769 <Arun.Ramadoss@microchip.com>; > UNGLinuxDriver <UNGLinuxDriver@microchip.com>; hkallweit1@gmail.com; > linux@armlinux.org.uk; davem@davemloft.net; edumazet@google.com; > kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; richardcochran@gmail.com; > vadim.fedorenko@linux.dev > Subject: Re: [PATCH net-next v5 3/5] net: phy: Kconfig: Add ptp library support > and 1588 optional flag in Microchip phys > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > > > How many different PTP implementations does Microchip have? > > > > > > I see mscc_ptp.c, lan743x_ptp.c, lan966x_ptp.c and sparx5_ptp.c. > > > Plus this one. > > > > > > > These are MAC specific PTP. The library that we implemented is for PHYs. > > And the difference is? Marvell has one PTP implementation they use in the > PHYs and MACs in Ethernet switches. The basic core is the same, with > different wrappers around it. > MAC and PHY uses different PTP IPs. Also register space is different for different PTP IP implementations. This Microchip PTP Phy library may not be relevant for other implementations. As I mentioned earlier all future Microchip Phys will use the same IP hence Microchip PTP library will be reused. > > > Does Microchip keep reinventing the wheel? Or can this library be > > > used in place of any of these? > > > > > As there are no register similarities between these implementations, > > we cannot use this library for the above mentioned MAC PTPs. > > > > > >And how many more ptp implementations will microchip have in the > > >future? Maybe MICROCHIP_PHYPTP is too generic, maybe you should > > >leave space for the next PTP implementation? > > > Microchip plan is to use this PTP IP in future PHYs. Hence this phy > > library will be reused in future PHYs. > > And future MACs? Future MACs may use different PTP IP. > > And has Microchip finial decided not to keep reinventing the wheel, and there > will never be a new PHY implementation? I ask, because what would its > KCONFIG symbol be? > For all future Microchip PHYs PTP IP will be same, hence the implementation and kconfig symbol is under MICROCHIP_PHYPTP to keep it more generic. > Andrew Thanks, Divya
> > And has Microchip finial decided not to keep reinventing the wheel, and there > > will never be a new PHY implementation? I ask, because what would its > > KCONFIG symbol be? > > > > For all future Microchip PHYs PTP IP will be same, hence the > implementation and kconfig symbol is under MICROCHIP_PHYPTP to keep > it more generic. So you would be happy for me to NACK all new PHY PTP implementations? Are you management happy with this statement? Even if they are, i still think you need a less generic KCONFIG symbol, i doubt somebody somewhere in Microchip can resist making yet another PTP implementation. Andrew
Hi Andrew, Thanks for your comments. > -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Thursday, December 5, 2024 8:06 PM > To: Divya Koppera - I30481 <Divya.Koppera@microchip.com> > Cc: Arun Ramadoss - I17769 <Arun.Ramadoss@microchip.com>; > UNGLinuxDriver <UNGLinuxDriver@microchip.com>; hkallweit1@gmail.com; > linux@armlinux.org.uk; davem@davemloft.net; edumazet@google.com; > kuba@kernel.org; pabeni@redhat.com; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; richardcochran@gmail.com; > vadim.fedorenko@linux.dev > Subject: Re: [PATCH net-next v5 3/5] net: phy: Kconfig: Add ptp library support > and 1588 optional flag in Microchip phys > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > > > And has Microchip finial decided not to keep reinventing the wheel, > > > and there will never be a new PHY implementation? I ask, because > > > what would its KCONFIG symbol be? > > > > > > > > For all future Microchip PHYs PTP IP will be same, hence the > > implementation and kconfig symbol is under MICROCHIP_PHYPTP to keep it > > more generic. > > So you would be happy for me to NACK all new PHY PTP implementations? > > Are you management happy with this statement? > > Even if they are, i still think you need a less generic KCONFIG symbol, i doubt > somebody somewhere in Microchip can resist making yet another PTP > implementation. > I understand your point. I will change the Kconfig symbol to "MICROCHIP_PHY_RDS_PTP". RDS is internal code name to identify the PHY PTP IP. I will also change the file names, macros, and functions to reflect the internal code name as per macro. > Andrew Thanks, Divya
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 15828f4710a9..e97d389bb250 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -287,8 +287,15 @@ config MICROCHIP_PHY config MICROCHIP_T1_PHY tristate "Microchip T1 PHYs" + select MICROCHIP_PHYPTP if NETWORK_PHY_TIMESTAMPING && \ + PTP_1588_CLOCK_OPTIONAL help - Supports the LAN87XX PHYs. + Supports the LAN8XXX PHYs. + +config MICROCHIP_PHYPTP + tristate "Microchip PHY PTP" + help + Currently supports LAN887X T1 PHY config MICROSEMI_PHY tristate "Microsemi PHYs"