RE: [PATCHv2] usb: gadget: f_fs: Add flags to descriptors block
Hi,
-Original Message-
From: Michal Nazarewicz [mailto:m...@google.com] On Behalf Of Michal
Nazarewicz
Sent: Wednesday, June 04, 2014 9:06 PM
To: Krzysztof Opasiak; 'Manu Gautam'
Cc: ba...@ti.com; ja...@codeaurora.org; pheat...@codeaurora.org;
linux-...@vger.kernel.org; linux-arm-msm@vger.kernel.org;
ben...@android.com; Andrzej Pietrasiewicz;
gre...@linuxfoundation.org; 'Manu Gautam'; Karol Lewandowski; Marek
Szyprowski
Subject: Re: [PATCHv2] usb: gadget: f_fs: Add flags to descriptors
block
-struct usb_functionfs_descs_head {
- __le32 magic;
- __le32 length;
- __le32 fs_count;
- __le32 hs_count;
-} __attribute__((packed));
On Wed, Jun 04 2014, Krzysztof Opasiak k.opas...@samsung.com
wrote:
I have tried to compile FFS examples with v3.15-rc8 but I have
faced an
issue that they are unable to build due to missing definition of
this
structure.
https://lkml.org/lkml/2014/5/21/522
Thank you for this link, I have missed that patch.
I don't care either way to be honest. I guess if there's non-
negligible
number of usb_functionfs_descs_head structure users out there, I
can
prepare a CL adding the structure back.
The number of FFS users which I know is limited to 3 maybe 4, so it's not too
much. I can port all of them to new API but I'm not sure if we would like to do
this?
There is also no structure definition for new API, maybe suitable
structure should be defined (struct usb_functionfs_descs_head2
for
example) to make userspace life easier?
That structure would not have many fields though, since what
exactly the
header contains depends on the flags. Whether fs_count, hs_count
and
ss_count fields are present depends on which bits in the flags are
set.
So the usb_functionfs_descs_head2 structure could only contain:
struct usb_functionfs_descs_head2 {
__le32 magic;
__le32 length;
__le32 flags;
};
I'm not sure if that would be particularly helpful.
I think that this structure is much more user friendly than copy-paste of those
3 fields in each ffs program.
--
BR's
Krzysztof Opasiak
Samsung RD Institute Poland
Samsung Electronics
k.opas...@samsung.com
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCHv2] usb: gadget: f_fs: Add flags to descriptors block
Hello Michal,
-Original Message-
From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
ow...@vger.kernel.org] On Behalf Of Michal Nazarewicz
Sent: Tuesday, February 25, 2014 6:02 PM
To: Manu Gautam
Cc: ba...@ti.com; ja...@codeaurora.org; pheat...@codeaurora.org;
linux-...@vger.kernel.org; linux-arm-msm@vger.kernel.org;
ben...@android.com; Andrzej Pietrasiewicz;
gre...@linuxfoundation.org; Manu Gautam
Subject: [PATCHv2] usb: gadget: f_fs: Add flags to descriptors
block
This reworks the way SuperSpeed descriptors are added and instead
of
having a magick after full and high speed descriptors, it reworks
the
whole descriptors block to include a flags field which lists which
descriptors are present and makes future extensions possible.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
drivers/usb/gadget/f_fs.c | 136 +++-
drivers/usb/gadget/u_fs.h | 12 ++--
include/uapi/linux/usb/functionfs.h | 49 -
3 files changed, 94 insertions(+), 103 deletions(-)
All right, with some fidling with my fan and limiting CPU frequency
to
the mininimum, I've managed to compile this patch and fixed all the
typos.
Manu, if you could include it in your series, adjust your user
space
client and test it, it would be wonderful. :]
(...) snip
diff --git a/include/uapi/linux/usb/functionfs.h
b/include/uapi/linux/usb/functionfs.h
index 0f8f7be..ee6fcbc 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -10,7 +10,14 @@
enum {
FUNCTIONFS_DESCRIPTORS_MAGIC = 1,
- FUNCTIONFS_STRINGS_MAGIC = 2
+ FUNCTIONFS_STRINGS_MAGIC = 2,
+ FUNCTIONFS_DESCRIPTORS_MAGIC_V2 = 3,
+};
+
+enum functionfs_flags {
+ FUNCTIONFS_HAS_FS_DESC = 1,
+ FUNCTIONFS_HAS_HS_DESC = 2,
+ FUNCTIONFS_HAS_SS_DESC = 4,
};
#define FUNCTIONFS_SS_DESC_MAGIC 0x0055DE5C
@@ -30,33 +37,39 @@ struct usb_endpoint_descriptor_no_audio {
/*
- * All numbers must be in little endian order.
- */
-
-struct usb_functionfs_descs_head {
- __le32 magic;
- __le32 length;
- __le32 fs_count;
- __le32 hs_count;
-} __attribute__((packed));
-
-/*
I have tried to compile FFS examples with v3.15-rc8 but I have faced an
issue that they are unable to build due to missing definition of this
structure. The same is with adb, sdb and all userspace apps which use
legacy API. What is the reason of removing it? Was your intentions?
Maybe would be a good idea to leave this structure untouched as legacy
userspace API?
There is also no structure definition for new API, maybe suitable
structure should be defined (struct usb_functionfs_descs_head2 for
example) to make userspace life easier?
--
BR's
Krzysztof Opasiak
Samsung RD Institute Poland
Samsung Electronics
k.opas...@samsung.com
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] usb: gadget: f_fs: Add flags to descriptors block
-struct usb_functionfs_descs_head {
-__le32 magic;
-__le32 length;
-__le32 fs_count;
-__le32 hs_count;
-} __attribute__((packed));
On Wed, Jun 04 2014, Krzysztof Opasiak k.opas...@samsung.com wrote:
I have tried to compile FFS examples with v3.15-rc8 but I have faced an
issue that they are unable to build due to missing definition of this
structure.
https://lkml.org/lkml/2014/5/21/522
The same is with adb, sdb and all userspace apps which use legacy
API. What is the reason of removing it? Was your intentions?
It was a mistake on my part.
Maybe would be a good idea to leave this structure untouched as legacy
userspace API?
I don't care either way to be honest. I guess if there's non-negligible
number of usb_functionfs_descs_head structure users out there, I can
prepare a CL adding the structure back.
There is also no structure definition for new API, maybe suitable
structure should be defined (struct usb_functionfs_descs_head2 for
example) to make userspace life easier?
That structure would not have many fields though, since what exactly the
header contains depends on the flags. Whether fs_count, hs_count and
ss_count fields are present depends on which bits in the flags are set.
So the usb_functionfs_descs_head2 structure could only contain:
struct usb_functionfs_descs_head2 {
__le32 magic;
__le32 length;
__le32 flags;
};
I'm not sure if that would be particularly helpful.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] usb: gadget: f_fs: Add flags to descriptors block
On 2/25/2014 10:32 PM, Michal Nazarewicz wrote: This reworks the way SuperSpeed descriptors are added and instead of having a magick after full and high speed descriptors, it reworks the whole descriptors block to include a flags field which lists which descriptors are present and makes future extensions possible. Signed-off-by: Michal Nazarewicz min...@mina86.com --- Manu, if you could include it in your series, adjust your user space client and test it, it would be wonderful. :] I have tested this with my userspace client. It works fine. I checked both DESC_MAGIC and DESC_MAGIC_V2 with and without passing ss_descs. I faced one issue when I was using MAGIC_V2 and specified ss_count as 0. This resulted in driver treating ss_count as start of descs blob. Later noticed that this is now changed and depending on the flags (descriptors) passed, these fields may or may not be present. I see that you have explicitly mentioned the same in functionfs.h. This looks fine to me. Additionally, there were could of typos (mentioned below) which I have fixed in the patch. #define FUNCTIONFS_SS_DESC_MAGIC 0x0055DE5C can be removed. + * | | fs_count | LE32 | number of full-speed descriptors | + * | | hs_count | LE32 | number of high-speed descriptors | + * | | ss_count | LE32 | number of high-speed descriptors | s/full/super + * | | fs_descrs | Descriptor[] | list of full-speed descriptors | + * | | hs_descrs | Descriptor[] | list of high-speed descriptors | + * | | ss_descrs | Descriptor[] | list of high-speed descriptors | s/full/super + * + * Depending on which flags are set, various fields may be missing in the + * structure. Any flags that are not recognised cause the whole block to be + * rejected with -ENOSYS. This is what I was talking about. -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] usb: gadget: f_fs: Add flags to descriptors block
This reworks the way SuperSpeed descriptors are added and instead of
having a magick after full and high speed descriptors, it reworks the
whole descriptors block to include a flags field which lists which
descriptors are present and makes future extensions possible.
Signed-off-by: Michal Nazarewicz min...@mina86.com
---
drivers/usb/gadget/f_fs.c | 136 +++-
drivers/usb/gadget/u_fs.h | 12 ++--
include/uapi/linux/usb/functionfs.h | 49 -
3 files changed, 94 insertions(+), 103 deletions(-)
All right, with some fidling with my fan and limiting CPU frequency to
the mininimum, I've managed to compile this patch and fixed all the
typos.
Manu, if you could include it in your series, adjust your user space
client and test it, it would be wonderful. :]
diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index 928959d..fab63d3 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -1167,7 +1167,7 @@ static void ffs_data_clear(struct ffs_data *ffs)
if (ffs-epfiles)
ffs_epfiles_destroy(ffs-epfiles, ffs-eps_count);
- kfree(ffs-raw_descs);
+ kfree(ffs-raw_descs_data);
kfree(ffs-raw_strings);
kfree(ffs-stringtabs);
}
@@ -1179,12 +1179,12 @@ static void ffs_data_reset(struct ffs_data *ffs)
ffs_data_clear(ffs);
ffs-epfiles = NULL;
+ ffs-raw_descs_data = NULL;
ffs-raw_descs = NULL;
ffs-raw_strings = NULL;
ffs-stringtabs = NULL;
- ffs-raw_fs_hs_descs_length = 0;
- ffs-raw_ss_descs_length = 0;
+ ffs-raw_descs_length = 0;
ffs-fs_descs_count = 0;
ffs-hs_descs_count = 0;
ffs-ss_descs_count = 0;
@@ -1598,89 +1598,76 @@ static int __ffs_data_do_entity(enum ffs_entity_type
type,
static int __ffs_data_got_descs(struct ffs_data *ffs,
char *const _data, size_t len)
{
- unsigned fs_count, hs_count, ss_count = 0;
- int fs_len, hs_len, ss_len, ret = -EINVAL;
- char *data = _data;
+ char *data = _data, *raw_descs;
+ unsigned counts[3], flags;
+ int ret = -EINVAL, i;
ENTER();
- if (unlikely(get_unaligned_le32(data) != FUNCTIONFS_DESCRIPTORS_MAGIC ||
-get_unaligned_le32(data + 4) != len))
+ if (get_unaligned_le32(data + 4) != len)
goto error;
- fs_count = get_unaligned_le32(data + 8);
- hs_count = get_unaligned_le32(data + 12);
-
- data += 16;
- len -= 16;
- if (likely(fs_count)) {
- fs_len = ffs_do_descs(fs_count, data, len,
- __ffs_data_do_entity, ffs);
- if (unlikely(fs_len 0)) {
- ret = fs_len;
+ switch (get_unaligned_le32(data)) {
+ case FUNCTIONFS_DESCRIPTORS_MAGIC:
+ flags = FUNCTIONFS_HAS_FS_DESC | FUNCTIONFS_HAS_HS_DESC;
+ data += 8;
+ len -= 8;
+ break;
+ case FUNCTIONFS_DESCRIPTORS_MAGIC_V2:
+ flags = get_unaligned_le32(data + 8);
+ if (flags ~(FUNCTIONFS_HAS_FS_DESC |
+ FUNCTIONFS_HAS_HS_DESC |
+ FUNCTIONFS_HAS_SS_DESC)) {
+ ret = -ENOSYS;
goto error;
}
-
- data += fs_len;
- len -= fs_len;
- } else {
- fs_len = 0;
+ data += 12;
+ len -= 12;
+ break;
+ default:
+ goto error;
}
- if (likely(hs_count)) {
- hs_len = ffs_do_descs(hs_count, data, len,
- __ffs_data_do_entity, ffs);
- if (unlikely(hs_len 0)) {
- ret = hs_len;
+ /* Read fs_count, hs_count and ss_count (if present) */
+ for (i = 0; i 3; ++i) {
+ if (!(flags (1 i))) {
+ counts[i] = 0;
+ } else if (len 4) {
goto error;
+ } else {
+ counts[i] = get_unaligned_le32(data);
+ data += 4;
+ len -= 4;
}
-
- data += hs_len;
- len -= hs_len;
- } else {
- hs_len = 0;
}
- if (len = 8) {
- /* Check SS_MAGIC for presence of ss_descs and get SS_COUNT */
- if (get_unaligned_le32(data) != FUNCTIONFS_SS_DESC_MAGIC)
- goto einval;
-
- ss_count = get_unaligned_le32(data + 4);
- data += 8;
- len -= 8;
- }
-
- if (!fs_count !hs_count !ss_count)
- goto einval;
-
- if (ss_count) {
- ss_len = ffs_do_descs(ss_count, data, len,
+ /* Read descriptors */
+ raw_descs = data;
+
Re: [PATCHv2] usb: gadget: f_fs: Add flags to descriptors block
Hi, On Tue, Feb 25, 2014 at 06:02:18PM +0100, Michal Nazarewicz wrote: This reworks the way SuperSpeed descriptors are added and instead of having a magick after full and high speed descriptors, it reworks the whole descriptors block to include a flags field which lists which descriptors are present and makes future extensions possible. Signed-off-by: Michal Nazarewicz min...@mina86.com --- drivers/usb/gadget/f_fs.c | 136 +++- drivers/usb/gadget/u_fs.h | 12 ++-- include/uapi/linux/usb/functionfs.h | 49 - 3 files changed, 94 insertions(+), 103 deletions(-) All right, with some fidling with my fan and limiting CPU frequency to the mininimum, I've managed to compile this patch and fixed all the typos. Manu, if you could include it in your series, adjust your user space client and test it, it would be wonderful. :] I'll wait for that then. Note that I want to close my tree next wednesday tops. -- balbi signature.asc Description: Digital signature
