Re: [net-next: PATCH 1/3] net: mvmdio: add ACPI support

From: Andrew Lunn
Date: Sun Jun 13 2021 - 15:38:45 EST


> @@ -336,7 +338,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
> dev_warn(&pdev->dev,
> "unsupported number of clocks, limiting to the first "
> __stringify(ARRAY_SIZE(dev->clk)) "\n");
> - } else {
> + } else if (!has_acpi_companion(&pdev->dev)) {
> dev->clk[0] = clk_get(&pdev->dev, NULL);
> if (PTR_ERR(dev->clk[0]) == -EPROBE_DEFER) {
> ret = -EPROBE_DEFER;

Is this needed? As you said, there are no clocks when ACPI is used, So
doesn't clk_get() return -ENODEV? Since this is not EPRODE_DEFER, it
keeps going. The clk_prepare_enable() won't be called.

> - ret = of_mdiobus_register(bus, pdev->dev.of_node);
> + if (pdev->dev.of_node)
> + ret = of_mdiobus_register(bus, pdev->dev.of_node);
> + else if (is_acpi_node(pdev->dev.fwnode))
> + ret = acpi_mdiobus_register(bus, pdev->dev.fwnode);
> + else
> + ret = -EINVAL;
> if (ret < 0) {
> dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
> goto out_mdio;
> @@ -383,6 +390,9 @@ static int orion_mdio_probe(struct platform_device *pdev)
> if (dev->err_interrupt > 0)
> writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
>
> + if (has_acpi_companion(&pdev->dev))
> + return ret;
> +

I think this can also be removed for the same reason.

We should try to avoid adding has_acpi_companion() and
!pdev->dev.of_node whenever we can. It makes the driver code too much
of a maze.

Andrew