Re: [PATCH] usb: chipidea: imx: improve usbmisc_imx7d_pullup()

["Peter Chen (CIX)" <peter.chen@kernel.org>]

On 25-08-11 18:08:33, Xu Yang wrote:
> When add workaround for ERR051725, the usbmisc will put PHY to
> Non-driving mode (OPMODE = 01) after stopping the device controller
> and put PHY back to Normal mode (OPMODE = 00) after starting the device
> controller.
> 
> However, this will bring issue for host controller. Because the PHY may
> stay in Non-driving mode after switching the role from device to host.
> Then the port will not work if USB device is attached. To fix this issue,
> improving the workaround by putting PHY to Non-driving mode for a certain
> period and back to Normal mode finally. To make host detect a disconnect
> signal, the period should be at least 125us (a micro-frame time) for
> high-speed link.
> 
> And only working as high-speed mode will need workaround for ERR051725.
> So this will also filter the pullup event for high-speed.
> 
> Fixes: 11992b410083 ("usb: chipidea: imx: implement workaround for ERR051725")
> Reviewed-by: Jun Li <jun.li@nxp.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c |  3 ++-
>  drivers/usb/chipidea/usbmisc_imx.c | 23 ++++++++++++++++-------
>  2 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index e1ec9b38f5b9..d7c2a1a3c271 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -338,7 +338,8 @@ static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int event)
>  			schedule_work(&ci->usb_phy->chg_work);
>  		break;
>  	case CI_HDRC_CONTROLLER_PULLUP_EVENT:
> -		if (ci->role == CI_ROLE_GADGET)
> +		if (ci->role == CI_ROLE_GADGET &&
> +		    ci->gadget.speed == USB_SPEED_HIGH)
>  			imx_usbmisc_pullup(data->usbmisc_data,
>  					   ci->gadget.connected);
>  		break;
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index 3d20c5e76c6a..b1418885707c 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -1068,15 +1068,24 @@ static void usbmisc_imx7d_pullup(struct imx_usbmisc_data *data, bool on)
>  	unsigned long flags;
>  	u32 val;
>  
> +	if (on)
> +		return;
> +
>  	spin_lock_irqsave(&usbmisc->lock, flags);
>  	val = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
> -	if (!on) {
> -		val &= ~MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_MASK;
> -		val |= MX7D_USBNC_USB_CTRL2_OPMODE(1);
> -		val |= MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_EN;
> -	} else {
> -		val &= ~MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_EN;
> -	}
> +	val &= ~MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_MASK;
> +	val |= MX7D_USBNC_USB_CTRL2_OPMODE(1);
> +	val |= MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_EN;
> +	writel(val, usbmisc->base + MX7D_USBNC_USB_CTRL2);
> +	spin_unlock_irqrestore(&usbmisc->lock, flags);
> +
> +	/* Last for at least 1 micro-frame to let host see disconnect signal */
> +	usleep_range(125, 150);
> +
> +	spin_lock_irqsave(&usbmisc->lock, flags);
> +	val &= ~MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_MASK;
> +	val |= MX7D_USBNC_USB_CTRL2_OPMODE(0);
> +	val &= ~MX7D_USBNC_USB_CTRL2_OPMODE_OVERRIDE_EN;
>  	writel(val, usbmisc->base + MX7D_USBNC_USB_CTRL2);
>  	spin_unlock_irqrestore(&usbmisc->lock, flags);
>  }
> -- 
> 2.34.1
> 

-- 

Best regards,
Peter