On Thu, Apr 20, 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.
>
> Signed-off-by: Udipto Goswami <quic_ugoswami@xxxxxxxxxxx>
> ---
>
> 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 | 124 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 124 insertions(+)
>
> diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
> index e4a2560b9dc0..b08604a73e69 100644
> --- a/drivers/usb/dwc3/debugfs.c
> +++ b/drivers/usb/dwc3/debugfs.c
> @@ -332,6 +332,13 @@ 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_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
Should we use pm_runtime_put_sync?
> + return -EINVAL;
Why not return "ret"?
> + }
>
> spin_lock_irqsave(&dwc->lock, flags);
> reg = dwc3_readl(dwc->regs, DWC3_GSTS);
> @@ -349,6 +356,7 @@ static int dwc3_lsp_show(struct seq_file *s, void *unused)
> break;
> }
> spin_unlock_irqrestore(&dwc->lock, flags);
> + pm_runtime_put(dwc->dev);
>
> return 0;
> }
> @@ -395,6 +403,13 @@ 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_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
> + return -EINVAL;
> + }
>
> spin_lock_irqsave(&dwc->lock, flags);
> reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> @@ -414,6 +429,7 @@ static int dwc3_mode_show(struct seq_file *s, void *unused)
> seq_printf(s, "UNKNOWN %08x\n", DWC3_GCTL_PRTCAP(reg));
> }
>
> + pm_runtime_put(dwc->dev);
> return 0;
> }
>
> @@ -463,6 +479,13 @@ 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_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
> + return -EINVAL;
> + }
>
> spin_lock_irqsave(&dwc->lock, flags);
> reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> @@ -493,6 +516,7 @@ static int dwc3_testmode_show(struct seq_file *s, void *unused)
> seq_printf(s, "UNKNOWN %d\n", reg);
> }
>
> + pm_runtime_put(dwc->dev);
> return 0;
> }
>
> @@ -509,6 +533,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 +551,17 @@ static ssize_t dwc3_testmode_write(struct file *file,
> else
> testmode = 0;
>
> + ret = pm_runtime_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
> + return -EINVAL;
> + }
> +
> spin_lock_irqsave(&dwc->lock, flags);
> dwc3_gadget_set_test_mode(dwc, testmode);
> spin_unlock_irqrestore(&dwc->lock, flags);
>
> + pm_runtime_put(dwc->dev);
> return count;
> }
>
> @@ -548,12 +580,20 @@ 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_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
> + return -EINVAL;
> + }
>
> 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(dwc->dev);
> return 0;
> }
>
> @@ -566,6 +606,7 @@ 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(dwc->dev);
> return 0;
> }
>
> @@ -584,6 +625,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 +645,17 @@ static ssize_t dwc3_link_state_write(struct file *file,
> else
> return -EINVAL;
>
> + ret = pm_runtime_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
> + return -EINVAL;
> + }
> +
> 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(dwc->dev);
> return -EINVAL;
> }
>
> @@ -616,12 +665,14 @@ 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(dwc->dev);
> return -EINVAL;
> }
>
> dwc3_gadget_set_link_state(dwc, state);
> spin_unlock_irqrestore(&dwc->lock, flags);
>
> + pm_runtime_put(dwc->dev);
> return count;
> }
>
> @@ -645,6 +696,13 @@ 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_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
> + return -EINVAL;
> + }
>
> spin_lock_irqsave(&dwc->lock, flags);
> val = dwc3_core_fifo_space(dep, DWC3_TXFIFO);
> @@ -657,6 +715,7 @@ 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(dwc->dev);
> return 0;
> }
>
> @@ -667,6 +726,13 @@ 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_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
> + return -EINVAL;
> + }
>
> spin_lock_irqsave(&dwc->lock, flags);
> val = dwc3_core_fifo_space(dep, DWC3_RXFIFO);
> @@ -679,6 +745,7 @@ 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(dwc->dev);
> return 0;
> }
>
> @@ -688,12 +755,20 @@ 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_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
> + return -EINVAL;
> + }
>
> 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(dwc->dev);
> return 0;
> }
>
> @@ -703,12 +778,20 @@ 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_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
> + return -EINVAL;
> + }
>
> 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(dwc->dev);
> return 0;
> }
>
> @@ -718,12 +801,20 @@ 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_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
> + return -EINVAL;
> + }
>
> 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(dwc->dev);
> return 0;
> }
>
> @@ -733,12 +824,20 @@ 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_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
> + return -EINVAL;
> + }
>
> 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(dwc->dev);
> return 0;
> }
>
> @@ -748,12 +847,20 @@ 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_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
> + return -EINVAL;
> + }
>
> 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(dwc->dev);
> return 0;
> }
>
> @@ -798,6 +905,13 @@ 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_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
> + return -EINVAL;
> + }
>
> spin_lock_irqsave(&dwc->lock, flags);
> if (dep->number <= 1) {
> @@ -827,6 +941,7 @@ static int dwc3_trb_ring_show(struct seq_file *s, void *unused)
> out:
> spin_unlock_irqrestore(&dwc->lock, flags);
>
> + pm_runtime_put(dwc->dev);
> return 0;
> }
>
> @@ -839,6 +954,13 @@ 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_get_sync(dwc->dev);
> + if (ret < 0) {
> + pm_runtime_put(dwc->dev);
> + return -EINVAL;
> + }
>
> spin_lock_irqsave(&dwc->lock, flags);
> reg = DWC3_GDBGLSPMUX_EPSELECT(dep->number);
> @@ -851,6 +973,7 @@ 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(dwc->dev);
> return 0;
> }
>
> @@ -910,6 +1033,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
>
Thanks,
Thinh
