Message ID | 20231208210458.912776-1-CFSworks@gmail.com |
---|---|
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp5719345vqy; Fri, 8 Dec 2023 13:05:21 -0800 (PST) X-Google-Smtp-Source: AGHT+IGOUA3EX4VNqGa+XjzH+SMZ5bAf+U8ME+Yvan4LAi1ctzZ4ry6eIWOoY+eXqGYV1XxvMbMg X-Received: by 2002:a05:6a00:10cc:b0:6ce:71af:842b with SMTP id d12-20020a056a0010cc00b006ce71af842bmr1654467pfu.8.1702069521632; Fri, 08 Dec 2023 13:05:21 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702069521; cv=none; d=google.com; s=arc-20160816; b=OAGJFoDB9gvUEdzKnTpahRHzP3QdtyZbSkqDXeroPhSNxSIwDhzKKUcIxivcKTnlXp Ffa+lGf4BlxcYyqDHlwUdRGu09BKOtD685/8oda0JhbqRQHXUIxKACcjWk/F5+nlU4x1 9gLgtZd1iV7olsihSUNSWbIC+8c40hD+wRyK5TSR5hQW4n1OgGfF+iNJsNWzV4N49OYB t/A/4+exST4yH2b5UzurSC2UPiGFsVDSUN9zOny6kgXHfLa7kZ8nfeaF3OzB6rg7DKce nynJhO8ghWwuP4Xf5H0iQ7WqOOyVruaJ/0+imfSYLUYai3Vq7IKK0QuRY/7In8qWX18A rJcQ== 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=YaHiAFt40ikMOsz0Nbfpr3rQu22G+/tz5j95vM1fm2s=; fh=d9TSyadBXPDybutOgGlYLsH9jqXf9pJuF0lO8Gj/Fg4=; b=X0jxqHKBDGhw8kvaXRrYhfulz1acpA+I9J9+ENjgERpmQipRdaHjKY3Bd6ckrva+RW QE0k+Q8m9YpB/4wRE78nolVvy8WJ4L9Fc/QI2F/o68uM6eRnTpCHrldtKdPJKHOHMvaB 88Sjp4amE9zudUVsqR/El4mgSHjEkpUR54evXlcs6lSQLRzyyKBObj4mcdDU/2qTKBxO B+LZ5Gstw37dDTpzL7g1q6xtS3lrH7TmEy16XQ+W4phlT1p9pEyLxCpam92Ntn4T+6c5 taVP+zK47erNHXEddjBEsuSE9ZX3mPxlCglvhSIG1s/4RmPlJ6Cy9oqWilRAPK7qs52Y jGow== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=hvb2CS2Q; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id u8-20020a63df08000000b00585a5e9a965si2076335pgg.161.2023.12.08.13.05.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Dec 2023 13:05:21 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=hvb2CS2Q; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id C79C081067BD; Fri, 8 Dec 2023 13:05:19 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234292AbjLHVFJ (ORCPT <rfc822;makky5685@gmail.com> + 99 others); Fri, 8 Dec 2023 16:05:09 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43952 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229913AbjLHVFH (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 8 Dec 2023 16:05:07 -0500 Received: from mail-il1-x131.google.com (mail-il1-x131.google.com [IPv6:2607:f8b0:4864:20::131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E4E01171E; Fri, 8 Dec 2023 13:05:13 -0800 (PST) Received: by mail-il1-x131.google.com with SMTP id e9e14a558f8ab-35d6644c1f2so6556365ab.0; Fri, 08 Dec 2023 13:05:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1702069513; x=1702674313; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=YaHiAFt40ikMOsz0Nbfpr3rQu22G+/tz5j95vM1fm2s=; b=hvb2CS2QIgueTVH/drFMVoPB61jOR20LDQJ/4PSCuF0+e2U9T5DR5CXc8BwPwdnjnY 3M5JrYRGWUhWp/bueVcpd5Ybes7mXlykQUhM7mUHwtWzz0lHEa5pSKihbB9IPoSODWCX 4C+P9t4tyVUKf78akTF7Zs45pWzNZYNEKMhCClulbzfulqoXAZmqh8PFQgRQvC5pLhTn ZSt0Lx/8u+OsOBTkAgyZVLIVd5F8uNjdZC72vRh2rraLUT8SQLuICji6Di6sMluOLOPV PLGBbPEEamOKOyRObL41nRDW9dJIHnsNBRWaCghHR6wsbUcWHenSjwiOX7AOZIkZeLtj am0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702069513; x=1702674313; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=YaHiAFt40ikMOsz0Nbfpr3rQu22G+/tz5j95vM1fm2s=; b=pWgqtLgHHcsu8AdlQZuVtsDk75tpgJbGZ7vKUrFnrwIURk/dw2pIQu+5V0L9eKLKBQ yC/3W6CBZb5Msyw2kwzx2evZxvcyPQk8Xk9EB0JzyMKNsNtPqQiJLqk7p9dQYKx7pdLZ ujbLK6PC5GzJeF+S6qEWatE1o63JQeeQ2XVSkuTWWJrEJHHlLUH5Ppj4HvkNbmoqrHDh n/d9sn72ZOGn12pTzJL4IzRBJ+1ovnojSqcHBCgblPaiZlwFK1/IqSfI1Blp9vSRX24c NXclShXIzL/s632k8rr2q4Pm0lQHInBP/eOF72Kw/jLbmFrJdnmzUEVCLX9sHdOewlxY /q4Q== X-Gm-Message-State: AOJu0Yxqm3V+Tyc8KbyAACnTR2r2bAsm8qFHb95EFjjvYhQFlo51lZsu GmhlODeWdktdZCyxKwbtn40= X-Received: by 2002:a05:6e02:2148:b0:35c:a889:76f2 with SMTP id d8-20020a056e02214800b0035ca88976f2mr588804ilv.8.1702069513024; Fri, 08 Dec 2023 13:05:13 -0800 (PST) Received: from celestia.nettie.lan ([2001:470:42c4:101:971d:15c7:de39:3b4b]) by smtp.gmail.com with ESMTPSA id g12-20020a056e020d0c00b0035d5a1760c1sm736458ilj.69.2023.12.08.13.05.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Dec 2023 13:05:12 -0800 (PST) From: Sam Edwards <cfsworks@gmail.com> X-Google-Original-From: Sam Edwards <CFSworks@gmail.com> To: Mathias Nyman <mathias.nyman@intel.com>, Thinh Nguyen <Thinh.Nguyen@synopsys.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Heiko Stuebner <heiko@sntech.de>, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Sam Edwards <CFSworks@gmail.com> Subject: [PATCH 0/2] Allow disabling USB3 ports in xHCI/DWC3 Date: Fri, 8 Dec 2023 14:04:56 -0700 Message-ID: <20231208210458.912776-1-CFSworks@gmail.com> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE 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-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Fri, 08 Dec 2023 13:05:19 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784749250580945932 X-GMAIL-MSGID: 1784749250580945932 |
Series |
Allow disabling USB3 ports in xHCI/DWC3
|
|
Message
Sam Edwards
Dec. 8, 2023, 9:04 p.m. UTC
Hi USB devs, This patchset is a semi-RFC: I haven't discussed this change yet, and it may turn out to be a bad idea. But if there is a consensus that this change is appropriate, these patches are the ones I'd submit for inclusion. These patches were developed while working with a SoC (Rockchip RK3588) that contains DWC3-OTG controllers and accompanying USB2 + USB3/DP PHYs. My target (Turing RK1) uses its first bus in USB2.0-OTG mode only: the associated USB3 PHY is reserved for DP. Worse, a driver for the USBDP block (though it exists) has not been merged to mainline. Without lighting up the PHY side of the PIPE, the DWC3 behaves erratically, even for USB2 operation. This could be addressed by patching in the (out-of-tree) USBDP driver and enabling only its USB backend. However, I found it cleaner (also from a user-friendliness standpoint) just to disable the unusable USB3 port. These patches achieve that by (1) making it possible to tell the xHCI driver to ignore any USB3 port(s), and (2) (perhaps more controversially) making the DWC3 driver disable USB3 host ports when `maximum-speed` isn't set high enough. There are other ways to disable the USB3 ports on RK3588, such as via some syscon registers. I figured I would start with the most general solution (benefitting other SoCs) first, getting more specific only if necessary. :) Thank you for your attention, Sam Sam Edwards (2): xhci: Introduce "disable-usb3" DT property/quirk usb: dwc3: host: Disable USB3 ports if maximum-speed doesn't permit USB3 Documentation/devicetree/bindings/usb/usb-xhci.yaml | 4 ++++ drivers/usb/dwc3/host.c | 5 ++++- drivers/usb/host/xhci-mem.c | 4 ++++ drivers/usb/host/xhci-plat.c | 3 +++ drivers/usb/host/xhci.h | 1 + 5 files changed, 16 insertions(+), 1 deletion(-)
Comments
On 8.12.2023 23.04, Sam Edwards wrote: > Hi USB devs, > > This patchset is a semi-RFC: I haven't discussed this change yet, and it may > turn out to be a bad idea. But if there is a consensus that this change is > appropriate, these patches are the ones I'd submit for inclusion. > > These patches were developed while working with a SoC (Rockchip RK3588) that > contains DWC3-OTG controllers and accompanying USB2 + USB3/DP PHYs. My target > (Turing RK1) uses its first bus in USB2.0-OTG mode only: the associated USB3 > PHY is reserved for DP. Worse, a driver for the USBDP block (though it exists) > has not been merged to mainline. Without lighting up the PHY side of the PIPE, > the DWC3 behaves erratically, even for USB2 operation. > > This could be addressed by patching in the (out-of-tree) USBDP driver and > enabling only its USB backend. However, I found it cleaner (also from a > user-friendliness standpoint) just to disable the unusable USB3 port. These > patches achieve that by (1) making it possible to tell the xHCI driver to > ignore any USB3 port(s), and (2) (perhaps more controversially) making the DWC3 > driver disable USB3 host ports when `maximum-speed` isn't set high enough. I don't think this will work as a generic xhci driver feature. Even if we ignore all USB3 ports in software they will for most xHC hosts be powered and enabled in hardware by default after controller reset. This means they perform link training, generate all kinds of events with interrupts (connect, over-current etc) that driver now can't handle. Sound like the setup you are using has a very specific issue, and it would need a narrow targeted quirk to solve it. > > There are other ways to disable the USB3 ports on RK3588, such as via some > syscon registers. I figured I would start with the most general solution > (benefitting other SoCs) first, getting more specific only if necessary. :) To me a specific solution to a specific problem like this sounds better. Thanks Mathias
Hi Mathias, On 12/14/23 04:05, Mathias Nyman wrote: > I don't think this will work as a generic xhci driver feature. > > Even if we ignore all USB3 ports in software they will for most xHC > hosts be powered > and enabled in hardware by default after controller reset. > > This means they perform link training, generate all kinds of events with > interrupts > (connect, over-current etc) that driver now can't handle. By this do you mean that having the xHCI driver ignore the USB3 ports isn't enough to ensure that PP=0 (and the driver would need to do a little bit more to make sure that the "parking brake" is on: e.g. initialize, but not use, the ports) or that the xHC's PP=0 signal isn't sufficient to keep the PHYs from trying to bring the link up and generating those interrupts (PP=0 really isn't enough, and there is no general "parking brake" to be found here)? > Sound like the setup you are using has a very specific issue, and it > would need > a narrow targeted quirk to solve it. I infer from this that you're against having a DT property added to xHCI? What if the property were to be narrowed in scope to "ignore the USB3 PHYs, they're disabled/absent" vs. this iteration's "disable the USB3 ports" meaning? If this quirk ends up landing in the dwc3 driver (since, arguably, DWC3 is the real misbehaving hw block in these circumstances), what would be your preferred mechanism of signaling to the xHCI layer "the USB3 PHYs have been disabled; please ignore"? > >> >> There are other ways to disable the USB3 ports on RK3588, such as via >> some >> syscon registers. I figured I would start with the most general solution >> (benefitting other SoCs) first, getting more specific only if >> necessary. :) > > To me a specific solution to a specific problem like this sounds better. I am starting to think so as well. I may shift my focus to DWC3 (with xHCI driver changes made only to facilitate them) for now, since `maximum-speed = "high-speed";` very reasonably (imo) should prevent registering the usb3 rhub -- though something may convince me otherwise in the near future. :) > Thanks > Mathias Thanks to you as well, this is exactly the type of feedback I was fishing for! Cheers, Sam
On 15.12.2023 23.59, Sam Edwards wrote: > Hi Mathias, > > On 12/14/23 04:05, Mathias Nyman wrote: >> I don't think this will work as a generic xhci driver feature. >> >> Even if we ignore all USB3 ports in software they will for most xHC hosts be powered >> and enabled in hardware by default after controller reset. >> >> This means they perform link training, generate all kinds of events with interrupts >> (connect, over-current etc) that driver now can't handle. > > By this do you mean that having the xHCI driver ignore the USB3 ports isn't enough to ensure that PP=0 (and the driver would need to do a little bit more to make sure that the "parking brake" is on: e.g. initialize, but not use, the ports) or that the xHC's PP=0 signal isn't sufficient to keep the PHYs from trying to bring the link up and generating those interrupts (PP=0 really isn't enough, and there is no general "parking brake" to be found here)? > Yes, in most cases PP==1 after xHC reset, here's some old debug output during boot: [ 2.571057] xhci_hcd 0000:00:0d.0: new USB bus registered, assigned bus number 2 [ 2.571061] xhci_hcd 0000:00:0d.0: Host supports USB 3.2 Enhanced SuperSpeed [ 2.571065] xhci_hcd 0000:00:0d.0: // Turn on HC, cmd = 0x5. [ 2.571067] xhci_hcd 0000:00:0d.0: Finished xhci_run for USB3 roothub [ 2.571093] usb usb2: New USB device found, idVendor=1d6b, idProduct=0003, bcdDevice= 5.15 [ 2.571095] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [ 2.571097] usb usb2: Product: xHCI Host Controller [ 2.571098] usb usb2: Manufacturer: Linux 5.15.57-06982-gf7339f7585d8-dirty xhci-hcd [ 2.571099] usb usb2: SerialNumber: 0000:00:0d.0 [ 2.571197] xHCI xhci_add_endpoint called for root hub [ 2.571199] xHCI xhci_check_bandwidth called for root hub [ 2.571224] hub 2-0:1.0: USB hub found [ 2.571230] hub 2-0:1.0: 2 ports detected [ 2.571279] xhci_hcd 0000:00:0d.0: set port power 2-1 ON, portsc: 0x2a0 Note that portsc: 0x2a0 entry above has PP=1, and it shows the portsc register value _before_ port power is set to 1 (ON). Port Status: 0x2a0 Disconnected Disabled Link: Rx Detect Powered Unknown port speed Forcing PP=0 could possibly prevent any events from those ports. >> Sound like the setup you are using has a very specific issue, and it would need >> a narrow targeted quirk to solve it. > > I infer from this that you're against having a DT property added to xHCI? What if the property were to be narrowed in scope to "ignore the USB3 PHYs, they're disabled/absent" vs. this iteration's "disable the USB3 ports" meaning? > > If this quirk ends up landing in the dwc3 driver (since, arguably, DWC3 is the real misbehaving hw block in these circumstances), what would be your preferred mechanism of signaling to the xHCI layer "the USB3 PHYs have been disabled; please ignore"? I don't have a good solution in mind for this so I'll just throw some ideas: What happends if you in some RK3588 platform code disable USB ports via those syscon registers, but let the xhci driver be, and USB3 roothub enumerates normally? Or if this is about a misbehaving USB3 PHY, how about adding the USB3 PHY driver that describes reality and fails when usb_phy_roothub_init() or usb_phy_roothub_set_mode() are called by for USB3 hcd during usb_add_hcd(USB3). xhci driver could then continue without the USB3 hcd. turning off USB3 ports. Adding the 'maximum-speed = "high-speed"' entry could also be one option. Thanks Mathias