RE: [PATCH net-next v3 12/14] onsemi: s2500: Add driver support for TS2500 MAC-PHY

[Selvamani Rajagopal]

> 
> > +#define S2500_MMS_MII (S2500_OA_TC6_MACPHY_MMS0 << 16)
> > +#define S2500_MMS_MAC (S2500_OA_TC6_MAC_MMS1 << 16)
> > +#define S2500_MMS_PCS (OA_TC6_PHY_C45_PCS_MMS2 << 16)
> > +#define S2500_MMS_PMDPMA (OA_TC6_PHY_C45_PMA_PMD_MMS3 << 16)
> > +#define S2500_MMS_VS1 (S2500_OA_TC6_VS1_MMS12 << 16)
> > +#define S2500_MMS_VS2 (OA_TC6_PHY_C45_VS_PLCA_MMS4 << 16)
> 
> This kind of suggests we should rethink the API for reading/writing
> registers. It should be possible to pass MMS and register as seperate
> values, and let the core combine them.


I agree. In fact, onsemi's original implementation had MMS and address as separate parameter. 
It is going to touch all the files. Let me know whether you want to do, If so, when.


> 
> 
> > + /* Convert the OID in host byte order */
> > + for (i = 2; i >= 0; --i) {
> > + addr[i] = 0;
> > + for (j = 0; j < 8; ++j) {
> > + addr[i] |= (val & 1) << (7 - j);
> > + val >>= 1;
> > + }
> > + }
> 
> That seems pretty complex. What exactly is it doing?

Could have been well commented. According to datesheet, phy-id-reg[16:31] mapped to OUI[2:17] and 
phy-id-reg[8:15] mapped to OUI[18:23]. I will add this to the comment.

> 
> Andrew