| Message ID | 2c68bdb1-9b53-ce0b-74d3-c7ea2d9e7ac0@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: phy: add interface mode PHY_INTERFACE_MODE_USXGMII | expand |
On 5/22/19 12:58 PM, Heiner Kallweit wrote: > So far we didn't support mode USXGMII, and in order to not break the > two Freescale boards mode XGMII was accepted for the AQR107 family > even though it doesn't support XGMII. Add USXGMII support to the > Aquantia PHY driver and change the phy connection type for the two > boards. > > As an additional note: Even though the handle is named aqr106 > there seem to be LS1046A boards with an AQR107. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> You can probably split the DTS changes and the PHY driver changes into a separate commits and just have the DTS changes come last? With that: Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
On 22.05.2019 22:07, Florian Fainelli wrote: > On 5/22/19 12:58 PM, Heiner Kallweit wrote: >> So far we didn't support mode USXGMII, and in order to not break the >> two Freescale boards mode XGMII was accepted for the AQR107 family >> even though it doesn't support XGMII. Add USXGMII support to the >> Aquantia PHY driver and change the phy connection type for the two >> boards. >> >> As an additional note: Even though the handle is named aqr106 >> there seem to be LS1046A boards with an AQR107. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > You can probably split the DTS changes and the PHY driver changes into a > separate commits and just have the DTS changes come last? With that: > To split the patches I would have to do: 1. Add USXGMII support to Aquantia PHY driver 2. DTS changes 3. Don't accept XGMII any longer in Aquantia PHY driver This seemed to me to be too much overhead considering the very small change. Just making the DTS changes a separate patch would break bisecting. > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> >
On 5/22/19 1:18 PM, Heiner Kallweit wrote: > On 22.05.2019 22:07, Florian Fainelli wrote: >> On 5/22/19 12:58 PM, Heiner Kallweit wrote: >>> So far we didn't support mode USXGMII, and in order to not break the >>> two Freescale boards mode XGMII was accepted for the AQR107 family >>> even though it doesn't support XGMII. Add USXGMII support to the >>> Aquantia PHY driver and change the phy connection type for the two >>> boards. >>> >>> As an additional note: Even though the handle is named aqr106 >>> there seem to be LS1046A boards with an AQR107. >>> >>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> >> You can probably split the DTS changes and the PHY driver changes into a >> separate commits and just have the DTS changes come last? With that: >> > To split the patches I would have to do: > 1. Add USXGMII support to Aquantia PHY driver > 2. DTS changes > 3. Don't accept XGMII any longer in Aquantia PHY driver > This seemed to me to be too much overhead considering the very small > change. > > Just making the DTS changes a separate patch would break bisecting. I fail to see how, you can't make use of "usxgmii" in DTS unless you define that as as a valid phy-mode property value (patch #1), and you can't have that possibly working until patch #2. Until then using "xgmii" is still supported and going to work. Once patch #3 which brings DTS lands in, you could possibly deprecate "xgmii" in the Aquantia PHY driver (or rather issue a big warning).
On Wed, May 22, 2019 at 09:58:32PM +0200, Heiner Kallweit wrote: > So far we didn't support mode USXGMII, and in order to not break the > two Freescale boards mode XGMII was accepted for the AQR107 family > even though it doesn't support XGMII. Add USXGMII support to the > Aquantia PHY driver and change the phy connection type for the two > boards. > > As an additional note: Even though the handle is named aqr106 > there seem to be LS1046A boards with an AQR107. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts | 2 +- > arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts | 2 +- > drivers/net/phy/aquantia_main.c | 6 +++++- > 3 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts > index 4223a2352..c2ce1a611 100644 > --- a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts > @@ -139,7 +139,7 @@ > > ethernet@f0000 { /* 10GEC1 */ > phy-handle = <&aqr105_phy>; > - phy-connection-type = "xgmii"; > + phy-connection-type = "usxgmii"; > }; > > mdio@fc000 { > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts > index 6a6514d0e..f927a8a25 100644 > --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts > @@ -147,7 +147,7 @@ > > ethernet@f0000 { /* 10GEC1 */ > phy-handle = <&aqr106_phy>; > - phy-connection-type = "xgmii"; > + phy-connection-type = "usxgmii"; > }; > > ethernet@f2000 { /* 10GEC2 */ > diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c > index 0fedd28fd..3f24c42a8 100644 > @@ -487,7 +491,7 @@ static int aqr107_config_init(struct phy_device *phydev) > /* Check that the PHY interface type is compatible */ > if (phydev->interface != PHY_INTERFACE_MODE_SGMII && > phydev->interface != PHY_INTERFACE_MODE_2500BASEX && > - phydev->interface != PHY_INTERFACE_MODE_XGMII && > + phydev->interface != PHY_INTERFACE_MODE_USXGMII && > phydev->interface != PHY_INTERFACE_MODE_10GKR) > return -ENODEV; Hi Heiner Just to reiterate Florian's point. We need to be careful with device tree blobs. We should try not to break them, at least not for a few cycles. I would much prefer to see a WARN_ON(phydev->interface == PHY_INTERFACE_MODE_XGMII, "Your devicetree is out of date, please update it"); and accept XGMII for this cycle. These are development boards, so in theory users are developers, so should know how to update the DT. Andrew
On 22.05.2019 22:58, Andrew Lunn wrote: > On Wed, May 22, 2019 at 09:58:32PM +0200, Heiner Kallweit wrote: >> So far we didn't support mode USXGMII, and in order to not break the >> two Freescale boards mode XGMII was accepted for the AQR107 family >> even though it doesn't support XGMII. Add USXGMII support to the >> Aquantia PHY driver and change the phy connection type for the two >> boards. >> >> As an additional note: Even though the handle is named aqr106 >> there seem to be LS1046A boards with an AQR107. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts | 2 +- >> arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts | 2 +- >> drivers/net/phy/aquantia_main.c | 6 +++++- >> 3 files changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts >> index 4223a2352..c2ce1a611 100644 >> --- a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts >> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts >> @@ -139,7 +139,7 @@ >> >> ethernet@f0000 { /* 10GEC1 */ >> phy-handle = <&aqr105_phy>; >> - phy-connection-type = "xgmii"; >> + phy-connection-type = "usxgmii"; >> }; >> >> mdio@fc000 { >> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts >> index 6a6514d0e..f927a8a25 100644 >> --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts >> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts >> @@ -147,7 +147,7 @@ >> >> ethernet@f0000 { /* 10GEC1 */ >> phy-handle = <&aqr106_phy>; >> - phy-connection-type = "xgmii"; >> + phy-connection-type = "usxgmii"; >> }; >> >> ethernet@f2000 { /* 10GEC2 */ >> diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c >> index 0fedd28fd..3f24c42a8 100644 >> @@ -487,7 +491,7 @@ static int aqr107_config_init(struct phy_device *phydev) >> /* Check that the PHY interface type is compatible */ >> if (phydev->interface != PHY_INTERFACE_MODE_SGMII && >> phydev->interface != PHY_INTERFACE_MODE_2500BASEX && >> - phydev->interface != PHY_INTERFACE_MODE_XGMII && >> + phydev->interface != PHY_INTERFACE_MODE_USXGMII && >> phydev->interface != PHY_INTERFACE_MODE_10GKR) >> return -ENODEV; > > Hi Heiner > > Just to reiterate Florian's point. We need to be careful with device > tree blobs. We should try not to break them, at least not for a few > cycles. > > I would much prefer to see a > > WARN_ON(phydev->interface == PHY_INTERFACE_MODE_XGMII, > "Your devicetree is out of date, please update it"); > > and accept XGMII for this cycle. These are development boards, so in > theory users are developers, so should know how to update the DT. > I see your point. Then I'll just change phylib and will let the NXP guys change the board DTS. > Andrew > Heiner
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts index 4223a2352..c2ce1a611 100644 --- a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts +++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts @@ -139,7 +139,7 @@ ethernet@f0000 { /* 10GEC1 */ phy-handle = <&aqr105_phy>; - phy-connection-type = "xgmii"; + phy-connection-type = "usxgmii"; }; mdio@fc000 { diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts index 6a6514d0e..f927a8a25 100644 --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts @@ -147,7 +147,7 @@ ethernet@f0000 { /* 10GEC1 */ phy-handle = <&aqr106_phy>; - phy-connection-type = "xgmii"; + phy-connection-type = "usxgmii"; }; ethernet@f2000 { /* 10GEC2 */ diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c index 0fedd28fd..3f24c42a8 100644 --- a/drivers/net/phy/aquantia_main.c +++ b/drivers/net/phy/aquantia_main.c @@ -27,6 +27,7 @@ #define MDIO_PHYXS_VEND_IF_STATUS_TYPE_MASK GENMASK(7, 3) #define MDIO_PHYXS_VEND_IF_STATUS_TYPE_KR 0 #define MDIO_PHYXS_VEND_IF_STATUS_TYPE_XFI 2 +#define MDIO_PHYXS_VEND_IF_STATUS_TYPE_USXGMII 3 #define MDIO_PHYXS_VEND_IF_STATUS_TYPE_SGMII 6 #define MDIO_PHYXS_VEND_IF_STATUS_TYPE_OCSGMII 10 @@ -360,6 +361,9 @@ static int aqr107_read_status(struct phy_device *phydev) case MDIO_PHYXS_VEND_IF_STATUS_TYPE_XFI: phydev->interface = PHY_INTERFACE_MODE_10GKR; break; + case MDIO_PHYXS_VEND_IF_STATUS_TYPE_USXGMII: + phydev->interface = PHY_INTERFACE_MODE_USXGMII; + break; case MDIO_PHYXS_VEND_IF_STATUS_TYPE_SGMII: phydev->interface = PHY_INTERFACE_MODE_SGMII; break; @@ -487,7 +491,7 @@ static int aqr107_config_init(struct phy_device *phydev) /* Check that the PHY interface type is compatible */ if (phydev->interface != PHY_INTERFACE_MODE_SGMII && phydev->interface != PHY_INTERFACE_MODE_2500BASEX && - phydev->interface != PHY_INTERFACE_MODE_XGMII && + phydev->interface != PHY_INTERFACE_MODE_USXGMII && phydev->interface != PHY_INTERFACE_MODE_10GKR) return -ENODEV;
So far we didn't support mode USXGMII, and in order to not break the two Freescale boards mode XGMII was accepted for the AQR107 family even though it doesn't support XGMII. Add USXGMII support to the Aquantia PHY driver and change the phy connection type for the two boards. As an additional note: Even though the handle is named aqr106 there seem to be LS1046A boards with an AQR107. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts | 2 +- arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts | 2 +- drivers/net/phy/aquantia_main.c | 6 +++++- 3 files changed, 7 insertions(+), 3 deletions(-)