Hi Michal,

Thank you for your review. I will implement the changes in a v2 series.

W dniu 02.07.2014 14:56, Michal Nazarewicz pisze:
On Wed, Jul 02 2014, Andrzej Pietrasiewicz <andrze...@samsung.com> wrote:
Add support for OS descriptors. The new format of descriptors is used,
because the "flags" field is required for extensions. os_count gives
the number of OSDesc[] elements.
The format of descriptors is given in include/uapi/linux/usb/functionfs.h.


<snip>


For now I've only skimmed over binding code.


  drivers/usb/gadget/function/f_fs.c  | 345 +++++++++++++++++++++++++++++++++++-
  drivers/usb/gadget/function/u_fs.h  |   7 +
  include/uapi/linux/usb/functionfs.h |  87 ++++++++-
  3 files changed, 432 insertions(+), 7 deletions(-)


<snip>


+/*
+ * Process all extended compatibility/extended property descriptors
+ * of a feature descriptor
+ */
+static int __must_check ffs_do_os_desc(char *data, unsigned len, u8 *valuep,
+                                      u16 feature_count,
+                                      ffs_os_desc_callback entity, void *priv,
+                                      struct usb_os_desc_header *desc)

The name of this function has to be changed.  Perhaps
ffs_do_os_single_desc?  It took me embarrassing amount of time to figure
out that ffs_do_os_descs and ffs_do_os_desc are different functions.


Would you like me to add a patch which does the same thing to the
"ffs_do_descs()/ffs_do_desc()" pair which has already been there?

<snip>

+#define FMT "invalid os descriptor length: %d pnl:%d pdl:%d (descriptor %d)\n"
+                       pr_vdebug(FMT, length, pnl, pdl, type);
+#undef FMT

Uh?  Why the #define?


To avoid a line over 80 chars. Any better way?


+                       return -EINVAL;
+               }
+               ++ffs->ms_os_descs_ext_prop_count;
+               ffs->ms_os_descs_ext_prop_name_len += (pnl << 1);

Why is pnl multiplied by two?

Strings in Feature Descriptors (Extended Properties, Extended Compatibility)
are returned to the host as "WCHAR"s, and so "property name length" must
reflect this.


AP

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to