• Douglas Anderson's avatar
    usb: dwc2: host: Manage frame nums better in scheduler · fb616e3f
    Douglas Anderson authored
    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: default avatarJohn Youn <johnyoun@synopsys.com>
    Signed-off-by: default avatarDouglas Anderson <dianders@chromium.org>
    Tested-by: default avatarHeiko Stuebner <heiko@sntech.de>
    Tested-by: default avatarStefan Wahren <stefan.wahren@i2se.com>
    Signed-off-by: default avatarFelipe Balbi <balbi@kernel.org>
    fb616e3f
hcd.h 26.3 KB