Hi, Roger Quadros <rogerq@xxxxxx> writes: >>>>>>>>> From: Pawel Laszczak <pawell@xxxxxxxxxxx> >>>>>>>>> >>>>>>>>> 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? >> > 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
Attachment:
signature.asc
Description: PGP signature
