lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 27 Aug 2020 00:24:55 +0000 From: Peter Chen <peter.chen@....com> To: Roger Quadros <rogerq@...com> CC: Pawel Laszczak <pawell@...ence.com>, "balbi@...nel.org" <balbi@...nel.org>, Rahul Kumar <kurahul@...ence.com>, "nsekhar@...com" <nsekhar@...com>, "vigneshr@...com" <vigneshr@...com>, "robh+dt@...nel.org" <robh+dt@...nel.org>, "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "devicetree@...r.kernel.org" <devicetree@...r.kernel.org> Subject: Re: [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup On 20-08-26 15:49:57, Roger Quadros wrote: > Peter, > > On 26/08/2020 11:07, Peter Chen wrote: > > > > > > > > > > > > > > On 20-08-26 04:04:01, Pawel Laszczak wrote: > > > > > > On 20-08-25 15:00:59, Roger Quadros wrote: > > > > > > > From: Pawel Laszczak <pawell@...ence.com> > > > > > > > > > > > > > > USB2.0 PHY hangs in Rx Compliance test when the incoming packet > > > > > > > amplitude is varied below and above the Squelch Level of Receiver > > > > > > > during the active packet multiple times. > > > > > > > > > > > > > > Version 1 of the controller allows PHY to be reset when RX fail > > > > > > > condition is detected to work around the above issue. This feature > > > > > > > is disabled by default and needs to be enabled using a bit from > > > > > > > the newly added PHYRST_CFG register. This patch enables the workaround. > > > > > > > > > > > > > > As there is no way to distinguish between the controller version > > > > > > > before the device controller is started we need to rely on a DT > > > > > > > property to decide when to apply the workaround. > > > > > > > > > > > > Pawel, it could know the controller version at cdns3_gadget_start, > > > > > > but the controller starts when it tries to bind gadget driver, at > > > > > > that time, it has already known the controller version. > > > > > > > > > > > > For me, the device controller starts is using USB_CONF.DEVEN (Device > > > > > > Enable) through usb_gadget_connect, I am not sure if it is the same > > > > > > with yours. > > > > > > > > > > > > > > > > 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 controller > > > > > version is placed in device register. > > > > > > > > > > > > > You may suggest your design team adding CHIP_VER register at global > > > > register region, it will easy the software engineer life. > > > > > > > >From what I read, this register is only enabling USB2 PHY reset > > > > software control, it needs for all chips with rev 0x0002450D, and the > > > > place you current change is only for 0x0002450D, right? > > > > > > Even I could say that this workaround should be enabled only for Specific USB2 > > > PHY (only 0x0002450D) > > > > > > This bit should not have any impact for Cadence PHY but it can has Impact for third > > > party PHYs. > > > > > > > So, it is related to specific PHY, but enable this specific PHY reset bit is at controller region, why don't > > put this enable bit at PHY region? > > 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 managed by the > controller driver. > > > > > So, you use controller's device property to know this specific PHY, can controller know this specific > > PHY dynamically? > > Still the PHY will have to tell the controller the enable that bit. How to do that? > > Adding a dt-property that vendors can used was the simplest option. > 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? -- Thanks, Peter Chen
Powered by blists - more mailing lists