Linux USB
 help / color / mirror / Atom feed
* Re: [PATCH] usb: cdns3: attempt to fix Kconfig dependencies
From: Peter Chen (CIX) @ 2026-04-03  7:50 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman
  Cc: Pawel Laszczak, Arnd Bergmann, Roger Quadros, linux-usb,
	linux-kernel
In-Reply-To: <20260402141008.2691819-1-arnd@kernel.org>

On 26-04-02 16:09:55, Arnd Bergmann wrote:
> 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")

Hi Arnd,

Thanks for fixing it, I am sorry for taking your effort debug it.

> 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.

It depends on Greg, I am okay for both ways. If Greg reverts the patch,
I will do below improvements and adapts for most of your changes for v3
patch.

cdns-usb-common.ko is a libary, and no USB/GADGET dependency, could builds in.
  ├── core.o  
  └── drd.o

cdns3-host.ko     -> depends on USB/XHCI(it is m when USB = m
cdns3.ko (gadget) -> depends on USB_GADGET
cdnsp.ko (gadget) -> depends on USB_GADGET
cdns3-plat.ko     -> assign host_init/gadget_init function pointer

In that way, below combination could work:
cdns-usb-common=y
cdns3-host=m
cdns3(p)-gadget=y

> @@ -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

%s/CONFIG_USB_CDNS_PLATFORM/USB_CDNS_PLATFORM

Peter

> +	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
> 

-- 

Best regards,
Peter

^ permalink raw reply

* Re: [PATCH] usb: cdns3: attempt to fix Kconfig dependencies
From: Arnd Bergmann @ 2026-04-03  8:39 UTC (permalink / raw)
  To: Peter Chen, Arnd Bergmann, Greg Kroah-Hartman
  Cc: Pawel Laszczak, Roger Quadros, linux-usb, linux-kernel
In-Reply-To: <ac9xVUVB/BKfBUmE@nchen-desktop>

On Fri, Apr 3, 2026, at 09:50, Peter Chen (CIX) wrote:
> On 26-04-02 16:09:55, Arnd Bergmann wrote:
>> ---
>> 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.
>
> It depends on Greg, I am okay for both ways. If Greg reverts the patch,
> I will do below improvements and adapts for most of your changes for v3
> patch.
>
> cdns-usb-common.ko is a libary, and no USB/GADGET dependency, could builds in.
>   ├── core.o  
>   └── drd.o
>
> cdns3-host.ko     -> depends on USB/XHCI(it is m when USB = m
> cdns3.ko (gadget) -> depends on USB_GADGET
> cdnsp.ko (gadget) -> depends on USB_GADGET
> cdns3-plat.ko     -> assign host_init/gadget_init function pointer

This all sounds good. Ideally cdns3-plat.ko would also lose the
dependency on cdns3.ko/cdnsp.ko

>> @@ -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
>
> %s/CONFIG_USB_CDNS_PLATFORM/USB_CDNS_PLATFORM

Oh, this means my patch did not actually work at all, and my testing
was pointless. I've fixed that now in my randconfig tree to see
if anything else comes up.

     Arnd

^ permalink raw reply

* Re: [RFC PATCH 1/2] xhci: prevent automatic endpoint restart after stall or error
From: Michal Pecio @ 2026-04-03  8:51 UTC (permalink / raw)
  To: stern@rowland.harvard.edu
  Cc: Thinh Nguyen, Mathias Nyman, linux-usb@vger.kernel.org,
	oneukum@suse.com, niklas.neronin@linux.intel.com
In-Reply-To: <28a00143-85fa-4043-93a0-c2b687ff1bcd@rowland.harvard.edu>

On Thu, 2 Apr 2026 22:42:39 -0400, stern@rowland.harvard.edu wrote:
> Every status other than 0 should mean that the endpoint's queue is 
> halted.  But not all statuses require a clear-halt or reset-endpoint
> to recover.  For instance, a short transfer when the URB_SHORT_NOT_OK
> flag is set.

This doesn't work in xhci-hcd and I'm not sure if it can. I don't recall
any way to make endpoints halt on short transfer at all.

The driver does exactly two things with this flag:
1. Isochronus frames get EREMOTEIO status when it's detected, although
   ISTR that URB_SHORT_NOT_OK isn't defined for isochronous URBs.
2. All others log a dbg() message. Supposedly, core is responsible for
   updating urb->status then.

> > * The USB core will not attempt to unlink pending URBs due to halted
> >   condition
> > * The HCD is responsible for completing or canceling queued URBs
> >   when the halted flag is set. Cancelled and newly submitted URBs
> > will be returned with -EPIPE as long as the halted flag is set  
> 
> Why make the HCD responsible for draining the queue?  It's like
> setting the halted flag; one central place is simpler and less
> error-prone than lots of separate places.

Is emptying the queue really a good idea at all? It obviously breaks
lazy drivers which just ignore errors and continue with the URBs they
have already submitted. Removing the URBs only adds more data loss.

> For newly submitted URBs, should the submission fail with -EPIPE, or 
> should the submission succeed and then the URB complete with -EPIPE?  
> The first is simpler, but drivers probably aren't prepared for it

Are they truly expecting the alternative? I bet most of them would
usb_clear_halt() for each received EPIPE, or not at all.

If completion unlinks remaining URBs before returning, this question
doesn't exist (if we fix restarting before completion). If it doesn't
unlink them, who even knows what the driver wants?

For example, some drivers resubmit the URB while handling EPROTO and
don't unlink anything. To me, it says "screw data loss and continue".

It admittedly doesn't work on SuperSpeed anymore, but not all drivers
need to worry about SuperSpeed. Including some old and lazy ones.

Regards,
Michal

^ permalink raw reply

* Re: [PATCH] usb: cdns3: attempt to fix Kconfig dependencies
From: Greg Kroah-Hartman @ 2026-04-03  8:54 UTC (permalink / raw)
  To: Peter Chen (CIX)
  Cc: Arnd Bergmann, Pawel Laszczak, Arnd Bergmann, Roger Quadros,
	linux-usb, linux-kernel
In-Reply-To: <ac9xVUVB/BKfBUmE@nchen-desktop>

On Fri, Apr 03, 2026 at 03:50:45PM +0800, Peter Chen (CIX) wrote:
> On 26-04-02 16:09:55, Arnd Bergmann wrote:
> > 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")
> 
> Hi Arnd,
> 
> Thanks for fixing it, I am sorry for taking your effort debug it.
> 
> > 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.
> 
> It depends on Greg, I am okay for both ways. If Greg reverts the patch,
> I will do below improvements and adapts for most of your changes for v3
> patch.

I'll be glad to revert if you want me to, just let me know.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH] usb: cdns3: attempt to fix Kconfig dependencies
From: Peter Chen (CIX) @ 2026-04-03  9:26 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Pawel Laszczak, Roger Quadros,
	linux-usb, linux-kernel
In-Reply-To: <26779a33-135a-4881-8614-9987c5e90380@app.fastmail.com>

On 26-04-03 10:39:18, Arnd Bergmann wrote:
> On Fri, Apr 3, 2026, at 09:50, Peter Chen (CIX) wrote:
> > On 26-04-02 16:09:55, Arnd Bergmann wrote:
> >> ---
> >> 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.
> >
> > It depends on Greg, I am okay for both ways. If Greg reverts the patch,
> > I will do below improvements and adapts for most of your changes for v3
> > patch.
> >
> > cdns-usb-common.ko is a libary, and no USB/GADGET dependency, could builds in.
> >   ├── core.o  
> >   └── drd.o
> >
> > cdns3-host.ko     -> depends on USB/XHCI(it is m when USB = m
> > cdns3.ko (gadget) -> depends on USB_GADGET
> > cdnsp.ko (gadget) -> depends on USB_GADGET
> > cdns3-plat.ko     -> assign host_init/gadget_init function pointer
> 
> This all sounds good. Ideally cdns3-plat.ko would also lose the
> dependency on cdns3.ko/cdnsp.ko

No, it can't. cdns3-plat.ko depends on cdns3.ko/cdsnp.ko or its
stub.

> 
> >> @@ -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
> >
> > %s/CONFIG_USB_CDNS_PLATFORM/USB_CDNS_PLATFORM
> 
> Oh, this means my patch did not actually work at all, and my testing
> was pointless. I've fixed that now in my randconfig tree to see
> if anything else comes up.
> 
>      Arnd

I am afraid it may need to change more for your patch, otherwise, it could have
error with below combinations:

USB=y USB_GADGET=y USB_CDNS_SUPPORT=y USB_CDNS_HOST=y USB_CDNS_PLATFORM=y USB_CDNS3=y
USB_CDNS3_GADGET=y USB_CDNS3_HOST=y USB_CDNSP=y USB_CDNSP_GADGET=m USB_CDNSP_HOST=y

USB_CDNS_PLATFORM reference to the cdnsp_gadget_init which is built as module.

drivers/usb/cdns3/cdns3-plat.c:52: undefined reference to `cdnsp_gadget_init'

So far, below changes works okay at my test:

diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
index 07ed694e10b8..57cd7e3b5b22 100644
--- a/drivers/usb/cdns3/Kconfig
+++ b/drivers/usb/cdns3/Kconfig
@@ -13,11 +13,13 @@ config USB_CDNS_HOST
 	bool
 	depends on USB=y || USB=USB_CDNS_SUPPORT
 
-config CONFIG_USB_CDNS_PLATFORM
+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
+	depends on USB_CDNS3_GADGET || !USB_CDNS3_GADGET
+	depends on USB_CDNSP_GADGET || !USB_CDNSP_GADGET
 	help
 	  The platform driver support is needed on any SoC integrating
 	  a variant of the Cadence USB3 or USBSSP dual-role controllers,
@@ -118,7 +120,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


-- 

Best regards,
Peter

^ permalink raw reply related

* Re: [PATCH] usb: cdns3: attempt to fix Kconfig dependencies
From: Peter Chen (CIX) @ 2026-04-03  9:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Arnd Bergmann, Pawel Laszczak, Arnd Bergmann, Roger Quadros,
	linux-usb, linux-kernel
In-Reply-To: <2026040328-residence-auction-d94d@gregkh>

On 26-04-03 10:54:44, Greg Kroah-Hartman wrote:
> On Fri, Apr 03, 2026 at 03:50:45PM +0800, Peter Chen (CIX) wrote:
> > On 26-04-02 16:09:55, Arnd Bergmann wrote:
> > > 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")
> > 
> > Hi Arnd,
> > 
> > Thanks for fixing it, I am sorry for taking your effort debug it.
> > 
> > > 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.
> > 
> > It depends on Greg, I am okay for both ways. If Greg reverts the patch,
> > I will do below improvements and adapts for most of your changes for v3
> > patch.
> 
> I'll be glad to revert if you want me to, just let me know.
> 

Thanks, Greg. Please revert below two patches in your usb-next branch:

6076388ca1ed usb: cdns3: Add USBSSP platform driver support
fb14e7f7cbb4 dt-bindings: usb: cdns,usb3: document USBSSP controller support

-- 

Best regards,
Peter

^ permalink raw reply

* [PATCH v7 0/2] Add Genesys logic GL3590 HUB support
From: Swati Agarwal @ 2026-04-03 10:04 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Icenowy Zheng, Matthias Kaehlcke, Alexey Charkov,
	Dmitry Baryshkov, Mike Looijmans, Pin-yen Lin,
	J . Neuschäfer, Chaoyi Chen, Krzysztof Kozlowski
  Cc: linux-usb, devicetree, linux-kernel, Swati Agarwal

Add Genesys logic GL3590 HUB support.

Changes in v7:
Split Driver and DT patch for USB1 controller and USB1 HUB support
Minor nit in commit text of DT binding patch
Added false status for vdd12 supply wherevr applicable

Changes in v6:
- Removed vdd12-supply property status for all hubs.
- Added connections between genesys hub, HD3 port controller and type-C
  connector as follows.

GL3590 USB     HD3SS3220 Type‑C  USB TYPE‑C
   HUB         Port Controller   Connector


HS HUB                           HS con

 |------------------------------------|

SS HUB          HD3-out          SBU con

 |--------------------|

                HD3-in           SS con
                |---------------------|

Changes in v5:
Updated comment description in DT.
Updated vdd-supply status for other hubs in bindings.

Changes in v4:
Updated power supply property for hub.
Updated details for all 4 ports of hub.

Changes in v3:
Updated binding properties for genesys hub.

Changes in v2:
Add Genesys Logic GL3590 hub support.
Rename hd3ss3220_ instance for primary port controller.

Link to v6:
https://lore.kernel.org/all/20260318040644.3591478-1-swati.agarwal@oss.qualcomm.com/

Link to v5:
https://lore.kernel.org/all/20260122092852.887624-1-swati.agarwal@oss.qualcomm.com/

Link to v4:
https://lore.kernel.org/all/20260120103312.2174727-1-swati.agarwal@oss.qualcomm.com/

Link to v3:
https://lore.kernel.org/all/20251220063537.3639535-1-swati.agarwal@oss.qualcomm.com/

Link to v2:
https://lore.kernel.org/all/20251216120749.94007-1-swati.agarwal@oss.qualcomm.com/

Link to v1:
https://lore.kernel.org/all/20251203-swati-v1-1-250efcb4e6a7@oss.qualcomm.com/


Swati Agarwal (2):
  dt-bindings: usb: Add Genesys Logic GL3590 hub
  usb: misc: onboard_usb_hub: Add Genesys Logic GL3590 hub support

 .../bindings/usb/genesys,gl850g.yaml           | 18 ++++++++++++++++++
 drivers/usb/misc/onboard_usb_dev.c             |  1 +
 drivers/usb/misc/onboard_usb_dev.h             |  8 ++++++++
 3 files changed, 27 insertions(+)

-- 
2.34.1


^ permalink raw reply

* [PATCH v7 1/2] dt-bindings: usb: Add Genesys Logic GL3590 hub
From: Swati Agarwal @ 2026-04-03 10:04 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Icenowy Zheng, Matthias Kaehlcke, Alexey Charkov,
	Dmitry Baryshkov, Mike Looijmans, Pin-yen Lin,
	J . Neuschäfer, Chaoyi Chen, Krzysztof Kozlowski
  Cc: linux-usb, devicetree, linux-kernel, Swati Agarwal
In-Reply-To: <20260403100435.3477729-1-swati.agarwal@oss.qualcomm.com>

Add the binding for the USB3.2 Genesys Logic GL3590 hub.
GL3590 hub requires 1.2V and 3.3V supplies for operation.

Signed-off-by: Swati Agarwal <swati.agarwal@oss.qualcomm.com>
---
 .../bindings/usb/genesys,gl850g.yaml           | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
index 9a94b2a74a1e..e8b8c03f87a0 100644
--- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
+++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
@@ -15,6 +15,7 @@ properties:
       - usb5e3,608
       - usb5e3,610
       - usb5e3,620
+      - usb5e3,625
       - usb5e3,626
 
   reg: true
@@ -26,6 +27,10 @@ properties:
     description:
       The regulator that provides 3.3V or 5.0V core power to the hub.
 
+  vdd12-supply:
+    description:
+      The regulator that provides 1.2V power to the hub.
+
   peer-hub: true
 
   ports:
@@ -56,6 +61,7 @@ allOf:
       properties:
         peer-hub: false
         vdd-supply: false
+        vdd12-supply: false
 
   - if:
       properties:
@@ -68,6 +74,18 @@ allOf:
       properties:
         peer-hub: true
         vdd-supply: true
+        vdd12-supply: false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - usb5e3,625
+    then:
+      properties:
+        peer-hub: true
+        vdd-supply: true
 
 unevaluatedProperties: false
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH v7 2/2] usb: misc: onboard_usb_hub: Add Genesys Logic GL3590 hub support
From: Swati Agarwal @ 2026-04-03 10:04 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Icenowy Zheng, Matthias Kaehlcke, Alexey Charkov,
	Dmitry Baryshkov, Mike Looijmans, Pin-yen Lin,
	J . Neuschäfer, Chaoyi Chen, Krzysztof Kozlowski
  Cc: linux-usb, devicetree, linux-kernel, Swati Agarwal
In-Reply-To: <20260403100435.3477729-1-swati.agarwal@oss.qualcomm.com>

Add support for the GL3590 4 ports USB3.2 hub.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Swati Agarwal <swati.agarwal@oss.qualcomm.com>
---
 drivers/usb/misc/onboard_usb_dev.c | 1 +
 drivers/usb/misc/onboard_usb_dev.h | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c
index 7cdbdfe07a76..11508ed4df25 100644
--- a/drivers/usb/misc/onboard_usb_dev.c
+++ b/drivers/usb/misc/onboard_usb_dev.c
@@ -665,6 +665,7 @@ static const struct usb_device_id onboard_dev_id_table[] = {
 	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0608) }, /* Genesys Logic GL850G USB 2.0 HUB */
 	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0610) }, /* Genesys Logic GL852G USB 2.0 HUB */
 	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0620) }, /* Genesys Logic GL3523 USB 3.1 HUB */
+	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0625) }, /* Genesys Logic GL3590 USB 3.2 HUB */
 	{ USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2412) }, /* USB2412 USB 2.0 HUB */
 	{ USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2514) }, /* USB2514B USB 2.0 HUB */
 	{ USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2517) }, /* USB2517 USB 2.0 HUB */
diff --git a/drivers/usb/misc/onboard_usb_dev.h b/drivers/usb/misc/onboard_usb_dev.h
index ac1aa3e122ad..3523f8f8a149 100644
--- a/drivers/usb/misc/onboard_usb_dev.h
+++ b/drivers/usb/misc/onboard_usb_dev.h
@@ -101,6 +101,13 @@ static const struct onboard_dev_pdata genesys_gl852g_data = {
 	.is_hub = true,
 };
 
+static const struct onboard_dev_pdata genesys_gl3590_data = {
+	.reset_us = 50,
+	.num_supplies = 2,
+	.supply_names = { "vdd", "vdd12" },
+	.is_hub = true,
+};
+
 static const struct onboard_dev_pdata usb_a_conn_data = {
 	.num_supplies = 1,
 	.supply_names = { "vbus" },
@@ -146,6 +153,7 @@ static const struct of_device_id onboard_dev_match[] = {
 	{ .compatible = "usb5e3,608", .data = &genesys_gl850g_data, },
 	{ .compatible = "usb5e3,610", .data = &genesys_gl852g_data, },
 	{ .compatible = "usb5e3,620", .data = &genesys_gl852g_data, },
+	{ .compatible = "usb5e3,625", .data = &genesys_gl3590_data, },
 	{ .compatible = "usb5e3,626", .data = &genesys_gl852g_data, },
 	{ .compatible = "usbbda,179", .data = &realtek_rtl8188etv_data, },
 	{ .compatible = "usbbda,411", .data = &realtek_rts5411_data, },
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v1 1/1] usb: typec: tipd: Restore generic TPS6598x contract interrupts
From: Sebastian Krzyszkowiak @ 2026-04-03 10:24 UTC (permalink / raw)
  To: Vincent Cloutier
  Cc: heikki.krogerus, gregkh, linux-usb, linux-kernel, sven,
	Vincent Cloutier, stable
In-Reply-To: <20260402000950.715470-1-vincent.cloutier@icloud.com>

On czwartek, 2 kwietnia 2026 02:09:50 czas środkowoeuropejski letni Vincent 
Cloutier wrote:
> 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>
> ---
>  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,

This driver has never handled these interrupts (as seen in the commit you 
referenced), so these should be added in patches that make it handle them.

You likely got confused by the patches that still stay in the downstream 
Librem 5 tree. We should get them cleaned up and mainlined, but until that 
happens this patch doesn't make much sense here I'm afraid.

S.



^ permalink raw reply

* RE: [PATCH] usb: cdnsp: Add support for device-only configuration
From: Pawel Laszczak @ 2026-04-03 10:33 UTC (permalink / raw)
  To: Peter Chen (CIX)
  Cc: Roger Quadros, Greg Kroah-Hartman, Bjorn Helgaas,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org
In-Reply-To: <ac9WnY5C+yTVi0X/@nchen-desktop>

>
>On 26-03-31 10:19:11, Pawel Laszczak via B4 Relay wrote:
>> From: Pawel Laszczak <pawell@cadence.com>
>>
>> This patch introduces support for operating the Cadence USBSSP (cdnsp)
>> controller in a peripheral-only mode, bypassing the Dual-Role Device
>> (DRD) logic.
>
>Is it possible the SoC customer (not Cadence internal) has this configuration too?

Yes, it is definitely possible for SoC customers to have this configuration.
The USBSSP IP is highly configurable, and some customers may choose a
peripheral-only implementation to optimize area or power.

>
>>
>> The change in BAR indexing (from BAR 2 to BAR 1) is a direct
>> consequence of switching from 64-bit to 32-bit addressing in the
>> Peripheral-only configuration.
>>
>> Tested on PCI platform with Device-only configuration. Platform-side
>> changes are included to support the PCI glue layer's property injection.
>>
>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>> ---
>> NOTE: This patch depends on Peter Chen's series "usb: cdns3: Add
>> USBSSP platform driver support", which was recently accepted by Greg
>> into the usb-testing branch (commit 6076388ca1ed).
>> ---
>>  drivers/usb/cdns3/cdns3-plat.c | 24 ++++++++++++---------
>> drivers/usb/cdns3/cdnsp-pci.c  | 47
>++++++++++++++++++++++++++++++++++--------
>>  drivers/usb/cdns3/core.c       |  3 ++-
>>  drivers/usb/cdns3/core.h       |  5 ++++-
>>  drivers/usb/cdns3/drd.c        | 16 ++++++++++++--
>>  include/linux/pci_ids.h        |  1 +
>>  6 files changed, 73 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/usb/cdns3/cdns3-plat.c
>> b/drivers/usb/cdns3/cdns3-plat.c index 71c612e27b73..33746e672cda
>> 100644
>> --- a/drivers/usb/cdns3/cdns3-plat.c
>> +++ b/drivers/usb/cdns3/cdns3-plat.c
>> @@ -75,6 +75,7 @@ static int cdns3_plat_probe(struct platform_device
>*pdev)
>>  	if (cdns->pdata && cdns->pdata->override_apb_timeout)
>>  		cdns->override_apb_timeout = cdns->pdata-
>>override_apb_timeout;
>>
>> +	cdns->no_drd = device_property_read_bool(dev, "no_drd");
>>  	platform_set_drvdata(pdev, cdns);
>>
>>  	ret = platform_get_irq_byname(pdev, "host"); @@ -107,21 +108,23 @@
>> static int cdns3_plat_probe(struct platform_device *pdev)
>>
>>  	cdns->dev_regs	= regs;
>>
>> -	cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
>> -	if (cdns->otg_irq < 0)
>> -		return dev_err_probe(dev, cdns->otg_irq,
>> -				     "Failed to get otg IRQ\n");
>> +	if (!cdns->no_drd) {
>> +		cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
>> +		if (cdns->otg_irq < 0)
>> +			return dev_err_probe(dev, cdns->otg_irq,
>> +					     "Failed to get otg IRQ\n");
>
>I suggest making "otg" register/irq as optional, if it is not existed, we set
>"no_drd" flag for cdns structure. In that case, we do not need to add new
>property.
>
>>
>> -	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg");
>> -	if (!res) {
>> -		dev_err(dev, "couldn't get otg resource\n");
>> -		return -ENXIO;
>> +		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>"otg");
>> +		if (!res) {
>> +			dev_err(dev, "couldn't get otg resource\n");
>> +			return -ENXIO;
>> +		}
>> +
>> +		cdns->otg_res = *res;
>>  	}
>>
>>  	cdns->phyrst_a_enable = device_property_read_bool(dev,
>> "cdns,phyrst-a-enable");
>>
>> -	cdns->otg_res = *res;
>> -
>>  	cdns->wakeup_irq = platform_get_irq_byname_optional(pdev,
>"wakeup");
>>  	if (cdns->wakeup_irq == -EPROBE_DEFER)
>>  		return cdns->wakeup_irq;
>> @@ -158,6 +161,7 @@ static int cdns3_plat_probe(struct platform_device
>*pdev)
>>  		goto err_cdns_init;
>>
>>  	cdns->gadget_init = cdns3_plat_gadget_init;
>> +
>
>Not related to this change.
>
>>  	ret = cdns_core_init_role(cdns);
>>  	if (ret)
>>  		goto err_cdns_init;
>> diff --git a/drivers/usb/cdns3/cdnsp-pci.c
>> b/drivers/usb/cdns3/cdnsp-pci.c index 432007cfe695..e20c59ceb8a4
>> 100644
>> --- a/drivers/usb/cdns3/cdnsp-pci.c
>> +++ b/drivers/usb/cdns3/cdnsp-pci.c
>> @@ -19,6 +19,7 @@
>>
>>  struct cdnsp_wrap {
>>  	struct platform_device *plat_dev;
>> +	struct property_entry prop[3];
>>  	struct resource dev_res[6];
>>  	int devfn;
>>  };
>> @@ -29,10 +30,15 @@ struct cdnsp_wrap {
>>  #define RES_HOST_ID		3
>>  #define RES_DEV_ID		4
>>  #define RES_DRD_ID		5
>> -
>> +/* DRD PCI configuration - 64-bit addressing */
>> +/* First PCI function */
>>  #define PCI_BAR_HOST		0
>> -#define PCI_BAR_OTG		0
>>  #define PCI_BAR_DEV		2
>> +/* Second PCI function */
>> +#define PCI_BAR_OTG		0
>> +/* Device only PCI configuration - 32-bit addressing */
>> +/* First PCI function */
>> +#define PCI_BAR_ONLY_DEV	1
>>
>>  #define PCI_DEV_FN_HOST_DEVICE	0
>>  #define PCI_DEV_FN_OTG		1
>> @@ -65,6 +71,7 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
>>  	struct cdnsp_wrap *wrap;
>>  	struct resource *res;
>>  	struct pci_dev *func;
>> +	bool no_drd = false;
>>  	int ret = 0;
>>
>>  	/*
>> @@ -75,11 +82,14 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
>>  		    pdev->devfn != PCI_DEV_FN_OTG))
>>  		return -EINVAL;
>>
>> +	if (pdev->device == PCI_DEVICE_ID_CDNS_UDC_USBSSP)
>> +		no_drd = true;
>> +
>>  	func = cdnsp_get_second_fun(pdev);
>> -	if (!func)
>> +	if (!func && !no_drd)
>>  		return -EINVAL;
>>
>> -	if (func->class == PCI_CLASS_SERIAL_USB_XHCI ||
>> +	if ((func && func->class == PCI_CLASS_SERIAL_USB_XHCI) ||
>>  	    pdev->class == PCI_CLASS_SERIAL_USB_XHCI) {
>>  		ret = -EINVAL;
>>  		goto put_pci;
>> @@ -93,7 +103,7 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
>>
>>  	pci_set_master(pdev);
>>
>> -	if (pci_is_enabled(func)) {
>> +	if (func && pci_is_enabled(func)) {
>>  		wrap = pci_get_drvdata(func);
>>  	} else {
>>  		wrap = kzalloc_obj(*wrap);
>> @@ -106,10 +116,13 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
>>  	res = wrap->dev_res;
>>
>>  	if (pdev->devfn == PCI_DEV_FN_HOST_DEVICE) {
>> +		int bar_dev = no_drd ? PCI_BAR_ONLY_DEV : PCI_BAR_DEV;
>> +
>>  		/* Function 0: host(BAR_0) + device(BAR_2). */
>>  		dev_dbg(&pdev->dev, "Initialize Device resources\n");
>> -		res[RES_DEV_ID].start = pci_resource_start(pdev, PCI_BAR_DEV);
>> -		res[RES_DEV_ID].end = pci_resource_end(pdev, PCI_BAR_DEV);
>> +
>> +		res[RES_DEV_ID].start = pci_resource_start(pdev, bar_dev);
>> +		res[RES_DEV_ID].end = pci_resource_end(pdev, bar_dev);
>>  		res[RES_DEV_ID].name = "dev";
>>  		res[RES_DEV_ID].flags = IORESOURCE_MEM;
>>  		dev_dbg(&pdev->dev, "USBSSP-DEV physical base addr: %pa\n",
>@@
>> -145,9 +158,20 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
>>  		wrap->dev_res[RES_IRQ_OTG_ID].flags = IORESOURCE_IRQ;
>>  	}
>>
>> -	if (pci_is_enabled(func)) {
>> +	if (no_drd || pci_is_enabled(func)) {
>> +		u8 idx = 0;
>> +
>>  		/* set up platform device info */
>>  		pdata.override_apb_timeout = CHICKEN_APB_TIMEOUT_VALUE;
>> +		if (no_drd) {
>> +			wrap->prop[idx++] =
>PROPERTY_ENTRY_STRING("dr_mode", "peripheral");
>> +			wrap->prop[idx++] =
>PROPERTY_ENTRY_BOOL("no_drd");
>> +		} else {
>> +			wrap->prop[idx++] =
>PROPERTY_ENTRY_STRING("dr_mode", "otg");
>> +			wrap->prop[idx++] = PROPERTY_ENTRY_BOOL("usb-role-
>switch");
>> +		}
>> +
>> +		wrap->prop[idx] = (struct property_entry){ };
>>  		memset(&plat_info, 0, sizeof(plat_info));
>>  		plat_info.parent = &pdev->dev;
>>  		plat_info.fwnode = pdev->dev.fwnode; @@ -158,6 +182,7 @@
>static int
>> cdnsp_pci_probe(struct pci_dev *pdev,
>>  		plat_info.dma_mask = pdev->dma_mask;
>>  		plat_info.data = &pdata;
>>  		plat_info.size_data = sizeof(pdata);
>> +		plat_info.properties = wrap->prop;
>>  		wrap->devfn = pdev->devfn;
>>  		/* register platform device */
>>  		wrap->plat_dev = platform_device_register_full(&plat_info);
>> @@ -185,13 +210,17 @@ static void cdnsp_pci_remove(struct pci_dev *pdev)
>>  	if (wrap->devfn == pdev->devfn)
>>  		platform_device_unregister(wrap->plat_dev);
>>
>> -	if (!pci_is_enabled(func))
>> +	if (!func || !pci_is_enabled(func))
>>  		kfree(wrap);
>>
>>  	pci_dev_put(func);
>>  }
>>
>>  static const struct pci_device_id cdnsp_pci_ids[] = {
>> +	{ PCI_DEVICE(PCI_VENDOR_ID_CDNS,
>PCI_DEVICE_ID_CDNS_UDC_USBSSP),
>> +	  .class = PCI_CLASS_SERIAL_USB_DEVICE },
>> +	{ PCI_DEVICE(PCI_VENDOR_ID_CDNS,
>PCI_DEVICE_ID_CDNS_UDC_USBSSP),
>> +	  .class = PCI_CLASS_SERIAL_USB_CDNS },
>>  	{ PCI_DEVICE(PCI_VENDOR_ID_CDNS, PCI_DEVICE_ID_CDNS_USBSSP),
>>  	  .class = PCI_CLASS_SERIAL_USB_DEVICE },
>>  	{ PCI_DEVICE(PCI_VENDOR_ID_CDNS, PCI_DEVICE_ID_CDNS_USBSSP),
>diff
>> --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c index
>> 10f00b6c3c83..72f7acba6258 100644
>> --- a/drivers/usb/cdns3/core.c
>> +++ b/drivers/usb/cdns3/core.c
>> @@ -71,7 +71,8 @@ static void cdns_role_stop(struct cdns *cdns)
>> static void cdns_exit_roles(struct cdns *cdns)  {
>>  	cdns_role_stop(cdns);
>> -	cdns_drd_exit(cdns);
>> +	if (!cdns->no_drd)
>> +		cdns_drd_exit(cdns);
>>  }
>>
>>  /**
>> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h index
>> dc8c4137de15..6abe231f4559 100644
>> --- a/drivers/usb/cdns3/core.h
>> +++ b/drivers/usb/cdns3/core.h
>> @@ -80,9 +80,11 @@ struct cdns3_platform_data {
>>   * @pdata: platform data from glue layer
>>   * @lock: spinlock structure
>>   * @xhci_plat_data: xhci private data structure pointer
>> + * @gadget_init: pointer to gadget initialization function
>>   * @override_apb_timeout: hold value of APB timeout. For value 0 the default
>>   *                        value in CHICKEN_BITS_3 will be preserved.
>> - * @gadget_init: pointer to gadget initialization function
>> + * @no_drd: DRD register block is inaccessible - driver handles only
>> + *          device mode.
>>   */
>>  struct cdns {
>>  	struct device			*dev;
>> @@ -122,6 +124,7 @@ struct cdns {
>>  	struct xhci_plat_priv		*xhci_plat_data;
>>  	int (*gadget_init)(struct cdns *cdns);
>>  	u32                             override_apb_timeout;
>> +	bool				no_drd;
>>  };
>>
>>  int cdns_hw_role_switch(struct cdns *cdns); diff --git
>> a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c index
>> 84fb38a5723a..38f3051c2188 100644
>> --- a/drivers/usb/cdns3/drd.c
>> +++ b/drivers/usb/cdns3/drd.c
>> @@ -107,7 +107,7 @@ void cdns_clear_vbus(struct cdns *cdns)  {
>>  	u32 reg;
>>
>> -	if (cdns->version != CDNSP_CONTROLLER_V2)
>> +	if (cdns->version != CDNSP_CONTROLLER_V2 || cdns->no_drd)
>>  		return;
>>
>>  	reg = readl(&cdns->otg_cdnsp_regs->override);
>> @@ -120,7 +120,7 @@ void cdns_set_vbus(struct cdns *cdns)  {
>>  	u32 reg;
>>
>> -	if (cdns->version != CDNSP_CONTROLLER_V2)
>> +	if (cdns->version != CDNSP_CONTROLLER_V2 || cdns->no_drd)
>>  		return;
>>
>>  	reg = readl(&cdns->otg_cdnsp_regs->override);
>> @@ -234,6 +234,9 @@ int cdns_drd_gadget_on(struct cdns *cdns)
>>  	u32 ready_bit;
>>  	int ret, val;
>>
>> +	if (cdns->no_drd)
>> +		return 0;
>> +
>
>You may add similar logic for cdns_power_is_lost and cdns_get_id.
>
>>  	/* switch OTG core */
>>  	writel(OTGCMD_DEV_BUS_REQ | reg, &cdns->otg_regs->cmd);
>>
>> @@ -265,6 +268,9 @@ void cdns_drd_gadget_off(struct cdns *cdns)  {
>>  	u32 val;
>>
>> +	if (cdns->no_drd)
>> +		return;
>> +
>>  	/*
>>  	 * Driver should wait at least 10us after disabling Device
>>  	 * before turning-off Device (DEV_BUS_DROP).
>> @@ -392,6 +398,12 @@ int cdns_drd_init(struct cdns *cdns)
>>  	u32 state, reg;
>>  	int ret;
>>
>> +	if (cdns->no_drd) {
>> +		cdns->version  = CDNSP_CONTROLLER_V2;
>> +		cdns->dr_mode = USB_DR_MODE_PERIPHERAL;
>
>With "no_drd", are configurations fixed, no other options?

Yes, no other option.

Pawel

>
>> +		return 0;
>> +	}
>> +
>>  	regs = devm_ioremap_resource(cdns->dev, &cdns->otg_res);
>>  	if (IS_ERR(regs))
>>  		return PTR_ERR(regs);
>> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index
>> 406abf629be2..a931fb201402 100644
>> --- a/include/linux/pci_ids.h
>> +++ b/include/linux/pci_ids.h
>> @@ -2424,6 +2424,7 @@
>>  #define PCI_DEVICE_ID_CDNS_USBSS	0x0100
>>  #define PCI_DEVICE_ID_CDNS_USB		0x0120
>>  #define PCI_DEVICE_ID_CDNS_USBSSP	0x0200
>> +#define PCI_DEVICE_ID_CDNS_UDC_USBSSP	0x0400
>>
>>  #define PCI_VENDOR_ID_ARECA		0x17d3
>>  #define PCI_DEVICE_ID_ARECA_1110	0x1110
>>
>> ---
>> base-commit: 5196f2c98340297e1fdd36555285e991ceddf776
>> change-id: 20260331-device_only-192d539430b5
>>
>> Best regards,
>> --
>> Pawel Laszczak <pawell@cadence.com>
>>
>>
>
>--
>
>Best regards,
>Peter

^ permalink raw reply

* Re: [PATCH v3 8/9] driver core: Replace dev->of_node_reused with DEV_FLAG_OF_NODE_REUSED
From: Mark Brown @ 2026-04-03 11:56 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Greg Kroah-Hartman, Rafael J . Wysocki, Danilo Krummrich,
	Alan Stern, Robin Murphy, Leon Romanovsky, Paul Burton,
	Saravana Kannan, Alexander Lobakin, Eric Dumazet, Toshi Kani,
	Christoph Hellwig, Alexey Kardashevskiy, Johan Hovold,
	alexander.stein, andrew, andrew, andriy.shevchenko, astewart,
	bhelgaas, brgl, davem, devicetree, driver-core, hkallweit1,
	jirislaby, joel, kees, kuba, lgirdwood, linux-arm-kernel,
	linux-aspeed, linux-kernel, linux-pci, linux-serial, linux-usb,
	linux, mani, netdev, pabeni, robh
In-Reply-To: <20260402174925.v3.8.I806b8636cd3724f6cd1f5e199318ab8694472d90@changeid>

[-- Attachment #1: Type: text/plain, Size: 282 bytes --]

On Thu, Apr 02, 2026 at 05:49:54PM -0700, Douglas Anderson wrote:
> In C, bitfields are not necessarily safe to modify from multiple
> threads without locking. Switch "of_node_reused" over to the "flags"
> field so modifications are safe.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH] usb: cdns3: attempt to fix Kconfig dependencies
From: Greg Kroah-Hartman @ 2026-04-03 12:03 UTC (permalink / raw)
  To: Peter Chen (CIX)
  Cc: Arnd Bergmann, Pawel Laszczak, Arnd Bergmann, Roger Quadros,
	linux-usb, linux-kernel
In-Reply-To: <ac+LEWMCQpLSnfoD@nchen-desktop>

On Fri, Apr 03, 2026 at 05:40:33PM +0800, Peter Chen (CIX) wrote:
> On 26-04-03 10:54:44, Greg Kroah-Hartman wrote:
> > On Fri, Apr 03, 2026 at 03:50:45PM +0800, Peter Chen (CIX) wrote:
> > > On 26-04-02 16:09:55, Arnd Bergmann wrote:
> > > > 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")
> > > 
> > > Hi Arnd,
> > > 
> > > Thanks for fixing it, I am sorry for taking your effort debug it.
> > > 
> > > > 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.
> > > 
> > > It depends on Greg, I am okay for both ways. If Greg reverts the patch,
> > > I will do below improvements and adapts for most of your changes for v3
> > > patch.
> > 
> > I'll be glad to revert if you want me to, just let me know.
> > 
> 
> Thanks, Greg. Please revert below two patches in your usb-next branch:
> 
> 6076388ca1ed usb: cdns3: Add USBSSP platform driver support
> fb14e7f7cbb4 dt-bindings: usb: cdns,usb3: document USBSSP controller support

Ick, doesn't revert cleanly, I'm going to also have to drop commit
7b7f2dd91382 ("usb: cdnsp: Add support for device-only configuration")
Will go do that now....

greg k-h


^ permalink raw reply

* Re: [PATCH] USB: cdc-acm: Add support for second ACM channel on more Nokia phones
From: Рыжов Фёдор @ 2026-04-03 13:20 UTC (permalink / raw)
  To: Oliver Neukum, linux-kernel@vger.kernel.org
  Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org
In-Reply-To: <7ae48e14-0228-4bd9-91df-306dcd12896c@suse.com>

30.03.2026 13:04, Oliver Neukum пишет:
> Hi Fyodor,
> 
> On 26.03.26 15:37, Рыжов Фёдор wrote:
> 
>> This patch extends the whitelist with all S60v3.x and S60v5.x phones,
>> supported by official "Nokia PC Suite" driver for Windows. Device IDs
>> are taken from its installation configuration file. Older S60v2.x and
>> S60v1.x devices were not added because I have no hardware to test.
> 
> Thank you for doing this and testing the patch. One nit.
> This has two IDs for the same model. This looks like a typo, even
> if it is real. Could you add a comment to such entries?
> 
>     Regards
>         Oliver
> 

Hello Oliver,

Thank you for your response.

Nokia phones have two kinds of model names: "commercial" (for example, "X3-02") and "type identifier" (my X3-02 is "RM-639"). There are cases when different phones (with different RM codes) were released under one commercial name. They look and work very similarly, but may have difference in memory amount, radio bands, SIM operator lock, region-specific SARs, etc. For example, T-Mobile branded Lumia 710 is "RM-809", but "global version" has code "RM-803".

Different RM-models have different incompatible firmwares. They also have different USB PIDs. That's why there are multiple lines with same model name.

I specified only the commercial model names in my patch, because my source of information (the driver configuration) does not provide the RM codes.

^ permalink raw reply

* Re: [PATCH v1 1/1] usb: typec: tipd: Restore generic TPS6598x contract interrupts
From: Vincent Cloutier @ 2026-04-03 14:09 UTC (permalink / raw)
  To: Sebastian Krzyszkowiak
  Cc: heikki.krogerus, gregkh, linux-usb, linux-kernel, sven,
	Vincent Cloutier, stable
In-Reply-To: <2374707.tdWV9SEqCh@pliszka>



> Le 3 avr. 2026 à 06:25, Sebastian Krzyszkowiak <sebastian.krzyszkowiak@puri.sm> a écrit :
> 
> On czwartek, 2 kwietnia 2026 02:09:50 czas środkowoeuropejski letni Vincent
> Cloutier wrote:
>> 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>
>> ---
>> 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,
> 
> This driver has never handled these interrupts (as seen in the commit you
> referenced), so these should be added in patches that make it handle them.
> 
> You likely got confused by the patches that still stay in the downstream
> Librem 5 tree. We should get them cleaned up and mainlined, but until that
> happens this patch doesn't make much sense here I'm afraid.
I’m afraid that’s exactly what happened… I have learned a lesson here. 
> 
> S.
> 
> 
Sorry about this noise and thank you all for your time!

Vincent

^ permalink raw reply

* Re: [RFC PATCH 1/2] xhci: prevent automatic endpoint restart after stall or error
From: stern @ 2026-04-03 14:55 UTC (permalink / raw)
  To: Michal Pecio
  Cc: Thinh Nguyen, Mathias Nyman, linux-usb@vger.kernel.org,
	oneukum@suse.com, niklas.neronin@linux.intel.com
In-Reply-To: <20260403105145.7e74a410.michal.pecio@gmail.com>

On Fri, Apr 03, 2026 at 10:51:45AM +0200, Michal Pecio wrote:
> On Thu, 2 Apr 2026 22:42:39 -0400, stern@rowland.harvard.edu wrote:
> > Every status other than 0 should mean that the endpoint's queue is 
> > halted.  But not all statuses require a clear-halt or reset-endpoint
> > to recover.  For instance, a short transfer when the URB_SHORT_NOT_OK
> > flag is set.
> 
> This doesn't work in xhci-hcd and I'm not sure if it can. I don't recall
> any way to make endpoints halt on short transfer at all.
> 
> The driver does exactly two things with this flag:
> 1. Isochronus frames get EREMOTEIO status when it's detected, although
>    ISTR that URB_SHORT_NOT_OK isn't defined for isochronous URBs.
> 2. All others log a dbg() message. Supposedly, core is responsible for
>    updating urb->status then.

Truly?  What happens if usb-storage is doing a large bulk-IN transfer 
comprising multiple data URBs (with URB_SHORT_NOT_OK set in all but the 
last one), and the device indicates an early end-of-data by sending a 
short packet in the middle?  Will xhci-hcd store the information in the 
following bulk-IN status transaction to the transfer buffer for the next 
URB in the queue of data URBs?

(That's a misleading question.  The BOT class specification requires the 
device to stall the bulk-IN endpoint following a short data transfer, so 
this scenario wouldn't arise -- although not all devices follow the spec 
in this regard.  And it might arise in other protocols.  Regardless, 
it's hard for me to believe there's no way to tell an xHCI controller to 
stop the endpoint on a short transfer.  EHCI, OHCI, and UHCI all have 
ways to do it.)

> > > * The USB core will not attempt to unlink pending URBs due to halted
> > >   condition
> > > * The HCD is responsible for completing or canceling queued URBs
> > >   when the halted flag is set. Cancelled and newly submitted URBs
> > > will be returned with -EPIPE as long as the halted flag is set  
> > 
> > Why make the HCD responsible for draining the queue?  It's like
> > setting the halted flag; one central place is simpler and less
> > error-prone than lots of separate places.
> 
> Is emptying the queue really a good idea at all? It obviously breaks
> lazy drivers which just ignore errors and continue with the URBs they
> have already submitted. Removing the URBs only adds more data loss.

We have to decide on a single approach that can work for everybody.  If 
it means that some class drivers will have to be changed, so be it.

A lazy driver that just ignores errors will currently not work very well 
if at all following an error on an xHCI host controller.  But yes, it 
will work with a USB-2 controller.

If we want to minimize the changes needed in class drivers then we will 
have to make the core and HCDs recover automatically from transaction 
errors.  Non-lazy class drivers still have a chance to do their own 
recovery when the completion handler for the bad URB runs.  To make 
their lives easier, we could add a usb_purge_endpoint_queue() routine to 
the core; this seems a lot safer than summarily unlinking all the URBs 
all the time.

Is that what we should do?

> > For newly submitted URBs, should the submission fail with -EPIPE, or 
> > should the submission succeed and then the URB complete with -EPIPE?  
> > The first is simpler, but drivers probably aren't prepared for it
> 
> Are they truly expecting the alternative? I bet most of them would
> usb_clear_halt() for each received EPIPE, or not at all.

They might do usb_clear_halt() if the completed URB had -EPIPE status.  
But they won't do it when usb_submit_urb() returns -EPIPE (or any other 
error, for that matter).

> If completion unlinks remaining URBs before returning, this question
> doesn't exist (if we fix restarting before completion). If it doesn't
> unlink them, who even knows what the driver wants?

Good point.

> For example, some drivers resubmit the URB while handling EPROTO and
> don't unlink anything. To me, it says "screw data loss and continue".
> 
> It admittedly doesn't work on SuperSpeed anymore, but not all drivers
> need to worry about SuperSpeed. Including some old and lazy ones.

Think of it this way: If any URBs remain in the queue when the 
completion handler returns, we can assume the class driver doesn't want 
them to be unlinked.  When that happens, the best thing the core/HCD can 
do is issue a clear-halt and then restart the endpoint, so the queued 
URBs will run.  Except for the -EPIPE case, where the device halted its 
side of the endpoint and sent a STALL packet -- it's reasonable to 
require class drivers to handle that case themselves.

On the other hand, if the queue is empty when the completion handler 
returns (or if all the URBs still in the queue have already been 
unlinked but not yet given back), is there any harm in the core issuing 
a clear-halt and restarting the endpoint (with the same exception as 
above)?  This doesn't allow the class driver to do any further error 
recovery of its own, but when you get down to it, what else is there for 
it to do?

Also, this way we don't have to reject any URBs submitted while endpoint 
is stopped.  They simply get added to the end of the queue, so they will 
run when the endpoint is restarted.

Alan Stern

^ permalink raw reply

* Re: [PATCH] usb: rtl8150: avoid using uninitialized CSCR value
From: Simon Horman @ 2026-04-03 15:45 UTC (permalink / raw)
  To: Morduan Zang
  Cc: Petko Manolov, 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 Thu, Apr 02, 2026 at 03:07:43PM +0800, 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

I wonder if we can handle this a bit more succinctly,
while still making it clear that the error is handled.
Something like this:

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 4cda0643afb6..816759ced56c 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -722,7 +722,8 @@ 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)
+		tmp = 0;
 	if (tmp & CSCR_LINK_STATUS)
 		netif_carrier_on(netdev);
 	else

^ permalink raw reply related

* Re: [PATCH] USB: cdc-acm: Add support for second ACM channel on more Nokia phones
From: Oliver Neukum @ 2026-04-03 16:06 UTC (permalink / raw)
  To: Рыжов Фёдор,
	Oliver Neukum, linux-kernel@vger.kernel.org
  Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org
In-Reply-To: <7dcc9837-52e8-4760-b710-403d464f908f@outlook.com>

Hi,

On 03.04.26 15:20, Рыжов Фёдор wrote:

> Nokia phones have two kinds of model names: "commercial" (for example, "X3-02") and "type identifier" (my X3-02 is "RM-639"). There are cases when different phones (with different RM codes) were released under one commercial name. They look and work very similarly, but may have difference in memory amount, radio bands, SIM operator lock, region-specific SARs, etc. For example, T-Mobile branded Lumia 710 is "RM-809", but "global version" has code "RM-803".
> 
> Different RM-models have different incompatible firmwares. They also have different USB PIDs. That's why there are multiple lines with same model name.

Thank you for this response. This is important information. Hence I'd
like to request that you add a general comment to the driver with your patch
giving exactly this explanation for device appearing twice. Right in front
of the declaration would be best. We really should make sure we do not
lose such information.

	Regards
		Oliver


^ permalink raw reply

* Re: [PATCH] usb: cdns3: attempt to fix Kconfig dependencies
From: Arnd Bergmann @ 2026-04-03 18:50 UTC (permalink / raw)
  To: Peter Chen
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Pawel Laszczak, Roger Quadros,
	linux-usb, linux-kernel
In-Reply-To: <ac+H2dzj5t1JJJd5@nchen-desktop>

On Fri, Apr 3, 2026, at 11:26, Peter Chen (CIX) wrote:
> On 26-04-03 10:39:18, Arnd Bergmann wrote:
>> On Fri, Apr 3, 2026, at 09:50, Peter Chen (CIX) wrote:
>
> I am afraid it may need to change more for your patch, otherwise, it could have
> error with below combinations:
>
> USB=y USB_GADGET=y USB_CDNS_SUPPORT=y USB_CDNS_HOST=y 
> USB_CDNS_PLATFORM=y USB_CDNS3=y
> USB_CDNS3_GADGET=y USB_CDNS3_HOST=y USB_CDNSP=y USB_CDNSP_GADGET=m 
> USB_CDNSP_HOST=y
>
> USB_CDNS_PLATFORM reference to the cdnsp_gadget_init which is built as module.
>
> drivers/usb/cdns3/cdns3-plat.c:52: undefined reference to `cdnsp_gadget_init'

Good catch! I ran into the same one after a few hundred more randonfig
builds and can confirm that your suggested fixup addresses all of the
problems found so far.

> @@ -13,11 +13,13 @@ config USB_CDNS_HOST
>  	bool
>  	depends on USB=y || USB=USB_CDNS_SUPPORT
> 
> -config CONFIG_USB_CDNS_PLATFORM
> +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
> +	depends on USB_CDNS3_GADGET || !USB_CDNS3_GADGET
> +	depends on USB_CDNSP_GADGET || !USB_CDNSP_GADGET
>  	help
>  	  The platform driver support is needed on any SoC integrating
>  	  a variant of the Cadence USB3 or USBSSP dual-role controllers,

The dependencies here are rather unwieldy, but it does look like
this is the minimum set we need with the current code.

The only other alternative I see would be to split up the
platform driver support into separate modules for cdns3 and
cdnsp as well, which would make the dependencies trivial but
require reworking of the actual in a way that I haven't
been able to figure out yet. If you are already integrating
other changes for the next attempt, maybe you can try to
come up with a solution for this as well.

     Arnd

^ permalink raw reply

* Re: xhci-hcd and URB_SHORT_NOT_OK
From: Michal Pecio @ 2026-04-03 19:13 UTC (permalink / raw)
  To: stern@rowland.harvard.edu
  Cc: Thinh Nguyen, Mathias Nyman, linux-usb@vger.kernel.org,
	oneukum@suse.com, niklas.neronin@linux.intel.com
In-Reply-To: <f3897bb1-fd9e-4321-bc62-31ae8329706b@rowland.harvard.edu>

On Fri, 3 Apr 2026 10:55:58 -0400, stern@rowland.harvard.edu wrote:
> > This doesn't work in xhci-hcd and I'm not sure if it can. I don't
> > recall any way to make endpoints halt on short transfer at all.
> > 
> > The driver does exactly two things with this flag:
> > 1. Isochronus frames get EREMOTEIO status when it's detected,
> >    although ISTR that URB_SHORT_NOT_OK isn't defined for isochronous
> > 2. All others log a dbg() message. Supposedly, core is
> >    responsible for updating urb->status then.  
> 
> Truly?  What happens if usb-storage is doing a large bulk-IN transfer
> comprising multiple data URBs (with URB_SHORT_NOT_OK set in all but
> the last one), and the device indicates an early end-of-data by
> sending a short packet in the middle?  Will xhci-hcd store the
> information in the following bulk-IN status transaction to the
> transfer buffer for the next URB in the queue of data URBs?

Not sure if I understand the question, but if you mean that the driver
submits multiple "data" URBs followed by a "status" URB to the same
pipe and expects it to stop when any data URB completes short, so that
all others can be unlinked before the status transfer, then nope, not
gonna happen. It will be racing with status being written to the data
URBs and likely lose. The status URB may never complete.

I really don't recall any way of stopping on short transfers and the
relevant xHCI 4.10.1.1 section isn't helpful. It's possible to stop
unconditionally and decide to restart or not, at some performance cost.

But short transfer always terminates the current URB, so natural way is
to coalesce all data into a big SG URB. Perhaps xhci-hcd could do it
transparently, though that would get awkward if a driver doesn't unlink
the remaining SHORT_NOT_OK URBs - we would need to "rewind".

On the upside, I don't see many users of SHORT_NOT_OK URB. This string
appears in Documentation, include, drivers/usb/host, files ending wih
hcd.c, devio/usbip, and usb_sg_init(). The latter is probably what you
were referring to, and it seems to have a shortcut for SG-capable HCs,
which hopefully doesn't require SHORT_NOT_OK to be functional?

Regards,
Michal

^ permalink raw reply

* Re: xhci-hcd and URB_SHORT_NOT_OK
From: stern @ 2026-04-03 20:17 UTC (permalink / raw)
  To: Michal Pecio
  Cc: Thinh Nguyen, Mathias Nyman, linux-usb@vger.kernel.org,
	oneukum@suse.com, niklas.neronin@linux.intel.com
In-Reply-To: <20260403211337.7b964af2.michal.pecio@gmail.com>

On Fri, Apr 03, 2026 at 09:13:37PM +0200, Michal Pecio wrote:
> On Fri, 3 Apr 2026 10:55:58 -0400, stern@rowland.harvard.edu wrote:
> > > This doesn't work in xhci-hcd and I'm not sure if it can. I don't
> > > recall any way to make endpoints halt on short transfer at all.
> > > 
> > > The driver does exactly two things with this flag:
> > > 1. Isochronus frames get EREMOTEIO status when it's detected,
> > >    although ISTR that URB_SHORT_NOT_OK isn't defined for isochronous
> > > 2. All others log a dbg() message. Supposedly, core is
> > >    responsible for updating urb->status then.  
> > 
> > Truly?  What happens if usb-storage is doing a large bulk-IN transfer
> > comprising multiple data URBs (with URB_SHORT_NOT_OK set in all but
> > the last one), and the device indicates an early end-of-data by
> > sending a short packet in the middle?  Will xhci-hcd store the
> > information in the following bulk-IN status transaction to the
> > transfer buffer for the next URB in the queue of data URBs?
> 
> Not sure if I understand the question, but if you mean that the driver
> submits multiple "data" URBs followed by a "status" URB to the same
> pipe and expects it to stop when any data URB completes short, so that
> all others can be unlinked before the status transfer, then nope, not
> gonna happen. It will be racing with status being written to the data
> URBs and likely lose. The status URB may never complete.
> 
> I really don't recall any way of stopping on short transfers and the
> relevant xHCI 4.10.1.1 section isn't helpful. It's possible to stop
> unconditionally and decide to restart or not, at some performance cost.
> 
> But short transfer always terminates the current URB, so natural way is
> to coalesce all data into a big SG URB. Perhaps xhci-hcd could do it
> transparently, though that would get awkward if a driver doesn't unlink
> the remaining SHORT_NOT_OK URBs - we would need to "rewind".
> 
> On the upside, I don't see many users of SHORT_NOT_OK URB. This string
> appears in Documentation, include, drivers/usb/host, files ending wih
> hcd.c, devio/usbip, and usb_sg_init(). The latter is probably what you
> were referring to, and it seems to have a shortcut for SG-capable HCs,
> which hopefully doesn't require SHORT_NOT_OK to be functional?

usb_sg_init() _is_ what I was thinking of, and I had forgotten about the 
single-URB shortcut for SG-capable HCs.

Clearly we're also going to have to update the kerneldoc, not just the 
code.  If an HCD doesn't support SG then it needs to support stopping an 
endpoint on short transfers.

Alan Stern

^ permalink raw reply

* [Bug 221318] New: mice behind ASMedia ASM1042A via Thunderbolt 2 never produce input, most likely due to interrupt pipe idle window during enumeration
From: bugzilla-daemon @ 2026-04-03 20:22 UTC (permalink / raw)
  To: linux-usb

https://bugzilla.kernel.org/show_bug.cgi?id=221318

            Bug ID: 221318
           Summary: mice behind ASMedia ASM1042A via Thunderbolt 2 never
                    produce input, most likely due to interrupt pipe idle
                    window during enumeration
           Product: Drivers
           Version: 2.5
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: high
          Priority: P3
         Component: USB
          Assignee: drivers_usb@kernel-bugs.kernel.org
          Reporter: manauer.uel@gmail.com
        Regression: No

This is filed under drivers/usb because the problem involves the interaction
between the usbhid driver, the USB host controller, and the Thunderbolt 2
bridge. The drivers/hid/usbhid subsystem is also directly involved,
specifically the behavior of the interrupt pipe after enumeration. There is no
drivers/thunderbolt component available to select, but the Thunderbolt 2 bridge
appears to be a contributing factor as described below.

On a MacBook Pro 13" Early 2015 (MacBookPro12,1) with an LG UltraWide (34UC98,
internal hub: ASMedia ASM1042A) monitor connected via Thunderbolt 2, USB mice
connected through the monitor's built-in hub never produce any input. This
happens consistently, whether the mouse is plugged in before boot or after. The
mouse is fully enumerated, appears in lsusb, has a correct
/dev/input/by-id/...-event-mouse node, and is recognized by libinput, but no
motion or button events are produced.


# Affected hardware:
- Machine: MacBook Pro 13" Early 2015 (MacBookPro12,1)
- Monitor: LG UltraWide (34UC98), connected via Thunderbolt 2
- USB host controller: ASMedia ASM1042A, PCI ID 1b21:1142
- Affected: any USB mouse connected through the monitor hub
- Not affected: keyboards on the same hub, mice connected to the same monitor
via USB instead of Thunderbolt 2


# Kernel versions tested:
Reproduces on Fedora 43 (6.17), and on a Rawhide kernel (7.0.0-0.rc4). Also
reproduces on CachyOS, Ubuntu and Pop!_OS, so it is not distribution-specific.


# Theory on why this is happening:
After enumeration, the usbhid driver stops submitting interrupt URBs if no
userspace application has opened the device yet. There is a brief idle window
before the desktop environment opens /dev/hidrawX. The ASMedia TT appears to be
sensitive to this idle condition when the hub is accessed through the
Thunderbolt 2 bridge. Once the interrupt pipe goes quiet, the TT most likely
stops delivering input silently and does not recover, even after userspace
opens the device. The same hub connected via USB does not exhibit this
behavior, pointing to the Thunderbolt 2 bridge as a contributing factor. Why
the Thunderbolt 2 bridge makes the difference is not clear to me.

One additional observation worth mentioning: plugging in an unrelated Logitech
wireless receiver into the monitor hub suddenly makes the wired mouse work
(re-plugging needed). The receiver belongs to a completely different wireless
mouse and has no logical connection to the wired mice. It prevents the problem,
though why exactly is unclear. The additional USB traffic it puts on the
controller might be the reason, but I am not sure.

Applying HID_QUIRK_ALWAYS_POLL via the kernel command line fixes the problem
for a specific mouse: usbhid.quirks=0x046d:0xc049:0x00000400

This strongly suggests the problem is the interrupt pipe going idle. As a
generic workaround, holding a file descriptor open on /dev/hidrawX via a udev
rule has the same effect and works for any mouse without needing vendor and
product IDs

Maybe consider keeping the interrupt pipe active by default for all HID mice
until userspace opens the device, or automatically applying
HID_QUIRK_ALWAYS_POLL for mice behind Thunderbolt bridges.

A workaround and write-up are available at:
https://github.com/NextBlaubeere/asmedia-usb-mouse-fix

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* [PATCH v1 0/2] Load cros_ec_ucsi from OF and ACPI nodes
From: Jameson Thies @ 2026-04-03 22:33 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, abhishekpandit, bleung, heikki.krogerus,
	akuchynski
  Cc: gregkh, devicetree, chrome-platform, linux-usb, linux-kernel,
	Jameson Thies

Hey everyone,
I sent these patches up as part of a larger series in late 2025 which
hasn't mergerd. The initial series modified dt-bindings, mfd and usb
subsystems, but had not been reviewed by mfd maintainers. Additionally,
it had not been acked by maintainers all land through one development
tree.

For context, here is the previous series
https://lore.kernel.org/all/20251013203331.398517-1-jthies@google.com/

I've split the series up, this is just the usb and dt-bindings update.
It builds independent from the mfd change.

Given the time since this was previously under review (~oct 2025), I've
cleared the exising review tags. Please take a look and let me know if
you have any questions.

Jameson Thies (2):
  dt-bindings: chrome: Add cros-ec-ucsi compatibility to typec binding
  usb: typec: cros_ec_ucsi: Load driver from OF and ACPI definitions

 .../bindings/chrome/google,cros-ec-typec.yaml | 19 +++++++++++---
 drivers/usb/typec/ucsi/cros_ec_ucsi.c         | 26 +++++++++++++++++--
 2 files changed, 39 insertions(+), 6 deletions(-)


base-commit: 81ebd43cc0d6d106ce7b6ccbf7b5e40ca7f5503d
-- 
2.53.0.1213.gd9a14994de-goog


^ permalink raw reply

* [PATCH v1 1/2] dt-bindings: chrome: Add cros-ec-ucsi compatibility to typec binding
From: Jameson Thies @ 2026-04-03 22:33 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, abhishekpandit, bleung, heikki.krogerus,
	akuchynski
  Cc: gregkh, devicetree, chrome-platform, linux-usb, linux-kernel,
	Jameson Thies
In-Reply-To: <20260403223357.1896403-1-jthies@google.com>

Chrome OS devices with discrete power delivery controllers (PDCs) allow
the host to read port status and control port behavior through a USB
Type-C Connector System Software (UCSI) interface with the embedded
controller (EC). This uses a separate interface driver than other
Chrome OS devices with a Type-C port manager in the EC FW. Those use
a host command interface supported by cros-ec-typec. Add a cros-ec-ucsi
compatibility string to the existing cros-ec-typec binding.

Additionally, update maintainer list to reflect cros-ec-ucsi and
cros-ec-typec driver maintainers.

Signed-off-by: Jameson Thies <jthies@google.com>
---
 .../bindings/chrome/google,cros-ec-typec.yaml | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
index 9f9816fbecbc..fd1a459879bd 100644
--- a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
+++ b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
@@ -8,17 +8,28 @@ title: Google Chrome OS EC(Embedded Controller) Type C port driver.
 
 maintainers:
   - Benson Leung <bleung@chromium.org>
-  - Prashant Malani <pmalani@chromium.org>
+  - Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
+  - Andrei Kuchynski <akuchynski@chromium.org>
+  - Łukasz Bartosik <ukaszb@chromium.org>
+  - Jameson Thies <jthies@google.com>
 
 description:
   Chrome OS devices have an Embedded Controller(EC) which has access to
   Type C port state. This node is intended to allow the host to read and
-  control the Type C ports. The node for this device should be under a
-  cros-ec node like google,cros-ec-spi.
+  control the Type C ports. This binding is compatible with both the
+  cros-ec-typec and cros-ec-ucsi drivers. The cros-ec-typec driver
+  supports the host command interface used by the Chrome OS EC with a
+  built-in Type-C port manager and external Type-C Port Controller
+  (TCPC). The cros-ec-ucsi driver supports the USB Type-C Connector
+  System Software (UCSI) interface used by the Chrome OS EC when the
+  platform has a separate power delivery controller (PDC). The node for
+  this device should be under a cros-ec node like google,cros-ec-spi.
 
 properties:
   compatible:
-    const: google,cros-ec-typec
+    enum:
+      - google,cros-ec-typec
+      - google,cros-ec-ucsi
 
   '#address-cells':
     const: 1
-- 
2.53.0.1213.gd9a14994de-goog


^ permalink raw reply related

* [PATCH v1 2/2] usb: typec: cros_ec_ucsi: Load driver from OF and ACPI definitions
From: Jameson Thies @ 2026-04-03 22:33 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, abhishekpandit, bleung, heikki.krogerus,
	akuchynski
  Cc: gregkh, devicetree, chrome-platform, linux-usb, linux-kernel,
	Jameson Thies
In-Reply-To: <20260403223357.1896403-1-jthies@google.com>

Add support for cros_ec_ucsi to load based on "google,cros-ec-ucsi"
compatible devices and "GOOG0021" ACPI nodes.

Signed-off-by: Jameson Thies <jthies@google.com>
---
 drivers/usb/typec/ucsi/cros_ec_ucsi.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/ucsi/cros_ec_ucsi.c b/drivers/usb/typec/ucsi/cros_ec_ucsi.c
index 6bca2dce211c..251aa7251ce6 100644
--- a/drivers/usb/typec/ucsi/cros_ec_ucsi.c
+++ b/drivers/usb/typec/ucsi/cros_ec_ucsi.c
@@ -5,11 +5,13 @@
  * Copyright 2024 Google LLC.
  */
 
+#include <linux/acpi.h>
 #include <linux/container_of.h>
 #include <linux/dev_printk.h>
 #include <linux/jiffies.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_data/cros_ec_commands.h>
 #include <linux/platform_data/cros_usbpd_notify.h>
 #include <linux/platform_data/cros_ec_proto.h>
@@ -257,7 +259,6 @@ static void cros_ucsi_destroy(struct cros_ucsi_data *udata)
 static int cros_ucsi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct cros_ec_dev *ec_data = dev_get_drvdata(dev->parent);
 	struct cros_ucsi_data *udata;
 	int ret;
 
@@ -265,9 +266,16 @@ static int cros_ucsi_probe(struct platform_device *pdev)
 	if (!udata)
 		return -ENOMEM;
 
+	/* ACPI and OF FW nodes for cros_ec_ucsi are children of the ChromeOS EC. If the
+	 * cros_ec_ucsi device has an ACPI or OF FW node, its parent is the ChromeOS EC device.
+	 * Platforms without a FW node for cros_ec_ucsi may add it as a subdevice of cros_ec_dev.
+	 */
 	udata->dev = dev;
+	if (is_acpi_device_node(dev->fwnode) || is_of_node(dev->fwnode))
+		udata->ec = dev_get_drvdata(dev->parent);
+	else
+		udata->ec = ((struct cros_ec_dev *)dev_get_drvdata(dev->parent))->ec_dev;
 
-	udata->ec = ec_data->ec_dev;
 	if (!udata->ec)
 		return dev_err_probe(dev, -ENODEV, "couldn't find parent EC device\n");
 
@@ -348,10 +356,24 @@ static const struct platform_device_id cros_ucsi_id[] = {
 };
 MODULE_DEVICE_TABLE(platform, cros_ucsi_id);
 
+static const struct acpi_device_id cros_ec_ucsi_acpi_device_ids[] = {
+	{ "GOOG0021", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, cros_ec_ucsi_acpi_device_ids);
+
+static const struct of_device_id cros_ucsi_of_match[] = {
+	{ .compatible = "google,cros-ec-ucsi", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, cros_ucsi_of_match);
+
 static struct platform_driver cros_ucsi_driver = {
 	.driver = {
 		.name = KBUILD_MODNAME,
 		.pm = &cros_ucsi_pm_ops,
+		.acpi_match_table = cros_ec_ucsi_acpi_device_ids,
+		.of_match_table = cros_ucsi_of_match,
 	},
 	.id_table = cros_ucsi_id,
 	.probe = cros_ucsi_probe,
-- 
2.53.0.1213.gd9a14994de-goog


^ 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