usb: dwc2: regression during boot on Raspberry Pi

[John Youn]

On 11/9/2015 12:43 PM, Stefan Wahren wrote:
> Hi,
> 
> Am 09.11.2015 um 12:53 schrieb Marek Szyprowski:
>> Hello,
>>
>>
>> This change looks reasonable, I remember the similar issue was in
>> s3c-hsotg driver
>> and it caused kernel ops on driver probe, when bootloader left dwc2
>> enabled.
>> The other way of solving it would be to add
>> irq_set_status_flags(irq, IRQ_NOAUTOEN);
>> before devm_request_irq() and then call enable_irq(irq) after
>> dwc2_lowlevel_hw_enable().
>>
>> Best regards
> 
> unfortunately this fixes only the oops. I need to revert commit 
> 09a75e85779014 ("usb: dwc2: refactor common low-level hw code to 
> platform.c") to get USB working again.
> 
> In dwc2_lowlevel_hw_init the following statement on Raspberry Pi seems 
> to never come true:
> 
> if (!hsotg->phy && !hsotg->uphy && !hsotg->plat) {
> 	dev_err(hsotg->dev, "no platform data or transceiver defined\n");
> 	return -EPROBE_DEFER;
> }
> 
> This looks wrong to me. According to dwc2 binding the PHY could be 
> optional, isn't it?
> 
> Best regards
> Stefan
> 


The gadget side required it but it looks like the host side
didn't.

When Marek did the low-level hw refactor, some of the gadget
stuff was brought up to the common layer, including the returning
of -EPROBE_DEFER when no phys are defined.

I think this requirement can be removed. At least only enforce it
for the gadget as before.

Could you see if the following fixes it?

Marek, could you review or test as well?

Regards,
John



diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index efa1872..08ae922 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -212,14 +212,26 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
 	 */
 	hsotg->phy = devm_phy_get(hsotg->dev, "usb2-phy");
 	if (IS_ERR(hsotg->phy)) {
-		hsotg->phy = NULL;
-		hsotg->uphy = devm_usb_get_phy(hsotg->dev, USB_PHY_TYPE_USB2);
-		if (IS_ERR(hsotg->uphy))
-			hsotg->uphy = NULL;
+		ret = PTR_ERR(hsotg->phy);
+		if (ret == -ENODEV)
+			hsotg->phy = NULL;
 		else
-			hsotg->plat = dev_get_platdata(hsotg->dev);
+			return ret;
 	}
 
+	if (!hsotg->phy) {
+		hsotg->uphy = devm_usb_get_phy(hsotg->dev, USB_PHY_TYPE_USB2);
+		if (IS_ERR(hsotg->uphy)) {
+			ret = PTR_ERR(hsotg->uphy);
+			if (ret == -ENODEV)
+				hsotg->uphy = NULL;
+			else
+				return ret;
+		}
+	}
+
+	hsotg->plat = dev_get_platdata(hsotg->dev);
+
 	if (hsotg->phy) {
 		/*
 		 * If using the generic PHY framework, check if the PHY bus
@@ -229,11 +241,6 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
 			hsotg->phyif = GUSBCFG_PHYIF8;
 	}
 
-	if (!hsotg->phy && !hsotg->uphy && !hsotg->plat) {
-		dev_err(hsotg->dev, "no platform data or transceiver defined\n");
-		return -EPROBE_DEFER;
-	}
-
 	/* Clock */
 	hsotg->clk = devm_clk_get(hsotg->dev, "otg");
 	if (IS_ERR(hsotg->clk)) {