| 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
| ||
|
Date: Wed, 21 Oct 2020 19:14:48 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: John Stultz <john.stultz@...aro.org>,
lkml <linux-kernel@...r.kernel.org>
CC: Yu Chen <chenyu56@...wei.com>, Felipe Balbi <balbi@...nel.org>,
Tejas Joglekar <Tejas.Joglekar@...opsys.com>,
Yang Fei <fei.yang@...el.com>,
YongQin Liu <yongqin.liu@...aro.org>,
Andrzej Pietrasiewicz <andrzej.p@...labora.com>,
Jun Li <lijun.kernel@...il.com>,
Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [RFC][PATCH] usb: dwc3: Add quirk to trigger a GCTL soft reset
for Hisilicon Kirin Soc Platform
John Stultz wrote:
> From: Yu Chen <chenyu56@...wei.com>
>
> With the current dwc3 code on the HiKey960 we often see the
> COREIDLE flag get stuck off in __dwc3_gadget_start(), which
> seems to prevent the reset irq and causes the USB gadget to
> fail to initialize.
>
> We had seen occasional initialization failures with older
> kernels but with recent 5.x era kernels it seemed to be becoming
> much more common, so I dug back through some older trees and
> realized I dropped this quirk from Yu Chen during upstreaming
> as I couldn't provide a proper rational for it and it didn't
> seem to be necessary. I now realize I was wrong.
>
> On the upside, I can now understand more why such a quirk is
> needed.
This shouldn't be a quirk. It's part of the programming guide when
switching mode in DRD. I don't know how we missed this.
>
> So to address a quirk in the DesignWare USB3 DRD Core of
> Hisilicon Kirin SoCs, this patch adds a quirk flag which
> executes a GCTL soft reset when we switch modes.
>
> Cc: Felipe Balbi <balbi@...nel.org>
> Cc: Tejas Joglekar <tejas.joglekar@...opsys.com>
> Cc: Yang Fei <fei.yang@...el.com>
> Cc: YongQin Liu <yongqin.liu@...aro.org>
> Cc: Andrzej Pietrasiewicz <andrzej.p@...labora.com>
> Cc: Thinh Nguyen <thinhn@...opsys.com>
> Cc: Jun Li <lijun.kernel@...il.com>
> Cc: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> Cc: Rob Herring <robh+dt@...nel.org>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: linux-usb@...r.kernel.org
> Cc: devicetree@...r.kernel.org
> Signed-off-by: Yu Chen <chenyu56@...wei.com>
> Signed-off-by: John Stultz <john.stultz@...aro.org>
> ---
> .../devicetree/bindings/usb/dwc3.txt | 3 +++
> drivers/usb/dwc3/core.c | 19 +++++++++++++++++++
> drivers/usb/dwc3/core.h | 1 +
> 3 files changed, 23 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
> index 1aae2b6160c1..f435bae0f172 100644
> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> @@ -81,6 +81,9 @@ Optional properties:
> - snps,dis-split-quirk: when set, change the way URBs are handled by the
> driver. Needed to avoid -EPROTO errors with usbhid
> on some devices (Hikey 970).
> + - snps,gctl-reset-quirk: When set, execute a soft reset on mode changes.
> + Needed to avoid COREIDLE getting stuck off and the
> + gadget to fail to initialize (HiKey960).
> - snps,is-utmi-l1-suspend: true when DWC3 asserts output signal
> utmi_l1_suspend_n, false when asserts utmi_sleep_n
> - snps,hird-threshold: HIRD threshold
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index bdf0925da6b6..b138c67e3892 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -114,6 +114,19 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
> dwc->current_dr_role = mode;
> }
>
> +static void dwc3_gctl_core_soft_reset(struct dwc3 *dwc)
> +{
> + int reg;
> +
> + reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> + reg |= (DWC3_GCTL_CORESOFTRESET);
> + dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> +
> + reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> + reg &= ~(DWC3_GCTL_CORESOFTRESET);
> + dwc3_writel(dwc->regs, DWC3_GCTL, reg);
> +}
> +
> static void __dwc3_set_mode(struct work_struct *work)
> {
> struct dwc3 *dwc = work_to_dwc(work);
> @@ -178,6 +191,10 @@ static void __dwc3_set_mode(struct work_struct *work)
> }
> break;
> case DWC3_GCTL_PRTCAP_DEVICE:
> + /* Execute a GCTL Core Soft Reset when switch mode */
> + if (dwc->gctl_reset_quirk)
> + dwc3_gctl_core_soft_reset(dwc);
> +
This should be done before dwc3_set_prtcap(), and this applies when
switching from device to host mode also. Make sure to check if the
controller is DRD before doing this.
> dwc3_event_buffers_setup(dwc);
>
> if (dwc->usb2_phy)
> @@ -1357,6 +1374,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
>
> dwc->dis_split_quirk = device_property_read_bool(dev,
> "snps,dis-split-quirk");
> + dwc->gctl_reset_quirk = device_property_read_bool(dev,
> + "snps,gctl-reset-quirk");
>
> dwc->lpm_nyet_threshold = lpm_nyet_threshold;
> dwc->tx_de_emphasis = tx_de_emphasis;
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 74323b10a64a..993f243aedc8 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1252,6 +1252,7 @@ struct dwc3 {
> unsigned dis_metastability_quirk:1;
>
> unsigned dis_split_quirk:1;
> + unsigned gctl_reset_quirk:1;
>
> u16 imod_interval;
> };
BR,
Thinh
Powered by blists - more mailing lists