Hi,

Roger Quadros [off-list ref] writes:

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

From: Pawel Laszczak <pawell@cadence.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.

quoted

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?

I think it is best to keep the flags specific to the issue rather than
a one flag for all issues with a specific version. This way you can
re-use the flag irrespective of IP version.

I second that. Specially when some SoC-manufacturers may implement ECO
fixes and not change IP revision.

But best case is that Cadence put a IP revision register in common area as you
have previously suggested so driver can automatically apply quirks to specific
versions.

little too late for that :-)

-- 
balbi