On Mon, Jun 12, 2017 at 11:57:43AM +0200, Antoine Tenart wrote: > +static void orion_mdio_xsmi_start_read_op(struct orion_mdio_dev *dev, > + int mii_id, int regnum) > +{ > + u16 dev_addr = (regnum >> 16) & GENMASK(4, 0); > + > + writel(regnum & GENMASK(15, 0), dev->regs + MVMDIO_XSMI_ADDR_REG); > + writel((mii_id << MVMDIO_XSMI_PHYADDR_SHIFT) | > + (dev_addr << MVMDIO_XSMI_DEVADDR_SHIFT) | > + MVMDIO_XSMI_READ_OPERATION, > + dev->regs + MVMDIO_XSMI_MGNT_REG);
So what happens if this function gets passed a Clause 22 formatted request? Both regnum and mii_id are in the range 0-31. MII_ADDR_C45 in regnum is clear. The answer is we produce two Clause 45 frames on the bus, which is certainly not correct. You need to trap and error out if MII_ADDR_C45 is not set (as I've already said previously.) The SMI operations need to do the reverse - they need to fail if they receive a regnum with MII_ADDR_C45 set, as they are unable to produce Clause 45 frames. > +static void orion_mdio_xsmi_write_op(struct orion_mdio_dev *dev, int mii_id, > + int regnum, u16 value) > +{ > + u16 dev_addr = (regnum >> 16) & GENMASK(4, 0); > + > + writel(regnum & GENMASK(15, 0), dev->regs + MVMDIO_XSMI_ADDR_REG); > + writel((mii_id << MVMDIO_XSMI_PHYADDR_SHIFT) | > + (dev_addr << MVMDIO_XSMI_DEVADDR_SHIFT) | > + MVMDIO_XSMI_WRITE_OPERATION | value, > + dev->regs + MVMDIO_XSMI_MGNT_REG); It's even more important here, as a Clause 22 write will produce a valid Clause 45 pair of frames with dev_addr = 0. Again, the corresponding SMI code needs to reject Clause 45 requests as the SMI interface is unable to produce Clause 45 frames. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.