[PATCH net-next v2 4/9] net: phy: ncn26000: Support for internal phy for S2500

[PATCH net-next v2 4/9] net: phy: ncn26000: Support for internal phy for S2500

[Selvamani Rajagopal]

Adding support for internal PHY of the integrated
media access controller S2500

Signed-off-by: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
---
 drivers/net/phy/ncn26000.c | 41 ++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/ncn26000.c b/drivers/net/phy/ncn26000.c
index d302e04c8..1961d7ff3 100644
--- a/drivers/net/phy/ncn26000.c
+++ b/drivers/net/phy/ncn26000.c
@@ -14,6 +14,7 @@
 
 #include "mdio-open-alliance.h"
 
+#define PHY_ID_S2500			0x180FF411
 #define PHY_ID_NCN26000			0x180FF5A1
 
 #define NCN26000_REG_IRQ_CTL            16
@@ -41,13 +42,15 @@
 
 static int ncn26000_config_init(struct phy_device *phydev)
 {
-	/* HW bug workaround: the default value of the PLCA TO_TIMER should be
-	 * 32, where the current version of NCN26000 reports 24. This will be
-	 * fixed in future PHY versions. For the time being, we force the
-	 * correct default here.
-	 */
-	return phy_write_mmd(phydev, MDIO_MMD_VEND2, MDIO_OATC14_PLCA_TOTMR,
-			     TO_TMR_DEFAULT);
+	const struct phy_driver *pdrv = phydev->drv;
+	int ret = 0;
+
+  /* NCN26000 PHY didn't have the default value */
+	if (pdrv->phy_id == PHY_ID_NCN26000)
+		ret = phy_modify_mmd(phydev, MDIO_MMD_VEND2,
+				     MDIO_OATC14_PLCA_TOTMR,
+				     0xFFFF, TO_TMR_DEFAULT);
+	return ret;
 }
 
 static int ncn26000_config_aneg(struct phy_device *phydev)
@@ -163,7 +166,7 @@ static int ncn26000_config_intr(struct phy_device *phydev)
 }
 
 /* Directly accessing via Clause 45 read avoids the extra
- * SPI accesses required by indirect access.
+ * SPI transactions required by indirect access.
  */
 static int ncn26000_read_mmd(struct phy_device *phydev, int dev, u16 reg)
 {
@@ -196,6 +199,27 @@ static int ncn26000_loopback(struct phy_device *phydev, bool enable, int speed)
 }
 
 static struct phy_driver ncn26000_driver[] = {
+	{
+		PHY_ID_MATCH_MODEL(PHY_ID_S2500),
+		.name                  = "S2500",
+		.features              = PHY_BASIC_T1S_P2MP_FEATURES,
+		.config_init           = ncn26000_config_init,
+		.config_intr           = ncn26000_config_intr,
+		.config_aneg           = ncn26000_config_aneg,
+		.read_status           = ncn26000_read_status,
+		.handle_interrupt      = ncn26000_handle_interrupt,
+		.set_loopback          = ncn26000_loopback,
+		.set_plca_cfg          = ncn26000_plca_set_cfg,
+		.get_plca_cfg          = genphy_c45_plca_get_cfg,
+		.get_plca_status       = genphy_c45_plca_get_status,
+		.soft_reset            = genphy_soft_reset,
+		.get_sqi               = genphy_c45_oatc14_get_sqi,
+		.get_sqi_max           = genphy_c45_oatc14_get_sqi_max,
+		.read_mmd              = ncn26000_read_mmd,
+		.write_mmd             = ncn26000_write_mmd,
+		.cable_test_get_status = genphy_c45_oatc14_cable_test_get_status,
+		.cable_test_start      = genphy_c45_oatc14_cable_test_start,
+	},
 	{
 		PHY_ID_MATCH_MODEL(PHY_ID_NCN26000),
 		.name                  = "NCN26000",
@@ -219,6 +243,7 @@ module_phy_driver(ncn26000_driver);
 
 static const struct mdio_device_id __maybe_unused ncn26000_tbl[] = {
 	{ PHY_ID_MATCH_MODEL(PHY_ID_NCN26000) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_S2500) },
 	{ }
 };
 
-- 
2.43.0

Re: [PATCH net-next v2 4/9] net: phy: ncn26000: Support for internal phy for S2500

[Andrew Lunn]

> @@ -163,7 +166,7 @@ static int ncn26000_config_intr(struct phy_device *phydev)
>  }
>  
>  /* Directly accessing via Clause 45 read avoids the extra
> - * SPI accesses required by indirect access.
> + * SPI transactions required by indirect access.
>   */

Please don't change things you added in the previous patch.

	Andrew

Re: [PATCH net-next v2 4/9] net: phy: ncn26000: Support for internal phy for S2500

[Andrew Lunn]

> +  /* NCN26000 PHY didn't have the default value */
> +	if (pdrv->phy_id == PHY_ID_NCN26000)


> +		PHY_ID_MATCH_MODEL(PHY_ID_S2500),

Since you are using PHY_ID_MATCH_MODEL(), you should use
phy_id_compare_model().

	Andrew

RE: [PATCH net-next v2 4/9] net: phy: ncn26000: Support for internal phy for S2500

[Selvamani Rajagopal]

> 
> 
> This Message Is From an External Sender
> This message came from outside your organization.
> 
> > @@ -163,7 +166,7 @@ static int ncn26000_config_intr(struct phy_device *phydev)
> > }
> >
> > /* Directly accessing via Clause 45 read avoids the extra
> > - * SPI accesses required by indirect access.
> > + * SPI transactions required by indirect access.
> > */
> 
> Please don't change things you added in the previous patch.

I thought v1 is abandoned once v2 is submitted. Didn't realized we diff v1 -> v2 too. Will be careful in avoiding it in future.

> 
> Andrew

Re: [PATCH net-next v2 4/9] net: phy: ncn26000: Support for internal phy for S2500

[Andrew Lunn]

On Mon, May 11, 2026 at 07:35:15PM +0000, Selvamani Rajagopal wrote:
> 
> 
> > 
> > 
> > This Message Is From an External Sender
> > This message came from outside your organization.
> > 
> > > @@ -163,7 +166,7 @@ static int ncn26000_config_intr(struct phy_device *phydev)
> > > }
> > >
> > > /* Directly accessing via Clause 45 read avoids the extra
> > > - * SPI accesses required by indirect access.
> > > + * SPI transactions required by indirect access.
> > > */
> > 
> > Please don't change things you added in the previous patch.
> 
> I thought v1 is abandoned once v2 is submitted. Didn't realized we diff v1 -> v2 too. Will be careful in avoiding it in future.

Wasn't this line added in a previous patch in this series? Patch 3/9 ?

       Andrew

RE: [PATCH net-next v2 4/9] net: phy: ncn26000: Support for internal phy for S2500

[Selvamani Rajagopal]

> > >
> > > This Message Is From an External Sender
> > > This message came from outside your organization.
> > >
> > > > @@ -163,7 +166,7 @@ static int ncn26000_config_intr(struct phy_device *phydev)
> > > > }
> > > >
> > > > /* Directly accessing via Clause 45 read avoids the extra
> > > > - * SPI accesses required by indirect access.
> > > > + * SPI transactions required by indirect access.
> > > > */
> > >
> > > Please don't change things you added in the previous patch.
> >
> > I thought v1 is abandoned once v2 is submitted. Didn't realized we diff v1 -> v2 too. Will
> be careful in avoiding it in future.
> 
> Wasn't this line added in a previous patch in this series? Patch 3/9 ?

Yes. Sorry about that. 

> 
> Andrew