Message ID | 20230512170107.18821-1-quic_kriskura@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp5264300vqo; Fri, 12 May 2023 10:09:37 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7Rs+HouW8BhN/K69n0kK4BIAR1QcmKbJhWbZ6QTaOqh8Uv7R1dwAOVsiUhH03y3upZtCVY X-Received: by 2002:a17:902:da90:b0:1aa:ed81:5d7f with SMTP id j16-20020a170902da9000b001aaed815d7fmr33486206plx.8.1683911377083; Fri, 12 May 2023 10:09:37 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1683911377; cv=none; d=google.com; s=arc-20160816; b=EpW599s3N34iDdnkIYMz/h5GtejFaSL9/Mzkqu7KNmOdoWsl6630112O9N0Sc7vMCo xGji4Q4ErVzDD+KHAnl3y9BrlwGQc2lPDAhNvo/bEHmT7+LOxVBCEG6bxLbz2Hjbx+wu zfRP1/Zn9gQ3bzFxMc+EM9cEsKs+l44V/ts/r9InMSAkq5cpKuYmuWg/GcI6mGE/jfBa SxSJwfmkb8phTofIjVP8t5DF3MZMPB7rBg1T18v2/11M2RGX6ApN1ybZ3rPB6K2NSg+e ugM/xOho62TQvGFtZWLRfNGGdwRM+gK5/X1toSGmP/ta4pFNrAfkBpVMmo7G2v5C+t7U Jdcw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=l+x8I3jMdw0dPpamdqg83+y7xFpkmuF7ZN4JBnW8eHY=; b=WO9xLNhlbzKVhEn5RtuAV22FXXjDnwL202SjlkZ0o2uxUkjwd+d0L+WHRVCD06e2q1 wCJU4BTONSj0RZM76xgYwG7AmJHLRFcQGKTSb/69CUFP6L5alB3tsQJQ32LB8r0Aj2da ISWjAjnOWkpNfrQPmdr9X5XMTbpOA+vDzHqXwBroWBJm99jnh1zoqQTFIBEo8dhdwAls paQg0b1GN6mgEW5JPY8JwqEeiEL8pzia7r+UM531JsEBkudN0i3XqvV6sW2m/bAdawUD yRoZFYzpXyyq4m92i+m/0sk3YnKIgFrG6Tz1KNtBRBeZSqO3DMM5GHQtYNX85Y3rQS55 ltWg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=HXqkf8IB; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id kb14-20020a170903338e00b001aafda896b3si8802129plb.626.2023.05.12.10.09.24; Fri, 12 May 2023 10:09:37 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=HXqkf8IB; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237714AbjELRB3 (ORCPT <rfc822;peekingduck44@gmail.com> + 99 others); Fri, 12 May 2023 13:01:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51290 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237550AbjELRB1 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 12 May 2023 13:01:27 -0400 Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8BB95A250; Fri, 12 May 2023 10:01:25 -0700 (PDT) Received: from pps.filterd (m0279868.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 34CDxbt7008999; Fri, 12 May 2023 17:01:22 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : to : cc : subject : date : message-id : mime-version : content-transfer-encoding : content-type; s=qcppdkim1; bh=l+x8I3jMdw0dPpamdqg83+y7xFpkmuF7ZN4JBnW8eHY=; b=HXqkf8IBF3DeGkIrK5YVmcTNyPwLow5BDqfAOgMBtTb9qH5A2wBotY57sG6n0mXnO1e4 +kWx+Kemw/AmO9HmaocjcZJ0T9Cl4fCuvGQV8qebv70TvWcdp/WVW9+oOUeTHf9Jqlaj t2Mh+RCnFQq/USL8k29Mi4EACnuUsYqpTlXx5kPr/9L5vGOUvs48Rt3OpCYU2wEjqYLl YZb4EJpippcisqlRTc1HjPrLaKehUgQo/cMZRIZjeyF9CqZFCf0giKvVAHLUKYHb7yXM +xRMZnv90jxdBtacFymhLZCvqBbYqJRcKBFpqnBUMclqUANAqffw2zqOD9QJWYhss5zW bw== Received: from nalasppmta02.qualcomm.com (Global_NAT1.qualcomm.com [129.46.96.20]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3qhpsdre58-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 12 May 2023 17:01:22 +0000 Received: from nalasex01a.na.qualcomm.com (nalasex01a.na.qualcomm.com [10.47.209.196]) by NALASPPMTA02.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 34CH1Ljk001778 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 12 May 2023 17:01:21 GMT Received: from hu-kriskura-hyd.qualcomm.com (10.80.80.8) by nalasex01a.na.qualcomm.com (10.47.209.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.42; Fri, 12 May 2023 10:01:18 -0700 From: Krishna Kurapati <quic_kriskura@quicinc.com> To: Thinh Nguyen <Thinh.Nguyen@synopsys.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org> CC: <linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <quic_ppratap@quicinc.com>, <quic_wcheng@quicinc.com>, <quic_jackp@quicinc.com>, Krishna Kurapati <quic_kriskura@quicinc.com> Subject: [RFC] usb: dwc3: core: set force_gen1 bit in USB31 devices if max speed is SS Date: Fri, 12 May 2023 22:31:07 +0530 Message-ID: <20230512170107.18821-1-quic_kriskura@quicinc.com> X-Mailer: git-send-email 2.40.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nalasex01a.na.qualcomm.com (10.47.209.196) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: bAuNgqmSHth-X4Ms25_G0nfWt_NIUVk6 X-Proofpoint-ORIG-GUID: bAuNgqmSHth-X4Ms25_G0nfWt_NIUVk6 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22 definitions=2023-05-12_10,2023-05-05_01,2023-02-09_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 spamscore=0 mlxscore=0 impostorscore=0 malwarescore=0 lowpriorityscore=0 suspectscore=0 phishscore=0 priorityscore=1501 mlxlogscore=858 bulkscore=0 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2304280000 definitions=main-2305120141 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1765709056284929252?= X-GMAIL-MSGID: =?utf-8?q?1765709056284929252?= |
Series |
[RFC] usb: dwc3: core: set force_gen1 bit in USB31 devices if max speed is SS
|
|
Commit Message
Krishna Kurapati
May 12, 2023, 5:01 p.m. UTC
Currently for dwc3_usb31 devices, if maximum_speed is limited to
super-speed in DT, then device mode is limited to SS, but host mode
still works in SSP.
The documentation for max-speed property is as follows:
"Tells USB controllers we want to work up to a certain speed.
Incase this isn't passed via DT, USB controllers should default to
their maximum HW capability."
It doesn't specify that the property is only for device mode.
Fix this by forcing controller supported max speed to Gen1 by
setting LLUCTL.Force_Gen1 bit if controller is DWC3_USB31 and
max speed is mentioned as SS in DT.
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
---
Discussion regarding the same at:
https://lore.kernel.org/all/e465c69c-3a9d-cbdb-d44e-96b99cfa1a92@quicinc.com/
drivers/usb/dwc3/core.c | 13 +++++++++++++
drivers/usb/dwc3/core.h | 4 ++++
2 files changed, 17 insertions(+)
Comments
On Fri, May 12, 2023, Krishna Kurapati wrote: > Currently for dwc3_usb31 devices, if maximum_speed is limited to We usually call the controller dwc_usb3, dwc_usb31, or dwc_usb32. > super-speed in DT, then device mode is limited to SS, but host mode > still works in SSP. > > The documentation for max-speed property is as follows: > > "Tells USB controllers we want to work up to a certain speed. > Incase this isn't passed via DT, USB controllers should default to > their maximum HW capability." > > It doesn't specify that the property is only for device mode. Since this isn't really a fix, can we rephrase the lines below > Fix this by forcing controller supported max speed to Gen1 by > setting LLUCTL.Force_Gen1 bit if controller is DWC3_USB31 and > max speed is mentioned as SS in DT. As follow: There are cases where we need to limit the host's maximum speed to SuperSpeed only. Use this property for host mode to contrain host's speed to SuperSpeed. > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > --- > Discussion regarding the same at: > https://urldefense.com/v3/__https://lore.kernel.org/all/e465c69c-3a9d-cbdb-d44e-96b99cfa1a92@quicinc.com/__;!!A4F2R9G_pg!YiQpjZIJAw-yu6gEwbKqb5nusjnKQ9dQJrulx39lQP-7JMhcNA2xd8uLJoZ_HE8SuG4Rm2uvhJTSdQ2k0fJVAxU2RWYHHg$ > > drivers/usb/dwc3/core.c | 13 +++++++++++++ > drivers/usb/dwc3/core.h | 4 ++++ > 2 files changed, 17 insertions(+) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 0beaab932e7d..989dc76ecbca 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -116,6 +116,18 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode) > dwc->current_dr_role = mode; > } > > +static void dwc3_configure_host_speed(struct dwc3 *dwc) > +{ > + u32 reg; > + > + if (DWC3_IP_IS(DWC31) && > + (dwc->maximum_speed == USB_SPEED_SUPER)) { > + reg = dwc3_readl(dwc->regs, DWC3_LLUCTL); > + reg |= DWC3_LLUCTL_FORCE_GEN1; > + dwc3_writel(dwc->regs, DWC3_LLUCTL, reg); > + } > +} > + > static void __dwc3_set_mode(struct work_struct *work) > { > struct dwc3 *dwc = work_to_dwc(work); > @@ -194,6 +206,7 @@ static void __dwc3_set_mode(struct work_struct *work) > > switch (desired_dr_role) { > case DWC3_GCTL_PRTCAP_HOST: > + dwc3_configure_host_speed(dwc); The LLUCTL doesn't change until there's a Vcc reset. Let's just initialize it once during dwc3_core_init() if the GHWPARAM indicates the controller is DRD or host only. > ret = dwc3_host_init(dwc); > if (ret) { > dev_err(dwc->dev, "failed to initialize host\n"); > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index d56457c02996..29b780a58dc6 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -121,6 +121,10 @@ > #define DWC3_GPRTBIMAP_FS0 0xc188 > #define DWC3_GPRTBIMAP_FS1 0xc18c > #define DWC3_GUCTL2 0xc19c > +#define DWC3_LLUCTL 0xd024 Please place the register according to its offset order. > + > +/* Force Gen1 speed on Gen2 link */ > +#define DWC3_LLUCTL_FORCE_GEN1 BIT(10) > > #define DWC3_VER_NUMBER 0xc1a0 > #define DWC3_VER_TYPE 0xc1a4 > -- > 2.40.0 > Thanks, Thinh
Hi Thinh, On 5/13/2023 12:16 AM, Thinh Nguyen wrote: > On Fri, May 12, 2023, Krishna Kurapati wrote: >> Currently for dwc3_usb31 devices, if maximum_speed is limited to > > We usually call the controller dwc_usb3, dwc_usb31, or dwc_usb32. > >> super-speed in DT, then device mode is limited to SS, but host mode >> still works in SSP. >> >> The documentation for max-speed property is as follows: >> >> "Tells USB controllers we want to work up to a certain speed. >> Incase this isn't passed via DT, USB controllers should default to >> their maximum HW capability." >> >> It doesn't specify that the property is only for device mode. > > Since this isn't really a fix, can we rephrase the lines below > >> Fix this by forcing controller supported max speed to Gen1 by >> setting LLUCTL.Force_Gen1 bit if controller is DWC3_USB31 and >> max speed is mentioned as SS in DT. > > As follow: > There are cases where we need to limit the host's maximum speed to > SuperSpeed only. Use this property for host mode to contrain host's > speed to SuperSpeed. > > Sure, will rephrase it accordingly. Thanks for the suggestion. >> >> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> >> --- >> Discussion regarding the same at: >> https://urldefense.com/v3/__https://lore.kernel.org/all/e465c69c-3a9d-cbdb-d44e-96b99cfa1a92@quicinc.com/__;!!A4F2R9G_pg!YiQpjZIJAw-yu6gEwbKqb5nusjnKQ9dQJrulx39lQP-7JMhcNA2xd8uLJoZ_HE8SuG4Rm2uvhJTSdQ2k0fJVAxU2RWYHHg$ >> >> drivers/usb/dwc3/core.c | 13 +++++++++++++ >> drivers/usb/dwc3/core.h | 4 ++++ >> 2 files changed, 17 insertions(+) >> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >> index 0beaab932e7d..989dc76ecbca 100644 >> --- a/drivers/usb/dwc3/core.c >> +++ b/drivers/usb/dwc3/core.c >> @@ -116,6 +116,18 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode) >> dwc->current_dr_role = mode; >> } >> >> +static void dwc3_configure_host_speed(struct dwc3 *dwc) >> +{ >> + u32 reg; >> + >> + if (DWC3_IP_IS(DWC31) && >> + (dwc->maximum_speed == USB_SPEED_SUPER)) { >> + reg = dwc3_readl(dwc->regs, DWC3_LLUCTL); >> + reg |= DWC3_LLUCTL_FORCE_GEN1; >> + dwc3_writel(dwc->regs, DWC3_LLUCTL, reg); >> + } >> +} >> + >> static void __dwc3_set_mode(struct work_struct *work) >> { >> struct dwc3 *dwc = work_to_dwc(work); >> @@ -194,6 +206,7 @@ static void __dwc3_set_mode(struct work_struct *work) >> >> switch (desired_dr_role) { >> case DWC3_GCTL_PRTCAP_HOST: >> + dwc3_configure_host_speed(dwc); > > The LLUCTL doesn't change until there's a Vcc reset. Let's just> initialize it once during dwc3_core_init() if the GHWPARAM indicates the > controller is DRD or host only. > I thought GCTL Core soft reset might clear this bit. That is why I placed it here. For device mode gadget.c takes care of limiting speed. So wanted to do this setting only for host mode, before we invoke host init. Thanks for letting know that only VCC reset affects this. Will move this check to core init. Regards, Krishna, >> ret = dwc3_host_init(dwc); >> if (ret) { >> dev_err(dwc->dev, "failed to initialize host\n"); >> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >> index d56457c02996..29b780a58dc6 100644 >> --- a/drivers/usb/dwc3/core.h >> +++ b/drivers/usb/dwc3/core.h >> @@ -121,6 +121,10 @@ >> #define DWC3_GPRTBIMAP_FS0 0xc188 >> #define DWC3_GPRTBIMAP_FS1 0xc18c >> #define DWC3_GUCTL2 0xc19c >> +#define DWC3_LLUCTL 0xd024 > > Please place the register according to its offset order. > >> + >> +/* Force Gen1 speed on Gen2 link */ >> +#define DWC3_LLUCTL_FORCE_GEN1 BIT(10) >> >> #define DWC3_VER_NUMBER 0xc1a0 >> #define DWC3_VER_TYPE 0xc1a4 >> -- >> 2.40.0 >> > > Thanks, > Thinh
On 5/13/2023 12:45 AM, Krishna Kurapati PSSNV wrote: > Hi Thinh, > > On 5/13/2023 12:16 AM, Thinh Nguyen wrote: >> On Fri, May 12, 2023, Krishna Kurapati wrote: >>> Currently for dwc3_usb31 devices, if maximum_speed is limited to >> >> We usually call the controller dwc_usb3, dwc_usb31, or dwc_usb32. >> >>> super-speed in DT, then device mode is limited to SS, but host mode >>> still works in SSP. >>> >>> The documentation for max-speed property is as follows: >>> >>> "Tells USB controllers we want to work up to a certain speed. >>> Incase this isn't passed via DT, USB controllers should default to >>> their maximum HW capability." >>> >>> It doesn't specify that the property is only for device mode. >> >> Since this isn't really a fix, can we rephrase the lines below >> >>> Fix this by forcing controller supported max speed to Gen1 by >>> setting LLUCTL.Force_Gen1 bit if controller is DWC3_USB31 and >>> max speed is mentioned as SS in DT. >> >> As follow: >> There are cases where we need to limit the host's maximum speed to >> SuperSpeed only. Use this property for host mode to contrain host's >> speed to SuperSpeed. >> >> > Sure, will rephrase it accordingly. Thanks for the suggestion. >>> >>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> >>> --- >>> Discussion regarding the same at: >>> https://urldefense.com/v3/__https://lore.kernel.org/all/e465c69c-3a9d-cbdb-d44e-96b99cfa1a92@quicinc.com/__;!!A4F2R9G_pg!YiQpjZIJAw-yu6gEwbKqb5nusjnKQ9dQJrulx39lQP-7JMhcNA2xd8uLJoZ_HE8SuG4Rm2uvhJTSdQ2k0fJVAxU2RWYHHg$ >>> >>> drivers/usb/dwc3/core.c | 13 +++++++++++++ >>> drivers/usb/dwc3/core.h | 4 ++++ >>> 2 files changed, 17 insertions(+) >>> >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>> index 0beaab932e7d..989dc76ecbca 100644 >>> --- a/drivers/usb/dwc3/core.c >>> +++ b/drivers/usb/dwc3/core.c >>> @@ -116,6 +116,18 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode) >>> dwc->current_dr_role = mode; >>> } >>> +static void dwc3_configure_host_speed(struct dwc3 *dwc) >>> +{ >>> + u32 reg; >>> + >>> + if (DWC3_IP_IS(DWC31) && >>> + (dwc->maximum_speed == USB_SPEED_SUPER)) { >>> + reg = dwc3_readl(dwc->regs, DWC3_LLUCTL); >>> + reg |= DWC3_LLUCTL_FORCE_GEN1; >>> + dwc3_writel(dwc->regs, DWC3_LLUCTL, reg); >>> + } >>> +} >>> + >>> static void __dwc3_set_mode(struct work_struct *work) >>> { >>> struct dwc3 *dwc = work_to_dwc(work); >>> @@ -194,6 +206,7 @@ static void __dwc3_set_mode(struct work_struct >>> *work) >>> switch (desired_dr_role) { >>> case DWC3_GCTL_PRTCAP_HOST: >>> + dwc3_configure_host_speed(dwc); >> > The LLUCTL doesn't change until there's a Vcc reset. Let's just> > initialize it once during dwc3_core_init() if the GHWPARAM indicates the >> controller is DRD or host only. >> > > I thought GCTL Core soft reset might clear this bit. That is why I > placed it here. For device mode gadget.c takes care of limiting speed. > So wanted to do this setting only for host mode, before we invoke host > init. > > Thanks for letting know that only VCC reset affects this. Will move this > check to core init. > > Regards, > Krishna, > In fact, The following snippet from programming guide is why I thought GCTL reset can affect this register: ( the below is from usb3 prog guide, I assume this would be applicable for usb31 as well) Fields for Register: GCTL Core Soft Reset (CoreSoftReset) ■ 1'b0 - No soft reset ■ 1'b1 - Soft reset to controller Clears the interrupts and all the CSRs except the following registers: ■ GCTL ■ GUCTL ■ GSTS ■ GSNPSID ■ GGPIO ■ GUID ■ GUSB2PHYCFGn registers ■ GUSB3PIPECTLn registers ■ DCFG ■ DCTL ■ DEVTEN ■ DSTS If so, don't we need to do this setting after every GCTL core soft reset ? Regards, Krishna, >>> ret = dwc3_host_init(dwc); >>> if (ret) { >>> dev_err(dwc->dev, "failed to initialize host\n"); >>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >>> index d56457c02996..29b780a58dc6 100644 >>> --- a/drivers/usb/dwc3/core.h >>> +++ b/drivers/usb/dwc3/core.h >>> @@ -121,6 +121,10 @@ >>> #define DWC3_GPRTBIMAP_FS0 0xc188 >>> #define DWC3_GPRTBIMAP_FS1 0xc18c >>> #define DWC3_GUCTL2 0xc19c >>> +#define DWC3_LLUCTL 0xd024 >> >> Please place the register according to its offset order. >> >>> + >>> +/* Force Gen1 speed on Gen2 link */ >>> +#define DWC3_LLUCTL_FORCE_GEN1 BIT(10) >>> #define DWC3_VER_NUMBER 0xc1a0 >>> #define DWC3_VER_TYPE 0xc1a4 >>> -- >>> 2.40.0 >>> >> >> Thanks, >> Thinh
On Sat, May 13, 2023, Krishna Kurapati PSSNV wrote: > > > On 5/13/2023 12:45 AM, Krishna Kurapati PSSNV wrote: > > Hi Thinh, > > > > On 5/13/2023 12:16 AM, Thinh Nguyen wrote: > > > On Fri, May 12, 2023, Krishna Kurapati wrote: > > > > Currently for dwc3_usb31 devices, if maximum_speed is limited to > > > > > > We usually call the controller dwc_usb3, dwc_usb31, or dwc_usb32. > > > > > > > super-speed in DT, then device mode is limited to SS, but host mode > > > > still works in SSP. > > > > > > > > The documentation for max-speed property is as follows: > > > > > > > > "Tells USB controllers we want to work up to a certain speed. > > > > Incase this isn't passed via DT, USB controllers should default to > > > > their maximum HW capability." > > > > > > > > It doesn't specify that the property is only for device mode. > > > > > > Since this isn't really a fix, can we rephrase the lines below > > > > > > > Fix this by forcing controller supported max speed to Gen1 by > > > > setting LLUCTL.Force_Gen1 bit if controller is DWC3_USB31 and > > > > max speed is mentioned as SS in DT. > > > > > > As follow: > > > There are cases where we need to limit the host's maximum speed to > > > SuperSpeed only. Use this property for host mode to contrain host's > > > speed to SuperSpeed. > > > > > > > > Sure, will rephrase it accordingly. Thanks for the suggestion. > > > > > > > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > > > > --- > > > > Discussion regarding the same at: > > > > https://urldefense.com/v3/__https://lore.kernel.org/all/e465c69c-3a9d-cbdb-d44e-96b99cfa1a92@quicinc.com/__;!!A4F2R9G_pg!YiQpjZIJAw-yu6gEwbKqb5nusjnKQ9dQJrulx39lQP-7JMhcNA2xd8uLJoZ_HE8SuG4Rm2uvhJTSdQ2k0fJVAxU2RWYHHg$ > > > > > > > > drivers/usb/dwc3/core.c | 13 +++++++++++++ > > > > drivers/usb/dwc3/core.h | 4 ++++ > > > > 2 files changed, 17 insertions(+) > > > > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > > > index 0beaab932e7d..989dc76ecbca 100644 > > > > --- a/drivers/usb/dwc3/core.c > > > > +++ b/drivers/usb/dwc3/core.c > > > > @@ -116,6 +116,18 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode) > > > > dwc->current_dr_role = mode; > > > > } > > > > +static void dwc3_configure_host_speed(struct dwc3 *dwc) > > > > +{ > > > > + u32 reg; > > > > + > > > > + if (DWC3_IP_IS(DWC31) && > > > > + (dwc->maximum_speed == USB_SPEED_SUPER)) { > > > > + reg = dwc3_readl(dwc->regs, DWC3_LLUCTL); > > > > + reg |= DWC3_LLUCTL_FORCE_GEN1; > > > > + dwc3_writel(dwc->regs, DWC3_LLUCTL, reg); > > > > + } > > > > +} > > > > + > > > > static void __dwc3_set_mode(struct work_struct *work) > > > > { > > > > struct dwc3 *dwc = work_to_dwc(work); > > > > @@ -194,6 +206,7 @@ static void __dwc3_set_mode(struct > > > > work_struct *work) > > > > switch (desired_dr_role) { > > > > case DWC3_GCTL_PRTCAP_HOST: > > > > + dwc3_configure_host_speed(dwc); > > > > The LLUCTL doesn't change until there's a Vcc reset. Let's just> > > initialize it once during dwc3_core_init() if the GHWPARAM indicates the > > > controller is DRD or host only. > > > > > > > I thought GCTL Core soft reset might clear this bit. That is why I > > placed it here. For device mode gadget.c takes care of limiting speed. > > So wanted to do this setting only for host mode, before we invoke host > > init. > > > > Thanks for letting know that only VCC reset affects this. Will move this > > check to core init. > > > > Regards, > > Krishna, > > > > In fact, The following snippet from programming guide is why I thought GCTL > reset can affect this register: ( the below is from usb3 prog guide, I > assume this would be applicable for usb31 as well) No. Don't use the same programming guide for different controller version. The version you're using (1.90a) deprecated GCTL.CoreSoftReset. > > Fields for Register: GCTL > Core Soft Reset (CoreSoftReset) > ■ 1'b0 - No soft reset > ■ 1'b1 - Soft reset to controller > > Clears the interrupts and all the CSRs except the following > registers: > ■ GCTL > ■ GUCTL > ■ GSTS > ■ GSNPSID > ■ GGPIO > ■ GUID > ■ GUSB2PHYCFGn registers > ■ GUSB3PIPECTLn registers > ■ DCFG > ■ DCTL > ■ DEVTEN > ■ DSTS > > If so, don't we need to do this setting after every GCTL core soft reset ? > Don't look at that list. It's incomplete. Depending on what programming guide version you have, usually the newer versions document these info under "Additional Register Information" IMHO, it's arguably cleaner to place it in dwc3_core_init(), but it's fine to place it in host init also. If you feel it's better to place it in host init, it's reasonable also. Thanks, Thinh
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 0beaab932e7d..989dc76ecbca 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -116,6 +116,18 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode) dwc->current_dr_role = mode; } +static void dwc3_configure_host_speed(struct dwc3 *dwc) +{ + u32 reg; + + if (DWC3_IP_IS(DWC31) && + (dwc->maximum_speed == USB_SPEED_SUPER)) { + reg = dwc3_readl(dwc->regs, DWC3_LLUCTL); + reg |= DWC3_LLUCTL_FORCE_GEN1; + dwc3_writel(dwc->regs, DWC3_LLUCTL, reg); + } +} + static void __dwc3_set_mode(struct work_struct *work) { struct dwc3 *dwc = work_to_dwc(work); @@ -194,6 +206,7 @@ static void __dwc3_set_mode(struct work_struct *work) switch (desired_dr_role) { case DWC3_GCTL_PRTCAP_HOST: + dwc3_configure_host_speed(dwc); ret = dwc3_host_init(dwc); if (ret) { dev_err(dwc->dev, "failed to initialize host\n"); diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index d56457c02996..29b780a58dc6 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -121,6 +121,10 @@ #define DWC3_GPRTBIMAP_FS0 0xc188 #define DWC3_GPRTBIMAP_FS1 0xc18c #define DWC3_GUCTL2 0xc19c +#define DWC3_LLUCTL 0xd024 + +/* Force Gen1 speed on Gen2 link */ +#define DWC3_LLUCTL_FORCE_GEN1 BIT(10) #define DWC3_VER_NUMBER 0xc1a0 #define DWC3_VER_TYPE 0xc1a4