Yes,
I added the next_sched_frame to track the next uframe that there is
anything to do in the periodic schedule. The SOF interrupt is then
disabled until that uframe (although the FIQ still runs each uframe to
decide whether to trigger the USB interrupt)
Gordon
On 21/08/2013 22:24, "Matthijs Kooijman" <matthijs@xxxxxxxx> wrote:
>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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
