Re: [PATCH v1 2/3] net/fsl: acpize xgmac_mdio

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Fri, Jun 19, 2020 at 08:29:27PM +0530, Calvin Johnson wrote:
> On Wed, Jun 17, 2020 at 06:49:31PM +0100, Russell King - ARM Linux admin wrote:
> > On Wed, Jun 17, 2020 at 10:45:34PM +0530, Calvin Johnson wrote:
> > > From: Jeremy Linton <jeremy.linton@arm.com>
> > > 
> > > Add ACPI support for xgmac MDIO bus registration while maintaining
> > > the existing DT support.
> > > 
> > > The function mdiobus_register() inside of_mdiobus_register(), brings
> > > up all the PHYs on the mdio bus and attach them to the bus.
> > > 
> > > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> > > Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com>
> > > ---
> > > 
> > >  drivers/net/ethernet/freescale/xgmac_mdio.c | 27 +++++++++++++--------
> > >  1 file changed, 17 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
> > > index c82c85ef5fb3..fb7f8caff643 100644
> > > --- a/drivers/net/ethernet/freescale/xgmac_mdio.c
> > > +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
> > > @@ -245,14 +245,14 @@ static int xgmac_mdio_probe(struct platform_device *pdev)
> > >  {
> > >  	struct device_node *np = pdev->dev.of_node;
> > >  	struct mii_bus *bus;
> > > -	struct resource res;
> > > +	struct resource *res;
> > >  	struct mdio_fsl_priv *priv;
> > >  	int ret;
> > >  
> > > -	ret = of_address_to_resource(np, 0, &res);
> > > -	if (ret) {
> > > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > +	if (!res) {
> > >  		dev_err(&pdev->dev, "could not obtain address\n");
> > > -		return ret;
> > > +		return -EINVAL;
> > >  	}
> > 
> > I think, as you're completely rewriting the resource handling, it would
> > be a good idea to switch over to using devm_* stuff here.
> > 
> > 	void __iomem *regs;
> > 
> > 	regs = devm_platform_ioremap_resource(pdev, 0);
> 
> I had used devm_ API earlier in this place and ran into a regression.
> This mdio driver is used by both DPAA-1 and DPAA-2. In DPAA2 case, this
> works fine.
> 
> But in DPAA-1 case, the existing device tree describes the memory map in a
> hierarchical manner. The FMan of DPAA-1 area include the MDIO, Port, MAC areas.
> Therefore, we may have to continue with existing method.

And you need to document that in comments in the driver, otherwise you
will have people come along and try to "clean up" the driver.

You can still use devm_ioremap() though.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!