Linux USB
 help / color / mirror / Atom feed
* Re: [PATCH] usb: typec: altmode: Fix altmode to handle multiple parners
From: François Scala @ 2026-04-02 18:10 UTC (permalink / raw)
  To: Heikki Krogerus; +Cc: linux-usb
In-Reply-To: <ac5uh_3N3q61efIb@kuha>

Hi,

On 02/04/2026 15.26, Heikki Krogerus wrote:
> No. You can not have more than a single partner per mode. Let's figure
> out the root issue. Please check the svids of the partner altmodes:
>
>          grep . /sys/class/typec/port0-partner/port0-partner.*/svid

/sys/class/typec/port2-partner/port2-partner.0/svid:8087
/sys/class/typec/port2-partner/port2-partner.1/svid:8087

> Then provide the trace output from the ucsi driver, dmesg output
> (full), and also acpidump of your system. The ucsi trace you can
> get like this (assuming you have the debugfs mounted at
> /sys/kernel/debug):
>
>          cd /sys/kernel/debug/tracing

That /sys/kernel/tracing (without /debug).

The traces with port3 (tb4) and port0 (tb5):

# entries-in-buffer/entries-written: 19/19   #P:16
#
#                                _-----=> irqs-off/BH-disabled
#                               / _----=> need-resched
#                              | / _---=> hardirq/softirq
#                              || / _--=> preempt-depth
#                              ||| / _-=> migrate-disable
#                              |||| /     delay
#           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
#              | |         |   |||||     |         |
      kworker/5:2-321     [005] .....   465.001617: 
ucsi_connector_change: port3 status: change=4000, opmode=4, connected=1, 
sourcing=1, partner_flags=1, partner_type=2, request_data_obj=00000000, 
BC status=1
      kworker/5:2-321     [005] .....   465.111779: 
ucsi_connector_change: port3 status: change=0060, opmode=3, connected=1, 
sourcing=1, partner_flags=1, partner_type=2, request_data_obj=13800000, 
BC status=1
      kworker/8:1-174     [008] .....   465.429999: 
ucsi_connector_change: port3 status: change=1000, opmode=5, connected=1, 
sourcing=0, partner_flags=1, partner_type=2, request_data_obj=13800000, 
BC status=1
      kworker/8:1-174     [008] .....   465.532708: 
ucsi_connector_change: port3 status: change=0060, opmode=3, connected=1, 
sourcing=0, partner_flags=1, partner_type=2, request_data_obj=42c4b12c, 
BC status=1
     kworker/10:1-178     [010] .....   465.883679: 
ucsi_connector_change: port3 status: change=0060, opmode=3, connected=1, 
sourcing=0, partner_flags=1, partner_type=2, request_data_obj=42c7a9ea, 
BC status=1
      kworker/3:0-13120   [003] .....   466.330330: 
ucsi_connector_change: port3 status: change=0060, opmode=3, connected=1, 
sourcing=0, partner_flags=1, partner_type=2, request_data_obj=82c7d1f4, 
BC status=1
    kworker/u64:2-356     [005] .....   466.890372: 
ucsi_register_altmode: partner alt mode: svid 8087, mode 1 vdo 8087a843
    kworker/u64:2-356     [005] .....   467.005141: 
ucsi_register_altmode: partner alt mode: svid 8087, mode 2 vdo 1
      kworker/2:1-177     [002] .....   471.027602: 
ucsi_connector_change: port3 status: change=0200, opmode=3, connected=1, 
sourcing=0, partner_flags=1, partner_type=2, request_data_obj=82c7d1f4, 
BC status=1
      kworker/2:1-177     [002] .....  1140.998711: 
ucsi_connector_change: port3 status: change=4000, opmode=1, connected=0, 
sourcing=0, partner_flags=0, partner_type=0, request_data_obj=82c7d1f4, 
BC status=0
      kworker/0:2-15093   [000] .....  1164.065528: 
ucsi_connector_change: port0 status: change=4000, opmode=4, connected=1, 
sourcing=1, partner_flags=1, partner_type=2, request_data_obj=00000000, 
BC status=1
      kworker/0:2-15093   [000] .....  1164.173696: 
ucsi_connector_change: port0 status: change=0060, opmode=3, connected=1, 
sourcing=1, partner_flags=1, partner_type=2, request_data_obj=13800000, 
BC status=1
      kworker/7:0-13116   [007] .....  1164.508267: 
ucsi_connector_change: port0 status: change=1000, opmode=5, connected=1, 
sourcing=0, partner_flags=1, partner_type=2, request_data_obj=13800000, 
BC status=1
      kworker/7:0-13116   [007] .....  1164.609765: 
ucsi_connector_change: port0 status: change=0060, opmode=3, connected=1, 
sourcing=0, partner_flags=1, partner_type=2, request_data_obj=42c4b12c, 
BC status=1
      kworker/6:3-360     [006] .....  1165.119848: 
ucsi_connector_change: port0 status: change=0060, opmode=3, connected=1, 
sourcing=0, partner_flags=1, partner_type=2, request_data_obj=82c7d1f4, 
BC status=1
      kworker/6:3-360     [006] .....  1165.208511: 
ucsi_connector_change: port0 status: change=0060, opmode=3, connected=1, 
sourcing=0, partner_flags=1, partner_type=2, request_data_obj=82c7d1f4, 
BC status=1
    kworker/u64:3-1005    [010] .....  1166.339613: 
ucsi_register_altmode: partner alt mode: svid 8087, mode 1 vdo 8087a843
    kworker/u64:3-1005    [011] .....  1166.454835: 
ucsi_register_altmode: partner alt mode: svid 8087, mode 2 vdo 1
      kworker/1:3-191     [001] .....  1170.026494: 
ucsi_connector_change: port0 status: change=0200, opmode=3, connected=1, 
sourcing=0, partner_flags=1, partner_type=2, request_data_obj=82c7d1f4, 
BC status=1

> For acpidump you need the acpica-tools installed:
>
>          acpidump -o my_acpi.dump

The file is quite large, how can I share it ? Or do you need a specific 
part ?

   -rw-r--r-- 1 fs fs 4.7M Apr  2 19:05 acpi-port3-20260402.dump


Thanks

François







^ permalink raw reply

* [PATCH] usb: gadget: f_hid: Add missing error code
From: Ethan Tidmore @ 2026-04-02 18:00 UTC (permalink / raw)
  To: gregkh, sigmaepsilon92
  Cc: peter, mhklinux, linuxhid, danisjiang, kees, hoff.benjamin.k,
	linux-usb, linux-kernel, Ethan Tidmore

Currently in cdev_alloc() error path no error code is assigned.

Assign error code '-ENOMEM'.

Detected by Smatch:
drivers/usb/gadget/function/f_hid.c:1291 hidg_bind()
warn: missing error code 'status'

Fixes: 81ebd43cc0d6d ("usb: gadget: f_hid: don't call cdev_init while cdev in use")
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
 drivers/usb/gadget/function/f_hid.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index e0c3f39ee95e..c5a12a6760ea 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -1278,8 +1278,10 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
 
 	/* create char device */
 	hidg->cdev = cdev_alloc();
-	if (!hidg->cdev)
+	if (!hidg->cdev) {
+		status = -ENOMEM;
 		goto fail_free_all;
+	}
 	hidg->cdev->ops = &f_hidg_fops;
 
 	status = cdev_device_add(hidg->cdev, &hidg->dev);
-- 
2.53.0


^ permalink raw reply related

* [PATCH] usb: storage: Expand range of matched versions for VL817 quirks entry
From: Daniel Brát @ 2026-04-02 17:24 UTC (permalink / raw)
  To: linux-usb
  Cc: Daniel Brát, Alan Stern, Greg Kroah-Hartman, usb-storage,
	linux-kernel

Expands range of matched bcdDevice values for the VL817 quirk entry.
This is based on experience with Axagon EE35-GTR rev1 3.5" HDD
enclosure, which reports its bcdDevice as 0x0843, but presumably other
vendors using this IC in their products may set it to any other value.

Signed-off-by: Daniel Brát <danek.brat@gmail.com>
---
 drivers/usb/storage/unusual_devs.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h
index 47f50d7a385c..255968f9ca42 100644
--- a/drivers/usb/storage/unusual_devs.h
+++ b/drivers/usb/storage/unusual_devs.h
@@ -2350,10 +2350,11 @@ UNUSUAL_DEV(  0x2027, 0xa001, 0x0000, 0x9999,
 		US_FL_SCM_MULT_TARG ),
 
 /*
- * Reported by DocMAX <mail@vacharakis.de>
- * and Thomas Weißschuh <linux@weissschuh.net>
+ * Reported by DocMAX <mail@vacharakis.de>,
+ * Thomas Weißschuh <linux@weissschuh.net>
+ * and Daniel Brát <danek.brat@gmail.com>
  */
