Re: [PATCH net-next v3 2/7] net: phy: mscc-miim: Add MDIO driver

[Florian Fainelli <f.fainelli@gmail.com>]

On 05/14/2018 01:04 PM, Alexandre Belloni wrote:
> Add a driver for the Microsemi MII Management controller (MIIM) found on
> Microsemi SoCs.
> On Ocelot, there are two controllers, one is connected to the internal
> PHYs, the other one can communicate with external PHYs.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

Nothing critical, so:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

[snip]

> +static int mscc_miim_reset(struct mii_bus *bus)
> +{
> +	struct mscc_miim_dev *miim = bus->priv;
> +
> +	if (miim->phy_regs) {
> +		writel(0, miim->phy_regs + MSCC_PHY_REG_PHY_CFG);
> +		writel(0x1ff, miim->phy_regs + MSCC_PHY_REG_PHY_CFG);
> +		mdelay(500);

Can this become an msleep() in the future?

> +	}
> +
> +	return 0;
> +}
> +
> +static int mscc_miim_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	struct mii_bus *bus;
> +	struct mscc_miim_dev *dev;
> +	int ret;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +
> +	bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*dev));
> +	if (!bus)
> +		return -ENOMEM;
> +
> +	bus->name = "mscc_miim";
> +	bus->read = mscc_miim_read;
> +	bus->write = mscc_miim_write;
> +	bus->reset = mscc_miim_reset;
> +	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(&pdev->dev));
> +	bus->parent = &pdev->dev;
> +
> +	dev = bus->priv;
> +	dev->regs = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(dev->regs)) {
> +		dev_err(&pdev->dev, "Unable to map MIIM registers\n");
> +		return PTR_ERR(dev->regs);
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (res) {
> +		dev->phy_regs = devm_ioremap_resource(&pdev->dev, res);
> +		if (IS_ERR(dev->phy_regs)) {
> +			dev_err(&pdev->dev, "Unable to map internal phy registers\n");
> +			return PTR_ERR(dev->phy_regs);
> +		}
> +	}
> +
> +	if (pdev->dev.of_node)
> +		ret = of_mdiobus_register(bus, pdev->dev.of_node);
> +	else
> +		ret = mdiobus_register(bus);

There are other drivers doing that, we should probably make that the
standard behavior of of_mdiobus_register(), like before, candidate for
another patch.
-- 
Florian