Re: [PATCH 2/5 v6] usb: Add usb_endpoint_descriptor to be part ofthe struct usb_ep

[Felipe Balbi]

On Sun, Mar 27, 2011 at 02:26:48PM +0200, Tanya Brokhman wrote:
> > 
> > On Wed, Mar 23, 2011 at 10:03:38AM +0200, Tatyana Brokhman wrote:
> > > Change usb_ep_enable() prototype to use endpoint descriptor from
> > usb_ep.
> > > This optimization spares the FDs from saving the endpoint chosen
> > > descriptor. This optimization is not full though. To fully exploit
> > this
> > > change one needs to update all the UDCs as well since in the current
> > > implementation each of them saves the endpoint descriptor in it's
> > > internal (and extended) endpoint structure.
> > 
> > I'm not sure this is such a good patch. But I don't have strong
> > arguments against it either.
> > 
> 
> Could you please share you concerns? Even if they are not strong arguments
> :).
> Our pros for this change were that it seems more accurate that the EP
> descriptor will be handled by the composite layer and not the FDs. Thus we
> tried to spare the FD from saving the chosen EP descriptor and passing it to
> different functions.
> This change is needed for our algorithm of SS implementation in the later
> patches in this series. 

Yes, I understood that much, but it's useful for UDCs to access the EP
descriptor (ok, we can still access it by dereferencing ep) since we can
(actually we need to) use that to let the HW know about max packet
sizes, endpoint types, etc etc.

Also, I'm not sure composite.c should know the details about the
endpoint. That's just a thin layer implementing the more generic part
(standard usb requests, handling, helpers for finding endpoints, etc),
but the one who should look into endpoint descriptors is the underlying
controller code for the reasons I listed above.

That's my feeling...

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/