Message ID | 20210604161250.49749-1-lxu@maxlinear.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [v3] net: phy: add Maxlinear GPY115/21x/24x driver | expand |
Context | Check | Description |
---|---|---|
netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On 6/4/2021 9:12 AM, Xu Liang wrote: > Add driver to support the Maxlinear GPY115, GPY211, GPY212, GPY215, > GPY241, GPY245 PHYs. > > Signed-off-by: Xu Liang <lxu@maxlinear.com> > --- > v2 changes: > Fix format warning from checkpath and some comments. > Use smaller PHY ID mask. > Split FWV register mask. > Call phy_trigger_machine if necessary when clear interrupt. > v3 changes: > Replace unnecessary phy_modify_mmd_changed with phy_modify_mmd > Move firmware version print to probe. > > MAINTAINERS | 6 + > drivers/net/phy/Kconfig | 6 + > drivers/net/phy/Makefile | 1 + > drivers/net/phy/mxl-gpy.c | 552 ++++++++++++++++++++++++++++++++++++++ > 4 files changed, 565 insertions(+) > create mode 100644 drivers/net/phy/mxl-gpy.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 948706174fae..140b19d038fb 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -11168,6 +11168,12 @@ W: https://linuxtv.org > T: git git://linuxtv.org/media_tree.git > F: drivers/media/radio/radio-maxiradio* > > +MAXLINEAR ETHERNET PHY DRIVER > +M: Xu Liang <lxu@maxlinear.com> > +L: netdev@vger.kernel.org > +S: Supported > +F: drivers/net/phy/mxl-gpy.c > + > MCAN MMIO DEVICE DRIVER > M: Chandrasekar Ramakrishnan <rcsekar@samsung.com> > L: linux-can@vger.kernel.org > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index 288bf405ebdb..d02098774d80 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -207,6 +207,12 @@ config MARVELL_88X2222_PHY > Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet > Transceiver. > > +config MXL_GPHY > + tristate "Maxlinear PHYs" > + help > + Support for the Maxlinear GPY115, GPY211, GPY212, GPY215, > + GPY241, GPY245 PHYs. > + > config MICREL_PHY > tristate "Micrel PHYs" > help > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > index bcda7ed2455d..1055fb73ac17 100644 > --- a/drivers/net/phy/Makefile > +++ b/drivers/net/phy/Makefile > @@ -70,6 +70,7 @@ obj-$(CONFIG_MICREL_PHY) += micrel.o > obj-$(CONFIG_MICROCHIP_PHY) += microchip.o > obj-$(CONFIG_MICROCHIP_T1_PHY) += microchip_t1.o > obj-$(CONFIG_MICROSEMI_PHY) += mscc/ > +obj-$(CONFIG_MXL_GPHY) += mxl-gpy.o Could you spell it out completely: CONFIG_MAXLINEAR_GPHY for consistency with the other vendor's and also to have some proper namespacing in case we see a non-Ethernet PHY being submitted for a Maxlinear product in the future? > obj-$(CONFIG_NATIONAL_PHY) += national.o > obj-$(CONFIG_NXP_C45_TJA11XX_PHY) += nxp-c45-tja11xx.o > obj-$(CONFIG_NXP_TJA11XX_PHY) += nxp-tja11xx.o > diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c > new file mode 100644 > index 000000000000..3e3d3096e858 > --- /dev/null > +++ b/drivers/net/phy/mxl-gpy.c > @@ -0,0 +1,552 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* Copyright (C) 2021 Maxlinear Corporation > + * Copyright (C) 2020 Intel Corporation > + * > + * Drivers for Maxlinear Ethernet GPY > + * > + */ > + > +#include <linux/version.h> > +#include <linux/module.h> > +#include <linux/bitfield.h> > +#include <linux/phy.h> > +#include <linux/netdevice.h> > + > +/* PHY ID */ > +#define PHY_ID_GPY 0x67C9DC00 > +#define PHY_ID_MASK GENMASK(31, 4) Consider initializing your phy_driver with PHY_ID_MATCH_MODEL() which would take care of populating the mask accordingly. [snip] > + > +static int gpy_read_abilities(struct phy_device *phydev) > +{ > + int ret; > + > + ret = genphy_read_abilities(phydev); > + if (ret < 0) > + return ret; > + > + /* Detect 2.5G/5G support. */ > + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_STAT2); > + if (ret < 0) > + return ret; > + if (!(ret & MDIO_PMA_STAT2_EXTABLE)) > + return 0; > + > + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_EXTABLE); > + if (ret < 0) > + return ret; > + if (!(ret & MDIO_PMA_EXTABLE_NBT)) > + return 0; > + > + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_NG_EXTABLE); > + if (ret < 0) > + return ret; > + > + linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, > + phydev->supported, > + ret & MDIO_PMA_NG_EXTABLE_2_5GBT); > + > + linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, > + phydev->supported, > + ret & MDIO_PMA_NG_EXTABLE_5GBT); This does not access vendor specific registers, should not this be part of the standard genphy_read_abilities() or moved to a helper? > + > + return 0; > +} > + > +static int gpy_config_init(struct phy_device *phydev) > +{ > + int ret; > + > + /* Mask all interrupts */ > + ret = phy_write(phydev, PHY_IMASK, 0); > + if (ret) > + return ret; > + > + /* Clear all pending interrupts */ > + ret = phy_read(phydev, PHY_ISTAT); > + return ret < 0 ? ret : 0; You can certainly simplify this to: return phy_read(phydev, PHY_ISTAT); [snip] > + > +MODULE_DESCRIPTION("Maxlinear Ethernet GPY Driver"); > +MODULE_AUTHOR("Maxlinear Corporation"); The author is not usually a company but an individual working on behalf of that company.
On 5/6/2021 12:24 am, Florian Fainelli wrote: > > > +/* PHY ID */ > > +#define PHY_ID_GPY 0x67C9DC00 > > +#define PHY_ID_MASK GENMASK(31, 4) > > Consider initializing your phy_driver with PHY_ID_MATCH_MODEL() which > would take care of populating the mask accordingly. > Thanks, I will update. > > + > > +static int gpy_read_abilities(struct phy_device *phydev) > > +{ > > + int ret; > > + > > + ret = genphy_read_abilities(phydev); > > + if (ret < 0) > > + return ret; > > + > > + /* Detect 2.5G/5G support. */ > > + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_STAT2); > > + if (ret < 0) > > + return ret; > > + if (!(ret & MDIO_PMA_STAT2_EXTABLE)) > > + return 0; > > + > > + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_EXTABLE); > > + if (ret < 0) > > + return ret; > > + if (!(ret & MDIO_PMA_EXTABLE_NBT)) > > + return 0; > > + > > + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_NG_EXTABLE); > > + if (ret < 0) > > + return ret; > > + > > + linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, > > + phydev->supported, > > + ret & MDIO_PMA_NG_EXTABLE_2_5GBT); > > + > > + linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, > > + phydev->supported, > > + ret & MDIO_PMA_NG_EXTABLE_5GBT); > > This does not access vendor specific registers, should not this be part > of the standard genphy_read_abilities() or moved to a helper? > genphy_read_abilities does not cover 2.5G. genphy_c45_pma_read_abilities checks C45 ids and this check fail if is_c45 is not set. Mix of C22 and C45 is handled here, as our device support C22 with extension of C45. > > + > > + return 0; > > +} > > + > > +static int gpy_config_init(struct phy_device *phydev) > > +{ > > + int ret; > > + > > + /* Mask all interrupts */ > > + ret = phy_write(phydev, PHY_IMASK, 0); > > + if (ret) > > + return ret; > > + > > + /* Clear all pending interrupts */ > > + ret = phy_read(phydev, PHY_ISTAT); > > + return ret < 0 ? ret : 0; > > You can certainly simplify this to: > > return phy_read(phydev, PHY_ISTAT); > I'm thinking to clearly differentiate negative, 0, and positive. I had experience that a positive value is treated as error sometimes. > [snip] > > > + > > +MODULE_DESCRIPTION("Maxlinear Ethernet GPY Driver"); > > +MODULE_AUTHOR("Maxlinear Corporation"); > > The author is not usually a company but an individual working on behalf > of that company. > -- > Florian Ok, I will update.
On 5/6/2021 12:24 am, Florian Fainelli wrote: > > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > > index 288bf405ebdb..d02098774d80 100644 > > --- a/drivers/net/phy/Kconfig > > +++ b/drivers/net/phy/Kconfig > > @@ -207,6 +207,12 @@ config MARVELL_88X2222_PHY > > Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet > > Transceiver. > > > > +config MXL_GPHY > > + tristate "Maxlinear PHYs" > > + help > > + Support for the Maxlinear GPY115, GPY211, GPY212, GPY215, > > + GPY241, GPY245 PHYs. > > + > > config MICREL_PHY > > tristate "Micrel PHYs" > > help > > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > > index bcda7ed2455d..1055fb73ac17 100644 > > --- a/drivers/net/phy/Makefile > > +++ b/drivers/net/phy/Makefile > > @@ -70,6 +70,7 @@ obj-$(CONFIG_MICREL_PHY) += micrel.o > > obj-$(CONFIG_MICROCHIP_PHY) += microchip.o > > obj-$(CONFIG_MICROCHIP_T1_PHY) += microchip_t1.o > > obj-$(CONFIG_MICROSEMI_PHY) += mscc/ > > +obj-$(CONFIG_MXL_GPHY) += mxl-gpy.o > > Could you spell it out completely: CONFIG_MAXLINEAR_GPHY for consistency > with the other vendor's and also to have some proper namespacing in case > we see a non-Ethernet PHY being submitted for a Maxlinear product in the > future? I will fix in v4 patch. Thank you.
Hello, > Add driver to support the Maxlinear GPY115, GPY211, GPY212, GPY215, > GPY241, GPY245 PHYs. to me this seems like an evolution of the Lantiq XWAY PHYs for which we already have a driver: intel-xway.c. Intel took over Lantiq some years ago and last year MaxLinear then took over what was formerly Lantiq from Intel. From what I can tell right away: the interrupt handling still seems to be the same. Also the GPHY firmware version register also was there on older SoCs (with two or more GPHYs embedded into the SoC). SGMII support is new. And I am not sure about Wake-on-LAN. Have you considered adding support for these new PHYs to intel-xway.c? Best regards, Martin
> > > +static int gpy_read_abilities(struct phy_device *phydev) > > > +{ > > > + int ret; > > > + > > > + ret = genphy_read_abilities(phydev); > > > + if (ret < 0) > > > + return ret; > > > + > > > + /* Detect 2.5G/5G support. */ > > > + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_STAT2); > > > + if (ret < 0) > > > + return ret; > > > + if (!(ret & MDIO_PMA_STAT2_EXTABLE)) > > > + return 0; > > > + > > > + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_EXTABLE); > > > + if (ret < 0) > > > + return ret; > > > + if (!(ret & MDIO_PMA_EXTABLE_NBT)) > > > + return 0; > > > + > > > + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_NG_EXTABLE); > > > + if (ret < 0) > > > + return ret; > > > + > > > + linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, > > > + phydev->supported, > > > + ret & MDIO_PMA_NG_EXTABLE_2_5GBT); > > > + > > > + linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, > > > + phydev->supported, > > > + ret & MDIO_PMA_NG_EXTABLE_5GBT); > > > > This does not access vendor specific registers, should not this be part > > of the standard genphy_read_abilities() or moved to a helper? > > > genphy_read_abilities does not cover 2.5G. > > genphy_c45_pma_read_abilities checks C45 ids and this check fail if > is_c45 is not set. You appear to of ignored my comment about this. Please add the helper to the core as i suggested, and then use genphy_c45_pma_read_abilities(). Andrew
On 5/6/2021 3:44 am, Martin Blumenstingl wrote: > This email was sent from outside of MaxLinear. > > > Hello, > >> Add driver to support the Maxlinear GPY115, GPY211, GPY212, GPY215, >> GPY241, GPY245 PHYs. > to me this seems like an evolution of the Lantiq XWAY PHYs for which > we already have a driver: intel-xway.c. > Intel took over Lantiq some years ago and last year MaxLinear then > took over what was formerly Lantiq from Intel. > > From what I can tell right away: the interrupt handling still seems > to be the same. Also the GPHY firmware version register also was there > on older SoCs (with two or more GPHYs embedded into the SoC). SGMII > support is new. And I am not sure about Wake-on-LAN. > > Have you considered adding support for these new PHYs to intel-xway.c? > > > > Best regards, > Martin > I'm thinking to separate them. They are based on different IP and have many differences. The XWAY PHYs are going to EoL (or maybe already EoL). It creates difficulty for the test coverage to put together.
On 5/6/2021 4:10 am, Andrew Lunn wrote: > This email was sent from outside of MaxLinear. > > >>>> +static int gpy_read_abilities(struct phy_device *phydev) >>>> +{ >>>> + int ret; >>>> + >>>> + ret = genphy_read_abilities(phydev); >>>> + if (ret < 0) >>>> + return ret; >>>> + >>>> + /* Detect 2.5G/5G support. */ >>>> + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_STAT2); >>>> + if (ret < 0) >>>> + return ret; >>>> + if (!(ret & MDIO_PMA_STAT2_EXTABLE)) >>>> + return 0; >>>> + >>>> + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_EXTABLE); >>>> + if (ret < 0) >>>> + return ret; >>>> + if (!(ret & MDIO_PMA_EXTABLE_NBT)) >>>> + return 0; >>>> + >>>> + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_NG_EXTABLE); >>>> + if (ret < 0) >>>> + return ret; >>>> + >>>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, >>>> + phydev->supported, >>>> + ret & MDIO_PMA_NG_EXTABLE_2_5GBT); >>>> + >>>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, >>>> + phydev->supported, >>>> + ret & MDIO_PMA_NG_EXTABLE_5GBT); >>> This does not access vendor specific registers, should not this be part >>> of the standard genphy_read_abilities() or moved to a helper? >>> >> genphy_read_abilities does not cover 2.5G. >> >> genphy_c45_pma_read_abilities checks C45 ids and this check fail if >> is_c45 is not set. > You appear to of ignored my comment about this. Please add the helper > to the core as i suggested, and then use > genphy_c45_pma_read_abilities(). > > Andrew > I'm new to upstream and do not know the process to change code in core. Should I add the support function in same patch?
> >>> This does not access vendor specific registers, should not this be part > >>> of the standard genphy_read_abilities() or moved to a helper? > >>> > >> genphy_read_abilities does not cover 2.5G. > >> > >> genphy_c45_pma_read_abilities checks C45 ids and this check fail if > >> is_c45 is not set. > > You appear to of ignored my comment about this. Please add the helper > > to the core as i suggested, and then use > > genphy_c45_pma_read_abilities(). > > > > Andrew > > > I'm new to upstream and do not know the process to change code in core. Pretty much the same way you change code in a driver. Submit a path! Please put it into a separate patch, so making a patch series. Please add some kernel doc style documentation, describing what the function does. Look at other functions in phy_device.c for examples. Anybody can change core code. It just gets looked at closer, and need to be generic. Andrew
On Sat, Jun 05, 2021 at 03:32:39AM +0000, Liang Xu wrote: > On 5/6/2021 3:44 am, Martin Blumenstingl wrote: > > This email was sent from outside of MaxLinear. > > > > > > Hello, > > > >> Add driver to support the Maxlinear GPY115, GPY211, GPY212, GPY215, > >> GPY241, GPY245 PHYs. > > to me this seems like an evolution of the Lantiq XWAY PHYs for which > > we already have a driver: intel-xway.c. > > Intel took over Lantiq some years ago and last year MaxLinear then > > took over what was formerly Lantiq from Intel. > > > > From what I can tell right away: the interrupt handling still seems > > to be the same. Also the GPHY firmware version register also was there > > on older SoCs (with two or more GPHYs embedded into the SoC). SGMII > > support is new. And I am not sure about Wake-on-LAN. > > > > Have you considered adding support for these new PHYs to intel-xway.c? The WOL interrupts are the same, which could imply WoL configuration is the same? If it is the same, it would be good to also add WoL to intel-xway.c. What this new driver does not yet do is LEDs. It would be interesting to see if the LED control is the same. Hopefully, one day, generic LED support will get added. If this PHY and intel-xway.c has the same LED code, we will want to share it. > They are based on different IP and have many differences. Please could you list the differences? Is the datasheet available? I found a datasheet for the XWAY, so it would be nice to be able to compare them. > The XWAY PHYs are going to EoL (or maybe already EoL). That does not matter. They are going to be in use for the next 5 years or more, until all the products using them have died or been replaced. Linux supports hardware that is in use, not just what vendors sell today. > It creates difficulty for the test coverage to put together. That also does not really matter. If somebody has both, does the work to merge the drivers and overall we have less code and more features, the patch will be accepted. Andrew
On 5/6/2021 10:51 pm, Andrew Lunn wrote: > This email was sent from outside of MaxLinear. > > >>>>> This does not access vendor specific registers, should not this be part >>>>> of the standard genphy_read_abilities() or moved to a helper? >>>>> >>>> genphy_read_abilities does not cover 2.5G. >>>> >>>> genphy_c45_pma_read_abilities checks C45 ids and this check fail if >>>> is_c45 is not set. >>> You appear to of ignored my comment about this. Please add the helper >>> to the core as i suggested, and then use >>> genphy_c45_pma_read_abilities(). >>> >>> Andrew >>> >> I'm new to upstream and do not know the process to change code in core. > Pretty much the same way you change code in a driver. Submit a path! > > Please put it into a separate patch, so making a patch series. Please > add some kernel doc style documentation, describing what the function > does. Look at other functions in phy_device.c for examples. > > Anybody can change core code. It just gets looked at closer, and need > to be generic. > > Andrew > Thank you. I will create 2 patches for the core modification and driver separately in next update.
On 6/6/2021 1:24 am, Andrew Lunn wrote: > This email was sent from outside of MaxLinear. > > > On Sat, Jun 05, 2021 at 03:32:39AM +0000, Liang Xu wrote: >> On 5/6/2021 3:44 am, Martin Blumenstingl wrote: >>> This email was sent from outside of MaxLinear. >>> >>> >>> Hello, >>> >>>> Add driver to support the Maxlinear GPY115, GPY211, GPY212, GPY215, >>>> GPY241, GPY245 PHYs. >>> to me this seems like an evolution of the Lantiq XWAY PHYs for which >>> we already have a driver: intel-xway.c. >>> Intel took over Lantiq some years ago and last year MaxLinear then >>> took over what was formerly Lantiq from Intel. >>> >>> From what I can tell right away: the interrupt handling still seems >>> to be the same. Also the GPHY firmware version register also was there >>> on older SoCs (with two or more GPHYs embedded into the SoC). SGMII >>> support is new. And I am not sure about Wake-on-LAN. >>> >>> Have you considered adding support for these new PHYs to intel-xway.c? > The WOL interrupts are the same, which could imply WoL configuration > is the same? If it is the same, it would be good to also add WoL to > intel-xway.c. > > What this new driver does not yet do is LEDs. It would be interesting > to see if the LED control is the same. Hopefully, one day, generic LED > support will get added. If this PHY and intel-xway.c has the same LED > code, we will want to share it. > >> They are based on different IP and have many differences. > Please could you list the differences? Is the datasheet available? I > found a datasheet for the XWAY, so it would be nice to be able to > compare them. > >> The XWAY PHYs are going to EoL (or maybe already EoL). > That does not matter. They are going to be in use for the next 5 years > or more, until all the products using them have died or been > replaced. Linux supports hardware that is in use, not just what > vendors sell today. > >> It creates difficulty for the test coverage to put together. > That also does not really matter. If somebody has both, does the work > to merge the drivers and overall we have less code and more features, > the patch will be accepted. > > > Andrew > > The datasheet of GPY2xx are only available under NDA currently, since we have to protect our IP for the new devices. I do not know much about XWAY feature set, but I guess the difference should be 2.5G support, C45 register set, PTP offload, MACsec offload, etc. Problem of merging the both drivers would be the verification of the old devices for which I do not have a test environment. I can't deliver code without testing, and it will be big problem for me if it breaks function of existing user/customer. We will check for options and need approval from company, but this will not be possible short term within this merge window. For now, can I upstream this new driver first, and merge the old driver into new one later? We will enhance the new driver step by step with different features, but currently we would like to make the first version available and working for our customers. Thanks & Regards, Xu Liang
On Mon, Jun 7, 2021 at 6:37 AM Liang Xu <lxu@maxlinear.com> wrote: [...] > >> It creates difficulty for the test coverage to put together. > > That also does not really matter. If somebody has both, does the work > > to merge the drivers and overall we have less code and more features, > > the patch will be accepted. > > > > > > Andrew > > > > > The datasheet of GPY2xx are only available under NDA currently, since we > have > > to protect our IP for the new devices. I don't understand how knowledge about some registers can lead to IP being stolen by other vendors > I do not know much about XWAY feature set, but I guess the difference should > > be 2.5G support, C45 register set, PTP offload, MACsec offload, etc. I think [0] lists the functional differences - GPY115 and newer seem to differ from older PHYs in: - xMII interface - MACSEC - IEEE-1588 v2 (PTP) - Syn-E (not sure what this is) - Thermal Sensor > Problem of merging the both drivers would be the verification of the old > devices > > for which I do not have a test environment. I can't deliver code without > testing, According to the GPY111 and GPY112 product pages the status is "Active" ("PARTS & PURCHASING" tab). quote from the same tab: "Active - the part is released for sale, standard product." I believe that these are rebranded Lantiq PHYs: GPY111 and GPY112 are using PHYID register values which are compatible with the intel-xway driver. So I think you can use these PHYs for testing Also people from the OpenWrt community (for example: me, possible also Aleksander and Hauke) can help testing on existing hardware [...] > We will check for options and need approval from company, but this will > not be > > possible short term within this merge window. I hope that it is possible with GPY111 and/or GPY112 as mentioned above > For now, can I upstream this new driver first, and merge the old driver > into new one later? My answer to this question depends on the actual differences between the "old" and "new" PHYs. For example: if WoL and LED configuration are (mostly) identical then personally I vote for having one driver from the beginning Best regards, Martin [0] https://www.maxlinear.com/products/connectivity/wired/ethernet/ethernet-transceivers-phy [1] https://www.maxlinear.com/product/connectivity/wired/ethernet/ethernet-transceivers-phy/gpy111
On 7/6/2021 12:06 pm, Liang Xu wrote: > On 5/6/2021 10:51 pm, Andrew Lunn wrote: >> This email was sent from outside of MaxLinear. >> >> >>>>>> This does not access vendor specific registers, should not this be part >>>>>> of the standard genphy_read_abilities() or moved to a helper? >>>>>> >>>>> genphy_read_abilities does not cover 2.5G. >>>>> >>>>> genphy_c45_pma_read_abilities checks C45 ids and this check fail if >>>>> is_c45 is not set. >>>> You appear to of ignored my comment about this. Please add the helper >>>> to the core as i suggested, and then use >>>> genphy_c45_pma_read_abilities(). >>>> >>>> Andrew >>>> >>> I'm new to upstream and do not know the process to change code in core. >> Pretty much the same way you change code in a driver. Submit a path! >> >> Please put it into a separate patch, so making a patch series. Please >> add some kernel doc style documentation, describing what the function >> does. Look at other functions in phy_device.c for examples. >> >> Anybody can change core code. It just gets looked at closer, and need >> to be generic. >> >> Andrew >> > Thank you. I will create 2 patches for the core modification and driver > separately > > in next update. > Hi Andrew, I need your advice regarding to our recent test in loopback. My current implementation uses "genphy_loopback" to enable/disable loopback mode. And it has intermittent issue (traffic not loopbacked) during the test with net-next. There are difference in the implementation in net-next and Linux v5.12. Net-next: int genphy_loopback(struct phy_device *phydev, bool enable) { if (enable) { u16 val, ctl = BMCR_LOOPBACK; int ret; if (phydev->speed == SPEED_1000) ctl |= BMCR_SPEED1000; else if (phydev->speed == SPEED_100) ctl |= BMCR_SPEED100; if (phydev->duplex == DUPLEX_FULL) ctl |= BMCR_FULLDPLX; phy_modify(phydev, MII_BMCR, ~0, ctl); ret = phy_read_poll_timeout(phydev, MII_BMSR, val, val & BMSR_LSTATUS, 5000, 500000, true); if (ret) return ret; } else { phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK, 0); phy_config_aneg(phydev); } return 0; } v5.12.11: int genphy_loopback(struct phy_device *phydev, bool enable) { return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK, enable ? BMCR_LOOPBACK : 0); } Not sure whether anyone else reported similar issue. Should I use phy_modify to set the LOOPBACK bit only in my driver implementation as force speed with loopback enable does not work in our device? Thanks & Regards, Xu Liang
> Net-next: > > int genphy_loopback(struct phy_device *phydev, bool enable) > { > if (enable) { > u16 val, ctl = BMCR_LOOPBACK; > int ret; > > if (phydev->speed == SPEED_1000) > ctl |= BMCR_SPEED1000; > else if (phydev->speed == SPEED_100) > ctl |= BMCR_SPEED100; > > if (phydev->duplex == DUPLEX_FULL) > ctl |= BMCR_FULLDPLX; > > phy_modify(phydev, MII_BMCR, ~0, ctl); > > ret = phy_read_poll_timeout(phydev, MII_BMSR, val, > val & BMSR_LSTATUS, > 5000, 500000, true); > if (ret) > return ret; > } else { > phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK, 0); > > phy_config_aneg(phydev); > } > > return 0; > } > > v5.12.11: > > int genphy_loopback(struct phy_device *phydev, bool enable) > { > return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK, > enable ? BMCR_LOOPBACK : 0); > } > > > Not sure whether anyone else reported similar issue. The commit message says: net: phy: genphy_loopback: add link speed configuration In case of loopback, in most cases we need to disable autoneg support and force some speed configuration. Otherwise, depending on currently active auto negotiated link speed, the loopback may or may not work. > Should I use phy_modify to set the LOOPBACK bit only in my driver > implementation as force speed with loopback enable does not work in our > device? So you appear to have the exact opposite problem, you need to use auto-neg, with yourself, in order to have link. So there are two solutions: 1) As you say, implement it in your driver 2) Add a second generic implementation, which enables autoneg, if it is not enabled, sets the loopback bit, and waits for the link to come up. Does your PHY driver error out when asked to do a forced mode? It probably should, if your silicon does not support that part of C22. Andrew
On 18/6/2021 10:01 pm, Andrew Lunn wrote: > This email was sent from outside of MaxLinear. > > >> Net-next: >> >> int genphy_loopback(struct phy_device *phydev, bool enable) >> { >> if (enable) { >> u16 val, ctl = BMCR_LOOPBACK; >> int ret; >> >> if (phydev->speed == SPEED_1000) >> ctl |= BMCR_SPEED1000; >> else if (phydev->speed == SPEED_100) >> ctl |= BMCR_SPEED100; >> >> if (phydev->duplex == DUPLEX_FULL) >> ctl |= BMCR_FULLDPLX; >> >> phy_modify(phydev, MII_BMCR, ~0, ctl); >> >> ret = phy_read_poll_timeout(phydev, MII_BMSR, val, >> val & BMSR_LSTATUS, >> 5000, 500000, true); >> if (ret) >> return ret; >> } else { >> phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK, 0); >> >> phy_config_aneg(phydev); >> } >> >> return 0; >> } >> >> v5.12.11: >> >> int genphy_loopback(struct phy_device *phydev, bool enable) >> { >> return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK, >> enable ? BMCR_LOOPBACK : 0); >> } >> >> >> Not sure whether anyone else reported similar issue. > The commit message says: > > net: phy: genphy_loopback: add link speed configuration > > In case of loopback, in most cases we need to disable autoneg support > and force some speed configuration. Otherwise, depending on currently > active auto negotiated link speed, the loopback may or may not work. > >> Should I use phy_modify to set the LOOPBACK bit only in my driver >> implementation as force speed with loopback enable does not work in our >> device? > So you appear to have the exact opposite problem, you need to use > auto-neg, with yourself, in order to have link. So there are two > solutions: > > 1) As you say, implement it in your driver > > 2) Add a second generic implementation, which enables autoneg, if it > is not enabled, sets the loopback bit, and waits for the link to come > up. > > Does your PHY driver error out when asked to do a forced mode? It > probably should, if your silicon does not support that part of C22. > > Andrew > Thank you for prompt reply. The forced mode is successful because we support forced mode in C22. The problem happens in the speed change no matter it's forced or auto-neg in our device. If I keep current status and just set the loopback bit, it works. This may be specific to our device, if nobody else report issue in the genphy_loopback implementation in net-next. Then I will implement it in my driver. Thanks & Regards, Xu Liang
On Fri, Jun 18, 2021 at 03:36:35PM +0000, Liang Xu wrote: > On 18/6/2021 10:01 pm, Andrew Lunn wrote: > > This email was sent from outside of MaxLinear. > > > > > >> Net-next: > >> > >> int genphy_loopback(struct phy_device *phydev, bool enable) > >> { > >> if (enable) { > >> u16 val, ctl = BMCR_LOOPBACK; > >> int ret; > >> > >> if (phydev->speed == SPEED_1000) > >> ctl |= BMCR_SPEED1000; > >> else if (phydev->speed == SPEED_100) > >> ctl |= BMCR_SPEED100; > The problem happens in the speed change no matter it's forced or > auto-neg in our device. You say speed change. So do you just need to add support for 10Mbps, so there is no speed change? Or are you saying phydev->speed does not match the actual speed? Andrew
On 21/6/2021 10:30 am, Andrew Lunn wrote: > This email was sent from outside of MaxLinear. > > > On Fri, Jun 18, 2021 at 03:36:35PM +0000, Liang Xu wrote: >> On 18/6/2021 10:01 pm, Andrew Lunn wrote: >>> This email was sent from outside of MaxLinear. >>> >>> >>>> Net-next: >>>> >>>> int genphy_loopback(struct phy_device *phydev, bool enable) >>>> { >>>> if (enable) { >>>> u16 val, ctl = BMCR_LOOPBACK; >>>> int ret; >>>> >>>> if (phydev->speed == SPEED_1000) >>>> ctl |= BMCR_SPEED1000; >>>> else if (phydev->speed == SPEED_100) >>>> ctl |= BMCR_SPEED100; >> The problem happens in the speed change no matter it's forced or >> auto-neg in our device. > You say speed change. So do you just need to add support for 10Mbps, > so there is no speed change? Or are you saying phydev->speed does not > match the actual speed? > > Andrew > We have 2.5G link speed, so mismatch happens. Thanks & Regards, Xu Liang
On Mon, Jun 21, 2021 at 05:05:06AM +0000, Liang Xu wrote: > On 21/6/2021 10:30 am, Andrew Lunn wrote: > > This email was sent from outside of MaxLinear. > > > > > > On Fri, Jun 18, 2021 at 03:36:35PM +0000, Liang Xu wrote: > >> On 18/6/2021 10:01 pm, Andrew Lunn wrote: > >>> This email was sent from outside of MaxLinear. > >>> > >>> > >>>> Net-next: > >>>> > >>>> int genphy_loopback(struct phy_device *phydev, bool enable) > >>>> { > >>>> if (enable) { > >>>> u16 val, ctl = BMCR_LOOPBACK; > >>>> int ret; > >>>> > >>>> if (phydev->speed == SPEED_1000) > >>>> ctl |= BMCR_SPEED1000; > >>>> else if (phydev->speed == SPEED_100) > >>>> ctl |= BMCR_SPEED100; > >> The problem happens in the speed change no matter it's forced or > >> auto-neg in our device. > > You say speed change. So do you just need to add support for 10Mbps, > > so there is no speed change? Or are you saying phydev->speed does not > > match the actual speed? > > > > Andrew > > > We have 2.5G link speed, so mismatch happens. Ah, yes. Sorry. Please implement it in your driver. Andrew
On 21/6/2021 8:48 pm, Andrew Lunn wrote: > This email was sent from outside of MaxLinear. > > > On Mon, Jun 21, 2021 at 05:05:06AM +0000, Liang Xu wrote: >> On 21/6/2021 10:30 am, Andrew Lunn wrote: >>> This email was sent from outside of MaxLinear. >>> >>> >>> On Fri, Jun 18, 2021 at 03:36:35PM +0000, Liang Xu wrote: >>>> On 18/6/2021 10:01 pm, Andrew Lunn wrote: >>>>> This email was sent from outside of MaxLinear. >>>>> >>>>> >>>>>> Net-next: >>>>>> >>>>>> int genphy_loopback(struct phy_device *phydev, bool enable) >>>>>> { >>>>>> if (enable) { >>>>>> u16 val, ctl = BMCR_LOOPBACK; >>>>>> int ret; >>>>>> >>>>>> if (phydev->speed == SPEED_1000) >>>>>> ctl |= BMCR_SPEED1000; >>>>>> else if (phydev->speed == SPEED_100) >>>>>> ctl |= BMCR_SPEED100; >>>> The problem happens in the speed change no matter it's forced or >>>> auto-neg in our device. >>> You say speed change. So do you just need to add support for 10Mbps, >>> so there is no speed change? Or are you saying phydev->speed does not >>> match the actual speed? >>> >>> Andrew >>> >> We have 2.5G link speed, so mismatch happens. > Ah, yes. Sorry. > > Please implement it in your driver. > > Andrew > Sure. Thank you for advice. Regards, Xu Liang
Hi Liang, On Wed, Jun 23, 2021 at 12:56 PM Liang Xu <lxu@maxlinear.com> wrote: [...] > Hi Martin, > > 1) The legacy PHY GPY111 does not share the same register format and address for WoL and LED registers. Therefore code saving with a common driver is not possible. > 2) The interrupt handling routine would be different when new features are added in driver, such as PTP offload, MACsec offload. These will be added step by step as enhancement patches afte initial version of this driver upstreamed. I think that would leave only few shared registers with the older PHYs (and thus the intel-xway driver). Due to the lack of a public datasheet however I have no way to confirm this. So with this information added to the patch description having a different driver is fine for me Maybe Andrew can also share his opinion - based on this new information - as he previously also suggested to extend the intel-xway driver instead of creating a new one > 3) The new PHY generation comes with a reasonable pre-configuration for the LED registers which does not need any modification usually. > In case a customer needs a specific configuration (e.g. traffic pulsing only, no link status) he needs to configure this via MDIO. There is also another option for a fixed preconfiguration with the support of an external flash. However, this is out of scope of the driver. older PHYs also do this, but not all boards use a reasonable LED setup > 4) Many old products are mostly used on embedded devices which do not support WoL. Therefore there was no request yet to supported by the legacy XWAY driver. my understanding of Andrew's argument is: "if the register layout is the same for old and new PHY then why would we do some extra effort to not add WoL support for the old PHYs" Based on your information above the register layout is different, so that means two different implementations are needed anyways. Best regards, Martin
On Wed, Jun 23, 2021 at 11:09:23PM +0200, Martin Blumenstingl wrote: > Hi Liang, > > On Wed, Jun 23, 2021 at 12:56 PM Liang Xu <lxu@maxlinear.com> wrote: > [...] > > Hi Martin, > > > > 1) The legacy PHY GPY111 does not share the same register format and address for WoL and LED registers. Therefore code saving with a common driver is not possible. > > 2) The interrupt handling routine would be different when new features are added in driver, such as PTP offload, MACsec offload. These will be added step by step as enhancement patches afte initial version of this driver upstreamed. > I think that would leave only few shared registers with the older PHYs > (and thus the intel-xway driver). Due to the lack of a public > datasheet however I have no way to confirm this. > So with this information added to the patch description having a > different driver is fine for me > > Maybe Andrew can also share his opinion - based on this new > information - as he previously also suggested to extend the intel-xway > driver instead of creating a new one If the potential sharing is minimal, then a new driver makes sense. So please do expand the patch description with a good justification. > > 3) The new PHY generation comes with a reasonable pre-configuration for the LED registers which does not need any modification usually. > > In case a customer needs a specific configuration (e.g. traffic pulsing only, no link status) he needs to configure this via MDIO. There is also another option for a fixed preconfiguration with the support of an external flash. However, this is out of scope of the driver. > older PHYs also do this, but not all boards use a reasonable LED setup > > > 4) Many old products are mostly used on embedded devices which do not support WoL. Therefore there was no request yet to supported by the legacy XWAY driver. > my understanding of Andrew's argument is: "if the register layout is > the same for old and new PHY then why would we do some extra effort to > not add WoL support for the old PHYs" > Based on your information above the register layout is different, so > that means two different implementations are needed anyways. It would be nice to add WoL support for the older devices. They might not be maxlinears latest and greatest, but there could be more of them actually in use at the moment than this new PHY. Andrew
diff --git a/MAINTAINERS b/MAINTAINERS index 948706174fae..140b19d038fb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11168,6 +11168,12 @@ W: https://linuxtv.org T: git git://linuxtv.org/media_tree.git F: drivers/media/radio/radio-maxiradio* +MAXLINEAR ETHERNET PHY DRIVER +M: Xu Liang <lxu@maxlinear.com> +L: netdev@vger.kernel.org +S: Supported +F: drivers/net/phy/mxl-gpy.c + MCAN MMIO DEVICE DRIVER M: Chandrasekar Ramakrishnan <rcsekar@samsung.com> L: linux-can@vger.kernel.org diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 288bf405ebdb..d02098774d80 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -207,6 +207,12 @@ config MARVELL_88X2222_PHY Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet Transceiver. +config MXL_GPHY + tristate "Maxlinear PHYs" + help + Support for the Maxlinear GPY115, GPY211, GPY212, GPY215, + GPY241, GPY245 PHYs. + config MICREL_PHY tristate "Micrel PHYs" help diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index bcda7ed2455d..1055fb73ac17 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -70,6 +70,7 @@ obj-$(CONFIG_MICREL_PHY) += micrel.o obj-$(CONFIG_MICROCHIP_PHY) += microchip.o obj-$(CONFIG_MICROCHIP_T1_PHY) += microchip_t1.o obj-$(CONFIG_MICROSEMI_PHY) += mscc/ +obj-$(CONFIG_MXL_GPHY) += mxl-gpy.o obj-$(CONFIG_NATIONAL_PHY) += national.o obj-$(CONFIG_NXP_C45_TJA11XX_PHY) += nxp-c45-tja11xx.o obj-$(CONFIG_NXP_TJA11XX_PHY) += nxp-tja11xx.o diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c new file mode 100644 index 000000000000..3e3d3096e858 --- /dev/null +++ b/drivers/net/phy/mxl-gpy.c @@ -0,0 +1,552 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* Copyright (C) 2021 Maxlinear Corporation + * Copyright (C) 2020 Intel Corporation + * + * Drivers for Maxlinear Ethernet GPY + * + */ + +#include <linux/version.h> +#include <linux/module.h> +#include <linux/bitfield.h> +#include <linux/phy.h> +#include <linux/netdevice.h> + +/* PHY ID */ +#define PHY_ID_GPY 0x67C9DC00 +#define PHY_ID_MASK GENMASK(31, 4) + +#define PHY_MIISTAT 0x18 /* MII state */ +#define PHY_IMASK 0x19 /* interrupt mask */ +#define PHY_ISTAT 0x1A /* interrupt status */ +#define PHY_FWV 0x1E /* firmware version */ + +#define PHY_MIISTAT_SPD_MASK GENMASK(2, 0) +#define PHY_MIISTAT_DPX BIT(3) +#define PHY_MIISTAT_LS BIT(10) + +#define PHY_MIISTAT_SPD_10 0 +#define PHY_MIISTAT_SPD_100 1 +#define PHY_MIISTAT_SPD_1000 2 +#define PHY_MIISTAT_SPD_2500 4 + +#define PHY_IMASK_WOL BIT(15) /* Wake-on-LAN */ +#define PHY_IMASK_ANC BIT(10) /* Auto-Neg complete */ +#define PHY_IMASK_ADSC BIT(5) /* Link auto-downspeed detect */ +#define PHY_IMASK_DXMC BIT(2) /* Duplex mode change */ +#define PHY_IMASK_LSPC BIT(1) /* Link speed change */ +#define PHY_IMASK_LSTC BIT(0) /* Link state change */ +#define PHY_IMASK_MASK (PHY_IMASK_LSTC | \ + PHY_IMASK_LSPC | \ + PHY_IMASK_DXMC | \ + PHY_IMASK_ADSC | \ + PHY_IMASK_ANC) + +#define PHY_FWV_REL_MASK BIT(15) +#define PHY_FWV_TYPE_MASK GENMASK(11, 8) +#define PHY_FWV_MINOR_MASK GENMASK(7, 0) + +/* ANEG dev */ +#define ANEG_MGBT_AN_CTRL 0x20 +#define ANEG_MGBT_AN_STAT 0x21 +#define CTRL_AB_2G5BT_BIT BIT(7) +#define CTRL_AB_FR_2G5BT BIT(5) +#define STAT_AB_2G5BT_BIT BIT(5) +#define STAT_AB_FR_2G5BT BIT(3) + +/* SGMII */ +#define VSPEC1_SGMII_CTRL 0x08 +#define VSPEC1_SGMII_CTRL_ANEN BIT(12) /* Aneg enable */ +#define VSPEC1_SGMII_CTRL_ANRS BIT(9) /* Restart Aneg */ +#define VSPEC1_SGMII_ANEN_ANRS (VSPEC1_SGMII_CTRL_ANEN | \ + VSPEC1_SGMII_CTRL_ANRS) + +/* WoL */ +#define VPSPEC2_WOL_CTL 0x0E06 +#define VPSPEC2_WOL_AD01 0x0E08 +#define VPSPEC2_WOL_AD23 0x0E09 +#define VPSPEC2_WOL_AD45 0x0E0A +#define WOL_EN BIT(0) + +static const struct { + int type; + int minor; +} ver_need_sgmii_reaneg[] = { + {7, 0x6D}, + {8, 0x6D}, + {9, 0x73}, +}; + +static int gpy_read_abilities(struct phy_device *phydev) +{ + int ret; + + ret = genphy_read_abilities(phydev); + if (ret < 0) + return ret; + + /* Detect 2.5G/5G support. */ + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_STAT2); + if (ret < 0) + return ret; + if (!(ret & MDIO_PMA_STAT2_EXTABLE)) + return 0; + + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_EXTABLE); + if (ret < 0) + return ret; + if (!(ret & MDIO_PMA_EXTABLE_NBT)) + return 0; + + ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_NG_EXTABLE); + if (ret < 0) + return ret; + + linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, + phydev->supported, + ret & MDIO_PMA_NG_EXTABLE_2_5GBT); + + linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, + phydev->supported, + ret & MDIO_PMA_NG_EXTABLE_5GBT); + + return 0; +} + +static int gpy_config_init(struct phy_device *phydev) +{ + int ret; + + /* Mask all interrupts */ + ret = phy_write(phydev, PHY_IMASK, 0); + if (ret) + return ret; + + /* Clear all pending interrupts */ + ret = phy_read(phydev, PHY_ISTAT); + return ret < 0 ? ret : 0; +} + +static int gpy_probe(struct phy_device *phydev) +{ + int ret; + + /* Show GPY PHY FW version in dmesg */ + ret = phy_read(phydev, PHY_FWV); + if (ret < 0) + return ret; + + phydev_info(phydev, "Firmware Version: 0x%04X (%s)\n", ret, + (ret & PHY_FWV_REL_MASK) ? "release" : "test"); + + return 0; +} + +static bool gpy_sgmii_need_reaneg(struct phy_device *phydev) +{ + int fw_ver, fw_type, fw_minor; + size_t i; + + fw_ver = phy_read(phydev, PHY_FWV); + if (fw_ver < 0) + return true; + + fw_type = FIELD_GET(PHY_FWV_TYPE_MASK, fw_ver); + fw_minor = FIELD_GET(PHY_FWV_MINOR_MASK, fw_ver); + + for (i = 0; i < ARRAY_SIZE(ver_need_sgmii_reaneg); i++) { + if (fw_type != ver_need_sgmii_reaneg[i].type) + continue; + if (fw_minor < ver_need_sgmii_reaneg[i].minor) + return true; + break; + } + + return false; +} + +static bool gpy_2500basex_chk(struct phy_device *phydev) +{ + int ret; + + ret = phy_read(phydev, PHY_MIISTAT); + if (ret < 0) { + phydev_err(phydev, "Error: MDIO register access failed: %d\n", + ret); + return false; + } + + if (!(ret & PHY_MIISTAT_LS) || + FIELD_GET(PHY_MIISTAT_SPD_MASK, ret) != PHY_MIISTAT_SPD_2500) + return false; + + phydev->speed = SPEED_2500; + phydev->interface = PHY_INTERFACE_MODE_2500BASEX; + phy_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_SGMII_CTRL, + VSPEC1_SGMII_CTRL_ANEN, 0); + return true; +} + +static bool gpy_sgmii_aneg_en(struct phy_device *phydev) +{ + int ret; + + ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_SGMII_CTRL); + if (ret < 0) { + phydev_err(phydev, "Error: MMD register access failed: %d\n", + ret); + return true; + } + + return (ret & VSPEC1_SGMII_CTRL_ANEN) ? true : false; +} + +static int gpy_config_aneg(struct phy_device *phydev) +{ + bool changed = false; + u32 adv; + int ret; + + if (phydev->autoneg == AUTONEG_DISABLE) { + /* Configure half duplex with genphy_setup_forced, + * because genphy_c45_pma_setup_forced does not support. + */ + return phydev->duplex != DUPLEX_FULL + ? genphy_setup_forced(phydev) + : genphy_c45_pma_setup_forced(phydev); + } + + ret = genphy_c45_an_config_aneg(phydev); + if (ret < 0) + return ret; + if (ret > 0) + changed = true; + + adv = linkmode_adv_to_mii_ctrl1000_t(phydev->advertising); + ret = phy_modify_changed(phydev, MII_CTRL1000, + ADVERTISE_1000FULL | ADVERTISE_1000HALF, + adv); + if (ret < 0) + return ret; + if (ret > 0) + changed = true; + + ret = genphy_c45_check_and_restart_aneg(phydev, changed); + if (ret < 0) + return ret; + + if (phydev->interface == PHY_INTERFACE_MODE_USXGMII || + phydev->interface == PHY_INTERFACE_MODE_INTERNAL) + return 0; + + /* No need to trigger re-ANEG if SGMII link speed is 2.5G + * or SGMII ANEG is disabled. + */ + if (!gpy_sgmii_need_reaneg(phydev) || gpy_2500basex_chk(phydev) || + !gpy_sgmii_aneg_en(phydev)) + return 0; + + /* There is a design constraint in GPY2xx device where SGMII AN is + * only triggered when there is change of speed. If, PHY link + * partner`s speed is still same even after PHY TPI is down and up + * again, SGMII AN is not triggered and hence no new in-band message + * from GPY to MAC side SGMII. + * This could cause an issue during power up, when PHY is up prior to + * MAC. At this condition, once MAC side SGMII is up, MAC side SGMII + * wouldn`t receive new in-band message from GPY with correct link + * status, speed and duplex info. + * + * 1) If PHY is already up and TPI link status is still down (such as + * hard reboot), TPI link status is polled for 4 seconds before + * retriggerring SGMII AN. + * 2) If PHY is already up and TPI link status is also up (such as soft + * reboot), polling of TPI link status is not needed and SGMII AN is + * immediately retriggered. + * 3) Other conditions such as PHY is down, speed change etc, skip + * retriggering SGMII AN. Note: in case of speed change, GPY FW will + * initiate SGMII AN. + */ + + if (phydev->state != PHY_UP) + return 0; + + ret = phy_read_poll_timeout(phydev, MII_BMSR, ret, ret & BMSR_LSTATUS, + 20000, 4000000, false); + if (ret == -ETIMEDOUT) + return 0; + else if (ret < 0) + return ret; + + /* Trigger SGMII AN. */ + return phy_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_SGMII_CTRL, + VSPEC1_SGMII_CTRL_ANRS, VSPEC1_SGMII_CTRL_ANRS); +} + +static void gpy_update_interface(struct phy_device *phydev) +{ + int ret; + + /* Interface mode is fixed for USXGMII and integrated PHY */ + if (phydev->interface == PHY_INTERFACE_MODE_USXGMII || + phydev->interface == PHY_INTERFACE_MODE_INTERNAL) + return; + + /* Automatically switch SERDES interface between SGMII and 2500-BaseX + * according to speed. Disable ANEG in 2500-BaseX mode. + */ + switch (phydev->speed) { + case SPEED_2500: + phydev->interface = PHY_INTERFACE_MODE_2500BASEX; + ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_SGMII_CTRL, + VSPEC1_SGMII_CTRL_ANEN, 0); + if (ret < 0) + phydev_err(phydev, + "Error: Disable of SGMII ANEG failed: %d\n", + ret); + break; + case SPEED_1000: + case SPEED_100: + case SPEED_10: + phydev->interface = PHY_INTERFACE_MODE_SGMII; + if (gpy_sgmii_aneg_en(phydev)) + break; + /* Enable and restart SGMII ANEG for 10/100/1000Mbps link speed + * if ANEG is disabled (in 2500-BaseX mode). + */ + ret = phy_modify_mmd(phydev, MDIO_MMD_VEND1, VSPEC1_SGMII_CTRL, + VSPEC1_SGMII_ANEN_ANRS, + VSPEC1_SGMII_ANEN_ANRS); + if (ret < 0) + phydev_err(phydev, + "Error: Enable of SGMII ANEG failed: %d\n", + ret); + break; + } +} + +static int gpy_read_status(struct phy_device *phydev) +{ + int ret; + + ret = genphy_update_link(phydev); + if (ret) + return ret; + + phydev->speed = SPEED_UNKNOWN; + phydev->duplex = DUPLEX_UNKNOWN; + phydev->pause = 0; + phydev->asym_pause = 0; + + if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) { + ret = genphy_c45_read_lpa(phydev); + if (ret < 0) + return ret; + + /* Read the link partner's 1G advertisement */ + ret = phy_read(phydev, MII_STAT1000); + if (ret < 0) + return ret; + mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising, ret); + } else if (phydev->autoneg == AUTONEG_DISABLE) { + linkmode_zero(phydev->lp_advertising); + } + + ret = phy_read(phydev, PHY_MIISTAT); + if (ret < 0) + return ret; + + phydev->link = (ret & PHY_MIISTAT_LS) ? 1 : 0; + phydev->duplex = (ret & PHY_MIISTAT_DPX) ? DUPLEX_FULL : DUPLEX_HALF; + switch (FIELD_GET(PHY_MIISTAT_SPD_MASK, ret)) { + case PHY_MIISTAT_SPD_10: + phydev->speed = SPEED_10; + break; + case PHY_MIISTAT_SPD_100: + phydev->speed = SPEED_100; + break; + case PHY_MIISTAT_SPD_1000: + phydev->speed = SPEED_1000; + break; + case PHY_MIISTAT_SPD_2500: + phydev->speed = SPEED_2500; + break; + } + + if (phydev->link) + gpy_update_interface(phydev); + + return 0; +} + +static int gpy_config_intr(struct phy_device *phydev) +{ + u16 mask = 0; + + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) + mask = PHY_IMASK_MASK; + + return phy_write(phydev, PHY_IMASK, mask); +} + +static irqreturn_t gpy_handle_interrupt(struct phy_device *phydev) +{ + int reg; + + reg = phy_read(phydev, PHY_ISTAT); + if (reg < 0) { + phy_error(phydev); + return IRQ_NONE; + } + + if (!(reg & PHY_IMASK_MASK)) + return IRQ_NONE; + + phy_trigger_machine(phydev); + + return IRQ_HANDLED; +} + +static int gpy_set_wol(struct phy_device *phydev, + struct ethtool_wolinfo *wol) +{ + struct net_device *attach_dev = phydev->attached_dev; + int ret; + + if (wol->wolopts & WAKE_MAGIC) { + /* MAC address - Byte0:Byte1:Byte2:Byte3:Byte4:Byte5 + * VPSPEC2_WOL_AD45 = Byte0:Byte1 + * VPSPEC2_WOL_AD23 = Byte2:Byte3 + * VPSPEC2_WOL_AD01 = Byte4:Byte5 + */ + ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND2, + VPSPEC2_WOL_AD45, + ((attach_dev->dev_addr[0] << 8) | + attach_dev->dev_addr[1])); + if (ret < 0) + return ret; + + ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND2, + VPSPEC2_WOL_AD23, + ((attach_dev->dev_addr[2] << 8) | + attach_dev->dev_addr[3])); + if (ret < 0) + return ret; + + ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND2, + VPSPEC2_WOL_AD01, + ((attach_dev->dev_addr[4] << 8) | + attach_dev->dev_addr[5])); + if (ret < 0) + return ret; + + /* Enable the WOL interrupt */ + ret = phy_write(phydev, PHY_IMASK, PHY_IMASK_WOL); + if (ret < 0) + return ret; + + /* Enable magic packet matching */ + ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND2, + VPSPEC2_WOL_CTL, + WOL_EN); + if (ret < 0) + return ret; + + /* Clear the interrupt status register. + * Only WoL is enabled so clear all. + */ + ret = phy_read(phydev, PHY_ISTAT); + if (ret < 0) + return ret; + } else { + /* Disable magic packet matching */ + ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2, + VPSPEC2_WOL_CTL, + WOL_EN); + if (ret < 0) + return ret; + } + + if (wol->wolopts & WAKE_PHY) { + /* Enable the link state change interrupt */ + ret = phy_set_bits(phydev, PHY_IMASK, PHY_IMASK_LSTC); + if (ret < 0) + return ret; + + /* Clear the interrupt status register */ + ret = phy_read(phydev, PHY_ISTAT); + if (ret < 0) + return ret; + + if (ret & (PHY_IMASK_MASK & ~PHY_IMASK_LSTC)) + phy_trigger_machine(phydev); + + return 0; + } + + /* Disable the link state change interrupt */ + return phy_clear_bits(phydev, PHY_IMASK, PHY_IMASK_LSTC); +} + +static void gpy_get_wol(struct phy_device *phydev, + struct ethtool_wolinfo *wol) +{ + int ret; + + wol->supported = WAKE_MAGIC | WAKE_PHY; + wol->wolopts = 0; + + ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, VPSPEC2_WOL_CTL); + if (ret & WOL_EN) + wol->wolopts |= WAKE_MAGIC; + + ret = phy_read(phydev, PHY_IMASK); + if (ret & PHY_IMASK_LSTC) + wol->wolopts |= WAKE_PHY; +} + +static int gpy_loopback(struct phy_device *phydev, bool enable) +{ + int ret; + + ret = genphy_loopback(phydev, enable); + if (!ret) { + /* It takes some time for PHY device to switch + * into/out-of loopback mode. + */ + usleep_range(100, 200); + } + + return ret; +} + +static struct phy_driver gpy_drivers[] = { + { + .phy_id = PHY_ID_GPY, + .phy_id_mask = PHY_ID_MASK, + .name = "Maxlinear Ethernet GPY", + .get_features = gpy_read_abilities, + .config_init = gpy_config_init, + .probe = gpy_probe, + .suspend = genphy_suspend, + .resume = genphy_resume, + .config_aneg = gpy_config_aneg, + .aneg_done = genphy_c45_aneg_done, + .read_status = gpy_read_status, + .config_intr = gpy_config_intr, + .handle_interrupt = gpy_handle_interrupt, + .set_wol = gpy_set_wol, + .get_wol = gpy_get_wol, + .set_loopback = gpy_loopback, + }, +}; +module_phy_driver(gpy_drivers); + +static struct mdio_device_id __maybe_unused gpy_tbl[] = { + {PHY_ID_GPY, PHY_ID_MASK}, + { } +}; +MODULE_DEVICE_TABLE(mdio, gpy_tbl); + +MODULE_DESCRIPTION("Maxlinear Ethernet GPY Driver"); +MODULE_AUTHOR("Maxlinear Corporation"); +MODULE_LICENSE("GPL");
Add driver to support the Maxlinear GPY115, GPY211, GPY212, GPY215, GPY241, GPY245 PHYs. Signed-off-by: Xu Liang <lxu@maxlinear.com> --- v2 changes: Fix format warning from checkpath and some comments. Use smaller PHY ID mask. Split FWV register mask. Call phy_trigger_machine if necessary when clear interrupt. v3 changes: Replace unnecessary phy_modify_mmd_changed with phy_modify_mmd Move firmware version print to probe. MAINTAINERS | 6 + drivers/net/phy/Kconfig | 6 + drivers/net/phy/Makefile | 1 + drivers/net/phy/mxl-gpy.c | 552 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 565 insertions(+) create mode 100644 drivers/net/phy/mxl-gpy.c