netdev - Re: [gpio:for-next 67/67] pch_gbe_main.c:undefined reference to `devm_gpio_request

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-next>] [day] [month] [year] [list]

Message-ID: <1382822142.23829.46.camel@dvhart-mobl4.amr.corp.intel.com>
Date:	Sat, 26 Oct 2013 22:15:42 +0100
From:	Darren Hart <dvhart@...ux.intel.com>
To:	David Cohen <david.a.cohen@...ux.intel.com>
Cc:	Linus Walleij <linus.walleij@...aro.org>,
	"David S. Miller" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Fengguang Wu <fengguang.wu@...el.com>,
	Alexandre Courbot <acourbot@...dia.com>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>
Subject: Re: [gpio:for-next 67/67] pch_gbe_main.c:undefined reference to
 `devm_gpio_request_one'

On Sat, 2013-10-26 at 21:33 +0100, Darren Hart wrote:
> On Fri, 2013-10-25 at 14:25 -0700, David Cohen wrote:
> > On 10/25/2013 02:21 PM, David Cohen wrote:
> > > Hi Linus,
> > >
> > > On 10/25/2013 03:49 AM, Linus Walleij wrote:
> > >> On Fri, Oct 25, 2013 at 12:41 PM, Linus Walleij
> > >> <linus.walleij@...aro.org> wrote:
> > >>
> > >>>> I wouldn't object to adding a dependency to GPIO_PCH and GPIOLIB
> > >>>> unconditionally for PCH_GBE as GPIO_PCH is the same chip... but I don't
> > >>>> know if David Miller would be amenable to that.
> > >>>
> > >>> Well we should probably just stick a dependency to GPIOLIB in there.
> > >>>
> > >>> - It #includes <linux/gpio.h>
> > >>> - It uses gpiolib functions to do something vital
> > >>>
> > >>> It was just happy that dummy versions were slotted in until now.
> > >>
> > >> ...or maybe I'm just confused now?
> > >>
> > >> Should we just add a static inline stub of devm_gpio_request_one()?
> > >
> > > I am not familiar with the HW. But checking the code, platform
> > > initialization should fail with a dummy devm_gpio_request_one()
> > > implementation. IMO it makes more sense to depend on GPIOLIB.
> > 
> > Actually, forget about it. Despite driver_data->platform_init() may
> > return error, probe() never checks for it. I think the driver must be
> > fixed, but in meanwhile a static inline stub seems reasonable.
> > 
> 
> Ah, that's a problem, and one I created :/ I'm traveling a bit through
> Europe atm for the conferences. I will try and have a look on the planes
> and add a check.

Ah, now I remember. The situation is this. If there is a cable plugged
into the jack, the PHY will not go to sleep. If there isn't, there is a
good chance it will go to sleep before the driver initializes. If it is
asleep, the scan for the PHY will fail. If it isn't, the scan will work
correctly and the device will be properly setup. The code currently
displays a dev error:

	ret = devm_gpio_request_one(&pdev->dev, gpio, flags,
				    "minnow_phy_reset");
	if (ret) {
		dev_err(&pdev->dev,
			"ERR: Can't request PHY reset GPIO line '%d'\n", gpio);

But deliberately does not about the probe because there is a chance
things will work just fine. If they do not, the PHY detection code will
print display errors about a failure to communicate over RGMII, and the
device probe will fail from there.

This still seems like the correct approach to me. Does anyone disagree?

(we still need to sort out the GPIOLIB and GPIO_SCH dependency though of
course)

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html