| Message ID | 506113fe-5bd9-bdd0-7858-2b702ca32d53@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | usb: host: xhci-plat: omit shared hcd if either of the root hubs has no ports | expand |
Hi, with v5.19-rc1 (which includes this patch) I encounter a NULL pointer exception during system resume on a SC7280 board, which has an USB2-only HCD: [ 40.490107] Internal error: Oops: 96000005 [#1] PREEMPT SMP [ 40.490121] coresight-dynamic-funnel 6b04000.funnel: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0 [ 40.495836] Modules linked in: veth rfcomm algif_hash algif_skcipher af_alg uinput venus_enc venus_dec videobuf2_dma_contig videobuf2_memops cros_ec_typec xt_MASQUERADE typec hci_uart btqca venus_core v4l2_mem2mem videobuf2_v4l2 qcom_q6v5_mss videobuf2_common qcom_pil_info qcom_q6v5 ipa qcom_common rmtfs_mem [ 40.506420] coresight-dynamic-funnel 6b04000.funnel: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs [ 40.506427] ip6table_nat fuse 8021q cdc_ether usbnet cfg80211 bluetooth ecdh_generic ecc r8152 mii joydev [ 40.534424] CPU: 4 PID: 68 Comm: kworker/u16:2 Not tainted 5.19.0-rc1 #160 01dfc77dff686f7aa36c93a01f531f57c578e1d9 [ 40.534433] Hardware name: Google Herobrine (rev1+) (DT) [ 40.544539] coresight-tmc 6b05000.etf: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0 [ 40.554421] Workqueue: events_unbound async_run_entry_fn [ 40.554435] pstate: 00400009 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 40.554441] pc : usb_hcd_is_primary_hcd+0x10/0x30 [ 40.556483] usb usb2: PM: usb_dev_resume+0x0/0x2c returned 0 after 123459 usecs [ 40.556698] usb 2-1: PM: calling usb_dev_resume+0x0/0x2c @ 3480, parent: usb2 [ 40.565157] coresight-tmc 6b05000.etf: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs [ 40.570596] lr : xhci_init+0x24/0xf8 [ 40.570604] sp : ffffffc008873c30 [ 40.570606] x29: ffffffc008873c30 x28: ffffffdb82157000 x27: 0000000000000402 [ 40.570615] x26: ffffff8080040838 x25: ffffff8080906a10 x24: 0000000000000002 [ 40.579923] coresight-etm4x 7040000.etm: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0 [ 40.585363] [ 40.585365] x23: ffffffdb82157000 x22: 0000000000000000 x21: 0000000000000000 [ 40.585372] x20: 0000000000000000 x19: ffffff8081f9c000 x18: 0000000000000800 [ 40.585378] x17: 0000000000000354 x16: ffffffdb82162cf8 x15: fffffffe020377c8 [ 40.592549] coresight-etm4x 7040000.etm: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs [ 40.597370] x14: 0000000000000000 x13: 0000000062a0f74e x12: 0000000000000018 [ 40.597378] x11: 0000000080200000 x10: fffffffe02037820 x9 : ffffffdb817e4490 [ 40.597385] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 000000000000003f [ 40.604934] coresight-etm4x 7140000.etm: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0 [ 40.612230] x5 : 0000000080200018 x4 : fffffffe02037820 x3 : 0000000080200018 [ 40.612237] x2 : ffffff8080de0400 x1 : 0000000000000000 x0 : 0000000000000000 [ 40.612244] Call trace: [ 40.612247] usb_hcd_is_primary_hcd+0x10/0x30 [ 40.621107] coresight-etm4x 7140000.etm: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs [ 40.624769] xhci_resume+0x378/0x5a8 [ 40.624775] xhci_plat_resume+0x70/0xac [ 40.624783] platform_pm_resume+0x44/0x58 [ 40.628215] coresight-etm4x 7240000.etm: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0 [ 40.635529] dpm_run_callback+0x54/0x1a0 [ 40.635538] device_resume+0x220/0x23c [ 40.635545] async_resume+0x34/0x84 [ 40.642885] coresight-etm4x 7240000.etm: PM: pm_generic_resume+0x0/0x48 returned 0 after 0 usecs [ 40.644448] usb usb3: PM: usb_dev_resume+0x0/0x2c returned 0 after 211490 usecs [ 40.644660] usb 3-1: PM: calling usb_dev_resume+0x0/0x2c @ 3486, parent: usb3 [ 40.652340] async_run_entry_fn+0x30/0xd8 [ 40.652349] process_one_work+0x190/0x3d0 [ 40.652356] worker_thread+0x230/0x3d4 [ 40.652361] kthread+0x104/0x2d0 [ 40.653923] coresight-etm4x 7340000.etm: PM: calling pm_generic_resume+0x0/0x48 @ 3467, parent: soc@0 [ 40.661229] ret_from_fork+0x10/0x20 [ 40.661243] Code: d503245f aa1e03e9 d503201f d503233f (f9413808) [ 40.661247] ---[ end trace 0000000000000000 ]--- [ 40.681525] Kernel panic - not syncing: Oops: Fatal exception [ 40.681529] SMP: stopping secondary CPUs [ 40.684977] Kernel Offset: 0x1b79000000 from 0xffffffc008000000 [ 40.684980] PHYS_OFFSET: 0x80000000 [ 40.684982] CPU features: 0x800,0002e015,19801c82 [ 40.684987] Memory Limit: none In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(), however this field isn't initialized by __usb_create_hcd() for a HCD without secondary controller. On Wed, Mar 16, 2022 at 11:11:33PM +0100, Heiner Kallweit wrote: > This patch prepares xhci-plat for the following scenario > - If either of the root hubs has no ports, then omit shared hcd > - Main hcd can be USB3 if there are no USB2 ports > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/usb/host/xhci-plat.c | 31 ++++++++++++++++++------------- > 1 file changed, 18 insertions(+), 13 deletions(-) > > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c > index 5d752b384..c512ec214 100644 > --- a/drivers/usb/host/xhci-plat.c > +++ b/drivers/usb/host/xhci-plat.c > @@ -180,7 +180,7 @@ static int xhci_plat_probe(struct platform_device *pdev) > struct device *sysdev, *tmpdev; > struct xhci_hcd *xhci; > struct resource *res; > - struct usb_hcd *hcd; > + struct usb_hcd *hcd, *usb3_hcd; > int ret; > int irq; > struct xhci_plat_priv *priv = NULL; > @@ -327,21 +327,26 @@ static int xhci_plat_probe(struct platform_device *pdev) > if (ret) > goto disable_usb_phy; > > - xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev, > - dev_name(&pdev->dev), hcd); > - if (!xhci->shared_hcd) { > - ret = -ENOMEM; > - goto dealloc_usb2_hcd; > - } > + if (!xhci_has_one_roothub(xhci)) { > + xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev, > + dev_name(&pdev->dev), hcd); > + if (!xhci->shared_hcd) { > + ret = -ENOMEM; > + goto dealloc_usb2_hcd; > + } > > - xhci->shared_hcd->tpl_support = hcd->tpl_support; > + xhci->shared_hcd->tpl_support = hcd->tpl_support; > + } > > - if (HCC_MAX_PSA(xhci->hcc_params) >= 4) > - xhci->shared_hcd->can_do_streams = 1; > + usb3_hcd = xhci_get_usb3_hcd(xhci); > + if (usb3_hcd && HCC_MAX_PSA(xhci->hcc_params) >= 4) > + usb3_hcd->can_do_streams = 1; > > - ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED); > - if (ret) > - goto put_usb3_hcd; > + if (xhci->shared_hcd) { > + ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED); > + if (ret) > + goto put_usb3_hcd; > + } > > device_enable_async_suspend(&pdev->dev); > pm_runtime_put_noidle(&pdev->dev);
On 8.6.2022 23.55, Matthias Kaehlcke wrote: > Hi, > > with v5.19-rc1 (which includes this patch) I encounter a NULL pointer > exception during system resume on a SC7280 board, which has an > USB2-only HCD: > ... > > In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(), > however this field isn't initialized by __usb_create_hcd() for a HCD > without secondary controller. Thanks for debugging this, I'll write a patch for it. Can you try it out? -Mathias
On Thu, Jun 09, 2022 at 02:38:45PM +0300, Mathias Nyman wrote: > On 8.6.2022 23.55, Matthias Kaehlcke wrote: > > Hi, > > > > with v5.19-rc1 (which includes this patch) I encounter a NULL pointer > > exception during system resume on a SC7280 board, which has an > > USB2-only HCD: > > > ... > > > > In the re-init path xhci_resume() passes 'hcd->primary_hcd' to hci_init(), > > however this field isn't initialized by __usb_create_hcd() for a HCD > > without secondary controller. > > Thanks for debugging this, I'll write a patch for it. > Can you try it out? Sure!
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index 5d752b384..c512ec214 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -180,7 +180,7 @@ static int xhci_plat_probe(struct platform_device *pdev) struct device *sysdev, *tmpdev; struct xhci_hcd *xhci; struct resource *res; - struct usb_hcd *hcd; + struct usb_hcd *hcd, *usb3_hcd; int ret; int irq; struct xhci_plat_priv *priv = NULL; @@ -327,21 +327,26 @@ static int xhci_plat_probe(struct platform_device *pdev) if (ret) goto disable_usb_phy; - xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev, - dev_name(&pdev->dev), hcd); - if (!xhci->shared_hcd) { - ret = -ENOMEM; - goto dealloc_usb2_hcd; - } + if (!xhci_has_one_roothub(xhci)) { + xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev, + dev_name(&pdev->dev), hcd); + if (!xhci->shared_hcd) { + ret = -ENOMEM; + goto dealloc_usb2_hcd; + } - xhci->shared_hcd->tpl_support = hcd->tpl_support; + xhci->shared_hcd->tpl_support = hcd->tpl_support; + } - if (HCC_MAX_PSA(xhci->hcc_params) >= 4) - xhci->shared_hcd->can_do_streams = 1; + usb3_hcd = xhci_get_usb3_hcd(xhci); + if (usb3_hcd && HCC_MAX_PSA(xhci->hcc_params) >= 4) + usb3_hcd->can_do_streams = 1; - ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED); - if (ret) - goto put_usb3_hcd; + if (xhci->shared_hcd) { + ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED); + if (ret) + goto put_usb3_hcd; + } device_enable_async_suspend(&pdev->dev); pm_runtime_put_noidle(&pdev->dev);
This patch prepares xhci-plat for the following scenario - If either of the root hubs has no ports, then omit shared hcd - Main hcd can be USB3 if there are no USB2 ports Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/usb/host/xhci-plat.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-)