> From: Matthijs Kooijman [mailto:matthijs@xxxxxxxx] > Sent: Monday, August 12, 2013 12:36 AM > To: Paul Zimmerman > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; devel@xxxxxxxxxxxxxxxxxxxxxx; > swarren@xxxxxxxxxxxxx; gordon.hollingworth@xxxxxxxxx; skrll@xxxxxxxxxx; Dom Cobley > Subject: Re: [PATCH 2/3] staging: dwc2: add NAK holdoff patch from downstream Pi kernel > > Hi Paul, > > > Add the NAK holdoff patch from the downstream Raspberry Pi kernel. > > This allows the transfer scheduler to better handle "cheeky devices > > that just hold off using NAKs". > > > @@ -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. -- Paul _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
