Wesley Cheng wrote: > > On 1/8/2021 4:44 PM, Thinh Nguyen wrote: >> Hi, >> >> John Stultz wrote: >>> On Fri, Jan 8, 2021 at 4:26 AM Felipe Balbi <balbi@xxxxxxxxxx> wrote: >>>> Hi, >>>> >>>> John Stultz <john.stultz@xxxxxxxxxx> writes: >>>>> From: Yu Chen <chenyu56@xxxxxxxxxx> >>>>> >>>>> Just resending this, as discussion died out a bit and I'm not >>>>> sure how to make further progress. See here for debug data that >>>>> was requested last time around: >>>>> https://urldefense.com/v3/__https://lore.kernel.org/lkml/CALAqxLXdnaUfJKx0aN9xWwtfWVjMWigPpy2aqsNj56yvnbU80g@xxxxxxxxxxxxxx/__;!!A4F2R9G_pg!LNzuprAeg-O80SgolYkIkW4-ne-M-yLWCDUY9MygAIrQC398Z6gRJ9wnsnlqd3w$ >>>>> >>>>> With the current dwc3 code on the HiKey960 we often see the >>>>> COREIDLE flag get stuck off in __dwc3_gadget_start(), which >>>>> seems to prevent the reset irq and causes the USB gadget to >>>>> fail to initialize. >>>>> >>>>> We had seen occasional initialization failures with older >>>>> kernels but with recent 5.x era kernels it seemed to be becoming >>>>> much more common, so I dug back through some older trees and >>>>> realized I dropped this quirk from Yu Chen during upstreaming >>>>> as I couldn't provide a proper rational for it and it didn't >>>>> seem to be necessary. I now realize I was wrong. >>>>> >>>>> After resubmitting the quirk, Thinh Nguyen pointed out that it >>>>> shouldn't be a quirk at all and it is actually mentioned in the >>>>> programming guide that it should be done when switching modes >>>>> in DRD. >>>>> >>>>> So, to avoid these !COREIDLE lockups seen on HiKey960, this >>>>> patch issues GCTL soft reset when switching modes if the >>>>> controller is in DRD mode. >>>>> >>>>> Cc: Felipe Balbi <balbi@xxxxxxxxxx> >>>>> Cc: Tejas Joglekar <tejas.joglekar@xxxxxxxxxxxx> >>>>> Cc: Yang Fei <fei.yang@xxxxxxxxx> >>>>> Cc: YongQin Liu <yongqin.liu@xxxxxxxxxx> >>>>> Cc: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxxxx> >>>>> Cc: Thinh Nguyen <thinhn@xxxxxxxxxxxx> >>>>> Cc: Jun Li <lijun.kernel@xxxxxxxxx> >>>>> Cc: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> >>>>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> >>>>> Cc: linux-usb@xxxxxxxxxxxxxxx >>>>> Signed-off-by: Yu Chen <chenyu56@xxxxxxxxxx> >>>>> Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx> >>>>> --- >>>>> v2: >>>>> * Rework to always call the GCTL soft reset in DRD mode, >>>>> rather then using a quirk as suggested by Thinh Nguyen >>>>> >>>>> v3: >>>>> * Move GCTL soft reset under the spinlock as suggested by >>>>> Thinh Nguyen >>>> Because this is such an invasive change, I would prefer that we get >>>> Tested-By tags from a good fraction of the users before applying these >>>> two changes. >>> I'm happy to reach out to folks to try to get that. Though I'm >>> wondering if it would be better to put it behind a dts quirk flag, as >>> originally proposed? >>> https://urldefense.com/v3/__https://lore.kernel.org/lkml/20201021181803.79650-1-john.stultz@xxxxxxxxxx/__;!!A4F2R9G_pg!LNzuprAeg-O80SgolYkIkW4-ne-M-yLWCDUY9MygAIrQC398Z6gRJ9wnRWITZfc$ >>> >>> That way folks can enable it for devices as they need? >>> >>> Again, I'm not trying to force this in as-is, just mostly sending it >>> out again for discussion to understand what other approach might work. >>> >>> thanks >>> -john >> A quirk would imply something is broken/diverged from the design right? >> But it's not the case here, and at least this is needed for HiKey960. >> Also, I think Rob will be ok with not adding 1 more quirk to the dwc3 >> devicetree. :) >> >> BR, >> Thinh >> > Hi All, > > Sorry for jumping in, but I checked the SNPS v1.90a databook, and that > seemed to remove the requirement for the GCTL.softreset before writing > to PRTCAPDIR. Should we consider adding a controller version/IP check? > Hi Wesley, >From what I see in the v1.90a databook and others, the flow remains the same. I need to check internally, but I'm not aware of the change. BR, Thinh
