[PATCH v2 net-next 0/3] add "nxp,phy-output-refclk" to instead of "nxp,rmii-refclk-in"

[PATCH v2 net-next 0/3] add "nxp,phy-output-refclk" to instead of "nxp,rmii-refclk-in"

[Wei Fang]

The TJA11xx PHYs have the capability to provide 50MHz reference clock
in RMII mode and output on REF_CLK pin. Therefore, add the new property
"nxp,phy-output-refclk" to support this feature.

Wei Fang (3):
  dt-bindings: net: tja11xx: use phy-output-refclk to instead of
    rmii-refclk-in
  net: phy: tja11xx: replace "nxp,rmii-refclk-in" with
    "nxp,phy-output-refclk"
  net: phy: c45-tja11xx: add support for outputing RMII reference clock

 .../devicetree/bindings/net/nxp,tja11xx.yaml  | 18 ++----------
 drivers/net/phy/nxp-c45-tja11xx.c             | 29 +++++++++++++++++--
 drivers/net/phy/nxp-c45-tja11xx.h             |  1 +
 drivers/net/phy/nxp-tja11xx.c                 | 13 ++++-----
 4 files changed, 37 insertions(+), 24 deletions(-)

-- 
2.34.1

[PATCH v2 net-next 1/3] dt-bindings: net: tja11xx: use phy-output-refclk to instead of rmii-refclk-in

[Wei Fang]

Per the RMII specification, the REF_CLK is sourced from MAC to PHY
or from an external source. But for TJA11xx PHYs, they support to
output a 50MHz RMII reference clock on REF_CLK pin. Previously the
"nxp,rmii-refclk-in" was added to indicate that in RMII mode, if
this property present, REF_CLK is input to the PHY, otherwise it
is output. This seems inappropriate now. Because according to the
RMII specification, the REF_CLK is originally input, so there is
no need to add an additional "nxp,rmii-refclk-in" property to
declare that REF_CLK is input. On the contrary, a vendor-defined
property needs to be added if we want the TJA11xx PHYs to provide
the 50MHz RMII reference clock. Therefore, it is more reasonable
to add "nxp,phy-output-refclk" instead of "nxp,rmii-refclk-in".

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
V2 changes:
1. Changed the property name from "nxp,reverse-mode" to
"nxp,phy-output-refclk".
2. Simplified the description of the property.
3. Modified the subject and commit message.
---
 .../devicetree/bindings/net/nxp,tja11xx.yaml   | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml
index 85bfa45f5122..eb813f7bf274 100644
--- a/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml
+++ b/Documentation/devicetree/bindings/net/nxp,tja11xx.yaml
@@ -32,21 +32,9 @@ patternProperties:
         description:
           The ID number for the child PHY. Should be +1 of parent PHY.
 
-      nxp,rmii-refclk-in:
+      nxp,phy-output-refclk:
         type: boolean
-        description: |
-          The REF_CLK is provided for both transmitted and received data
-          in RMII mode. This clock signal is provided by the PHY and is
-          typically derived from an external 25MHz crystal. Alternatively,
-          a 50MHz clock signal generated by an external oscillator can be
-          connected to pin REF_CLK. A third option is to connect a 25MHz
-          clock to pin CLK_IN_OUT. So, the REF_CLK should be configured
-          as input or output according to the actual circuit connection.
-          If present, indicates that the REF_CLK will be configured as
-          interface reference clock input when RMII mode enabled.
-          If not present, the REF_CLK will be configured as interface
-          reference clock output when RMII mode enabled.
-          Only supported on TJA1100 and TJA1101.
+        description: Enable 50MHz RMII reference clock output on REF_CLK pin.
 
     required:
       - reg
@@ -61,7 +49,7 @@ examples:
 
         tja1101_phy0: ethernet-phy@4 {
             reg = <0x4>;
-            nxp,rmii-refclk-in;
+            nxp,phy-output-refclk;
         };
     };
   - |
-- 
2.34.1

[PATCH v2 net-next 2/3] net: phy: tja11xx: replace "nxp,rmii-refclk-in" with "nxp,phy-output-refclk"

[Wei Fang]

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(-)

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

Re: [PATCH v2 net-next 2/3] net: phy: tja11xx: replace "nxp,rmii-refclk-in" with "nxp,phy-output-refclk"

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1978 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

RE: [PATCH v2 net-next 2/3] net: phy: tja11xx: replace "nxp,rmii-refclk-in" with "nxp,phy-output-refclk"

[Wei Fang]

> -----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
> >

Re: [PATCH v2 net-next 2/3] net: phy: tja11xx: replace "nxp,rmii-refclk-in" with "nxp,phy-output-refclk"

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3747 bytes --]

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
> > >

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

RE: [PATCH v2 net-next 2/3] net: phy: tja11xx: replace "nxp,rmii-refclk-in" with "nxp,phy-output-refclk"

[Wei Fang]

> -----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.

Re: [PATCH v2 net-next 2/3] net: phy: tja11xx: replace "nxp,rmii-refclk-in" with "nxp,phy-output-refclk"

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3307 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 net-next 3/3] net: phy: c45-tja11xx: add support for outputing RMII reference clock

[Wei Fang]

For TJA11xx PHYs, they have the capability to output 50MHz reference
clock on REF_CLK pin in RMII mode, which is called "revRMII" mode in
the PHY data sheet.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
V2 changes:
1. Changed the property name.
2. Modified the subject and commit message.
---
 drivers/net/phy/nxp-c45-tja11xx.c | 29 +++++++++++++++++++++++++++--
 drivers/net/phy/nxp-c45-tja11xx.h |  1 +
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/nxp-c45-tja11xx.c b/drivers/net/phy/nxp-c45-tja11xx.c
index 5af5ade4fc64..880d4ca883a8 100644
--- a/drivers/net/phy/nxp-c45-tja11xx.c
+++ b/drivers/net/phy/nxp-c45-tja11xx.c
@@ -10,6 +10,7 @@
 #include <linux/kernel.h>
 #include <linux/mii.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/phy.h>
 #include <linux/processor.h>
 #include <linux/property.h>
@@ -185,6 +186,8 @@
 
 #define NXP_C45_SKB_CB(skb)	((struct nxp_c45_skb_cb *)(skb)->cb)
 
+#define TJA11XX_REVERSE_MODE		BIT(0)
+
 struct nxp_c45_phy;
 
 struct nxp_c45_skb_cb {
@@ -1510,6 +1513,7 @@ static int nxp_c45_get_delays(struct phy_device *phydev)
 
 static int nxp_c45_set_phy_mode(struct phy_device *phydev)
 {
+	struct nxp_c45_phy *priv = phydev->priv;
 	int ret;
 
 	ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_ABILITIES);
@@ -1561,8 +1565,13 @@ static int nxp_c45_set_phy_mode(struct phy_device *phydev)
 			phydev_err(phydev, "rmii mode not supported\n");
 			return -EINVAL;
 		}
-		phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_MII_BASIC_CONFIG,
-			      MII_BASIC_CONFIG_RMII);
+
+		if (priv->flags & TJA11XX_REVERSE_MODE)
+			phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_MII_BASIC_CONFIG,
+				      MII_BASIC_CONFIG_RMII | MII_BASIC_CONFIG_REV);
+		else
+			phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_MII_BASIC_CONFIG,
+				      MII_BASIC_CONFIG_RMII);
 		break;
 	case PHY_INTERFACE_MODE_SGMII:
 		if (!(ret & SGMII_ABILITY)) {
@@ -1623,6 +1632,20 @@ static int nxp_c45_get_features(struct phy_device *phydev)
 	return genphy_c45_pma_read_abilities(phydev);
 }
 
+static int nxp_c45_parse_dt(struct phy_device *phydev)
+{
+	struct device_node *node = phydev->mdio.dev.of_node;
+	struct nxp_c45_phy *priv = phydev->priv;
+
+	if (!IS_ENABLED(CONFIG_OF_MDIO))
+		return 0;
+
+	if (of_property_read_bool(node, "nxp,phy-output-refclk"))
+		priv->flags |= TJA11XX_REVERSE_MODE;
+
+	return 0;
+}
+
 static int nxp_c45_probe(struct phy_device *phydev)
 {
 	struct nxp_c45_phy *priv;
@@ -1642,6 +1665,8 @@ static int nxp_c45_probe(struct phy_device *phydev)
 
 	phydev->priv = priv;
 
+	nxp_c45_parse_dt(phydev);
+
 	mutex_init(&priv->ptp_lock);
 
 	phy_abilities = phy_read_mmd(phydev, MDIO_MMD_VEND1,
diff --git a/drivers/net/phy/nxp-c45-tja11xx.h b/drivers/net/phy/nxp-c45-tja11xx.h
index f364fca68f0b..8b5fc383752b 100644
--- a/drivers/net/phy/nxp-c45-tja11xx.h
+++ b/drivers/net/phy/nxp-c45-tja11xx.h
@@ -28,6 +28,7 @@ struct nxp_c45_phy {
 	int extts_index;
 	bool extts;
 	struct nxp_c45_macsec *macsec;
+	u32 flags;
 };
 
 #if IS_ENABLED(CONFIG_MACSEC)
-- 
2.34.1