Re: [PATCH 3/3] usb: cdns3: Enable workaround for USB2.0 PHY Rx compliance test PHY lockup

[Peter Chen <peter.chen@nxp.com>]

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>
> >>
> >> 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?

Peter

> Pawel
> 
> >Peter
> >
> >
> >>
> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> >> Signed-off-by: Roger Quadros <rogerq@ti.com>
> >> ---
> >>  drivers/usb/cdns3/core.c |  2 ++
> >>  drivers/usb/cdns3/core.h |  1 +
> >>  drivers/usb/cdns3/drd.c  | 12 ++++++++++++
> >>  drivers/usb/cdns3/drd.h  |  5 ++++-
> >>  4 files changed, 19 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> >> index 5c1586ec7824..34b36487682b 100644
> >> --- a/drivers/usb/cdns3/core.c
> >> +++ b/drivers/usb/cdns3/core.c
> >> @@ -443,6 +443,8 @@ static int cdns3_probe(struct platform_device *pdev)
> >>  		return -ENXIO;
> >>  	}
> >>
> >> +	cdns->phyrst_a_enable = device_property_read_bool(dev, "cdns,phyrst-a-enable");
> >> +
> >>  	cdns->otg_res = *res;
> >>
> >>  	mutex_init(&cdns->mutex);
> >> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
> >> index 1ad1f1fe61e9..24cf0f1b5726 100644
> >> --- a/drivers/usb/cdns3/core.h
> >> +++ b/drivers/usb/cdns3/core.h
> >> @@ -76,6 +76,7 @@ struct cdns3 {
> >>  #define CDNS3_CONTROLLER_V0	0
> >>  #define CDNS3_CONTROLLER_V1	1
> >>  	u32				version;
> >> +	bool				phyrst_a_enable;
> >>
> >>  	int				otg_irq;
> >>  	int				dev_irq;
> >> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
> >> index 6234bcd6158a..b74803e9703d 100644
> >> --- a/drivers/usb/cdns3/drd.c
> >> +++ b/drivers/usb/cdns3/drd.c
> >> @@ -42,6 +42,18 @@ int cdns3_set_mode(struct cdns3 *cdns, enum usb_dr_mode mode)
> >>  			reg = readl(&cdns->otg_v1_regs->override);
> >>  			reg |= OVERRIDE_IDPULLUP;
> >>  			writel(reg, &cdns->otg_v1_regs->override);
> >> +
> >> +			/*
> >> +			 * Enable work around feature built into the
> >> +			 * controller to address issue with RX Sensitivity
> >> +			 * est (EL_17) for USB2 PHY. The issue only occures
> >> +			 * for 0x0002450D controller version.
> >> +			 */
> >> +			if (cdns->phyrst_a_enable) {
> >> +				reg = readl(&cdns->otg_v1_regs->phyrst_cfg);
> >> +				reg |= PHYRST_CFG_PHYRST_A_ENABLE;
> >> +				writel(reg, &cdns->otg_v1_regs->phyrst_cfg);
> >> +			}
> >>  		} else {
> >>  			reg = readl(&cdns->otg_v0_regs->ctrl1);
> >>  			reg |= OVERRIDE_IDPULLUP_V0;
> >> diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
> >> index 7e7cf7fa2dd3..f1ccae285a16 100644
> >> --- a/drivers/usb/cdns3/drd.h
> >> +++ b/drivers/usb/cdns3/drd.h
> >> @@ -31,7 +31,7 @@ struct cdns3_otg_regs {
> >>  	__le32 simulate;
> >>  	__le32 override;
> >>  	__le32 susp_ctrl;
> >> -	__le32 reserved4;
> >> +	__le32 phyrst_cfg;
> >>  	__le32 anasts;
> >>  	__le32 adp_ramp_time;
> >>  	__le32 ctrl1;
> >> @@ -153,6 +153,9 @@ struct cdns3_otg_common_regs {
> >>  /* Only for CDNS3_CONTROLLER_V0 version */
> >>  #define OVERRIDE_IDPULLUP_V0		BIT(24)
> >>
> >> +/* PHYRST_CFG - bitmasks */
> >> +#define PHYRST_CFG_PHYRST_A_ENABLE     BIT(0)
> >> +
> >>  #define CDNS3_ID_PERIPHERAL		1
> >>  #define CDNS3_ID_HOST			0
> >>
> >> --
> >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> >> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> >>
> >
> >--
> >
> >Thanks,
> >Peter Chen

-- 

Thanks,
Peter Chen