linux-kernel - Re: [RFC][PATCH] HACK: usb: dwc2: Workaround case where GOTGCTL state is wrong

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <30669194-dc9c-503b-b84d-a7cfd23bbfa6@synopsys.com>
Date:   Tue, 6 Dec 2016 19:52:32 -0800
From:   John Youn <John.Youn@...opsys.com>
To:     John Stultz <john.stultz@...aro.org>,
        lkml <linux-kernel@...r.kernel.org>
CC:     Wei Xu <xuwei5@...ilicon.com>, Guodong Xu <guodong.xu@...aro.org>,
        "Amit Pundir" <amit.pundir@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        John Youn <John.Youn@...opsys.com>,
        Douglas Anderson <dianders@...omium.org>,
        Chen Yu <chenyu56@...wei.com>,
        Kishon Vijay Abraham I <kishon@...com>,
        Felipe Balbi <felipe.balbi@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Subject: Re: [RFC][PATCH] HACK: usb: dwc2: Workaround case where GOTGCTL state
 is wrong

On 12/6/2016 5:48 PM, John Stultz wrote:
> Hey John,
>   Just wanted to send this by you, as it seems something is
> slightly off with the GOTGCTL state when removing a otg adapter
> cable. The following seems to work around the issue I'm seeing.
> 
> Let me know if you have any thoughts on this.
> thanks
> -john
> 
> 
> When removing a USB-A to USB-otg adapter cable, we get a change
> status irq, and then in dwc2_conn_id_status_change, we
> erroniously see the GOTGCTL_CONID_B flag set. This causes us to

This is the correct behavior for an OTG controller. When you unplug a
cable or plug in the B end of a cable, the ID pin floats, indicating
it is a B-Device.

When you plug in an A-cable, which is what your adapter is, it will
ground the pin, meaning A-device.

> get  stuck in the "while (!dwc2_is_device_mode(hsotg))" loop,
> spitting out "Waiting for Peripheral Mode, Mode=Host" warnings
> until it fails out many seconds later.

This is weird. Once the ID pin goes to B, the core should become a
peripheral and this should be reflected in the status registers.

> 
> This patch works around the issue by re-reading the GOTGCTL
> state to check if the GOTGCTL_CONID_B is still set and if not
> restarting the change status logic.

This also seems weird. The connector id status shouldn't go back to A,
assuming you've left the cable unplugged.

Is the controller supposed to work in both peripheral and host modes?

> 
> I suspect this isn't the best solution, but it seems to work
> well for me.
> 

The workaround seems fine, but still, this indicates that something
wrong is going on somwhere.

You can add my ack:

Acked-by: John Youn <johnyoun@...opsys.com>

Regards,
John


> Feedback would be greatly appreciated!
> 
> Cc: Wei Xu <xuwei5@...ilicon.com>
> Cc: Guodong Xu <guodong.xu@...aro.org>
> Cc: Amit Pundir <amit.pundir@...aro.org>
> Cc: Rob Herring <robh+dt@...nel.org>
> Cc: John Youn <johnyoun@...opsys.com>
> Cc: Douglas Anderson <dianders@...omium.org>
> Cc: Chen Yu <chenyu56@...wei.com>
> Cc: Kishon Vijay Abraham I <kishon@...com>
> Cc: Felipe Balbi <felipe.balbi@...ux.intel.com>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: linux-usb@...r.kernel.org
> Signed-off-by: John Stultz <john.stultz@...aro.org>
> ---
>  drivers/usb/dwc2/hcd.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 143da47..6d6802a 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -3203,7 +3203,7 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
>  	dev_dbg(hsotg->dev, "gotgctl=%0x\n", gotgctl);
>  	dev_dbg(hsotg->dev, "gotgctl.b.conidsts=%d\n",
>  		!!(gotgctl & GOTGCTL_CONID_B));
> -
> +again:
>  	/* B-Device connector (Device Mode) */
>  	if (gotgctl & GOTGCTL_CONID_B) {
>  		/* Wait for switch to device mode */
> @@ -3219,6 +3219,9 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
>  				 dwc2_is_host_mode(hsotg) ? "Host" :
>  				 "Peripheral");
>  			usleep_range(20000, 40000);
> +			gotgctl = dwc2_readl(hsotg->regs + GOTGCTL);
> +			if (!(gotgctl & GOTGCTL_CONID_B))
> +				goto again;
>  			if (++count > 250)
>  				break;
>  		}
>