netdev - Re: [PATCH v1 2/3] net/fsl: acpize xgmac

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200619150248.GP1551@shell.armlinux.org.uk>
Date:   Fri, 19 Jun 2020 16:02:48 +0100
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Calvin Johnson <calvin.johnson@....nxp.com>
Cc:     Jeremy Linton <jeremy.linton@....com>, Jon <jon@...id-run.com>,
        Cristi Sovaiala <cristian.sovaiala@....com>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Andrew Lunn <andrew@...n.ch>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Madalin Bucur <madalin.bucur@....nxp.com>,
        netdev@...r.kernel.org, linux.cj@...il.com
Subject: Re: [PATCH v1 2/3] net/fsl: acpize xgmac_mdio

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@....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@....com>
> > > Signed-off-by: Calvin Johnson <calvin.johnson@....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!