On 16.03.23 12:17, Greg Kroah-Hartman wrote:
On Thu, Mar 16, 2023 at 12:28:58PM +0530, 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, bail out of debugfs
function is dwc3 is suspended.
Signed-off-by: Udipto Goswami <quic_ugoswami@xxxxxxxxxxx>
---
v3: Replace pr_err to dev_err.
drivers/usb/dwc3/debugfs.c | 60 ++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index 850df0e6bcab..4a9d0994f3b4 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -544,6 +544,12 @@ static int dwc3_link_state_show(struct seq_file *s, void *unused)
u32 reg;
u8 speed;
+ if (pm_runtime_suspended(dwc->dev)) {
+ dev_err(dwc->dev,
+ "Invalid operation, DWC3 suspended!");
+ return -EINVAL;
+ }
What prevents the device from being suspened right after you check this?
Indeed. If you really need a semantics of not waking up the device if
somebody reads through debugfs, the attached patch should do the job.
But do you really need it?
Regards
Oliver
From 86454823b682a9b2414c0bbb45c3ed2f9d0a1f5f Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@xxxxxxxx>
Date: Thu, 16 Mar 2023 12:32:55 +0100
Subject: [PATCH] dwc3: debugfs: no register access if suspended
Preventing register access while suspended and
prevent suspension during access via debugfs
Signed-off-by: Oliver Neukum <oneukum@xxxxxxxx>
---
drivers/usb/dwc3/debugfs.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index 850df0e6bcab..6922076572cd 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -543,13 +543,19 @@ static int dwc3_link_state_show(struct seq_file *s, void *unused)
enum dwc3_link_state state;
u32 reg;
u8 speed;
+ int ret = 0;
+ if (!pm_runtime_get_if_in_use(dwc->dev)) {
+ ret = -EINVAL;
+ dev_err(dwc->dev,
+ "Invalid operation, DWC3 suspended!");
+ goto err_nolock;
+ }
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);
- return 0;
+ goto err;
}
reg = dwc3_readl(dwc->regs, DWC3_DSTS);
@@ -559,9 +565,11 @@ static int dwc3_link_state_show(struct seq_file *s, void *unused)
seq_printf(s, "%s\n", (speed >= DWC3_DSTS_SUPERSPEED) ?
dwc3_gadget_link_string(state) :
dwc3_gadget_hs_link_string(state));
+err:
spin_unlock_irqrestore(&dwc->lock, flags);
-
- return 0;
+ pm_runtime_put(dwc->dev);
+err_nolock:
+ return ret;
}
static int dwc3_link_state_open(struct inode *inode, struct file *file)
--
2.39.2
