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

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]

Date:   Sun, 13 Jun 2021 21:34:22 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Marcin Wojtas <mw@...ihalf.com>
Cc:     linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        davem@...emloft.net, kuba@...nel.org, linux@...linux.org.uk,
        jaz@...ihalf.com, gjb@...ihalf.com, upstream@...ihalf.com,
        Samer.El-Haj-Mahmoud@....com, jon@...id-run.com
Subject: Re: [net-next: PATCH 1/3] net: mvmdio: add ACPI support

> @@ -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