linux-kernel - Re: [PATCH v1 3/7] net/fsl: add ACPI support for mdio bus

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, 2 Feb 2020 19:44:40 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Calvin Johnson <calvin.johnson@....com>, linux.cj@...il.com,
        Jon Nettleton <jon@...id-run.com>, linux@...linux.org.uk,
        Makarand Pawagi <makarand.pawagi@....com>,
        cristian.sovaiala@....com, laurentiu.tudor@....com,
        ioana.ciornei@....com, V.Sethi@....com, pankaj.bansal@....com,
        "Rajesh V . Bikkina" <rajesh.bikkina@....com>
Cc:     Calvin Johnson <calvin.johnson@....nxp.com>,
        "David S. Miller" <davem@...emloft.net>,
        Madalin Bucur <madalin.bucur@....nxp.com>,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v1 3/7] net/fsl: add ACPI support for mdio bus



On 1/31/2020 7:34 AM, Calvin Johnson wrote:
> From: Calvin Johnson <calvin.johnson@....nxp.com>
> 
> Add ACPI support for MDIO bus registration while maintaining
> the existing DT support.
> 
> Signed-off-by: Calvin Johnson <calvin.johnson@....nxp.com>
> ---

[snip]

>  	bus = mdiobus_alloc_size(sizeof(struct mdio_fsl_priv));
> @@ -263,25 +265,41 @@ static int xgmac_mdio_probe(struct platform_device *pdev)
>  	bus->read = xgmac_mdio_read;
>  	bus->write = xgmac_mdio_write;
>  	bus->parent = &pdev->dev;
> -	snprintf(bus->id, MII_BUS_ID_SIZE, "%llx", (unsigned long long)res.start);
> +	snprintf(bus->id, MII_BUS_ID_SIZE, "%llx",
> +		 (unsigned long long)res->start);

You could omit this clean up change.

>  
>  	/* Set the PHY base address */
>  	priv = bus->priv;
> -	priv->mdio_base = of_iomap(np, 0);
> +	priv->mdio_base = devm_ioremap_resource(&pdev->dev, res);
>  	if (!priv->mdio_base) {

This probably needs to become IS_ERR() instead of a plain NULL check

>  		ret = -ENOMEM;
>  		goto err_ioremap;
>  	}
>  
> -	priv->is_little_endian = of_property_read_bool(pdev->dev.of_node,
> -						       "little-endian");
> -
> -	priv->has_a011043 = of_property_read_bool(pdev->dev.of_node,
> -						  "fsl,erratum-a011043");
> -
> -	ret = of_mdiobus_register(bus, np);
> -	if (ret) {
> -		dev_err(&pdev->dev, "cannot register MDIO bus\n");
> +	if (is_of_node(pdev->dev.fwnode)) {
> +		priv->is_little_endian = of_property_read_bool(pdev->dev.of_node,
> +							       "little-endian");
> +
> +		priv->has_a011043 = of_property_read_bool(pdev->dev.of_node,
> +							  "fsl,erratum-a011043");
> +
> +		ret = of_mdiobus_register(bus, np);
> +		if (ret) {
> +			dev_err(&pdev->dev, "cannot register MDIO bus\n");
> +			goto err_registration;
> +		}
> +	} else if (is_acpi_node(pdev->dev.fwnode)) {
> +		priv->is_little_endian =
> +			fwnode_property_read_bool(pdev->dev.fwnode,
> +						  "little-endian");
> +		ret = fwnode_mdiobus_register(bus, pdev->dev.fwnode);
> +		if (ret) {
> +			dev_err(&pdev->dev, "cannot register MDIO bus\n");
> +			goto err_registration;
> +		}

The little-endian property read can be moved out of the DT/ACPI paths
and you can just use device_property_read_bool() for that purpose.
Having both fwnode_mdiobus_register() and of_mdiobus_register() looks
fairly redundant, you could quite easily introduce a wrapper:
device_mdiobus_register() which internally takes the appropriate DT/ACPI
paths as needed.
-- 
Florian