Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Peter Chen <peter.chen@nxp.com>
To:     Roger Quadros <rogerq@ti.com>
CC:     Pawel Laszczak <pawell@cadence.com>,
        "balbi@kernel.org" <balbi@kernel.org>,
        Rahul Kumar <kurahul@cadence.com>,
        "nsekhar@ti.com" <nsekhar@ti.com>,
        "vigneshr@ti.com" <vigneshr@ti.com>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx
 compliance test PHY lockup
Thread-Topic: [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx
 compliance test PHY lockup
Thread-Index: AQHWete/XCUsbMkzVE2tONyDrFDh16lJudEAgAAMW4CAADVhAIAACEuAgAACF/CAAFMvgIAAwdIA
Date:   Thu, 27 Aug 2020 00:24:55 +0000
Message-ID: <20200827002339.GA17559@b29397-desktop>
References: <20200825120059.12436-1-rogerq@ti.com>
 <20200825120059.12436-4-rogerq@ti.com> <20200826031948.GA7646@b29397-desktop>
 <DM6PR07MB5529A43AFDEB25993595DB59DD540@DM6PR07MB5529.namprd07.prod.outlook.com>
 <20200826071504.GA19661@b29397-desktop>
 <DM6PR07MB5529EB2FB7E3380321191B44DD540@DM6PR07MB5529.namprd07.prod.outlook.com>
 <AM7PR04MB71576DF6C03387C7628DBE3A8B540@AM7PR04MB7157.eurprd04.prod.outlook.com>
 <ab38721a-ef48-c6a7-aa33-3085ca7b8852@ti.com>
In-Reply-To: <ab38721a-ef48-c6a7-aa33-3085ca7b8852@ti.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <C19BF47BDD710F45AB0258E5658DA04F@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: d1360a51-3280-4529-ce2b-08d84a1f9f9f
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Aug 2020 00:24:55.4451
 (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: MkZvKLcQWvuv73vn2KiiuzvQhmwh4bftG7od8ysJOuGPwRPWL2SIMXPuQbgz3zSmIpR1rVNKhJ2lkPyFxd83pQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM7PR04MB7077
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 20-08-26 15:49:57, Roger Quadros wrote:
> Peter,
>=20
> On 26/08/2020 11:07, Peter Chen wrote:
> > >=20
> > >=20
> > > >=20
> > > > On 20-08-26 04:04:01, Pawel Laszczak wrote:
> > > > > > On 20-08-25 15:00:59, Roger Quadros wrote:
> > > > > > > From: Pawel Laszczak <pawell@cadence.com>
> > > > > > >=20
> > > > > > > USB2.0 PHY hangs in Rx Compliance test when the incoming pack=
et
> > > > > > > amplitude is varied below and above the Squelch Level of Rece=
iver
> > > > > > > during the active packet multiple times.
> > > > > > >=20
> > > > > > > Version 1 of the controller allows PHY to be reset when RX fa=
il
> > > > > > > condition is detected to work around the above issue. This fe=
ature
> > > > > > > is disabled by default and needs to be enabled using a bit fr=
om
> > > > > > > the newly added PHYRST_CFG register. This patch enables the w=
orkaround.
> > > > > > >=20
> > > > > > > As there is no way to distinguish between the controller vers=
ion
> > > > > > > before the device controller is started we need to rely on a =
DT
> > > > > > > property to decide when to apply the workaround.
> > > > > >=20
> > > > > > Pawel, it could know the controller version at cdns3_gadget_sta=
rt,
> > > > > > but the controller starts when it tries to bind gadget driver, =
at
> > > > > > that time, it has already known the controller version.
> > > > > >=20
> > > > > > For me, the device controller starts is using USB_CONF.DEVEN (D=
evice
> > > > > > Enable) through usb_gadget_connect, I am not sure if it is the =
same
> > > > > > with yours.
> > > > > >=20
> > > > >=20
> > > > > Yes in device mode driver knows controller version but this
> > > > > workaround Must be enabled also in host mode. In host mode the
> > > > > controller doesn't have access to device registers. The controlle=
r
> > > > > version is placed in device register.
> > > > >=20
> > > >=20
> > > > You may suggest your design team adding CHIP_VER register at global
> > > > register region, it will easy the software engineer life.
> > > >=20
> > > >From what I read, this register is only enabling USB2 PHY reset
> > > > software control, it needs for all chips with rev 0x0002450D, and t=
he
> > > > place you current change is only for 0x0002450D, right?
> > >=20
> > > Even I could say that this workaround should be enabled only for Spec=
ific USB2
> > > PHY  (only 0x0002450D)
> > >=20
> > > This bit should not have any impact for Cadence PHY but it can has Im=
pact for third
> > > party PHYs.
> > >=20
> >=20
> > So, it is related to specific PHY, but enable this specific PHY reset b=
it is at controller region, why don't
> > put this enable bit at PHY region?
>=20
> I think this is related to Controller + PHY combination.
> The fix for the issue is via a bit in the controller, so it needs to be m=
anaged by the
> controller driver.
>=20
> >=20
> > So, you use controller's device property to know this specific PHY, can=
 controller know this specific
> > PHY dynamically?
>=20
> Still the PHY will have to tell the controller the enable that bit. How t=
o do that?
>=20
> Adding a dt-property that vendors can used was the simplest option.
>=20

Ok, does all controllers with ver 0x0002450D need this fix? I just think
if we introduce a flag stands for ver 0x0002450D in case this ver has
other issues in future or just using phy reset enable property?

Pawel & Roger, what's your opinion?

--=20

Thanks,
Peter Chen=