-UNUSUAL_DEV( 0x2109, 0x0715, 0x9999, 0x9999,
+UNUSUAL_DEV( 0x2109, 0x0715, 0x0000, 0x9999,
 		"VIA Labs, Inc.",
 		"VL817 SATA Bridge",
 		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
-- 
2.39.5


^ permalink raw reply related

* Re: [PATCH v2 1/4] dt-bindings: usb: dwc3-xilinx: Add MMI USB support on Versal Gen2 platform
From: Krzysztof Kozlowski @ 2026-04-02 16:36 UTC (permalink / raw)
  To: Pandey, Radhey Shyam, Radhey Shyam Pandey
  Cc: gregkh, robh, krzk+dt, conor+dt, michal.simek, Thinh.Nguyen,
	p.zabel, linux-usb, devicetree, linux-arm-kernel, linux-kernel,
	git
In-Reply-To: <3d64cd29-d981-4e84-8106-8085250fc502@amd.com>

On 31/03/2026 11:18, Pandey, Radhey Shyam wrote:
>> On Tue, Mar 31, 2026 at 12:33:01AM +0530, Radhey Shyam Pandey wrote:
>>> Versal Gen2 platform multimedia integrated (MMI) module has a USB3.2 Gen
>>> 2x1 Dual Role Device IP. Introduce a new compatibility string to support
>>> it. The USB wrapper registers reside in the MMI UDH system-level control
>>> registers (SLCR) block, so instead of a dedicated reg property, add
>>> xlnx,usb-syscon phandle with four cells specifying register offsets for
>>> USB2 PHY, USB3 PHY, USB DRD, and USB power configuration within the SLCR.
>>>
>>> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
>>> ---
>>> Changes for v2:
>>> - Add blank line after compatible as suggested by Krzysztof.
>>> - Retain the mmi suffix in the compatible string, as this USB 3.2 Gen2
>>>    IP from Synopsys is part of the dedicated Multimedia Interface. The
>>>    Versal Gen2 platform also includes a separate USB 2.0 controller,
>>>    and the mmi suffix uniquely distinguishes between the two USB
>>>    controllers. MMI is an independent subsystem particularly targeted for
>>>    deployment in Multi-Media related applications. The MMI block include
>>>    following submodules: UDH: USB3.2 Gen 2x1 Dual Role Device, DisplayPort
>>>    Transmit Controller, Security Module (ESM) for DisplayPort and HDMI
>>>    Controllers, DP AUX-I2C PHY.
>>> - For MMI USB define parent address space i.e UDH block.
>>> - Fix inconsistent MHz spacing to use SI convention with spaces.
>>> - Move description before $ref and items in xlnx,usb-syscon property.
>>> - Restore original zynqmp-dwc3 example, add new versal2-mmi-dwc3 example.
>>> - Use 'usb' node name (without unit address) for versal2 example since
>>>    it has no reg property.
>>> - Use 1/1 address/size configuration in versal2 example, use lowercase
>>>    hex in syscon offsets.
>>> ---
>>>   .../devicetree/bindings/usb/dwc3-xilinx.yaml  | 70 ++++++++++++++++++-
>>>   1 file changed, 67 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3-xilinx.yaml b/Documentation/devicetree/bindings/usb/dwc3-xilinx.yaml
>>> index d6823ef5f9a7..5e31b961aff7 100644
>>> --- a/Documentation/devicetree/bindings/usb/dwc3-xilinx.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/dwc3-xilinx.yaml
>>> @@ -15,6 +15,8 @@ properties:
>>>         - enum:
>>>             - xlnx,zynqmp-dwc3
>>>             - xlnx,versal-dwc3
>>> +          - xlnx,versal2-mmi-dwc3
>> I am not going to ask the same questions.
> 
> I have provided the explanation in v2 changelog.
> Retain the mmi suffix in the compatible string, as this USB 3.2 Gen2
> IP from Synopsys is part of the dedicated Multimedia Interface. The
> Versal Gen2 platform also includes a separate USB 2.0 controller,
> and the mmi suffix uniquely distinguishes between the two USB
> controllers.
> 
> Let me know if you still have reservation in this approach (using
> <vendor>,<soc>-<subsystem>-<ip>). Or any other alternative
> based on IP version etc please suggest.

Commit msg must explain that you have two different devices. And then
explain what is the difference between versal2-dwc3 and versal2-mmi-dwc3.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH] usb: rtl8150: avoid using uninitialized CSCR value
From: Petko Manolov @ 2026-04-02 15:51 UTC (permalink / raw)
  To: Morduan Zang
  Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, linux-usb, netdev, linux-kernel,
	syzbot+9db6c624635564ad813c
In-Reply-To: <93FF85BB9F33CD2B+20260402070743.20641-1-zhangdandan@uniontech.com>

On 26-04-02 15:07:43, Morduan Zang wrote:
> Check get_registers() when reading CSCR in set_carrier().
> If the control transfer fails, don't use the stack value.
> 
> Reported-by: syzbot+9db6c624635564ad813c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=9db6c624635564ad813c
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Morduan Zang <zhangdandan@uniontech.com>
> ---
>  drivers/net/usb/rtl8150.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> index 4cda0643afb6..7e32726d3e6f 100644
> --- a/drivers/net/usb/rtl8150.c
> +++ b/drivers/net/usb/rtl8150.c
> @@ -722,7 +722,11 @@ static void set_carrier(struct net_device *netdev)
>  	rtl8150_t *dev = netdev_priv(netdev);
>  	short tmp;
>  
> -	get_registers(dev, CSCR, 2, &tmp);
> +	if (get_registers(dev, CSCR, 2, &tmp) < 0) {
> +		netif_carrier_off(netdev);
> +		return;
> +	}
> +
>  	if (tmp & CSCR_LINK_STATUS)
>  		netif_carrier_on(netdev);
>  	else
> -- 

Nice catch.  You can add my Acked-by.


cheers,
Petko

^ permalink raw reply

* Re: [PATCH] usb: cdns3: attempt to fix Kconfig dependencies
From: Arnd Bergmann @ 2026-04-02 15:21 UTC (permalink / raw)
  To: Arnd Bergmann, Peter Chen, Pawel Laszczak, Greg Kroah-Hartman
  Cc: Roger Quadros, linux-usb, linux-kernel
In-Reply-To: <20260402141008.2691819-1-arnd@kernel.org>

On Thu, Apr 2, 2026, at 16:09, Arnd Bergmann wrote:
>
> My patch has seen some fairly light build testing at this point and
> is clearly an improvement over what is in linux-next, but there is
> still a risk that I missed some other corner cases.

This is one change I missed:

diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
index 07ed694e10b8..ea87ac22f7cf 100644
--- a/drivers/usb/cdns3/Kconfig
+++ b/drivers/usb/cdns3/Kconfig
@@ -118,7 +118,7 @@ config USB_CDNSP
 if USB_CDNSP
 
 config USB_CDNSP_GADGET
-       bool "Cadence USBSSP device controller"
+       tristate "Cadence USBSSP device controller"
        depends on USB_GADGET=y || USB_GADGET=USB_CDNSP
        help
          Say Y here to enable device controller functionality of the

I'll wait for other comments or build failures before resending
the updated patch.

     Arnd

^ permalink raw reply related

* Re: [PATCH] usb: core: Fix bandwidth for devices with invalid wBytesPerInterval
From: Michal Pecio @ 2026-04-02 15:03 UTC (permalink / raw)
  To: Greg KH
  Cc: Alan Stern, Xuetao (kirin), linux-usb, linux-kernel, caiyadong,
	stable
In-Reply-To: <2026040221-reclusive-garland-6281@gregkh>

On Thu, 2 Apr 2026 16:09:51 +0200, Greg KH wrote:
> I didn't realize this was on "real" devices, sorry.  I thought this
> was only a fuzzing thing.  So yes, fix up the broken descriptor after
> warning about it is the correct thing to do.

Such descriptors are not illegal per USB3 spec and they are seen in the
wild, at least on isochronous endpoints, and they were correct so far.

What's broken is that some particular device specifies less than its
driver actually submits (and presumably needs to submit for it to work).

Regards,
Michal

^ permalink raw reply

* Re: [PATCH v10 6/6] usb: typec: tcpm/tcpci_maxim: deprecate WAR for setting charger mode
From: Heikki Krogerus @ 2026-04-02 14:33 UTC (permalink / raw)
  To: amitsd
  Cc: André Draszik, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Greg Kroah-Hartman, Jagan Sridharan, Mark Brown,
	Matti Vaittinen, Andrew Morton, Sebastian Reichel, Peter Griffin,
	Tudor Ambarus, Alim Akhtar, linux-kernel, devicetree, linux-usb,
	linux-pm, linux-arm-kernel, linux-samsung-soc, RD Babiera,
	Kyle Tso
In-Reply-To: <20260331-max77759-charger-v10-6-76f59233c369@google.com>

Hi Amit,

> +static int get_vbus_regulator_handle(struct max_tcpci_chip *chip)
> +{
> +	if (IS_ERR_OR_NULL(chip->vbus_reg)) {
> +		chip->vbus_reg = devm_regulator_get_exclusive(chip->dev,
> +							      "vbus");

Sorry to go back to this, but why can't you just get the regulator in
max_tcpci_probe()?

thanks,

> +		if (IS_ERR_OR_NULL(chip->vbus_reg)) {
> +			dev_err(chip->dev,
> +				"Failed to get vbus regulator handle\n");
> +			return -ENODEV;
> +		}
> +	}
> +
> +	return 0;
> +}

-- 
heikki

^ permalink raw reply

* [PATCH] usb: cdns3: attempt to fix Kconfig dependencies
From: Arnd Bergmann @ 2026-04-02 14:09 UTC (permalink / raw)
  To: Peter Chen, Pawel Laszczak, Greg Kroah-Hartman
  Cc: Arnd Bergmann, Roger Quadros, linux-usb, linux-kernel

From: Arnd Bergmann <arnd@arndb.de>

The way that dependencies between host and gadget mode, as well as cdns3
and cdnsp were handled was rather fragile before commit 6076388ca1ed
("usb: cdns3: Add USBSSP platform driver support").

After those changes, I get randconfig build failures:

arm-linux-gnueabi-ld: drivers/usb/cdns3/cdnsp-gadget.o: in function `__cdnsp_gadget_init':
cdnsp-gadget.c:(.text+0x12da): undefined reference to `cdns_drd_gadget_on'
arm-linux-gnueabi-ld: drivers/usb/cdns3/cdnsp-gadget.o: in function `cdnsp_gadget_pullup':
cdnsp-gadget.c:(.text+0x3030): undefined reference to `cdns_clear_vbus'
arm-linux-gnueabi-ld: cdnsp-gadget.c:(.text+0x3138): undefined reference to `cdns_set_vbus'
arm-linux-gnueabi-ld: drivers/usb/cdns3/cdnsp-gadget.o: in function `cdnsp_gadget_exit':
cdnsp-gadget.c:(.text+0xe0): undefined reference to `cdns_drd_gadget_off'

and I see additional configurations that are broken. The main problem
here is that the 'common' module links against both host and gadget
support if they are enabled, but there are insufficient protections
agains it being built-in if only one of them is built-in and the other
is in a loadable module, causing link failures.

The use of IS_REACHABLE() in gadget-export.h works around a similar
problem if one of cdns3 and cdnsp is built-in but the other one is
=m. This one is worse because instead of a clear link failure, the
logic just makes it not work at all despite support being enabled.

To improve this mess, throw out both the Makefile hacks and the
IS_REACHABLE() hack and replace these with regular Kconfig dependencies
that ensure each driver is only enabled when its dependencies are there,
as we do in most other drivers. The main downside here is that there is no
good way to have built-in gadget support on cdn3 along with USB=m. Fixing
this part proper would require cleaning up the code to turn the 'common'
parts into a library module that only gets called by the other drivers
but does not interact with either host or gadget support itself.

Another problem that is not solved by this patch is the way that
platform specific glue logic in this driver relies on having
a soc specific device as the parent of a generic child, instead of
the specific driver just calling into a common helper module.
This may be impossible to fix without breaking the DT bindings.

Fixes: 6076388ca1ed ("usb: cdns3: Add USBSSP platform driver support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
TBH, I would be more comfortable with reverting 6076388ca1ed altogether
and asking for a new version with the proper fixups included along
with more testing for the next merge window.

My patch has seen some fairly light build testing at this point and
is clearly an improvement over what is in linux-next, but there is
still a risk that I missed some other corner cases.
---
 drivers/usb/cdns3/Kconfig         | 26 +++++++++++++++++++++-----
 drivers/usb/cdns3/Makefile        | 17 +++++------------
 drivers/usb/cdns3/gadget-export.h |  4 ++--
 3 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
index 97fa84dddbca..07ed694e10b8 100644
--- a/drivers/usb/cdns3/Kconfig
+++ b/drivers/usb/cdns3/Kconfig
@@ -1,6 +1,7 @@
 config USB_CDNS_SUPPORT
 	tristate "Cadence USB Support"
 	depends on USB_SUPPORT && (USB || USB_GADGET) && HAS_DMA
+	depends on USB || !USB
 	select USB_XHCI_PLATFORM if USB_XHCI_HCD
 	select USB_ROLE_SWITCH
 	help
@@ -10,12 +11,24 @@ config USB_CDNS_SUPPORT
 
 config USB_CDNS_HOST
 	bool
+	depends on USB=y || USB=USB_CDNS_SUPPORT
+
+config CONFIG_USB_CDNS_PLATFORM
+	tristate "Cadence USB3 generic platform support"
+	depends on USB_CDNSP || USB_CDNS3
+	depends on USB_CDNSP || !USB_CDNSP
+	depends on USB_CDNS3 || !USB_CDNS3
+	help
+	  The platform driver support is needed on any SoC integrating
+	  a variant of the Cadence USB3 or USBSSP dual-role controllers,
+	  e.g. the TI K3 or NXP i.MX series of SoCs.
 
 if USB_CDNS_SUPPORT
 
 config USB_CDNS3
 	tristate "Cadence USB3 Dual-Role Controller"
 	depends on USB_CDNS_SUPPORT
+	select USB_CDNS_HOST if USB_CDNS3_HOST
 	help
 	  Say Y here if your system has a Cadence USB3 dual-role controller.
 	  It supports: dual-role switch, Host-only, and Peripheral-only.
@@ -23,8 +36,9 @@ config USB_CDNS3
 if USB_CDNS3
 
 config USB_CDNS3_GADGET
-	bool "Cadence USB3 device controller"
+	tristate "Cadence USB3 device controller"
 	depends on USB_GADGET=y || USB_GADGET=USB_CDNS3
+	depends on USB_CDNS_SUPPORT
 	help
 	  Say Y here to enable device controller functionality of the
 	  Cadence USBSS-DEV driver.
@@ -34,8 +48,7 @@ config USB_CDNS3_GADGET
 
 config USB_CDNS3_HOST
 	bool "Cadence USB3 host controller"
-	depends on USB=y || USB=USB_CDNS3
-	select USB_CDNS_HOST
+	depends on USB=y || USB=USB_CDNS_SUPPORT
 	help
 	  Say Y here to enable host controller functionality of the
 	  Cadence driver.
@@ -57,6 +70,7 @@ config USB_CDNS3_PCI_WRAP
 config USB_CDNS3_TI
 	tristate "Cadence USB3 support on TI platforms"
 	depends on ARCH_K3 || COMPILE_TEST
+	depends on USB_CDNS_PLATFORM
 	default USB_CDNS3
 	help
 	  Say 'Y' or 'M' here if you are building for Texas Instruments
@@ -67,6 +81,7 @@ config USB_CDNS3_TI
 config USB_CDNS3_IMX
 	tristate "Cadence USB3 support on NXP i.MX platforms"
 	depends on ARCH_MXC || COMPILE_TEST
+	depends on USB_CDNS_PLATFORM
 	default USB_CDNS3
 	help
 	  Say 'Y' or 'M' here if you are building for NXP i.MX
@@ -77,6 +92,7 @@ config USB_CDNS3_IMX
 config USB_CDNS3_STARFIVE
 	tristate "Cadence USB3 support on StarFive SoC platforms"
 	depends on ARCH_STARFIVE || COMPILE_TEST
+	depends on USB_CDNS_PLATFORM
 	help
 	  Say 'Y' or 'M' here if you are building for StarFive SoCs
 	  platforms that contain Cadence USB3 controller core.
@@ -91,6 +107,7 @@ endif # USB_CDNS3
 config USB_CDNSP
 	tristate "Cadence USBSSP Dual-Role Controller"
 	depends on USB_CDNS_SUPPORT
+	select USB_CDNS_HOST if USB_CDNSP_HOST
 	help
 	  Say Y here if your system has a Cadence USBSSP dual-role controller.
 	  It supports: dual-role switch, Host-only, and Peripheral-only.
@@ -115,8 +132,7 @@ config USB_CDNSP_GADGET
 
 config USB_CDNSP_HOST
 	bool "Cadence USBSSP host controller"
-	depends on USB=y || USB=USB_CDNSP
-	select USB_CDNS_HOST
+	depends on USB=y || USB=USB_CDNS_SUPPORT
 	help
 	  Say Y here to enable host controller functionality of the
 	  Cadence driver.
diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
index 63484f145bb9..f1e00ab3d729 100644
--- a/drivers/usb/cdns3/Makefile
+++ b/drivers/usb/cdns3/Makefile
@@ -5,32 +5,25 @@ CFLAGS_cdnsp-trace.o				:= -I$(src)
 
 cdns-usb-common-y				:= core.o drd.o
 
-ifeq ($(CONFIG_USB),m)
-obj-m						+= cdns-usb-common.o
-obj-m						+= cdns3-plat.o
-else
 obj-$(CONFIG_USB_CDNS_SUPPORT)			+= cdns-usb-common.o
-obj-$(CONFIG_USB_CDNS_SUPPORT)			+= cdns3-plat.o
-endif
+
+obj-$(CONFIG_USB_CDNS_PLATFORM)			+= cdns3-plat.o
 
 cdns-usb-common-$(CONFIG_USB_CDNS_HOST) 	+= host.o
 
 # For CDNS3 gadget
-ifneq ($(CONFIG_USB_CDNS3_GADGET),)
 cdns3-y						:= cdns3-gadget.o cdns3-ep0.o
 cdns3-$(CONFIG_TRACING)				+= cdns3-trace.o
-obj-$(CONFIG_USB_CDNS3)				+= cdns3.o
-endif
+obj-$(CONFIG_USB_CDNS3_GADGET)			+= cdns3.o
+
 obj-$(CONFIG_USB_CDNS3_PCI_WRAP)		+= cdns3-pci-wrap.o
 obj-$(CONFIG_USB_CDNS3_TI)			+= cdns3-ti.o
 obj-$(CONFIG_USB_CDNS3_IMX)			+= cdns3-imx.o
 obj-$(CONFIG_USB_CDNS3_STARFIVE)		+= cdns3-starfive.o
 
 # For CDNSP gadget
-ifneq ($(CONFIG_USB_CDNSP_GADGET),)
 cdnsp-y						:= cdnsp-ring.o cdnsp-gadget.o \
 						   cdnsp-mem.o cdnsp-ep0.o
 cdnsp-$(CONFIG_TRACING)				+= cdnsp-trace.o
-obj-$(CONFIG_USB_CDNSP)				+= cdnsp.o
-endif
+obj-$(CONFIG_USB_CDNSP_GADGET)			+= cdnsp.o
 obj-$(CONFIG_USB_CDNSP_PCI)			+= cdnsp-pci.o
diff --git a/drivers/usb/cdns3/gadget-export.h b/drivers/usb/cdns3/gadget-export.h
index 0cb600e2b5d2..c37b6269b001 100644
--- a/drivers/usb/cdns3/gadget-export.h
+++ b/drivers/usb/cdns3/gadget-export.h
@@ -10,7 +10,7 @@
 #ifndef __LINUX_CDNS3_GADGET_EXPORT
 #define __LINUX_CDNS3_GADGET_EXPORT
 
-#if defined(CONFIG_USB_CDNSP_GADGET) && IS_REACHABLE(CONFIG_USB_CDNSP)
+#if IS_ENABLED(CONFIG_USB_CDNSP_GADGET)
 
 int cdnsp_gadget_init(struct cdns *cdns);
 #else
@@ -22,7 +22,7 @@ static inline int cdnsp_gadget_init(struct cdns *cdns)
 
 #endif /* CONFIG_USB_CDNSP_GADGET */
 
-#if defined(CONFIG_USB_CDNS3_GADGET) && IS_REACHABLE(CONFIG_USB_CDNS3)
+#if IS_ENABLED(CONFIG_USB_CDNS3_GADGET)
 
 int cdns3_gadget_init(struct cdns *cdns);
 #else
-- 
2.39.5


^ permalink raw reply related

* Re: [PATCH] usb: core: Fix bandwidth for devices with invalid wBytesPerInterval
From: Greg KH @ 2026-04-02 14:09 UTC (permalink / raw)
  To: Alan Stern; +Cc: Xuetao (kirin), linux-usb, linux-kernel, caiyadong, stable
In-Reply-To: <74f1bb0d-24c3-44be-9583-0585863cdae3@rowland.harvard.edu>

On Thu, Apr 02, 2026 at 09:56:51AM -0400, Alan Stern wrote:
> On Thu, Apr 02, 2026 at 02:59:35PM +0800, Xuetao (kirin) wrote:
> > 2、Following Alan's suggestion in another email, should I check whether
> > wBytesPerInterval is a valid value and handle it in the
> > usb_parse_ss_endpoint_companion() ?
> 
> Yes, IMO.
> 
> > However, when parsing the device descriptor, we do not know whether the
> > actual data length transmitted by the peripheral is greater than
> > wBytesPerInterval.
> 
> Note: wBytesPerInterval is in the endpoint descriptor, not the device 
> descriptor.
> 
> > Therefore, would it be sufficient to only add a check for whether
> > wBytesPerInterval is 0 in the existing flow, and if it is 0, set
> > wBytesPerInterval to cpu_to_le16(max_tx) by default?
> > 
> > For example, modify it in the following way:
> > 
> >      if (le16_to_cpu(desc->wBytesPerInterval) > max_tx ||
> > le16_to_cpu(desc->wBytesPerInterval) == 0) {
> >         dev_notice(ddev, "%s endpoint with wBytesPerInterval of %d in "
> >                 "config %d interface %d altsetting %d ep %d: "
> >                 "setting to %d\n",
> >                 usb_endpoint_xfer_isoc(&ep->desc) ? "Isoc" : "Int",
> >                 le16_to_cpu(desc->wBytesPerInterval),
> >                 cfgno, inum, asnum, ep->desc.bEndpointAddress,
> >                 max_tx);
> >         ep->ss_ep_comp.wBytesPerInterval = cpu_to_le16(max_tx);
> >     }
> > 
> >  Could you please give me some advice? Thanks.
> 
> Try it and see if it fixes the problems you see with the network 
> adapters.
> 
> I saw the Greg said not to change the descriptors and just fail the 
> device, but we already make this sort of change to correct other errors 
> so there doesn't seem to be any reason not to do it here as well.  
> Especially if it allows people to use devices that otherwise would not 
> work.

I didn't realize this was on "real" devices, sorry.  I thought this was
only a fuzzing thing.  So yes, fix up the broken descriptor after
warning about it is the correct thing to do.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v1 1/1] usb: typec: tipd: Restore generic TPS6598x contract interrupts
From: Heikki Krogerus @ 2026-04-02 14:06 UTC (permalink / raw)
  To: Vincent Cloutier
  Cc: gregkh, linux-usb, linux-kernel, sven, Vincent Cloutier, stable
In-Reply-To: <20260402000950.715470-1-vincent.cloutier@icloud.com>

Wed, Apr 01, 2026 at 08:09:50PM -0400, Vincent Cloutier kirjoitti:
> From: Vincent Cloutier <vincent@cloutier.co>
> 
> The generic TPS6598x interrupt handler still relies on
> PP_SWITCH_CHANGED, NEW_CONTRACT_AS_CONSUMER, HARD_RESET, and
> STATUS_UPDATE, but the irq_mask1 refactor only kept
> POWER_STATUS_UPDATE, DATA_STATUS_UPDATE, and PLUG_EVENT in
> tps6598x_data.
> 
> On the librem5 that leaves PD partners stuck at the 500 mA fallback
> because the active contract is never refreshed after attach.
> 
> Restore the missing interrupt bits so the existing handler paths are
> reachable again. This fixes USB-C charging negotiation on the librem5:
> after a replug the TPS6598x source power supply reports 3 A instead of
> 500 mA and the BQ25890 input limit follows suit.
> 
> Fixes: b3dddff502c5 ("usb: typec: tipd: Move initial irq mask to tipd_data")
> Cc: stable@vger.kernel.org
> Signed-off-by: Vincent Cloutier <vincent@cloutier.co>

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/tipd/core.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 84ee5687bb27..83f2fec6e34e 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -2395,7 +2395,11 @@ static const struct tipd_data tps6598x_data = {
>  	.irq_handler = tps6598x_interrupt,
>  	.irq_mask1 = TPS_REG_INT_POWER_STATUS_UPDATE |
>  		     TPS_REG_INT_DATA_STATUS_UPDATE |
> -		     TPS_REG_INT_PLUG_EVENT,
> +		     TPS_REG_INT_PLUG_EVENT |
> +		     TPS_REG_INT_PP_SWITCH_CHANGED |
> +		     TPS_REG_INT_NEW_CONTRACT_AS_CONSUMER |
> +		     TPS_REG_INT_HARD_RESET |
> +		     TPS_REG_INT_STATUS_UPDATE,
>  	.tps_struct_size = sizeof(struct tps6598x),
>  	.register_port = tps6598x_register_port,
>  	.unregister_port = tps6598x_unregister_port,

-- 
heikki

^ permalink raw reply

* Re: [PATCH] usb: core: Fix bandwidth for devices with invalid wBytesPerInterval
From: Alan Stern @ 2026-04-02 13:56 UTC (permalink / raw)
  To: Xuetao (kirin); +Cc: Greg KH, linux-usb, linux-kernel, caiyadong, stable
In-Reply-To: <c463f9ed-22ed-4ee6-b4fa-2933770e9c4c@huawei.com>

On Thu, Apr 02, 2026 at 02:59:35PM +0800, Xuetao (kirin) wrote:
> 2、Following Alan's suggestion in another email, should I check whether
> wBytesPerInterval is a valid value and handle it in the
> usb_parse_ss_endpoint_companion() ?

Yes, IMO.

> However, when parsing the device descriptor, we do not know whether the
> actual data length transmitted by the peripheral is greater than
> wBytesPerInterval.

Note: wBytesPerInterval is in the endpoint descriptor, not the device 
descriptor.

> Therefore, would it be sufficient to only add a check for whether
> wBytesPerInterval is 0 in the existing flow, and if it is 0, set
> wBytesPerInterval to cpu_to_le16(max_tx) by default?
> 
> For example, modify it in the following way:
> 
>      if (le16_to_cpu(desc->wBytesPerInterval) > max_tx ||
> le16_to_cpu(desc->wBytesPerInterval) == 0) {
>         dev_notice(ddev, "%s endpoint with wBytesPerInterval of %d in "
>                 "config %d interface %d altsetting %d ep %d: "
>                 "setting to %d\n",
>                 usb_endpoint_xfer_isoc(&ep->desc) ? "Isoc" : "Int",
>                 le16_to_cpu(desc->wBytesPerInterval),
>                 cfgno, inum, asnum, ep->desc.bEndpointAddress,
>                 max_tx);
>         ep->ss_ep_comp.wBytesPerInterval = cpu_to_le16(max_tx);
>     }
> 
>  Could you please give me some advice? Thanks.

Try it and see if it fixes the problems you see with the network 
adapters.

I saw the Greg said not to change the descriptors and just fail the 
device, but we already make this sort of change to correct other errors 
so there doesn't seem to be any reason not to do it here as well.  
Especially if it allows people to use devices that otherwise would not 
work.

Alan Stern

^ permalink raw reply

* Re: [PATCH] usb: typec: altmode: Fix altmode to handle multiple parners
From: Greg KH @ 2026-04-02 13:35 UTC (permalink / raw)
  To: François Scala; +Cc: linux-usb, Heikki Krogerus
In-Reply-To: <4502b982-277f-4b96-85f6-55e6d5f4417a@scala.name>

On Thu, Apr 02, 2026 at 02:27:13PM +0200, François Scala wrote:
> 
> On 02/04/2026 14.18, Greg KH wrote:
> > And what commit id does this fix?
> 
> The altmode struct was added in commit
> 8a37d87d72f0c69f837229c04d2fcd7117ea57e7

Great, when you resend this, and properly add all of the needed
maintainers that scripts/get_maintainer.pl tells you to add, can you add
the correct "Fixes:" tag line as well?

thanks,

greg k-h

^ permalink raw reply

* RE: [PATCH net-next] r8152: Add helper functions for SRAM2
From: Hayes Wang @ 2026-04-02 13:30 UTC (permalink / raw)
  To: Chih Kai Hsu, kuba@kernel.org, davem@davemloft.net
  Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org, edumazet@google.com, bjorn@mork.no,
	pabeni@redhat.com, Chih Kai Hsu
In-Reply-To: <20260401115542.34601-1-nic_swsd@realtek.com>

> From: Chih Kai Hsu <hsu.chih.kai@realtek.com>
> Sent: Wednesday, April 1, 2026 7:56 PM
> To: kuba@kernel.org; davem@davemloft.net
> Cc: netdev@vger.kernel.org; nic_swsd <nic_swsd@realtek.com>;
> linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org;
> edumazet@google.com; bjorn@mork.no; pabeni@redhat.com; Chih Kai Hsu
> <hsu.chih.kai@realtek.com>
> Subject: [PATCH net-next] r8152: Add helper functions for SRAM2
> 
> Add the following helper functions for SRAM2 access to simplify the code and
> improve readability:
> 
> - sram2_write() - write data to SRAM2 address
> - sram2_read() - read data from SRAM2 address
> - sram2_write_w0w1() - read-modify-write operation
> 
> Signed-off-by: Chih Kai Hsu <hsu.chih.kai@realtek.com>

Reviewed-by: Hayes Wang <hayeswang@realtek.com>

Best Regards,
Hayes


^ permalink raw reply

* Re: [PATCH] usb: typec: altmode: Fix altmode to handle multiple parners
From: Heikki Krogerus @ 2026-04-02 13:26 UTC (permalink / raw)
  To: François Scala; +Cc: linux-usb
In-Reply-To: <20260402120433.22967-1-francois@scala.name>

Hi,

On Thu, Apr 02, 2026 at 02:04:32PM +0200, François Scala wrote:
> From: François Scala <francois@scala.name>
> 
> The partner field in the altmode struct is a single pointer and the sysfs
> symlink uses a fixed name. This cause an error when a second partner is
> registered.
> 
>   sysfs: cannot create duplicate filename '/devices/platform/USBC000:00/typec/port0/port0.0/partner'

I think what you seeing here is either a bug in our ucsi driver, or
(more likely) a problem in the firmware.

> The field is replaced by an array of pointers and the symlink use the
> device name to avoid conflict.
> 
>   /sys/devices/platform/USBC000:00/typec/port0/port0.0/
>   lrwxrwxrwx 1 root root    0 Apr  2 09:12 port0-partner.0 -> ../port0-partner/port0-partner.0
>   lrwxrwxrwx 1 root root    0 Apr  2 09:12 port0-partner.1 -> ../port0-partner/port0-partner.1

No. You can not have more than a single partner per mode. Let's figure
out the root issue. Please check the svids of the partner altmodes:

        grep . /sys/class/typec/port0-partner/port0-partner.*/svid

Then provide the trace output from the ucsi driver, dmesg output
(full), and also acpidump of your system. The ucsi trace you can
get like this (assuming you have the debugfs mounted at
/sys/kernel/debug):

        cd /sys/kernel/debug/tracing
        echo 1 > events/ucsi/enable
        # plug-in your device
        # wait for a few seconds
        cat trace

For acpidump you need the acpica-tools installed:

        acpidump -o my_acpi.dump

thanks,

-- 
heikki

^ permalink raw reply

* Re: [PATCH] usb: chipidea: udc: reject non-control requests while controller is suspended
From: Andreea.Popescu @ 2026-04-02 13:21 UTC (permalink / raw)
  To: Xu Yang
  Cc: Peter Chen, Greg Kroah-Hartman, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <fbrg36w35eu7txbsshh32descrn6bzfypjqs6wwb76ijjbay52@opctausuvgi6>

On Wed, Apr 01, 2026 at 10:47:54AM +0000, Andreea.Popescu@aumovio.com wrote:
>> On Tue, Mar 31, 2026 at 12:21:45PM +0000, Andreea.Popescu@aumovio.com wrote:
>> >> When Linux runtime PM autosuspends a ChipIdea UDC that is still
>> >> enumerated by the host, the driver gates the PHY clocks and marks
>> >> the controller as suspended (ci->in_lpm = 1) but deliberately leaves
>> >> gadget.speed unchanged so upper-layer gadget drivers do not see a
>> >> spurious disconnect.
>> >
>> >It's strange that chipidea UDC will runtime suspend even it's already
>> >enumerated by the host. AFAIK, the udc driver will call pm_runtime_get_sync()
>> >in ci_hdrc_gadget_connect(is_active = true), so it will be in runtime active
>> >state all the time unless a explicit pm_runtime_put/_autosuspend() is called
>> >in somewhere.
>> >
>> >Would you share more details how device controller go to runtime suspended?
>> >Thanks,
>> >Xu Yang
>> Thank you very much for taking the time and pointing this. It made me realize a very important distinction. I am using an I.MX board, due to this I will split my answer, so you can decide if it's >still worth what I am proposing or you can just reject it. Either way, I am most grateful.
>
>Thank you for the information.
>
>> Still applicable to 6.19 mainline:
>> ep_queue returning 0 for USB_SPEED_UNKNOWN: I believe there might be the following window: _gadget_stop_activity() sets gadget.speed = USB_SPEED_UNKNOWN, but ep_queue is called before that >completes from a concurrent context. The return 0 is misleading and should be -ESHUTDOWN.
>
>I think this will hardly happen or the window is very small. Because when
>the driver sees the speed is USB_SPEED_UNKNOWN, it has most likely already
>seen that ep->enabled is false. _gadget_stop_activity() will call usb_ep_disable(),
>so usb_ep_queue() will return -ESHUTDOWN early.
>
>> I.MX specific: On i.MX SoCs the chipidea controller sits inside a power domain managed by imx-blk-ctrl or the GPC. When that parent domain is shut down by the platform PM framework, >pm_runtime_force_suspend() is called on the chipidea device, bypassing usage_count entirely and invoking ci_runtime_suspend → ci_controller_suspend → ci->in_lpm = true. This happens while VBUS is >still present and the gadget is enumerated. This is the actual path I observed and it is platform-specific, not a general chipidea mainline issue. Due to this, please disregard the proposed change >with _ep_queue guard on ci->in_lpm
>
>Then I guess the PM framework is working abnormally. How could a parent
>domain shut down itself when its active subdomain and users are using it?
Thank you very much for all the reviews! I would like to withdraw the patch proposal.
I will describe the issue that I have and why the proposed solution seems to work on my scenario, however I fully agree I might be looking at the wrong place,taking the wrong approach.
Topology: PC (windows) (host) <-usb 1-0-> (gadget) I.MX board (linux) (host) <-usb 1-1-> Board2 (linux) (I am really sorry for the generic description, but I can not allowed to disclose more details).
Board 2 generates logs which are routed to I.MX to be further stored on PC.
Without patch, logs on IMX:
hub 1-0:1.0: state 7 ports 1 chg 0000 evt 0002
ci_hdrc ci_hdrc.0: GetStatus port:1 status 10001803 8  ACK POWER sig=j CSC CONNECT
usb usb1-port1: status 0101, change 0001, 12 Mb/s
usb usb1-port1: debounce total 100ms stable 100ms status 0x101
ci_hdrc ci_hdrc.0: port 1 reset complete, port enabled
ci_hdrc ci_hdrc.0: GetStatus port:1 status 18001205 12  ACK POWER sig=se0 LPM PE CONNECT
usb 1-1: new high-speed USB device number 3 using ci_hdrc
ci_hdrc ci_hdrc.0: port 1 reset complete, port enabled
ci_hdrc ci_hdrc.0: GetStatus port:1 status 18001205 12  ACK POWER sig=se0 LPM PE CONNECT
usb 1-1: skipped 3 descriptors after interface
usb 1-1: default language 0x0409
usb 1-1: udev 3, busnum 1, minor = 2
usb 1-1: usb_probe_device
usb 1-1: configuration #1 chosen from 1 choice
usb 1-1: adding 1-1:1.0 (config #1, interface 0)
usb_bridge 1-1:1.0: usb_probe_interface
usb_bridge 1-1:1.0: usb_probe_interface - got id
diag_bridge 1-1:1.0: usb_probe_interface
diag_bridge 1-1:1.0: usb_probe_interface - got id
Logs flow, but in time they start stalling, they accumulate on IMX side until the pool is exhausted, on PC logs are received slow until they stop completly. Just around this timeframe in the middle of a diag log being received on IMX to be send to the PC I see:
(this is the imx usb connected to the pc)
ci_hdrc ci_hdrc.1: at ci_runtime_suspend
imx_usb 5b0e0000.usb: at imx_controller_suspend    
After this point there is no recovery until reboot or until I physically disconnect the usb from the pc. Problem is that board2 due to not receiving any completition has at this point the usb endpoints stuck (at least for diag interface), no new events possible,so things only get worse.

With patch:
Same logs until problem happens. When previously they would have stalled, now I start seeing
 configfs-gadget gadget: usb_diag_write: cannot queue read request 
 diag: In diag_usb_write, error writing to usb channel diag_mdm, err: -5.
 This rejection is a valid response for board2 and eventhough logs are lost, there is no stuck condition anymore on usb endpoints.
Problem is easily seen when windows starts reseting the USB and afterwards eventhough no reset happens anymore, it gets stuck in this situation.
I thank you for your time and most usefull information


________________________________________
De la: Xu Yang <xu.yang_2@nxp.com>
Trimis: joi, 2 aprilie 2026 13:16
Către: Popescu, Andreea
Cc: Peter Chen; Greg Kroah-Hartman; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
Subiect: Re: [PATCH] usb: chipidea: udc: reject non-control requests while controller is suspended

On Wed, Apr 01, 2026 at 10:47:54AM +0000, Andreea.Popescu@aumovio.com wrote:
> On Tue, Mar 31, 2026 at 12:21:45PM +0000, Andreea.Popescu@aumovio.com wrote:
> >> When Linux runtime PM autosuspends a ChipIdea UDC that is still
> >> enumerated by the host, the driver gates the PHY clocks and marks
> >> the controller as suspended (ci->in_lpm = 1) but deliberately leaves
> >> gadget.speed unchanged so upper-layer gadget drivers do not see a
> >> spurious disconnect.
> >
> >It's strange that chipidea UDC will runtime suspend even it's already
> >enumerated by the host. AFAIK, the udc driver will call pm_runtime_get_sync()
> >in ci_hdrc_gadget_connect(is_active = true), so it will be in runtime active
> >state all the time unless a explicit pm_runtime_put/_autosuspend() is called
> >in somewhere.
> >
> >Would you share more details how device controller go to runtime suspended?
> >Thanks,
> >Xu Yang
> Thank you very much for taking the time and pointing this. It made me realize a very important distinction. I am using an I.MX board, due to this I will split my answer, so you can decide if it's still worth what I am proposing or you can just reject it. Either way, I am most grateful.

Thank you for the information.

> Still applicable to 6.19 mainline:
> ep_queue returning 0 for USB_SPEED_UNKNOWN: I believe there might be the following window: _gadget_stop_activity() sets gadget.speed = USB_SPEED_UNKNOWN, but ep_queue is called before that completes from a concurrent context. The return 0 is misleading and should be -ESHUTDOWN.

I think this will hardly happen or the window is very small. Because when
the driver sees the speed is USB_SPEED_UNKNOWN, it has most likely already
seen that ep->enabled is false. _gadget_stop_activity() will call usb_ep_disable(),
so usb_ep_queue() will return -ESHUTDOWN early.

> I.MX specific: On i.MX SoCs the chipidea controller sits inside a power domain managed by imx-blk-ctrl or the GPC. When that parent domain is shut down by the platform PM framework, pm_runtime_force_suspend() is called on the chipidea device, bypassing usage_count entirely and invoking ci_runtime_suspend → ci_controller_suspend → ci->in_lpm = true. This happens while VBUS is still present and the gadget is enumerated. This is the actual path I observed and it is platform-specific, not a general chipidea mainline issue. Due to this, please disregard the proposed change with _ep_queue guard on ci->in_lpm

Then I guess the PM framework is working abnormally. How could a parent
domain shut down itself when its active subdomain and users are using it?

Thanks,
Xu Yang


^ permalink raw reply

* [PATCH 25/25] usb: xhci: Make usb_host_endpoint.hcpriv survive endpoint_disable()
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Michal Pecio, stable, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>

From: Michal Pecio <michal.pecio@gmail.com>

xHCI hardware maintains its endpoint state between add_endpoint()
and drop_endpoint() calls followed by successful check_bandwidth().
So does the driver.

Core may call endpoint_disable() during xHCI endpoint life, so don't
clear host_ep->hcpriv then, because this breaks endpoint_reset().

If a driver calls usb_set_interface(), submits URBs which make host
sequence state non-zero and calls usb_clear_halt(), the device clears
its sequence state but xhci_endpoint_reset() bails out. The next URB
malfunctions: USB2 loses one packet, USB3 gets Transaction Error or
may not complete at all on some (buggy?) HCs from ASMedia and AMD.
This is triggered by uvcvideo on bulk video devices.

The code was copied from ehci_endpoint_disable() but it isn't needed
here - hcpriv should only be NULL on emulated root hub endpoints.
It might prevent resetting and inadvertently enabling a disabled and
dropped endpoint, but core shouldn't try to reset dropped endpoints.

Document xhci requirements regarding hcpriv. They are currently met.

Fixes: 18b74067ac78 ("xhci: Fix use-after-free regression in xhci clear hub TT implementation")
Cc: stable@vger.kernel.org
Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 1 -
 include/linux/usb.h     | 3 ++-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 6d27c471d4da..a54f5b57f205 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3292,7 +3292,6 @@ static void xhci_endpoint_disable(struct usb_hcd *hcd,
 		xhci_dbg(xhci, "endpoint disable with ep_state 0x%x\n",
 			 ep->ep_state);
 done:
-	host_ep->hcpriv = NULL;
 	spin_unlock_irqrestore(&xhci->lock, flags);
 }
 
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 04277af4bb9d..27e95eade121 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -54,7 +54,8 @@ struct ep_device;
  * @eusb2_isoc_ep_comp: eUSB2 isoc companion descriptor for this endpoint
  * @urb_list: urbs queued to this endpoint; maintained by usbcore
  * @hcpriv: for use by HCD; typically holds hardware dma queue head (QH)
- *	with one or more transfer descriptors (TDs) per urb
+ *	with one or more transfer descriptors (TDs) per urb; must be preserved
+ *	by core while BW is allocated for the endpoint
  * @ep_dev: ep_device for sysfs info
  * @extra: descriptors following this endpoint in the configuration
  * @extralen: how many bytes of "extra" are valid
-- 
2.43.0


^ permalink raw reply related

* [PATCH 24/25] usb: xhci: remove duplicate '0x' prefix
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>

From: Niklas Neronin <niklas.neronin@linux.intel.com>

Prefix "0x" is automatically added by '%pad'.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c  | 6 +++---
 drivers/usb/host/xhci-ring.c | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index a9fd26559e50..997fe90f54e5 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -994,14 +994,14 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id,
 	if (!dev->out_ctx)
 		goto fail;
 
-	xhci_dbg(xhci, "Slot %d output ctx = 0x%pad (dma)\n", slot_id, &dev->out_ctx->dma);
+	xhci_dbg(xhci, "Slot %d output ctx = %pad (dma)\n", slot_id, &dev->out_ctx->dma);
 
 	/* Allocate the (input) device context for address device command */
 	dev->in_ctx = xhci_alloc_container_ctx(xhci, XHCI_CTX_TYPE_INPUT, flags);
 	if (!dev->in_ctx)
 		goto fail;
 
-	xhci_dbg(xhci, "Slot %d input ctx = 0x%pad (dma)\n", slot_id, &dev->in_ctx->dma);
+	xhci_dbg(xhci, "Slot %d input ctx = %pad (dma)\n", slot_id, &dev->in_ctx->dma);
 
 	/* Initialize the cancellation and bandwidth list for each ep */
 	for (i = 0; i < 31; i++) {
@@ -2424,7 +2424,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 
 	xhci->dcbaa->dma = dma;
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-		       "Device context base array address = 0x%pad (DMA), %p (virt)",
+		       "Device context base array address = %pad (DMA), %p (virt)",
 		       &xhci->dcbaa->dma, xhci->dcbaa);
 
 	/*
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 3589af0e2768..e47e644b296e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -755,7 +755,7 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
 	}
 
 	if ((ep->ep_state & SET_DEQ_PENDING)) {
-		xhci_warn(xhci, "Set TR Deq already pending, don't submit for 0x%pad\n",
+		xhci_warn(xhci, "Set TR Deq already pending, don't submit for %pad\n",
 			  &addr);
 		return -EBUSY;
 	}
@@ -763,7 +763,7 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
 	/* This function gets called from contexts where it cannot sleep */
 	cmd = xhci_alloc_command(xhci, false, GFP_ATOMIC);
 	if (!cmd) {
-		xhci_warn(xhci, "Can't alloc Set TR Deq cmd 0x%pad\n", &addr);
+		xhci_warn(xhci, "Can't alloc Set TR Deq cmd %pad\n", &addr);
 		return -ENOMEM;
 	}
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH 23/25] usb: xhci: move roothub port limit validation
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>

From: Niklas Neronin <niklas.neronin@linux.intel.com>

Function xhci_setup_port_arrays() limits the number of roothub ports
for both USB 2 and 3, this causes code repetition.

Solve this by moving roothub port limits validation to
xhci_create_rhub_port_array().

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c | 45 +++++++++++++++----------------------
 1 file changed, 18 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 4156822eb000..a9fd26559e50 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2165,15 +2165,28 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
 	/* FIXME: Should we disable ports not in the Extended Capabilities? */
 }
 
-static void xhci_create_rhub_port_array(struct xhci_hcd *xhci,
-					struct xhci_hub *rhub, gfp_t flags)
+static void xhci_create_rhub_port_array(struct xhci_hcd *xhci, struct xhci_hub *rhub,
+					unsigned int max_ports, gfp_t flags)
 {
 	int port_index = 0;
 	int i;
 	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
 
-	if (!rhub->num_ports)
+	if (!rhub->num_ports) {
+		xhci_info(xhci, "USB%u root hub has no ports\n", rhub->maj_rev);
 		return;
+	}
+
+	/*
+	 * Place limits on the number of roothub ports so that the hub
+	 * descriptors aren't longer than the USB core will allocate.
+	 */
+	if (rhub->num_ports > max_ports) {
+		xhci->usb3_rhub.num_ports = max_ports;
+		xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Limiting USB%u root hub ports to %u",
+			       rhub->maj_rev, max_ports);
+	}
+
 	rhub->ports = kcalloc_node(rhub->num_ports, sizeof(*rhub->ports),
 			flags, dev_to_node(dev));
 	if (!rhub->ports)
@@ -2269,30 +2282,8 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags)
 		       "Found %u USB 2.0 ports and %u USB 3.0 ports.",
 		       xhci->usb2_rhub.num_ports, xhci->usb3_rhub.num_ports);
 
-	/* Place limits on the number of roothub ports so that the hub
-	 * descriptors aren't longer than the USB core will allocate.
-	 */
-	if (xhci->usb3_rhub.num_ports > USB_SS_MAXPORTS) {
-		xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-				"Limiting USB 3.0 roothub ports to %u.",
-				USB_SS_MAXPORTS);
-		xhci->usb3_rhub.num_ports = USB_SS_MAXPORTS;
-	}
-	if (xhci->usb2_rhub.num_ports > USB_MAXCHILDREN) {
-		xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-				"Limiting USB 2.0 roothub ports to %u.",
-				USB_MAXCHILDREN);
-		xhci->usb2_rhub.num_ports = USB_MAXCHILDREN;
-	}
-
-	if (!xhci->usb2_rhub.num_ports)
-		xhci_info(xhci, "USB2 root hub has no ports\n");
-
-	if (!xhci->usb3_rhub.num_ports)
-		xhci_info(xhci, "USB3 root hub has no ports\n");
-
-	xhci_create_rhub_port_array(xhci, &xhci->usb2_rhub, flags);
-	xhci_create_rhub_port_array(xhci, &xhci->usb3_rhub, flags);
+	xhci_create_rhub_port_array(xhci, &xhci->usb2_rhub, USB_MAXCHILDREN, flags);
+	xhci_create_rhub_port_array(xhci, &xhci->usb3_rhub, USB_SS_MAXPORTS, flags);
 
 	return 0;
 }
-- 
2.43.0


^ permalink raw reply related

* [PATCH 22/25] usb: xhci: simpilfy resume root hub code
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>

From: Niklas Neronin <niklas.neronin@linux.intel.com>

Resume roothubs without checking 'retval' value, as it is always '0'.
Due to changes made in commit 79989bd4ab86 ("xhci: always resume roothubs
if xHC was reset during resume") the check is redundant.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ece3ff7916ff..6d27c471d4da 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1245,29 +1245,25 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
 
 	xhci_dbc_resume(xhci);
 
-	if (retval == 0) {
-		/*
-		 * Resume roothubs only if there are pending events.
-		 * USB 3 devices resend U3 LFPS wake after a 100ms delay if
-		 * the first wake signalling failed, give it that chance if
-		 * there are suspended USB 3 devices.
-		 */
-		if (xhci->usb3_rhub.bus_state.suspended_ports ||
-		    xhci->usb3_rhub.bus_state.bus_suspended)
-			suspended_usb3_devs = true;
+	/*
+	 * Resume roothubs only if there are pending events.
+	 * USB 3 devices resend U3 LFPS wake after a 100ms delay if
+	 * the first wake signalling failed, give it that chance if
+	 * there are suspended USB 3 devices.
+	 */
+	if (xhci->usb3_rhub.bus_state.suspended_ports || xhci->usb3_rhub.bus_state.bus_suspended)
+		suspended_usb3_devs = true;
 
+	pending_portevent = xhci_pending_portevent(xhci);
+	if (suspended_usb3_devs && !pending_portevent && is_auto_resume) {
+		msleep(120);
 		pending_portevent = xhci_pending_portevent(xhci);
+	}
 
-		if (suspended_usb3_devs && !pending_portevent && is_auto_resume) {
-			msleep(120);
-			pending_portevent = xhci_pending_portevent(xhci);
-		}
-
-		if (pending_portevent) {
-			if (xhci->shared_hcd)
-				usb_hcd_resume_root_hub(xhci->shared_hcd);
-			usb_hcd_resume_root_hub(hcd);
-		}
+	if (pending_portevent) {
+		if (xhci->shared_hcd)
+			usb_hcd_resume_root_hub(xhci->shared_hcd);
+		usb_hcd_resume_root_hub(hcd);
 	}
 
 	/*
-- 
2.43.0


^ permalink raw reply related

* [PATCH 21/25] usb: xhci: cleanup xhci_hub_report_usb3_link_state()
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>

From: Niklas Neronin <niklas.neronin@linux.intel.com>

Improve readability of xhci_hub_report_usb3_link_state().

Comments are shortened and clarified, and the code now makes it explicit
when the Port Link State (PLS) value is modified versus when other status
bits are updated.

No functional changes.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-hub.c | 56 +++++++++++++------------------------
 1 file changed, 20 insertions(+), 36 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 8e134f6b4e37..bacd0ddd0d09 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -850,53 +850,37 @@ void xhci_test_and_clear_bit(struct xhci_hcd *xhci, struct xhci_port *port,
 }
 
 /* Updates Link Status for super Speed port */
-static void xhci_hub_report_usb3_link_state(struct xhci_hcd *xhci,
-		u32 *status, u32 portsc)
+static void xhci_hub_report_usb3_link_state(struct xhci_hcd *xhci, u32 *status, u32 portsc)
 {
 	u32 pls = portsc & PORT_PLS_MASK;
 
-	/* When the CAS bit is set then warm reset
-	 * should be performed on port
+	/*
+	 * CAS indicates that a warm reset is required, it may be set in any
+	 * link state and is only present on roothubs.
 	 */
 	if (portsc & PORT_CAS) {
-		/* The CAS bit can be set while the port is
-		 * in any link state.
-		 * Only roothubs have CAS bit, so we
-		 * pretend to be in compliance mode
-		 * unless we're already in compliance
-		 * or the inactive state.
+		/*
+		 * If not already in Compliance or Inactive state,
+		 * report Compliance Mode so the hub logic triggers a warm reset.
 		 */
-		if (pls != XDEV_COMP_MODE &&
-		    pls != XDEV_INACTIVE) {
+		if (pls != XDEV_COMP_MODE && pls != XDEV_INACTIVE)
 			pls = USB_SS_PORT_LS_COMP_MOD;
-		}
-		/* Return also connection bit -
-		 * hub state machine resets port
-		 * when this bit is set.
-		 */
-		pls |= USB_PORT_STAT_CONNECTION;
-	} else {
+
+		/* Signal a connection change to force a reset */
+		*status |= USB_PORT_STAT_CONNECTION;
+	} else if (pls == XDEV_RESUME) {
 		/*
-		 * Resume state is an xHCI internal state.  Do not report it to
-		 * usb core, instead, pretend to be U3, thus usb core knows
-		 * it's not ready for transfer.
+		 * Resume is an internal xHCI-only state and must not be exposed
+		 * to usbcore. Report it as U3 so transfers are blocked.
 		 */
-		if (pls == XDEV_RESUME) {
-			*status |= USB_SS_PORT_LS_U3;
-			return;
-		}
-
+		pls = USB_SS_PORT_LS_U3;
+	} else if (pls == XDEV_COMP_MODE) {
 		/*
-		 * If CAS bit isn't set but the Port is already at
-		 * Compliance Mode, fake a connection so the USB core
-		 * notices the Compliance state and resets the port.
-		 * This resolves an issue generated by the SN65LVPE502CP
-		 * in which sometimes the port enters compliance mode
-		 * caused by a delay on the host-device negotiation.
+		 * Some hardware may enter Compliance Mode without CAS.
+		 * Fake a connection event so usbcore notices and resets the port.
 		 */
-		if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) &&
-				(pls == XDEV_COMP_MODE))
-			pls |= USB_PORT_STAT_CONNECTION;
+		if (xhci->quirks & XHCI_COMP_MODE_QUIRK)
+			*status |= USB_PORT_STAT_CONNECTION;
 	}
 
 	/* update status field */
-- 
2.43.0


^ permalink raw reply related

* [PATCH 20/25] usb: xhci: rename parameter to match argument 'portsc'
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>

From: Niklas Neronin <niklas.neronin@linux.intel.com>

A previous patch renamed the temporary variable holding the value read
from the PORTSC register from 'temp' to 'portsc'. This patch follows up
by updating the parameter names of all helper functions called from
xhci_hub_control() that receive a PORTSC value, as well as the functions
they call.

Function changed:
xhci_get_port_status()
 L xhci_get_usb3_port_status()
    L xhci_hub_report_usb3_link_state()
    L xhci_del_comp_mod_timer()
xhci_get_ext_port_status()
xhci_port_state_to_neutral()
xhci_clear_port_change_bit()
xhci_port_speed()

The reason for the rename is to differentiate between port
status/change bit to be written to PORTSC and replying to hub-class
USB requests. Each of them use their specific macros.

Use "portsc" name for PORTSC values and "status" for values intended
for replying to hub-class USB request.

A dedicated structure for USB hub port status responses
('struct usb_port_status' from ch11.h) exists and will be integrated in
a later patch.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-hub.c | 61 ++++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 9cd64d6989c9..8e134f6b4e37 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -375,11 +375,11 @@ static void xhci_hub_descriptor(struct usb_hcd *hcd, struct xhci_hcd *xhci,
 
 }
 
-static unsigned int xhci_port_speed(unsigned int port_status)
+static unsigned int xhci_port_speed(int portsc)
 {
-	if (DEV_LOWSPEED(port_status))
+	if (DEV_LOWSPEED(portsc))
 		return USB_PORT_STAT_LOW_SPEED;
-	if (DEV_HIGHSPEED(port_status))
+	if (DEV_HIGHSPEED(portsc))
 		return USB_PORT_STAT_HIGH_SPEED;
 	/*
 	 * FIXME: Yes, we should check for full speed, but the core uses that as
@@ -429,9 +429,9 @@ static unsigned int xhci_port_speed(unsigned int port_status)
 
 /**
  * xhci_port_state_to_neutral() - Clean up read portsc value back into writeable
- * @state: u32 port value read from portsc register to be cleanup up
+ * @portsc: u32 port value read from portsc register to be cleanup up
  *
- * Given a port state, this function returns a value that would result in the
+ * Given a portsc, this function returns a value that would result in the
  * port being in the same state, if the value was written to the port status
  * control register.
  * Save Read Only (RO) bits and save read/write bits where
@@ -442,10 +442,10 @@ static unsigned int xhci_port_speed(unsigned int port_status)
  * changing port state.
  */
 
-u32 xhci_port_state_to_neutral(u32 state)
+u32 xhci_port_state_to_neutral(u32 portsc)
 {
 	/* Save read-only status and port state */
-	return (state & XHCI_PORT_RO) | (state & XHCI_PORT_RWS);
+	return (portsc & XHCI_PORT_RO) | (portsc & XHCI_PORT_RWS);
 }
 EXPORT_SYMBOL_GPL(xhci_port_state_to_neutral);
 
@@ -578,7 +578,7 @@ static void xhci_disable_port(struct xhci_hcd *xhci, struct xhci_port *port)
 }
 
 static void xhci_clear_port_change_bit(struct xhci_hcd *xhci, u16 wValue, struct xhci_port *port,
-				       u32 port_status)
+				       u32 portsc)
 {
 	char *port_change_bit;
 	u32 status;
@@ -621,11 +621,11 @@ static void xhci_clear_port_change_bit(struct xhci_hcd *xhci, u16 wValue, struct
 		return;
 	}
 	/* Change bits are all write 1 to clear */
-	xhci_portsc_writel(port, port_status | status);
-	port_status = xhci_portsc_readl(port);
+	xhci_portsc_writel(port, portsc | status);
+	portsc = xhci_portsc_readl(port);
 
 	xhci_dbg(xhci, "clear port%d %s change, portsc: 0x%x\n",
-		 port->hcd_portnum + 1, port_change_bit, port_status);
+		 port->hcd_portnum + 1, port_change_bit, portsc);
 }
 
 struct xhci_hub *xhci_get_rhub(struct usb_hcd *hcd)
@@ -851,14 +851,14 @@ void xhci_test_and_clear_bit(struct xhci_hcd *xhci, struct xhci_port *port,
 
 /* Updates Link Status for super Speed port */
 static void xhci_hub_report_usb3_link_state(struct xhci_hcd *xhci,
-		u32 *status, u32 status_reg)
+		u32 *status, u32 portsc)
 {
-	u32 pls = status_reg & PORT_PLS_MASK;
+	u32 pls = portsc & PORT_PLS_MASK;
 
 	/* When the CAS bit is set then warm reset
 	 * should be performed on port
 	 */
-	if (status_reg & PORT_CAS) {
+	if (portsc & PORT_CAS) {
 		/* The CAS bit can be set while the port is
 		 * in any link state.
 		 * Only roothubs have CAS bit, so we
@@ -910,10 +910,10 @@ static void xhci_hub_report_usb3_link_state(struct xhci_hcd *xhci,
  * the compliance mode timer is deleted. A port won't enter
  * compliance mode if it has previously entered U0.
  */
-static void xhci_del_comp_mod_timer(struct xhci_hcd *xhci, u32 status, int portnum)
+static void xhci_del_comp_mod_timer(struct xhci_hcd *xhci, u32 portsc, int portnum)
 {
 	u32 all_ports_seen_u0 = ((1 << xhci->usb3_rhub.num_ports) - 1);
-	bool port_in_u0 = ((status & PORT_PLS_MASK) == XDEV_U0);
+	bool port_in_u0 = ((portsc & PORT_PLS_MASK) == XDEV_U0);
 
 	if (!(xhci->quirks & XHCI_COMP_MODE_QUIRK))
 		return;
@@ -1018,13 +1018,13 @@ static int xhci_handle_usb2_port_link_resume(struct xhci_port *port,
 	return 0;
 }
 
-static u32 xhci_get_ext_port_status(u32 raw_port_status, u32 port_li)
+static u32 xhci_get_ext_port_status(u32 portsc, u32 port_li)
 {
 	u32 ext_stat = 0;
 	int speed_id;
 
 	/* only support rx and tx lane counts of 1 in usb3.1 spec */
-	speed_id = DEV_PORT_SPEED(raw_port_status);
+	speed_id = DEV_PORT_SPEED(portsc);
 	ext_stat |= speed_id;		/* bits 3:0, RX speed id */
 	ext_stat |= speed_id << 4;	/* bits 7:4, TX speed id */
 
@@ -1150,7 +1150,7 @@ static void xhci_get_usb2_port_status(struct xhci_port *port, u32 *status,
  *  - Drop and reacquire the xHCI lock, in order to wait for port resume.
  */
 static u32 xhci_get_port_status(struct usb_hcd *hcd, struct xhci_bus_state *bus_state,
-				int portnum, u32 raw_port_status, unsigned long *flags)
+				int portnum, u32 portsc, unsigned long *flags)
 	__releases(&xhci->lock)
 	__acquires(&xhci->lock)
 {
@@ -1162,33 +1162,32 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd, struct xhci_bus_state *bus_
 	port = rhub->ports[portnum];
 
 	/* common wPortChange bits */
-	if (raw_port_status & PORT_CSC)
+	if (portsc & PORT_CSC)
 		status |= USB_PORT_STAT_C_CONNECTION << 16;
-	if (raw_port_status & PORT_PEC)
+	if (portsc & PORT_PEC)
 		status |= USB_PORT_STAT_C_ENABLE << 16;
-	if ((raw_port_status & PORT_OCC))
+	if (portsc & PORT_OCC)
 		status |= USB_PORT_STAT_C_OVERCURRENT << 16;
-	if ((raw_port_status & PORT_RC))
+	if (portsc & PORT_RC)
 		status |= USB_PORT_STAT_C_RESET << 16;
 
 	/* common wPortStatus bits */
-	if (raw_port_status & PORT_CONNECT) {
+	if (portsc & PORT_CONNECT) {
 		status |= USB_PORT_STAT_CONNECTION;
-		status |= xhci_port_speed(raw_port_status);
+		status |= xhci_port_speed(portsc);
 	}
-	if (raw_port_status & PORT_PE)
+	if (portsc & PORT_PE)
 		status |= USB_PORT_STAT_ENABLE;
-	if (raw_port_status & PORT_OC)
+	if (portsc & PORT_OC)
 		status |= USB_PORT_STAT_OVERCURRENT;
-	if (raw_port_status & PORT_RESET)
+	if (portsc & PORT_RESET)
 		status |= USB_PORT_STAT_RESET;
 
 	/* USB2 and USB3 specific bits, including Port Link State */
 	if (hcd->speed >= HCD_USB3)
-		xhci_get_usb3_port_status(port, &status, raw_port_status);
+		xhci_get_usb3_port_status(port, &status, portsc);
 	else
-		xhci_get_usb2_port_status(port, &status, raw_port_status,
-					  flags);
+		xhci_get_usb2_port_status(port, &status, portsc, flags);
 
 	if (bus_state->port_c_suspend & (1 << portnum))
 		status |= USB_PORT_STAT_C_SUSPEND << 16;
-- 
2.43.0


^ permalink raw reply related

* [PATCH 19/25] usb: xhci: add PORTSC variable to xhci_hub_control()
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>

From: Niklas Neronin <niklas.neronin@linux.intel.com>

The variable 'temp' is used multiple times throughout xhci_hub_control()
for holding only PORTSC register values.

As a follow-up to introducing a dedicated variable for PORTPMSC, rename
all remaining 'temp' to 'portsc'. This improves readability and clarifies
what is being modified.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-hub.c | 102 ++++++++++++++++++------------------
 1 file changed, 51 insertions(+), 51 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 4da3b48dfce0..9cd64d6989c9 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1202,7 +1202,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	int max_ports;
 	unsigned long flags;
-	u32 temp, portpmsc, status;
+	u32 portsc, portpmsc, status;
 	int retval = 0;
 	struct xhci_bus_state *bus_state;
 	u16 link_state;
@@ -1258,19 +1258,19 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			goto error;
 
 		port = ports[portnum];
-		temp = xhci_portsc_readl(port);
-		if (temp == ~(u32)0) {
+		portsc = xhci_portsc_readl(port);
+		if (portsc == ~(u32)0) {
 			xhci_hc_died(xhci);
 			retval = -ENODEV;
 			break;
 		}
-		trace_xhci_get_port_status(port, temp);
-		status = xhci_get_port_status(hcd, bus_state, portnum, temp, &flags);
+		trace_xhci_get_port_status(port, portsc);
+		status = xhci_get_port_status(hcd, bus_state, portnum, portsc, &flags);
 		if (status == 0xffffffff)
 			goto error;
 
 		xhci_dbg(xhci, "Get port status %d-%d read: 0x%x, return 0x%x",
-			 hcd->self.busnum, portnum + 1, temp, status);
+			 hcd->self.busnum, portnum + 1, portsc, status);
 
 		put_unaligned(cpu_to_le32(status), (__le32 *) buf);
 		/* if USB 3.1 extended port status return additional 4 bytes */
@@ -1283,7 +1283,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 				break;
 			}
 			port_li = readl(&port->port_reg->portli);
-			status = xhci_get_ext_port_status(temp, port_li);
+			status = xhci_get_ext_port_status(portsc, port_li);
 			put_unaligned_le32(status, &buf[4]);
 		}
 		break;
@@ -1293,18 +1293,18 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			goto error;
 
 		port = ports[portnum];
-		temp = xhci_portsc_readl(port);
-		if (temp == ~(u32)0) {
+		portsc = xhci_portsc_readl(port);
+		if (portsc == ~(u32)0) {
 			xhci_hc_died(xhci);
 			retval = -ENODEV;
 			break;
 		}
-		temp = xhci_port_state_to_neutral(temp);
+		portsc = xhci_port_state_to_neutral(portsc);
 		/* FIXME: What new port features do we need to support? */
 		switch (wValue) {
 		case USB_PORT_FEAT_SUSPEND:
-			temp = xhci_portsc_readl(port);
-			if ((temp & PORT_PLS_MASK) != XDEV_U0) {
+			portsc = xhci_portsc_readl(port);
+			if ((portsc & PORT_PLS_MASK) != XDEV_U0) {
 				/* Resume the port to U0 first */
 				xhci_set_link_state(xhci, port, XDEV_U0);
 				spin_unlock_irqrestore(&xhci->lock, flags);
@@ -1315,9 +1315,9 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			 * a port unless the port reports that it is in the
 			 * enabled (PED = ‘1’,PLS < ‘3’) state.
 			 */
-			temp = xhci_portsc_readl(port);
-			if ((temp & PORT_PE) == 0 || (temp & PORT_RESET)
-				|| (temp & PORT_PLS_MASK) >= XDEV_U3) {
+			portsc = xhci_portsc_readl(port);
+			if ((portsc & PORT_PE) == 0 || (portsc & PORT_RESET) ||
+			    (portsc & PORT_PLS_MASK) >= XDEV_U3) {
 				xhci_warn(xhci, "USB core suspending port %d-%d not in U0/U1/U2\n",
 					  hcd->self.busnum, portnum + 1);
 				goto error;
@@ -1338,26 +1338,26 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			msleep(10); /* wait device to enter */
 			spin_lock_irqsave(&xhci->lock, flags);
 
-			temp = xhci_portsc_readl(port);
+			portsc = xhci_portsc_readl(port);
 			bus_state->suspended_ports |= 1 << portnum;
 			break;
 		case USB_PORT_FEAT_LINK_STATE:
 			link_state = (wIndex & 0xff00) >> 3;
-			temp = xhci_portsc_readl(port);
+			portsc = xhci_portsc_readl(port);
 			/* Disable port */
 			if (link_state == USB_SS_PORT_LS_SS_DISABLED) {
 				xhci_dbg(xhci, "Disable port %d-%d\n",
 					 hcd->self.busnum, portnum + 1);
-				temp = xhci_port_state_to_neutral(temp);
+				portsc = xhci_port_state_to_neutral(portsc);
 				/*
 				 * Clear all change bits, so that we get a new
 				 * connection event.
 				 */
-				temp |= PORT_CSC | PORT_PEC | PORT_WRC |
-					PORT_OCC | PORT_RC | PORT_PLC |
-					PORT_CEC;
-				xhci_portsc_writel(port, temp | PORT_PE);
-				temp = xhci_portsc_readl(port);
+				portsc |= PORT_CSC | PORT_PEC | PORT_WRC |
+					  PORT_OCC | PORT_RC | PORT_PLC |
+					  PORT_CEC;
+				xhci_portsc_writel(port, portsc | PORT_PE);
+				portsc = xhci_portsc_readl(port);
 				break;
 			}
 
@@ -1366,7 +1366,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 				xhci_dbg(xhci, "Enable port %d-%d\n",
 					 hcd->self.busnum, portnum + 1);
 				xhci_set_link_state(xhci, port,	XDEV_RXDETECT);
-				temp = xhci_portsc_readl(port);
+				portsc = xhci_portsc_readl(port);
 				break;
 			}
 
@@ -1390,7 +1390,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 					break;
 				}
 
-				if ((temp & PORT_CONNECT)) {
+				if ((portsc & PORT_CONNECT)) {
 					xhci_warn(xhci, "Can't set compliance mode when port is connected\n");
 					goto error;
 				}
@@ -1399,11 +1399,11 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 					 hcd->self.busnum, portnum + 1);
 				xhci_set_link_state(xhci, port, XDEV_COMP_MODE);
 
-				temp = xhci_portsc_readl(port);
+				portsc = xhci_portsc_readl(port);
 				break;
 			}
 			/* Port must be enabled */
-			if (!(temp & PORT_PE)) {
+			if (!(portsc & PORT_PE)) {
 				retval = -ENODEV;
 				break;
 			}
@@ -1422,7 +1422,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			 * completion
 			 */
 			if (link_state == USB_SS_PORT_LS_U0) {
-				u32 pls = temp & PORT_PLS_MASK;
+				u32 pls = portsc & PORT_PLS_MASK;
 				bool wait_u0 = false;
 
 				/* already in U0 */
@@ -1447,7 +1447,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 					xhci_dbg(xhci, "missing U0 port change event for port %d-%d\n",
 						 hcd->self.busnum, portnum + 1);
 				spin_lock_irqsave(&xhci->lock, flags);
-				temp = xhci_portsc_readl(port);
+				portsc = xhci_portsc_readl(port);
 				break;
 			}
 
@@ -1465,12 +1465,12 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 				spin_unlock_irqrestore(&xhci->lock, flags);
 				while (retries--) {
 					usleep_range(4000, 8000);
-					temp = xhci_portsc_readl(port);
-					if ((temp & PORT_PLS_MASK) == XDEV_U3)
+					portsc = xhci_portsc_readl(port);
+					if ((portsc & PORT_PLS_MASK) == XDEV_U3)
 						break;
 				}
 				spin_lock_irqsave(&xhci->lock, flags);
-				temp = xhci_portsc_readl(port);
+				portsc = xhci_portsc_readl(port);
 				bus_state->suspended_ports |= 1 << portnum;
 			}
 			break;
@@ -1484,24 +1484,24 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			xhci_set_port_power(xhci, port, true, &flags);
 			break;
 		case USB_PORT_FEAT_RESET:
-			temp = (temp | PORT_RESET);
-			xhci_portsc_writel(port, temp);
+			portsc |= PORT_RESET;
+			xhci_portsc_writel(port, portsc);
 
-			temp = xhci_portsc_readl(port);
+			portsc = xhci_portsc_readl(port);
 			xhci_dbg(xhci, "set port reset, actual port %d-%d status  = 0x%x\n",
-				 hcd->self.busnum, portnum + 1, temp);
+				 hcd->self.busnum, portnum + 1, portsc);
 			break;
 		case USB_PORT_FEAT_REMOTE_WAKE_MASK:
 			wake_mask = wIndex & 0xff00;
 			xhci_set_remote_wake_mask(xhci, port, wake_mask);
-			temp = xhci_portsc_readl(port);
+			portsc = xhci_portsc_readl(port);
 			xhci_dbg(xhci, "set port remote wake mask, actual port %d-%d status  = 0x%x\n",
-				 hcd->self.busnum, portnum + 1, temp);
+				 hcd->self.busnum, portnum + 1, portsc);
 			break;
 		case USB_PORT_FEAT_BH_PORT_RESET:
-			temp |= PORT_WR;
-			xhci_portsc_writel(port, temp);
-			temp = xhci_portsc_readl(port);
+			portsc |= PORT_WR;
+			xhci_portsc_writel(port, portsc);
+			portsc = xhci_portsc_readl(port);
 			break;
 		case USB_PORT_FEAT_U1_TIMEOUT:
 			if (hcd->speed < HCD_USB3)
@@ -1538,7 +1538,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			goto error;
 		}
 		/* unblock any posted writes */
-		temp = xhci_portsc_readl(port);
+		portsc = xhci_portsc_readl(port);
 		break;
 	case ClearPortFeature:
 		portnum = (wIndex & 0xff) - 1;
@@ -1546,23 +1546,23 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			goto error;
 
 		port = ports[portnum];
-		temp = xhci_portsc_readl(port);
-		if (temp == ~(u32)0) {
+		portsc = xhci_portsc_readl(port);
+		if (portsc == ~(u32)0) {
 			xhci_hc_died(xhci);
 			retval = -ENODEV;
 			break;
 		}
 		/* FIXME: What new port features do we need to support? */
-		temp = xhci_port_state_to_neutral(temp);
+		portsc = xhci_port_state_to_neutral(portsc);
 		switch (wValue) {
 		case USB_PORT_FEAT_SUSPEND:
-			temp = xhci_portsc_readl(port);
+			portsc = xhci_portsc_readl(port);
 			xhci_dbg(xhci, "clear USB_PORT_FEAT_SUSPEND\n");
-			xhci_dbg(xhci, "PORTSC %04x\n", temp);
-			if (temp & PORT_RESET)
+			xhci_dbg(xhci, "PORTSC %04x\n", portsc);
+			if (portsc & PORT_RESET)
 				goto error;
-			if ((temp & PORT_PLS_MASK) == XDEV_U3) {
-				if ((temp & PORT_PE) == 0)
+			if ((portsc & PORT_PLS_MASK) == XDEV_U3) {
+				if ((portsc & PORT_PE) == 0)
 					goto error;
 
 				set_bit(portnum, &bus_state->resuming_ports);
@@ -1593,7 +1593,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 		case USB_PORT_FEAT_C_ENABLE:
 		case USB_PORT_FEAT_C_PORT_LINK_STATE:
 		case USB_PORT_FEAT_C_PORT_CONFIG_ERROR:
-			xhci_clear_port_change_bit(xhci, wValue, port, temp);
+			xhci_clear_port_change_bit(xhci, wValue, port, portsc);
 			break;
 		case USB_PORT_FEAT_ENABLE:
 			xhci_disable_port(xhci, port);
-- 
2.43.0


^ permalink raw reply related

* [PATCH 18/25] usb: xhci: add PORTPMSC variable to xhci_hub_control()
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>

From: Niklas Neronin <niklas.neronin@linux.intel.com>

The code handling U1/U2 timeout updates reads and modifies the PORTPMSC
register using the generic 'temp' variable, which is also used for
PORTSC. This makes the code hard to read and increases the risk of mixing
up register contents.

Introduce a dedicated 'portpmsc' variable for PORTPMSC accesses and use
it in both U1 and U2 timeout handlers. This makes the intent clearer and
keeps register operations logically separated.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-hub.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 7fb17799cfdc..4da3b48dfce0 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1202,7 +1202,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	int max_ports;
 	unsigned long flags;
-	u32 temp, status;
+	u32 temp, portpmsc, status;
 	int retval = 0;
 	struct xhci_bus_state *bus_state;
 	u16 link_state;
@@ -1508,20 +1508,20 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 				goto error;
 
 			timeout = (wIndex & 0xff00) >> 8;
-			temp = readl(&port->port_reg->portpmsc);
-			temp &= ~PORT_U1_TIMEOUT_MASK;
-			temp |= PORT_U1_TIMEOUT(timeout);
-			writel(temp, &port->port_reg->portpmsc);
+			portpmsc = readl(&port->port_reg->portpmsc);
+			portpmsc &= ~PORT_U1_TIMEOUT_MASK;
+			portpmsc |= PORT_U1_TIMEOUT(timeout);
+			writel(portpmsc, &port->port_reg->portpmsc);
 			break;
 		case USB_PORT_FEAT_U2_TIMEOUT:
 			if (hcd->speed < HCD_USB3)
 				goto error;
 
 			timeout = (wIndex & 0xff00) >> 8;
-			temp = readl(&port->port_reg->portpmsc);
-			temp &= ~PORT_U2_TIMEOUT_MASK;
-			temp |= PORT_U2_TIMEOUT(timeout);
-			writel(temp, &port->port_reg->portpmsc);
+			portpmsc = readl(&port->port_reg->portpmsc);
+			portpmsc &= ~PORT_U2_TIMEOUT_MASK;
+			portpmsc |= PORT_U2_TIMEOUT(timeout);
+			writel(portpmsc, &port->port_reg->portpmsc);
 			break;
 		case USB_PORT_FEAT_TEST:
 			/* 4.19.6 Port Test Modes (USB2 Test Mode) */
-- 
2.43.0


^ permalink raw reply related

* [PATCH 17/25] usb: xhci: separate use of USB Chapter 11 PLS macros from xHCI-specific PLS macros
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>

From: Niklas Neronin <niklas.neronin@linux.intel.com>

The xhci driver uses two different sources for Port Link State (PLS):
  1. The PLS field in the PORTSC register (bits 8:5).
  2. The PLS value encoded in bits 15:8 of the USB request wIndex,
     received by xhci_hub_control().

While both represent similar link states, they differ in a few details,
for example, xHCI's Resume State. Because of these differences, the xhci
driver defines its own set of PLS macros in xhci-port.h, which are intended
to be used when reading and writing  PORTSC. The generic USB Chapter 11
macros in ch11.h should only be used when parsing or replying to hub-class
USB requests.

To avoid mixing these two representations and prevent incorrect state
reporting, replace all uses of Chapter 11 PLS macros with the xHCI
versions when interacting with the PORTSC register.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-hub.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index b57fe0967e10..7fb17799cfdc 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -866,8 +866,8 @@ static void xhci_hub_report_usb3_link_state(struct xhci_hcd *xhci,
 		 * unless we're already in compliance
 		 * or the inactive state.
 		 */
-		if (pls != USB_SS_PORT_LS_COMP_MOD &&
-		    pls != USB_SS_PORT_LS_SS_INACTIVE) {
+		if (pls != XDEV_COMP_MODE &&
+		    pls != XDEV_INACTIVE) {
 			pls = USB_SS_PORT_LS_COMP_MOD;
 		}
 		/* Return also connection bit -
@@ -895,7 +895,7 @@ static void xhci_hub_report_usb3_link_state(struct xhci_hcd *xhci,
 		 * caused by a delay on the host-device negotiation.
 		 */
 		if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) &&
-				(pls == USB_SS_PORT_LS_COMP_MOD))
+				(pls == XDEV_COMP_MODE))
 			pls |= USB_PORT_STAT_CONNECTION;
 	}
 
@@ -1365,7 +1365,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			if (link_state == USB_SS_PORT_LS_RX_DETECT) {
 				xhci_dbg(xhci, "Enable port %d-%d\n",
 					 hcd->self.busnum, portnum + 1);
-				xhci_set_link_state(xhci, port,	link_state);
+				xhci_set_link_state(xhci, port,	XDEV_RXDETECT);
 				temp = xhci_portsc_readl(port);
 				break;
 			}
@@ -1397,7 +1397,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 
 				xhci_dbg(xhci, "Enable compliance mode transition for port %d-%d\n",
 					 hcd->self.busnum, portnum + 1);
-				xhci_set_link_state(xhci, port, link_state);
+				xhci_set_link_state(xhci, port, XDEV_COMP_MODE);
 
 				temp = xhci_portsc_readl(port);
 				break;
@@ -1435,7 +1435,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 					reinit_completion(&port->u3exit_done);
 				}
 				if (pls <= XDEV_U3) /* U1, U2, U3 */
-					xhci_set_link_state(xhci, port, USB_SS_PORT_LS_U0);
+					xhci_set_link_state(xhci, port, XDEV_U0);
 				if (!wait_u0) {
 					if (pls > XDEV_U3)
 						goto error;
@@ -1461,7 +1461,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 					xhci_stop_device(xhci, port->slot_id, 1);
 					spin_lock_irqsave(&xhci->lock, flags);
 				}
-				xhci_set_link_state(xhci, port, USB_SS_PORT_LS_U3);
+				xhci_set_link_state(xhci, port, XDEV_U3);
 				spin_unlock_irqrestore(&xhci->lock, flags);
 				while (retries--) {
 					usleep_range(4000, 8000);
-- 
2.43.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox