Hi, Ran Wang <ran.wan...@nxp.com> writes: >> Yinbo Zhu <yinbo....@nxp.com> writes: >> >> Yinbo Zhu <yinbo....@nxp.com> writes: >> >> >> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >> >> >> > index >> >> >> > 98bce85c29d0..a133d8490322 100644 >> >> >> > --- a/drivers/usb/dwc3/core.c >> >> >> > +++ b/drivers/usb/dwc3/core.c >> >> >> > @@ -300,8 +300,7 @@ static void >> >> >> > dwc3_frame_length_adjustment(struct >> >> >> > dwc3 *dwc) >> >> >> > >> >> >> > reg = dwc3_readl(dwc->regs, DWC3_GFLADJ); >> >> >> > dft = reg & DWC3_GFLADJ_30MHZ_MASK; >> >> >> > - if (!dev_WARN_ONCE(dwc->dev, dft == dwc->fladj, >> >> >> > - "request value same as default, ignoring\n")) { >> >> >> > + if (dft != dwc->fladj) { >> >> >> >> >> >> if the value isn't different, why do you want to change it? >> >> >> >> >> >> -- >> >> >> Balbi >> >> > Hi Balbi, >> >> > >> >> > I don't change any value. I was remove that call trace. >> >> >> >> Sure you do. The splat only shows when you request a FLADJ value >> >> that's the same as the one already in the register. The reason you >> >> see the splat is because your requested value is what's already in the HW. >> >> >> >> So, again, why are you adding this device tree property if the value >> >> is already the correct one? >> >> >> >> > In addition that GFLADJ_30MHZ value intial value is 0, and it's >> >> > value must be 0x20, if not, usb will not work. >> >> >> >> it's not zero, otherwise the splat wouldn't trigger. You're >> >> requesting the value that's already in your register by default. >> >> >> >> -- >> >> Balbi >> > >> > Hi Balbi, >> > >> > According that rm doc that GFLADJ_30MHZ has a default value is 0x20, >> > when GFLADJ_30MHZ_REG_SEL is 0, this 0x20 is a hard-coded value. >> > >> > But in fact, that default value is 0, please you note! >> > >> > Then according that xhci spec 5.2.4, that register the sixth bit if is >> > 0, then that can support Frame Lenth Timing value. >> > >> > So set GFLADJ_30MHZ_REG_SEL to 1 for use FLADJ, then I find that it >> > must use 0x20 usb will work well, even thoug xhci can permit >> > GFLADJ_30MHZ use other value >> >> You only get the splat because you try to sent GFLADJ to 0x20 and it's >> ALREADY >> 0x20. This means that you don't need the property in DTS. >> >> > In addition about what you said is about dts patch, and that patch had >> > merged by upstream, patch owner isn't me, >> >> Well, then remove the setting from DTS, since clearly it's not needed. > > Please considering below scenarios on the same board which needs GFLADJ > property on kernel DTS: > > 1. Board boot to U-Boot first, then load kernel. In this case, we need kernel > DTS > help to get GFLADJ setting right, everything is as expected. > > 2. Board boot to U-Boot console, then execute 'usb start' under U-Boot > console to init > DWC3 controller, then load kernel. In this case, actually GFLADJ is > correctly > configured already, and the GFLADJ config double-checking is fine > (because kernel > cannot know if U-Boot has initialized it or not), but warning looks not > necessary. > > 3. Board boot to kernel, GFLADJ get set from DTS, then system suspend & > resume. In this case > when resuming, GFLADJ setting has been restored correctly, so here we > might not need > send out the warning message (double-checking might be fine). > > So, what's your suggestion to remove this looks non-necessary warning message?
now this is well explained! So the value in the register is *NOT* 0x20 by default, however, u-boot _can_ use dwc3 if we're flashing, then it'll result in the splat. Okay, this is a valid scenario that the kernel should consider. I agree that we should remove the WARN() from there. Thanks -- balbi
signature.asc
Description: PGP signature