| Message ID | 20230621165853.52273-1-ruppala@nvidia.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: Enhance fixed PHY to support 10G and 5G | expand |
On Wed, Jun 21, 2023 at 10:28:53PM +0530, Revanth Kumar Uppala wrote: > Add 10G and 5G speed entries for fixed PHY > framework.These are needed for the platforms which > doesn't have a PHY driver. > > Signed-off-by: Revanth Kumar Uppala <ruppala@nvidia.com> > Signed-off-by: Narayan Reddy <narayanr@nvidia.com> This is the second time you have sent me this patch. You have failed to answer the questions i asked you the last time..... Andrew --- pw-bot: cr
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Wednesday, June 21, 2023 11:00 PM > To: Revanth Kumar Uppala <ruppala@nvidia.com> > Cc: hkallweit1@gmail.com; netdev@vger.kernel.org; linux- > tegra@vger.kernel.org; Narayan Reddy <narayanr@nvidia.com> > Subject: Re: [PATCH] net: phy: Enhance fixed PHY to support 10G and 5G > > External email: Use caution opening links or attachments > > > On Wed, Jun 21, 2023 at 10:28:53PM +0530, Revanth Kumar Uppala wrote: > > Add 10G and 5G speed entries for fixed PHY framework.These are needed > > for the platforms which doesn't have a PHY driver. > > > > Signed-off-by: Revanth Kumar Uppala <ruppala@nvidia.com> > > Signed-off-by: Narayan Reddy <narayanr@nvidia.com> > > This is the second time you have sent me this patch. You have failed to answer > the questions i asked you the last time..... Apologies for sending twice. C45 registers are not defined in the kernel as of now. But, we need to display the speed as 5G/10G when the same is configured as fixed link in DT node. It will be great if you can share any data for handling this. As of now, with this change we have taken care of providing proper speed log in kernel when 5G/10G is added as fixed links in DT node. Thanks, Revanth Uppala > > Andrew > > --- > pw-bot: cr
On Fri, Jun 23, 2023 at 12:28:49PM +0000, Revanth Kumar Uppala wrote: > > > > -----Original Message----- > > From: Andrew Lunn <andrew@lunn.ch> > > Sent: Wednesday, June 21, 2023 11:00 PM > > To: Revanth Kumar Uppala <ruppala@nvidia.com> > > Cc: hkallweit1@gmail.com; netdev@vger.kernel.org; linux- > > tegra@vger.kernel.org; Narayan Reddy <narayanr@nvidia.com> > > Subject: Re: [PATCH] net: phy: Enhance fixed PHY to support 10G and 5G > > > > External email: Use caution opening links or attachments > > > > > > On Wed, Jun 21, 2023 at 10:28:53PM +0530, Revanth Kumar Uppala wrote: > > > Add 10G and 5G speed entries for fixed PHY framework.These are needed > > > for the platforms which doesn't have a PHY driver. > > > > > > Signed-off-by: Revanth Kumar Uppala <ruppala@nvidia.com> > > > Signed-off-by: Narayan Reddy <narayanr@nvidia.com> > > > > This is the second time you have sent me this patch. You have failed to answer > > the questions i asked you the last time..... > Apologies for sending twice. > C45 registers are not defined in the kernel as of now. But, we need to display the speed as 5G/10G when the same is configured as fixed link in DT node. > It will be great if you can share any data for handling this. > As of now, with this change we have taken care of providing proper speed log in kernel when 5G/10G is added as fixed links in DT node. This is architecturally wrong. As i said, swphy emulates a C22 PHY, and a C22 PHY does not support speeds greater than 1G. To make swphy really support 5G and 10G, you would need to add C45 support, and then extend the default genphy driver to look at the C45 registers as well. However, that is all pointless. As i said, phylink fixed-link is not limited to 1G speeds. Given what i see in Cc: i assume this is for a tegre SoC? And that uses a Synopsys MAC? So you probably want to modify the dwc driver to use phylink. Andrew
On Fri, Jun 23, 2023 at 03:29:13PM +0200, Andrew Lunn wrote: > On Fri, Jun 23, 2023 at 12:28:49PM +0000, Revanth Kumar Uppala wrote: > > > > > > > -----Original Message----- > > > From: Andrew Lunn <andrew@lunn.ch> > > > Sent: Wednesday, June 21, 2023 11:00 PM > > > To: Revanth Kumar Uppala <ruppala@nvidia.com> > > > Cc: hkallweit1@gmail.com; netdev@vger.kernel.org; linux- > > > tegra@vger.kernel.org; Narayan Reddy <narayanr@nvidia.com> > > > Subject: Re: [PATCH] net: phy: Enhance fixed PHY to support 10G and 5G > > > > > > External email: Use caution opening links or attachments > > > > > > > > > On Wed, Jun 21, 2023 at 10:28:53PM +0530, Revanth Kumar Uppala wrote: > > > > Add 10G and 5G speed entries for fixed PHY framework.These are needed > > > > for the platforms which doesn't have a PHY driver. > > > > > > > > Signed-off-by: Revanth Kumar Uppala <ruppala@nvidia.com> > > > > Signed-off-by: Narayan Reddy <narayanr@nvidia.com> > > > > > > This is the second time you have sent me this patch. You have failed to answer > > > the questions i asked you the last time..... > > Apologies for sending twice. > > C45 registers are not defined in the kernel as of now. But, we need to display the speed as 5G/10G when the same is configured as fixed link in DT node. > > It will be great if you can share any data for handling this. > > As of now, with this change we have taken care of providing proper speed log in kernel when 5G/10G is added as fixed links in DT node. > > This is architecturally wrong. As i said, swphy emulates a C22 PHY, > and a C22 PHY does not support speeds greater than 1G. To make swphy > really support 5G and 10G, you would need to add C45 support, and then > extend the default genphy driver to look at the C45 registers as well. > > However, that is all pointless. As i said, phylink fixed-link is not > limited to 1G speeds. Given what i see in Cc: i assume this is for a > tegre SoC? And that uses a Synopsys MAC? So you probably want to > modify the dwc driver to use phylink. Absolutely correct. I seem to recall having had this come up before, and I think it was explained at the time, but I don't seem to find anything in my "sent" mailboxes for the start of 2022 to present. (To nvidia) The classical swphy/fixed-phy offers a software emulated clause 22 PHY so that phylib can be re-used to make a fixed link work without needing special code paths in phylib nor in MAC drivers. However, clause 22 PHYs do not support speeds in excess of 1G, so this places a hard ceiling on the speed that can be supported with this method. PHYLIB's clause 45 support is specific to vendor PHYs, and the "generic" implementation only supports 10G speed. Emulating a specific vendor PHY to achieve this old way of supporting fixed links when we have a better way is really a waste of effort. The "better way" is phylink, which makes fixed links work without needing to resort to PHY emulation, and thus it can support any speed that a MAC happens to support. This is the modern way. We (the phylib and phylink maintainers) will not entertain extending the old now legacy method using swphy/fixed-phy for fixed links to include any faster speeds. Thanks.
> -----Original Message----- > From: Russell King <linux@armlinux.org.uk> > Sent: Friday, June 23, 2023 7:25 PM > To: Andrew Lunn <andrew@lunn.ch>; Revanth Kumar Uppala > <ruppala@nvidia.com>; Narayan Reddy <narayanr@nvidia.com> > Cc: hkallweit1@gmail.com; netdev@vger.kernel.org; linux- > tegra@vger.kernel.org > Subject: Re: [PATCH] net: phy: Enhance fixed PHY to support 10G and 5G > > External email: Use caution opening links or attachments > > > On Fri, Jun 23, 2023 at 03:29:13PM +0200, Andrew Lunn wrote: > > On Fri, Jun 23, 2023 at 12:28:49PM +0000, Revanth Kumar Uppala wrote: > > > > > > > > > > -----Original Message----- > > > > From: Andrew Lunn <andrew@lunn.ch> > > > > Sent: Wednesday, June 21, 2023 11:00 PM > > > > To: Revanth Kumar Uppala <ruppala@nvidia.com> > > > > Cc: hkallweit1@gmail.com; netdev@vger.kernel.org; linux- > > > > tegra@vger.kernel.org; Narayan Reddy <narayanr@nvidia.com> > > > > Subject: Re: [PATCH] net: phy: Enhance fixed PHY to support 10G > > > > and 5G > > > > > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > On Wed, Jun 21, 2023 at 10:28:53PM +0530, Revanth Kumar Uppala wrote: > > > > > Add 10G and 5G speed entries for fixed PHY framework.These are > > > > > needed for the platforms which doesn't have a PHY driver. > > > > > > > > > > Signed-off-by: Revanth Kumar Uppala <ruppala@nvidia.com> > > > > > Signed-off-by: Narayan Reddy <narayanr@nvidia.com> > > > > > > > > This is the second time you have sent me this patch. You have > > > > failed to answer the questions i asked you the last time..... > > > Apologies for sending twice. > > > C45 registers are not defined in the kernel as of now. But, we need to display > the speed as 5G/10G when the same is configured as fixed link in DT node. > > > It will be great if you can share any data for handling this. > > > As of now, with this change we have taken care of providing proper speed > log in kernel when 5G/10G is added as fixed links in DT node. > > > > This is architecturally wrong. As i said, swphy emulates a C22 PHY, > > and a C22 PHY does not support speeds greater than 1G. To make swphy > > really support 5G and 10G, you would need to add C45 support, and then > > extend the default genphy driver to look at the C45 registers as well. > > > > However, that is all pointless. As i said, phylink fixed-link is not > > limited to 1G speeds. Given what i see in Cc: i assume this is for a > > tegre SoC? And that uses a Synopsys MAC? So you probably want to > > modify the dwc driver to use phylink. > > Absolutely correct. I seem to recall having had this come up before, and I think it > was explained at the time, but I don't seem to find anything in my "sent" > mailboxes for the start of 2022 to present. > > (To nvidia) > > The classical swphy/fixed-phy offers a software emulated clause 22 PHY so that > phylib can be re-used to make a fixed link work without needing special code > paths in phylib nor in MAC drivers. > > However, clause 22 PHYs do not support speeds in excess of 1G, so this places a > hard ceiling on the speed that can be supported with this method. PHYLIB's > clause 45 support is specific to vendor PHYs, and the "generic" implementation > only supports 10G speed. Emulating a specific vendor PHY to achieve this old > way of supporting fixed links when we have a better way is really a waste of > effort. > > The "better way" is phylink, which makes fixed links work without needing to > resort to PHY emulation, and thus it can support any speed that a MAC happens > to support. This is the modern way. We will look into what you have suggested. Thanks for the feedback, Revanth Uppala > > We (the phylib and phylink maintainers) will not entertain extending the old now > legacy method using swphy/fixed-phy for fixed links to include any faster > speeds. > > Thanks. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
diff --git a/drivers/net/phy/swphy.c b/drivers/net/phy/swphy.c index 59f1ba4d49bc..5380f952e846 100644 --- a/drivers/net/phy/swphy.c +++ b/drivers/net/phy/swphy.c @@ -8,6 +8,7 @@ * Anton Vorontsov <avorontsov@ru.mvista.com> * * Copyright (c) 2006-2007 MontaVista Software, Inc. + * Copyright (c) 2023, NVIDIA CORPORATION. All rights reserved. */ #include <linux/export.h> #include <linux/mii.h> @@ -29,6 +30,8 @@ enum { SWMII_SPEED_10 = 0, SWMII_SPEED_100, SWMII_SPEED_1000, + SWMII_SPEED_5000, + SWMII_SPEED_10000, SWMII_DUPLEX_HALF = 0, SWMII_DUPLEX_FULL, }; @@ -51,6 +54,10 @@ static const struct swmii_regs speed[] = { .lpagb = LPA_1000FULL | LPA_1000HALF, .estat = ESTATUS_1000_TFULL | ESTATUS_1000_THALF, }, + [SWMII_SPEED_5000] = { + }, + [SWMII_SPEED_10000] = { + }, }; static const struct swmii_regs duplex[] = { @@ -71,6 +78,10 @@ static const struct swmii_regs duplex[] = { static int swphy_decode_speed(int speed) { switch (speed) { + case 10000: + return SWMII_SPEED_10000; + case 5000: + return SWMII_SPEED_5000; case 1000: return SWMII_SPEED_1000; case 100: