Hi!
> From: Thinh Nguyen <thinh.nguyen@xxxxxxxxxxxx>
>
> [ 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.
> +++ 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..
> @@ -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.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Attachment:
signature.asc
Description: Digital signature
