Hi Andrew, Andrew Lunn <and...@lunn.ch> writes:
> -- compatible : Should be one of "marvell,mv88e6085", > +- compatible : Should be one of "marvell,mv88e6085" or > + "marvell,mv88e6390" Just curious here, mv88e6085 was choosen because it was the smaller product ID supported. Following that logic, shouldn't mv88e6190 be choosen here instead of mv88e6390? > +static const struct mv88e6xxx_ops mv88e6390_ops = { > + .set_switch_mac = mv88e6xxx_g2_set_switch_mac, > + .phy_read = mv88e6xxx_g2_smi_phy_read, > + .phy_write = mv88e6xxx_g2_smi_phy_write, > + .port_set_link = mv88e6xxx_port_set_link, > + .port_set_duplex = mv88e6xxx_port_set_duplex, > + .port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay, > + .port_set_speed = mv88e6390_port_set_speed, > +}; > + > +static const struct mv88e6xxx_ops mv88e6390x_ops = { > + .set_switch_mac = mv88e6xxx_g2_set_switch_mac, > + .phy_read = mv88e6xxx_g2_smi_phy_read, > + .phy_write = mv88e6xxx_g2_smi_phy_write, > + .port_set_link = mv88e6xxx_port_set_link, > + .port_set_duplex = mv88e6xxx_port_set_duplex, > + .port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay, > + .port_set_speed = mv88e6390x_port_set_speed, > +}; Even if it is a bit more verbose, I'd intentionally keep one mv88e6xxx_ops structure per chip. Using per-family structure is error-prone and simpler is better here. Thanks, Vivien