* [PATCH v3 0/3] Fix cold plugged USB device on certain PCIe USB cards @ 2021-09-08 9:53 Kishon Vijay Abraham I 2021-09-08 9:53 ` [PATCH v3 1/3] usb: core: hcd: Add support for deferring roothub registration Kishon Vijay Abraham I ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Kishon Vijay Abraham I @ 2021-09-08 9:53 UTC (permalink / raw) To: Greg Kroah-Hartman, Mathias Nyman, Alan Stern Cc: linux-usb, linux-kernel, chris.chiu, Kishon Vijay Abraham I, lokeshvutla Cold plugged USB device was not detected on certain PCIe USB cards (like Inateck card connected to AM64 EVM or connected to J7200 EVM). Re-plugging the USB device always gets it enumerated. This issue was discussed in https://lore.kernel.org/r/772e4001-178e-4918-032c-6e625bdded24@ti.com and https://bugzilla.kernel.org/show_bug.cgi?id=214021 So the suggested solution is to register both root hubs along with the second hcd for xhci. RFC Patch series can be found at [1] v1 Patch series can be found at [3] v2 Patch series can be found at [4] Changes from RFC: 1) Mathias identified potential issues with the RFC patch [2] and suggested the solution to use HCD flags. This series implements it. Changes from v1: 1) Fixed code comments pointed out by Alan Stern 2) Renamed the HCD flag variable to "HCD_FLAG_DEFER_RH_REGISTER" from "HCD_FLAG_DEFER_PRI_RH_REGISTER" Changes from v2: 1) Fixed issue in error path handling pointed by Alan [1] -> https://lore.kernel.org/linux-usb/20210824105302.25382-1-kishon@ti.com/ [2] -> https://lore.kernel.org/linux-usb/06693934-28f2-d59e-b004-62cabd3f9e8e@linux.intel.com [3] -> https://lore.kernel.org/r/20210825105132.10420-1-kishon@ti.com [4] -> https://lore.kernel.org/r/20210826111426.751-1-kishon@ti.com Kishon Vijay Abraham I (3): usb: core: hcd: Add support for deferring roothub registration xhci: Set HCD flag to defer primary roothub registration usb: core: hcd: Modularize HCD stop configuration in usb_stop_hcd() drivers/usb/core/hcd.c | 69 +++++++++++++++++++++++++++-------------- drivers/usb/host/xhci.c | 1 + include/linux/usb/hcd.h | 2 ++ 3 files changed, 49 insertions(+), 23 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/3] usb: core: hcd: Add support for deferring roothub registration 2021-09-08 9:53 [PATCH v3 0/3] Fix cold plugged USB device on certain PCIe USB cards Kishon Vijay Abraham I @ 2021-09-08 9:53 ` Kishon Vijay Abraham I 2021-09-08 14:51 ` Alan Stern 2021-09-08 15:46 ` kernel test robot 2021-09-08 9:53 ` [PATCH v3 2/3] xhci: Set HCD flag to defer primary " Kishon Vijay Abraham I 2021-09-08 9:53 ` [PATCH v3 3/3] usb: core: hcd: Modularize HCD stop configuration in usb_stop_hcd() Kishon Vijay Abraham I 2 siblings, 2 replies; 7+ messages in thread From: Kishon Vijay Abraham I @ 2021-09-08 9:53 UTC (permalink / raw) To: Greg Kroah-Hartman, Mathias Nyman, Alan Stern Cc: linux-usb, linux-kernel, chris.chiu, Kishon Vijay Abraham I, lokeshvutla It has been observed with certain PCIe USB cards (like Inateck connected to AM64 EVM or J7200 EVM) that as soon as the primary roothub is registered, port status change is handled even before xHC is running leading to cold plug USB devices not detected. For such cases, registering both the root hubs along with the second HCD is required. Add support for deferring roothub registration in usb_add_hcd(), so that both primary and secondary roothubs are registered along with the second HCD. CC: stable@vger.kernel.org # 5.4+ Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com> Tested-by: Chris Chiu <chris.chiu@canonical.com> --- drivers/usb/core/hcd.c | 30 ++++++++++++++++++++++++------ include/linux/usb/hcd.h | 2 ++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 0f8b7c93310e..30bbf4ac4284 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2775,6 +2775,7 @@ int usb_add_hcd(struct usb_hcd *hcd, { int retval; struct usb_device *rhdev; + struct usb_hcd *shared_hcd; if (!hcd->skip_phy_initialization && usb_hcd_is_primary_hcd(hcd)) { hcd->phy_roothub = usb_phy_roothub_alloc(hcd->self.sysdev); @@ -2935,13 +2936,26 @@ int usb_add_hcd(struct usb_hcd *hcd, goto err_hcd_driver_start; } + /* starting here, usbcore will pay attention to the shared HCD roothub */ + shared_hcd = hcd->shared_hcd; + if (!usb_hcd_is_primary_hcd(hcd) && shared_hcd && HCD_DEFER_RH_REGISTER(shared_hcd)) { + retval = register_root_hub(shared_hcd); + if (retval != 0) + goto err_register_root_hub; + + if (shared_hcd->uses_new_polling && HCD_POLL_RH(shared_hcd)) + usb_hcd_poll_rh_status(shared_hcd); + } + /* starting here, usbcore will pay attention to this root hub */ - retval = register_root_hub(hcd); - if (retval != 0) - goto err_register_root_hub; + if (!HCD_DEFER_RH_REGISTER(hcd)) { + retval = register_root_hub(hcd); + if (retval != 0) + goto err_register_root_hub; - if (hcd->uses_new_polling && HCD_POLL_RH(hcd)) - usb_hcd_poll_rh_status(hcd); + if (hcd->uses_new_polling && HCD_POLL_RH(hcd)) + usb_hcd_poll_rh_status(hcd); + } return retval; @@ -2985,6 +2999,7 @@ EXPORT_SYMBOL_GPL(usb_add_hcd); void usb_remove_hcd(struct usb_hcd *hcd) { struct usb_device *rhdev = hcd->self.root_hub; + unsigned rh_registered; dev_info(hcd->self.controller, "remove, state %x\n", hcd->state); @@ -2995,6 +3010,8 @@ void usb_remove_hcd(struct usb_hcd *hcd) dev_dbg(hcd->self.controller, "roothub graceful disconnect\n"); spin_lock_irq (&hcd_root_hub_lock); + if (hcd->rh_registered == 1) + rh_registered = 1; hcd->rh_registered = 0; spin_unlock_irq (&hcd_root_hub_lock); @@ -3004,7 +3021,8 @@ void usb_remove_hcd(struct usb_hcd *hcd) cancel_work_sync(&hcd->died_work); mutex_lock(&usb_bus_idr_lock); - usb_disconnect(&rhdev); /* Sets rhdev to NULL */ + if (rh_registered == 1) + usb_disconnect(&rhdev); /* Sets rhdev to NULL */ mutex_unlock(&usb_bus_idr_lock); /* diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index 548a028f2dab..2c1fc9212cf2 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -124,6 +124,7 @@ struct usb_hcd { #define HCD_FLAG_RH_RUNNING 5 /* root hub is running? */ #define HCD_FLAG_DEAD 6 /* controller has died? */ #define HCD_FLAG_INTF_AUTHORIZED 7 /* authorize interfaces? */ +#define HCD_FLAG_DEFER_RH_REGISTER 8 /* Defer roothub registration */ /* The flags can be tested using these macros; they are likely to * be slightly faster than test_bit(). @@ -134,6 +135,7 @@ struct usb_hcd { #define HCD_WAKEUP_PENDING(hcd) ((hcd)->flags & (1U << HCD_FLAG_WAKEUP_PENDING)) #define HCD_RH_RUNNING(hcd) ((hcd)->flags & (1U << HCD_FLAG_RH_RUNNING)) #define HCD_DEAD(hcd) ((hcd)->flags & (1U << HCD_FLAG_DEAD)) +#define HCD_DEFER_RH_REGISTER(hcd) ((hcd)->flags & (1U << HCD_FLAG_DEFER_RH_REGISTER)) /* * Specifies if interfaces are authorized by default -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/3] usb: core: hcd: Add support for deferring roothub registration 2021-09-08 9:53 ` [PATCH v3 1/3] usb: core: hcd: Add support for deferring roothub registration Kishon Vijay Abraham I @ 2021-09-08 14:51 ` Alan Stern 2021-09-09 6:35 ` Kishon Vijay Abraham I 2021-09-08 15:46 ` kernel test robot 1 sibling, 1 reply; 7+ messages in thread From: Alan Stern @ 2021-09-08 14:51 UTC (permalink / raw) To: Kishon Vijay Abraham I Cc: Greg Kroah-Hartman, Mathias Nyman, linux-usb, linux-kernel, chris.chiu, lokeshvutla On Wed, Sep 08, 2021 at 03:23:10PM +0530, Kishon Vijay Abraham I wrote: > It has been observed with certain PCIe USB cards (like Inateck connected > to AM64 EVM or J7200 EVM) that as soon as the primary roothub is > registered, port status change is handled even before xHC is running > leading to cold plug USB devices not detected. For such cases, registering > both the root hubs along with the second HCD is required. Add support for > deferring roothub registration in usb_add_hcd(), so that both primary and > secondary roothubs are registered along with the second HCD. > > CC: stable@vger.kernel.org # 5.4+ > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com> > Tested-by: Chris Chiu <chris.chiu@canonical.com> > --- > drivers/usb/core/hcd.c | 30 ++++++++++++++++++++++++------ > include/linux/usb/hcd.h | 2 ++ > 2 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c > index 0f8b7c93310e..30bbf4ac4284 100644 > --- a/drivers/usb/core/hcd.c > +++ b/drivers/usb/core/hcd.c > @@ -2775,6 +2775,7 @@ int usb_add_hcd(struct usb_hcd *hcd, > { > int retval; > struct usb_device *rhdev; > + struct usb_hcd *shared_hcd; > > if (!hcd->skip_phy_initialization && usb_hcd_is_primary_hcd(hcd)) { > hcd->phy_roothub = usb_phy_roothub_alloc(hcd->self.sysdev); > @@ -2935,13 +2936,26 @@ int usb_add_hcd(struct usb_hcd *hcd, > goto err_hcd_driver_start; > } > > + /* starting here, usbcore will pay attention to the shared HCD roothub */ > + shared_hcd = hcd->shared_hcd; > + if (!usb_hcd_is_primary_hcd(hcd) && shared_hcd && HCD_DEFER_RH_REGISTER(shared_hcd)) { > + retval = register_root_hub(shared_hcd); > + if (retval != 0) > + goto err_register_root_hub; > + > + if (shared_hcd->uses_new_polling && HCD_POLL_RH(shared_hcd)) > + usb_hcd_poll_rh_status(shared_hcd); > + } > + > /* starting here, usbcore will pay attention to this root hub */ > - retval = register_root_hub(hcd); > - if (retval != 0) > - goto err_register_root_hub; > + if (!HCD_DEFER_RH_REGISTER(hcd)) { > + retval = register_root_hub(hcd); > + if (retval != 0) > + goto err_register_root_hub; > > - if (hcd->uses_new_polling && HCD_POLL_RH(hcd)) > - usb_hcd_poll_rh_status(hcd); > + if (hcd->uses_new_polling && HCD_POLL_RH(hcd)) > + usb_hcd_poll_rh_status(hcd); > + } > > return retval; > > @@ -2985,6 +2999,7 @@ EXPORT_SYMBOL_GPL(usb_add_hcd); > void usb_remove_hcd(struct usb_hcd *hcd) > { > struct usb_device *rhdev = hcd->self.root_hub; > + unsigned rh_registered; This really should be a bool, not an unsigned. (Relatively unimportant.) > dev_info(hcd->self.controller, "remove, state %x\n", hcd->state); > > @@ -2995,6 +3010,8 @@ void usb_remove_hcd(struct usb_hcd *hcd) > > dev_dbg(hcd->self.controller, "roothub graceful disconnect\n"); > spin_lock_irq (&hcd_root_hub_lock); > + if (hcd->rh_registered == 1) > + rh_registered = 1; Just say: rh_registered = hcd->rh_registered > hcd->rh_registered = 0; > spin_unlock_irq (&hcd_root_hub_lock); > > @@ -3004,7 +3021,8 @@ void usb_remove_hcd(struct usb_hcd *hcd) > cancel_work_sync(&hcd->died_work); > > mutex_lock(&usb_bus_idr_lock); > - usb_disconnect(&rhdev); /* Sets rhdev to NULL */ > + if (rh_registered == 1) Just say: if (rh_registered) > + usb_disconnect(&rhdev); /* Sets rhdev to NULL */ Didn't the compiler warn about using a possibly uninitialized variable? The overall logic looks okay, apart from this one point. Alan Stern > mutex_unlock(&usb_bus_idr_lock); > > /* > diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h > index 548a028f2dab..2c1fc9212cf2 100644 > --- a/include/linux/usb/hcd.h > +++ b/include/linux/usb/hcd.h > @@ -124,6 +124,7 @@ struct usb_hcd { > #define HCD_FLAG_RH_RUNNING 5 /* root hub is running? */ > #define HCD_FLAG_DEAD 6 /* controller has died? */ > #define HCD_FLAG_INTF_AUTHORIZED 7 /* authorize interfaces? */ > +#define HCD_FLAG_DEFER_RH_REGISTER 8 /* Defer roothub registration */ > > /* The flags can be tested using these macros; they are likely to > * be slightly faster than test_bit(). > @@ -134,6 +135,7 @@ struct usb_hcd { > #define HCD_WAKEUP_PENDING(hcd) ((hcd)->flags & (1U << HCD_FLAG_WAKEUP_PENDING)) > #define HCD_RH_RUNNING(hcd) ((hcd)->flags & (1U << HCD_FLAG_RH_RUNNING)) > #define HCD_DEAD(hcd) ((hcd)->flags & (1U << HCD_FLAG_DEAD)) > +#define HCD_DEFER_RH_REGISTER(hcd) ((hcd)->flags & (1U << HCD_FLAG_DEFER_RH_REGISTER)) > > /* > * Specifies if interfaces are authorized by default > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/3] usb: core: hcd: Add support for deferring roothub registration 2021-09-08 14:51 ` Alan Stern @ 2021-09-09 6:35 ` Kishon Vijay Abraham I 0 siblings, 0 replies; 7+ messages in thread From: Kishon Vijay Abraham I @ 2021-09-09 6:35 UTC (permalink / raw) To: Alan Stern Cc: Greg Kroah-Hartman, Mathias Nyman, linux-usb, linux-kernel, chris.chiu, lokeshvutla Hi Alan, On 08/09/21 8:21 pm, Alan Stern wrote: > On Wed, Sep 08, 2021 at 03:23:10PM +0530, Kishon Vijay Abraham I wrote: >> It has been observed with certain PCIe USB cards (like Inateck connected >> to AM64 EVM or J7200 EVM) that as soon as the primary roothub is >> registered, port status change is handled even before xHC is running >> leading to cold plug USB devices not detected. For such cases, registering >> both the root hubs along with the second HCD is required. Add support for >> deferring roothub registration in usb_add_hcd(), so that both primary and >> secondary roothubs are registered along with the second HCD. >> >> CC: stable@vger.kernel.org # 5.4+ >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com> >> Tested-by: Chris Chiu <chris.chiu@canonical.com> >> --- >> drivers/usb/core/hcd.c | 30 ++++++++++++++++++++++++------ >> include/linux/usb/hcd.h | 2 ++ >> 2 files changed, 26 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c >> index 0f8b7c93310e..30bbf4ac4284 100644 >> --- a/drivers/usb/core/hcd.c >> +++ b/drivers/usb/core/hcd.c >> @@ -2775,6 +2775,7 @@ int usb_add_hcd(struct usb_hcd *hcd, >> { >> int retval; >> struct usb_device *rhdev; >> + struct usb_hcd *shared_hcd; >> >> if (!hcd->skip_phy_initialization && usb_hcd_is_primary_hcd(hcd)) { >> hcd->phy_roothub = usb_phy_roothub_alloc(hcd->self.sysdev); >> @@ -2935,13 +2936,26 @@ int usb_add_hcd(struct usb_hcd *hcd, >> goto err_hcd_driver_start; >> } >> >> + /* starting here, usbcore will pay attention to the shared HCD roothub */ >> + shared_hcd = hcd->shared_hcd; >> + if (!usb_hcd_is_primary_hcd(hcd) && shared_hcd && HCD_DEFER_RH_REGISTER(shared_hcd)) { >> + retval = register_root_hub(shared_hcd); >> + if (retval != 0) >> + goto err_register_root_hub; >> + >> + if (shared_hcd->uses_new_polling && HCD_POLL_RH(shared_hcd)) >> + usb_hcd_poll_rh_status(shared_hcd); >> + } >> + >> /* starting here, usbcore will pay attention to this root hub */ >> - retval = register_root_hub(hcd); >> - if (retval != 0) >> - goto err_register_root_hub; >> + if (!HCD_DEFER_RH_REGISTER(hcd)) { >> + retval = register_root_hub(hcd); >> + if (retval != 0) >> + goto err_register_root_hub; >> >> - if (hcd->uses_new_polling && HCD_POLL_RH(hcd)) >> - usb_hcd_poll_rh_status(hcd); >> + if (hcd->uses_new_polling && HCD_POLL_RH(hcd)) >> + usb_hcd_poll_rh_status(hcd); >> + } >> >> return retval; >> >> @@ -2985,6 +2999,7 @@ EXPORT_SYMBOL_GPL(usb_add_hcd); >> void usb_remove_hcd(struct usb_hcd *hcd) >> { >> struct usb_device *rhdev = hcd->self.root_hub; >> + unsigned rh_registered; > > This really should be a bool, not an unsigned. (Relatively unimportant.) > >> dev_info(hcd->self.controller, "remove, state %x\n", hcd->state); >> >> @@ -2995,6 +3010,8 @@ void usb_remove_hcd(struct usb_hcd *hcd) >> >> dev_dbg(hcd->self.controller, "roothub graceful disconnect\n"); >> spin_lock_irq (&hcd_root_hub_lock); >> + if (hcd->rh_registered == 1) >> + rh_registered = 1; > > Just say: rh_registered = hcd->rh_registered > >> hcd->rh_registered = 0; >> spin_unlock_irq (&hcd_root_hub_lock); >> >> @@ -3004,7 +3021,8 @@ void usb_remove_hcd(struct usb_hcd *hcd) >> cancel_work_sync(&hcd->died_work); >> >> mutex_lock(&usb_bus_idr_lock); >> - usb_disconnect(&rhdev); /* Sets rhdev to NULL */ >> + if (rh_registered == 1) > > Just say: if (rh_registered) > >> + usb_disconnect(&rhdev); /* Sets rhdev to NULL */ > > Didn't the compiler warn about using a possibly uninitialized variable? I didn't see any. Possibly the warning levels are not correctly set in my setup. Will address your comment and send the patch. Thank You, Kishon > > The overall logic looks okay, apart from this one point. > > Alan Stern > >> mutex_unlock(&usb_bus_idr_lock); >> >> /* >> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h >> index 548a028f2dab..2c1fc9212cf2 100644 >> --- a/include/linux/usb/hcd.h >> +++ b/include/linux/usb/hcd.h >> @@ -124,6 +124,7 @@ struct usb_hcd { >> #define HCD_FLAG_RH_RUNNING 5 /* root hub is running? */ >> #define HCD_FLAG_DEAD 6 /* controller has died? */ >> #define HCD_FLAG_INTF_AUTHORIZED 7 /* authorize interfaces? */ >> +#define HCD_FLAG_DEFER_RH_REGISTER 8 /* Defer roothub registration */ >> >> /* The flags can be tested using these macros; they are likely to >> * be slightly faster than test_bit(). >> @@ -134,6 +135,7 @@ struct usb_hcd { >> #define HCD_WAKEUP_PENDING(hcd) ((hcd)->flags & (1U << HCD_FLAG_WAKEUP_PENDING)) >> #define HCD_RH_RUNNING(hcd) ((hcd)->flags & (1U << HCD_FLAG_RH_RUNNING)) >> #define HCD_DEAD(hcd) ((hcd)->flags & (1U << HCD_FLAG_DEAD)) >> +#define HCD_DEFER_RH_REGISTER(hcd) ((hcd)->flags & (1U << HCD_FLAG_DEFER_RH_REGISTER)) >> >> /* >> * Specifies if interfaces are authorized by default >> -- >> 2.17.1 >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/3] usb: core: hcd: Add support for deferring roothub registration 2021-09-08 9:53 ` [PATCH v3 1/3] usb: core: hcd: Add support for deferring roothub registration Kishon Vijay Abraham I 2021-09-08 14:51 ` Alan Stern @ 2021-09-08 15:46 ` kernel test robot 1 sibling, 0 replies; 7+ messages in thread From: kernel test robot @ 2021-09-08 15:46 UTC (permalink / raw) To: Kishon Vijay Abraham I, Greg Kroah-Hartman, Mathias Nyman, Alan Stern Cc: llvm, kbuild-all, linux-usb, linux-kernel, chris.chiu, Kishon Vijay Abraham I, lokeshvutla [-- Attachment #1: Type: text/plain, Size: 6273 bytes --] Hi Kishon, I love your patch! Perhaps something to improve: [auto build test WARNING on usb/usb-testing] [also build test WARNING on v5.14 next-20210908] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Kishon-Vijay-Abraham-I/Fix-cold-plugged-USB-device-on-certain-PCIe-USB-cards/20210908-175547 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing config: riscv-randconfig-r012-20210908 (attached as .config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 9c476172b93367d2cb88d7d3f4b1b5b456fa6020) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install riscv cross compiling tool for clang build # apt-get install binutils-riscv64-linux-gnu # https://github.com/0day-ci/linux/commit/03e83cc595ea0a47e7f25c7d7b541d704b06932c git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Kishon-Vijay-Abraham-I/Fix-cold-plugged-USB-device-on-certain-PCIe-USB-cards/20210908-175547 git checkout 03e83cc595ea0a47e7f25c7d7b541d704b06932c # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): In file included from drivers/usb/core/hcd.c:21: In file included from arch/riscv/include/asm/io.h:136: include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __raw_readb(PCI_IOBASE + addr); ~~~~~~~~~~ ^ include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr)); ~~~~~~~~~~ ^ include/uapi/linux/byteorder/little_endian.h:36:51: note: expanded from macro '__le16_to_cpu' #define __le16_to_cpu(x) ((__force __u16)(__le16)(x)) ^ In file included from drivers/usb/core/hcd.c:21: In file included from arch/riscv/include/asm/io.h:136: include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); ~~~~~~~~~~ ^ include/uapi/linux/byteorder/little_endian.h:34:51: note: expanded from macro '__le32_to_cpu' #define __le32_to_cpu(x) ((__force __u32)(__le32)(x)) ^ In file included from drivers/usb/core/hcd.c:21: In file included from arch/riscv/include/asm/io.h:136: include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writeb(value, PCI_IOBASE + addr); ~~~~~~~~~~ ^ include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr); ~~~~~~~~~~ ^ include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr); ~~~~~~~~~~ ^ include/asm-generic/io.h:1024:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port; ~~~~~~~~~~ ^ >> drivers/usb/core/hcd.c:3013:6: warning: variable 'rh_registered' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] if (hcd->rh_registered == 1) ^~~~~~~~~~~~~~~~~~~~~~~ drivers/usb/core/hcd.c:3024:6: note: uninitialized use occurs here if (rh_registered == 1) ^~~~~~~~~~~~~ drivers/usb/core/hcd.c:3013:2: note: remove the 'if' if its condition is always true if (hcd->rh_registered == 1) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/usb/core/hcd.c:3002:24: note: initialize the variable 'rh_registered' to silence this warning unsigned rh_registered; ^ = 0 8 warnings generated. vim +3013 drivers/usb/core/hcd.c 2989 2990 /** 2991 * usb_remove_hcd - shutdown processing for generic HCDs 2992 * @hcd: the usb_hcd structure to remove 2993 * 2994 * Context: task context, might sleep. 2995 * 2996 * Disconnects the root hub, then reverses the effects of usb_add_hcd(), 2997 * invoking the HCD's stop() method. 2998 */ 2999 void usb_remove_hcd(struct usb_hcd *hcd) 3000 { 3001 struct usb_device *rhdev = hcd->self.root_hub; 3002 unsigned rh_registered; 3003 3004 dev_info(hcd->self.controller, "remove, state %x\n", hcd->state); 3005 3006 usb_get_dev(rhdev); 3007 clear_bit(HCD_FLAG_RH_RUNNING, &hcd->flags); 3008 if (HC_IS_RUNNING (hcd->state)) 3009 hcd->state = HC_STATE_QUIESCING; 3010 3011 dev_dbg(hcd->self.controller, "roothub graceful disconnect\n"); 3012 spin_lock_irq (&hcd_root_hub_lock); > 3013 if (hcd->rh_registered == 1) 3014 rh_registered = 1; 3015 hcd->rh_registered = 0; 3016 spin_unlock_irq (&hcd_root_hub_lock); 3017 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 34797 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 2/3] xhci: Set HCD flag to defer primary roothub registration 2021-09-08 9:53 [PATCH v3 0/3] Fix cold plugged USB device on certain PCIe USB cards Kishon Vijay Abraham I 2021-09-08 9:53 ` [PATCH v3 1/3] usb: core: hcd: Add support for deferring roothub registration Kishon Vijay Abraham I @ 2021-09-08 9:53 ` Kishon Vijay Abraham I 2021-09-08 9:53 ` [PATCH v3 3/3] usb: core: hcd: Modularize HCD stop configuration in usb_stop_hcd() Kishon Vijay Abraham I 2 siblings, 0 replies; 7+ messages in thread From: Kishon Vijay Abraham I @ 2021-09-08 9:53 UTC (permalink / raw) To: Greg Kroah-Hartman, Mathias Nyman, Alan Stern Cc: linux-usb, linux-kernel, chris.chiu, Kishon Vijay Abraham I, lokeshvutla Set "HCD_FLAG_DEFER_RH_REGISTER" to hcd->flags in xhci_run() to defer registering primary roothub in usb_add_hcd(). This will make sure both primary roothub and secondary roothub will be registered along with the second HCD. This is required for cold plugged USB devices to be detected in certain PCIe USB cards (like Inateck USB card connected to AM64 EVM or J7200 EVM). CC: stable@vger.kernel.org # 5.4+ Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> Suggested-by: Mathias Nyman <mathias.nyman@linux.intel.com> Tested-by: Chris Chiu <chris.chiu@canonical.com> --- drivers/usb/host/xhci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 3618070eba78..7176058c7f3e 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -692,6 +692,7 @@ int xhci_run(struct usb_hcd *hcd) if (ret) xhci_free_command(xhci, command); } + set_bit(HCD_FLAG_DEFER_RH_REGISTER, &hcd->flags); xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Finished xhci_run for USB2 roothub"); -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 3/3] usb: core: hcd: Modularize HCD stop configuration in usb_stop_hcd() 2021-09-08 9:53 [PATCH v3 0/3] Fix cold plugged USB device on certain PCIe USB cards Kishon Vijay Abraham I 2021-09-08 9:53 ` [PATCH v3 1/3] usb: core: hcd: Add support for deferring roothub registration Kishon Vijay Abraham I 2021-09-08 9:53 ` [PATCH v3 2/3] xhci: Set HCD flag to defer primary " Kishon Vijay Abraham I @ 2021-09-08 9:53 ` Kishon Vijay Abraham I 2 siblings, 0 replies; 7+ messages in thread From: Kishon Vijay Abraham I @ 2021-09-08 9:53 UTC (permalink / raw) To: Greg Kroah-Hartman, Mathias Nyman, Alan Stern Cc: linux-usb, linux-kernel, chris.chiu, Kishon Vijay Abraham I, lokeshvutla No functional change. Since configuration to stop HCD is invoked from multiple places, group all of them in usb_stop_hcd(). Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> Acked-by: Alan Stern <stern@rowland.harvard.edu> Tested-by: Chris Chiu <chris.chiu@canonical.com> --- drivers/usb/core/hcd.c | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 30bbf4ac4284..6f72da245f69 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2760,6 +2760,26 @@ static void usb_put_invalidate_rhdev(struct usb_hcd *hcd) usb_put_dev(rhdev); } +/** + * usb_stop_hcd - Halt the HCD + * @hcd: the usb_hcd that has to be halted + * + * Stop the root-hub polling timer and invoke the HCD's ->stop callback. + */ +static void usb_stop_hcd(struct usb_hcd *hcd) +{ + hcd->rh_pollable = 0; + clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); + del_timer_sync(&hcd->rh_timer); + + hcd->driver->stop(hcd); + hcd->state = HC_STATE_HALT; + + /* In case the HCD restarted the timer, stop it again. */ + clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); + del_timer_sync(&hcd->rh_timer); +} + /** * usb_add_hcd - finish generic HCD structure initialization and register * @hcd: the usb_hcd structure to initialize @@ -2960,13 +2980,7 @@ int usb_add_hcd(struct usb_hcd *hcd, return retval; err_register_root_hub: - hcd->rh_pollable = 0; - clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); - del_timer_sync(&hcd->rh_timer); - hcd->driver->stop(hcd); - hcd->state = HC_STATE_HALT; - clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); - del_timer_sync(&hcd->rh_timer); + usb_stop_hcd(hcd); err_hcd_driver_start: if (usb_hcd_is_primary_hcd(hcd) && hcd->irq > 0) free_irq(irqnum, hcd); @@ -3040,16 +3054,7 @@ void usb_remove_hcd(struct usb_hcd *hcd) * interrupt occurs), but usb_hcd_poll_rh_status() won't invoke * the hub_status_data() callback. */ - hcd->rh_pollable = 0; - clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); - del_timer_sync(&hcd->rh_timer); - - hcd->driver->stop(hcd); - hcd->state = HC_STATE_HALT; - - /* In case the HCD restarted the timer, stop it again. */ - clear_bit(HCD_FLAG_POLL_RH, &hcd->flags); - del_timer_sync(&hcd->rh_timer); + usb_stop_hcd(hcd); if (usb_hcd_is_primary_hcd(hcd)) { if (hcd->irq > 0) -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-09-09 6:52 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-08 9:53 [PATCH v3 0/3] Fix cold plugged USB device on certain PCIe USB cards Kishon Vijay Abraham I 2021-09-08 9:53 ` [PATCH v3 1/3] usb: core: hcd: Add support for deferring roothub registration Kishon Vijay Abraham I 2021-09-08 14:51 ` Alan Stern 2021-09-09 6:35 ` Kishon Vijay Abraham I 2021-09-08 15:46 ` kernel test robot 2021-09-08 9:53 ` [PATCH v3 2/3] xhci: Set HCD flag to defer primary " Kishon Vijay Abraham I 2021-09-08 9:53 ` [PATCH v3 3/3] usb: core: hcd: Modularize HCD stop configuration in usb_stop_hcd() Kishon Vijay Abraham I
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).