On Tue, May 5, 2020 at 2:08 PM Andrey Konovalov <andreyk...@google.com> wrote:
>
> On Tue, May 5, 2020 at 9:34 AM Felipe Balbi <ba...@kernel.org> wrote:
> >
> >
> > Hi,
> >
> > Andrey Konovalov <andreyk...@google.com> writes:
> >
> > > Currently automatic gadget endpoint selection based on required features
> > > doesn't work. Raw Gadget tries iterating over the list of available
> > > endpoints and finding one that has the right direction and transfer type.
> > > Unfortunately selecting arbitrary gadget endpoints (even if they satisfy
> > > feature requirements) doesn't work, as (depending on the UDC driver) they
> > > might have fixed addresses, and one also needs to provide matching
> > > endpoint addresses in the descriptors sent to the host.
> > >
> > > The composite framework deals with this by assigning endpoint addresses
> > > in usb_ep_autoconfig() before enumeration starts. This approach won't work
> > > with Raw Gadget as the endpoints are supposed to be enabled after a
> > > set_configuration/set_interface request from the host, so it's too late to
> > > patch the endpoint descriptors that had already been sent to the host.
> > >
> > > For Raw Gadget we take another approach. Similarly to GadgetFS, we allow
> > > the user to make the decision as to which gadget endpoints to use.
> > >
> > > This patch adds another Raw Gadget ioctl USB_RAW_IOCTL_EPS_INFO that
> > > exposes information about all non-control endpoints that a currently
> > > connected UDC has. This information includes endpoints addresses, as well
> > > as their capabilities and limits to allow the user to choose the most
> > > fitting gadget endpoint.
> > >
> > > The USB_RAW_IOCTL_EP_ENABLE ioctl is updated to use the proper endpoint
> > > validation routine usb_gadget_ep_match_desc() and also now accepts an
> > > optional usb_ss_ep_comp_descriptor argument.
> > >
> > > These changes affect the portability of the gadgets that use Raw Gadget
> > > when running on different UDCs. Nevertheless, as long as the user relies
> > > on the information provided by USB_RAW_IOCTL_EPS_INFO to dynamically
> > > choose endpoint addresses, UDC-agnostic gadgets can still be written with
> > > Raw Gadget.
> > >
> > > Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface")
> > > Signed-off-by: Andrey Konovalov <andreyk...@google.com>
> > > ---
> > >
> > > Changes v1 -> v2:
> > > - Validate endpoint number against dev->eps_num instead of
> > > USB_RAW_EPS_NUM_MAX.
> > > - Dropped maxburst from struct usb_raw_ep_limits, added reserved field
> > > instead.
> > > - Don't allocate struct usb_raw_eps_info on stack, it's too large.
> > > - Assign SuperSpeed endpoint companion descriptor to usb_ep when enabling
> > > endpoints.
> > > - Fixed typos in commit message.
> > >
> > > ---
> > > Documentation/usb/raw-gadget.rst | 6 +-
> > > drivers/usb/gadget/legacy/raw_gadget.c | 217 +++++++++++++++++--------
> > > include/uapi/linux/usb/raw_gadget.h | 88 +++++++++-
> > > 3 files changed, 231 insertions(+), 80 deletions(-)
> > >
> > > diff --git a/Documentation/usb/raw-gadget.rst
> > > b/Documentation/usb/raw-gadget.rst
> > > index 9e78cb858f86..42bd446d72b2 100644
> > > --- a/Documentation/usb/raw-gadget.rst
> > > +++ b/Documentation/usb/raw-gadget.rst
> > > @@ -27,11 +27,7 @@ differences are:
> > > 3. Raw Gadget provides a way to select a UDC device/driver to bind to,
> > > while GadgetFS currently binds to the first available UDC.
> > >
> > > -4. Raw Gadget uses predictable endpoint names (handles) across different
> > > - UDCs (as long as UDCs have enough endpoints of each required transfer
> > > - type).
> > > -
> > > -5. Raw Gadget has ioctl-based interface instead of a filesystem-based
> > > one.
> > > +4. Raw Gadget has ioctl-based interface instead of a filesystem-based
> > > one.
> > >
> > > Userspace interface
> > > ~~~~~~~~~~~~~~~~~~~
> > > diff --git a/drivers/usb/gadget/legacy/raw_gadget.c
> > > b/drivers/usb/gadget/legacy/raw_gadget.c
> > > index 7b241992ad5a..e6abbc15341a 100644
> > > --- a/drivers/usb/gadget/legacy/raw_gadget.c
> > > +++ b/drivers/usb/gadget/legacy/raw_gadget.c
> > > @@ -7,6 +7,7 @@
> > > */
> > >
> > > #include <linux/compiler.h>
> > > +#include <linux/ctype.h>
> > > #include <linux/debugfs.h>
> > > #include <linux/delay.h>
> > > #include <linux/kref.h>
> > > @@ -123,8 +124,6 @@ static void raw_event_queue_destroy(struct
> > > raw_event_queue *queue)
> > >
> > > struct raw_dev;
> > >
> > > -#define USB_RAW_MAX_ENDPOINTS 32
> > > -
> > > enum ep_state {
> > > STATE_EP_DISABLED,
> > > STATE_EP_ENABLED,
> > > @@ -134,6 +133,7 @@ struct raw_ep {
> > > struct raw_dev *dev;
> > > enum ep_state state;
> > > struct usb_ep *ep;
> > > + u8 addr;
> > > struct usb_request *req;
> > > bool urb_queued;
> > > bool disabling;
> > > @@ -168,7 +168,8 @@ struct raw_dev {
> > > bool ep0_out_pending;
> > > bool ep0_urb_queued;
> > > ssize_t ep0_status;
> > > - struct raw_ep eps[USB_RAW_MAX_ENDPOINTS];
> > > + struct raw_ep eps[USB_RAW_EPS_NUM_MAX];
> > > + int eps_num;
> > >
> > > struct completion ep0_done;
> > > struct raw_event_queue queue;
> > > @@ -202,7 +203,7 @@ static void dev_free(struct kref *kref)
> > > usb_ep_free_request(dev->gadget->ep0, dev->req);
> > > }
> > > raw_event_queue_destroy(&dev->queue);
> > > - for (i = 0; i < USB_RAW_MAX_ENDPOINTS; i++) {
> > > + for (i = 0; i < dev->eps_num; i++) {
> > > if (dev->eps[i].state != STATE_EP_ENABLED)
> > > continue;
> > > usb_ep_disable(dev->eps[i].ep);
> > > @@ -249,12 +250,26 @@ static void gadget_ep0_complete(struct usb_ep *ep,
> > > struct usb_request *req)
> > > complete(&dev->ep0_done);
> > > }
> > >
> > > +static u8 get_ep_addr(const char *name)
> > > +{
> > > + /* If the endpoint has fixed function (named as e.g.
> > > "ep12out-bulk"),
> > > + * parse the endpoint address from its name. We deliberately use
> > > + * deprecated simple_strtoul() function here, as the number isn't
> > > + * followed by '\0' nor '\n'.
> > > + */
> > > + if (isdigit(name[2]))
> > > + return simple_strtoul(&name[2], NULL, 10);
> > > + /* Otherwise the endpoint is configurable (named as e.g. "ep-a"). */
> > > + return USB_RAW_EP_ADDR_ANY;
> > > +}
> > > +
> > > static int gadget_bind(struct usb_gadget *gadget,
> > > struct usb_gadget_driver *driver)
> > > {
> > > - int ret = 0;
> > > + int ret = 0, i = 0;
> > > struct raw_dev *dev = container_of(driver, struct raw_dev, driver);
> > > struct usb_request *req;
> > > + struct usb_ep *ep;
> > > unsigned long flags;
> > >
> > > if (strcmp(gadget->name, dev->udc_name) != 0)
> > > @@ -273,6 +288,13 @@ static int gadget_bind(struct usb_gadget *gadget,
> > > dev->req->context = dev;
> > > dev->req->complete = gadget_ep0_complete;
> > > dev->gadget = gadget;
> > > + gadget_for_each_ep(ep, dev->gadget) {
> > > + dev->eps[i].ep = ep;
> > > + dev->eps[i].addr = get_ep_addr(ep->name);
> > > + dev->eps[i].state = STATE_EP_DISABLED;
> > > + i++;
> > > + }
> > > + dev->eps_num = i;
> > > spin_unlock_irqrestore(&dev->lock, flags);
> > >
> > > /* Matches kref_put() in gadget_unbind(). */
> > > @@ -555,7 +577,7 @@ static void *raw_alloc_io_data(struct usb_raw_ep_io
> > > *io, void __user *ptr,
> > >
> > > if (copy_from_user(io, ptr, sizeof(*io)))
> > > return ERR_PTR(-EFAULT);
> > > - if (io->ep >= USB_RAW_MAX_ENDPOINTS)
> > > + if (io->ep >= USB_RAW_EPS_NUM_MAX)
> > > return ERR_PTR(-EINVAL);
> > > if (!usb_raw_io_flags_valid(io->flags))
> > > return ERR_PTR(-EINVAL);
> > > @@ -682,52 +704,34 @@ static int raw_ioctl_ep0_read(struct raw_dev *dev,
> > > unsigned long value)
> > > return ret;
> > > }
> > >
> > > -static bool check_ep_caps(struct usb_ep *ep,
> > > - struct usb_endpoint_descriptor *desc)
> > > -{
> > > - switch (usb_endpoint_type(desc)) {
> > > - case USB_ENDPOINT_XFER_ISOC:
> > > - if (!ep->caps.type_iso)
> > > - return false;
> > > - break;
> > > - case USB_ENDPOINT_XFER_BULK:
> > > - if (!ep->caps.type_bulk)
> > > - return false;
> > > - break;
> > > - case USB_ENDPOINT_XFER_INT:
> > > - if (!ep->caps.type_int)
> > > - return false;
> > > - break;
> > > - default:
> > > - return false;
> > > - }
> > > -
> > > - if (usb_endpoint_dir_in(desc) && !ep->caps.dir_in)
> > > - return false;
> > > - if (usb_endpoint_dir_out(desc) && !ep->caps.dir_out)
> > > - return false;
> > > -
> > > - return true;
> > > -}
> > > -
> > > static int raw_ioctl_ep_enable(struct raw_dev *dev, unsigned long value)
> > > {
> > > int ret = 0, i;
> > > unsigned long flags;
> > > - struct usb_endpoint_descriptor *desc;
> > > - struct usb_ep *ep = NULL;
> > > + struct usb_raw_ep_descs *descs;
> > > + struct usb_endpoint_descriptor *ep_desc;
> > > + struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
> > > + struct raw_ep *ep;
> > >
> > > - desc = memdup_user((void __user *)value, sizeof(*desc));
> > > - if (IS_ERR(desc))
> > > - return PTR_ERR(desc);
> > > + descs = memdup_user((void __user *)value, sizeof(*descs));
> > > + if (IS_ERR(descs))
> > > + return PTR_ERR(descs);
> > > +
> > > + ep_desc = &descs->ep;
> > > + /*
> > > + * Assume that SuperSpeed endpoint companion descriptor is not
> > > present
> > > + * if its length is 0.
> > > + */
> > > + if (descs->comp.bLength != 0)
> > > + comp_desc = &descs->comp;
> > >
> > > /*
> > > * Endpoints with a maxpacket length of 0 can cause crashes in UDC
> > > * drivers.
> > > */
> > > - if (usb_endpoint_maxp(desc) == 0) {
> > > + if (usb_endpoint_maxp(ep_desc) == 0) {
> > > dev_dbg(dev->dev, "fail, bad endpoint maxpacket\n");
> > > - kfree(desc);
> > > + kfree(descs);
> > > return -EINVAL;
> > > }
> > >
> > > @@ -743,41 +747,34 @@ static int raw_ioctl_ep_enable(struct raw_dev *dev,
> > > unsigned long value)
> > > goto out_free;
> > > }
> > >
> > > - for (i = 0; i < USB_RAW_MAX_ENDPOINTS; i++) {
> > > - if (dev->eps[i].state == STATE_EP_ENABLED)
> > > + for (i = 0; i < dev->eps_num; i++) {
> > > + ep = &dev->eps[i];
> > > + if (ep->state != STATE_EP_DISABLED)
> > > continue;
> > > - break;
> > > - }
> > > - if (i == USB_RAW_MAX_ENDPOINTS) {
> > > - dev_dbg(&dev->gadget->dev,
> > > - "fail, no device endpoints available\n");
> > > - ret = -EBUSY;
> > > - goto out_free;
> > > - }
> > > -
> > > - gadget_for_each_ep(ep, dev->gadget) {
> > > - if (ep->enabled)
> > > + if (ep->addr != usb_endpoint_num(ep_desc) &&
> > > + ep->addr != USB_RAW_EP_ADDR_ANY)
> > > continue;
> > > - if (!check_ep_caps(ep, desc))
> > > + if (!usb_gadget_ep_match_desc(dev->gadget, ep->ep,
> > > + ep_desc, comp_desc))
> > > continue;
> > > - ep->desc = desc;
> > > - ret = usb_ep_enable(ep);
> > > + ep->ep->desc = ep_desc;
> > > + ep->ep->comp_desc = comp_desc;
> > > + ret = usb_ep_enable(ep->ep);
> > > if (ret < 0) {
> > > dev_err(&dev->gadget->dev,
> > > "fail, usb_ep_enable returned %d\n", ret);
> > > goto out_free;
> > > }
> > > - dev->eps[i].req = usb_ep_alloc_request(ep, GFP_ATOMIC);
> > > - if (!dev->eps[i].req) {
> > > + ep->req = usb_ep_alloc_request(ep->ep, GFP_ATOMIC);
> > > + if (!ep->req) {
> > > dev_err(&dev->gadget->dev,
> > > "fail, usb_ep_alloc_request failed\n");
> > > - usb_ep_disable(ep);
> > > + usb_ep_disable(ep->ep);
> > > ret = -ENOMEM;
> > > goto out_free;
> > > }
> > > - dev->eps[i].ep = ep;
> > > - dev->eps[i].state = STATE_EP_ENABLED;
> > > - ep->driver_data = &dev->eps[i];
> > > + ep->state = STATE_EP_ENABLED;
> > > + ep->ep->driver_data = ep;
> > > ret = i;
> > > goto out_unlock;
> > > }
> > > @@ -786,7 +783,7 @@ static int raw_ioctl_ep_enable(struct raw_dev *dev,
> > > unsigned long value)
> > > ret = -EBUSY;
> > >
> > > out_free:
> > > - kfree(desc);
> > > + kfree(descs);
> > > out_unlock:
> > > spin_unlock_irqrestore(&dev->lock, flags);
> > > return ret;
> > > @@ -796,10 +793,6 @@ static int raw_ioctl_ep_disable(struct raw_dev *dev,
> > > unsigned long value)
> > > {
> > > int ret = 0, i = value;
> > > unsigned long flags;
> > > - const void *desc;
> > > -
> > > - if (i < 0 || i >= USB_RAW_MAX_ENDPOINTS)
> > > - return -EINVAL;
> > >
> > > spin_lock_irqsave(&dev->lock, flags);
> > > if (dev->state != STATE_DEV_RUNNING) {
> > > @@ -812,6 +805,11 @@ static int raw_ioctl_ep_disable(struct raw_dev *dev,
> > > unsigned long value)
> > > ret = -EBUSY;
> > > goto out_unlock;
> > > }
> > > + if (i < 0 || i >= dev->eps_num) {
> > > + dev_dbg(dev->dev, "fail, invalid endpoint\n");
> > > + ret = -EBUSY;
> > > + goto out_unlock;
> > > + }
> > > if (dev->eps[i].state != STATE_EP_ENABLED) {
> > > dev_dbg(&dev->gadget->dev, "fail, endpoint is not
> > > enabled\n");
> > > ret = -EINVAL;
> > > @@ -836,10 +834,13 @@ static int raw_ioctl_ep_disable(struct raw_dev
> > > *dev, unsigned long value)
> > >
> > > spin_lock_irqsave(&dev->lock, flags);
> > > usb_ep_free_request(dev->eps[i].ep, dev->eps[i].req);
> > > - desc = dev->eps[i].ep->desc;
> > > + /*
> > > + * This kfree() frees both endpoint and SuperSpeed
> > > + * endpoint companion descriptors.
> > > + */
> > > + kfree(dev->eps[i].ep->desc);
> > > dev->eps[i].ep = NULL;
> > > dev->eps[i].state = STATE_EP_DISABLED;
> > > - kfree(desc);
> > > dev->eps[i].disabling = false;
> > >
> > > out_unlock:
> > > @@ -868,7 +869,7 @@ static int raw_process_ep_io(struct raw_dev *dev,
> > > struct usb_raw_ep_io *io,
> > > {
> > > int ret = 0;
> > > unsigned long flags;
> > > - struct raw_ep *ep = &dev->eps[io->ep];
> > > + struct raw_ep *ep;
> > > DECLARE_COMPLETION_ONSTACK(done);
> > >
> > > spin_lock_irqsave(&dev->lock, flags);
> > > @@ -882,6 +883,12 @@ static int raw_process_ep_io(struct raw_dev *dev,
> > > struct usb_raw_ep_io *io,
> > > ret = -EBUSY;
> > > goto out_unlock;
> > > }
> > > + if (io->ep >= dev->eps_num) {
> > > + dev_dbg(&dev->gadget->dev, "fail, invalid endpoint\n");
> > > + ret = -EINVAL;
> > > + goto out_unlock;
> > > + }
> > > + ep = &dev->eps[io->ep];
> > > if (ep->state != STATE_EP_ENABLED) {
> > > dev_dbg(&dev->gadget->dev, "fail, endpoint is not
> > > enabled\n");
> > > ret = -EBUSY;
> > > @@ -1027,6 +1034,71 @@ static int raw_ioctl_vbus_draw(struct raw_dev
> > > *dev, unsigned long value)
> > > return ret;
> > > }
> > >
> > > +static void fill_ep_caps(struct usb_ep_caps *caps,
> > > + struct usb_raw_ep_caps *raw_caps)
> > > +{
> > > + raw_caps->type_control = caps->type_control;
> > > + raw_caps->type_iso = caps->type_iso;
> > > + raw_caps->type_bulk = caps->type_bulk;
> > > + raw_caps->type_int = caps->type_int;
> > > + raw_caps->dir_in = caps->dir_in;
> > > + raw_caps->dir_out = caps->dir_out;
> > > +}
> > > +
> > > +static void fill_ep_limits(struct usb_ep *ep, struct usb_raw_ep_limits
> > > *limits)
> > > +{
> > > + limits->maxpacket_limit = ep->maxpacket_limit;
> > > + limits->max_streams = ep->max_streams;
> > > +}
> > > +
> > > +static int raw_ioctl_eps_info(struct raw_dev *dev, unsigned long value)
> > > +{
> > > + int ret = 0, i;
> > > + unsigned long flags;
> > > + struct usb_raw_eps_info *info;
> > > + struct raw_ep *ep;
> > > +
> > > + info = kmalloc(sizeof(*info), GFP_KERNEL);
> > > + if (!info) {
> > > + ret = -ENOMEM;
> > > + goto out;
> > > + }
> > > +
> > > + spin_lock_irqsave(&dev->lock, flags);
> > > + if (dev->state != STATE_DEV_RUNNING) {
> > > + dev_dbg(dev->dev, "fail, device is not running\n");
> > > + ret = -EINVAL;
> > > + spin_unlock_irqrestore(&dev->lock, flags);
> > > + goto out_free;
> > > + }
> > > + if (!dev->gadget) {
> > > + dev_dbg(dev->dev, "fail, gadget is not bound\n");
> > > + ret = -EBUSY;
> > > + spin_unlock_irqrestore(&dev->lock, flags);
> > > + goto out_free;
> > > + }
> > > +
> > > + memset(info, 0, sizeof(*info));
> > > + for (i = 0; i < dev->eps_num; i++) {
> > > + ep = &dev->eps[i];
> > > + strscpy(&info->eps[i].name[0], ep->ep->name,
> > > + USB_RAW_EP_NAME_MAX);
> > > + info->eps[i].addr = ep->addr;
> > > + fill_ep_caps(&ep->ep->caps, &info->eps[i].caps);
> > > + fill_ep_limits(ep->ep, &info->eps[i].limits);
> > > + }
> > > + ret = dev->eps_num;
> > > + spin_unlock_irqrestore(&dev->lock, flags);
> > > +
> > > + if (copy_to_user((void __user *)value, info, sizeof(*info)))
> > > + ret = -EFAULT;
> > > +
> > > +out_free:
> > > + kfree(info);
> > > +out:
> > > + return ret;
> > > +}
> > > +
> > > static long raw_ioctl(struct file *fd, unsigned int cmd, unsigned long
> > > value)
> > > {
> > > struct raw_dev *dev = fd->private_data;
> > > @@ -1069,6 +1141,9 @@ static long raw_ioctl(struct file *fd, unsigned int
> > > cmd, unsigned long value)
> > > case USB_RAW_IOCTL_VBUS_DRAW:
> > > ret = raw_ioctl_vbus_draw(dev, value);
> > > break;
> > > + case USB_RAW_IOCTL_EPS_INFO:
> > > + ret = raw_ioctl_eps_info(dev, value);
> > > + break;
> > > default:
> > > ret = -EINVAL;
> > > }
> > > diff --git a/include/uapi/linux/usb/raw_gadget.h
> > > b/include/uapi/linux/usb/raw_gadget.h
> > > index 8544802b25bd..722124fff290 100644
> > > --- a/include/uapi/linux/usb/raw_gadget.h
> > > +++ b/include/uapi/linux/usb/raw_gadget.h
> > > @@ -93,6 +93,79 @@ struct usb_raw_ep_io {
> > > __u8 data[0];
> > > };
> > >
> > > +/*
> > > + * struct usb_raw_ep_descs - argument for USB_RAW_IOCTL_EP_ENABLE ioctl.
> > > + * @ep: Endpoint descriptor.
> > > + * @comp: SuperSpeed Endpoint Companion descriptor.
> > > + *
> > > + * Both of these descriptors are only used by the gadget subsystem
> > > including
> > > + * the UDC driver and don't affect the descriptors that are sent to the
> > > host.
> > > + * However, the user must make sure that the host and the gadget sides
> > > agree
> > > + * on the used endpoint parameters (such as e.g. endpoint addresses).
> > > + */
> > > +struct usb_raw_ep_descs {
> > > + struct usb_endpoint_descriptor ep;
> > > + struct usb_ss_ep_comp_descriptor comp;
> > > +};
> > > +
> > > +/* Maximum number of non-control endpoints in struct usb_raw_eps_info. */
> > > +#define USB_RAW_EPS_NUM_MAX 30
> > > +
> > > +/* Maximum length of UDC endpoint name in struct usb_raw_ep_info. */
> > > +#define USB_RAW_EP_NAME_MAX 16
> > > +
> > > +/* Used as addr in struct usb_raw_ep_info if endpoint accepts any
> > > address. */
> > > +#define USB_RAW_EP_ADDR_ANY 0xff
> > > +
> > > +/*
> > > + * struct usb_raw_ep_caps - exposes endpoint capabilities from struct
> > > usb_ep
> > > + * (technically from its member struct usb_ep_caps).
> > > + */
> > > +struct usb_raw_ep_caps {
> > > + __u32 type_control : 1;
> > > + __u32 type_iso : 1;
> > > + __u32 type_bulk : 1;
> > > + __u32 type_int : 1;
> > > + __u32 dir_in : 1;
> > > + __u32 dir_out : 1;
> > > +};
> > > +
> > > +/*
> > > + * struct usb_raw_ep_limits - exposes endpoint limits from struct usb_ep.
> > > + * @maxpacket_limit: Maximum packet size value supported by this
> > > endpoint.
> > > + * @max_streams: maximum number of streams supported by this endpoint
> > > + * (actual number is 2^n).
> > > + * @reserved: Empty, reserved for potential future extensions.
> > > + */
> > > +struct usb_raw_ep_limits {
> > > + __u16 maxpacket_limit;
> > > + __u16 max_streams;
> > > + __u32 reserved;
> > > +};
> > > +
> > > +/*
> > > + * struct usb_raw_ep_info - stores information about a gadget endpoint.
> > > + * @name: Name of the endpoint as it is defined in the UDC driver.
> > > + * @addr: Address of the endpoint that must be specified in the endpoint
> > > + * descriptor passed to USB_RAW_IOCTL_EP_ENABLE ioctl.
> > > + * @caps: Endpoint capabilities.
> > > + * @limits: Endpoint limits.
> > > + */
> > > +struct usb_raw_ep_info {
> > > + __u8 name[USB_RAW_EP_NAME_MAX];
> > > + __u32 addr;
> > > + struct usb_raw_ep_caps caps;
> > > + struct usb_raw_ep_limits limits;
> > > +};
> > > +
> > > +/*
> > > + * struct usb_raw_eps_info - argument for USB_RAW_IOCTL_EPS_INFO ioctl.
> > > + * eps: Structures that store information about non-control endpoints.
> > > + */
> > > +struct usb_raw_eps_info {
> > > + struct usb_raw_ep_info eps[USB_RAW_EPS_NUM_MAX];
> > > +};
> > > +
> > > /*
> > > * Initializes a Raw Gadget instance.
> > > * Accepts a pointer to the usb_raw_init struct as an argument.
> > > @@ -126,12 +199,12 @@ struct usb_raw_ep_io {
> > > #define USB_RAW_IOCTL_EP0_READ _IOWR('U', 4, struct
> > > usb_raw_ep_io)
> > >
> > > /*
> > > - * Finds an endpoint that supports the transfer type specified in the
> > > - * descriptor and enables it.
> > > - * Accepts a pointer to the usb_endpoint_descriptor struct as an
> > > argument.
> > > + * Finds an endpoint that satisfies the parameters specified in the
> > > provided
> > > + * descriptors (address, transfer type, etc.) and enables it.
> > > + * Accepts a pointer to the usb_raw_ep_descs struct as an argument.
> > > * Returns enabled endpoint handle on success or negative error code on
> > > failure.
> > > */
> > > -#define USB_RAW_IOCTL_EP_ENABLE _IOW('U', 5, struct
> > > usb_endpoint_descriptor)
> > > +#define USB_RAW_IOCTL_EP_ENABLE _IOW('U', 5, struct
> > > usb_raw_ep_descs)
> > >
> > > /* Disables specified endpoint.
> > > * Accepts endpoint handle as an argument.
> > > @@ -164,4 +237,11 @@ struct usb_raw_ep_io {
> > > */
> > > #define USB_RAW_IOCTL_VBUS_DRAW _IOW('U', 10, __u32)
> > >
> > > +/* Fills in the usb_raw_eps_info structure with information about
> > > non-control
> > > + * endpoints available for the currently connected UDC.
> > > + * Returns the number of available endpoints on success or negative
> > > error code
> > > + * on failure.
> > > + */
> > > +#define USB_RAW_IOCTL_EPS_INFO _IOR('U', 11, struct
> > > usb_raw_eps_info)
> > > +
> >
> > here you're changing userspace ABI. Aren't we going to possibly break
> > some existing applications?
>
> Hi Felipe,
>
> I've been working on tests for Raw Gadget for the last few weeks [1],
> which revealed a few problems with the interface. This isn't yet
> included into any released kernel, so my hope that changing the ABI is
> OK during the rc stage.
Let me know what you would prefer here though. It might make sense to
keep the current ioctl for USB2-only devices and add another one for
USB3 in the future. I'm not sure what's the best design decision.
I also have another patch (which I haven't sent yet) that changes the
ABI a bit to support USB3 streams. But same, I don't know if it's
better to change the ABI right now or to expand it by adding another
ioctl in the future.
