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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 22, 2024 at 09:37:11AM +0000, Wei Fang wrote:
> > -----Original Message-----
> > From: Conor Dooley <conor@xxxxxxxxxx>
> > Sent: 2024年8月22日 16:47
> > To: Wei Fang <wei.fang@xxxxxxx>
> > Cc: davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> > pabeni@xxxxxxxxxx; robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx;
> > conor+dt@xxxxxxxxxx; andrew@xxxxxxx; f.fainelli@xxxxxxxxx;
> > hkallweit1@xxxxxxxxx; linux@xxxxxxxxxxxxxxx; Andrei Botila (OSS)
> > <andrei.botila@xxxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx;
> > devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > 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@xxxxxxx>
> > > ---
> > > 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: signature.asc
Description: PGP signature


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux