Hi,
Pavel Machek wrote:
Hi!
From: Thinh Nguyen thinh.nguyen@synopsys.com
[ Upstream commit 0d36dede457873404becd7c9cb9d0f2bcfd0dcd9 ]
Highspeed device and below has different state names than superspeed and higher. Add proper checks and printouts of link states for highspeed and below.
Just noticed some more oddity:
- case DWC3_LINK_STATE_RESUME:
return "Resume";- default:
return "UNKNOWN link state\n";- }
"UNKNOWN" would be consistent with the rest of the file.
Leaving the "link state" there may be fine for now due to the way it's printed in the log making it clearer.
+++ b/drivers/usb/dwc3/debugfs.c @@ -433,13 +433,17 @@ static int dwc3_link_state_show(struct seq_file *s, void *unused) unsigned long flags; enum dwc3_link_state state; u32 reg;
- u8 speed;
spin_lock_irqsave(&dwc->lock, flags); reg = dwc3_readl(dwc->regs, DWC3_DSTS); state = DWC3_DSTS_USBLNKST(reg);
- spin_unlock_irqrestore(&dwc->lock, flags);
- speed = reg & DWC3_DSTS_CONNECTSPD;
- seq_printf(s, "%s\n", dwc3_gadget_link_string(state));
- seq_printf(s, "%s\n", (speed >= DWC3_DSTS_SUPERSPEED) ?
dwc3_gadget_link_string(state) :dwc3_gadget_hs_link_string(state));- spin_unlock_irqrestore(&dwc->lock, flags);
return 0; }
The locking change is really wrong, right? There's no reason to do seq_printfs under spinlock..
Yes, it can be unlocked earlier.
@@ -477,6 +483,15 @@ static ssize_t dwc3_link_state_write(struct
file *file,
return -EINVAL;spin_lock_irqsave(&dwc->lock, flags);
- reg = dwc3_readl(dwc->regs, DWC3_DSTS);
- speed = reg & DWC3_DSTS_CONNECTSPD;
- if (speed < DWC3_DSTS_SUPERSPEED &&
state != DWC3_LINK_STATE_RECOV) {spin_unlock_irqrestore(&dwc->lock, flags);return -EINVAL;- }
- dwc3_gadget_set_link_state(dwc, state); spin_unlock_irqrestore(&dwc->lock, flags);
This might be ok but is not mentioned in the changelog.
I'll spell it out next time when I mention "add proper checks" in the commit message as it obviously wasn't clear.
Thanks, Thinh