Messages in this thread |  | | From | Calvin Johnson <> | Date | Wed, 5 Feb 2020 00:16:39 +0530 | Subject | Re: [PATCH v1 3/7] net/fsl: add ACPI support for mdio bus |
| |
On Sun, Feb 02, 2020 at 07:44:40PM -0800, Florian Fainelli wrote: > > > 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. Sure, will avoid split to newline. > > > > /* 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 Ok. Will take care in v2. > > > 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. Had some difficulty with DT while using fwnode APIs. Will resolve them and provide better integrated code.
Thanks
Calvin
|  |