RE: [PATCH net-next 2/2] net: dsa: microchip: Add SGMII port support to KSZ9477 switch

[Tristram.Ha]

> > +static void port_sgmii_r(struct ksz_device *dev, uint port, u16 devid, u16 reg,
> > +                      u16 *buf, u16 len)
> > +{
> > +     u32 data;
> > +
> > +     port_sgmii_s(dev, port, devid, reg, len);
> > +     while (len) {
> > +             ksz_pread32(dev, port, REG_PORT_SGMII_DATA__4, &data);
> > +             *buf++ = (u16)data;
> > +             len--;
> > +     }
> > +}
> > +
> > +static void port_sgmii_w(struct ksz_device *dev, uint port, u16 devid, u16 reg,
> > +                      u16 *buf, u16 len)
> > +{
> > +     u32 data;
> > +
> > +     port_sgmii_s(dev, port, devid, reg, len);
> > +     while (len) {
> > +             data = *buf++;
> > +             ksz_pwrite32(dev, port, REG_PORT_SGMII_DATA__4, data);
> > +             len--;
> > +     }
> > +}
> 
> This kind of looks like a C45 only MDIO bus.
> 
> #define MMD_DEVICE_ID_VENDOR_MII        0x1F
> 
> #define SR_MII                          MMD_DEVICE_ID_VENDOR_MII
> 
> This is identical to MDIO_MMD_VEND2.
> 
> #define SR_MII_RESET                    BIT(15)
> #define SR_MII_LOOPBACK                 BIT(14)
> #define SR_MII_SPEED_100MBIT            BIT(13)
> #define SR_MII_AUTO_NEG_ENABLE          BIT(12)
> #define SR_MII_POWER_DOWN               BIT(11)
> #define SR_MII_AUTO_NEG_RESTART         BIT(9)
> #define SR_MII_FULL_DUPLEX              BIT(8)
> #define SR_MII_SPEED_1000MBIT           BIT(6)
> 
> A standard BMCR.
> 
> #define MMD_SR_MII_STATUS               0x0001
> #define MMD_SR_MII_ID_1                 0x0002
> #define MMD_SR_MII_ID_2                 0x0003
> #define MMD_SR_MII_AUTO_NEGOTIATION     0x0004
> 
> Same as:
> 
> #define MII_BMSR                0x01    /* Basic mode status register  */
> #define MII_PHYSID1             0x02    /* PHYS ID 1                   */
> #define MII_PHYSID2             0x03    /* PHYS ID 2                   */
> #define MII_ADVERTISE           0x04    /* Advertisement control reg   */
> 
> So i think your first patch should be to replace all these with the
> standard macros.
> 
> That will then help make it clearer how much is generic, could the
> existing helpers be used, probably with a wrapper to make your C22
> device mapped to C45 MDIO_MMD_VEND2 look like a C22 device?

I just realize there are already 1000BASEX definitions in mii.h that can
be used.  I will try to use generic names as much as possible.