linux-kernel - RE: [PATCH v7 3/5] usb: dwc2: add generic PHY framework support for dwc2 usb controler platform driver.

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:	Tue, 13 Jan 2015 00:54:45 +0000
From:	Paul Zimmerman <Paul.Zimmerman@...opsys.com>
To:	Yunzhi Li <lyz@...k-chips.com>,
	"Dinh Nguyen (dinguyen@...nsource.altera.com)" 
	<dinguyen@...nsource.altera.com>,
	"robert.jarzmik@...e.fr" <robert.jarzmik@...e.fr>
CC:	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"heiko@...ech.de" <heiko@...ech.de>,
	"jwerner@...omium.org" <jwerner@...omium.org>,
	"dianders@...omium.org" <dianders@...omium.org>,
	"olof@...om.net" <olof@...om.net>,
	"huangtao@...k-chips.com" <huangtao@...k-chips.com>,
	"zyw@...k-chips.com" <zyw@...k-chips.com>,
	"cf@...k-chips.com" <cf@...k-chips.com>,
	"linux-rockchip@...ts.infradead.org" 
	<linux-rockchip@...ts.infradead.org>,
	"linus.walleij@...aro.org" <linus.walleij@...aro.org>
Subject: RE: [PATCH v7 3/5] usb: dwc2: add generic PHY framework support for
 dwc2 usb controler platform driver.

> From: Paul Zimmerman
> Sent: Saturday, January 10, 2015 3:52 PM
> 
> > From: Yunzhi Li [mailto:lyz@...k-chips.com]
> > Sent: Saturday, January 10, 2015 8:07 AM
> >
> > 在 2015/1/9 10:15, Paul Zimmerman 写道:
> > >> [...]
> > >>   	/*
> > >> -	 * Attempt to find a generic PHY, then look for an old style
> > >> -	 * USB PHY, finally fall back to pdata
> > >> +	 * If platform probe couldn't find a generic PHY or an old style
> > >> +	 * USB PHY, fall back to pdata
> > >>   	 */
> > >> -	phy = devm_phy_get(dev, "usb2-phy");
> > >> -	if (IS_ERR(phy)) {
> > >> -		uphy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> > >> -		if (IS_ERR(uphy)) {
> > >> -			/* Fallback for pdata */
> > >> -			plat = dev_get_platdata(dev);
> > >> -			if (!plat) {
> > >> -				dev_err(dev,
> > >> -				"no platform data or transceiver defined\n");
> > >> -				return -EPROBE_DEFER;
> > >> -			}
> > >> -			hsotg->plat = plat;
> > >> -		} else
> > >> -			hsotg->uphy = uphy;
> > >> -	} else {
> > >> -		hsotg->phy = phy;
> > >> +	if (IS_ERR_OR_NULL(hsotg->phy) && IS_ERR_OR_NULL(hsotg->uphy)) {
> > >> +		plat = dev_get_platdata(dev);
> > >> +		if (!plat) {
> > >> +			dev_err(dev,
> > >> +			"no platform data or transceiver defined\n");
> > >> +			return -EPROBE_DEFER;
> > > Hi Yunzhi,
> > >
> > > Testing Felipe's testing/next branch on an Altera SOCFPGA platform,
> > > the driver never loads because it always returns -EPROBE_DEFER here.
> > > Apparently the SOCFPGA platform does not have any platform data
> > > defined, because dev_get_platdata() always returns NULL.
> > >
> > > If I remove the -EPROBE_DEFER return and have it continue on, the
> > > driver works. Reverting the patch also makes it work.
> > 
> > When I debug this problem, I checked socfpga.dtsi, there is a
> > usbphy node defined for each
> > dwc2 controller, so I think when running dwc2_driver_probe() uphy =
> > devm_usb_get_phy()
> > should get a valid usbphy pointer and hsotg->uphy will not be NULL or
> > ERROR, then in dwc2_gadget_init()
> > it will not return -EPROBE_DEFER. I have no idea about why you meet
> > -EPROBE_DEFER, could you please tell
> > me what's the return value of devm_usb_get_phy() on your socfpga board ?
> 
> I'm away from the hardware right now, but I just found this in a saved
> boot log:
> 
> [    1.097268] usb_phy_generic soc:usbphy@0: Error requesting RESET GPIO
> [    1.097285] usb_phy_generic: probe of soc:usbphy@0 failed with error -2
> 
> So that probably explains it. I'll dig into this some more on Monday.

The patch below fixes it. And it seems like the right thing to me,
since GPIOs should be optional for a generic phy, I would think. But
my device tree foo is very weak, so I'm not sure.

CCing Robert, who touched the generic phy code last. Robert, what do
you think?

-- 
Paul


diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index dd05254..9a826ff 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -218,10 +218,10 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop,
 			clk_rate = 0;
 
 		needs_vcc = of_property_read_bool(node, "vcc-supply");
-		nop->gpiod_reset = devm_gpiod_get(dev, "reset-gpios");
+		nop->gpiod_reset = devm_gpiod_get_optional(dev, "reset-gpios");
 		err = PTR_ERR(nop->gpiod_reset);
 		if (!err) {
-			nop->gpiod_vbus = devm_gpiod_get(dev,
+			nop->gpiod_vbus = devm_gpiod_get_optional(dev,
 							 "vbus-detect-gpio");
 			err = PTR_ERR(nop->gpiod_vbus);
 		}
--