2019-07-29 06:46:28

by Yinbo Zhu

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

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 <[email protected]>
---
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


2019-08-08 05:15:40

by Felipe Balbi

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


Hi,

Yinbo Zhu <[email protected]> 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 <[email protected]>
> ---
> 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

2019-08-12 03:12:14

by Yinbo Zhu

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



> -----Original Message-----
> From: Felipe Balbi [mailto:[email protected]]
> Sent: 2019??8??8?? 13:07
> To: Yinbo Zhu <[email protected]>; Greg Kroah-Hartman
> <[email protected]>; [email protected]; open list
> <[email protected]>
> Cc: Yinbo Zhu <[email protected]>; Xiaobo Xie <[email protected]>; Jiafei
> Pan <[email protected]>; Ran Wang <[email protected]>
> Subject: [EXT] Re: [PATCH v1] usb: dwc3: remove the call trace of USBx_GFLADJ
>
> Caution: EXT Email
>
> Hi,
>
> Yinbo Zhu <[email protected]> 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 <[email protected]>
> > ---
> > 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


2019-08-12 05:29:30

by Felipe Balbi

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


Hi,

Yinbo Zhu <[email protected]> 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

2019-08-15 04:18:56

by Yinbo Zhu

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



> -----Original Message-----
> From: Felipe Balbi [mailto:[email protected]]
> Sent: 2019??8??12?? 13:27
> To: Yinbo Zhu <[email protected]>; Greg Kroah-Hartman
> <[email protected]>; [email protected]; open list
> <[email protected]>
> Cc: Xiaobo Xie <[email protected]>; Jiafei Pan <[email protected]>; Ran
> Wang <[email protected]>
> Subject: RE: [EXT] Re: [PATCH v1] usb: dwc3: remove the call trace of
> USBx_GFLADJ
>
> Caution: EXT Email
>
> Hi,
>
> Yinbo Zhu <[email protected]> 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.



2019-08-15 06:09:06

by Felipe Balbi

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


Hi,

Yinbo Zhu <[email protected]> writes:
>> Yinbo Zhu <[email protected]> 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

2019-08-27 02:41:10

by Ran Wang

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

Hi Felipe,

On Thursday, August 15, 2019 13:59, Felipe Balbi wrote:
>
> Hi,
>
> Yinbo Zhu <[email protected]> writes:
> >> Yinbo Zhu <[email protected]> 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

2019-08-27 11:57:49

by Felipe Balbi

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


Hi,

Ran Wang <[email protected]> writes:
>> Yinbo Zhu <[email protected]> writes:
>> >> Yinbo Zhu <[email protected]> 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


Attachments:
signature.asc (847.00 B)

2019-09-05 02:22:13

by Yinbo Zhu

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

Hi Balbi,

If no other doubts, please help apply it.

Thanks,
Regards,
Yinbo Zhu.

-----Original Message-----
From: Felipe Balbi <[email protected]>
Sent: 2019??8??27?? 19:55
To: Ran Wang <[email protected]>; Yinbo Zhu <[email protected]>; Greg Kroah-Hartman <[email protected]>; [email protected]; open list <[email protected]>
Cc: Xiaobo Xie <[email protected]>; Jiafei Pan <[email protected]>
Subject: RE: [EXT] Re: [PATCH v1] usb: dwc3: remove the call trace of USBx_GFLADJ


Hi,

Ran Wang <[email protected]> writes:
>> Yinbo Zhu <[email protected]> writes:
>> >> Yinbo Zhu <[email protected]> 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