| Message ID | 20240822013721.203161-3-wei.fang@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | add "nxp,phy-output-refclk" to instead of "nxp,rmii-refclk-in" | expand |
On Thu, Aug 22, 2024 at 09:37:20AM +0800, Wei Fang wrote: > As the new property "nxp,phy-output-refclk" is added to instead of > the "nxp,rmii-refclk-in" property, so replace the "nxp,rmii-refclk-in" > property used in the driver with the "nxp,reverse-mode" property and > make slight modifications. Can you explain what makes this backwards compatible please? > > Signed-off-by: Wei Fang <wei.fang@nxp.com> > --- > V2 changes: > 1. Changed the property name. > --- > drivers/net/phy/nxp-tja11xx.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/phy/nxp-tja11xx.c b/drivers/net/phy/nxp-tja11xx.c > index 2c263ae44b4f..7aa0599c38c3 100644 > --- a/drivers/net/phy/nxp-tja11xx.c > +++ b/drivers/net/phy/nxp-tja11xx.c > @@ -78,8 +78,7 @@ > #define MII_COMMCFG 27 > #define MII_COMMCFG_AUTO_OP BIT(15) > > -/* Configure REF_CLK as input in RMII mode */ > -#define TJA110X_RMII_MODE_REFCLK_IN BIT(0) > +#define TJA11XX_REVERSE_MODE BIT(0) > > struct tja11xx_priv { > char *hwmon_name; > @@ -274,10 +273,10 @@ static int tja11xx_get_interface_mode(struct phy_device *phydev) > mii_mode = MII_CFG1_REVMII_MODE; > break; > case PHY_INTERFACE_MODE_RMII: > - if (priv->flags & TJA110X_RMII_MODE_REFCLK_IN) > - mii_mode = MII_CFG1_RMII_MODE_REFCLK_IN; > - else > + if (priv->flags & TJA11XX_REVERSE_MODE) > mii_mode = MII_CFG1_RMII_MODE_REFCLK_OUT; > + else > + mii_mode = MII_CFG1_RMII_MODE_REFCLK_IN; > break; > default: > return -EINVAL; > @@ -517,8 +516,8 @@ static int tja11xx_parse_dt(struct phy_device *phydev) > if (!IS_ENABLED(CONFIG_OF_MDIO)) > return 0; > > - if (of_property_read_bool(node, "nxp,rmii-refclk-in")) > - priv->flags |= TJA110X_RMII_MODE_REFCLK_IN; > + if (of_property_read_bool(node, "nxp,phy-output-refclk")) > + priv->flags |= TJA11XX_REVERSE_MODE; > > return 0; > } > -- > 2.34.1 >
> -----Original Message----- > From: Conor Dooley <conor@kernel.org> > Sent: 2024年8月22日 16:47 > To: Wei Fang <wei.fang@nxp.com> > Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; > conor+dt@kernel.org; andrew@lunn.ch; f.fainelli@gmail.com; > hkallweit1@gmail.com; linux@armlinux.org.uk; Andrei Botila (OSS) > <andrei.botila@oss.nxp.com>; netdev@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v2 net-next 2/3] net: phy: tja11xx: replace > "nxp,rmii-refclk-in" with "nxp,phy-output-refclk" > > On Thu, Aug 22, 2024 at 09:37:20AM +0800, Wei Fang wrote: > > As the new property "nxp,phy-output-refclk" is added to instead of the > > "nxp,rmii-refclk-in" property, so replace the "nxp,rmii-refclk-in" > > property used in the driver with the "nxp,reverse-mode" property and > > make slight modifications. > > Can you explain what makes this backwards compatible please? > It does not backward compatible, the related PHY nodes in DTS also need to be updated. I have not seen "nxp,rmii-refclk-in" used in the upstream. For nodes that do not use " nxp,rmii-refclk-in", they need to be updated, but unfortunately I cannot confirm which DTS use TJA11XX PHY, and there may be no relevant nodes in upstream DTS. > > > > Signed-off-by: Wei Fang <wei.fang@nxp.com> > > --- > > V2 changes: > > 1. Changed the property name. > > --- > > drivers/net/phy/nxp-tja11xx.c | 13 ++++++------- > > 1 file changed, 6 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/net/phy/nxp-tja11xx.c > > b/drivers/net/phy/nxp-tja11xx.c index 2c263ae44b4f..7aa0599c38c3 > > 100644 > > --- a/drivers/net/phy/nxp-tja11xx.c > > +++ b/drivers/net/phy/nxp-tja11xx.c > > @@ -78,8 +78,7 @@ > > #define MII_COMMCFG 27 > > #define MII_COMMCFG_AUTO_OP BIT(15) > > > > -/* Configure REF_CLK as input in RMII mode */ > > -#define TJA110X_RMII_MODE_REFCLK_IN BIT(0) > > +#define TJA11XX_REVERSE_MODE BIT(0) > > > > struct tja11xx_priv { > > char *hwmon_name; > > @@ -274,10 +273,10 @@ static int tja11xx_get_interface_mode(struct > phy_device *phydev) > > mii_mode = MII_CFG1_REVMII_MODE; > > break; > > case PHY_INTERFACE_MODE_RMII: > > - if (priv->flags & TJA110X_RMII_MODE_REFCLK_IN) > > - mii_mode = MII_CFG1_RMII_MODE_REFCLK_IN; > > - else > > + if (priv->flags & TJA11XX_REVERSE_MODE) > > mii_mode = MII_CFG1_RMII_MODE_REFCLK_OUT; > > + else > > + mii_mode = MII_CFG1_RMII_MODE_REFCLK_IN; > > break; > > default: > > return -EINVAL; > > @@ -517,8 +516,8 @@ static int tja11xx_parse_dt(struct phy_device > *phydev) > > if (!IS_ENABLED(CONFIG_OF_MDIO)) > > return 0; > > > > - if (of_property_read_bool(node, "nxp,rmii-refclk-in")) > > - priv->flags |= TJA110X_RMII_MODE_REFCLK_IN; > > + if (of_property_read_bool(node, "nxp,phy-output-refclk")) > > + priv->flags |= TJA11XX_REVERSE_MODE; > > > > return 0; > > } > > -- > > 2.34.1 > >
On Thu, Aug 22, 2024 at 09:37:11AM +0000, Wei Fang wrote: > > -----Original Message----- > > From: Conor Dooley <conor@kernel.org> > > Sent: 2024年8月22日 16:47 > > To: Wei Fang <wei.fang@nxp.com> > > Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; > > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; > > conor+dt@kernel.org; andrew@lunn.ch; f.fainelli@gmail.com; > > hkallweit1@gmail.com; linux@armlinux.org.uk; Andrei Botila (OSS) > > <andrei.botila@oss.nxp.com>; netdev@vger.kernel.org; > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH v2 net-next 2/3] net: phy: tja11xx: replace > > "nxp,rmii-refclk-in" with "nxp,phy-output-refclk" > > > > On Thu, Aug 22, 2024 at 09:37:20AM +0800, Wei Fang wrote: > > > As the new property "nxp,phy-output-refclk" is added to instead of the > > > "nxp,rmii-refclk-in" property, so replace the "nxp,rmii-refclk-in" > > > property used in the driver with the "nxp,reverse-mode" property and > > > make slight modifications. > > > > Can you explain what makes this backwards compatible please? > > > It does not backward compatible, the related PHY nodes in DTS also > need to be updated. I have not seen "nxp,rmii-refclk-in" used in the > upstream. Since you have switched the polarity, devicestrees that contain "nxp,rmii-refclk-in" would actually not need an update to preserve functionality. However... > For nodes that do not use " nxp,rmii-refclk-in", they need > to be updated, but unfortunately I cannot confirm which DTS use > TJA11XX PHY, and there may be no relevant nodes in upstream DTS. ...as you say here, all tja11xx phy nodes that do not have the property would need to be updated to retain functionality. Given you can't even determine which devicetrees would need to be updated, I'm going to have to NAK this change as an unnecessary ABI break. Thanks, Conor. > > > > > > > Signed-off-by: Wei Fang <wei.fang@nxp.com> > > > --- > > > V2 changes: > > > 1. Changed the property name. > > > --- > > > drivers/net/phy/nxp-tja11xx.c | 13 ++++++------- > > > 1 file changed, 6 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/net/phy/nxp-tja11xx.c > > > b/drivers/net/phy/nxp-tja11xx.c index 2c263ae44b4f..7aa0599c38c3 > > > 100644 > > > --- a/drivers/net/phy/nxp-tja11xx.c > > > +++ b/drivers/net/phy/nxp-tja11xx.c > > > @@ -78,8 +78,7 @@ > > > #define MII_COMMCFG 27 > > > #define MII_COMMCFG_AUTO_OP BIT(15) > > > > > > -/* Configure REF_CLK as input in RMII mode */ > > > -#define TJA110X_RMII_MODE_REFCLK_IN BIT(0) > > > +#define TJA11XX_REVERSE_MODE BIT(0) > > > > > > struct tja11xx_priv { > > > char *hwmon_name; > > > @@ -274,10 +273,10 @@ static int tja11xx_get_interface_mode(struct > > phy_device *phydev) > > > mii_mode = MII_CFG1_REVMII_MODE; > > > break; > > > case PHY_INTERFACE_MODE_RMII: > > > - if (priv->flags & TJA110X_RMII_MODE_REFCLK_IN) > > > - mii_mode = MII_CFG1_RMII_MODE_REFCLK_IN; > > > - else > > > + if (priv->flags & TJA11XX_REVERSE_MODE) > > > mii_mode = MII_CFG1_RMII_MODE_REFCLK_OUT; > > > + else > > > + mii_mode = MII_CFG1_RMII_MODE_REFCLK_IN; > > > break; > > > default: > > > return -EINVAL; > > > @@ -517,8 +516,8 @@ static int tja11xx_parse_dt(struct phy_device > > *phydev) > > > if (!IS_ENABLED(CONFIG_OF_MDIO)) > > > return 0; > > > > > > - if (of_property_read_bool(node, "nxp,rmii-refclk-in")) > > > - priv->flags |= TJA110X_RMII_MODE_REFCLK_IN; > > > + if (of_property_read_bool(node, "nxp,phy-output-refclk")) > > > + priv->flags |= TJA11XX_REVERSE_MODE; > > > > > > return 0; > > > } > > > -- > > > 2.34.1 > > >
> -----Original Message----- > From: Conor Dooley <conor@kernel.org> > Sent: 2024年8月23日 0:14 > To: Wei Fang <wei.fang@nxp.com> > Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; > conor+dt@kernel.org; andrew@lunn.ch; f.fainelli@gmail.com; > hkallweit1@gmail.com; linux@armlinux.org.uk; Andrei Botila (OSS) > <andrei.botila@oss.nxp.com>; netdev@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v2 net-next 2/3] net: phy: tja11xx: replace > "nxp,rmii-refclk-in" with "nxp,phy-output-refclk" > > On Thu, Aug 22, 2024 at 09:37:11AM +0000, Wei Fang wrote: > > > -----Original Message----- > > > From: Conor Dooley <conor@kernel.org> > > > Sent: 2024年8月22日 16:47 > > > To: Wei Fang <wei.fang@nxp.com> > > > Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; > > > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; > > > conor+dt@kernel.org; andrew@lunn.ch; f.fainelli@gmail.com; > > > hkallweit1@gmail.com; linux@armlinux.org.uk; Andrei Botila (OSS) > > > <andrei.botila@oss.nxp.com>; netdev@vger.kernel.org; > > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > > > Subject: Re: [PATCH v2 net-next 2/3] net: phy: tja11xx: replace > > > "nxp,rmii-refclk-in" with "nxp,phy-output-refclk" > > > > > > On Thu, Aug 22, 2024 at 09:37:20AM +0800, Wei Fang wrote: > > > > As the new property "nxp,phy-output-refclk" is added to instead of > > > > the "nxp,rmii-refclk-in" property, so replace the "nxp,rmii-refclk-in" > > > > property used in the driver with the "nxp,reverse-mode" property > > > > and make slight modifications. > > > > > > Can you explain what makes this backwards compatible please? > > > > > It does not backward compatible, the related PHY nodes in DTS also > > need to be updated. I have not seen "nxp,rmii-refclk-in" used in the > > upstream. > > Since you have switched the polarity, devicestrees that contain > "nxp,rmii-refclk-in" would actually not need an update to preserve > functionality. However... > > > For nodes that do not use " nxp,rmii-refclk-in", they need to be > > updated, but unfortunately I cannot confirm which DTS use TJA11XX PHY, > > and there may be no relevant nodes in upstream DTS. > > ...as you say here, all tja11xx phy nodes that do not have the property would > need to be updated to retain functionality. Given you can't even determine > which devicetrees would need to be updated, I'm going to have to NAK this > change as an unnecessary ABI break. > Okay, that make sense, "nxp,rmii-refclk-in" was added only for TJA1100 and TJA1101, although it does not seem to be a suitable property now, it cannot be changed at present. :( Since TJA1103/TJA1104/TJA1120/TJA1121 use different driver than TJA1100 and TJA1101, which is nxp-c4-tja11xx. I think it's fine to add " nxp,phy-output-refclk " for these PHYs, so I will remove this patch from the patch set.
On Fri, Aug 23, 2024 at 01:31:02AM +0000, Wei Fang wrote: > > -----Original Message----- > > From: Conor Dooley <conor@kernel.org> > > Sent: 2024年8月23日 0:14 > > To: Wei Fang <wei.fang@nxp.com> > > Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; > > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; > > conor+dt@kernel.org; andrew@lunn.ch; f.fainelli@gmail.com; > > hkallweit1@gmail.com; linux@armlinux.org.uk; Andrei Botila (OSS) > > <andrei.botila@oss.nxp.com>; netdev@vger.kernel.org; > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: Re: [PATCH v2 net-next 2/3] net: phy: tja11xx: replace > > "nxp,rmii-refclk-in" with "nxp,phy-output-refclk" > > > > On Thu, Aug 22, 2024 at 09:37:11AM +0000, Wei Fang wrote: > > > > -----Original Message----- > > > > From: Conor Dooley <conor@kernel.org> > > > > Sent: 2024年8月22日 16:47 > > > > To: Wei Fang <wei.fang@nxp.com> > > > > Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; > > > > pabeni@redhat.com; robh@kernel.org; krzk+dt@kernel.org; > > > > conor+dt@kernel.org; andrew@lunn.ch; f.fainelli@gmail.com; > > > > hkallweit1@gmail.com; linux@armlinux.org.uk; Andrei Botila (OSS) > > > > <andrei.botila@oss.nxp.com>; netdev@vger.kernel.org; > > > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > > > > Subject: Re: [PATCH v2 net-next 2/3] net: phy: tja11xx: replace > > > > "nxp,rmii-refclk-in" with "nxp,phy-output-refclk" > > > > > > > > On Thu, Aug 22, 2024 at 09:37:20AM +0800, Wei Fang wrote: > > > > > As the new property "nxp,phy-output-refclk" is added to instead of > > > > > the "nxp,rmii-refclk-in" property, so replace the "nxp,rmii-refclk-in" > > > > > property used in the driver with the "nxp,reverse-mode" property > > > > > and make slight modifications. > > > > > > > > Can you explain what makes this backwards compatible please? > > > > > > > It does not backward compatible, the related PHY nodes in DTS also > > > need to be updated. I have not seen "nxp,rmii-refclk-in" used in the > > > upstream. > > > > Since you have switched the polarity, devicestrees that contain > > "nxp,rmii-refclk-in" would actually not need an update to preserve > > functionality. However... > > > > > For nodes that do not use " nxp,rmii-refclk-in", they need to be > > > updated, but unfortunately I cannot confirm which DTS use TJA11XX PHY, > > > and there may be no relevant nodes in upstream DTS. > > > > ...as you say here, all tja11xx phy nodes that do not have the property would > > need to be updated to retain functionality. Given you can't even determine > > which devicetrees would need to be updated, I'm going to have to NAK this > > change as an unnecessary ABI break. > > > > Okay, that make sense, "nxp,rmii-refclk-in" was added only for TJA1100 and > TJA1101, although it does not seem to be a suitable property now, it cannot > be changed at present. :( > Since TJA1103/TJA1104/TJA1120/TJA1121 use different driver than TJA1100 > and TJA1101, which is nxp-c4-tja11xx. I think it's fine to add " nxp,phy-output-refclk " > for these PHYs, so I will remove this patch from the patch set. If they use a different binding, then yeah, you can add use the new name/polarity for those devices.
diff --git a/drivers/net/phy/nxp-tja11xx.c b/drivers/net/phy/nxp-tja11xx.c index 2c263ae44b4f..7aa0599c38c3 100644 --- a/drivers/net/phy/nxp-tja11xx.c +++ b/drivers/net/phy/nxp-tja11xx.c @@ -78,8 +78,7 @@ #define MII_COMMCFG 27 #define MII_COMMCFG_AUTO_OP BIT(15) -/* Configure REF_CLK as input in RMII mode */ -#define TJA110X_RMII_MODE_REFCLK_IN BIT(0) +#define TJA11XX_REVERSE_MODE BIT(0) struct tja11xx_priv { char *hwmon_name; @@ -274,10 +273,10 @@ static int tja11xx_get_interface_mode(struct phy_device *phydev) mii_mode = MII_CFG1_REVMII_MODE; break; case PHY_INTERFACE_MODE_RMII: - if (priv->flags & TJA110X_RMII_MODE_REFCLK_IN) - mii_mode = MII_CFG1_RMII_MODE_REFCLK_IN; - else + if (priv->flags & TJA11XX_REVERSE_MODE) mii_mode = MII_CFG1_RMII_MODE_REFCLK_OUT; + else + mii_mode = MII_CFG1_RMII_MODE_REFCLK_IN; break; default: return -EINVAL; @@ -517,8 +516,8 @@ static int tja11xx_parse_dt(struct phy_device *phydev) if (!IS_ENABLED(CONFIG_OF_MDIO)) return 0; - if (of_property_read_bool(node, "nxp,rmii-refclk-in")) - priv->flags |= TJA110X_RMII_MODE_REFCLK_IN; + if (of_property_read_bool(node, "nxp,phy-output-refclk")) + priv->flags |= TJA11XX_REVERSE_MODE; return 0; }
As the new property "nxp,phy-output-refclk" is added to instead of the "nxp,rmii-refclk-in" property, so replace the "nxp,rmii-refclk-in" property used in the driver with the "nxp,reverse-mode" property and make slight modifications. Signed-off-by: Wei Fang <wei.fang@nxp.com> --- V2 changes: 1. Changed the property name. --- drivers/net/phy/nxp-tja11xx.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)