netdev - 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]

Date:	Fri, 18 Mar 2016 20:12:42 +0100
From:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>
To:	Sebastian Frias <sf84@...oste.net>
Cc:	Daniel Mack <daniel@...que.org>,
	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
	lkml <linux-kernel@...r.kernel.org>, mason <slash.tmp@...e.fr>,
	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

Hello Sebastian,

On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote:
> On 03/18/2016 01:54 PM, Uwe Kleine-König wrote:
> > From a driver perspecitive, it would be nice if devm_gpiod_get_optional
> > returned NULL iff the respective gpio isn't specified even with
> > GPIOLIB=n, but this isn't sensible either because it would result in
> > quite some gpiolib code to not being conditionally compiled on
> > CONFIG_GPIOLIB any more.
> 
> Let's say that was the case, what would the PHY code do?

With reset gpios it might not be that critical, but consider an optional
enable gpio. (Optional in the sense, that some device have it and others
don't, e.g. because the pin is pulled into active level by hardware.)

Now you do:

	gpiod = gpiod_get_optional("enable");

and if gpiod now is an error pointer, you must assume that you cannot
operate the device. And even with GPIOLIB=n (and gpiod = ERR_PTR(-ENOSYS))
you cannot ignore the error. For consistency I'd recommend to do the
same for reset even though there is a chance to get a working device.

> What would you think of making at803x_link_change_notify() print a
> message every time it should do a reset but does not has a way to do it?

Then this question is obsolete because the device doesn't probe.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |