RE: [PATCH net-next 4/5] onsemi: ncn260xx: Add driver support for NCN26010 and TS2500 MAC-PHY

[Selvamani Rajagopal]

> -----Original Message-----
> From: Andrew Lunn <andrew@xxxxxxx>
> Sent: Friday, May 1, 2026 1:22 PM
> To: Selvamani Rajagopal <Selvamani.Rajagopal@xxxxxxxxxx>
> Cc: Piergiorgio Beruto <Pier.Beruto@xxxxxxxxxx>; andrew+netdev@xxxxxxx;
> davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx;
> netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH net-next 4/5] onsemi: ncn260xx: Add driver support for NCN26010
> and TS2500 MAC-PHY
> 
> 
> This Message Is From an External Sender
> This message came from outside your organization.
> 
> > +static int mmd2mms(int mmd)
> > +{
> > + int ret = -EOPNOTSUPP;
> > +
> > + switch (mmd) {
> > + case MDIO_MMD_PCS:
> > + ret = OA_TC6_PHY_C45_PCS_MMS2;
> > + break;
> > + case MDIO_MMD_PMAPMD:
> > + ret = OA_TC6_PHY_C45_PMA_PMD_MMS3;
> > + break;
> > + case MDIO_MMD_VEND2:
> > + ret = OA_TC6_PHY_C45_VS_PLCA_MMS4;
> > + break;
> > + case MDIO_MMD_VEND1:
> > + ret = ONMPH_OA_TC6_VEND1_MMS12;
> > + break;
> > + default:
> > + break;
> > + }
> > + return ret;
> > +}
> 
> So you seem to be compliant with the standard. I've not seen anything
> use MDIO_MMD_POWER_UNIT so not having that should not be an
> issue. MDIO_MMD_AN is used by a number of PHYs, but i assume yours
> does not.
> 
> 802.3 C45 says that if a register does not exist, it should read
> 0. What would happen if a read was made to
> OA_TC6_PHY_C45_AUTO_NEG_MMS5, rather than returning EOPNOTSUPP?
> 
> Table 6 says nothing about MMD 30, which you map to 12. 10-15 are
> defined as vendor specific, so that is O.K.
> 
> But can we simply this. Add something like
> 
> void oa_tc6_set_vend1_mms(struct oa_tc6 *tc6, int mms)
> {
> tc6->vend1_mms = mms;
> }
> 
> and make oa_tc6_get_phy_c45_mms() look at its value?

Sure. I can do that. 

Since we don't support PCS, and POWER_UNIT, may I suggest alternative solution?
What if, instead of passing mms, we pass a function pointer?.

In oa_tc6.c, we will add a function, 
void oa_tc6_set_mmd_mms_mapper(struct oa_tc6 *tc6, int (*mapper_function)(int devnum))
{
    tc6-> mmd_mms_mapper = mapper;
}

And all the calls to oa_tc6_get_phy_c45_mms in oa_tc6, could be replaced with tc6->mmd_mms_mapper(denum)

This gives flexibility to vendors as mostly likely, all the vendors may not share the same set of MDIO_MMD_XXX support.

> 
> Andrew