> +static int > +ipq8064_mdio_read(struct mii_bus *bus, int phy_addr, int reg_offset) > +{ > + struct ipq8064_mdio *priv = bus->priv; > + u32 miiaddr = MII_BUSY | MII_CLKRANGE_250_300M; > + u32 ret_val; > + int err; Hi Ansuel Reverse Christmas tree. priv needs to move down a line. > + > + /* Reject clause 45 */ > + if (reg_offset & MII_ADDR_C45) > + return -EOPNOTSUPP; > + > + miiaddr |= ((phy_addr << MII_ADDR_SHIFT) & MII_ADDR_MASK) | > + ((reg_offset << MII_REG_SHIFT) & MII_REG_MASK); > + > + regmap_write(priv->base, MII_ADDR_REG_ADDR, miiaddr); > + usleep_range(8, 10); > + > + err = ipq8064_mdio_wait_busy(priv); > + if (err) > + return err; > + > + regmap_read(priv->base, MII_DATA_REG_ADDR, &ret_val); > + return (int)ret_val; > +} > + > +static int > +ipq8064_mdio_write(struct mii_bus *bus, int phy_addr, int reg_offset, u16 data) > +{ > + struct ipq8064_mdio *priv = bus->priv; > + u32 miiaddr = MII_WRITE | MII_BUSY | MII_CLKRANGE_250_300M; Same here. > +static int > +ipq8064_mdio_probe(struct platform_device *pdev) > +{ > + struct device_node *np = pdev->dev.of_node; > + struct ipq8064_mdio *priv; > + struct mii_bus *bus; > + int ret; > + > + bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*priv)); > + if (!bus) > + return -ENOMEM; > + > + bus->name = "ipq8064_mdio_bus"; > + bus->read = ipq8064_mdio_read; > + bus->write = ipq8064_mdio_write; > + snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(&pdev->dev)); > + bus->parent = &pdev->dev; > + > + priv = bus->priv; > + priv->base = syscon_node_to_regmap(np); > + if (IS_ERR(priv->base) && priv->base != ERR_PTR(-EPROBE_DEFER)) > + priv->base = syscon_regmap_lookup_by_phandle(np, "master"); "master" is not documented in the binding. Do we really need two different ways to get the base address? Andrew