[PATCH v1] usb: dwc3: remove the call trace of USBx_GFLADJ

[PATCH v1] usb: dwc3: remove the call trace of USBx_GFLADJ

[Yinbo Zhu]

layerscape board sometimes reported some usb call trace, that is due to
kernel sent LPM tokerns automatically when it has no pending transfers
and think that the link is idle enough to enter L1, which procedure will
ask usb register has a recovery,then kernel will compare USBx_GFLADJ and
set GFLADJ_30MHZ, GFLADJ_30MHZ_REG until GFLADJ_30MHZ is equal 0x20, if
the conditions were met then issue occur, but whatever the conditions
whether were met that usb is all need keep GFLADJ_30MHZ of value is 0x20
(xhci spec ask use GFLADJ_30MHZ to adjust any offset from clock source
that generates the clock that drives the SOF counter, 0x20 is default
value of it)That is normal logic, so need remove the call trace.

Signed-off-by: Yinbo Zhu <yinbo.zhu@nxp.com>
---
 drivers/usb/dwc3/core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

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) {
 		reg &= ~DWC3_GFLADJ_30MHZ_MASK;
 		reg |= DWC3_GFLADJ_30MHZ_SDBND_SEL | dwc->fladj;
 		dwc3_writel(dwc->regs, DWC3_GFLADJ, reg);
-- 
2.17.1

Re: [PATCH v1] usb: dwc3: remove the call trace of USBx_GFLADJ

[Felipe Balbi]

Hi,

Yinbo Zhu <yinbo.zhu@nxp.com> writes:

> layerscape board sometimes reported some usb call trace, that is due to
> kernel sent LPM tokerns automatically when it has no pending transfers
> and think that the link is idle enough to enter L1, which procedure will
> ask usb register has a recovery,then kernel will compare USBx_GFLADJ and
> set GFLADJ_30MHZ, GFLADJ_30MHZ_REG until GFLADJ_30MHZ is equal 0x20, if
> the conditions were met then issue occur, but whatever the conditions
> whether were met that usb is all need keep GFLADJ_30MHZ of value is 0x20
> (xhci spec ask use GFLADJ_30MHZ to adjust any offset from clock source
> that generates the clock that drives the SOF counter, 0x20 is default
> value of it)That is normal logic, so need remove the call trace.
>
> Signed-off-by: Yinbo Zhu <yinbo.zhu@nxp.com>
> ---
>  drivers/usb/dwc3/core.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> 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

RE: [EXT] Re: [PATCH v1] usb: dwc3: remove the call trace of USBx_GFLADJ

[Yinbo Zhu]

> -----Original Message-----
> From: Felipe Balbi [mailto:balbi@kernel.org]
> Sent: 2019年8月8日 13:07
> To: Yinbo Zhu <yinbo.zhu@nxp.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; linux-usb@vger.kernel.org; open list
> <linux-kernel@vger.kernel.org>
> Cc: Yinbo Zhu <yinbo.zhu@nxp.com>; Xiaobo Xie <xiaobo.xie@nxp.com>; Jiafei
> Pan <jiafei.pan@nxp.com>; Ran Wang <ran.wang_1@nxp.com>
> Subject: [EXT] Re: [PATCH v1] usb: dwc3: remove the call trace of USBx_GFLADJ
> 
> Caution: EXT Email
> 
> Hi,
> 
> Yinbo Zhu <yinbo.zhu@nxp.com> writes:
> 
> > layerscape board sometimes reported some usb call trace, that is due
> > to kernel sent LPM tokerns automatically when it has no pending
> > transfers and think that the link is idle enough to enter L1, which
> > procedure will ask usb register has a recovery,then kernel will
> > compare USBx_GFLADJ and set GFLADJ_30MHZ, GFLADJ_30MHZ_REG until
> > GFLADJ_30MHZ is equal 0x20, if the conditions were met then issue
> > occur, but whatever the conditions whether were met that usb is all
> > need keep GFLADJ_30MHZ of value is 0x20 (xhci spec ask use
> > GFLADJ_30MHZ to adjust any offset from clock source that generates the
> > clock that drives the SOF counter, 0x20 is default value of it)That is normal logic,
> so need remove the call trace.
> >
> > Signed-off-by: Yinbo Zhu <yinbo.zhu@nxp.com>
> > ---
> >  drivers/usb/dwc3/core.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > 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.
In addition that GFLADJ_30MHZ value intial value is 0, and it's value must be 0x20, if not, usb will not work.

Regards,
Yinbo

  

RE: [EXT] Re: [PATCH v1] usb: dwc3: remove the call trace of USBx_GFLADJ

[Felipe Balbi]

Hi,

Yinbo Zhu <yinbo.zhu@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

RE: [EXT] Re: [PATCH v1] usb: dwc3: remove the call trace of USBx_GFLADJ

[Yinbo Zhu]

> -----Original Message-----
> From: Felipe Balbi [mailto:balbi@kernel.org]
> Sent: 2019年8月12日 13:27
> To: Yinbo Zhu <yinbo.zhu@nxp.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; linux-usb@vger.kernel.org; open list
> <linux-kernel@vger.kernel.org>
> Cc: Xiaobo Xie <xiaobo.xie@nxp.com>; Jiafei Pan <jiafei.pan@nxp.com>; Ran
> Wang <ran.wang_1@nxp.com>
> Subject: RE: [EXT] Re: [PATCH v1] usb: dwc3: remove the call trace of
> USBx_GFLADJ
> 
> Caution: EXT Email
> 
> Hi,
> 
> Yinbo Zhu <yinbo.zhu@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
In addition about what you said is about dts patch, and that patch had merged by upstream, patch owner isn't me, 
My patch is only for remove the call-trace, about why remove it commit information has detail introduce please check!

Thanks,
Yinbo.

RE: [EXT] Re: [PATCH v1] usb: dwc3: remove the call trace of USBx_GFLADJ

[Felipe Balbi]

Hi,

Yinbo Zhu <yinbo.zhu@nxp.com> writes:
>> Yinbo Zhu <yinbo.zhu@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.

> My patch is only for remove the call-trace, about why remove it commit
> information has detail introduce please check!

-- 
balbi

RE: [EXT] Re: [PATCH v1] usb: dwc3: remove the call trace of USBx_GFLADJ

[Ran Wang]

Hi Felipe,

On Thursday, August 15, 2019 13:59, Felipe Balbi wrote:
> 
> Hi,
> 
> Yinbo Zhu <yinbo.zhu@nxp.com> writes:
> >> Yinbo Zhu <yinbo.zhu@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?

Thanks & Regards,
Ran

> > My patch is only for remove the call-trace, about why remove it commit
> > information has detail introduce please check!
> 
> --
> balbi

RE: [EXT] Re: [PATCH v1] usb: dwc3: remove the call trace of USBx_GFLADJ

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3853 bytes --]


Hi,

Ran Wang <ran.wang_1@nxp.com> writes:
>> Yinbo Zhu <yinbo.zhu@nxp.com> writes:
>> >> Yinbo Zhu <yinbo.zhu@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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

RE: [EXT] Re: [PATCH v1] usb: dwc3: remove the call trace of USBx_GFLADJ

[Yinbo Zhu]

Hi Balbi,

If no other doubts, please help apply it.

Thanks,
Regards,
Yinbo Zhu.

-----Original Message-----
From: Felipe Balbi <balbi@kernel.org> 
Sent: 2019年8月27日 19:55
To: Ran Wang <ran.wang_1@nxp.com>; Yinbo Zhu <yinbo.zhu@nxp.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; linux-usb@vger.kernel.org; open list <linux-kernel@vger.kernel.org>
Cc: Xiaobo Xie <xiaobo.xie@nxp.com>; Jiafei Pan <jiafei.pan@nxp.com>
Subject: RE: [EXT] Re: [PATCH v1] usb: dwc3: remove the call trace of USBx_GFLADJ


Hi,

Ran Wang <ran.wang_1@nxp.com> writes:
>> Yinbo Zhu <yinbo.zhu@nxp.com> writes:
>> >> Yinbo Zhu <yinbo.zhu@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