Hi Paul,

> > > @@ -365,6 +366,7 @@ struct dwc2_hsotg {
> > >   u8 otg_port;
> > >   u32 *frame_list;
> > >   dma_addr_t frame_list_dma;
> > > + int next_sched_frame;
> > 
> > This variable is still not really used, I think. Most of the mentions in
> > the patch are assignments, except for these two:
> > 
> > > +         if (list_empty(&hsotg->periodic_sched_inactive) ||
> > > +             dwc2_frame_num_le(qh->sched_frame, hsotg->next_sched_frame))
> > > +                 hsotg->next_sched_frame = qh->sched_frame;
> > > +
> > ...
> > > +                         if (!dwc2_frame_num_le(hsotg->next_sched_frame,
> > > +                                                qh->sched_frame))
> > > +                                 hsotg->next_sched_frame =
> > > +                                                 qh->sched_frame;
> > 
> > However, these two "uses" of the variable have only the single purpose
> > of updating the variable itself, no other behaviour is influenced by it.
> > In effect, the variable does not influence the code at all and can be
> > dropped, significantly simplifying this patch.
> 
> Yep, that's kind of embarrassing. I will have to go back and read the
> downstream driver code again to figure out how this code is supposed to
> work. Thanks for the review.
Not sure if you figured this out already, but I seem to remember that
this next_sched_frame variable code was used by the driver to disable
the SOF interrupt when it wasn't needed and/or for this priority
interrupt thing on ARM (FIQ?). Not entirely sure, though.

Gr.

Matthijs
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to