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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ffa13558-8ca0-00e5-9603-db8ee25a483e@gmail.com>
Date:   Mon, 14 May 2018 13:52:01 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Alexandre Belloni <alexandre.belloni@...tlin.com>,
        "David S . Miller" <davem@...emloft.net>
Cc:     Allan Nielsen <Allan.Nielsen@...rosemi.com>,
        razvan.stefanescu@....com, po.liu@....com,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Andrew Lunn <andrew@...n.ch>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-mips@...ux-mips.org
Subject: Re: [PATCH net-next v3 2/7] net: phy: mscc-miim: Add MDIO driver

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@...n.ch>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@...tlin.com>

Nothing critical, so:

Reviewed-by: Florian Fainelli <f.fainelli@...il.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