> From: Matthijs Kooijman [mailto:matth...@stdin.nl] > Sent: Monday, August 12, 2013 12:36 AM > To: Paul Zimmerman > Cc: gre...@linuxfoundation.org; linux-...@vger.kernel.org; > de...@linuxdriverproject.org; > swar...@wwwdotorg.org; gordon.hollingwo...@gmail.com; sk...@netbsd.org; 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 de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
