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

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

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