* [PATCH v8 1/3] usb: gadget: f_fs: fix the redundant ep files problem
2014-09-09 6:23 [PATCH v8 0/3] usb: gadget: f_fs: userspace API fixes and improvements Robert Baldyga
@ 2014-09-09 6:23 ` Robert Baldyga
2014-09-09 15:04 ` Felipe Balbi
2014-09-09 6:23 ` [PATCH v8 2/3] usb: gadget: f_fs: add ioctl returning ep descriptor Robert Baldyga
2014-09-09 6:23 ` [PATCH v8 3/3] usb: gadget: f_fs: virtual endpoint address mapping Robert Baldyga
2 siblings, 1 reply; 6+ messages in thread
From: Robert Baldyga @ 2014-09-09 6:23 UTC (permalink / raw)
To: balbi; +Cc: gregkh, linux-usb, linux-kernel, Robert Baldyga
Up to now, when endpoint addresses in descriptors were non-consecutive,
there were created redundant files, which could cause problems in kernel,
when user tried to read/write to them. It was result of fact that maximum
endpoint address was taken as total number of endpoints in funciton.
This patch adds endpoint descriptors counting and storing their addresses
in eps_addrmap to verify their cohesion in each speed.
Endpoint address map would be also useful for further features, just like
vitual endpoint address mapping.
Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
---
drivers/usb/gadget/function/f_fs.c | 66 +++++++++++++++++++++++++++++++-------
drivers/usb/gadget/function/u_fs.h | 2 ++
2 files changed, 57 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index dc30adf..0dc3552d 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -155,6 +155,12 @@ struct ffs_io_data {
struct usb_request *req;
};
+struct ffs_desc_helper {
+ struct ffs_data *ffs;
+ unsigned interfaces_count;
+ unsigned eps_count;
+};
+
static int __must_check ffs_epfiles_create(struct ffs_data *ffs);
static void ffs_epfiles_destroy(struct ffs_epfile *epfiles, unsigned count);
@@ -1830,7 +1836,8 @@ static int __ffs_data_do_entity(enum ffs_entity_type type,
u8 *valuep, struct usb_descriptor_header *desc,
void *priv)
{
- struct ffs_data *ffs = priv;
+ struct ffs_desc_helper *helper = priv;
+ struct usb_endpoint_descriptor *d;
ENTER();
@@ -1844,8 +1851,8 @@ static int __ffs_data_do_entity(enum ffs_entity_type type,
* encountered interface "n" then there are at least
* "n+1" interfaces.
*/
- if (*valuep >= ffs->interfaces_count)
- ffs->interfaces_count = *valuep + 1;
+ if (*valuep >= helper->interfaces_count)
+ helper->interfaces_count = *valuep + 1;
break;
case FFS_STRING:
@@ -1853,14 +1860,22 @@ static int __ffs_data_do_entity(enum ffs_entity_type type,
* Strings are indexed from 1 (0 is magic ;) reserved
* for languages list or some such)
*/
- if (*valuep > ffs->strings_count)
- ffs->strings_count = *valuep;
+ if (*valuep > helper->ffs->strings_count)
+ helper->ffs->strings_count = *valuep;
break;
case FFS_ENDPOINT:
- /* Endpoints are indexed from 1 as well. */
- if ((*valuep & USB_ENDPOINT_NUMBER_MASK) > ffs->eps_count)
- ffs->eps_count = (*valuep & USB_ENDPOINT_NUMBER_MASK);
+ d = (void *)desc;
+ helper->eps_count++;
+ if (helper->eps_count >= 15)
+ return -EINVAL;
+ /* Check if descriptors for any speed were already parsed */
+ if (!helper->ffs->eps_count && !helper->ffs->interfaces_count)
+ helper->ffs->eps_addrmap[helper->eps_count] =
+ d->bEndpointAddress;
+ else if (helper->ffs->eps_addrmap[helper->eps_count] !=
+ d->bEndpointAddress)
+ return -EINVAL;
break;
}
@@ -2053,6 +2068,7 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
char *data = _data, *raw_descs;
unsigned os_descs_count = 0, counts[3], flags;
int ret = -EINVAL, i;
+ struct ffs_desc_helper helper;
ENTER();
@@ -2101,13 +2117,29 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
/* Read descriptors */
raw_descs = data;
+ helper.ffs = ffs;
for (i = 0; i < 3; ++i) {
if (!counts[i])
continue;
+ helper.interfaces_count = 0;
+ helper.eps_count = 0;
ret = ffs_do_descs(counts[i], data, len,
- __ffs_data_do_entity, ffs);
+ __ffs_data_do_entity, &helper);
if (ret < 0)
goto error;
+ if (!ffs->eps_count && !ffs->interfaces_count) {
+ ffs->eps_count = helper.eps_count;
+ ffs->interfaces_count = helper.interfaces_count;
+ } else {
+ if (ffs->eps_count != helper.eps_count) {
+ ret = -EINVAL;
+ goto error;
+ }
+ if (ffs->interfaces_count != helper.interfaces_count) {
+ ret = -EINVAL;
+ goto error;
+ }
+ }
data += ret;
len -= ret;
}
@@ -2342,9 +2374,18 @@ static void ffs_event_add(struct ffs_data *ffs,
spin_unlock_irqrestore(&ffs->ev.waitq.lock, flags);
}
-
/* Bind/unbind USB function hooks *******************************************/
+static int ffs_ep_addr2idx(struct ffs_data *ffs, u8 endpoint_address)
+{
+ int i;
+
+ for (i = 1; i < ARRAY_SIZE(ffs->eps_addrmap); ++i)
+ if (ffs->eps_addrmap[i] == endpoint_address)
+ return i;
+ return -ENOENT;
+}
+
static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
struct usb_descriptor_header *desc,
void *priv)
@@ -2378,7 +2419,10 @@ static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
if (!desc || desc->bDescriptorType != USB_DT_ENDPOINT)
return 0;
- idx = (ds->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK) - 1;
+ idx = ffs_ep_addr2idx(func->ffs, ds->bEndpointAddress) - 1;
+ if (idx < 0)
+ return idx;
+
ffs_ep = func->eps + idx;
if (unlikely(ffs_ep->descs[ep_desc_id])) {
diff --git a/drivers/usb/gadget/function/u_fs.h b/drivers/usb/gadget/function/u_fs.h
index 63d6e71..d48897e 100644
--- a/drivers/usb/gadget/function/u_fs.h
+++ b/drivers/usb/gadget/function/u_fs.h
@@ -224,6 +224,8 @@ struct ffs_data {
void *ms_os_descs_ext_prop_name_avail;
void *ms_os_descs_ext_prop_data_avail;
+ u8 eps_addrmap[15];
+
unsigned short strings_count;
unsigned short interfaces_count;
unsigned short eps_count;
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v8 1/3] usb: gadget: f_fs: fix the redundant ep files problem
2014-09-09 6:23 ` [PATCH v8 1/3] usb: gadget: f_fs: fix the redundant ep files problem Robert Baldyga
@ 2014-09-09 15:04 ` Felipe Balbi
2014-09-09 15:09 ` Felipe Balbi
0 siblings, 1 reply; 6+ messages in thread
From: Felipe Balbi @ 2014-09-09 15:04 UTC (permalink / raw)
To: Robert Baldyga; +Cc: balbi, gregkh, linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1462 bytes --]
On Tue, Sep 09, 2014 at 08:23:15AM +0200, Robert Baldyga wrote:
> Up to now, when endpoint addresses in descriptors were non-consecutive,
> there were created redundant files, which could cause problems in kernel,
> when user tried to read/write to them. It was result of fact that maximum
> endpoint address was taken as total number of endpoints in funciton.
>
> This patch adds endpoint descriptors counting and storing their addresses
> in eps_addrmap to verify their cohesion in each speed.
>
> Endpoint address map would be also useful for further features, just like
> vitual endpoint address mapping.
>
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> Acked-by: Michal Nazarewicz <mina86@mina86.com>
This is a fix, which sits in my 'fixes' branch. You can't include it
here. This series should be rebased on top of my 'next' but it should
not include any fixes, only new code.
This is why we generally ask people to split the series up into fixes
and new code and not make them depend on each other. You need to
remember that, rather frequently, my 'next' branch is based off of -rc1.
I only merge an -rc back into my 'next' when the conflicts start to
become non-trivial because of all the dependencies people add :-)
I'll see if I can fix this up myself, but if I can't, then we either
postpone this series to v3.19 or you get it done properly and without
any cross-dependencies :-)
cheers
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v8 1/3] usb: gadget: f_fs: fix the redundant ep files problem
2014-09-09 15:04 ` Felipe Balbi
@ 2014-09-09 15:09 ` Felipe Balbi
0 siblings, 0 replies; 6+ messages in thread
From: Felipe Balbi @ 2014-09-09 15:09 UTC (permalink / raw)
To: Felipe Balbi; +Cc: Robert Baldyga, gregkh, linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1808 bytes --]
On Tue, Sep 09, 2014 at 10:04:26AM -0500, Felipe Balbi wrote:
> On Tue, Sep 09, 2014 at 08:23:15AM +0200, Robert Baldyga wrote:
> > Up to now, when endpoint addresses in descriptors were non-consecutive,
> > there were created redundant files, which could cause problems in kernel,
> > when user tried to read/write to them. It was result of fact that maximum
> > endpoint address was taken as total number of endpoints in funciton.
> >
> > This patch adds endpoint descriptors counting and storing their addresses
> > in eps_addrmap to verify their cohesion in each speed.
> >
> > Endpoint address map would be also useful for further features, just like
> > vitual endpoint address mapping.
> >
> > Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> > Acked-by: Michal Nazarewicz <mina86@mina86.com>
>
> This is a fix, which sits in my 'fixes' branch. You can't include it
> here. This series should be rebased on top of my 'next' but it should
> not include any fixes, only new code.
>
> This is why we generally ask people to split the series up into fixes
> and new code and not make them depend on each other. You need to
> remember that, rather frequently, my 'next' branch is based off of -rc1.
>
> I only merge an -rc back into my 'next' when the conflicts start to
> become non-trivial because of all the dependencies people add :-)
>
> I'll see if I can fix this up myself, but if I can't, then we either
> postpone this series to v3.19 or you get it done properly and without
> any cross-dependencies :-)
alright, so patch 2 I can apply. Patch 3 I really cannot because it
depends on epaddr_map to create proper files on the filesystem.
I'll see if I can wait for -rc5 before merging patch 3, but it's likely
it'll slip into v3.19.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v8 2/3] usb: gadget: f_fs: add ioctl returning ep descriptor
2014-09-09 6:23 [PATCH v8 0/3] usb: gadget: f_fs: userspace API fixes and improvements Robert Baldyga
2014-09-09 6:23 ` [PATCH v8 1/3] usb: gadget: f_fs: fix the redundant ep files problem Robert Baldyga
@ 2014-09-09 6:23 ` Robert Baldyga
2014-09-09 6:23 ` [PATCH v8 3/3] usb: gadget: f_fs: virtual endpoint address mapping Robert Baldyga
2 siblings, 0 replies; 6+ messages in thread
From: Robert Baldyga @ 2014-09-09 6:23 UTC (permalink / raw)
To: balbi; +Cc: gregkh, linux-usb, linux-kernel, Robert Baldyga
This patch introduces ioctl named FUNCTIONFS_ENDPOINT_DESC, which
returns endpoint descriptor to userspace. It works only if function
is active.
Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
---
drivers/usb/gadget/function/f_fs.c | 23 +++++++++++++++++++++++
include/uapi/linux/usb/functionfs.h | 6 ++++++
2 files changed, 29 insertions(+)
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 0dc3552d..978a4c9 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1032,6 +1032,29 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code,
case FUNCTIONFS_ENDPOINT_REVMAP:
ret = epfile->ep->num;
break;
+ case FUNCTIONFS_ENDPOINT_DESC:
+ {
+ int desc_idx;
+ struct usb_endpoint_descriptor *desc;
+
+ switch (epfile->ffs->gadget->speed) {
+ case USB_SPEED_SUPER:
+ desc_idx = 2;
+ break;
+ case USB_SPEED_HIGH:
+ desc_idx = 1;
+ break;
+ default:
+ desc_idx = 0;
+ }
+ desc = epfile->ep->descs[desc_idx];
+
+ spin_unlock_irq(&epfile->ffs->eps_lock);
+ ret = copy_to_user((void *)value, desc, sizeof(*desc));
+ if (ret)
+ ret = -EFAULT;
+ return ret;
+ }
default:
ret = -ENOTTY;
}
diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h
index 6d2a16b..7c7a2fe 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -275,6 +275,12 @@ struct usb_functionfs_event {
*/
#define FUNCTIONFS_ENDPOINT_REVMAP _IO('g', 129)
+/*
+ * Returns endpoint descriptor. If function is not active returns -ENODEV.
+ */
+#define FUNCTIONFS_ENDPOINT_DESC _IOR('g', 130, \
+ struct usb_endpoint_descriptor)
+
#endif /* _UAPI__LINUX_FUNCTIONFS_H__ */
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v8 3/3] usb: gadget: f_fs: virtual endpoint address mapping
2014-09-09 6:23 [PATCH v8 0/3] usb: gadget: f_fs: userspace API fixes and improvements Robert Baldyga
2014-09-09 6:23 ` [PATCH v8 1/3] usb: gadget: f_fs: fix the redundant ep files problem Robert Baldyga
2014-09-09 6:23 ` [PATCH v8 2/3] usb: gadget: f_fs: add ioctl returning ep descriptor Robert Baldyga
@ 2014-09-09 6:23 ` Robert Baldyga
2 siblings, 0 replies; 6+ messages in thread
From: Robert Baldyga @ 2014-09-09 6:23 UTC (permalink / raw)
To: balbi; +Cc: gregkh, linux-usb, linux-kernel, Robert Baldyga
This patch introduces virtual endpoint address mapping. It separates
function logic form physical endpoint addresses making it more hardware
independent.
Following modifications changes user space API, so to enable them user
have to switch on the FUNCTIONFS_VIRTUAL_ADDR flag in descriptors.
Endpoints are now refered using virtual endpoint addresses chosen by
user in endpoint descpriptors. This applies to each context when endpoint
address can be used:
- when accessing endpoint files in FunctionFS filesystemi (in file name),
- in setup requests directed to specific endpoint (in wIndex field),
- in descriptors returned by FUNCTIONFS_ENDPOINT_DESC ioctl.
In endpoint file names the endpoint address number is formatted as
double-digit hexadecimal value ("ep%02x") which has few advantages -
it is easy to parse, allows to easly recognize endpoint direction basing
on its name (IN endpoint number starts with digit 8, and OUT with 0)
which can be useful for debugging purpose, and it makes easier to introduce
further features allowing to use each endpoint number in both directions
to have more endpoints available for function if hardware supports this
(for example we could have ep01 which is endpoint 1 with OUT direction,
and ep81 which is endpoint 1 with IN direction).
Physical endpoint address can be still obtained using ioctl named
FUNCTIONFS_ENDPOINT_REVMAP, but now it's not neccesary to handle
USB transactions properly.
Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
---
drivers/usb/gadget/function/f_fs.c | 23 +++++++++++++++++++++--
drivers/usb/gadget/function/u_fs.h | 2 ++
include/uapi/linux/usb/functionfs.h | 1 +
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 978a4c9..4d3a0d5 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1557,7 +1557,10 @@ static int ffs_epfiles_create(struct ffs_data *ffs)
epfile->ffs = ffs;
mutex_init(&epfile->mutex);
init_waitqueue_head(&epfile->wait);
- sprintf(epfiles->name, "ep%u", i);
+ if (ffs->user_flags & FUNCTIONFS_VIRTUAL_ADDR)
+ sprintf(epfiles->name, "ep%02x", ffs->eps_addrmap[i]);
+ else
+ sprintf(epfiles->name, "ep%u", i);
if (!unlikely(ffs_sb_create_file(ffs->sb, epfiles->name, epfile,
&ffs_epfile_operations,
&epfile->dentry))) {
@@ -2106,10 +2109,12 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
break;
case FUNCTIONFS_DESCRIPTORS_MAGIC_V2:
flags = get_unaligned_le32(data + 8);
+ ffs->user_flags = flags;
if (flags & ~(FUNCTIONFS_HAS_FS_DESC |
FUNCTIONFS_HAS_HS_DESC |
FUNCTIONFS_HAS_SS_DESC |
- FUNCTIONFS_HAS_MS_OS_DESC)) {
+ FUNCTIONFS_HAS_MS_OS_DESC |
+ FUNCTIONFS_VIRTUAL_ADDR)) {
ret = -ENOSYS;
goto error;
}
@@ -2464,7 +2469,13 @@ static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
} else {
struct usb_request *req;
struct usb_ep *ep;
+ u8 bEndpointAddress;
+ /*
+ * We back up bEndpointAddress because autoconfig overwrites
+ * it with physical endpoint address.
+ */
+ bEndpointAddress = ds->bEndpointAddress;
pr_vdebug("autoconfig\n");
ep = usb_ep_autoconfig(func->gadget, ds);
if (unlikely(!ep))
@@ -2479,6 +2490,12 @@ static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
ffs_ep->req = req;
func->eps_revmap[ds->bEndpointAddress &
USB_ENDPOINT_NUMBER_MASK] = idx + 1;
+ /*
+ * If we use virtual address mapping, we restore
+ * original bEndpointAddress value.
+ */
+ if (func->ffs->user_flags & FUNCTIONFS_VIRTUAL_ADDR)
+ ds->bEndpointAddress = bEndpointAddress;
}
ffs_dump_mem(": Rewritten ep desc", ds, ds->bLength);
@@ -2923,6 +2940,8 @@ static int ffs_func_setup(struct usb_function *f,
ret = ffs_func_revmap_ep(func, le16_to_cpu(creq->wIndex));
if (unlikely(ret < 0))
return ret;
+ if (func->ffs->user_flags & FUNCTIONFS_VIRTUAL_ADDR)
+ ret = func->ffs->eps_addrmap[ret];
break;
default:
diff --git a/drivers/usb/gadget/function/u_fs.h b/drivers/usb/gadget/function/u_fs.h
index d48897e..cd128e3 100644
--- a/drivers/usb/gadget/function/u_fs.h
+++ b/drivers/usb/gadget/function/u_fs.h
@@ -224,6 +224,8 @@ struct ffs_data {
void *ms_os_descs_ext_prop_name_avail;
void *ms_os_descs_ext_prop_data_avail;
+ unsigned user_flags;
+
u8 eps_addrmap[15];
unsigned short strings_count;
diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h
index 7c7a2fe..295ba29 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -19,6 +19,7 @@ enum functionfs_flags {
FUNCTIONFS_HAS_HS_DESC = 2,
FUNCTIONFS_HAS_SS_DESC = 4,
FUNCTIONFS_HAS_MS_OS_DESC = 8,
+ FUNCTIONFS_VIRTUAL_ADDR = 16,
};
/* Descriptor of an non-audio endpoint */
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread