RE: [PATCH 2/3] staging: dwc2: add NAK holdoff patch from downstream Pi kernel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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


--
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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux