[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6501a53b-40aa-5374-3c4a-6b21824f82fd@gmail.com>
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
Powered by blists - more mailing lists