Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Ran Wang <ran.wang_1@nxp.com>
To:     Felipe Balbi <balbi@kernel.org>, Yinbo Zhu <yinbo.zhu@nxp.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "linux-usb@vger.kernel.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
Thread-Topic: [EXT] Re: [PATCH v1] usb: dwc3: remove the call trace of
 USBx_GFLADJ
Thread-Index: AQHVRdjlAL5PjFT4bkqsQBug9WvpnabwwpYAgAYomICAACZOgIAEo6yAgAAcWACAEp/msA==
Date:   Tue, 27 Aug 2019 02:37:45 +0000
Message-ID: <DB8PR04MB682695EEB3E358BDB3E2D5D0F1A00@DB8PR04MB6826.eurprd04.prod.outlook.com>
References: <20190729064607.8131-1-yinbo.zhu@nxp.com>
 <875zn8nt31.fsf@gmail.com>
 <VI1PR04MB41580DBF1A0C2DC143EE9CE3E9D30@VI1PR04MB4158.eurprd04.prod.outlook.com>
 <87h86nvtqo.fsf@gmail.com>
 <VI1PR04MB4158B49129BE9E3C00997555E9AC0@VI1PR04MB4158.eurprd04.prod.outlook.com>
 <87v9uzt1dr.fsf@gmail.com>
In-Reply-To: <87v9uzt1dr.fsf@gmail.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 7d1f106d-fd46-493f-4b97-08d72a978b33
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Aug 2019 02:37:45.8316
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: FRk8nN4KP1p3H/0pWbhWqI894eKm9Eq3jqjpbB4PEOJExV8maEqZoALP2FB7NEYaOc638QQhC73vKRiyjQM9zw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB8PR04MB5691
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Felipe,

On Thursday, August 15, 2019 13:59, Felipe Balbi wrote:
>=20
> Hi,
>=20
> 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 =3D dwc3_readl(dwc->regs, DWC3_GFLADJ);
> >> >> >       dft =3D reg & DWC3_GFLADJ_30MHZ_MASK;
> >> >> > -     if (!dev_WARN_ONCE(dwc->dev, dft =3D=3D dwc->fladj,
> >> >> > -         "request value same as default, ignoring\n")) {
> >> >> > +     if (dft !=3D 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
>=20
> You only get the splat because you try to sent GFLADJ to 0x20 and it's AL=
READY
> 0x20. This means that you don't need the property in DTS.
>=20
> > In addition about what you said is about dts patch, and that patch had
> > merged by upstream, patch owner isn't me,
>=20
> Well, then remove the setting from DTS, since clearly it's not needed.

Please considering below scenarios on the same board which needs GFLADJ pro=
perty on kernel DTS:

1. Board boot to U-Boot first, then load kernel. In this case, we need kern=
el 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 cons=
ole to init
    DWC3 controller, then load kernel. In this case, actually GFLADJ is cor=
rectly
    configured already, and the GFLADJ config double-checking is fine (beca=
use 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 & res=
ume. In this case
    when resuming, GFLADJ setting has been restored correctly, so here we m=
ight not need
    send out the warning message (double-checking might be fine).

So, what's your suggestion to remove this looks non-necessary warning messa=
ge?

Thanks & Regards,
Ran

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