From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Nazarewicz Subject: [RFC] usb: gadget: f_fs: Add flags to descriptors block Date: Tue, 25 Feb 2014 15:01:33 +0100 Message-ID: References: <1387877408-17567-1-git-send-email-mgautam@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mail-ea0-f182.google.com ([209.85.215.182]:64733 "EHLO mail-ea0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752072AbaBYOyR convert rfc822-to-8bit (ORCPT ); Tue, 25 Feb 2014 09:54:17 -0500 Received: by mail-ea0-f182.google.com with SMTP id r15so176673ead.41 for ; Tue, 25 Feb 2014 06:54:16 -0800 (PST) In-Reply-To: <1387877408-17567-1-git-send-email-mgautam@codeaurora.org> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: balbi@ti.com, jackp@codeaurora.org, pheatwol@codeaurora.org Cc: linux-usb@vger.kernel.org, linux-arm-msm@vger.kernel.org, benoit@android.com, andrzej.p@samsung.com, gregkh@linuxfoundation.org, Manu Gautam 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 --- Just to repeat what I wrote on my phone which is unable to send text/plain messages: On Tue, Feb 25 2014, Micha=C5=82 Nazarewicz wrote: > On Feb 25, 2014 5:47 AM, "Manu Gautam" wrote= : >> Is this patch good now to be added to your queue (if not already in)= ? >> It applied cleanly on your next branch. > > Actually, recent LWN article about flags in syscalls got me thinking = and > maybe it's good this patch is not yet in Felipe's queue. > > I admit I screwed up when I designed the headers format, but now that= we > change it to accommodate SuperSpeed, I think we should go ahead and c= hange > the format by adding flags field. > > [=E2=80=A6] > > Sorry to mention this only now, but I believe that's the right thing = to do. This patch implements this new format and in doing so simplifies, in my opinion, the code a bit. It needs to be applied on top of Manu's patch.=20 It has not been tested at all, not even compile-tested. This is because whenever I try to compile the kernel, I get this nice message in my logs: mce: [Hardware Error]: Machine check events logged thermal_sys: Critical temperature reached(100 C),shutting down thermal_sys: Critical temperature reached(100 C),shutting down Sorry about that. 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(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 928959d..7d01bd6 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); =20 - 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); =20 ffs->epfiles =3D NULL; + ffs->raw_descs_data =3D NULL; ffs->raw_descs =3D NULL; ffs->raw_strings =3D NULL; ffs->stringtabs =3D NULL; =20 - ffs->raw_fs_hs_descs_length =3D 0; - ffs->raw_ss_descs_length =3D 0; + ffs->real_descs_length =3D 0; ffs->fs_descs_count =3D 0; ffs->hs_descs_count =3D 0; ffs->ss_descs_count =3D 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 =3D 0; - int fs_len, hs_len, ss_len, ret =3D -EINVAL; - char *data =3D _data; + char *data =3D _data, *raw_descs; + unsigned counts[3], flags; + int ret =3D -EINVAL, i; =20 ENTER(); =20 - if (unlikely(get_unaligned_le32(data) !=3D FUNCTIONFS_DESCRIPTORS_MAG= IC || - get_unaligned_le32(data + 4) !=3D len)) + if (get_unaligned_le32(data + 4) !=3D len) goto error; - fs_count =3D get_unaligned_le32(data + 8); - hs_count =3D get_unaligned_le32(data + 12); - - data +=3D 16; - len -=3D 16; =20 - if (likely(fs_count)) { - fs_len =3D ffs_do_descs(fs_count, data, len, - __ffs_data_do_entity, ffs); - if (unlikely(fs_len < 0)) { - ret =3D fs_len; + switch (get_unaligned_le32(data)) { + case FUNCTIONFS_DESCRIPTORS_MAGIC: + flags =3D FUNCTIONFS_HAS_FS_DESC | FUNCTIONFS_HAS_HS_DESC; + data +=3D 8; + len -=3D 8; + break; + case FUNCTIONFS_DESCRIPTORS_MAGIC_V2: + flags =3D get_unaligned_le32(data + 8); + if (flags & ~(FUNCTIONFS_HAS_FS_DESC | + FUNCTIONFS_HAS_HS_DESC | + FUNCTIONFS_HAS_SS_DESC)) { + ret =3D -ENOSYS; goto error; } - - data +=3D fs_len; - len -=3D fs_len; - } else { - fs_len =3D 0; + data +=3D 12; + len -=3D 12; + break; + default: + goto error; } =20 - if (likely(hs_count)) { - hs_len =3D ffs_do_descs(hs_count, data, len, - __ffs_data_do_entity, ffs); - if (unlikely(hs_len < 0)) { - ret =3D hs_len; + /* Read fs_count, hs_count and ss_count (if present) */ + for (i =3D 0; i < 3; ++i) { + if (!(flags & (1 << i))) { + counts[i] =3D 0; + } else if (len < 4) { goto error; + } else { + counts[i] =3D get_unaligned_le32(data); + data +=3D 4; + len -=3D 4; } - - data +=3D hs_len; - len -=3D hs_len; - } else { - hs_len =3D 0; } =20 - if (len >=3D 8) { - /* Check SS_MAGIC for presence of ss_descs and get SS_COUNT */ - if (get_unaligned_le32(data) !=3D FUNCTIONFS_SS_DESC_MAGIC) - goto einval; - - ss_count =3D get_unaligned_le32(data + 4); - data +=3D 8; - len -=3D 8; - } - - if (!fs_count && !hs_count && !ss_count) - goto einval; - - if (ss_count) { - ss_len =3D ffs_do_descs(ss_count, data, len, + /* Read descriptors */ + raw_descs =3D data; + for (i =3D 0; i < 3; ++i) { + if (!counts[i]) + continue; + ret =3D ffs_do_descs(descs[i].count, data, len, __ffs_data_do_entity, ffs); - if (unlikely(ss_len < 0)) { - ret =3D ss_len; + if (ret < 0) goto error; - } - ret =3D ss_len; - } else { - ss_len =3D 0; - ret =3D 0; + data +=3D ret; + len -=3D ret; } =20 - if (unlikely(len !=3D ret)) - goto einval; + if (raw_descs =3D=3D data || len) { + ret =3D -EINVAL; + goto error; + } =20 - ffs->raw_fs_hs_descs_length =3D fs_len + hs_len; - ffs->raw_ss_descs_length =3D ss_len; - ffs->raw_descs =3D _data; - ffs->fs_descs_count =3D fs_count; - ffs->hs_descs_count =3D hs_count; - ffs->ss_descs_count =3D ss_count; + ffs->raw_descs_data =3D _data; + ffs->raw_descs =3D raw_descs; + ffs->raw_descs_length =3D data - raw_descs; + ffs->fs_descs_count =3D counts[0]; + ffs->hs_descs_count =3D counts[1]; + ffs->ss_descs_count =3D counts[2]; =20 return 0; =20 -einval: - ret =3D -EINVAL; error: kfree(_data); return ret; @@ -2092,8 +2079,7 @@ static int _ffs_func_bind(struct usb_configuratio= n *c, vla_item_with_sz(d, struct usb_descriptor_header *, ss_descs, super ? ffs->ss_descs_count + 1 : 0); vla_item_with_sz(d, short, inums, ffs->interfaces_count); - vla_item_with_sz(d, char, raw_descs, - ffs->raw_fs_hs_descs_length + ffs->raw_ss_descs_length); + vla_item_with_sz(d, char, raw_descs, ffs->raw_raw_descs_length); char *vlabuf; =20 ENTER(); @@ -2109,15 +2095,9 @@ static int _ffs_func_bind(struct usb_configurati= on *c, =20 /* Zero */ memset(vla_ptr(vlabuf, d, eps), 0, d_eps__sz); - /* Copy only raw (hs,fs) descriptors (until ss_magic and ss_count) */ - memcpy(vla_ptr(vlabuf, d, raw_descs), ffs->raw_descs + 16, - ffs->raw_fs_hs_descs_length); - /* Copy SS descs present @ header + hs_fs_descs + ss_magic + ss_count= */ - if (func->ffs->ss_descs_count) - memcpy(vla_ptr(vlabuf, d, raw_descs) + - ffs->raw_fs_hs_descs_length, - ffs->raw_descs + 16 + ffs->raw_fs_hs_descs_length + 8, - ffs->raw_ss_descs_length); + /* Copy descriptors */ + memcpy(vla_ptr(vlabuf, d, raw_descs), ffs->raw_descs, + ffs->raw_descs_length); =20 memset(vla_ptr(vlabuf, d, inums), 0xff, d_inums__sz); for (ret =3D ffs->eps_count; ret; --ret) { @@ -2599,7 +2579,7 @@ static int _ffs_name_dev(struct ffs_dev *dev, con= st char *name) existing =3D _ffs_find_dev(name); if (existing) return -EBUSY; -=09 + dev->name =3D name; =20 return 0; @@ -2682,7 +2662,7 @@ static void ffs_release_dev(struct ffs_data *ffs_= data) ffs_dev =3D ffs_data->private_data; if (ffs_dev) ffs_dev->mounted =3D false; -=09 + if (ffs_dev->ffs_release_dev_callback) ffs_dev->ffs_release_dev_callback(ffs_dev); =20 diff --git a/drivers/usb/gadget/u_fs.h b/drivers/usb/gadget/u_fs.h index 08bf26e..866c848 100644 --- a/drivers/usb/gadget/u_fs.h +++ b/drivers/usb/gadget/u_fs.h @@ -210,15 +210,13 @@ struct ffs_data { =20 /* filled by __ffs_data_got_descs() */ /* - * Real descriptors are 16 bytes after raw_descs (so you need - * to skip 16 bytes (ie. ffs->raw_descs + 16) to get to the - * first full speed descriptor). - * raw_fs_hs_descs_length does not have those 16 bytes added. - * ss_descs are 8 bytes (ss_magic + count) pass the hs_descs + * raw_descs is what you kfree, real_descs points inside of raw_descs= , + * where full speed, high speed and super speed descriptors start. + * real_descs_length is the length of all those descriptors. */ const void *raw_descs; - unsigned raw_fs_hs_descs_length; - unsigned raw_ss_descs_length; + const void *real_descs; + unsigned real_descs_length; unsigned fs_descs_count; unsigned hs_descs_count; unsigned ss_descs_count; diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/u= sb/functionfs.h index 0f8f7be..ee6fcbc 100644 --- a/include/uapi/linux/usb/functionfs.h +++ b/include/uapi/linux/usb/functionfs.h @@ -10,7 +10,14 @@ =20 enum { FUNCTIONFS_DESCRIPTORS_MAGIC =3D 1, - FUNCTIONFS_STRINGS_MAGIC =3D 2 + FUNCTIONFS_STRINGS_MAGIC =3D 2, + FUNCTIONFS_DESCRIPTORS_MAGIC_V2 =3D 3, +}; + +enum functionfs_flags { + FUNCTIONFS_HAS_FS_DESC =3D 1, + FUNCTIONFS_HAS_HS_DESC =3D 2, + FUNCTIONFS_HAS_SS_DESC =3D 4, }; =20 #define FUNCTIONFS_SS_DESC_MAGIC 0x0055DE5C @@ -30,33 +37,39 @@ struct usb_endpoint_descriptor_no_audio { =20 =20 /* - * 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)); - -/* * Descriptors format: * * | off | name | type | description = | * |-----+-----------+--------------+---------------------------------= -----| - * | 0 | magic | LE32 | FUNCTIONFS_{FS,HS}_DESCRIPTORS_M= AGIC | + * | 0 | magic | LE32 | FUNCTIONFS_DESCRIPTORS_MAGIC_V2 = | + * | 4 | length | LE32 | length of the whole data chunk = | + * | 8 | flags | LE32 | combination of functionfs_flags = | + * | | fs_count | LE32 | number of full-speed descriptors= | + * | | hs_count | LE32 | number of high-speed descriptors= | + * | | ss_count | LE32 | number of high-speed descriptors= | + * | | fs_descrs | Descriptor[] | list of full-speed descriptors = | + * | | hs_descrs | Descriptor[] | list of high-speed descriptors = | + * | | ss_descrs | Descriptor[] | list of high-speed descriptors = | + * + * 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. + * + * Legacy descriptors format: + * + * | off | name | type | description = | + * |-----+-----------+--------------+---------------------------------= -----| + * | 0 | magic | LE32 | FUNCTIONFS_DESCRIPTORS_MAGIC = | * | 4 | length | LE32 | length of the whole data chunk = | * | 8 | fs_count | LE32 | number of full-speed descriptors= | * | 12 | hs_count | LE32 | number of high-speed descriptors= | * | 16 | fs_descrs | Descriptor[] | list of full-speed descriptors = | * | | hs_descrs | Descriptor[] | list of high-speed descriptors = | - * | | ss_magic | LE32 | FUNCTIONFS_SS_DESC_MAGIC = | - * | | ss_count | LE32 | number of super-speed descriptor= s | - * | | ss_descrs | Descriptor[] | list of super-speed descriptors = | * - * ss_magic: if present then it implies that SS_DESCs are also present - * descs are just valid USB descriptors and have the following format: + * All numbers must be in little endian order. + * + * Descriptor[] is an array of valid USB descriptors which have the fo= llowing + * format: * * | off | name | type | description | * |-----+-----------------+------+--------------------------| --=20 1.9.0.rc1.175.g0b1dcb5