| Message ID | 20230321072946.935211-1-haotienh@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] usb: xhci: tegra: fix sleep in atomic call | expand |
On Tue, Mar 21, 2023 at 03:29:46PM +0800, Haotien Hsu wrote: > From: Wayne Chang <waynec@nvidia.com> > > When we set the OTG port to Host mode, we observed the following splat: What "OTG" port? That is not a USB thing anymore at all. How is this part of a xhci controller? > [ 167.057718] BUG: sleeping function called from invalid context at > include/linux/sched/mm.h:229 > [ 167.057872] Workqueue: events tegra_xusb_usb_phy_work > [ 167.057954] Call trace: > [ 167.057962] dump_backtrace+0x0/0x210 > [ 167.057996] show_stack+0x30/0x50 > [ 167.058020] dump_stack_lvl+0x64/0x84 > [ 167.058065] dump_stack+0x14/0x34 > [ 167.058100] __might_resched+0x144/0x180 > [ 167.058140] __might_sleep+0x64/0xd0 > [ 167.058171] slab_pre_alloc_hook.constprop.0+0xa8/0x110 > [ 167.058202] __kmalloc_track_caller+0x74/0x2b0 > [ 167.058233] kvasprintf+0xa4/0x190 > [ 167.058261] kasprintf+0x58/0x90 > [ 167.058285] tegra_xusb_find_port_node.isra.0+0x58/0xd0 > [ 167.058334] tegra_xusb_find_port+0x38/0xa0 > [ 167.058380] tegra_xusb_padctl_get_usb3_companion+0x38/0xd0 > [ 167.058430] tegra_xhci_id_notify+0x8c/0x1e0 > [ 167.058473] notifier_call_chain+0x88/0x100 > [ 167.058506] atomic_notifier_call_chain+0x44/0x70 > [ 167.058537] tegra_xusb_usb_phy_work+0x60/0xd0 > [ 167.058581] process_one_work+0x1dc/0x4c0 > [ 167.058618] worker_thread+0x54/0x410 > [ 167.058650] kthread+0x188/0x1b0 > [ 167.058672] ret_from_fork+0x10/0x20 > > The function tegra_xusb_padctl_get_usb3_companion eventually calls > tegra_xusb_find_port and this in turn calls kasprintf which might sleep > and so cannot be called from an atomic context. > > Fix this by moving the call to tegra_xusb_padctl_get_usb3_companion to > the tegra_xhci_id_work function where it is really needed. > > Fixes: f836e7843036 ("usb: xhci-tegra: Add OTG support") > Cc: stable@vger.kernel.org > Signed-off-by: Wayne Chang <waynec@nvidia.com> > Signed-off-by: Haotien Hsu <haotienh@nvidia.com> > --- > V3 -> V4: Remove copyright change from this patch > V2 -> V3: Add version information > V1 -> V2: Add "Fixes" and "Cc:" lines and update copyright years > --- > drivers/usb/host/xhci-tegra.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c > index 1ff22f675930..b40e897ec092 100644 > --- a/drivers/usb/host/xhci-tegra.c > +++ b/drivers/usb/host/xhci-tegra.c > @@ -1360,6 +1360,10 @@ static void tegra_xhci_id_work(struct work_struct *work) > > mutex_unlock(&tegra->lock); > > + tegra->otg_usb3_port = tegra_xusb_padctl_get_usb3_companion( > + tegra->padctl, > + tegra->otg_usb2_port); > + You have 100 columns, please use them, ending a line with "(" is not generally a good idea. thanks, greg k-h
On Thu, 2023-03-23 at 17:22 +0100, Greg Kroah-Hartman wrote: > External email: Use caution opening links or attachments > > > On Tue, Mar 21, 2023 at 03:29:46PM +0800, Haotien Hsu wrote: > > From: Wayne Chang <waynec@nvidia.com> > > > > When we set the OTG port to Host mode, we observed the following > > splat: > > What "OTG" port? That is not a USB thing anymore at all. How is > this > part of a xhci controller? I will use 'dual-role port' instead. When the role of a super-speed port is switched, we need to clear or set port power. Our host mode controller and device mode controller share the same super-speed port hardware. When a SS port is going to be switched to device mode role, we have to clear PP at host mode controller side to release SS port hardware to device mode controller. When a SS port is going to be switched to host mode role, we have to set PP at host mode controller side to acquire SS port hardware. > > > [ 167.057718] BUG: sleeping function called from invalid context > > at > > include/linux/sched/mm.h:229 > > [ 167.057872] Workqueue: events tegra_xusb_usb_phy_work > > [ 167.057954] Call trace: > > [ 167.057962] dump_backtrace+0x0/0x210 > > [ 167.057996] show_stack+0x30/0x50 > > [ 167.058020] dump_stack_lvl+0x64/0x84 > > [ 167.058065] dump_stack+0x14/0x34 > > [ 167.058100] __might_resched+0x144/0x180 > > [ 167.058140] __might_sleep+0x64/0xd0 > > [ 167.058171] slab_pre_alloc_hook.constprop.0+0xa8/0x110 > > [ 167.058202] __kmalloc_track_caller+0x74/0x2b0 > > [ 167.058233] kvasprintf+0xa4/0x190 > > [ 167.058261] kasprintf+0x58/0x90 > > [ 167.058285] tegra_xusb_find_port_node.isra.0+0x58/0xd0 > > [ 167.058334] tegra_xusb_find_port+0x38/0xa0 > > [ 167.058380] tegra_xusb_padctl_get_usb3_companion+0x38/0xd0 > > [ 167.058430] tegra_xhci_id_notify+0x8c/0x1e0 > > [ 167.058473] notifier_call_chain+0x88/0x100 > > [ 167.058506] atomic_notifier_call_chain+0x44/0x70 > > [ 167.058537] tegra_xusb_usb_phy_work+0x60/0xd0 > > [ 167.058581] process_one_work+0x1dc/0x4c0 > > [ 167.058618] worker_thread+0x54/0x410 > > [ 167.058650] kthread+0x188/0x1b0 > > [ 167.058672] ret_from_fork+0x10/0x20 > > > > The function tegra_xusb_padctl_get_usb3_companion eventually calls > > tegra_xusb_find_port and this in turn calls kasprintf which might > > sleep > > and so cannot be called from an atomic context. > > > > Fix this by moving the call to tegra_xusb_padctl_get_usb3_companion > > to > > the tegra_xhci_id_work function where it is really needed. > > > > Fixes: f836e7843036 ("usb: xhci-tegra: Add OTG support") > > Cc: stable@vger.kernel.org > > Signed-off-by: Wayne Chang <waynec@nvidia.com> > > Signed-off-by: Haotien Hsu <haotienh@nvidia.com> > > --- > > V3 -> V4: Remove copyright change from this patch > > V2 -> V3: Add version information > > V1 -> V2: Add "Fixes" and "Cc:" lines and update copyright years > > --- > > drivers/usb/host/xhci-tegra.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci- > > tegra.c > > index 1ff22f675930..b40e897ec092 100644 > > --- a/drivers/usb/host/xhci-tegra.c > > +++ b/drivers/usb/host/xhci-tegra.c > > @@ -1360,6 +1360,10 @@ static void tegra_xhci_id_work(struct > > work_struct *work) > > > > mutex_unlock(&tegra->lock); > > > > + tegra->otg_usb3_port = tegra_xusb_padctl_get_usb3_companion( > > + tegra- > > >padctl, > > + tegra- > > >otg_usb2_port); > > + > > You have 100 columns, please use them, ending a line with "(" is not > generally a good idea. I will resubmit a new patch. > > thanks, > > greg k-h
On Fri, 2023-03-24 at 06:46 +0000, Haotien Hsu wrote: > On Thu, 2023-03-23 at 17:22 +0100, Greg Kroah-Hartman wrote: > > External email: Use caution opening links or attachments > > > > > > On Tue, Mar 21, 2023 at 03:29:46PM +0800, Haotien Hsu wrote: > > > From: Wayne Chang <waynec@nvidia.com> > > > > > > When we set the OTG port to Host mode, we observed the following > > > splat: > > > > What "OTG" port? That is not a USB thing anymore at all. How is > > this > > part of a xhci controller? > > I will use 'dual-role port' instead. > > When the role of a super-speed port is switched, we need to clear or > set XHCI PORTSC.PP. > Our host mode controller and device mode controller share the same > super-speed port hardware. > When a super-speed port is going to be switched to device mode role, > we have to clear XHCI PORTSC.PP at host mode controller side to > release super-speed port hardware to device mode controller. > When a super-speed port is going to be switched to host mode role, we > have to set XHCI PORTSC.PP at host mode controller side to > acquire super-speed port hardware. > I replaced SS with super-speed and PP with 'XHCI PORTSC.PP' in the above description to make it clear. > > > [ 167.057718] BUG: sleeping function called from invalid context > > > at > > > include/linux/sched/mm.h:229 > > > [ 167.057872] Workqueue: events tegra_xusb_usb_phy_work > > > [ 167.057954] Call trace: > > > [ 167.057962] dump_backtrace+0x0/0x210 > > > [ 167.057996] show_stack+0x30/0x50 > > > [ 167.058020] dump_stack_lvl+0x64/0x84 > > > [ 167.058065] dump_stack+0x14/0x34 > > > [ 167.058100] __might_resched+0x144/0x180 > > > [ 167.058140] __might_sleep+0x64/0xd0 > > > [ 167.058171] slab_pre_alloc_hook.constprop.0+0xa8/0x110 > > > [ 167.058202] __kmalloc_track_caller+0x74/0x2b0 > > > [ 167.058233] kvasprintf+0xa4/0x190 > > > [ 167.058261] kasprintf+0x58/0x90 > > > [ 167.058285] tegra_xusb_find_port_node.isra.0+0x58/0xd0 > > > [ 167.058334] tegra_xusb_find_port+0x38/0xa0 > > > [ 167.058380] tegra_xusb_padctl_get_usb3_companion+0x38/0xd0 > > > [ 167.058430] tegra_xhci_id_notify+0x8c/0x1e0 > > > [ 167.058473] notifier_call_chain+0x88/0x100 > > > [ 167.058506] atomic_notifier_call_chain+0x44/0x70 > > > [ 167.058537] tegra_xusb_usb_phy_work+0x60/0xd0 > > > [ 167.058581] process_one_work+0x1dc/0x4c0 > > > [ 167.058618] worker_thread+0x54/0x410 > > > [ 167.058650] kthread+0x188/0x1b0 > > > [ 167.058672] ret_from_fork+0x10/0x20 > > > > > > The function tegra_xusb_padctl_get_usb3_companion eventually > > > calls > > > tegra_xusb_find_port and this in turn calls kasprintf which might > > > sleep > > > and so cannot be called from an atomic context. > > > > > > Fix this by moving the call to > > > tegra_xusb_padctl_get_usb3_companion > > > to > > > the tegra_xhci_id_work function where it is really needed. > > > > > > Fixes: f836e7843036 ("usb: xhci-tegra: Add OTG support") > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Wayne Chang <waynec@nvidia.com> > > > Signed-off-by: Haotien Hsu <haotienh@nvidia.com> > > > --- > > > V3 -> V4: Remove copyright change from this patch > > > V2 -> V3: Add version information > > > V1 -> V2: Add "Fixes" and "Cc:" lines and update copyright years > > > --- > > > drivers/usb/host/xhci-tegra.c | 7 ++++--- > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/usb/host/xhci-tegra.c > > > b/drivers/usb/host/xhci- > > > tegra.c > > > index 1ff22f675930..b40e897ec092 100644 > > > --- a/drivers/usb/host/xhci-tegra.c > > > +++ b/drivers/usb/host/xhci-tegra.c > > > @@ -1360,6 +1360,10 @@ static void tegra_xhci_id_work(struct > > > work_struct *work) > > > > > > mutex_unlock(&tegra->lock); > > > > > > + tegra->otg_usb3_port = > > > tegra_xusb_padctl_get_usb3_companion( > > > + tegra- > > > > padctl, > > > + tegra- > > > > otg_usb2_port); > > > + > > > > You have 100 columns, please use them, ending a line with "(" is > > not > > generally a good idea. > > I will resubmit a new patch. > > > thanks, > > > > greg k-h
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c index 1ff22f675930..b40e897ec092 100644 --- a/drivers/usb/host/xhci-tegra.c +++ b/drivers/usb/host/xhci-tegra.c @@ -1360,6 +1360,10 @@ static void tegra_xhci_id_work(struct work_struct *work) mutex_unlock(&tegra->lock); + tegra->otg_usb3_port = tegra_xusb_padctl_get_usb3_companion( + tegra->padctl, + tegra->otg_usb2_port); + if (tegra->host_mode) { /* switch to host mode */ if (tegra->otg_usb3_port >= 0) { @@ -1474,9 +1478,6 @@ static int tegra_xhci_id_notify(struct notifier_block *nb, } tegra->otg_usb2_port = tegra_xusb_get_usb2_port(tegra, usbphy); - tegra->otg_usb3_port = tegra_xusb_padctl_get_usb3_companion( - tegra->padctl, - tegra->otg_usb2_port); tegra->host_mode = (usbphy->last_event == USB_EVENT_ID) ? true : false;