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
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
