From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 73087CA9EAE for ; Tue, 29 Oct 2019 21:21:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3CC5A2087F for ; Tue, 29 Oct 2019 21:21:50 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="ojI912WJ" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729155AbfJ2VVq (ORCPT ); Tue, 29 Oct 2019 17:21:46 -0400 Received: from mail-oi1-f195.google.com ([209.85.167.195]:42233 "EHLO mail-oi1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728530AbfJ2VVp (ORCPT ); Tue, 29 Oct 2019 17:21:45 -0400 Received: by mail-oi1-f195.google.com with SMTP id i185so117434oif.9 for ; Tue, 29 Oct 2019 14:21:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=a8xArhRDoNXUbWX4EdmjweiHTW4/JnIyhzo/xyW6XxM=; b=ojI912WJLMKSe3iQP4WzhQapOtESvGrxE3lsv+poUzeRJOvYMlT5VDHYgM0JzH0W+P GwzA2iaxTkr79Yd7ObLdokzSiwQXcbC/T1LuORrYy328g18YaLRVvCn1qXXpijqq92ic y7EvOOdGC232K3ShfM7UVEDVke2LadybNi/xrh83ohR8YkA8eoS19aIBf+woFwI/EvbQ uawUf7MN1Dk0I8RJ/p5G8dDBLsKvnWVo1Yq3Glnb0hp2iL8M192isnr7unr+xY8qMPqO f1/ylqy+PdUlVsVFUJvGsAtP8hnvDi2U50zbUseLrGC+gle/3Qf42CzO81+L00jOcQ/6 B9mQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=a8xArhRDoNXUbWX4EdmjweiHTW4/JnIyhzo/xyW6XxM=; b=VVQQKsM3enxASUNwe5Y3uLTBnYEWebJV0hYnF6hTBAVR/ltdk7gR1/ag/IAA/wO92P ZA+bMFIQqpIU0zjBvCH+LamkqHl6nr1TuFNWYLfn1y8pWhEPXTB6QvUEzlrPWvBayx9u 9cgdngEinHrXxc4+HBGiWyVcjhsBWAYOEiCQr/CkJutQEjzKfFXhQz22wIsTUggRzgJ3 57O47N7QyNaFIK+ZMq9j5ytDfXRxmRoePpDwc9TTw3pgRTAAVV0WFS2SNISvy1BlxswS w/rtboF+a6IoyWFh3LHKJp12q/h2eEChhHZkZ+Ti7zlk9omVrSa3dvVYlLyXgSkgt3YN 92zg== X-Gm-Message-State: APjAAAVhGBFdMHdoGZbNFIst3Fi4drL2gOWr5lnozb/71fopciT++p3t wueWzt+ZO+/yfpgI9g9alekhjefFGVNVKKhTs8vxtQ== X-Google-Smtp-Source: APXvYqz9iikOCmyCYThBR+xrlLZBkbgNK8dGvSf09Nn4AtfDYJp882m2GUQge7vjRFhlgRNd3utC76a1kjoznAPknhc= X-Received: by 2002:aca:58c2:: with SMTP id m185mr6043338oib.128.1572384104335; Tue, 29 Oct 2019 14:21:44 -0700 (PDT) MIME-Version: 1.0 References: <20191028215919.83697-1-john.stultz@linaro.org> <20191028215919.83697-3-john.stultz@linaro.org> <87pnifj4tg.fsf@gmail.com> In-Reply-To: <87pnifj4tg.fsf@gmail.com> From: John Stultz Date: Tue, 29 Oct 2019 14:21:31 -0700 Message-ID: Subject: Re: [PATCH v4 2/9] usb: dwc3: Execute GCTL Core Soft Reset while switch modes To: Felipe Balbi Cc: lkml , Yu Chen , Greg Kroah-Hartman , Rob Herring , Mark Rutland , ShuFan Lee , Heikki Krogerus , Suzuki K Poulose , Chunfeng Yun , Hans de Goede , Andy Shevchenko , Jun Li , Valentin Schneider , Jack Pham , Linux USB List , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" Content-Type: text/plain; charset="UTF-8" Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org On Tue, Oct 29, 2019 at 2:09 AM Felipe Balbi wrote: > John Stultz writes: > > From: Yu Chen > > > > On the HiKey960, we need to do a GCTL soft reset when > > switching modes. > > > > Jack Pham also noted that in the Synopsys databook it > > mentions performing a GCTL CoreSoftReset when changing the > > PrtCapDir between device & host modes. > > > > So this patch always does a GCTL Core Soft Reset when > > changing the mode. > > > > Cc: Greg Kroah-Hartman > > Cc: Rob Herring > > Cc: Mark Rutland > > CC: ShuFan Lee > > Cc: Heikki Krogerus > > Cc: Suzuki K Poulose > > Cc: Chunfeng Yun > > Cc: Yu Chen > > Cc: Felipe Balbi > > Cc: Hans de Goede > > Cc: Andy Shevchenko > > Cc: Jun Li > > Cc: Valentin Schneider > > Cc: Jack Pham > > Cc: linux-usb@vger.kernel.org > > Cc: devicetree@vger.kernel.org > > Signed-off-by: Yu Chen > > Signed-off-by: John Stultz > > --- > > v3: Remove quirk conditional, as Jack Pham noted the > > Synopsis databook states this should be done generally. > > Also, at Jacks' suggestion, make the reset call before > > changing the prtcap direction. > > --- > > drivers/usb/dwc3/core.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > index 999ce5e84d3c..a039e35ec7ad 100644 > > --- a/drivers/usb/dwc3/core.c > > +++ b/drivers/usb/dwc3/core.c > > @@ -112,6 +112,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) > > +{ > > + u32 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); > > @@ -154,6 +167,9 @@ static void __dwc3_set_mode(struct work_struct *work) > > > > spin_lock_irqsave(&dwc->lock, flags); > > > > + /* Execute a GCTL Core Soft Reset when switch mode */ > > + dwc3_gctl_core_soft_reset(dwc); > > + > > This is totally unnecessary. We have several platforms supporting dual > role *without* this trick. The only reason why the databook mentions a > reset is because some registers are shadowed, meaning that they share > the same physical space and just appear as different things for SW. The > reason being that Synopsys wanted to reduce the area of the IP and > decided to shadow registers which are mutually exclusive. Ok. I've dropped this for now. Without this I do see an occasional issues seemingly more frequently where he board seems to initialize improperly on boot (usb-c is connected, but it doesn't seem to detect until I unplug and replug), but it also trips (though seemingly less frequently) without this, so this may be just affecting the timing of a initialization race issue. I'll watch this for more info and follow up on it later. Thanks for the review! -john