linux-kernel - Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <56F26D27.3020106@free.fr>
Date:	Wed, 23 Mar 2016 11:17:11 +0100
From:	Mason <slash.tmp@...e.fr>
To:	Sebastian Frias <sf84@...oste.net>,
	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
Cc:	Uwe Kleine-Koenig <u.kleine-koenig@...gutronix.de>,
	Daniel Mack <daniel@...que.org>,
	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
	lkml <linux-kernel@...r.kernel.org>,
	Florian Fainelli <f.fainelli@...il.com>,
	Mans Rullgard <mans@...sr.com>,
	Fabio Estevam <festevam@...il.com>,
	Martin Blumenstingl <martin.blumenstingl@...il.com>,
	Linus Walleij <linus.walleij@...aro.org>
Subject: Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB

On 22/03/2016 20:42, Uwe Kleine-König wrote:

> Preconditions:
>  - Some of the devices a given driver handles have a reset line and
>    others don't.
>  - A non-empty subset (maybe all) of the devices that have a reset line
>    require that this reset line is used.
> 
> Then the way to handle this in the driver should be done as follows:
> 
>   unless reset_handling_not_necessary():
>     gpio = gpiod_get_optional("reset")
>     if IS_ERR(gpio):
>       return PTR_ERR(gpio)
> 
> Checking for -ENOSYS or GPIOLIB=n is not allowed because the device
> you're currently handling might need the GPIO, so you must not continue
> without the ability to control the line.
> 
> So the options you have (as you have a phy that doesn't need the reset
> handling):
> 
>  - enable GPIOLIB (either in your .config or introduce a Kconfig
>    dependency)
>  - improve reset_handling_not_necessary() to return true for your case
> 
> There is nothing else.

Here are some numbers for GPIOLIB, on an ARM build:

   text	   data	    bss	    dec	    hex	filename
   1830	      0	      0	   1830	    726	devres.o
    627	      0	      0	    627	    273	gpiolib-legacy.o
  11018	     40	      4	  11062	   2b36	gpiolib.o
   1598	      0	      0	   1598	    63e	gpiolib-of.o
--------------------------------------------------------
  15073	     40	      4	  15117	   3b0d	built-in.o

So ~15 kilobytes.


By the way, since the "reset-by-GPIO" solution is used only for
the Atheros 8030, would it be possible to make the
devm_gpiod_get_optional conditional on ATH8030_PHY_ID?

I'm thinking of something along these lines, for illustration:

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 2d020a3ec0b5..576e7873e049 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -198,12 +198,16 @@ static int at803x_probe(struct phy_device *phydev)
        if (!priv)
                return -ENOMEM;
 
+       if (phydev->drv->phy_id != ATH8030_PHY_ID)
+               goto no_gpio;
+
        gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
        if (IS_ERR(gpiod_reset))
                return PTR_ERR(gpiod_reset);
 
        priv->gpiod_reset = gpiod_reset;
 
+no_gpio:
        phydev->priv = priv;
 
        return 0;


Regards.