summaryrefslogtreecommitdiff
path: root/drivers/usb/dwc2/hcd.h
diff options
context:
space:
mode:
authorDouglas Anderson <dianders@chromium.org>2016-01-29 05:20:08 +0300
committerFelipe Balbi <balbi@kernel.org>2016-03-04 16:14:44 +0300
commitfb616e3f837eee20cc0c6d5866983f7d2730d5a3 (patch)
tree1586db4c44fecbe53ea19c54d965de0f1ea793d6 /drivers/usb/dwc2/hcd.h
parent483bb2544c470aebdcc41199c0b145d05058a3bd (diff)
downloadlinux-fb616e3f837eee20cc0c6d5866983f7d2730d5a3.tar.xz
usb: dwc2: host: Manage frame nums better in scheduler
The dwc2 scheduler (contained in hcd_queue.c) was a bit confusing in the way it initted / kept track of which frames a QH was going to be active in. Let's clean things up a little bit in preparation for a rewrite of the microframe scheduler. Specifically: * Old code would pick a frame number in dwc2_qh_init() and would try to pick it "in a slightly future (micro)frame". As far as I can tell the reason for this was that there was a delay between dwc2_qh_init() and when we actually wanted to dwc2_hcd_qh_add(). ...but apparently this attempt to be slightly in the future wasn't enough because dwc2_hcd_qh_add() then had code to reset things if the frame _wasn't_ in the future. There's no reason not to just pick the frame later. For non-periodic QH we now pick the frame in dwc2_hcd_qh_add(). For periodic QH we pick the frame at dwc2_schedule_periodic() time. * The old "dwc2_qh_init() actually assigned to "hsotg->frame_number". This doesn't seem like a great idea since that variable is supposed to be used to keep track of which SOF the interrupt handler has seen. Let's be clean: anyone who wants the current frame number (instead of the one as of the last interrupt) should ask for it. * The old code wasn't terribly consistent about trying to use the frame that the microframe scheduler assigned to it. In dwc2_sched_periodic_split() when it was scheduling the first frame it always "ORed" in 0x7 (!). Since the frame goes on the wire 1 uFrame after next_active_frame it meant that the SSPLIT would always try for uFrame 0 and the transaction would happen on the low speed bus during uFrame 1. This is irregardless of what the microframe scheduler said. * The old code assumed it would get called to schedule the next in a periodic split very quickly. That is if next_active_frame was 0 (transfer on wire in uFrame 1) it assumed it was getting called to schedule the next uFrame during uFrame 1 too (so it could queue something up for uFrame 2). It should be possible to actually queue something up for uFrame 2 while in uFrame 2 (AKA queue up ASAP). To do this, code needs to look at the previously scheduled frame when deciding when to next be active, not look at the current frame number. * If there was no microframe scheduler, the old code would check for whether we should be active using "qh->next_active_frame == frame_number". This seemed like a race waiting to happen. ...plus there's no way that you wouldn't want to schedule if next_active_frame was actually less than frame number. Note that this change doesn't make 100% sense on its own since it's expecting some sanity in the frame numbers assigned by the microframe scheduler and (as per the future patch which rewries it) I think that the current microframe scheduler is quite insane. However, it seems like splitting this up from the microframe scheduler patch makes things into smaller chunks and hopefully adds to clarity rather than reduces it. The two patches could certainly be squashed. Not that in the very least, I don't see any obvious bad behavior introduced with just this patch. I've attempted to keep the config parameter to disable the microframe scheduler in tact in this change, though I'm not sure it's worth it. Obviously the code is touched a lot so it's possible I regressed something when the microframe scheduler is disabled, though I did some basic testing and it seemed to work OK. I'm still not 100% sure why you wouldn't want the microframe scheduler (presuming it works), so maybe a future patch (or a future version of this patch?) could remove that parameter. Acked-by: John Youn <johnyoun@synopsys.com> Signed-off-by: Douglas Anderson <dianders@chromium.org> Tested-by: Heiko Stuebner <heiko@sntech.de> Tested-by: Stefan Wahren <stefan.wahren@i2se.com> Signed-off-by: Felipe Balbi <balbi@kernel.org>
Diffstat (limited to 'drivers/usb/dwc2/hcd.h')
-rw-r--r--drivers/usb/dwc2/hcd.h10
1 files changed, 7 insertions, 3 deletions
diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
index 10c35585a2bd..fd266ac53a28 100644
--- a/drivers/usb/dwc2/hcd.h
+++ b/drivers/usb/dwc2/hcd.h
@@ -244,8 +244,11 @@ enum dwc2_transaction_type {
* the bus. We'll move the qh to active here. If the
* host is in high speed mode this will be a uframe. If
* the host is in low speed mode this will be a full frame.
+ * @start_active_frame: If we are partway through a split transfer, this will be
+ * what next_active_frame was when we started. Otherwise
+ * it should always be the same as next_active_frame.
+ * @assigned_uframe: The uframe (0 -7) assigned by dwc2_find_uframe().
* @frame_usecs: Internal variable used by the microframe scheduler
- * @start_split_frame: (Micro)frame at which last start split was initialized
* @ntd: Actual number of transfer descriptors in a list
* @qtd_list: List of QTDs for this QH
* @channel: Host channel currently processing transfers for this QH
@@ -279,8 +282,9 @@ struct dwc2_qh {
u16 host_us;
u16 host_interval;
u16 next_active_frame;
+ u16 start_active_frame;
+ u16 assigned_uframe;
u16 frame_usecs[8];
- u16 start_split_frame;
u16 ntd;
struct list_head qtd_list;
struct dwc2_host_chan *channel;
@@ -746,7 +750,7 @@ do { \
_qtd_ = list_entry((_qh_)->qtd_list.next, struct dwc2_qtd, \
qtd_list_entry); \
if (usb_pipeint(_qtd_->urb->pipe) && \
- (_qh_)->start_split_frame != 0 && !_qtd_->complete_split) { \
+ (_qh_)->start_active_frame != 0 && !_qtd_->complete_split) { \
_hfnum_.d32 = dwc2_readl((_hcd_)->regs + HFNUM); \
switch (_hfnum_.b.frnum & 0x7) { \
case 7: \