Re: [PATCH/RESEND v15 1/10] usb: Add usb_endpoint_descriptor to be part of the struct usb_ep

2011-06-09 Thread Felipe Balbi 
Hi,

On Mon, Jun 06, 2011 at 02:40:45PM +0300, 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.
 
 Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org

now that I look at this patch carefully, won't this break all gadget
drivers ? I mean, if I apply this patch, all gadget drivers will be
using descriptor from struct usb_ep, but noone is actually assigning
that pointer.

A better approach would be to:

(a) add the field to struct usb_ep
(b) make each controller assign that pointer while still keeping the
last one (keep the old interface too, make one patch per
controller)
(c) make each gadget/function driver use the new interface (one patch
per driver)
(d) remove the old interface from all controller (one patch for all of
them)

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH/RESEND v15 1/10] usb: Add usb_endpoint_descriptor to be part of the struct usb_ep

2011-06-09 Thread Felipe Balbi 
Hi again,

On Thu, Jun 09, 2011 at 11:32:10AM +0300, Felipe Balbi wrote:
 On Mon, Jun 06, 2011 at 02:40:45PM +0300, 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.
  
  Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org
 
 now that I look at this patch carefully, won't this break all gadget
 drivers ? I mean, if I apply this patch, all gadget drivers will be
 using descriptor from struct usb_ep, but noone is actually assigning
 that pointer.

actually, I was wrong. The gadget driver is setting it, not the
controller anymore. I'll take this series in.

-- 
balbi


signature.asc
Description: Digital signature

[PATCH/RESEND v15 1/10] usb: Add usb_endpoint_descriptor to be part of the struct usb_ep

2011-06-06 Thread Tatyana Brokhman 
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.

Signed-off-by: Tatyana Brokhman tlin...@codeaurora.org

---
 drivers/usb/gadget/dbgp.c   |8 +---
 drivers/usb/gadget/f_acm.c  |9 -
 drivers/usb/gadget/f_audio.c|5 ++---
 drivers/usb/gadget/f_ecm.c  |   17 -
 drivers/usb/gadget/f_eem.c  |   10 +-
 drivers/usb/gadget/f_fs.c   |3 ++-
 drivers/usb/gadget/f_hid.c  |5 ++---
 drivers/usb/gadget/f_loopback.c |   14 ++
 drivers/usb/gadget/f_mass_storage.c |3 ++-
 drivers/usb/gadget/f_ncm.c  |   17 -
 drivers/usb/gadget/f_obex.c |6 +++---
 drivers/usb/gadget/f_phonet.c   |9 -
 drivers/usb/gadget/f_rndis.c|   15 +++
 drivers/usb/gadget/f_serial.c   |4 ++--
 drivers/usb/gadget/f_sourcesink.c   |   10 --
 drivers/usb/gadget/f_subset.c   |8 
 drivers/usb/gadget/f_uvc.c  |6 --
 drivers/usb/gadget/file_storage.c   |3 ++-
 drivers/usb/gadget/gmidi.c  |6 --
 drivers/usb/gadget/inode.c  |6 --
 drivers/usb/gadget/printer.c|   26 ++
 drivers/usb/gadget/u_ether.c|   12 ++--
 drivers/usb/gadget/u_ether.h|4 
 drivers/usb/gadget/u_serial.c   |4 ++--
 drivers/usb/gadget/u_serial.h   |2 --
 include/linux/usb/gadget.h  |   16 +++-
 26 files changed, 111 insertions(+), 117 deletions(-)

diff --git a/drivers/usb/gadget/dbgp.c b/drivers/usb/gadget/dbgp.c
index dbe92ee..052209e 100644
--- a/drivers/usb/gadget/dbgp.c
+++ b/drivers/usb/gadget/dbgp.c
@@ -173,7 +173,9 @@ fail_1:
 
 static int __enable_ep(struct usb_ep *ep, struct usb_endpoint_descriptor *desc)
 {
-   int err = usb_ep_enable(ep, desc);
+   int err;
+   ep-desc = desc;
+   err = usb_ep_enable(ep);
ep-driver_data = dbgp.gadget;
return err;
 }
@@ -268,8 +270,8 @@ static int __init dbgp_configure_endpoints(struct 
usb_gadget *gadget)
dbgp.serial-in = dbgp.i_ep;
dbgp.serial-out = dbgp.o_ep;
 
-   dbgp.serial-in_desc = i_desc;
-   dbgp.serial-out_desc = o_desc;
+   dbgp.serial-in-desc = i_desc;
+   dbgp.serial-out-desc = o_desc;
 
if (gserial_setup(gadget, 1)  0) {
stp = 3;
diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c
index bd6226c..d04b4a6 100644
--- a/drivers/usb/gadget/f_acm.c
+++ b/drivers/usb/gadget/f_acm.c
@@ -62,7 +62,6 @@ struct f_acm {
struct acm_ep_descs hs;
 
struct usb_ep   *notify;
-   struct usb_endpoint_descriptor  *notify_desc;
struct usb_request  *notify_req;
 
struct usb_cdc_line_coding  port_line_coding;   /* 8-N-1 etc */
@@ -405,11 +404,11 @@ static int acm_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
usb_ep_disable(acm-notify);
} else {
VDBG(cdev, init acm ctrl interface %d\n, intf);
-   acm-notify_desc = ep_choose(cdev-gadget,
+   acm-notify-desc = ep_choose(cdev-gadget,
acm-hs.notify,
acm-fs.notify);
}
-   usb_ep_enable(acm-notify, acm-notify_desc);
+   usb_ep_enable(acm-notify);
acm-notify-driver_data = acm;
 
} else if (intf == acm-data_id) {
@@ -418,9 +417,9 @@ static int acm_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
gserial_disconnect(acm-port);
} else {
DBG(cdev, activate acm ttyGS%d\n, acm-port_num);
-   acm-port.in_desc = ep_choose(cdev-gadget,
+   acm-port.in-desc = ep_choose(cdev-gadget,
acm-hs.in, acm-fs.in);
-   acm-port.out_desc = ep_choose(cdev-gadget,
+   acm-port.out-desc = ep_choose(cdev-gadget,
acm-hs.out, acm-fs.out);
}
gserial_connect(acm-port, acm-port_num);
diff --git a/drivers/usb/gadget/f_audio.c b/drivers/usb/gadget/f_audio.c
index 8ee330a..02a0270 100644
--- a/drivers/usb/gadget/f_audio.c
+++ b/drivers/usb/gadget/f_audio.c
@@ -279,7 +279,6 @@ struct f_audio {
 
/* endpoints handle full and/or high speeds */
struct usb_ep   *out_ep;
-   struct