| Message ID | 20230505155103.30098-1-quic_ugoswami@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v9] usb: dwc3: debugfs: Prevent any register access when devices is runtime suspended | expand |
On Fri, May 05, 2023, Udipto Goswami wrote: > When the dwc3 device is runtime suspended, various required clocks would > get disabled and it is not guaranteed that access to any registers would > work. Depending on the SoC glue, a register read could be as benign as > returning 0 or be fatal enough to hang the system. > > In order to prevent such scenarios of fatal errors, make sure to resume > dwc3 then allow the function to proceed. > > Fixes: 62ba09d6bb63 ("usb: dwc3: debugfs: Dump internal LSP and ep registers") This fix goes before the above change. This also touches on many places and is more than 100 lines. While this is a fix, I'm not sure if Cc stable is needed. Perhaps others can comment. Thanks, Thinh > Cc: stable@vger.kernel.org > Signed-off-by: Udipto Goswami <quic_ugoswami@quicinc.com> > --- > v9: Fixed function dwc3_rx_fifo_size_show & return values in function > dwc3_link_state_write along with minor changes for code symmetry. > v8: Replace pm_runtime_get_sync with pm_runtime_resume_and get. > v7: Replaced pm_runtime_put with pm_runtime_put_sync & returned proper values. > v6: Added changes to handle get_dync failure appropriately. > v5: Reworked the patch to resume dwc3 while accessing the registers. > v4: Introduced pm_runtime_get_if_in_use in order to make sure dwc3 isn't > suspended while accessing the registers. > v3: Replace pr_err to dev_err. > v2: Replaced return 0 with -EINVAL & seq_puts with pr_err. > > drivers/usb/dwc3/debugfs.c | 109 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 109 insertions(+) > > diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c > index e4a2560b9dc0..ebf03468fac4 100644 > --- a/drivers/usb/dwc3/debugfs.c > +++ b/drivers/usb/dwc3/debugfs.c > @@ -332,6 +332,11 @@ static int dwc3_lsp_show(struct seq_file *s, void *unused) > unsigned int current_mode; > unsigned long flags; > u32 reg; > + int ret; > + > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > > spin_lock_irqsave(&dwc->lock, flags); > reg = dwc3_readl(dwc->regs, DWC3_GSTS); > @@ -350,6 +355,8 @@ static int dwc3_lsp_show(struct seq_file *s, void *unused) > } > spin_unlock_irqrestore(&dwc->lock, flags); > > + pm_runtime_put_sync(dwc->dev); > + > return 0; > } > > @@ -395,6 +402,11 @@ static int dwc3_mode_show(struct seq_file *s, void *unused) > struct dwc3 *dwc = s->private; > unsigned long flags; > u32 reg; > + int ret; > + > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > > spin_lock_irqsave(&dwc->lock, flags); > reg = dwc3_readl(dwc->regs, DWC3_GCTL); > @@ -414,6 +426,8 @@ static int dwc3_mode_show(struct seq_file *s, void *unused) > seq_printf(s, "UNKNOWN %08x\n", DWC3_GCTL_PRTCAP(reg)); > } > > + pm_runtime_put_sync(dwc->dev); > + > return 0; > } > > @@ -463,6 +477,11 @@ static int dwc3_testmode_show(struct seq_file *s, void *unused) > struct dwc3 *dwc = s->private; > unsigned long flags; > u32 reg; > + int ret; > + > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > > spin_lock_irqsave(&dwc->lock, flags); > reg = dwc3_readl(dwc->regs, DWC3_DCTL); > @@ -493,6 +512,8 @@ static int dwc3_testmode_show(struct seq_file *s, void *unused) > seq_printf(s, "UNKNOWN %d\n", reg); > } > > + pm_runtime_put_sync(dwc->dev); > + > return 0; > } > > @@ -509,6 +530,7 @@ static ssize_t dwc3_testmode_write(struct file *file, > unsigned long flags; > u32 testmode = 0; > char buf[32]; > + int ret; > > if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count))) > return -EFAULT; > @@ -526,10 +548,16 @@ static ssize_t dwc3_testmode_write(struct file *file, > else > testmode = 0; > > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > + > spin_lock_irqsave(&dwc->lock, flags); > dwc3_gadget_set_test_mode(dwc, testmode); > spin_unlock_irqrestore(&dwc->lock, flags); > > + pm_runtime_put_sync(dwc->dev); > + > return count; > } > > @@ -548,12 +576,18 @@ static int dwc3_link_state_show(struct seq_file *s, void *unused) > enum dwc3_link_state state; > u32 reg; > u8 speed; > + int ret; > + > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > > spin_lock_irqsave(&dwc->lock, flags); > reg = dwc3_readl(dwc->regs, DWC3_GSTS); > if (DWC3_GSTS_CURMOD(reg) != DWC3_GSTS_CURMOD_DEVICE) { > seq_puts(s, "Not available\n"); > spin_unlock_irqrestore(&dwc->lock, flags); > + pm_runtime_put_sync(dwc->dev); > return 0; > } > > @@ -566,6 +600,8 @@ static int dwc3_link_state_show(struct seq_file *s, void *unused) > dwc3_gadget_hs_link_string(state)); > spin_unlock_irqrestore(&dwc->lock, flags); > > + pm_runtime_put_sync(dwc->dev); > + > return 0; > } > > @@ -584,6 +620,7 @@ static ssize_t dwc3_link_state_write(struct file *file, > char buf[32]; > u32 reg; > u8 speed; > + int ret; > > if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count))) > return -EFAULT; > @@ -603,10 +640,15 @@ static ssize_t dwc3_link_state_write(struct file *file, > else > return -EINVAL; > > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > + > spin_lock_irqsave(&dwc->lock, flags); > reg = dwc3_readl(dwc->regs, DWC3_GSTS); > if (DWC3_GSTS_CURMOD(reg) != DWC3_GSTS_CURMOD_DEVICE) { > spin_unlock_irqrestore(&dwc->lock, flags); > + pm_runtime_put_sync(dwc->dev); > return -EINVAL; > } > > @@ -616,12 +658,15 @@ static ssize_t dwc3_link_state_write(struct file *file, > if (speed < DWC3_DSTS_SUPERSPEED && > state != DWC3_LINK_STATE_RECOV) { > spin_unlock_irqrestore(&dwc->lock, flags); > + pm_runtime_put_sync(dwc->dev); > return -EINVAL; > } > > dwc3_gadget_set_link_state(dwc, state); > spin_unlock_irqrestore(&dwc->lock, flags); > > + pm_runtime_put_sync(dwc->dev); > + > return count; > } > > @@ -645,6 +690,11 @@ static int dwc3_tx_fifo_size_show(struct seq_file *s, void *unused) > unsigned long flags; > u32 mdwidth; > u32 val; > + int ret; > + > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > > spin_lock_irqsave(&dwc->lock, flags); > val = dwc3_core_fifo_space(dep, DWC3_TXFIFO); > @@ -657,6 +707,8 @@ static int dwc3_tx_fifo_size_show(struct seq_file *s, void *unused) > seq_printf(s, "%u\n", val); > spin_unlock_irqrestore(&dwc->lock, flags); > > + pm_runtime_put_sync(dwc->dev); > + > return 0; > } > > @@ -667,6 +719,11 @@ static int dwc3_rx_fifo_size_show(struct seq_file *s, void *unused) > unsigned long flags; > u32 mdwidth; > u32 val; > + int ret; > + > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > > spin_lock_irqsave(&dwc->lock, flags); > val = dwc3_core_fifo_space(dep, DWC3_RXFIFO); > @@ -679,6 +736,8 @@ static int dwc3_rx_fifo_size_show(struct seq_file *s, void *unused) > seq_printf(s, "%u\n", val); > spin_unlock_irqrestore(&dwc->lock, flags); > > + pm_runtime_put_sync(dwc->dev); > + > return 0; > } > > @@ -688,12 +747,19 @@ static int dwc3_tx_request_queue_show(struct seq_file *s, void *unused) > struct dwc3 *dwc = dep->dwc; > unsigned long flags; > u32 val; > + int ret; > + > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > > spin_lock_irqsave(&dwc->lock, flags); > val = dwc3_core_fifo_space(dep, DWC3_TXREQQ); > seq_printf(s, "%u\n", val); > spin_unlock_irqrestore(&dwc->lock, flags); > > + pm_runtime_put_sync(dwc->dev); > + > return 0; > } > > @@ -703,12 +769,19 @@ static int dwc3_rx_request_queue_show(struct seq_file *s, void *unused) > struct dwc3 *dwc = dep->dwc; > unsigned long flags; > u32 val; > + int ret; > + > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > > spin_lock_irqsave(&dwc->lock, flags); > val = dwc3_core_fifo_space(dep, DWC3_RXREQQ); > seq_printf(s, "%u\n", val); > spin_unlock_irqrestore(&dwc->lock, flags); > > + pm_runtime_put_sync(dwc->dev); > + > return 0; > } > > @@ -718,12 +791,19 @@ static int dwc3_rx_info_queue_show(struct seq_file *s, void *unused) > struct dwc3 *dwc = dep->dwc; > unsigned long flags; > u32 val; > + int ret; > + > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > > spin_lock_irqsave(&dwc->lock, flags); > val = dwc3_core_fifo_space(dep, DWC3_RXINFOQ); > seq_printf(s, "%u\n", val); > spin_unlock_irqrestore(&dwc->lock, flags); > > + pm_runtime_put_sync(dwc->dev); > + > return 0; > } > > @@ -733,12 +813,19 @@ static int dwc3_descriptor_fetch_queue_show(struct seq_file *s, void *unused) > struct dwc3 *dwc = dep->dwc; > unsigned long flags; > u32 val; > + int ret; > + > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > > spin_lock_irqsave(&dwc->lock, flags); > val = dwc3_core_fifo_space(dep, DWC3_DESCFETCHQ); > seq_printf(s, "%u\n", val); > spin_unlock_irqrestore(&dwc->lock, flags); > > + pm_runtime_put_sync(dwc->dev); > + > return 0; > } > > @@ -748,12 +835,19 @@ static int dwc3_event_queue_show(struct seq_file *s, void *unused) > struct dwc3 *dwc = dep->dwc; > unsigned long flags; > u32 val; > + int ret; > + > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > > spin_lock_irqsave(&dwc->lock, flags); > val = dwc3_core_fifo_space(dep, DWC3_EVENTQ); > seq_printf(s, "%u\n", val); > spin_unlock_irqrestore(&dwc->lock, flags); > > + pm_runtime_put_sync(dwc->dev); > + > return 0; > } > > @@ -798,6 +892,11 @@ static int dwc3_trb_ring_show(struct seq_file *s, void *unused) > struct dwc3 *dwc = dep->dwc; > unsigned long flags; > int i; > + int ret; > + > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > > spin_lock_irqsave(&dwc->lock, flags); > if (dep->number <= 1) { > @@ -827,6 +926,8 @@ static int dwc3_trb_ring_show(struct seq_file *s, void *unused) > out: > spin_unlock_irqrestore(&dwc->lock, flags); > > + pm_runtime_put_sync(dwc->dev); > + > return 0; > } > > @@ -839,6 +940,11 @@ static int dwc3_ep_info_register_show(struct seq_file *s, void *unused) > u32 lower_32_bits; > u32 upper_32_bits; > u32 reg; > + int ret; > + > + ret = pm_runtime_resume_and_get(dwc->dev); > + if (ret < 0) > + return ret; > > spin_lock_irqsave(&dwc->lock, flags); > reg = DWC3_GDBGLSPMUX_EPSELECT(dep->number); > @@ -851,6 +957,8 @@ static int dwc3_ep_info_register_show(struct seq_file *s, void *unused) > seq_printf(s, "0x%016llx\n", ep_info); > spin_unlock_irqrestore(&dwc->lock, flags); > > + pm_runtime_put_sync(dwc->dev); > + > return 0; > } > > @@ -910,6 +1018,7 @@ void dwc3_debugfs_init(struct dwc3 *dwc) > dwc->regset->regs = dwc3_regs; > dwc->regset->nregs = ARRAY_SIZE(dwc3_regs); > dwc->regset->base = dwc->regs - DWC3_GLOBALS_REGS_START; > + dwc->regset->dev = dwc->dev; > > root = debugfs_create_dir(dev_name(dwc->dev), usb_debug_root); > dwc->debug_root = root; > -- > 2.17.1 >
On Sat, May 06, 2023 at 01:30:52AM +0000, Thinh Nguyen wrote: Udipto, looks like you just ignored my comment about fixing up the patch Subject. > On Fri, May 05, 2023, Udipto Goswami wrote: > > When the dwc3 device is runtime suspended, various required clocks would > > get disabled and it is not guaranteed that access to any registers would > > work. Depending on the SoC glue, a register read could be as benign as > > returning 0 or be fatal enough to hang the system. > > > > In order to prevent such scenarios of fatal errors, make sure to resume > > dwc3 then allow the function to proceed. > > > > Fixes: 62ba09d6bb63 ("usb: dwc3: debugfs: Dump internal LSP and ep registers") > > This fix goes before the above change. Yes, this clearly is not the commit that first introduced this issue. Either add a Fixes tag for the oldest one or add one for each commit that introduced debugfs attributes with this issues. > This also touches on many places and is more than 100 lines. While this > is a fix, I'm not sure if Cc stable is needed. Perhaps others can > comment. I believe this should be backported as it fixes a crash/hang. The stable rules are flexible, but it may also be possible to break the patch up in pieces and add a corresponding Fixes tag. Johan
On 5/8/23 5:04 PM, Johan Hovold wrote: > On Sat, May 06, 2023 at 01:30:52AM +0000, Thinh Nguyen wrote: > > Udipto, looks like you just ignored my comment about fixing up the patch > Subject. Hi Johan, Apologies for this, i missed the comment on the Subject will rectify it. > >> On Fri, May 05, 2023, Udipto Goswami wrote: >>> When the dwc3 device is runtime suspended, various required clocks would >>> get disabled and it is not guaranteed that access to any registers would >>> work. Depending on the SoC glue, a register read could be as benign as >>> returning 0 or be fatal enough to hang the system. >>> >>> In order to prevent such scenarios of fatal errors, make sure to resume >>> dwc3 then allow the function to proceed. >>> >>> Fixes: 62ba09d6bb63 ("usb: dwc3: debugfs: Dump internal LSP and ep registers") >> >> This fix goes before the above change. > > Yes, this clearly is not the commit that first introduced this issue. > > Either add a Fixes tag for the oldest one or add one for each commit > that introduced debugfs attributes with this issues. > >> This also touches on many places and is more than 100 lines. While this >> is a fix, I'm not sure if Cc stable is needed. Perhaps others can >> comment. > > I believe this should be backported as it fixes a crash/hang. > > The stable rules are flexible, but it may also be possible to break the > patch up in pieces and add a corresponding Fixes tag. Agree, I will add a fixes tag for the oldest change that introduced the debugfs attributes instead of breaking it to multiple patches and adding fixes for each one. (I think the present code changes can stay in one patch as we are fixing the same issue in all the functions). Let me know if you think otherwise? Thanks, -Udipto
On Mon, May 08, 2023, Johan Hovold wrote: > > The stable rules are flexible, but it may also be possible to break the > patch up in pieces and add a corresponding Fixes tag. > > Johan In this case, I'd prefer to keep it all in a single patch to easily track the change. Anyone who needs this for the older kernel can rework the upstream version for backport. Thanks, Thinh
On Mon, May 08, 2023 at 09:18:17PM +0530, Udipto Goswami wrote: > > I believe this should be backported as it fixes a crash/hang. > > > > The stable rules are flexible, but it may also be possible to break the > > patch up in pieces and add a corresponding Fixes tag. > > Agree, I will add a fixes tag for the oldest change that introduced the > debugfs attributes instead of breaking it to multiple patches and adding > fixes for each one. (I think the present code changes can stay in one > patch as we are fixing the same issue in all the functions). > > Let me know if you think otherwise? Sounds good to me. Note that the fix depends on 30332eeefec8 ("debugfs: regset32: Add Runtime PM support") which went into 5.7. This can be documented as Cc: stable@vger.kernel.org # 3.2: 30332eeefec8: debugfs: regset32: Add Runtime PM support (see Documentation/process/stable-kernel-rules.rst). Note that this issue appears to have been there since the driver was first merged in 3.2. Johan
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c index e4a2560b9dc0..ebf03468fac4 100644 --- a/drivers/usb/dwc3/debugfs.c +++ b/drivers/usb/dwc3/debugfs.c @@ -332,6 +332,11 @@ static int dwc3_lsp_show(struct seq_file *s, void *unused) unsigned int current_mode; unsigned long flags; u32 reg; + int ret; + + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; spin_lock_irqsave(&dwc->lock, flags); reg = dwc3_readl(dwc->regs, DWC3_GSTS); @@ -350,6 +355,8 @@ static int dwc3_lsp_show(struct seq_file *s, void *unused) } spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); + return 0; } @@ -395,6 +402,11 @@ static int dwc3_mode_show(struct seq_file *s, void *unused) struct dwc3 *dwc = s->private; unsigned long flags; u32 reg; + int ret; + + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; spin_lock_irqsave(&dwc->lock, flags); reg = dwc3_readl(dwc->regs, DWC3_GCTL); @@ -414,6 +426,8 @@ static int dwc3_mode_show(struct seq_file *s, void *unused) seq_printf(s, "UNKNOWN %08x\n", DWC3_GCTL_PRTCAP(reg)); } + pm_runtime_put_sync(dwc->dev); + return 0; } @@ -463,6 +477,11 @@ static int dwc3_testmode_show(struct seq_file *s, void *unused) struct dwc3 *dwc = s->private; unsigned long flags; u32 reg; + int ret; + + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; spin_lock_irqsave(&dwc->lock, flags); reg = dwc3_readl(dwc->regs, DWC3_DCTL); @@ -493,6 +512,8 @@ static int dwc3_testmode_show(struct seq_file *s, void *unused) seq_printf(s, "UNKNOWN %d\n", reg); } + pm_runtime_put_sync(dwc->dev); + return 0; } @@ -509,6 +530,7 @@ static ssize_t dwc3_testmode_write(struct file *file, unsigned long flags; u32 testmode = 0; char buf[32]; + int ret; if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count))) return -EFAULT; @@ -526,10 +548,16 @@ static ssize_t dwc3_testmode_write(struct file *file, else testmode = 0; + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; + spin_lock_irqsave(&dwc->lock, flags); dwc3_gadget_set_test_mode(dwc, testmode); spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); + return count; } @@ -548,12 +576,18 @@ static int dwc3_link_state_show(struct seq_file *s, void *unused) enum dwc3_link_state state; u32 reg; u8 speed; + int ret; + + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; spin_lock_irqsave(&dwc->lock, flags); reg = dwc3_readl(dwc->regs, DWC3_GSTS); if (DWC3_GSTS_CURMOD(reg) != DWC3_GSTS_CURMOD_DEVICE) { seq_puts(s, "Not available\n"); spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); return 0; } @@ -566,6 +600,8 @@ static int dwc3_link_state_show(struct seq_file *s, void *unused) dwc3_gadget_hs_link_string(state)); spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); + return 0; } @@ -584,6 +620,7 @@ static ssize_t dwc3_link_state_write(struct file *file, char buf[32]; u32 reg; u8 speed; + int ret; if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count))) return -EFAULT; @@ -603,10 +640,15 @@ static ssize_t dwc3_link_state_write(struct file *file, else return -EINVAL; + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; + spin_lock_irqsave(&dwc->lock, flags); reg = dwc3_readl(dwc->regs, DWC3_GSTS); if (DWC3_GSTS_CURMOD(reg) != DWC3_GSTS_CURMOD_DEVICE) { spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); return -EINVAL; } @@ -616,12 +658,15 @@ static ssize_t dwc3_link_state_write(struct file *file, if (speed < DWC3_DSTS_SUPERSPEED && state != DWC3_LINK_STATE_RECOV) { spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); return -EINVAL; } dwc3_gadget_set_link_state(dwc, state); spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); + return count; } @@ -645,6 +690,11 @@ static int dwc3_tx_fifo_size_show(struct seq_file *s, void *unused) unsigned long flags; u32 mdwidth; u32 val; + int ret; + + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; spin_lock_irqsave(&dwc->lock, flags); val = dwc3_core_fifo_space(dep, DWC3_TXFIFO); @@ -657,6 +707,8 @@ static int dwc3_tx_fifo_size_show(struct seq_file *s, void *unused) seq_printf(s, "%u\n", val); spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); + return 0; } @@ -667,6 +719,11 @@ static int dwc3_rx_fifo_size_show(struct seq_file *s, void *unused) unsigned long flags; u32 mdwidth; u32 val; + int ret; + + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; spin_lock_irqsave(&dwc->lock, flags); val = dwc3_core_fifo_space(dep, DWC3_RXFIFO); @@ -679,6 +736,8 @@ static int dwc3_rx_fifo_size_show(struct seq_file *s, void *unused) seq_printf(s, "%u\n", val); spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); + return 0; } @@ -688,12 +747,19 @@ static int dwc3_tx_request_queue_show(struct seq_file *s, void *unused) struct dwc3 *dwc = dep->dwc; unsigned long flags; u32 val; + int ret; + + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; spin_lock_irqsave(&dwc->lock, flags); val = dwc3_core_fifo_space(dep, DWC3_TXREQQ); seq_printf(s, "%u\n", val); spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); + return 0; } @@ -703,12 +769,19 @@ static int dwc3_rx_request_queue_show(struct seq_file *s, void *unused) struct dwc3 *dwc = dep->dwc; unsigned long flags; u32 val; + int ret; + + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; spin_lock_irqsave(&dwc->lock, flags); val = dwc3_core_fifo_space(dep, DWC3_RXREQQ); seq_printf(s, "%u\n", val); spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); + return 0; } @@ -718,12 +791,19 @@ static int dwc3_rx_info_queue_show(struct seq_file *s, void *unused) struct dwc3 *dwc = dep->dwc; unsigned long flags; u32 val; + int ret; + + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; spin_lock_irqsave(&dwc->lock, flags); val = dwc3_core_fifo_space(dep, DWC3_RXINFOQ); seq_printf(s, "%u\n", val); spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); + return 0; } @@ -733,12 +813,19 @@ static int dwc3_descriptor_fetch_queue_show(struct seq_file *s, void *unused) struct dwc3 *dwc = dep->dwc; unsigned long flags; u32 val; + int ret; + + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; spin_lock_irqsave(&dwc->lock, flags); val = dwc3_core_fifo_space(dep, DWC3_DESCFETCHQ); seq_printf(s, "%u\n", val); spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); + return 0; } @@ -748,12 +835,19 @@ static int dwc3_event_queue_show(struct seq_file *s, void *unused) struct dwc3 *dwc = dep->dwc; unsigned long flags; u32 val; + int ret; + + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; spin_lock_irqsave(&dwc->lock, flags); val = dwc3_core_fifo_space(dep, DWC3_EVENTQ); seq_printf(s, "%u\n", val); spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); + return 0; } @@ -798,6 +892,11 @@ static int dwc3_trb_ring_show(struct seq_file *s, void *unused) struct dwc3 *dwc = dep->dwc; unsigned long flags; int i; + int ret; + + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; spin_lock_irqsave(&dwc->lock, flags); if (dep->number <= 1) { @@ -827,6 +926,8 @@ static int dwc3_trb_ring_show(struct seq_file *s, void *unused) out: spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); + return 0; } @@ -839,6 +940,11 @@ static int dwc3_ep_info_register_show(struct seq_file *s, void *unused) u32 lower_32_bits; u32 upper_32_bits; u32 reg; + int ret; + + ret = pm_runtime_resume_and_get(dwc->dev); + if (ret < 0) + return ret; spin_lock_irqsave(&dwc->lock, flags); reg = DWC3_GDBGLSPMUX_EPSELECT(dep->number); @@ -851,6 +957,8 @@ static int dwc3_ep_info_register_show(struct seq_file *s, void *unused) seq_printf(s, "0x%016llx\n", ep_info); spin_unlock_irqrestore(&dwc->lock, flags); + pm_runtime_put_sync(dwc->dev); + return 0; } @@ -910,6 +1018,7 @@ void dwc3_debugfs_init(struct dwc3 *dwc) dwc->regset->regs = dwc3_regs; dwc->regset->nregs = ARRAY_SIZE(dwc3_regs); dwc->regset->base = dwc->regs - DWC3_GLOBALS_REGS_START; + dwc->regset->dev = dwc->dev; root = debugfs_create_dir(dev_name(dwc->dev), usb_debug_root); dwc->debug_root = root;
When the dwc3 device is runtime suspended, various required clocks would get disabled and it is not guaranteed that access to any registers would work. Depending on the SoC glue, a register read could be as benign as returning 0 or be fatal enough to hang the system. In order to prevent such scenarios of fatal errors, make sure to resume dwc3 then allow the function to proceed. Fixes: 62ba09d6bb63 ("usb: dwc3: debugfs: Dump internal LSP and ep registers") Cc: stable@vger.kernel.org Signed-off-by: Udipto Goswami <quic_ugoswami@quicinc.com> --- v9: Fixed function dwc3_rx_fifo_size_show & return values in function dwc3_link_state_write along with minor changes for code symmetry. v8: Replace pm_runtime_get_sync with pm_runtime_resume_and get. v7: Replaced pm_runtime_put with pm_runtime_put_sync & returned proper values. v6: Added changes to handle get_dync failure appropriately. v5: Reworked the patch to resume dwc3 while accessing the registers. v4: Introduced pm_runtime_get_if_in_use in order to make sure dwc3 isn't suspended while accessing the registers. v3: Replace pr_err to dev_err. v2: Replaced return 0 with -EINVAL & seq_puts with pr_err. drivers/usb/dwc3/debugfs.c | 109 +++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+)