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@xxxxxxxxxxxxxxx
> Signed-off-by: Udipto Goswami <quic_ugoswami@xxxxxxxxxxx>
> ---
> 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
>
