Messages in this thread Patch in this message | | | Date | Fri, 23 Sep 2022 23:12:28 +0200 | Subject | Re: [PATCH v4] usb: dwc3: Don't switch OTG -> peripheral if extcon is present | From | Ferry Toth <> |
| |
Hi,
Op 23-09-2022 om 18:42 schreef Andy Shevchenko: > On Thu, Sep 22, 2022 at 04:32:55PM -0700, Andrey Smirnov wrote: >> On Thu, Sep 22, 2022 at 3:23 AM Ferry Toth <fntoth@gmail.com> wrote: >>> On 22-09-2022 12:08, Andy Shevchenko wrote: >>> On Sun, Apr 03, 2022 at 09:49:07AM -0700, Andrey Smirnov wrote: > FYI: For now I sent a revert, but if we got a solution quicker we always > can choose the course of actions. > >>> If the extcon device exists, get the mode from the extcon device. If >>> the controller is DRD and the driver is unable to determine the mode, >>> only then default the dr_mode to USB_DR_MODE_PERIPHERAL. >>> >>> According to Ferry (Cc'ed) this broke Intel Merrifield platform. Ferry, can you >>> share bisect log? >>> >>> I can but not right now. But what I did was bisect between 5.18.0 (good) and 5.19.0 (bad) then when I got near the culprit (~20 remaining) based on the commit message I tried 0f01017191384e3962fa31520a9fd9846c3d352f "usb: dwc3: Don't switch OTG -> peripheral if extcon is present" (bad) and commit before that (good). >>> >>> The effect of the patch is that on Merrifield (I tested with Intel Edison Arduino board which has a HW switch to select between host and device mode) device mode works but in host mode USB is completely not working. >>> >>> Currently on host mode - when working - superfluous error messages from tusb1210 appear. When host mode is not working there are no tusb1210 messages in the logs / on the console at all. Seemingly tusb1210 is not probed, which points in the direction of a relation to extcon. >>> >>> Taking into account the late cycle, I would like to revert the change. And >>> Ferry and I would help to test any other (non-regressive) approach). >>> >>> I have not yet tested if a simple revert fixes the problem but will tonight. >>> >>> >>> I would be happy to test other approaches too. >> >> It's a bit hard for me to suggest an alternative approach without >> knowing how things are breaking in this case. I'd love to order one of >> those boards to repro and fix this on my end, but it looks like this >> HW is EOLed and out of stock in most places. If you guys know how to >> get my hands on those boards I'm all ears. > There are still some second hand Intel Edison boards flying around > (but maybe cost a bit more than expected) and there are also > Dell Venue 7 3740 tablets based on the same platform/SoC. The latter > option though requires more actions in order something to be boot > there. > > In any case, it's probably quicker to ask Ferry or me for testing. > (Although currently I have no access to the board to test OTG, it's > remote device which I can only power on and off and it has always > be in host mode.) > >> Barring that, Ferry can you dig more into this failure? E.g. is it this hunk >> >> @@ -85,7 +86,7 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc) >> * mode. If the controller supports DRD but the dr_mode is not >> * specified or set to OTG, then set the mode to peripheral. >> */ >> - if (mode == USB_DR_MODE_OTG && >> + if (mode == USB_DR_MODE_OTG && !dwc->edev && >> (!IS_ENABLED(CONFIG_USB_ROLE_SWITCH) || >> !device_property_read_bool(dwc->dev, "usb-role-switch")) && >> !DWC3_VER_IS_PRIOR(DWC3, 330A)) >> @@ -1632,6 +1633,51 @@ static void dwc3_check_params(struct dwc3 *dwc) >> } >> } >> >> that's problematic or moving > I think you wanted to revert only this line and test?
On v6.0-rc6 and reverting manually only this line
- if (mode == USB_DR_MODE_OTG && !dwc->edev &&
+ if (mode == USB_DR_MODE_OTG &&
host mode still does not work (no change visible).
> >> static int dwc3_probe(struct platform_device *pdev) >> { >> struct device *dev = &pdev->dev; >> @@ -1744,6 +1790,13 @@ static int dwc3_probe(struct platform_device *pdev) >> goto err2; >> } >> >> + dwc->edev = dwc3_get_extcon(dwc); >> + if (IS_ERR(dwc->edev)) { >> + ret = PTR_ERR(dwc->edev); >> + dev_err_probe(dwc->dev, ret, "failed to get extcon\n"); >> + goto err3; >> + } >> + >> ret = dwc3_get_dr_mode(dwc); >> if (ret) >> goto err3; >> >> to happen earlier? > It is not always possible to have an extcon driver available, that's why in > some cases the probe of it defers. I dunno how your patch supposed to work > in that case. > >> Does tracing the "mrfld_bcove_pwrsrc" driver (the >> excton provider in this case AFIACT) show anything interesting? > I believe there is nothing interesting. >
| |