[PATCHv3 19/29] usb: gadget: printer: add container_of helper for printer_dev

[Andrzej Pietrasiewicz]

5 uses of container_of() in the same context justify wrapping it
in a static inline function.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 806475c..955847f 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -89,6 +89,11 @@ struct printer_dev {
struct usb_function function;
 };
 
+static inline struct printer_dev *func_to_printer(struct usb_function *f)
+{
+   return container_of(f, struct printer_dev, function);
+}
+
 static struct printer_dev usb_printer_gadget;
 
 /*-*/
@@ -973,7 +978,7 @@ static void printer_soft_reset(struct printer_dev *dev)
 static int printer_func_setup(struct usb_function *f,
const struct usb_ctrlrequest *ctrl)
 {
-   struct printer_dev *dev = container_of(f, struct printer_dev, function);
+   struct printer_dev *dev = func_to_printer(f);
struct usb_composite_dev *cdev = f->config->cdev;
struct usb_request  *req = cdev->req;
int value = -EOPNOTSUPP;
@@ -1047,7 +1052,7 @@ static int __init printer_func_bind(struct 
usb_configuration *c,
struct usb_function *f)
 {
struct usb_gadget *gadget = c->cdev->gadget;
-   struct printer_dev *dev = container_of(f, struct printer_dev, function);
+   struct printer_dev *dev = func_to_printer(f);
struct device *pdev;
struct usb_composite_dev *cdev = c->cdev;
struct usb_ep *in_ep;
@@ -1159,7 +1164,7 @@ static void printer_func_unbind(struct usb_configuration 
*c,
struct printer_dev  *dev;
struct usb_request  *req;
 
-   dev = container_of(f, struct printer_dev, function);
+   dev = func_to_printer(f);
 
device_destroy(usb_gadget_class, g_printer_devno);
 
@@ -1200,7 +1205,7 @@ static void printer_func_unbind(struct usb_configuration 
*c,
 static int printer_func_set_alt(struct usb_function *f,
unsigned intf, unsigned alt)
 {
-   struct printer_dev *dev = container_of(f, struct printer_dev, function);
+   struct printer_dev *dev = func_to_printer(f);
int ret = -ENOTSUPP;
 
if (!alt)
@@ -1211,7 +1216,7 @@ static int printer_func_set_alt(struct usb_function *f,
 
 static void printer_func_disable(struct usb_function *f)
 {
-   struct printer_dev *dev = container_of(f, struct printer_dev, function);
+   struct printer_dev *dev = func_to_printer(f);
unsigned long   flags;
 
DBG(dev, "%s\n", __func__);
-- 
1.9.1

--
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

[PATCHv3 21/29] usb: gadget: printer: name class specific requests

[Andrzej Pietrasiewicz]

Avoid using magic numbers.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 955847f..78f5154 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -47,6 +47,9 @@ USB_GADGET_COMPOSITE_OPTIONS();
 
 #define DRIVER_DESC"Printer Gadget"
 #define DRIVER_VERSION "2007 OCT 06"
+#define GET_DEVICE_ID  0
+#define GET_PORT_STATUS1
+#define SOFT_RESET 2
 
 static const char shortname [] = "printer";
 static const char driver_desc [] = DRIVER_DESC;
@@ -992,7 +995,7 @@ static int printer_func_setup(struct usb_function *f,
switch (ctrl->bRequestType&USB_TYPE_MASK) {
case USB_TYPE_CLASS:
switch (ctrl->bRequest) {
-   case 0: /* Get the IEEE-1284 PNP String */
+   case GET_DEVICE_ID: /* Get the IEEE-1284 PNP String */
/* Only one printer interface is supported. */
if ((wIndex>>8) != dev->interface)
break;
@@ -1003,7 +1006,7 @@ static int printer_func_setup(struct usb_function *f,
&dev->pnp_string[2]);
break;
 
-   case 1: /* Get Port Status */
+   case GET_PORT_STATUS: /* Get Port Status */
/* Only one printer interface is supported. */
if (wIndex != dev->interface)
break;
@@ -1012,7 +1015,7 @@ static int printer_func_setup(struct usb_function *f,
value = min(wLength, (u16) 1);
break;
 
-   case 2: /* Soft Reset */
+   case SOFT_RESET: /* Soft Reset */
/* Only one printer interface is supported. */
if (wIndex != dev->interface)
break;
-- 
1.9.1

--
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

[PATCHv3 23/29] usb: gadget: printer: allocate printer_dev instances dynamically

[Andrzej Pietrasiewicz]

With all the obstacles removed it is possible to allow more than one
instance of the printer function. Since the function requires allocating
character device region, a maximum number of allowed instances is defined.
Such an approach is used in f_acm and in f_hid.
With multiple instances it does not make sense to depend on a
lock_printer_io member of a dynamically allocated (and destroyed) struct
printer_dev to clean up after all instances of the printer function.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 62 -
 1 file changed, 40 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index c059af1..d1f85f8 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -51,11 +51,12 @@ USB_GADGET_COMPOSITE_OPTIONS();
 #define GET_PORT_STATUS1
 #define SOFT_RESET 2
 
+#define PRINTER_MINORS 4
+
 static const char shortname [] = "printer";
 static const char driver_desc [] = DRIVER_DESC;
 
-static dev_t g_printer_devno;
-
+static int major, minors;
 static struct class *usb_gadget_class;
 
 /*-*/
@@ -84,6 +85,7 @@ struct printer_dev {
u8  *current_rx_buf;
u8  printer_status;
u8  reset_printer;
+   int minor;
struct cdev printer_cdev;
u8  printer_cdev_open;
wait_queue_head_t   wait;
@@ -97,8 +99,6 @@ static inline struct printer_dev *func_to_printer(struct 
usb_function *f)
return container_of(f, struct printer_dev, function);
 }
 
-static struct printer_dev usb_printer_gadget;
-
 /*-*/
 
 /* DO NOT REUSE THESE IDs with a protocol-incompatible driver!!  Ever!!
@@ -1096,6 +1096,7 @@ static int __init printer_func_bind(struct 
usb_configuration *c,
struct usb_ep *in_ep;
struct usb_ep *out_ep = NULL;
struct usb_request *req;
+   dev_t devt;
int id;
int ret;
u32 i;
@@ -1153,8 +1154,9 @@ autoconf_fail:
}
 
/* Setup the sysfs files for the printer gadget. */
-   pdev = device_create(usb_gadget_class, NULL, g_printer_devno,
- NULL, "g_printer");
+   devt = MKDEV(major, dev->minor);
+   pdev = device_create(usb_gadget_class, NULL, devt,
+ NULL, "g_printer%d", dev->minor);
if (IS_ERR(pdev)) {
ERROR(dev, "Failed to create device: g_printer\n");
ret = PTR_ERR(pdev);
@@ -1167,7 +1169,7 @@ autoconf_fail:
 */
cdev_init(&dev->printer_cdev, &printer_io_operations);
dev->printer_cdev.owner = THIS_MODULE;
-   ret = cdev_add(&dev->printer_cdev, g_printer_devno, 1);
+   ret = cdev_add(&dev->printer_cdev, devt, 1);
if (ret) {
ERROR(dev, "Failed to open char device\n");
goto fail_cdev_add;
@@ -1176,7 +1178,7 @@ autoconf_fail:
return 0;
 
 fail_cdev_add:
-   device_destroy(usb_gadget_class, g_printer_devno);
+   device_destroy(usb_gadget_class, devt);
 
 fail_rx_reqs:
while (!list_empty(&dev->rx_reqs)) {
@@ -1204,7 +1206,7 @@ static void printer_func_unbind(struct usb_configuration 
*c,
 
dev = func_to_printer(f);
 
-   device_destroy(usb_gadget_class, g_printer_devno);
+   device_destroy(usb_gadget_class, MKDEV(major, dev->minor));
 
/* Remove Character Device */
cdev_del(&dev->printer_cdev);
@@ -1238,6 +1240,7 @@ static void printer_func_unbind(struct usb_configuration 
*c,
printer_req_free(dev->out_ep, req);
}
usb_free_all_descriptors(f);
+   kfree(dev);
 }
 
 static int printer_func_set_alt(struct usb_function *f,
@@ -1271,14 +1274,21 @@ static struct usb_configuration printer_cfg_driver = {
 };
 
 static int f_printer_bind_config(struct usb_configuration *c, char *pnp_str,
-char *pnp_string, unsigned q_len)
+char *pnp_string, unsigned q_len, int minor)
 {
struct printer_dev  *dev;
int status = -ENOMEM;
size_t  len;
 
-   dev = &usb_printer_gadget;
+   if (minor >= minors)
+   return -ENOENT;
+
+   dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+   if (!dev)
+   return -ENOMEM;
+
dev->pnp_string = pnp_string;
+   dev->minor = minor;
 
dev->function.name = shortname;
dev->function.bind = printer_func_bind;
@@ -1315,8 +1325,10 @@ static int f_printer_bind_

[PATCHv3 26/29] usb: gadget: printer: convert to new interface of f_printer

[Andrzej Pietrasiewicz]

The goal is to remove the old function interface, so its (only) user
must be converted to the new interface.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/Kconfig   |  1 +
 drivers/usb/gadget/legacy/printer.c | 50 ++---
 2 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/gadget/legacy/Kconfig 
b/drivers/usb/gadget/legacy/Kconfig
index 113c87e..d5a7102 100644
--- a/drivers/usb/gadget/legacy/Kconfig
+++ b/drivers/usb/gadget/legacy/Kconfig
@@ -301,6 +301,7 @@ config USB_MIDI_GADGET
 config USB_G_PRINTER
tristate "Printer Gadget"
select USB_LIBCOMPOSITE
+   select USB_F_PRINTER
help
  The Printer Gadget channels data between the USB host and a
  userspace program driving the print engine. The user space
diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 770b504..a8050f8 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -29,12 +29,7 @@ USB_GADGET_COMPOSITE_OPTIONS();
 static const char shortname [] = "printer";
 static const char driver_desc [] = DRIVER_DESC;
 
-/*
- * This will be changed when f_printer is converted
- * to the new function interface.
- */
-#define USBF_PRINTER_INCLUDED
-#include "f_printer.c"
+#include "u_printer.h"
 
 /*-*/
 
@@ -65,6 +60,9 @@ module_param(qlen, uint, S_IRUGO|S_IWUSR);
 
 #define QLEN   qlen
 
+static struct usb_function_instance *fi_printer;
+static struct usb_function *f_printer;
+
 /*-*/
 
 /*
@@ -131,6 +129,7 @@ static struct usb_configuration printer_cfg_driver = {
 static int __init printer_do_config(struct usb_configuration *c)
 {
struct usb_gadget   *gadget = c->cdev->gadget;
+   int status = 0;
 
usb_ep_autoconfig_reset(gadget);
 
@@ -142,20 +141,41 @@ static int __init printer_do_config(struct 
usb_configuration *c)
printer_cfg_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
}
 
-   return f_printer_bind_config(c, iPNPstring, pnp_string, QLEN, 0);
+   f_printer = usb_get_function(fi_printer);
+   if (IS_ERR(f_printer))
+   return PTR_ERR(f_printer);
+
+   status = usb_add_function(c, f_printer);
+   if (status < 0)
+   usb_put_function(f_printer);
+
+   return status;
 }
 
 static int __init printer_bind(struct usb_composite_dev *cdev)
 {
-   int ret;
+   struct f_printer_opts *opts;
+   int ret, len;
 
-   ret = gprinter_setup(PRINTER_MINORS);
-   if (ret)
-   return ret;
+   fi_printer = usb_get_function_instance("printer");
+   if (IS_ERR(fi_printer))
+   return PTR_ERR(fi_printer);
+
+   if (iPNPstring)
+   strlcpy(&pnp_string[2], iPNPstring, PNP_STRING_LEN - 2);
+
+   len = strlen(pnp_string);
+   pnp_string[0] = (len >> 8) & 0xFF;
+   pnp_string[1] = len & 0xFF;
+
+   opts = container_of(fi_printer, struct f_printer_opts, func_inst);
+   opts->minor = 0;
+   memcpy(opts->pnp_string, pnp_string, PNP_STRING_LEN);
+   opts->q_len = QLEN;
 
ret = usb_string_ids_tab(cdev, strings);
if (ret < 0) {
-   gprinter_cleanup();
+   usb_put_function_instance(fi_printer);
return ret;
}
device_desc.iManufacturer = strings[USB_GADGET_MANUFACTURER_IDX].id;
@@ -164,7 +184,7 @@ static int __init printer_bind(struct usb_composite_dev 
*cdev)
 
ret = usb_add_config(cdev, &printer_cfg_driver, printer_do_config);
if (ret) {
-   gprinter_cleanup();
+   usb_put_function_instance(fi_printer);
return ret;
}
usb_composite_overwrite_options(cdev, &coverwrite);
@@ -173,7 +193,9 @@ static int __init printer_bind(struct usb_composite_dev 
*cdev)
 
 static int __exit printer_unbind(struct usb_composite_dev *cdev)
 {
-   gprinter_cleanup();
+   usb_put_function(f_printer);
+   usb_put_function_instance(fi_printer);
+
return 0;
 }
 
-- 
1.9.1

--
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

[PATCHv3 11/29] usb: gadget: printer: call usb_add_function() last

[Andrzej Pietrasiewicz]

Conversion to the new function interface requires splitting a
_bind_config() function into two parts: allocation of
container_of struct usb_function and invocation of usb_add_function().
This patch moves the latter to the end of the f_printer_bind_config()
in order to enable conversion to the new interface.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index c857044..5dbb93a 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1254,11 +1254,6 @@ static int f_printer_bind_config(struct 
usb_configuration *c, char *pnp_str,
INIT_LIST_HEAD(&dev->rx_reqs);
INIT_LIST_HEAD(&dev->rx_buffers);
 
-   dev->q_len = q_len;
-   status = usb_add_function(c, &dev->function);
-   if (status)
-   return status;
-
if (pnp_str)
strlcpy(&pnp_string[2], pnp_str, sizeof(pnp_string) - 2);
 
@@ -1280,7 +1275,11 @@ static int f_printer_bind_config(struct 
usb_configuration *c, char *pnp_str,
dev->current_rx_req = NULL;
dev->current_rx_bytes = 0;
dev->current_rx_buf = NULL;
+   dev->q_len = q_len;
 
+   status = usb_add_function(c, &dev->function);
+   if (status)
+   return status;
INFO(dev, "%s, version: " DRIVER_VERSION "\n", driver_desc);
return 0;
 }
-- 
1.9.1

--
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

[PATCHv3 28/29] usb: gadget: printer: use module_usb_composite_driver helper macro

[Andrzej Pietrasiewicz]

Substitute some boilerplate code with a dedicated macro.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 14 +-
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index a8050f8..d5b6ee7 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -208,19 +208,7 @@ static __refdata struct usb_composite_driver 
printer_driver = {
.unbind = printer_unbind,
 };
 
-static int __init
-init(void)
-{
-   return usb_composite_probe(&printer_driver);
-}
-module_init(init);
-
-static void __exit
-cleanup(void)
-{
-   usb_composite_unregister(&printer_driver);
-}
-module_exit(cleanup);
+module_usb_composite_driver(printer_driver);
 
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR("Craig Nadler");
-- 
1.9.1

--
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

[PATCHv3 22/29] usb: gadget: printer: add req_match for printer function

[Andrzej Pietrasiewicz]

Verify that a given usb_ctrlrequest is meant for printer function.
The following parts of the request are tested:

- bmRequestType:Data transfer direction
- bmRequestType:Type
- bmRequestType:Recipient
- bRequest
- wValue for bRequest 1 and 2
- wLength

Additionally, the request is considered meant for this function
iff the decoded interface number matches dev->interface.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 78f5154..c059af1 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -974,6 +974,41 @@ static void printer_soft_reset(struct printer_dev *dev)
 
 /*-*/
 
+static bool gprinter_req_match(struct usb_function *f,
+  const struct usb_ctrlrequest *ctrl)
+{
+   struct printer_dev  *dev = func_to_printer(f);
+   u16 w_index = le16_to_cpu(ctrl->wIndex);
+   u16 w_value = le16_to_cpu(ctrl->wValue);
+   u16 w_length = le16_to_cpu(ctrl->wLength);
+
+   if ((ctrl->bRequestType & USB_RECIP_MASK) != USB_RECIP_INTERFACE ||
+   (ctrl->bRequestType & USB_TYPE_MASK) != USB_TYPE_CLASS)
+   return false;
+
+   switch (ctrl->bRequest) {
+   case GET_DEVICE_ID:
+   w_index >>= 8;
+   if (w_length <= PNP_STRING_LEN &&
+   (USB_DIR_IN & ctrl->bRequestType))
+   break;
+   return false;
+   case GET_PORT_STATUS:
+   if (!w_value && w_length == 1 &&
+   (USB_DIR_IN & ctrl->bRequestType))
+   break;
+   return false;
+   case SOFT_RESET:
+   if (!w_value && !w_length &&
+  (USB_DIR_OUT & ctrl->bRequestType))
+   break;
+   /* fall through */
+   default:
+   return false;
+   }
+   return w_index == dev->interface;
+}
+
 /*
  * The setup() callback implements all the ep0 functionality that's not
  * handled lower down.
@@ -1251,6 +1286,7 @@ static int f_printer_bind_config(struct usb_configuration 
*c, char *pnp_str,
dev->function.unbind = printer_func_unbind;
dev->function.set_alt = printer_func_set_alt;
dev->function.disable = printer_func_disable;
+   dev->function.req_match = gprinter_req_match;
INIT_LIST_HEAD(&dev->tx_reqs);
INIT_LIST_HEAD(&dev->rx_reqs);
INIT_LIST_HEAD(&dev->rx_buffers);
-- 
1.9.1

--
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

[PATCHv3 05/29] usb: gadget: printer: revert usb_add_function() effect in error recovery

[Andrzej Pietrasiewicz]

Whenever the "goto fail" branch is taken, the effect of usb_add_function()
should be reverted.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 12247d3..eb02a6b 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1285,6 +1285,7 @@ static int __init printer_bind_config(struct 
usb_configuration *c)
 
 fail:
printer_cfg_unbind(c);
+   usb_remove_function(c, &dev->function);
return status;
 }
 
-- 
1.9.1

--
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

[PATCHv3 10/29] usb: gadget: printer: move function-related bind code to function's bind

[Andrzej Pietrasiewicz]

In order to factor out a reusable f_printer.c, the code related to the
function should be placed in functions related to the function.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 114 +---
 1 file changed, 66 insertions(+), 48 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 494cd8a..c857044 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -85,6 +85,7 @@ struct printer_dev {
struct cdev printer_cdev;
u8  printer_cdev_open;
wait_queue_head_t   wait;
+   unsignedq_len;
struct usb_function function;
 };
 
@@ -1045,18 +1046,25 @@ unknown:
 static int __init printer_func_bind(struct usb_configuration *c,
struct usb_function *f)
 {
+   struct usb_gadget *gadget = c->cdev->gadget;
struct printer_dev *dev = container_of(f, struct printer_dev, function);
+   struct device *pdev;
struct usb_composite_dev *cdev = c->cdev;
struct usb_ep *in_ep;
struct usb_ep *out_ep = NULL;
+   struct usb_request *req;
int id;
int ret;
+   u32 i;
 
id = usb_interface_id(c, f);
if (id < 0)
return id;
intf_desc.bInterfaceNumber = id;
 
+   /* finish hookup to lower layer ... */
+   dev->gadget = gadget;
+
/* all we really need is bulk IN/OUT */
in_ep = usb_ep_autoconfig(cdev->gadget, &fs_ep_in_desc);
if (!in_ep) {
@@ -1085,7 +1093,64 @@ autoconf_fail:
 
dev->in_ep = in_ep;
dev->out_ep = out_ep;
+
+   ret = -ENOMEM;
+   for (i = 0; i < dev->q_len; i++) {
+   req = printer_req_alloc(dev->in_ep, USB_BUFSIZE, GFP_KERNEL);
+   if (!req)
+   goto fail_tx_reqs;
+   list_add(&req->list, &dev->tx_reqs);
+   }
+
+   for (i = 0; i < dev->q_len; i++) {
+   req = printer_req_alloc(dev->out_ep, USB_BUFSIZE, GFP_KERNEL);
+   if (!req)
+   goto fail_rx_reqs;
+   list_add(&req->list, &dev->rx_reqs);
+   }
+
+   /* Setup the sysfs files for the printer gadget. */
+   pdev = device_create(usb_gadget_class, NULL, g_printer_devno,
+ NULL, "g_printer");
+   if (IS_ERR(pdev)) {
+   ERROR(dev, "Failed to create device: g_printer\n");
+   ret = PTR_ERR(pdev);
+   goto fail_rx_reqs;
+   }
+
+   /*
+* Register a character device as an interface to a user mode
+* program that handles the printer specific functionality.
+*/
+   cdev_init(&dev->printer_cdev, &printer_io_operations);
+   dev->printer_cdev.owner = THIS_MODULE;
+   ret = cdev_add(&dev->printer_cdev, g_printer_devno, 1);
+   if (ret) {
+   ERROR(dev, "Failed to open char device\n");
+   goto fail_cdev_add;
+   }
+
return 0;
+
+fail_cdev_add:
+   device_destroy(usb_gadget_class, g_printer_devno);
+
+fail_rx_reqs:
+   while (!list_empty(&dev->rx_reqs)) {
+   req = container_of(dev->rx_reqs.next, struct usb_request, list);
+   list_del(&req->list);
+   printer_req_free(dev->out_ep, req);
+   }
+
+fail_tx_reqs:
+   while (!list_empty(&dev->tx_reqs)) {
+   req = container_of(dev->tx_reqs.next, struct usb_request, list);
+   list_del(&req->list);
+   printer_req_free(dev->in_ep, req);
+   }
+
+   return ret;
+
 }
 
 static void printer_func_unbind(struct usb_configuration *c,
@@ -1173,13 +1238,9 @@ static struct usb_configuration printer_cfg_driver = {
 static int f_printer_bind_config(struct usb_configuration *c, char *pnp_str,
 unsigned q_len)
 {
-   struct usb_gadget   *gadget = c->cdev->gadget;
struct printer_dev  *dev;
-   struct device   *pdev;
int status = -ENOMEM;
size_t  len;
-   u32 i;
-   struct usb_request  *req;
 
dev = &usb_printer_gadget;
 
@@ -1193,31 +1254,11 @@ static int f_printer_bind_config(struct 
usb_configuration *c, char *pnp_str,
INIT_LIST_HEAD(&dev->rx_reqs);
INIT_LIST_HEAD(&dev->rx_buffers);
 
+   dev->q_len = q_len;
status = usb_add_function(c, &dev->function);
if (status)
return status;
 
-   /* Setup the sysfs files for the printer gadget. */
-   pdev = device_create(usb_gadget_class, NULL, g_printer_devno,
-   

[PATCHv3 29/29] usb: gadget: printer: add configfs support

[Andrzej Pietrasiewicz]

Add support for configfs interface so that f_printer can be used as a
component of usb gadgets composed with it.

Signed-off-by: Andrzej Pietrasiewicz 
---
 .../ABI/testing/configfs-usb-gadget-printer|   9 ++
 Documentation/usb/gadget-testing.txt   |  47 
 drivers/usb/gadget/Kconfig |  13 +++
 drivers/usb/gadget/function/f_printer.c| 130 -
 drivers/usb/gadget/function/u_printer.h|   7 ++
 5 files changed, 204 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-printer

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-printer 
b/Documentation/ABI/testing/configfs-usb-gadget-printer
new file mode 100644
index 000..6b0714e
--- /dev/null
+++ b/Documentation/ABI/testing/configfs-usb-gadget-printer
@@ -0,0 +1,9 @@
+What:  /config/usb-gadget/gadget/functions/printer.name
+Date:  Apr 2015
+KernelVersion: 4.1
+Description:
+   The attributes:
+
+   pnp_string  - Data to be passed to the host in pnp string
+   q_len   - Number of requests per endpoint
+
diff --git a/Documentation/usb/gadget-testing.txt 
b/Documentation/usb/gadget-testing.txt
index 076ac7b..f45b2bf 100644
--- a/Documentation/usb/gadget-testing.txt
+++ b/Documentation/usb/gadget-testing.txt
@@ -19,6 +19,7 @@ provided by gadgets.
 16. UAC1 function
 17. UAC2 function
 18. UVC function
+19. PRINTER function
 
 
 1. ACM function
@@ -726,3 +727,49 @@ with these patches:
 http://www.spinics.net/lists/linux-usb/msg99220.html
 
 host: luvcview -f yuv
+
+19. PRINTER function
+
+
+The function is provided by usb_f_printer.ko module.
+
+Function-specific configfs interface
+
+
+The function name to use when creating the function directory is "printer".
+The printer function provides these attributes in its function directory:
+
+   pnp_string  - Data to be passed to the host in pnp string
+   q_len   - Number of requests per endpoint
+
+Testing the PRINTER function
+
+
+The most basic testing:
+
+device: run the gadget
+# ls -l /devices/virtual/usb_printer_gadget/
+
+should show g_printer.
+
+If udev is active, then /dev/g_printer should appear automatically.
+
+host:
+
+If udev is active, then e.g. /dev/usb/lp0 should appear.
+
+host->device transmission:
+
+device:
+# cat /dev/g_printer
+host:
+# cat > /dev/usb/lp0
+
+device->host transmission:
+
+# cat > /dev/g_printer
+host:
+# cat /dev/usb/lp0
+
+More advanced testing can be done with the prn_example
+described in Documentation/usb/gadget-printer.txt.
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 9d507cf..3bb0e67 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -437,6 +437,19 @@ config USB_CONFIGFS_F_UVC
  device. It provides a userspace API to process UVC control requests
  and stream video data to the host.
 
+config USB_CONFIGFS_F_PRINTER
+   bool "Printer function"
+   select USB_F_PRINTER
+   help
+ The Printer function channels data between the USB host and a
+ userspace program driving the print engine. The user space
+ program reads and writes the device file /dev/g_printer to
+ receive or send printer data. It can use ioctl calls to
+ the device file to get or set printer status.
+
+ For more information, see Documentation/usb/gadget_printer.txt
+ which includes sample code for accessing the device file.
+
 source "drivers/usb/gadget/legacy/Kconfig"
 
 endchoice
diff --git a/drivers/usb/gadget/function/f_printer.c 
b/drivers/usb/gadget/function/f_printer.c
index 7afe17d..757fcf0 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -1140,6 +1140,117 @@ static void printer_func_disable(struct usb_function *f)
spin_unlock_irqrestore(&dev->lock, flags);
 }
 
+static inline struct f_printer_opts
+*to_f_printer_opts(struct config_item *item)
+{
+   return container_of(to_config_group(item), struct f_printer_opts,
+   func_inst.group);
+}
+
+CONFIGFS_ATTR_STRUCT(f_printer_opts);
+CONFIGFS_ATTR_OPS(f_printer_opts);
+
+static void printer_attr_release(struct config_item *item)
+{
+   struct f_printer_opts *opts = to_f_printer_opts(item);
+
+   usb_put_function_instance(&opts->func_inst);
+}
+
+static struct configfs_item_operations printer_item_ops = {
+   .release= printer_attr_release,
+   .show_attribute = f_printer_opts_attr_show,
+   .store_attribute = f_printer_opts_attr_store,
+};
+
+static ssize_t f_printer_opts_pnp_string_show(struct f_printer_opts *opts,
+ char *page)
+{
+   int result;
+
+   mutex_lock(&opts

[PATCHv3 24/29] usb: gadget: printer: factor out f_printer

[Andrzej Pietrasiewicz]

The legacy printer gadget now contains both a reusable printer function
and legacy gadget proper implementations interwoven, but logically
separate. This patch factors out a reusable f_printer.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/f_printer.c | 1279 +++
 drivers/usb/gadget/legacy/printer.c | 1255 +-
 2 files changed, 1285 insertions(+), 1249 deletions(-)
 create mode 100644 drivers/usb/gadget/function/f_printer.c

diff --git a/drivers/usb/gadget/function/f_printer.c 
b/drivers/usb/gadget/function/f_printer.c
new file mode 100644
index 000..0847972
--- /dev/null
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -0,0 +1,1279 @@
+/*
+ * f_printer.c - USB printer function driver
+ *
+ * Copied from drivers/usb/gadget/legacy/printer.c,
+ * which was:
+ *
+ * printer.c -- Printer gadget driver
+ *
+ * Copyright (C) 2003-2005 David Brownell
+ * Copyright (C) 2006 Craig W. Nadler
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#define PNP_STRING_LEN 1024
+#define PRINTER_MINORS 4
+#define GET_DEVICE_ID  0
+#define GET_PORT_STATUS1
+#define SOFT_RESET 2
+
+static int major, minors;
+static struct class *usb_gadget_class;
+
+/*-*/
+
+struct printer_dev {
+   spinlock_t  lock;   /* lock this structure */
+   /* lock buffer lists during read/write calls */
+   struct mutexlock_printer_io;
+   struct usb_gadget   *gadget;
+   s8  interface;
+   struct usb_ep   *in_ep, *out_ep;
+
+   struct list_headrx_reqs;/* List of free RX structs */
+   struct list_headrx_reqs_active; /* List of Active RX xfers */
+   struct list_headrx_buffers; /* List of completed xfers */
+   /* wait until there is data to be read. */
+   wait_queue_head_t   rx_wait;
+   struct list_headtx_reqs;/* List of free TX structs */
+   struct list_headtx_reqs_active; /* List of Active TX xfers */
+   /* Wait until there are write buffers available to use. */
+   wait_queue_head_t   tx_wait;
+   /* Wait until all write buffers have been sent. */
+   wait_queue_head_t   tx_flush_wait;
+   struct usb_request  *current_rx_req;
+   size_t  current_rx_bytes;
+   u8  *current_rx_buf;
+   u8  printer_status;
+   u8  reset_printer;
+   int minor;
+   struct cdev printer_cdev;
+   u8  printer_cdev_open;
+   wait_queue_head_t   wait;
+   unsignedq_len;
+   char*pnp_string;/* We don't own memory! */
+   struct usb_function function;
+};
+
+static inline struct printer_dev *func_to_printer(struct usb_function *f)
+{
+   return container_of(f, struct printer_dev, function);
+}
+
+/*-*/
+
+/*
+ * DESCRIPTORS ... most are static, but strings and (full) configuration
+ * descriptors are built on demand.
+ */
+
+/* holds our biggest descriptor */
+#define USB_DESC_BUFSIZE   256
+#define USB_BUFSIZE8192
+
+static struct usb_interface_descriptor intf_desc = {
+   .bLength =  sizeof(intf_desc),
+   .bDescriptorType =  USB_DT_INTERFACE,
+   .bNumEndpoints =2,
+   .bInterfaceClass =  USB_CLASS_PRINTER,
+   .bInterfaceSubClass =   1,  /* Printer Sub-Class */
+   .bInterfaceProtocol =   2,  /* Bi-Directional */
+   .iInterface =   0
+};
+
+static struct usb_endpoint_descriptor fs_ep_in_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+   .bEndpointAddress = USB_DIR_IN,
+   .bmAttributes = USB_ENDPOINT_XFER_BULK
+};
+
+static struct usb_endpoint_descriptor fs_ep_out_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+   .bEndpointAddress = USB_DIR_OUT,
+   .bmAttributes = USB_ENDPOINT_XFER_BULK
+};
+
+static struct usb_descriptor_header *fs_printer_fun

[PATCHv3 02/29] usb: gadget: printer: enqueue printer's response for setup request

[Andrzej Pietrasiewicz]

Function-specific setup requests should be handled in such a way, that
apart from filling in the data buffer, the requests are also actually
enqueued: if function-specific setup is called from composte_setup(),
the "usb_ep_queue()" block of code in composite_setup() is skipped.

The printer function lacks this part and it results in e.g. get device id
requests failing: the host expects some response, the device prepares it
but does not equeue it for sending to the host, so the host finally asserts
timeout.

This patch adds enqueueing the prepared responses.

Cc:  # v3.4+
Fixes: 2e87edf49227: "usb: gadget: make g_printer use composite"
Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 9054598..6385c19 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1031,6 +1031,15 @@ unknown:
break;
}
/* host either stalls (value < 0) or reports success */
+   if (value >= 0) {
+   req->length = value;
+   req->zero = value < wLength;
+   value = usb_ep_queue(cdev->gadget->ep0, req, GFP_ATOMIC);
+   if (value < 0) {
+   ERROR(dev, "%s:%d Error!\n", __func__, __LINE__);
+   req->status = 0;
+   }
+   }
return value;
 }
 
-- 
1.9.1

--
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

[PATCHv3 09/29] usb: gadget: printer: standardize printer_do_config

[Andrzej Pietrasiewicz]

Follow the convention of distributing source code between
_do_config() and _bind_config().

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 39 +++--
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index c865833..494cd8a 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1170,7 +1170,8 @@ static struct usb_configuration printer_cfg_driver = {
.bmAttributes   = USB_CONFIG_ATT_ONE | USB_CONFIG_ATT_SELFPOWER,
 };
 
-static int __init printer_do_config(struct usb_configuration *c)
+static int f_printer_bind_config(struct usb_configuration *c, char *pnp_str,
+unsigned q_len)
 {
struct usb_gadget   *gadget = c->cdev->gadget;
struct printer_dev  *dev;
@@ -1180,8 +1181,6 @@ static int __init printer_do_config(struct 
usb_configuration *c)
u32 i;
struct usb_request  *req;
 
-   usb_ep_autoconfig_reset(gadget);
-
dev = &usb_printer_gadget;
 
dev->function.name = shortname;
@@ -1219,21 +1218,13 @@ static int __init printer_do_config(struct 
usb_configuration *c)
goto fail;
}
 
-   if (iPNPstring)
-   strlcpy(&pnp_string[2], iPNPstring, (sizeof pnp_string)-2);
+   if (pnp_str)
+   strlcpy(&pnp_string[2], pnp_str, sizeof(pnp_string) - 2);
 
len = strlen(pnp_string);
pnp_string[0] = (len >> 8) & 0xFF;
pnp_string[1] = len & 0xFF;
 
-   usb_gadget_set_selfpowered(gadget);
-
-   if (gadget_is_otg(gadget)) {
-   otg_descriptor.bmAttributes |= USB_OTG_HNP;
-   printer_cfg_driver.descriptors = otg_desc;
-   printer_cfg_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
-   }
-
spin_lock_init(&dev->lock);
mutex_init(&dev->lock_printer_io);
INIT_LIST_HEAD(&dev->tx_reqs_active);
@@ -1250,14 +1241,14 @@ static int __init printer_do_config(struct 
usb_configuration *c)
dev->current_rx_buf = NULL;
 
status = -ENOMEM;
-   for (i = 0; i < QLEN; i++) {
+   for (i = 0; i < q_len; i++) {
req = printer_req_alloc(dev->in_ep, USB_BUFSIZE, GFP_KERNEL);
if (!req)
goto fail;
list_add(&req->list, &dev->tx_reqs);
}
 
-   for (i = 0; i < QLEN; i++) {
+   for (i = 0; i < q_len; i++) {
req = printer_req_alloc(dev->out_ep, USB_BUFSIZE, GFP_KERNEL);
if (!req)
goto fail;
@@ -1276,6 +1267,24 @@ fail:
return status;
 }
 
+static int __init printer_do_config(struct usb_configuration *c)
+{
+   struct usb_gadget   *gadget = c->cdev->gadget;
+
+   usb_ep_autoconfig_reset(gadget);
+
+   usb_gadget_set_selfpowered(gadget);
+
+   if (gadget_is_otg(gadget)) {
+   otg_descriptor.bmAttributes |= USB_OTG_HNP;
+   printer_cfg_driver.descriptors = otg_desc;
+   printer_cfg_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
+   }
+
+   return f_printer_bind_config(c, iPNPstring, QLEN);
+
+}
+
 static int __init printer_bind(struct usb_composite_dev *cdev)
 {
int ret;
-- 
1.9.1

--
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

[PATCHv3 03/29] usb: gadget: printer: remove unused and empty printer_unbind

[Andrzej Pietrasiewicz]

The unbind() method is optional is usb_composite_driver.
In this particular driver the method does nothing so it can be removed.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 6385c19..21ea317 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1288,11 +1288,6 @@ fail:
return status;
 }
 
-static int printer_unbind(struct usb_composite_dev *cdev)
-{
-   return 0;
-}
-
 static int __init printer_bind(struct usb_composite_dev *cdev)
 {
int ret;
@@ -1317,7 +1312,6 @@ static __refdata struct usb_composite_driver 
printer_driver = {
.strings= dev_strings,
.max_speed  = USB_SPEED_SUPER,
.bind   = printer_bind,
-   .unbind = printer_unbind,
 };
 
 static int __init
-- 
1.9.1

--
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

[PATCHv3 00/29] Equivalent of g_printer with configfs

[Andrzej Pietrasiewicz]

This series aims at integrating configfs into printer, the way
it has been done for acm, ncm, ecm, eem, ecm subset, rndis, obex, phonet,
mass_storage, FunctionFS, loopback, sourcesink, uac1, uac2, uvc, midi
and hid.

The printer gadget before the changes has some bugs, and although it
uses the composite framework, it is not ready for splitting into
a reusable f_printer.c and a legacy printer.c. Most of the patches
in the series are preparation for factoring out f_printer.c. Once
f_printer.c is extracted from printer.c, it can be converted to
use the new function interface, and then the usual procedure of
adding configfs support follows.

Rebased onto Felipe's testing/next.

v2..v3:

- merged v1 with v2 version
- fixed the bug with handling of req_match method according to Bin Liu's
suggestion
- removed Felipe's Signed-off-by because the whole v1 + v2 series
was dropped by him

BACKWARD COMPATIBILITY
==

Please note that the old g_printer.ko is still available and works.
There is just one minor difference, the dev node at the gadget's side
is called /dev/g_printer instead of /dev/g_printer. This makes
sense, because there are more than one instances of this function allowed
after the series is applied. If /dev/g_printer is needed, it can be
easily added as a symlink with udev rules.

USING THE NEW "GADGET"
==

Please refer to this post:

http://www.spinics.net/lists/linux-usb/msg76388.html

for general information from Sebastian on how to use configfs-based
gadgets (*).

With configfs the procedure is as follows, compared to the information
mentioned above (*):

instead of mkdir functions/acm.ttyS1 do

mkdir functions/printer.

e.g. mkdir functions/printer.usb0.

In the printer.usb0 directory there will be the following attributes:


pnp_string  - Data to be passed to the host in pnp string
q_len   - Number of requests per endpoint.

Below is a script which creates a printer gadget on a board with dwc2:

$ modprobe libcomposite
$ mount none cfg -t configfs
$ mkdir cfg/usb_gadget/g1
$ cd cfg/usb_gadget/g1
$ mkdir configs/c.1
$ mkdir functions/printer.usb0
$ mkdir strings/0x409
$ mkdir configs/c.1/strings/0x409
$ echo 0x0525 > idVendor
$ echo 0xa4a8 > idProduct
$ echo 1 > strings/0x409/serialnumber
$ echo Samsung > strings/0x409/manufacturer
$ echo Printer Gadget > strings/0x409/product
$ echo 10 > functions/printer.usb0/q_len
$ echo "MFG:linux;MDL:g_printer;CLS:PRINTER;SN:1;" > 
functions/printer.usb0/pnp_string
$ echo "Conf 1" > configs/c.1/strings/0x409/configuration
$ echo 120 > configs/c.1/MaxPower
$ ln -s functions/printer.usb0 configs/c.1
$ echo 1248.hsotg > UDC

TESTING THE FUNCTION


The most basic testing:

device: run the gadget,

ls -l /devices/virtual/usb_printer_gadget/

should show g_printer.

If udev is active, then /dev/g_printer should appear automatically.

host:

If udev is active, then e.g. /dev/usb/lp0 should appear.

host->device transmission:

device:
# cat /dev/g_printer
host:
# cat > /dev/usb/lp0

device->host transmission:

# cat > /dev/g_printer
host:
# cat /dev/usb/lp0

More advanced testing can be done with the prn_example
described in Documentation/usb/gadget-printer.txt.



Andrzej Pietrasiewicz (29):
  usb: gadget: composite: don't try standard handling for non-standard
requests
  usb: gadget: printer: enqueue printer's response for setup request
  usb: gadget: printer: remove unused and empty printer_unbind
  usb: gadget: printer: eliminate random pointer dereference
  usb: gadget: printer: revert usb_add_function() effect in error
recovery
  usb: gadget: printer: add missing error handling
  usb: gadget: printer: eliminate pdev member of struct printer_dev
  usb: gadget: printer: follow the naming convention for usb_add_config
callback
  usb: gadget: printer: standardize printer_do_config
  usb: gadget: printer: move function-related bind code to function's
bind
  usb: gadget: printer: call usb_add_function() last
  usb: gadget: printer: move function-related unbind code to function's
unbind
  usb: gadget: printer: define pnp string buffer length
  usb: gadget: printer: don't access file global pnp_string in
function's code
  usb: gadget: printer: add setup and cleanup functions
  usb: gadget: printer: call gprinter_setup() from gadget's bind
  usb: gadget: printer: eliminate file global printer_mutex
  usb: gadget: printer: don't access file global usb_printer_gadget in
function's code
  usb: gadget: printer: add container_of helper for printer_dev
  usb: gadget: composite: add req_match method to usb_function
  usb: gadget: printer: name class specific requests
  usb: gadget: printer: add req_match for printer function
  usb: gadget: printer: allocate printer_dev instances dynamically
  usb: gadget: printer: factor out

Re: [PATCH 20/29] usb: gadget: composite: add req_match method to usb_function

[Andrzej Pietrasiewicz]

W dniu 02.03.2015 o 23:03, Bin Liu pisze:

Hi,

On Mon, Feb 23, 2015 at 9:02 AM, Andrzej Pietrasiewicz
 wrote:

Non-standard requests can encode the actual interface number in a
non-standard way. For example composite_setup() assumes
that it is w_index && 0xFF, but the printer function encodes the interface
number in a context-dependet way (either w_index or w_index >> 8).
This can lead to such requests being directed to wrong functions.

This patch adds req_match() method to usb_function. Its purpose is to
verify that a given request can be handled by a given function.
If any function within a configuration provides the method and it returns
true, then it is assumed that the right function is found.

If a function uses req_match(), it should try as hard as possible to
determine if the request is meant for it.

If no functions in a configuration provide req_match or none of them
returns true, then fall back to the usual approach.

Signed-off-by: Andrzej Pietrasiewicz 
---
  drivers/usb/gadget/composite.c | 7 ++-
  include/linux/usb/composite.h  | 3 +++
  2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 9fb9231..07cee80 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1758,6 +1758,11 @@ unknown:
  * take such requests too, if that's ever needed:  to work
  * in config 0, etc.
  */
+   list_for_each_entry(f, &cdev->config->functions, list)
+   if (f->req_match && f->req_match(f, ctrl))
+   break;


In this loop, if f->req_match is NULL, or f->req_match() returns
false, f becomes non-NULL at the end of the loop, which causes kernel
panic later.


+   if (&f->list != &cdev->config->functions)
+   goto try_fun_setup;


The following change fixes it.

+   list_for_each_entry(f, &cdev->config->functions, list)
+   if (f->req_match && f->req_match(f, ctrl))
+   goto try_fun_setup;
+
+   f = NULL;
+


Right. Thanks!

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

Re: [PATCH 20/29] usb: gadget: composite: add req_match method to usb_function

[Andrzej Pietrasiewicz]

W dniu 27.02.2015 o 21:55, Felipe Balbi pisze:

On Mon, Feb 23, 2015 at 04:02:09PM +0100, Andrzej Pietrasiewicz wrote:

Non-standard requests can encode the actual interface number in a
non-standard way. For example composite_setup() assumes
that it is w_index && 0xFF, but the printer function encodes the interface
number in a context-dependet way (either w_index or w_index >> 8).
This can lead to such requests being directed to wrong functions.

This patch adds req_match() method to usb_function. Its purpose is to
verify that a given request can be handled by a given function.
If any function within a configuration provides the method and it returns
true, then it is assumed that the right function is found.

If a function uses req_match(), it should try as hard as possible to
determine if the request is meant for it.

If no functions in a configuration provide req_match or none of them
returns true, then fall back to the usual approach.

Signed-off-by: Andrzej Pietrasiewicz 


this regresses testusb at least on am335x:

test 14: control writes


When I tried it on odroid u3 (with dwc2) at the host side it says:

/dev/bus/usb/002/009 test 14 --> 22 (Invalid argument)

(run as root at host)

but there is no problem at the device side.

On the same board with dummy hcd the result for test 14 is:

/dev/bus/usb/002/002 test 14 --> 25 (Inappropriate ioctl for device)

(run as root at "host")

but no problem with the device.

I'm running tools/usb/testusb with -a option. Do I need some
more preparation to actually run the tests? Perhaps I don't
experience the problem because there is no attempt to
actually trigger composite_setup() call?

On the other hand I'm thinking what could be wrong with the patch
adding req_match? With the patch applied usb_function structure
has one more member, which is (1) checked for not being NULL and,
if non-NULL it is (2) dereferenced in composite_setup(). The problem
you experience might be that (1) succeeds (req_match != NULL) and
then (2) explodes, because the pointer is actually either garbage or
some other data mis-interpreted. The struct usb_function in question
is allocated with kzalloc() in both f_loopback.c:loopback_alloc()
and f_sourcesink.c:source_sink_alloc_func() so there is no way
the pointer is not initialized to NULL, so (1) should fail
in the first place. But if (1) does not fail the member in question
is non-NULL, but how could it become non-NULL if properly initialized?
The only possibilities are: explicit assignment, unintentional memory
overwrite or mismatch between headers with which your modules/and or your
kernel have been built. If you did not add explicit assignment, and
indeed you didn't, explicit assignment is out of the question. Unintentional
memory overwrite might happen with e.g. memcpy() or variants of strcpy(),
but both f_sourcesink.c and f_loopback.c do not contain any  "cpy" string.
However silly this might sound, would you be able to double check
that you used consistent headers+kernel binary+module binaries?

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

Re: [PATCH 20/29] usb: gadget: composite: add req_match method to usb_function

[Andrzej Pietrasiewicz]

W dniu 27.02.2015 o 21:58, Felipe Balbi pisze:

Hi,

On Fri, Feb 27, 2015 at 02:55:25PM -0600, Felipe Balbi wrote:

On Mon, Feb 23, 2015 at 04:02:09PM +0100, Andrzej Pietrasiewicz wrote:

Non-standard requests can encode the actual interface number in a
non-standard way. For example composite_setup() assumes
that it is w_index && 0xFF, but the printer function encodes the interface
number in a context-dependet way (either w_index or w_index >> 8).
This can lead to such requests being directed to wrong functions.

This patch adds req_match() method to usb_function. Its purpose is to
verify that a given request can be handled by a given function.
If any function within a configuration provides the method and it returns
true, then it is assumed that the right function is found.

If a function uses req_match(), it should try as hard as possible to
determine if the request is meant for it.

If no functions in a configuration provide req_match or none of them
returns true, then fall back to the usual approach.

Signed-off-by: Andrzej Pietrasiewicz 


this regresses testusb at least on am335x:



I don't have this particular hardware but will try looking into the issue.
As soon as I have some results I will let you know.

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

[PATCHv2 3/6] usb: gadget: printer: convert to new interface of f_printer

[Andrzej Pietrasiewicz]

The goal is to remove the old function interface, so its (only) user
must be converted to the new interface.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/Kconfig   |  1 +
 drivers/usb/gadget/legacy/printer.c | 50 ++---
 2 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/gadget/legacy/Kconfig 
b/drivers/usb/gadget/legacy/Kconfig
index 113c87e..d5a7102 100644
--- a/drivers/usb/gadget/legacy/Kconfig
+++ b/drivers/usb/gadget/legacy/Kconfig
@@ -301,6 +301,7 @@ config USB_MIDI_GADGET
 config USB_G_PRINTER
tristate "Printer Gadget"
select USB_LIBCOMPOSITE
+   select USB_F_PRINTER
help
  The Printer Gadget channels data between the USB host and a
  userspace program driving the print engine. The user space
diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 770b504..a8050f8 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -29,12 +29,7 @@ USB_GADGET_COMPOSITE_OPTIONS();
 static const char shortname [] = "printer";
 static const char driver_desc [] = DRIVER_DESC;
 
-/*
- * This will be changed when f_printer is converted
- * to the new function interface.
- */
-#define USBF_PRINTER_INCLUDED
-#include "f_printer.c"
+#include "u_printer.h"
 
 /*-*/
 
@@ -65,6 +60,9 @@ module_param(qlen, uint, S_IRUGO|S_IWUSR);
 
 #define QLEN   qlen
 
+static struct usb_function_instance *fi_printer;
+static struct usb_function *f_printer;
+
 /*-*/
 
 /*
@@ -131,6 +129,7 @@ static struct usb_configuration printer_cfg_driver = {
 static int __init printer_do_config(struct usb_configuration *c)
 {
struct usb_gadget   *gadget = c->cdev->gadget;
+   int status = 0;
 
usb_ep_autoconfig_reset(gadget);
 
@@ -142,20 +141,41 @@ static int __init printer_do_config(struct 
usb_configuration *c)
printer_cfg_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
}
 
-   return f_printer_bind_config(c, iPNPstring, pnp_string, QLEN, 0);
+   f_printer = usb_get_function(fi_printer);
+   if (IS_ERR(f_printer))
+   return PTR_ERR(f_printer);
+
+   status = usb_add_function(c, f_printer);
+   if (status < 0)
+   usb_put_function(f_printer);
+
+   return status;
 }
 
 static int __init printer_bind(struct usb_composite_dev *cdev)
 {
-   int ret;
+   struct f_printer_opts *opts;
+   int ret, len;
 
-   ret = gprinter_setup(PRINTER_MINORS);
-   if (ret)
-   return ret;
+   fi_printer = usb_get_function_instance("printer");
+   if (IS_ERR(fi_printer))
+   return PTR_ERR(fi_printer);
+
+   if (iPNPstring)
+   strlcpy(&pnp_string[2], iPNPstring, PNP_STRING_LEN - 2);
+
+   len = strlen(pnp_string);
+   pnp_string[0] = (len >> 8) & 0xFF;
+   pnp_string[1] = len & 0xFF;
+
+   opts = container_of(fi_printer, struct f_printer_opts, func_inst);
+   opts->minor = 0;
+   memcpy(opts->pnp_string, pnp_string, PNP_STRING_LEN);
+   opts->q_len = QLEN;
 
ret = usb_string_ids_tab(cdev, strings);
if (ret < 0) {
-   gprinter_cleanup();
+   usb_put_function_instance(fi_printer);
return ret;
}
device_desc.iManufacturer = strings[USB_GADGET_MANUFACTURER_IDX].id;
@@ -164,7 +184,7 @@ static int __init printer_bind(struct usb_composite_dev 
*cdev)
 
ret = usb_add_config(cdev, &printer_cfg_driver, printer_do_config);
if (ret) {
-   gprinter_cleanup();
+   usb_put_function_instance(fi_printer);
return ret;
}
usb_composite_overwrite_options(cdev, &coverwrite);
@@ -173,7 +193,9 @@ static int __init printer_bind(struct usb_composite_dev 
*cdev)
 
 static int __exit printer_unbind(struct usb_composite_dev *cdev)
 {
-   gprinter_cleanup();
+   usb_put_function(f_printer);
+   usb_put_function_instance(fi_printer);
+
return 0;
 }
 
-- 
1.9.1

--
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

[PATCHv2 5/6] usb: gadget: printer: use module_usb_composite_driver helper macro

[Andrzej Pietrasiewicz]

Substitute some boilerplate code with a dedicated macro.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 14 +-
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index a8050f8..d5b6ee7 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -208,19 +208,7 @@ static __refdata struct usb_composite_driver 
printer_driver = {
.unbind = printer_unbind,
 };
 
-static int __init
-init(void)
-{
-   return usb_composite_probe(&printer_driver);
-}
-module_init(init);
-
-static void __exit
-cleanup(void)
-{
-   usb_composite_unregister(&printer_driver);
-}
-module_exit(cleanup);
+module_usb_composite_driver(printer_driver);
 
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR("Craig Nadler");
-- 
1.9.1

--
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

[PATCHv2 0/6] Configfs support for printer gadget

[Andrzej Pietrasiewicz]

This is a follow-up to this thread:

http://www.spinics.net/lists/linux-usb/msg121791.html

after patches 01..24 have been applied by Felipe.

This series contains the remainig patches rebased onto Felipe's
testing/next.

Andrzej Pietrasiewicz (6):
  usb: gadget: f_printer: eliminate legacy comment
  usb: gadget: f_printer: convert to new function interface with
backward compatibility
  usb: gadget: printer: convert to new interface of f_printer
  usb: gadget: f_printer: remove compatibility layer
  usb: gadget: printer: use module_usb_composite_driver helper macro
  usb: gadget: printer: add configfs support

 .../ABI/testing/configfs-usb-gadget-printer|   9 +
 Documentation/usb/gadget-testing.txt   |  47 
 drivers/usb/gadget/Kconfig |  16 ++
 drivers/usb/gadget/function/Makefile   |   2 +
 drivers/usb/gadget/function/f_printer.c| 307 +
 drivers/usb/gadget/function/u_printer.h|  37 +++
 drivers/usb/gadget/legacy/Kconfig  |   1 +
 drivers/usb/gadget/legacy/printer.c|  63 +++--
 8 files changed, 400 insertions(+), 82 deletions(-)
 create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-printer
 create mode 100644 drivers/usb/gadget/function/u_printer.h

-- 
1.9.1

--
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

[PATCHv2 6/6] usb: gadget: printer: add configfs support

[Andrzej Pietrasiewicz]

Add support for configfs interface so that f_printer can be used as a
component of usb gadgets composed with it.

Signed-off-by: Andrzej Pietrasiewicz 
---
 .../ABI/testing/configfs-usb-gadget-printer|   9 ++
 Documentation/usb/gadget-testing.txt   |  47 
 drivers/usb/gadget/Kconfig |  13 +++
 drivers/usb/gadget/function/f_printer.c| 130 -
 drivers/usb/gadget/function/u_printer.h|   7 ++
 5 files changed, 204 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-printer

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-printer 
b/Documentation/ABI/testing/configfs-usb-gadget-printer
new file mode 100644
index 000..6b0714e
--- /dev/null
+++ b/Documentation/ABI/testing/configfs-usb-gadget-printer
@@ -0,0 +1,9 @@
+What:  /config/usb-gadget/gadget/functions/printer.name
+Date:  Apr 2015
+KernelVersion: 4.1
+Description:
+   The attributes:
+
+   pnp_string  - Data to be passed to the host in pnp string
+   q_len   - Number of requests per endpoint
+
diff --git a/Documentation/usb/gadget-testing.txt 
b/Documentation/usb/gadget-testing.txt
index 076ac7b..f45b2bf 100644
--- a/Documentation/usb/gadget-testing.txt
+++ b/Documentation/usb/gadget-testing.txt
@@ -19,6 +19,7 @@ provided by gadgets.
 16. UAC1 function
 17. UAC2 function
 18. UVC function
+19. PRINTER function
 
 
 1. ACM function
@@ -726,3 +727,49 @@ with these patches:
 http://www.spinics.net/lists/linux-usb/msg99220.html
 
 host: luvcview -f yuv
+
+19. PRINTER function
+
+
+The function is provided by usb_f_printer.ko module.
+
+Function-specific configfs interface
+
+
+The function name to use when creating the function directory is "printer".
+The printer function provides these attributes in its function directory:
+
+   pnp_string  - Data to be passed to the host in pnp string
+   q_len   - Number of requests per endpoint
+
+Testing the PRINTER function
+
+
+The most basic testing:
+
+device: run the gadget
+# ls -l /devices/virtual/usb_printer_gadget/
+
+should show g_printer.
+
+If udev is active, then /dev/g_printer should appear automatically.
+
+host:
+
+If udev is active, then e.g. /dev/usb/lp0 should appear.
+
+host->device transmission:
+
+device:
+# cat /dev/g_printer
+host:
+# cat > /dev/usb/lp0
+
+device->host transmission:
+
+# cat > /dev/g_printer
+host:
+# cat /dev/usb/lp0
+
+More advanced testing can be done with the prn_example
+described in Documentation/usb/gadget-printer.txt.
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 9d507cf..3bb0e67 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -437,6 +437,19 @@ config USB_CONFIGFS_F_UVC
  device. It provides a userspace API to process UVC control requests
  and stream video data to the host.
 
+config USB_CONFIGFS_F_PRINTER
+   bool "Printer function"
+   select USB_F_PRINTER
+   help
+ The Printer function channels data between the USB host and a
+ userspace program driving the print engine. The user space
+ program reads and writes the device file /dev/g_printer to
+ receive or send printer data. It can use ioctl calls to
+ the device file to get or set printer status.
+
+ For more information, see Documentation/usb/gadget_printer.txt
+ which includes sample code for accessing the device file.
+
 source "drivers/usb/gadget/legacy/Kconfig"
 
 endchoice
diff --git a/drivers/usb/gadget/function/f_printer.c 
b/drivers/usb/gadget/function/f_printer.c
index 7afe17d..757fcf0 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -1140,6 +1140,117 @@ static void printer_func_disable(struct usb_function *f)
spin_unlock_irqrestore(&dev->lock, flags);
 }
 
+static inline struct f_printer_opts
+*to_f_printer_opts(struct config_item *item)
+{
+   return container_of(to_config_group(item), struct f_printer_opts,
+   func_inst.group);
+}
+
+CONFIGFS_ATTR_STRUCT(f_printer_opts);
+CONFIGFS_ATTR_OPS(f_printer_opts);
+
+static void printer_attr_release(struct config_item *item)
+{
+   struct f_printer_opts *opts = to_f_printer_opts(item);
+
+   usb_put_function_instance(&opts->func_inst);
+}
+
+static struct configfs_item_operations printer_item_ops = {
+   .release= printer_attr_release,
+   .show_attribute = f_printer_opts_attr_show,
+   .store_attribute = f_printer_opts_attr_store,
+};
+
+static ssize_t f_printer_opts_pnp_string_show(struct f_printer_opts *opts,
+ char *page)
+{
+   int result;
+
+   mutex_lock(&opts

[PATCHv2 2/6] usb: gadget: f_printer: convert to new function interface with backward compatibility

[Andrzej Pietrasiewicz]

In order to add configfs support, a usb function must be converted to use
the new interface. This patch converts the function to the new interface
and provides backward compatiblity layer, which can be removed after
all its users are converted to use the new interface.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/Kconfig  |   3 +
 drivers/usb/gadget/function/Makefile|   2 +
 drivers/usb/gadget/function/f_printer.c | 185 +++-
 drivers/usb/gadget/function/u_printer.h |  30 ++
 drivers/usb/gadget/legacy/printer.c |   1 +
 5 files changed, 220 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/gadget/function/u_printer.h

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index b454d05..9d507cf 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -196,6 +196,9 @@ config USB_F_MIDI
 config USB_F_HID
tristate
 
+config USB_F_PRINTER
+   tristate
+
 choice
tristate "USB Gadget Drivers"
default USB_ETH
diff --git a/drivers/usb/gadget/function/Makefile 
b/drivers/usb/gadget/function/Makefile
index f71b1aa..bd7def5 100644
--- a/drivers/usb/gadget/function/Makefile
+++ b/drivers/usb/gadget/function/Makefile
@@ -42,3 +42,5 @@ usb_f_midi-y  := f_midi.o
 obj-$(CONFIG_USB_F_MIDI)   += usb_f_midi.o
 usb_f_hid-y:= f_hid.o
 obj-$(CONFIG_USB_F_HID)+= usb_f_hid.o
+usb_f_printer-y:= f_printer.o
+obj-$(CONFIG_USB_F_PRINTER)+= usb_f_printer.o
diff --git a/drivers/usb/gadget/function/f_printer.c 
b/drivers/usb/gadget/function/f_printer.c
index 0847972..93f4d4e 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -46,6 +47,8 @@
 #include 
 #include 
 
+#include "u_printer.h"
+
 #define PNP_STRING_LEN 1024
 #define PRINTER_MINORS 4
 #define GET_DEVICE_ID  0
@@ -54,6 +57,10 @@
 
 static int major, minors;
 static struct class *usb_gadget_class;
+#ifndef USBF_PRINTER_INCLUDED
+static DEFINE_IDA(printer_ida);
+static DEFINE_MUTEX(printer_ida_lock); /* protects access do printer_ida */
+#endif
 
 /*-*/
 
@@ -999,7 +1006,7 @@ unknown:
return value;
 }
 
-static int __init printer_func_bind(struct usb_configuration *c,
+static int printer_func_bind(struct usb_configuration *c,
struct usb_function *f)
 {
struct usb_gadget *gadget = c->cdev->gadget;
@@ -,6 +1118,7 @@ fail_tx_reqs:
 
 }
 
+#ifdef USBF_PRINTER_INCLUDED
 static void printer_func_unbind(struct usb_configuration *c,
struct usb_function *f)
 {
@@ -1155,6 +1163,7 @@ static void printer_func_unbind(struct usb_configuration 
*c,
usb_free_all_descriptors(f);
kfree(dev);
 }
+#endif
 
 static int printer_func_set_alt(struct usb_function *f,
unsigned intf, unsigned alt)
@@ -1180,6 +1189,7 @@ static void printer_func_disable(struct usb_function *f)
spin_unlock_irqrestore(&dev->lock, flags);
 }
 
+#ifdef USBF_PRINTER_INCLUDED
 static int f_printer_bind_config(struct usb_configuration *c, char *pnp_str,
 char *pnp_string, unsigned q_len, int minor)
 {
@@ -1240,6 +1250,179 @@ static int f_printer_bind_config(struct 
usb_configuration *c, char *pnp_str,
INFO(dev, "%s, version: " DRIVER_VERSION "\n", driver_desc);
return 0;
 }
+#else
+static inline int gprinter_get_minor(void)
+{
+   return ida_simple_get(&printer_ida, 0, 0, GFP_KERNEL);
+}
+
+static inline void gprinter_put_minor(int minor)
+{
+   ida_simple_remove(&printer_ida, minor);
+}
+
+static int gprinter_setup(int);
+static void gprinter_cleanup(void);
+
+static void gprinter_free_inst(struct usb_function_instance *f)
+{
+   struct f_printer_opts *opts;
+
+   opts = container_of(f, struct f_printer_opts, func_inst);
+
+   mutex_lock(&printer_ida_lock);
+
+   gprinter_put_minor(opts->minor);
+   if (idr_is_empty(&printer_ida.idr))
+   gprinter_cleanup();
+
+   mutex_unlock(&printer_ida_lock);
+
+   kfree(opts);
+}
+
+static struct usb_function_instance *gprinter_alloc_inst(void)
+{
+   struct f_printer_opts *opts;
+   struct usb_function_instance *ret;
+   int status = 0;
+
+   opts = kzalloc(sizeof(*opts), GFP_KERNEL);
+   if (!opts)
+   return ERR_PTR(-ENOMEM);
+
+   opts->func_inst.free_func_inst = gprinter_free_inst;
+   ret = &opts->func_inst;
+
+   mutex_lock(&printer_ida_lock);
+
+   if (idr_is_empty(&printer_ida.idr)) {
+   status = gprinter_setup(PRINTER_MINORS);
+   if (status) {
+   r

[PATCHv2 4/6] usb: gadget: f_printer: remove compatibility layer

[Andrzej Pietrasiewicz]

There are no old interface users left, so it can be removed.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/f_printer.c | 113 
 1 file changed, 113 deletions(-)

diff --git a/drivers/usb/gadget/function/f_printer.c 
b/drivers/usb/gadget/function/f_printer.c
index 93f4d4e..7afe17d 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -57,10 +57,8 @@
 
 static int major, minors;
 static struct class *usb_gadget_class;
-#ifndef USBF_PRINTER_INCLUDED
 static DEFINE_IDA(printer_ida);
 static DEFINE_MUTEX(printer_ida_lock); /* protects access do printer_ida */
-#endif
 
 /*-*/
 
@@ -1118,53 +1116,6 @@ fail_tx_reqs:
 
 }
 
-#ifdef USBF_PRINTER_INCLUDED
-static void printer_func_unbind(struct usb_configuration *c,
-   struct usb_function *f)
-{
-   struct printer_dev  *dev;
-   struct usb_request  *req;
-
-   dev = func_to_printer(f);
-
-   device_destroy(usb_gadget_class, MKDEV(major, dev->minor));
-
-   /* Remove Character Device */
-   cdev_del(&dev->printer_cdev);
-
-   /* we must already have been disconnected ... no i/o may be active */
-   WARN_ON(!list_empty(&dev->tx_reqs_active));
-   WARN_ON(!list_empty(&dev->rx_reqs_active));
-
-   /* Free all memory for this driver. */
-   while (!list_empty(&dev->tx_reqs)) {
-   req = container_of(dev->tx_reqs.next, struct usb_request,
-   list);
-   list_del(&req->list);
-   printer_req_free(dev->in_ep, req);
-   }
-
-   if (dev->current_rx_req != NULL)
-   printer_req_free(dev->out_ep, dev->current_rx_req);
-
-   while (!list_empty(&dev->rx_reqs)) {
-   req = container_of(dev->rx_reqs.next,
-   struct usb_request, list);
-   list_del(&req->list);
-   printer_req_free(dev->out_ep, req);
-   }
-
-   while (!list_empty(&dev->rx_buffers)) {
-   req = container_of(dev->rx_buffers.next,
-   struct usb_request, list);
-   list_del(&req->list);
-   printer_req_free(dev->out_ep, req);
-   }
-   usb_free_all_descriptors(f);
-   kfree(dev);
-}
-#endif
-
 static int printer_func_set_alt(struct usb_function *f,
unsigned intf, unsigned alt)
 {
@@ -1189,68 +1140,6 @@ static void printer_func_disable(struct usb_function *f)
spin_unlock_irqrestore(&dev->lock, flags);
 }
 
-#ifdef USBF_PRINTER_INCLUDED
-static int f_printer_bind_config(struct usb_configuration *c, char *pnp_str,
-char *pnp_string, unsigned q_len, int minor)
-{
-   struct printer_dev  *dev;
-   int status = -ENOMEM;
-   size_t  len;
-
-   if (minor >= minors)
-   return -ENOENT;
-
-   dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-   if (!dev)
-   return -ENOMEM;
-
-   dev->pnp_string = pnp_string;
-   dev->minor = minor;
-
-   dev->function.name = shortname;
-   dev->function.bind = printer_func_bind;
-   dev->function.setup = printer_func_setup;
-   dev->function.unbind = printer_func_unbind;
-   dev->function.set_alt = printer_func_set_alt;
-   dev->function.disable = printer_func_disable;
-   dev->function.req_match = gprinter_req_match;
-   INIT_LIST_HEAD(&dev->tx_reqs);
-   INIT_LIST_HEAD(&dev->rx_reqs);
-   INIT_LIST_HEAD(&dev->rx_buffers);
-
-   if (pnp_str)
-   strlcpy(&dev->pnp_string[2], pnp_str, PNP_STRING_LEN - 2);
-
-   len = strlen(pnp_string);
-   pnp_string[0] = (len >> 8) & 0xFF;
-   pnp_string[1] = len & 0xFF;
-
-   spin_lock_init(&dev->lock);
-   mutex_init(&dev->lock_printer_io);
-   INIT_LIST_HEAD(&dev->tx_reqs_active);
-   INIT_LIST_HEAD(&dev->rx_reqs_active);
-   init_waitqueue_head(&dev->rx_wait);
-   init_waitqueue_head(&dev->tx_wait);
-   init_waitqueue_head(&dev->tx_flush_wait);
-
-   dev->interface = -1;
-   dev->printer_cdev_open = 0;
-   dev->printer_status = PRINTER_NOT_ERROR;
-   dev->current_rx_req = NULL;
-   dev->current_rx_bytes = 0;
-   dev->current_rx_buf = NULL;
-   dev->q_len = q_len;
-
-   status = usb_add_function(c, &dev->function);
-   if (status) {
-   kfree(dev);
-   return status;
-   }
-
-   INFO(dev, "%s, version: " DRIVER_VERSION "\n", driver_desc);
-   return 0;
-}
-#else
 static inline int gprinter_get_minor(voi

[PATCHv2 1/6] usb: gadget: f_printer: eliminate legacy comment

[Andrzej Pietrasiewicz]

With multiple (and dynamically allocated and removed) instances of the
printer function it does not make sense to depend on a component of a
particular struct printer_dev in order to clean up after _all_ printer
instances.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/f_printer.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_printer.c 
b/drivers/usb/gadget/function/f_printer.c
index 170f9b1..0847972 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -1268,7 +1268,6 @@ static int gprinter_setup(int count)
return status;
 }
 
-/* must be called with struct printer_dev's lock_printer_io held */
 static void gprinter_cleanup(void)
 {
if (major) {
-- 
1.9.1

--
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

Re: [PATCH 00/29] Equivalent of g_printer with configfs

[Andrzej Pietrasiewicz]

W dniu 23.02.2015 o 16:01, Andrzej Pietrasiewicz pisze:

This series aims at integrating configfs into hid, the way


Of course I meant printer. Sorry about confusion.

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

[PATCH 04/29] usb: gadget: printer: eliminate random pointer dereference

[Andrzej Pietrasiewicz]

struct printer_dev contains 3 list heads: tx_reqs, rx_reqs and rx_buffers.
There is just one instance of this structure in the driver and it is
file static, and as such initialized with all zeros.

If device_create() or cdev_add() fails then "goto fail" branch is taken,
which results in printer_cfg_unbind() call. The latter checks if
tx_reqs, rx_reqs and rx_buffers lists are empty. The check for emptiness
is in fact a check whether the "next" member of struct list_head points
to the head of the list. But the heads of the lists in question have
not been initialized yet and, as mentioned above, contain all zeros,
so list_empty() returns false and respective "while" loop body starts
executing. Here, container_of() just subtracts the offset of a struct
usb_request member from an address of this same member, which results in
a value somewhere near 0 or 0xfff...ff. And the argument to list_del()
dereferences such a pointer which causes a disaster.

This patch moves respective INIT_LIST_HEAD() invocations to a point before
"goto fail" branch can be taken.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 21ea317..12247d3 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1190,6 +1190,9 @@ static int __init printer_bind_config(struct 
usb_configuration *c)
dev->function.unbind = printer_func_unbind;
dev->function.set_alt = printer_func_set_alt;
dev->function.disable = printer_func_disable;
+   INIT_LIST_HEAD(&dev->tx_reqs);
+   INIT_LIST_HEAD(&dev->rx_reqs);
+   INIT_LIST_HEAD(&dev->rx_buffers);
 
status = usb_add_function(c, &dev->function);
if (status)
@@ -1233,11 +1236,8 @@ static int __init printer_bind_config(struct 
usb_configuration *c)
 
spin_lock_init(&dev->lock);
mutex_init(&dev->lock_printer_io);
-   INIT_LIST_HEAD(&dev->tx_reqs);
INIT_LIST_HEAD(&dev->tx_reqs_active);
-   INIT_LIST_HEAD(&dev->rx_reqs);
INIT_LIST_HEAD(&dev->rx_reqs_active);
-   INIT_LIST_HEAD(&dev->rx_buffers);
init_waitqueue_head(&dev->rx_wait);
init_waitqueue_head(&dev->tx_wait);
init_waitqueue_head(&dev->tx_flush_wait);
-- 
1.9.1

--
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

[PATCH 10/29] usb: gadget: printer: move function-related bind code to function's bind

[Andrzej Pietrasiewicz]

In order to factor out a reusable f_printer.c, the code related to the
function should be placed in functions related to the function.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 114 +---
 1 file changed, 66 insertions(+), 48 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 494cd8a..c857044 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -85,6 +85,7 @@ struct printer_dev {
struct cdev printer_cdev;
u8  printer_cdev_open;
wait_queue_head_t   wait;
+   unsignedq_len;
struct usb_function function;
 };
 
@@ -1045,18 +1046,25 @@ unknown:
 static int __init printer_func_bind(struct usb_configuration *c,
struct usb_function *f)
 {
+   struct usb_gadget *gadget = c->cdev->gadget;
struct printer_dev *dev = container_of(f, struct printer_dev, function);
+   struct device *pdev;
struct usb_composite_dev *cdev = c->cdev;
struct usb_ep *in_ep;
struct usb_ep *out_ep = NULL;
+   struct usb_request *req;
int id;
int ret;
+   u32 i;
 
id = usb_interface_id(c, f);
if (id < 0)
return id;
intf_desc.bInterfaceNumber = id;
 
+   /* finish hookup to lower layer ... */
+   dev->gadget = gadget;
+
/* all we really need is bulk IN/OUT */
in_ep = usb_ep_autoconfig(cdev->gadget, &fs_ep_in_desc);
if (!in_ep) {
@@ -1085,7 +1093,64 @@ autoconf_fail:
 
dev->in_ep = in_ep;
dev->out_ep = out_ep;
+
+   ret = -ENOMEM;
+   for (i = 0; i < dev->q_len; i++) {
+   req = printer_req_alloc(dev->in_ep, USB_BUFSIZE, GFP_KERNEL);
+   if (!req)
+   goto fail_tx_reqs;
+   list_add(&req->list, &dev->tx_reqs);
+   }
+
+   for (i = 0; i < dev->q_len; i++) {
+   req = printer_req_alloc(dev->out_ep, USB_BUFSIZE, GFP_KERNEL);
+   if (!req)
+   goto fail_rx_reqs;
+   list_add(&req->list, &dev->rx_reqs);
+   }
+
+   /* Setup the sysfs files for the printer gadget. */
+   pdev = device_create(usb_gadget_class, NULL, g_printer_devno,
+ NULL, "g_printer");
+   if (IS_ERR(pdev)) {
+   ERROR(dev, "Failed to create device: g_printer\n");
+   ret = PTR_ERR(pdev);
+   goto fail_rx_reqs;
+   }
+
+   /*
+* Register a character device as an interface to a user mode
+* program that handles the printer specific functionality.
+*/
+   cdev_init(&dev->printer_cdev, &printer_io_operations);
+   dev->printer_cdev.owner = THIS_MODULE;
+   ret = cdev_add(&dev->printer_cdev, g_printer_devno, 1);
+   if (ret) {
+   ERROR(dev, "Failed to open char device\n");
+   goto fail_cdev_add;
+   }
+
return 0;
+
+fail_cdev_add:
+   device_destroy(usb_gadget_class, g_printer_devno);
+
+fail_rx_reqs:
+   while (!list_empty(&dev->rx_reqs)) {
+   req = container_of(dev->rx_reqs.next, struct usb_request, list);
+   list_del(&req->list);
+   printer_req_free(dev->out_ep, req);
+   }
+
+fail_tx_reqs:
+   while (!list_empty(&dev->tx_reqs)) {
+   req = container_of(dev->tx_reqs.next, struct usb_request, list);
+   list_del(&req->list);
+   printer_req_free(dev->in_ep, req);
+   }
+
+   return ret;
+
 }
 
 static void printer_func_unbind(struct usb_configuration *c,
@@ -1173,13 +1238,9 @@ static struct usb_configuration printer_cfg_driver = {
 static int f_printer_bind_config(struct usb_configuration *c, char *pnp_str,
 unsigned q_len)
 {
-   struct usb_gadget   *gadget = c->cdev->gadget;
struct printer_dev  *dev;
-   struct device   *pdev;
int status = -ENOMEM;
size_t  len;
-   u32 i;
-   struct usb_request  *req;
 
dev = &usb_printer_gadget;
 
@@ -1193,31 +1254,11 @@ static int f_printer_bind_config(struct 
usb_configuration *c, char *pnp_str,
INIT_LIST_HEAD(&dev->rx_reqs);
INIT_LIST_HEAD(&dev->rx_buffers);
 
+   dev->q_len = q_len;
status = usb_add_function(c, &dev->function);
if (status)
return status;
 
-   /* Setup the sysfs files for the printer gadget. */
-   pdev = device_create(usb_gadget_class, NULL, g_printer_devno,
-   

[PATCH 12/29] usb: gadget: printer: move function-related unbind code to function's unbind

[Andrzej Pietrasiewicz]

In order to factor out a reusable f_printer.c, the code related to the
function should be placed in functions related to the function.

printer_cfg_unbind() becomes empty, so it is removed.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 58 -
 1 file changed, 25 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 5dbb93a..84e6cdd 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1156,43 +1156,11 @@ fail_tx_reqs:
 static void printer_func_unbind(struct usb_configuration *c,
struct usb_function *f)
 {
-   usb_free_all_descriptors(f);
-}
-
-static int printer_func_set_alt(struct usb_function *f,
-   unsigned intf, unsigned alt)
-{
-   struct printer_dev *dev = container_of(f, struct printer_dev, function);
-   int ret = -ENOTSUPP;
-
-   if (!alt)
-   ret = set_interface(dev, intf);
-
-   return ret;
-}
-
-static void printer_func_disable(struct usb_function *f)
-{
-   struct printer_dev *dev = container_of(f, struct printer_dev, function);
-   unsigned long   flags;
-
-   DBG(dev, "%s\n", __func__);
-
-   spin_lock_irqsave(&dev->lock, flags);
-   printer_reset_interface(dev);
-   spin_unlock_irqrestore(&dev->lock, flags);
-}
-
-static void printer_cfg_unbind(struct usb_configuration *c)
-{
struct printer_dev  *dev;
struct usb_request  *req;
 
dev = &usb_printer_gadget;
 
-   DBG(dev, "%s\n", __func__);
-
-   /* Remove sysfs files */
device_destroy(usb_gadget_class, g_printer_devno);
 
/* Remove Character Device */
@@ -1226,11 +1194,35 @@ static void printer_cfg_unbind(struct usb_configuration 
*c)
list_del(&req->list);
printer_req_free(dev->out_ep, req);
}
+   usb_free_all_descriptors(f);
+}
+
+static int printer_func_set_alt(struct usb_function *f,
+   unsigned intf, unsigned alt)
+{
+   struct printer_dev *dev = container_of(f, struct printer_dev, function);
+   int ret = -ENOTSUPP;
+
+   if (!alt)
+   ret = set_interface(dev, intf);
+
+   return ret;
+}
+
+static void printer_func_disable(struct usb_function *f)
+{
+   struct printer_dev *dev = container_of(f, struct printer_dev, function);
+   unsigned long   flags;
+
+   DBG(dev, "%s\n", __func__);
+
+   spin_lock_irqsave(&dev->lock, flags);
+   printer_reset_interface(dev);
+   spin_unlock_irqrestore(&dev->lock, flags);
 }
 
 static struct usb_configuration printer_cfg_driver = {
.label  = "printer",
-   .unbind = printer_cfg_unbind,
.bConfigurationValue= 1,
.bmAttributes   = USB_CONFIG_ATT_ONE | USB_CONFIG_ATT_SELFPOWER,
 };
-- 
1.9.1

--
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

[PATCH 02/29] usb: gadget: printer: enqueue printer's response for setup request

[Andrzej Pietrasiewicz]

Function-specific setup requests should be handled in such a way, that
apart from filling in the data buffer, the requests are also actually
enqueued: if function-specific setup is called from composte_setup(),
the "usb_ep_queue()" block of code in composite_setup() is skipped.

The printer function lacks this part and it results in e.g. get device id
requests failing: the host expects some response, the device prepares it
but does not equeue it for sending to the host, so the host finally asserts
timeout.

This patch adds enqueueing the prepared responses.

Cc:  # v3.4+
Fixes: 2e87edf49227: "usb: gadget: make g_printer use composite"
Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 9054598..6385c19 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1031,6 +1031,15 @@ unknown:
break;
}
/* host either stalls (value < 0) or reports success */
+   if (value >= 0) {
+   req->length = value;
+   req->zero = value < wLength;
+   value = usb_ep_queue(cdev->gadget->ep0, req, GFP_ATOMIC);
+   if (value < 0) {
+   ERROR(dev, "%s:%d Error!\n", __func__, __LINE__);
+   req->status = 0;
+   }
+   }
return value;
 }
 
-- 
1.9.1

--
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

[PATCH 03/29] usb: gadget: printer: remove unused and empty printer_unbind

[Andrzej Pietrasiewicz]

The unbind() method is optional is usb_composite_driver.
In this particular driver the method does nothing so it can be removed.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 6385c19..21ea317 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1288,11 +1288,6 @@ fail:
return status;
 }
 
-static int printer_unbind(struct usb_composite_dev *cdev)
-{
-   return 0;
-}
-
 static int __init printer_bind(struct usb_composite_dev *cdev)
 {
int ret;
@@ -1317,7 +1312,6 @@ static __refdata struct usb_composite_driver 
printer_driver = {
.strings= dev_strings,
.max_speed  = USB_SPEED_SUPER,
.bind   = printer_bind,
-   .unbind = printer_unbind,
 };
 
 static int __init
-- 
1.9.1

--
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

[PATCH 08/29] usb: gadget: printer: follow the naming convention for usb_add_config callback

[Andrzej Pietrasiewicz]

Legacy gadgets, before converting them to the new function framework,
used to use the name _do_config() for usb_add_config()'s
callback.

This patch changes the name so that it is easier to follow
the convention.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index a9c3e57..c865833 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1170,7 +1170,7 @@ static struct usb_configuration printer_cfg_driver = {
.bmAttributes   = USB_CONFIG_ATT_ONE | USB_CONFIG_ATT_SELFPOWER,
 };
 
-static int __init printer_bind_config(struct usb_configuration *c)
+static int __init printer_do_config(struct usb_configuration *c)
 {
struct usb_gadget   *gadget = c->cdev->gadget;
struct printer_dev  *dev;
@@ -1287,7 +1287,7 @@ static int __init printer_bind(struct usb_composite_dev 
*cdev)
device_desc.iProduct = strings[USB_GADGET_PRODUCT_IDX].id;
device_desc.iSerialNumber = strings[USB_GADGET_SERIAL_IDX].id;
 
-   ret = usb_add_config(cdev, &printer_cfg_driver, printer_bind_config);
+   ret = usb_add_config(cdev, &printer_cfg_driver, printer_do_config);
if (ret)
return ret;
usb_composite_overwrite_options(cdev, &coverwrite);
-- 
1.9.1

--
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

[PATCH 16/29] usb: gadget: printer: call gprinter_setup() from gadget's bind

[Andrzej Pietrasiewicz]

Call gprinter_setup() from gadget's bind instead of module's init.
Call gprinter_cleaup() corerspondingly. This detaches printer function's
logic from legacy printer gadget's implementation.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 35 ++-
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 7635f12..d5fb41b 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1330,45 +1330,47 @@ static int __init printer_bind(struct usb_composite_dev 
*cdev)
 {
int ret;
 
+   ret = gprinter_setup();
+   if (ret)
+   return ret;
+
ret = usb_string_ids_tab(cdev, strings);
-   if (ret < 0)
+   if (ret < 0) {
+   gprinter_cleanup();
return ret;
+   }
device_desc.iManufacturer = strings[USB_GADGET_MANUFACTURER_IDX].id;
device_desc.iProduct = strings[USB_GADGET_PRODUCT_IDX].id;
device_desc.iSerialNumber = strings[USB_GADGET_SERIAL_IDX].id;
 
ret = usb_add_config(cdev, &printer_cfg_driver, printer_do_config);
-   if (ret)
+   if (ret) {
+   gprinter_cleanup();
return ret;
+   }
usb_composite_overwrite_options(cdev, &coverwrite);
return ret;
 }
 
+static int __exit printer_unbind(struct usb_composite_dev *cdev)
+{
+   gprinter_cleanup();
+   return 0;
+}
+
 static __refdata struct usb_composite_driver printer_driver = {
.name   = shortname,
.dev= &device_desc,
.strings= dev_strings,
.max_speed  = USB_SPEED_SUPER,
.bind   = printer_bind,
+   .unbind = printer_unbind,
 };
 
 static int __init
 init(void)
 {
-   int status;
-
-   status = gprinter_setup();
-   if (status)
-   return status;
-
-   status = usb_composite_probe(&printer_driver);
-   if (status) {
-   class_destroy(usb_gadget_class);
-   unregister_chrdev_region(g_printer_devno, 1);
-   pr_err("usb_gadget_probe_driver %x\n", status);
-   }
-
-   return status;
+   return usb_composite_probe(&printer_driver);
 }
 module_init(init);
 
@@ -1377,7 +1379,6 @@ cleanup(void)
 {
mutex_lock(&usb_printer_gadget.lock_printer_io);
usb_composite_unregister(&printer_driver);
-   gprinter_cleanup();
mutex_unlock(&usb_printer_gadget.lock_printer_io);
 }
 module_exit(cleanup);
-- 
1.9.1

--
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

[PATCH 05/29] usb: gadget: printer: revert usb_add_function() effect in error recovery

[Andrzej Pietrasiewicz]

Whenever the "goto fail" branch is taken, the effect of usb_add_function()
should be reverted.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 12247d3..eb02a6b 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1285,6 +1285,7 @@ static int __init printer_bind_config(struct 
usb_configuration *c)
 
 fail:
printer_cfg_unbind(c);
+   usb_remove_function(c, &dev->function);
return status;
 }
 
-- 
1.9.1

--
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

[PATCH 09/29] usb: gadget: printer: standardize printer_do_config

[Andrzej Pietrasiewicz]

Follow the convention of distributing source code between
_do_config() and _bind_config().

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 39 +++--
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index c865833..494cd8a 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1170,7 +1170,8 @@ static struct usb_configuration printer_cfg_driver = {
.bmAttributes   = USB_CONFIG_ATT_ONE | USB_CONFIG_ATT_SELFPOWER,
 };
 
-static int __init printer_do_config(struct usb_configuration *c)
+static int f_printer_bind_config(struct usb_configuration *c, char *pnp_str,
+unsigned q_len)
 {
struct usb_gadget   *gadget = c->cdev->gadget;
struct printer_dev  *dev;
@@ -1180,8 +1181,6 @@ static int __init printer_do_config(struct 
usb_configuration *c)
u32 i;
struct usb_request  *req;
 
-   usb_ep_autoconfig_reset(gadget);
-
dev = &usb_printer_gadget;
 
dev->function.name = shortname;
@@ -1219,21 +1218,13 @@ static int __init printer_do_config(struct 
usb_configuration *c)
goto fail;
}
 
-   if (iPNPstring)
-   strlcpy(&pnp_string[2], iPNPstring, (sizeof pnp_string)-2);
+   if (pnp_str)
+   strlcpy(&pnp_string[2], pnp_str, sizeof(pnp_string) - 2);
 
len = strlen(pnp_string);
pnp_string[0] = (len >> 8) & 0xFF;
pnp_string[1] = len & 0xFF;
 
-   usb_gadget_set_selfpowered(gadget);
-
-   if (gadget_is_otg(gadget)) {
-   otg_descriptor.bmAttributes |= USB_OTG_HNP;
-   printer_cfg_driver.descriptors = otg_desc;
-   printer_cfg_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
-   }
-
spin_lock_init(&dev->lock);
mutex_init(&dev->lock_printer_io);
INIT_LIST_HEAD(&dev->tx_reqs_active);
@@ -1250,14 +1241,14 @@ static int __init printer_do_config(struct 
usb_configuration *c)
dev->current_rx_buf = NULL;
 
status = -ENOMEM;
-   for (i = 0; i < QLEN; i++) {
+   for (i = 0; i < q_len; i++) {
req = printer_req_alloc(dev->in_ep, USB_BUFSIZE, GFP_KERNEL);
if (!req)
goto fail;
list_add(&req->list, &dev->tx_reqs);
}
 
-   for (i = 0; i < QLEN; i++) {
+   for (i = 0; i < q_len; i++) {
req = printer_req_alloc(dev->out_ep, USB_BUFSIZE, GFP_KERNEL);
if (!req)
goto fail;
@@ -1276,6 +1267,24 @@ fail:
return status;
 }
 
+static int __init printer_do_config(struct usb_configuration *c)
+{
+   struct usb_gadget   *gadget = c->cdev->gadget;
+
+   usb_ep_autoconfig_reset(gadget);
+
+   usb_gadget_set_selfpowered(gadget);
+
+   if (gadget_is_otg(gadget)) {
+   otg_descriptor.bmAttributes |= USB_OTG_HNP;
+   printer_cfg_driver.descriptors = otg_desc;
+   printer_cfg_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
+   }
+
+   return f_printer_bind_config(c, iPNPstring, QLEN);
+
+}
+
 static int __init printer_bind(struct usb_composite_dev *cdev)
 {
int ret;
-- 
1.9.1

--
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

[PATCH 15/29] usb: gadget: printer: add setup and cleanup functions

[Andrzej Pietrasiewicz]

Factor out gprinter_setup() and gprinter_cleanup() so that it is
easy to change the place they are called from.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 46 +
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 42c46da..7635f12 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1298,6 +1298,34 @@ static int __init printer_do_config(struct 
usb_configuration *c)
 
 }
 
+int gprinter_setup(void)
+{
+   int status;
+
+   usb_gadget_class = class_create(THIS_MODULE, "usb_printer_gadget");
+   if (IS_ERR(usb_gadget_class)) {
+   status = PTR_ERR(usb_gadget_class);
+   pr_err("unable to create usb_gadget class %d\n", status);
+   return status;
+   }
+
+   status = alloc_chrdev_region(&g_printer_devno, 0, 1,
+   "USB printer gadget");
+   if (status) {
+   pr_err("alloc_chrdev_region %d\n", status);
+   class_destroy(usb_gadget_class);
+   }
+
+   return status;
+}
+
+/* must be called with struct printer_dev's lock_printer_io held */
+void gprinter_cleanup(void)
+{
+   unregister_chrdev_region(g_printer_devno, 1);
+   class_destroy(usb_gadget_class);
+}
+
 static int __init printer_bind(struct usb_composite_dev *cdev)
 {
int ret;
@@ -1329,20 +1357,9 @@ init(void)
 {
int status;
 
-   usb_gadget_class = class_create(THIS_MODULE, "usb_printer_gadget");
-   if (IS_ERR(usb_gadget_class)) {
-   status = PTR_ERR(usb_gadget_class);
-   pr_err("unable to create usb_gadget class %d\n", status);
-   return status;
-   }
-
-   status = alloc_chrdev_region(&g_printer_devno, 0, 1,
-   "USB printer gadget");
-   if (status) {
-   pr_err("alloc_chrdev_region %d\n", status);
-   class_destroy(usb_gadget_class);
+   status = gprinter_setup();
+   if (status)
return status;
-   }
 
status = usb_composite_probe(&printer_driver);
if (status) {
@@ -1360,8 +1377,7 @@ cleanup(void)
 {
mutex_lock(&usb_printer_gadget.lock_printer_io);
usb_composite_unregister(&printer_driver);
-   unregister_chrdev_region(g_printer_devno, 1);
-   class_destroy(usb_gadget_class);
+   gprinter_cleanup();
mutex_unlock(&usb_printer_gadget.lock_printer_io);
 }
 module_exit(cleanup);
-- 
1.9.1

--
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

[PATCH 13/29] usb: gadget: printer: define pnp string buffer length

[Andrzej Pietrasiewicz]

Avoid using magic numbers.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 84e6cdd..db5e2f0 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -276,9 +276,11 @@ static inline struct usb_endpoint_descriptor 
*ep_desc(struct usb_gadget *gadget,
 
 /* descriptors that are built on-demand */
 
+#define PNP_STRING_LEN 1024
+
 static charproduct_desc [40] = DRIVER_DESC;
 static charserial_num [40] = "1";
-static charpnp_string [1024] =
+static charpnp_string[PNP_STRING_LEN] =
"XXMFG:linux;MDL:g_printer;CLS:PRINTER;SN:1;";
 
 /* static strings, in UTF-8 */
@@ -1247,7 +1249,7 @@ static int f_printer_bind_config(struct usb_configuration 
*c, char *pnp_str,
INIT_LIST_HEAD(&dev->rx_buffers);
 
if (pnp_str)
-   strlcpy(&pnp_string[2], pnp_str, sizeof(pnp_string) - 2);
+   strlcpy(&pnp_string[2], pnp_str, PNP_STRING_LEN - 2);
 
len = strlen(pnp_string);
pnp_string[0] = (len >> 8) & 0xFF;
-- 
1.9.1

--
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

[PATCH 18/29] usb: gadget: printer: don't access file global usb_printer_gadget in function's code

[Andrzej Pietrasiewicz]

The printer_dev can be recovered from printer_func_unbind() function's
parameters.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 9691e57..ef4f4ce 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1159,7 +1159,7 @@ static void printer_func_unbind(struct usb_configuration 
*c,
struct printer_dev  *dev;
struct usb_request  *req;
 
-   dev = &usb_printer_gadget;
+   dev = container_of(f, struct printer_dev, function);
 
device_destroy(usb_gadget_class, g_printer_devno);
 
-- 
1.9.1

--
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

[PATCH 28/29] usb: gadget: printer: use module_usb_composite_driver helper macro

[Andrzej Pietrasiewicz]

Substitute some boilerplate code with a dedicated macro.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 14 +-
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index a8050f8..d5b6ee7 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -208,19 +208,7 @@ static __refdata struct usb_composite_driver 
printer_driver = {
.unbind = printer_unbind,
 };
 
-static int __init
-init(void)
-{
-   return usb_composite_probe(&printer_driver);
-}
-module_init(init);
-
-static void __exit
-cleanup(void)
-{
-   usb_composite_unregister(&printer_driver);
-}
-module_exit(cleanup);
+module_usb_composite_driver(printer_driver);
 
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_AUTHOR("Craig Nadler");
-- 
1.9.1

--
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

[PATCH 21/29] usb: gadget: printer: name class specific requests

[Andrzej Pietrasiewicz]

Avoid using magic numbers.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index f103c11..fcfa618 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -47,6 +47,9 @@ USB_GADGET_COMPOSITE_OPTIONS();
 
 #define DRIVER_DESC"Printer Gadget"
 #define DRIVER_VERSION "2007 OCT 06"
+#define GET_DEVICE_ID  0
+#define GET_PORT_STATUS1
+#define SOFT_RESET 2
 
 static const char shortname [] = "printer";
 static const char driver_desc [] = DRIVER_DESC;
@@ -992,7 +995,7 @@ static int printer_func_setup(struct usb_function *f,
switch (ctrl->bRequestType&USB_TYPE_MASK) {
case USB_TYPE_CLASS:
switch (ctrl->bRequest) {
-   case 0: /* Get the IEEE-1284 PNP String */
+   case GET_DEVICE_ID: /* Get the IEEE-1284 PNP String */
/* Only one printer interface is supported. */
if ((wIndex>>8) != dev->interface)
break;
@@ -1003,7 +1006,7 @@ static int printer_func_setup(struct usb_function *f,
&dev->pnp_string[2]);
break;
 
-   case 1: /* Get Port Status */
+   case GET_PORT_STATUS: /* Get Port Status */
/* Only one printer interface is supported. */
if (wIndex != dev->interface)
break;
@@ -1012,7 +1015,7 @@ static int printer_func_setup(struct usb_function *f,
value = min(wLength, (u16) 1);
break;
 
-   case 2: /* Soft Reset */
+   case SOFT_RESET: /* Soft Reset */
/* Only one printer interface is supported. */
if (wIndex != dev->interface)
break;
-- 
1.9.1

--
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

[PATCH 19/29] usb: gadget: printer: add container_of helper for printer_dev

[Andrzej Pietrasiewicz]

5 uses of container_of() in the same context justify wrapping it
in a static inline function.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index ef4f4ce..f103c11 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -89,6 +89,11 @@ struct printer_dev {
struct usb_function function;
 };
 
+static inline struct printer_dev *func_to_printer(struct usb_function *f)
+{
+   return container_of(f, struct printer_dev, function);
+}
+
 static struct printer_dev usb_printer_gadget;
 
 /*-*/
@@ -973,7 +978,7 @@ static void printer_soft_reset(struct printer_dev *dev)
 static int printer_func_setup(struct usb_function *f,
const struct usb_ctrlrequest *ctrl)
 {
-   struct printer_dev *dev = container_of(f, struct printer_dev, function);
+   struct printer_dev *dev = func_to_printer(f);
struct usb_composite_dev *cdev = f->config->cdev;
struct usb_request  *req = cdev->req;
int value = -EOPNOTSUPP;
@@ -1047,7 +1052,7 @@ static int __init printer_func_bind(struct 
usb_configuration *c,
struct usb_function *f)
 {
struct usb_gadget *gadget = c->cdev->gadget;
-   struct printer_dev *dev = container_of(f, struct printer_dev, function);
+   struct printer_dev *dev = func_to_printer(f);
struct device *pdev;
struct usb_composite_dev *cdev = c->cdev;
struct usb_ep *in_ep;
@@ -1159,7 +1164,7 @@ static void printer_func_unbind(struct usb_configuration 
*c,
struct printer_dev  *dev;
struct usb_request  *req;
 
-   dev = container_of(f, struct printer_dev, function);
+   dev = func_to_printer(f);
 
device_destroy(usb_gadget_class, g_printer_devno);
 
@@ -1200,7 +1205,7 @@ static void printer_func_unbind(struct usb_configuration 
*c,
 static int printer_func_set_alt(struct usb_function *f,
unsigned intf, unsigned alt)
 {
-   struct printer_dev *dev = container_of(f, struct printer_dev, function);
+   struct printer_dev *dev = func_to_printer(f);
int ret = -ENOTSUPP;
 
if (!alt)
@@ -1211,7 +1216,7 @@ static int printer_func_set_alt(struct usb_function *f,
 
 static void printer_func_disable(struct usb_function *f)
 {
-   struct printer_dev *dev = container_of(f, struct printer_dev, function);
+   struct printer_dev *dev = func_to_printer(f);
unsigned long   flags;
 
DBG(dev, "%s\n", __func__);
-- 
1.9.1

--
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

[PATCH 26/29] usb: gadget: printer: convert to new interface of f_printer

[Andrzej Pietrasiewicz]

The goal is to remove the old function interface, so its (only) user
must be converted to the new interface.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/Kconfig   |  1 +
 drivers/usb/gadget/legacy/printer.c | 50 ++---
 2 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/gadget/legacy/Kconfig 
b/drivers/usb/gadget/legacy/Kconfig
index 113c87e..d5a7102 100644
--- a/drivers/usb/gadget/legacy/Kconfig
+++ b/drivers/usb/gadget/legacy/Kconfig
@@ -301,6 +301,7 @@ config USB_MIDI_GADGET
 config USB_G_PRINTER
tristate "Printer Gadget"
select USB_LIBCOMPOSITE
+   select USB_F_PRINTER
help
  The Printer Gadget channels data between the USB host and a
  userspace program driving the print engine. The user space
diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index 770b504..a8050f8 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -29,12 +29,7 @@ USB_GADGET_COMPOSITE_OPTIONS();
 static const char shortname [] = "printer";
 static const char driver_desc [] = DRIVER_DESC;
 
-/*
- * This will be changed when f_printer is converted
- * to the new function interface.
- */
-#define USBF_PRINTER_INCLUDED
-#include "f_printer.c"
+#include "u_printer.h"
 
 /*-*/
 
@@ -65,6 +60,9 @@ module_param(qlen, uint, S_IRUGO|S_IWUSR);
 
 #define QLEN   qlen
 
+static struct usb_function_instance *fi_printer;
+static struct usb_function *f_printer;
+
 /*-*/
 
 /*
@@ -131,6 +129,7 @@ static struct usb_configuration printer_cfg_driver = {
 static int __init printer_do_config(struct usb_configuration *c)
 {
struct usb_gadget   *gadget = c->cdev->gadget;
+   int status = 0;
 
usb_ep_autoconfig_reset(gadget);
 
@@ -142,20 +141,41 @@ static int __init printer_do_config(struct 
usb_configuration *c)
printer_cfg_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
}
 
-   return f_printer_bind_config(c, iPNPstring, pnp_string, QLEN, 0);
+   f_printer = usb_get_function(fi_printer);
+   if (IS_ERR(f_printer))
+   return PTR_ERR(f_printer);
+
+   status = usb_add_function(c, f_printer);
+   if (status < 0)
+   usb_put_function(f_printer);
+
+   return status;
 }
 
 static int __init printer_bind(struct usb_composite_dev *cdev)
 {
-   int ret;
+   struct f_printer_opts *opts;
+   int ret, len;
 
-   ret = gprinter_setup(PRINTER_MINORS);
-   if (ret)
-   return ret;
+   fi_printer = usb_get_function_instance("printer");
+   if (IS_ERR(fi_printer))
+   return PTR_ERR(fi_printer);
+
+   if (iPNPstring)
+   strlcpy(&pnp_string[2], iPNPstring, PNP_STRING_LEN - 2);
+
+   len = strlen(pnp_string);
+   pnp_string[0] = (len >> 8) & 0xFF;
+   pnp_string[1] = len & 0xFF;
+
+   opts = container_of(fi_printer, struct f_printer_opts, func_inst);
+   opts->minor = 0;
+   memcpy(opts->pnp_string, pnp_string, PNP_STRING_LEN);
+   opts->q_len = QLEN;
 
ret = usb_string_ids_tab(cdev, strings);
if (ret < 0) {
-   gprinter_cleanup();
+   usb_put_function_instance(fi_printer);
return ret;
}
device_desc.iManufacturer = strings[USB_GADGET_MANUFACTURER_IDX].id;
@@ -164,7 +184,7 @@ static int __init printer_bind(struct usb_composite_dev 
*cdev)
 
ret = usb_add_config(cdev, &printer_cfg_driver, printer_do_config);
if (ret) {
-   gprinter_cleanup();
+   usb_put_function_instance(fi_printer);
return ret;
}
usb_composite_overwrite_options(cdev, &coverwrite);
@@ -173,7 +193,9 @@ static int __init printer_bind(struct usb_composite_dev 
*cdev)
 
 static int __exit printer_unbind(struct usb_composite_dev *cdev)
 {
-   gprinter_cleanup();
+   usb_put_function(f_printer);
+   usb_put_function_instance(fi_printer);
+
return 0;
 }
 
-- 
1.9.1

--
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

[PATCH 06/29] usb: gadget: printer: add missing error handling

[Andrzej Pietrasiewicz]

If cdev_add() in printer_bind_config() fails, care is taken to
reverse the effects of initializations completed until the fail
happens. But if printer_req_alloc() fails, it is just one of the
two lists that is cleaned up while the effects of cdev_add()
and device_create() are not reverted.

This patch changes error handling so that at least as much cleanup is done
as when a failure happens before printer_req_alloc() invocations.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 23 +--
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index eb02a6b..bbcd6aa 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1249,31 +1249,18 @@ static int __init printer_bind_config(struct 
usb_configuration *c)
dev->current_rx_bytes = 0;
dev->current_rx_buf = NULL;
 
+   status = -ENOMEM;
for (i = 0; i < QLEN; i++) {
req = printer_req_alloc(dev->in_ep, USB_BUFSIZE, GFP_KERNEL);
-   if (!req) {
-   while (!list_empty(&dev->tx_reqs)) {
-   req = container_of(dev->tx_reqs.next,
-   struct usb_request, list);
-   list_del(&req->list);
-   printer_req_free(dev->in_ep, req);
-   }
-   return -ENOMEM;
-   }
+   if (!req)
+   goto fail;
list_add(&req->list, &dev->tx_reqs);
}
 
for (i = 0; i < QLEN; i++) {
req = printer_req_alloc(dev->out_ep, USB_BUFSIZE, GFP_KERNEL);
-   if (!req) {
-   while (!list_empty(&dev->rx_reqs)) {
-   req = container_of(dev->rx_reqs.next,
-   struct usb_request, list);
-   list_del(&req->list);
-   printer_req_free(dev->out_ep, req);
-   }
-   return -ENOMEM;
-   }
+   if (!req)
+   goto fail;
list_add(&req->list, &dev->rx_reqs);
}
 
-- 
1.9.1

--
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

[PATCH 29/29] usb: gadget: printer: add configfs support

[Andrzej Pietrasiewicz]

Add support for configfs interface so that f_printer can be used as a
component of usb gadgets composed with it.

Signed-off-by: Andrzej Pietrasiewicz 
---
 .../ABI/testing/configfs-usb-gadget-printer|   9 ++
 Documentation/usb/gadget-testing.txt   |  47 
 drivers/usb/gadget/Kconfig |  13 +++
 drivers/usb/gadget/function/f_printer.c| 130 -
 drivers/usb/gadget/function/u_printer.h|   7 ++
 5 files changed, 204 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-printer

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-printer 
b/Documentation/ABI/testing/configfs-usb-gadget-printer
new file mode 100644
index 000..6b0714e
--- /dev/null
+++ b/Documentation/ABI/testing/configfs-usb-gadget-printer
@@ -0,0 +1,9 @@
+What:  /config/usb-gadget/gadget/functions/printer.name
+Date:  Apr 2015
+KernelVersion: 4.1
+Description:
+   The attributes:
+
+   pnp_string  - Data to be passed to the host in pnp string
+   q_len   - Number of requests per endpoint
+
diff --git a/Documentation/usb/gadget-testing.txt 
b/Documentation/usb/gadget-testing.txt
index 076ac7b..f45b2bf 100644
--- a/Documentation/usb/gadget-testing.txt
+++ b/Documentation/usb/gadget-testing.txt
@@ -19,6 +19,7 @@ provided by gadgets.
 16. UAC1 function
 17. UAC2 function
 18. UVC function
+19. PRINTER function
 
 
 1. ACM function
@@ -726,3 +727,49 @@ with these patches:
 http://www.spinics.net/lists/linux-usb/msg99220.html
 
 host: luvcview -f yuv
+
+19. PRINTER function
+
+
+The function is provided by usb_f_printer.ko module.
+
+Function-specific configfs interface
+
+
+The function name to use when creating the function directory is "printer".
+The printer function provides these attributes in its function directory:
+
+   pnp_string  - Data to be passed to the host in pnp string
+   q_len   - Number of requests per endpoint
+
+Testing the PRINTER function
+
+
+The most basic testing:
+
+device: run the gadget
+# ls -l /devices/virtual/usb_printer_gadget/
+
+should show g_printer.
+
+If udev is active, then /dev/g_printer should appear automatically.
+
+host:
+
+If udev is active, then e.g. /dev/usb/lp0 should appear.
+
+host->device transmission:
+
+device:
+# cat /dev/g_printer
+host:
+# cat > /dev/usb/lp0
+
+device->host transmission:
+
+# cat > /dev/g_printer
+host:
+# cat /dev/usb/lp0
+
+More advanced testing can be done with the prn_example
+described in Documentation/usb/gadget-printer.txt.
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 9d507cf..3bb0e67 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -437,6 +437,19 @@ config USB_CONFIGFS_F_UVC
  device. It provides a userspace API to process UVC control requests
  and stream video data to the host.
 
+config USB_CONFIGFS_F_PRINTER
+   bool "Printer function"
+   select USB_F_PRINTER
+   help
+ The Printer function channels data between the USB host and a
+ userspace program driving the print engine. The user space
+ program reads and writes the device file /dev/g_printer to
+ receive or send printer data. It can use ioctl calls to
+ the device file to get or set printer status.
+
+ For more information, see Documentation/usb/gadget_printer.txt
+ which includes sample code for accessing the device file.
+
 source "drivers/usb/gadget/legacy/Kconfig"
 
 endchoice
diff --git a/drivers/usb/gadget/function/f_printer.c 
b/drivers/usb/gadget/function/f_printer.c
index d168511..5522bb4 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -1140,6 +1140,117 @@ static void printer_func_disable(struct usb_function *f)
spin_unlock_irqrestore(&dev->lock, flags);
 }
 
+static inline struct f_printer_opts
+*to_f_printer_opts(struct config_item *item)
+{
+   return container_of(to_config_group(item), struct f_printer_opts,
+   func_inst.group);
+}
+
+CONFIGFS_ATTR_STRUCT(f_printer_opts);
+CONFIGFS_ATTR_OPS(f_printer_opts);
+
+static void printer_attr_release(struct config_item *item)
+{
+   struct f_printer_opts *opts = to_f_printer_opts(item);
+
+   usb_put_function_instance(&opts->func_inst);
+}
+
+static struct configfs_item_operations printer_item_ops = {
+   .release= printer_attr_release,
+   .show_attribute = f_printer_opts_attr_show,
+   .store_attribute = f_printer_opts_attr_store,
+};
+
+static ssize_t f_printer_opts_pnp_string_show(struct f_printer_opts *opts,
+ char *page)
+{
+   int result;
+
+   mutex_lock(&opts

[PATCH 20/29] usb: gadget: composite: add req_match method to usb_function

[Andrzej Pietrasiewicz]

Non-standard requests can encode the actual interface number in a
non-standard way. For example composite_setup() assumes
that it is w_index && 0xFF, but the printer function encodes the interface
number in a context-dependet way (either w_index or w_index >> 8).
This can lead to such requests being directed to wrong functions.

This patch adds req_match() method to usb_function. Its purpose is to
verify that a given request can be handled by a given function.
If any function within a configuration provides the method and it returns
true, then it is assumed that the right function is found.

If a function uses req_match(), it should try as hard as possible to
determine if the request is meant for it.

If no functions in a configuration provide req_match or none of them
returns true, then fall back to the usual approach.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/composite.c | 7 ++-
 include/linux/usb/composite.h  | 3 +++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 9fb9231..07cee80 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1758,6 +1758,11 @@ unknown:
 * take such requests too, if that's ever needed:  to work
 * in config 0, etc.
 */
+   list_for_each_entry(f, &cdev->config->functions, list)
+   if (f->req_match && f->req_match(f, ctrl))
+   break;
+   if (&f->list != &cdev->config->functions)
+   goto try_fun_setup;
switch (ctrl->bRequestType & USB_RECIP_MASK) {
case USB_RECIP_INTERFACE:
if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
@@ -1775,7 +1780,7 @@ unknown:
f = NULL;
break;
}
-
+try_fun_setup:
if (f && f->setup)
value = f->setup(f, ctrl);
else {
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 3d87def..51f477a 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -147,6 +147,7 @@ struct usb_os_desc_table {
  * then only altsetting zero is supported.
  * @disable: (REQUIRED) Indicates the function should be disabled.  Reasons
  * include host resetting or reconfiguring the gadget, and disconnection.
+ * @req_match: Tests if a given class request can be handled by this function.
  * @setup: Used for interface-specific control requests.
  * @suspend: Notifies functions when the host stops sending USB traffic.
  * @resume: Notifies functions when the host restarts USB traffic.
@@ -211,6 +212,8 @@ struct usb_function {
int (*get_alt)(struct usb_function *,
unsigned interface);
void(*disable)(struct usb_function *);
+   bool(*req_match)(struct usb_function *,
+   const struct usb_ctrlrequest *);
int (*setup)(struct usb_function *,
const struct usb_ctrlrequest *);
void(*suspend)(struct usb_function *);
-- 
1.9.1

--
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

[PATCH 23/29] usb: gadget: printer: allocate printer_dev instances dynamically

[Andrzej Pietrasiewicz]

With all the obstacles removed it is possible to allow more than one
instance of the printer function. Since the function requires allocating
character device region, a maximum number of allowed instances is defined.
Such an approach is used in f_acm and in f_hid.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 61 -
 1 file changed, 40 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index c693a07..f0baea0 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -51,11 +51,12 @@ USB_GADGET_COMPOSITE_OPTIONS();
 #define GET_PORT_STATUS1
 #define SOFT_RESET 2
 
+#define PRINTER_MINORS 4
+
 static const char shortname [] = "printer";
 static const char driver_desc [] = DRIVER_DESC;
 
-static dev_t g_printer_devno;
-
+static int major, minors;
 static struct class *usb_gadget_class;
 
 /*-*/
@@ -84,6 +85,7 @@ struct printer_dev {
u8  *current_rx_buf;
u8  printer_status;
u8  reset_printer;
+   int minor;
struct cdev printer_cdev;
u8  printer_cdev_open;
wait_queue_head_t   wait;
@@ -97,8 +99,6 @@ static inline struct printer_dev *func_to_printer(struct 
usb_function *f)
return container_of(f, struct printer_dev, function);
 }
 
-static struct printer_dev usb_printer_gadget;
-
 /*-*/
 
 /* DO NOT REUSE THESE IDs with a protocol-incompatible driver!!  Ever!!
@@ -1096,6 +1096,7 @@ static int __init printer_func_bind(struct 
usb_configuration *c,
struct usb_ep *in_ep;
struct usb_ep *out_ep = NULL;
struct usb_request *req;
+   dev_t devt;
int id;
int ret;
u32 i;
@@ -1153,8 +1154,9 @@ autoconf_fail:
}
 
/* Setup the sysfs files for the printer gadget. */
-   pdev = device_create(usb_gadget_class, NULL, g_printer_devno,
- NULL, "g_printer");
+   devt = MKDEV(major, dev->minor);
+   pdev = device_create(usb_gadget_class, NULL, devt,
+ NULL, "g_printer%d", dev->minor);
if (IS_ERR(pdev)) {
ERROR(dev, "Failed to create device: g_printer\n");
ret = PTR_ERR(pdev);
@@ -1167,7 +1169,7 @@ autoconf_fail:
 */
cdev_init(&dev->printer_cdev, &printer_io_operations);
dev->printer_cdev.owner = THIS_MODULE;
-   ret = cdev_add(&dev->printer_cdev, g_printer_devno, 1);
+   ret = cdev_add(&dev->printer_cdev, devt, 1);
if (ret) {
ERROR(dev, "Failed to open char device\n");
goto fail_cdev_add;
@@ -1176,7 +1178,7 @@ autoconf_fail:
return 0;
 
 fail_cdev_add:
-   device_destroy(usb_gadget_class, g_printer_devno);
+   device_destroy(usb_gadget_class, devt);
 
 fail_rx_reqs:
while (!list_empty(&dev->rx_reqs)) {
@@ -1204,7 +1206,7 @@ static void printer_func_unbind(struct usb_configuration 
*c,
 
dev = func_to_printer(f);
 
-   device_destroy(usb_gadget_class, g_printer_devno);
+   device_destroy(usb_gadget_class, MKDEV(major, dev->minor));
 
/* Remove Character Device */
cdev_del(&dev->printer_cdev);
@@ -1238,6 +1240,7 @@ static void printer_func_unbind(struct usb_configuration 
*c,
printer_req_free(dev->out_ep, req);
}
usb_free_all_descriptors(f);
+   kfree(dev);
 }
 
 static int printer_func_set_alt(struct usb_function *f,
@@ -1271,14 +1274,21 @@ static struct usb_configuration printer_cfg_driver = {
 };
 
 static int f_printer_bind_config(struct usb_configuration *c, char *pnp_str,
-char *pnp_string, unsigned q_len)
+char *pnp_string, unsigned q_len, int minor)
 {
struct printer_dev  *dev;
int status = -ENOMEM;
size_t  len;
 
-   dev = &usb_printer_gadget;
+   if (minor >= minors)
+   return -ENOENT;
+
+   dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+   if (!dev)
+   return -ENOMEM;
+
dev->pnp_string = pnp_string;
+   dev->minor = minor;
 
dev->function.name = shortname;
dev->function.bind = printer_func_bind;
@@ -1315,8 +1325,10 @@ static int f_printer_bind_config(struct 
usb_configuration *c, char *pnp_str,
dev->q_len = q_len;
 
status = usb_add_function(c, &dev->function);
-   if (status)
+   if (status) {
+   kfree

[PATCH 25/29] usb: gadget: f_printer: convert to new function interface with backward compatibility

[Andrzej Pietrasiewicz]

In order to add configfs support, a usb function must be converted to use
the new interface. This patch converts the function to the new interface
and provides backward compatiblity layer, which can be removed after
all its users are converted to use the new interface.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/Kconfig  |   3 +
 drivers/usb/gadget/function/Makefile|   2 +
 drivers/usb/gadget/function/f_printer.c | 182 +++-
 drivers/usb/gadget/function/u_printer.h |  33 ++
 drivers/usb/gadget/legacy/printer.c |   1 +
 5 files changed, 220 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/gadget/function/u_printer.h

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index b454d05..9d507cf 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -196,6 +196,9 @@ config USB_F_MIDI
 config USB_F_HID
tristate
 
+config USB_F_PRINTER
+   tristate
+
 choice
tristate "USB Gadget Drivers"
default USB_ETH
diff --git a/drivers/usb/gadget/function/Makefile 
b/drivers/usb/gadget/function/Makefile
index f71b1aa..bd7def5 100644
--- a/drivers/usb/gadget/function/Makefile
+++ b/drivers/usb/gadget/function/Makefile
@@ -42,3 +42,5 @@ usb_f_midi-y  := f_midi.o
 obj-$(CONFIG_USB_F_MIDI)   += usb_f_midi.o
 usb_f_hid-y:= f_hid.o
 obj-$(CONFIG_USB_F_HID)+= usb_f_hid.o
+usb_f_printer-y:= f_printer.o
+obj-$(CONFIG_USB_F_PRINTER)+= usb_f_printer.o
diff --git a/drivers/usb/gadget/function/f_printer.c 
b/drivers/usb/gadget/function/f_printer.c
index 7474eae..f527d834 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -46,6 +47,8 @@
 #include 
 #include 
 
+#include "u_printer.h"
+
 #define PNP_STRING_LEN 1024
 #define PRINTER_MINORS 4
 #define GET_DEVICE_ID  0
@@ -54,6 +57,10 @@
 
 static int major, minors;
 static struct class *usb_gadget_class;
+#ifndef USBF_PRINTER_INCLUDED
+static DEFINE_IDA(printer_ida);
+static DEFINE_MUTEX(printer_ida_lock); /* protects access do printer_ida */
+#endif
 
 /*-*/
 
@@ -999,7 +1006,7 @@ unknown:
return value;
 }
 
-static int __init printer_func_bind(struct usb_configuration *c,
+static int printer_func_bind(struct usb_configuration *c,
struct usb_function *f)
 {
struct usb_gadget *gadget = c->cdev->gadget;
@@ -,6 +1118,7 @@ fail_tx_reqs:
 
 }
 
+#ifdef USBF_PRINTER_INCLUDED
 static void printer_func_unbind(struct usb_configuration *c,
struct usb_function *f)
 {
@@ -1155,6 +1163,7 @@ static void printer_func_unbind(struct usb_configuration 
*c,
usb_free_all_descriptors(f);
kfree(dev);
 }
+#endif
 
 static int printer_func_set_alt(struct usb_function *f,
unsigned intf, unsigned alt)
@@ -1180,6 +1189,7 @@ static void printer_func_disable(struct usb_function *f)
spin_unlock_irqrestore(&dev->lock, flags);
 }
 
+#ifdef USBF_PRINTER_INCLUDED
 static int f_printer_bind_config(struct usb_configuration *c, char *pnp_str,
 char *pnp_string, unsigned q_len, int minor)
 {
@@ -1240,6 +1250,176 @@ static int f_printer_bind_config(struct 
usb_configuration *c, char *pnp_str,
INFO(dev, "%s, version: " DRIVER_VERSION "\n", driver_desc);
return 0;
 }
+#else
+static inline int gprinter_get_minor(void)
+{
+   return ida_simple_get(&printer_ida, 0, 0, GFP_KERNEL);
+}
+
+static inline void gprinter_put_minor(int minor)
+{
+   ida_simple_remove(&printer_ida, minor);
+}
+
+static void gprinter_free_inst(struct usb_function_instance *f)
+{
+   struct f_printer_opts *opts;
+
+   opts = container_of(f, struct f_printer_opts, func_inst);
+
+   mutex_lock(&printer_ida_lock);
+
+   gprinter_put_minor(opts->minor);
+   if (idr_is_empty(&printer_ida.idr))
+   gprinter_cleanup();
+
+   mutex_unlock(&printer_ida_lock);
+
+   kfree(opts);
+}
+
+static struct usb_function_instance *gprinter_alloc_inst(void)
+{
+   struct f_printer_opts *opts;
+   struct usb_function_instance *ret;
+   int status = 0;
+
+   opts = kzalloc(sizeof(*opts), GFP_KERNEL);
+   if (!opts)
+   return ERR_PTR(-ENOMEM);
+
+   opts->func_inst.free_func_inst = gprinter_free_inst;
+   ret = &opts->func_inst;
+
+   mutex_lock(&printer_ida_lock);
+
+   if (idr_is_empty(&printer_ida.idr)) {
+   status = gprinter_setup(PRINTER_MINORS);
+   if (status) {
+   ret = ERR_PTR(status);
+   kfree(opts);
+

[PATCH 14/29] usb: gadget: printer: don't access file global pnp_string in function's code

[Andrzej Pietrasiewicz]

In order to factor out a reusable f_printer, the function's code should
not use file global variables related to legacy printer gadget's
implementation.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index db5e2f0..42c46da 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -86,6 +86,7 @@ struct printer_dev {
u8  printer_cdev_open;
wait_queue_head_t   wait;
unsignedq_len;
+   char*pnp_string;/* We don't own memory! */
struct usb_function function;
 };
 
@@ -994,10 +995,10 @@ static int printer_func_setup(struct usb_function *f,
if ((wIndex>>8) != dev->interface)
break;
 
-   value = (pnp_string[0]<<8)|pnp_string[1];
-   memcpy(req->buf, pnp_string, value);
+   value = (dev->pnp_string[0] << 8) | dev->pnp_string[1];
+   memcpy(req->buf, dev->pnp_string, value);
DBG(dev, "1284 PNP String: %x %s\n", value,
-   &pnp_string[2]);
+   &dev->pnp_string[2]);
break;
 
case 1: /* Get Port Status */
@@ -1230,13 +1231,14 @@ static struct usb_configuration printer_cfg_driver = {
 };
 
 static int f_printer_bind_config(struct usb_configuration *c, char *pnp_str,
-unsigned q_len)
+char *pnp_string, unsigned q_len)
 {
struct printer_dev  *dev;
int status = -ENOMEM;
size_t  len;
 
dev = &usb_printer_gadget;
+   dev->pnp_string = pnp_string;
 
dev->function.name = shortname;
dev->function.bind = printer_func_bind;
@@ -1249,7 +1251,7 @@ static int f_printer_bind_config(struct usb_configuration 
*c, char *pnp_str,
INIT_LIST_HEAD(&dev->rx_buffers);
 
if (pnp_str)
-   strlcpy(&pnp_string[2], pnp_str, PNP_STRING_LEN - 2);
+   strlcpy(&dev->pnp_string[2], pnp_str, PNP_STRING_LEN - 2);
 
len = strlen(pnp_string);
pnp_string[0] = (len >> 8) & 0xFF;
@@ -1292,7 +1294,7 @@ static int __init printer_do_config(struct 
usb_configuration *c)
printer_cfg_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
}
 
-   return f_printer_bind_config(c, iPNPstring, QLEN);
+   return f_printer_bind_config(c, iPNPstring, pnp_string, QLEN);
 
 }
 
-- 
1.9.1

--
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

[PATCH 27/29] usb: gadget: f_printer: remove compatibility layer

[Andrzej Pietrasiewicz]

There are no old interface users left, so it can be removed.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/f_printer.c | 113 
 1 file changed, 113 deletions(-)

diff --git a/drivers/usb/gadget/function/f_printer.c 
b/drivers/usb/gadget/function/f_printer.c
index f527d834..d168511 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -57,10 +57,8 @@
 
 static int major, minors;
 static struct class *usb_gadget_class;
-#ifndef USBF_PRINTER_INCLUDED
 static DEFINE_IDA(printer_ida);
 static DEFINE_MUTEX(printer_ida_lock); /* protects access do printer_ida */
-#endif
 
 /*-*/
 
@@ -1118,53 +1116,6 @@ fail_tx_reqs:
 
 }
 
-#ifdef USBF_PRINTER_INCLUDED
-static void printer_func_unbind(struct usb_configuration *c,
-   struct usb_function *f)
-{
-   struct printer_dev  *dev;
-   struct usb_request  *req;
-
-   dev = func_to_printer(f);
-
-   device_destroy(usb_gadget_class, MKDEV(major, dev->minor));
-
-   /* Remove Character Device */
-   cdev_del(&dev->printer_cdev);
-
-   /* we must already have been disconnected ... no i/o may be active */
-   WARN_ON(!list_empty(&dev->tx_reqs_active));
-   WARN_ON(!list_empty(&dev->rx_reqs_active));
-
-   /* Free all memory for this driver. */
-   while (!list_empty(&dev->tx_reqs)) {
-   req = container_of(dev->tx_reqs.next, struct usb_request,
-   list);
-   list_del(&req->list);
-   printer_req_free(dev->in_ep, req);
-   }
-
-   if (dev->current_rx_req != NULL)
-   printer_req_free(dev->out_ep, dev->current_rx_req);
-
-   while (!list_empty(&dev->rx_reqs)) {
-   req = container_of(dev->rx_reqs.next,
-   struct usb_request, list);
-   list_del(&req->list);
-   printer_req_free(dev->out_ep, req);
-   }
-
-   while (!list_empty(&dev->rx_buffers)) {
-   req = container_of(dev->rx_buffers.next,
-   struct usb_request, list);
-   list_del(&req->list);
-   printer_req_free(dev->out_ep, req);
-   }
-   usb_free_all_descriptors(f);
-   kfree(dev);
-}
-#endif
-
 static int printer_func_set_alt(struct usb_function *f,
unsigned intf, unsigned alt)
 {
@@ -1189,68 +1140,6 @@ static void printer_func_disable(struct usb_function *f)
spin_unlock_irqrestore(&dev->lock, flags);
 }
 
-#ifdef USBF_PRINTER_INCLUDED
-static int f_printer_bind_config(struct usb_configuration *c, char *pnp_str,
-char *pnp_string, unsigned q_len, int minor)
-{
-   struct printer_dev  *dev;
-   int status = -ENOMEM;
-   size_t  len;
-
-   if (minor >= minors)
-   return -ENOENT;
-
-   dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-   if (!dev)
-   return -ENOMEM;
-
-   dev->pnp_string = pnp_string;
-   dev->minor = minor;
-
-   dev->function.name = shortname;
-   dev->function.bind = printer_func_bind;
-   dev->function.setup = printer_func_setup;
-   dev->function.unbind = printer_func_unbind;
-   dev->function.set_alt = printer_func_set_alt;
-   dev->function.disable = printer_func_disable;
-   dev->function.req_match = gprinter_req_match;
-   INIT_LIST_HEAD(&dev->tx_reqs);
-   INIT_LIST_HEAD(&dev->rx_reqs);
-   INIT_LIST_HEAD(&dev->rx_buffers);
-
-   if (pnp_str)
-   strlcpy(&dev->pnp_string[2], pnp_str, PNP_STRING_LEN - 2);
-
-   len = strlen(pnp_string);
-   pnp_string[0] = (len >> 8) & 0xFF;
-   pnp_string[1] = len & 0xFF;
-
-   spin_lock_init(&dev->lock);
-   mutex_init(&dev->lock_printer_io);
-   INIT_LIST_HEAD(&dev->tx_reqs_active);
-   INIT_LIST_HEAD(&dev->rx_reqs_active);
-   init_waitqueue_head(&dev->rx_wait);
-   init_waitqueue_head(&dev->tx_wait);
-   init_waitqueue_head(&dev->tx_flush_wait);
-
-   dev->interface = -1;
-   dev->printer_cdev_open = 0;
-   dev->printer_status = PRINTER_NOT_ERROR;
-   dev->current_rx_req = NULL;
-   dev->current_rx_bytes = 0;
-   dev->current_rx_buf = NULL;
-   dev->q_len = q_len;
-
-   status = usb_add_function(c, &dev->function);
-   if (status) {
-   kfree(dev);
-   return status;
-   }
-
-   INFO(dev, "%s, version: " DRIVER_VERSION "\n", driver_desc);
-   return 0;
-}
-#else
 static inline int gprinter_get_minor(void)
 {
   

[PATCH 11/29] usb: gadget: printer: call usb_add_function() last

[Andrzej Pietrasiewicz]

Conversion to the new function interface requires splitting a
_bind_config() function into two parts: allocation of
container_of struct usb_function and invocation of usb_add_function().
This patch moves the latter to the end of the f_printer_bind_config()
in order to enable conversion to the new interface.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index c857044..5dbb93a 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1254,11 +1254,6 @@ static int f_printer_bind_config(struct 
usb_configuration *c, char *pnp_str,
INIT_LIST_HEAD(&dev->rx_reqs);
INIT_LIST_HEAD(&dev->rx_buffers);
 
-   dev->q_len = q_len;
-   status = usb_add_function(c, &dev->function);
-   if (status)
-   return status;
-
if (pnp_str)
strlcpy(&pnp_string[2], pnp_str, sizeof(pnp_string) - 2);
 
@@ -1280,7 +1275,11 @@ static int f_printer_bind_config(struct 
usb_configuration *c, char *pnp_str,
dev->current_rx_req = NULL;
dev->current_rx_bytes = 0;
dev->current_rx_buf = NULL;
+   dev->q_len = q_len;
 
+   status = usb_add_function(c, &dev->function);
+   if (status)
+   return status;
INFO(dev, "%s, version: " DRIVER_VERSION "\n", driver_desc);
return 0;
 }
-- 
1.9.1

--
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

[PATCH 01/29] usb: gadget: composite: don't try standard handling for non-standard requests

[Andrzej Pietrasiewicz]

If a non-standard request is processed and its parameters just happen
to match those of some standard request, the logic of composite_setup()
can be fooled, so don't even try any switch cases, just go to the
proper place where unknown requests are handled.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/composite.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 13adfd1..9fb9231 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1472,6 +1472,13 @@ composite_setup(struct usb_gadget *gadget, const struct 
usb_ctrlrequest *ctrl)
req->length = 0;
gadget->ep0->driver_data = cdev;
 
+   /*
+* Don't let non-standard requests match any of the cases below
+* by accident.
+*/
+   if ((ctrl->bRequestType & USB_TYPE_MASK) != USB_TYPE_STANDARD)
+   goto unknown;
+
switch (ctrl->bRequest) {
 
/* we handle all standard USB descriptors */
-- 
1.9.1

--
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

[PATCH 24/29] usb: gadget: printer: factor out f_printer

[Andrzej Pietrasiewicz]

The legacy printer gadget now contains both a reusable printer function
and legacy gadget proper implementations interwoven, but logically
separate. This patch factors out a reusable f_printer.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/f_printer.c | 1280 +++
 drivers/usb/gadget/legacy/printer.c | 1256 +-
 2 files changed, 1286 insertions(+), 1250 deletions(-)
 create mode 100644 drivers/usb/gadget/function/f_printer.c

diff --git a/drivers/usb/gadget/function/f_printer.c 
b/drivers/usb/gadget/function/f_printer.c
new file mode 100644
index 000..7474eae
--- /dev/null
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -0,0 +1,1280 @@
+/*
+ * f_printer.c - USB printer function driver
+ *
+ * Copied from drivers/usb/gadget/legacy/printer.c,
+ * which was:
+ *
+ * printer.c -- Printer gadget driver
+ *
+ * Copyright (C) 2003-2005 David Brownell
+ * Copyright (C) 2006 Craig W. Nadler
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#define PNP_STRING_LEN 1024
+#define PRINTER_MINORS 4
+#define GET_DEVICE_ID  0
+#define GET_PORT_STATUS1
+#define SOFT_RESET 2
+
+static int major, minors;
+static struct class *usb_gadget_class;
+
+/*-*/
+
+struct printer_dev {
+   spinlock_t  lock;   /* lock this structure */
+   /* lock buffer lists during read/write calls */
+   struct mutexlock_printer_io;
+   struct usb_gadget   *gadget;
+   s8  interface;
+   struct usb_ep   *in_ep, *out_ep;
+
+   struct list_headrx_reqs;/* List of free RX structs */
+   struct list_headrx_reqs_active; /* List of Active RX xfers */
+   struct list_headrx_buffers; /* List of completed xfers */
+   /* wait until there is data to be read. */
+   wait_queue_head_t   rx_wait;
+   struct list_headtx_reqs;/* List of free TX structs */
+   struct list_headtx_reqs_active; /* List of Active TX xfers */
+   /* Wait until there are write buffers available to use. */
+   wait_queue_head_t   tx_wait;
+   /* Wait until all write buffers have been sent. */
+   wait_queue_head_t   tx_flush_wait;
+   struct usb_request  *current_rx_req;
+   size_t  current_rx_bytes;
+   u8  *current_rx_buf;
+   u8  printer_status;
+   u8  reset_printer;
+   int minor;
+   struct cdev printer_cdev;
+   u8  printer_cdev_open;
+   wait_queue_head_t   wait;
+   unsignedq_len;
+   char*pnp_string;/* We don't own memory! */
+   struct usb_function function;
+};
+
+static inline struct printer_dev *func_to_printer(struct usb_function *f)
+{
+   return container_of(f, struct printer_dev, function);
+}
+
+/*-*/
+
+/*
+ * DESCRIPTORS ... most are static, but strings and (full) configuration
+ * descriptors are built on demand.
+ */
+
+/* holds our biggest descriptor */
+#define USB_DESC_BUFSIZE   256
+#define USB_BUFSIZE8192
+
+static struct usb_interface_descriptor intf_desc = {
+   .bLength =  sizeof(intf_desc),
+   .bDescriptorType =  USB_DT_INTERFACE,
+   .bNumEndpoints =2,
+   .bInterfaceClass =  USB_CLASS_PRINTER,
+   .bInterfaceSubClass =   1,  /* Printer Sub-Class */
+   .bInterfaceProtocol =   2,  /* Bi-Directional */
+   .iInterface =   0
+};
+
+static struct usb_endpoint_descriptor fs_ep_in_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+   .bEndpointAddress = USB_DIR_IN,
+   .bmAttributes = USB_ENDPOINT_XFER_BULK
+};
+
+static struct usb_endpoint_descriptor fs_ep_out_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+   .bEndpointAddress = USB_DIR_OUT,
+   .bmAttributes = USB_ENDPOINT_XFER_BULK
+};
+
+static struct usb_descriptor_header *fs_printer_fun

[PATCH 17/29] usb: gadget: printer: eliminate file global printer_mutex

[Andrzej Pietrasiewicz]

The mutex is a legacy after semi-automatic Big Kernel Lock removal.
printer_open() does its own locking, so no need to duplicate it.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index d5fb41b..9691e57 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -48,7 +48,6 @@ USB_GADGET_COMPOSITE_OPTIONS();
 #define DRIVER_DESC"Printer Gadget"
 #define DRIVER_VERSION "2007 OCT 06"
 
-static DEFINE_MUTEX(printer_mutex);
 static const char shortname [] = "printer";
 static const char driver_desc [] = DRIVER_DESC;
 
@@ -420,7 +419,6 @@ printer_open(struct inode *inode, struct file *fd)
unsigned long   flags;
int ret = -EBUSY;
 
-   mutex_lock(&printer_mutex);
dev = container_of(inode->i_cdev, struct printer_dev, printer_cdev);
 
spin_lock_irqsave(&dev->lock, flags);
@@ -436,7 +434,6 @@ printer_open(struct inode *inode, struct file *fd)
spin_unlock_irqrestore(&dev->lock, flags);
 
DBG(dev, "printer_open returned %x\n", ret);
-   mutex_unlock(&printer_mutex);
return ret;
 }
 
-- 
1.9.1

--
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

[PATCH 22/29] usb: gadget: printer: add req_match for printer function

[Andrzej Pietrasiewicz]

Verify that a given usb_ctrlrequest is meant for printer function.
The following parts of the request are tested:

- bmRequestType:Data transfer direction
- bmRequestType:Type
- bmRequestType:Recipient
- bRequest
- wValue for bRequest 1 and 2
- wLength

Additionally, the request is considered meant for this function
iff the decoded interface number matches dev->interface.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index fcfa618..c693a07 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -974,6 +974,41 @@ static void printer_soft_reset(struct printer_dev *dev)
 
 /*-*/
 
+static bool gprinter_req_match(struct usb_function *f,
+  const struct usb_ctrlrequest *ctrl)
+{
+   struct printer_dev  *dev = func_to_printer(f);
+   u16 w_index = le16_to_cpu(ctrl->wIndex);
+   u16 w_value = le16_to_cpu(ctrl->wValue);
+   u16 w_length = le16_to_cpu(ctrl->wLength);
+
+   if ((ctrl->bRequestType & USB_RECIP_MASK) != USB_RECIP_INTERFACE ||
+   (ctrl->bRequestType & USB_TYPE_MASK) != USB_TYPE_CLASS)
+   return false;
+
+   switch (ctrl->bRequest) {
+   case GET_DEVICE_ID:
+   w_index >>= 8;
+   if (w_length <= PNP_STRING_LEN &&
+   (USB_DIR_IN & ctrl->bRequestType))
+   break;
+   return false;
+   case GET_PORT_STATUS:
+   if (!w_value && w_length == 1 &&
+   (USB_DIR_IN & ctrl->bRequestType))
+   break;
+   return false;
+   case SOFT_RESET:
+   if (!w_value && !w_length &&
+  (USB_DIR_OUT & ctrl->bRequestType))
+   break;
+   /* fall through */
+   default:
+   return false;
+   }
+   return w_index == dev->interface;
+}
+
 /*
  * The setup() callback implements all the ep0 functionality that's not
  * handled lower down.
@@ -1251,6 +1286,7 @@ static int f_printer_bind_config(struct usb_configuration 
*c, char *pnp_str,
dev->function.unbind = printer_func_unbind;
dev->function.set_alt = printer_func_set_alt;
dev->function.disable = printer_func_disable;
+   dev->function.req_match = gprinter_req_match;
INIT_LIST_HEAD(&dev->tx_reqs);
INIT_LIST_HEAD(&dev->rx_reqs);
INIT_LIST_HEAD(&dev->rx_buffers);
-- 
1.9.1

--
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

[PATCH 07/29] usb: gadget: printer: eliminate pdev member of struct printer_dev

[Andrzej Pietrasiewicz]

The pdev member of struct printer_dev is not used outside
printer_bind_config(), so it can just as well be a local variable there.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/legacy/printer.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c 
b/drivers/usb/gadget/legacy/printer.c
index bbcd6aa..a9c3e57 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -83,7 +83,6 @@ struct printer_dev {
u8  printer_status;
u8  reset_printer;
struct cdev printer_cdev;
-   struct device   *pdev;
u8  printer_cdev_open;
wait_queue_head_t   wait;
struct usb_function function;
@@ -1175,6 +1174,7 @@ static int __init printer_bind_config(struct 
usb_configuration *c)
 {
struct usb_gadget   *gadget = c->cdev->gadget;
struct printer_dev  *dev;
+   struct device   *pdev;
int status = -ENOMEM;
size_t  len;
u32 i;
@@ -1199,11 +1199,11 @@ static int __init printer_bind_config(struct 
usb_configuration *c)
return status;
 
/* Setup the sysfs files for the printer gadget. */
-   dev->pdev = device_create(usb_gadget_class, NULL, g_printer_devno,
+   pdev = device_create(usb_gadget_class, NULL, g_printer_devno,
  NULL, "g_printer");
-   if (IS_ERR(dev->pdev)) {
+   if (IS_ERR(pdev)) {
ERROR(dev, "Failed to create device: g_printer\n");
-   status = PTR_ERR(dev->pdev);
+   status = PTR_ERR(pdev);
goto fail;
}
 
-- 
1.9.1

--
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

[PATCH 00/29] Equivalent of g_printer with configfs

[Andrzej Pietrasiewicz]

This series aims at integrating configfs into hid, the way
it has been done for acm, ncm, ecm, eem, ecm subset, rndis, obex, phonet,
mass_storage, FunctionFS, loopback, sourcesink, uac1, uac2, uvc, midi
and hid.

The printer gadget before the changes has some bugs, and although it
uses the composite framework, it is not ready for splitting into
a reusable f_printer.c and a legacy printer.c. Most of the patches
in the series are preparation for factoring out f_printer.c. Once
f_printer.c is extracted from printer.c, it can be converted to
use the new function interface, and then the usual procedure of
adding configfs support follows.

Rebased onto Felipe's master.

BACKWARD COMPATIBILITY
==

Please note that the old g_printer.ko is still available and works.
There is just one minor difference, the dev node at the gadget's side
is called /dev/g_printer instead of /dev/g_printer. This makes
sense, because there are more than one instances of this function allowed
after the series is applied. If /dev/g_printer is needed, it can be
easily added as a symlink with udev rules.

USING THE NEW "GADGET"
==

Please refer to this post:

http://www.spinics.net/lists/linux-usb/msg76388.html

for general information from Sebastian on how to use configfs-based
gadgets (*).

With configfs the procedure is as follows, compared to the information
mentioned above (*):

instead of mkdir functions/acm.ttyS1 do

mkdir functions/printer.

e.g. mkdir functions/printer.usb0.

In the printer.usb0 directory there will be the following attributes:


pnp_string  - Data to be passed to the host in pnp string
q_len   - Number of requests per endpoint.

Below is a script which creates a printer gadget on a board with dwc2:

$ modprobe libcomposite
$ mount none cfg -t configfs
$ mkdir cfg/usb_gadget/g1
$ cd cfg/usb_gadget/g1
$ mkdir configs/c.1
$ mkdir functions/printer.usb0
$ mkdir strings/0x409
$ mkdir configs/c.1/strings/0x409
$ echo 0x0525 > idVendor
$ echo 0xa4a8 > idProduct
$ echo 1 > strings/0x409/serialnumber
$ echo Samsung > strings/0x409/manufacturer
$ echo Printer Gadget > strings/0x409/product
$ echo 10 > functions/printer.usb0/q_len
$ echo "MFG:linux;MDL:g_printer;CLS:PRINTER;SN:1;" > 
functions/printer.usb0/pnp_string
$ echo "Conf 1" > configs/c.1/strings/0x409/configuration
$ echo 120 > configs/c.1/MaxPower
$ ln -s functions/printer.usb0 configs/c.1
$ echo 1248.hsotg > UDC

TESTING THE FUNCTION


The most basic testing:

device: run the gadget,

ls -l /devices/virtual/usb_printer_gadget/

should show g_printer.

If udev is active, then /dev/g_printer should appear automatically.

host:

If udev is active, then e.g. /dev/usb/lp0 should appear.

host->device transmission:

device:
# cat /dev/g_printer
host:
# cat > /dev/usb/lp0

device->host transmission:

# cat > /dev/g_printer
host:
# cat /dev/usb/lp0

More advanced testing can be done with the prn_example
described in Documentation/usb/gadget-printer.txt.

Andrzej Pietrasiewicz (29):
  usb: gadget: composite: don't try standard handling for non-standard
requests
  usb: gadget: printer: enqueue printer's response for setup request
  usb: gadget: printer: remove unused and empty printer_unbind
  usb: gadget: printer: eliminate random pointer dereference
  usb: gadget: printer: revert usb_add_function() effect in error
recovery
  usb: gadget: printer: add missing error handling
  usb: gadget: printer: eliminate pdev member of struct printer_dev
  usb: gadget: printer: follow the naming convention for usb_add_config
callback
  usb: gadget: printer: standardize printer_do_config
  usb: gadget: printer: move function-related bind code to function's
bind
  usb: gadget: printer: call usb_add_function() last
  usb: gadget: printer: move function-related unbind code to function's
unbind
  usb: gadget: printer: define pnp string buffer length
  usb: gadget: printer: don't access file global pnp_string in
function's code
  usb: gadget: printer: add setup and cleanup functions
  usb: gadget: printer: call gprinter_setup() from gadget's bind
  usb: gadget: printer: eliminate file global printer_mutex
  usb: gadget: printer: don't access file global usb_printer_gadget in
function's code
  usb: gadget: printer: add container_of helper for printer_dev
  usb: gadget: composite: add req_match method to usb_function
  usb: gadget: printer: name class specific requests
  usb: gadget: printer: add req_match for printer function
  usb: gadget: printer: allocate printer_dev instances dynamically
  usb: gadget: printer: factor out f_printer
  usb: gadget: f_printer: convert to new function interface with
backward compatibility
  usb: gadget: printer: convert to new interface of f_printer
  usb: gadget: f_printer: remove compatibility layer
  us

Re: [PATCH 1/2] usb: gadget: udc-core: independent registration of gadgets and gadget drivers

[Andrzej Pietrasiewicz]

W dniu 17.02.2015 o 22:02, Ruslan Bilovol pisze:

Hi Andrzej,

On Mon, Feb 16, 2015 at 10:07 AM, Andrzej Pietrasiewicz
 wrote:

W dniu 15.02.2015 o 23:43, Ruslan Bilovol pisze:





In my opinion all things which you have described are working out-of-box
when you use configfs interface. It's mostly ready so you may create
equivalent of most legacy gadgets (apart from printer and tcm) and
just bind from one udc to another whenever you want.



It's because legacy gadgets are easy to use and usually don't need any
userspace-side configuration. Are there any plans to remove legacy
drivers in the future?



I'm not going to express strong opinions here, but their name implies
that this can happen, some time in the future.

And I also think it will not happen before the userspace part
(libusbg, gt, gadgetd etc) is mature enough. My personal opinion
in that matter is that it will take at least a couple of years
to remove legacy gadgets entirely.


OK, so it looks like there is a sense even to add new gadget/functions
with legacy support



I'm not sure what you mean exactly.

For sure legacy gadgets are supported as long as they are
a part of the mainline kernel. So any changes you make
to the kernel must not affect the legacy gadgets, or you
need to modify the legacy gadgets too and have them working.

But adding new legacy-style gadgets is a completely different
story. IMHO you need a _very_ good reason to succeed,
but I remember Felipe expressing an opinion that chances
or merging another legacy gadget were zero.

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

Re: [PATCH 1/2] usb: gadget: udc-core: independent registration of gadgets and gadget drivers

[Andrzej Pietrasiewicz]

W dniu 15.02.2015 o 23:43, Ruslan Bilovol pisze:





In my opinion all things which you have described are working out-of-box
when you use configfs interface. It's mostly ready so you may create
equivalent of most legacy gadgets (apart from printer and tcm) and
just bind from one udc to another whenever you want.


It's because legacy gadgets are easy to use and usually don't need any
userspace-side configuration. Are there any plans to remove legacy
drivers in the future?



I'm not going to express strong opinions here, but their name implies
that this can happen, some time in the future.

And I also think it will not happen before the userspace part
(libusbg, gt, gadgetd etc) is mature enough. My personal opinion
in that matter is that it will take at least a couple of years
to remove legacy gadgets entirely.

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

[PATCH] usb: gadget: configfs: don't NUL-terminate (sub)compatible ids

[Andrzej Pietrasiewicz]

The "Extended Compat ID OS Feature Descriptor Specification" does not
require the (sub)compatible ids to be NUL-terminated, because they
are placed in a fixed-size buffer and only unused parts of it should
contain NULs. If the buffer is fully utilized, there is no place for NULs.

Consequently, the code which uses desc->ext_compat_id never expects the
data contained to be NUL terminated.

If the compatible id is stored after sub-compatible id, and the compatible
id is full length (8 bytes), the (useless) NUL terminator overwrites the
first byte of the sub-compatible id.

If the sub-compatible id is full length (8 bytes), the (useless) NUL
terminator ends up out of the buffer. The situation can happen in the RNDIS
function, where the buffer is a part of struct f_rndis_opts. The next
member of struct f_rndis_opts is a mutex, so its first byte gets
overwritten. The said byte is a part of a mutex'es member which contains
the information on whether the muext is locked or not. This can lead to a
deadlock, because, in a configfs-composed gadget when a function is linked
into a configuration with config_usb_cfg_link(), usb_get_function()
is called, which then calls rndis_alloc(), which tries locking the same
mutex and (wrongly) finds it already locked.

This patch eliminates NUL terminating of the (sub)compatible id.

Cc:  # v3.16+
Fixes: da4243145fb1: "usb: gadget: configfs: OS Extended Compatibility 
descriptors support"
Reported-by: Dan Carpenter 
Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/configfs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 7564814..c42765b3a0 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1161,7 +1161,6 @@ static ssize_t interf_grp_compatible_id_store(struct 
usb_os_desc *desc,
if (desc->opts_mutex)
mutex_lock(desc->opts_mutex);
memcpy(desc->ext_compat_id, page, l);
-   desc->ext_compat_id[l] = '\0';
 
if (desc->opts_mutex)
mutex_unlock(desc->opts_mutex);
@@ -1192,7 +1191,6 @@ static ssize_t interf_grp_sub_compatible_id_store(struct 
usb_os_desc *desc,
if (desc->opts_mutex)
mutex_lock(desc->opts_mutex);
memcpy(desc->ext_compat_id + 8, page, l);
-   desc->ext_compat_id[l + 8] = '\0';
 
if (desc->opts_mutex)
mutex_unlock(desc->opts_mutex);
-- 
1.9.1

--
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

Re: usb: gadget: configfs: OS Extended Compatibility descriptors support

[Andrzej Pietrasiewicz]

W dniu 13.02.2015 o 09:06, Dan Carpenter pisze:

Hello Andrzej Pietrasiewicz,



Hello Dan,

Thank you for finding the problem.


The patch da4243145fb1: "usb: gadget: configfs: OS Extended
Compatibility descriptors support" from May 8, 2014, leads to the
following Smatch warning:

drivers/usb/gadget/configfs.c:1195 interf_grp_sub_compatible_id_store()
error: buffer overflow 'desc->ext_compat_id' 16 <= 16







Then we are putting the NULL terminator one space beyond the end of the
array.  ->ext_compat_id is set in rndis_alloc_inst().

This is not a false postive, but I'm not positive how we should fix it.



I know how to fix it and will do it soon.

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

[PATCH 2/3] usb: gadget: rndis: style correction

[Andrzej Pietrasiewicz]

Don't use a space between function name and parameter list opening bracket.
All other functions in this file comply wich checkpatch rules.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/rndis.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/rndis.h 
b/drivers/usb/gadget/function/rndis.h
index 338deb7..1cddd99 100644
--- a/drivers/usb/gadget/function/rndis.h
+++ b/drivers/usb/gadget/function/rndis.h
@@ -206,7 +206,7 @@ int  rndis_set_param_vendor(struct rndis_params *params, 
u32 vendorID,
const char *vendorDescr);
 int  rndis_set_param_medium(struct rndis_params *params, u32 medium,
 u32 speed);
-void rndis_add_hdr (struct sk_buff *skb);
+void rndis_add_hdr(struct sk_buff *skb);
 int rndis_rm_hdr(struct gether *port, struct sk_buff *skb,
struct sk_buff_head *list);
 u8   *rndis_get_next_response(struct rndis_params *params, u32 *length);
-- 
1.9.1

--
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

[PATCH 3/3] usb: gadget: rndis: remove the limit of available rndis connections

[Andrzej Pietrasiewicz]

RNDIS function has a limitation on the number of allowed instances.
So far it has been RNDIS_MAX_CONFIGS, which happens to be one.
In order to eliminate this kind of arbitrary limitation we should not
preallocate a predefined (RNDIS_MAX_CONFIGS) array of struct rndis_params
instances but instead allow allocating them on demand.

This patch allocates struct rndis_params on demand in rndis_register().
Coversly, the structure is free()'d in rndis_deregister().
If CONFIG_USB_GADGET_DEBUG_FILES is set, the proc files are created which
is the same behaviour as before, but the moment of creation is delayed
until struct rndis_params is actually allocated.

rnids_init() and rndis_exit() have nothing to do, so they are eliminated.

Signed-off-by: Andrzej Pietrasiewicz 
---
 Documentation/usb/gadget-testing.txt  |   2 -
 drivers/usb/gadget/function/f_rndis.c |  22 +-
 drivers/usb/gadget/function/rndis.c   | 140 +++---
 drivers/usb/gadget/function/u_rndis.h |   2 -
 4 files changed, 78 insertions(+), 88 deletions(-)

diff --git a/Documentation/usb/gadget-testing.txt 
b/Documentation/usb/gadget-testing.txt
index 076ac7b..339b612 100644
--- a/Documentation/usb/gadget-testing.txt
+++ b/Documentation/usb/gadget-testing.txt
@@ -525,8 +525,6 @@ Except for ifname they can be written to until the function 
is linked to a
 configuration. The ifname is read-only and contains the name of the interface
 which was assigned by the net core, e. g. usb0.
 
-By default there can be only 1 RNDIS interface in the system.
-
 Testing the RNDIS function
 --
 
diff --git a/drivers/usb/gadget/function/f_rndis.c 
b/drivers/usb/gadget/function/f_rndis.c
index 2dafe72..32985da 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -1012,26 +1012,6 @@ static struct usb_function *rndis_alloc(struct 
usb_function_instance *fi)
return &rndis->port.func;
 }
 
-DECLARE_USB_FUNCTION(rndis, rndis_alloc_inst, rndis_alloc);
-
-static int __init rndis_mod_init(void)
-{
-   int ret;
-
-   ret = rndis_init();
-   if (ret)
-   return ret;
-
-   return usb_function_register(&rndisusb_func);
-}
-module_init(rndis_mod_init);
-
-static void __exit rndis_mod_exit(void)
-{
-   usb_function_unregister(&rndisusb_func);
-   rndis_exit();
-}
-module_exit(rndis_mod_exit);
-
+DECLARE_USB_FUNCTION_INIT(rndis, rndis_alloc_inst, rndis_alloc);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("David Brownell");
diff --git a/drivers/usb/gadget/function/rndis.c 
b/drivers/usb/gadget/function/rndis.c
index 01a3b58..dd68000 100644
--- a/drivers/usb/gadget/function/rndis.c
+++ b/drivers/usb/gadget/function/rndis.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -57,10 +58,13 @@ MODULE_PARM_DESC (rndis_debug, "enable debugging");
 #define rndis_debug0
 #endif
 
-#define RNDIS_MAX_CONFIGS  1
+#ifdef CONFIG_USB_GADGET_DEBUG_FILES
 
+#defineNAME_TEMPLATE "driver/rndis-%03d"
 
-static rndis_params rndis_per_dev_params[RNDIS_MAX_CONFIGS];
+#endif /* CONFIG_USB_GADGET_DEBUG_FILES */
+
+static DEFINE_IDA(rndis_ida);
 
 /* Driver Version */
 static const __le32 rndis_driver_version = cpu_to_le32(1);
@@ -69,6 +73,11 @@ static const __le32 rndis_driver_version = cpu_to_le32(1);
 static rndis_resp_t *rndis_add_response(struct rndis_params *params,
u32 length);
 
+#ifdef CONFIG_USB_GADGET_DEBUG_FILES
+
+static const struct file_operations rndis_proc_fops;
+
+#endif /* CONFIG_USB_GADGET_DEBUG_FILES */
 
 /* supported OIDs */
 static const u32 oid_supported_list[] =
@@ -850,38 +859,93 @@ int rndis_msg_parser(struct rndis_params *params, u8 *buf)
 }
 EXPORT_SYMBOL_GPL(rndis_msg_parser);
 
+static inline int rndis_get_nr(void)
+{
+   return ida_simple_get(&rndis_ida, 0, 0, GFP_KERNEL);
+}
+
+static inline void rndis_put_nr(int nr)
+{
+   ida_simple_remove(&rndis_ida, nr);
+}
+
 struct rndis_params *rndis_register(void (*resp_avail)(void *v), void *v)
 {
+   struct rndis_params *params;
u8 i;
 
if (!resp_avail)
return ERR_PTR(-EINVAL);
 
-   for (i = 0; i < RNDIS_MAX_CONFIGS; i++) {
-   if (!rndis_per_dev_params[i].used) {
-   rndis_per_dev_params[i].used = 1;
-   rndis_per_dev_params[i].resp_avail = resp_avail;
-   rndis_per_dev_params[i].v = v;
-   pr_debug("%s: configNr = %d\n", __func__, i);
-   return &rndis_per_dev_params[i];
+   i = rndis_get_nr();
+   if (i < 0) {
+   pr_debug("failed\n");
+
+   return ERR_PTR(-ENODEV);
+   }
+
+   params = kzalloc(sizeof(*params), GFP_KERNEL);
+   if (!params) {
+   rndis_put_n

[PATCH 0/3] Eliminate limitation on the number of RNDIS instances

[Andrzej Pietrasiewicz]

This is meant for 3.21.

RNDIS function has a limitation on the number of allowed instances.
So far it has been RNDIS_MAX_CONFIGS, which happens to be one.
In order to eliminate this kind of arbitrary limitation we should not
preallocate a predefined (RNDIS_MAX_CONFIGS) array of struct rndis_params
instances but instead allow allocating them on demand.

This short series eliminates the limit.

The first patch prepares the conversion proper by changing the way
each struct rndis_params instance is referenced throughout the API:
instead of an index into some array a pointer to actual instance
is passed around.

The second patch is a small style correction and the third
does the conversion proper by changing the way instances of
struct rndis_params are allocated: instead of a predefined array
they are now allocated on demand.

Rebased onto Felipe's testing/next.

Andrzej Pietrasiewicz (3):
  usb: gadget: rndis: use rndis_params instead of configNr
  usb: gadget: rndis: style correction
  usb: gadget: rndis: remove the limit of available rndis connections

 Documentation/usb/gadget-testing.txt  |   2 -
 drivers/usb/gadget/function/f_rndis.c |  60 ++
 drivers/usb/gadget/function/rndis.c   | 347 +-
 drivers/usb/gadget/function/rndis.h   |  31 +--
 drivers/usb/gadget/function/u_rndis.h |   2 -
 5 files changed, 205 insertions(+), 237 deletions(-)

-- 
1.9.1

--
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

[PATCH 1/3] usb: gadget: rndis: use rndis_params instead of configNr

[Andrzej Pietrasiewicz]

RNDIS function has a limitation on the number of allowed instances.
So far it has been RNDIS_MAX_CONFIGS, which happens to be one.
In order to eliminate this kind of arbitrary limitation we should not
preallocate a predefined (RNDIS_MAX_CONFIGS) array of struct rndis_params
instances but instead allow allocating them on demand.

This patch prepares the elimination of the said limit by converting all the
functions which accept rndis config number to accept a pointer to the
actual struct rndis_params. Consequently, rndis_register() returns
a pointer to a corresponding struct rndis_params instance. The pointer
is then always used by f_rndis.c instead of config number when it talks
to rndis.c API.

A nice side-effect of the changes is that many lines of code in rndis.c
become shorter and fit in 80 columns.

If a function prototype changes in rndis.h a style cleanup is made
at the same time, otherwise checkpatch complains that the patch
has style problems.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/f_rndis.c |  38 +++---
 drivers/usb/gadget/function/rndis.c   | 213 +++---
 drivers/usb/gadget/function/rndis.h   |  29 ++---
 3 files changed, 129 insertions(+), 151 deletions(-)

diff --git a/drivers/usb/gadget/function/f_rndis.c 
b/drivers/usb/gadget/function/f_rndis.c
index 829edf8..2dafe72 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -76,7 +76,7 @@ struct f_rndis {
u8  ethaddr[ETH_ALEN];
u32 vendorID;
const char  *manufacturer;
-   int config;
+   struct rndis_params *params;
 
struct usb_ep   *notify;
struct usb_request  *notify_req;
@@ -453,7 +453,7 @@ static void rndis_command_complete(struct usb_ep *ep, 
struct usb_request *req)
 
/* received RNDIS command from USB_CDC_SEND_ENCAPSULATED_COMMAND */
 // spin_lock(&dev->lock);
-   status = rndis_msg_parser(rndis->config, (u8 *) req->buf);
+   status = rndis_msg_parser(rndis->params, (u8 *) req->buf);
if (status < 0)
pr_err("RNDIS command error %d, %d/%d\n",
status, req->actual, req->length);
@@ -499,12 +499,12 @@ rndis_setup(struct usb_function *f, const struct 
usb_ctrlrequest *ctrl)
u32 n;
 
/* return the result */
-   buf = rndis_get_next_response(rndis->config, &n);
+   buf = rndis_get_next_response(rndis->params, &n);
if (buf) {
memcpy(req->buf, buf, n);
req->complete = rndis_response_complete;
req->context = rndis;
-   rndis_free_response(rndis->config, buf);
+   rndis_free_response(rndis->params, buf);
value = n;
}
/* else stalls ... spec says to avoid that */
@@ -597,7 +597,7 @@ static int rndis_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
if (IS_ERR(net))
return PTR_ERR(net);
 
-   rndis_set_param_dev(rndis->config, net,
+   rndis_set_param_dev(rndis->params, net,
&rndis->port.cdc_filter);
} else
goto fail;
@@ -617,7 +617,7 @@ static void rndis_disable(struct usb_function *f)
 
DBG(cdev, "rndis deactivated\n");
 
-   rndis_uninit(rndis->config);
+   rndis_uninit(rndis->params);
gether_disconnect(&rndis->port);
 
usb_ep_disable(rndis->notify);
@@ -640,9 +640,9 @@ static void rndis_open(struct gether *geth)
 
DBG(cdev, "%s\n", __func__);
 
-   rndis_set_param_medium(rndis->config, RNDIS_MEDIUM_802_3,
+   rndis_set_param_medium(rndis->params, RNDIS_MEDIUM_802_3,
bitrate(cdev->gadget) / 100);
-   rndis_signal_connect(rndis->config);
+   rndis_signal_connect(rndis->params);
 }
 
 static void rndis_close(struct gether *geth)
@@ -651,8 +651,8 @@ static void rndis_close(struct gether *geth)
 
DBG(geth->func.config->cdev, "%s\n", __func__);
 
-   rndis_set_param_medium(rndis->config, RNDIS_MEDIUM_802_3, 0);
-   rndis_signal_disconnect(rndis->config);
+   rndis_set_param_medium(rndis->params, RNDIS_MEDIUM_802_3, 0);
+   rndis_signal_disconnect(rndis->params);
 }
 
 /*-*/
@@ -796,11 +796,11 @@ rndis_bind(struct usb_configuration *c, struct 
usb_function *f)
rndis->port.open = rnd

Re: f_hid, f_mass_storage, and f_rdnis via configfs on platform/intel-mid

[Andrzej Pietrasiewicz]

W dniu 04.02.2015 o 15:33, Chris McClimans pisze:

Hi Andrzej,

Thank you for your time.

I'd like to take a stab at FunctionFS before I start looking at
backporting hid and uvc from 3.19+

I found a lot of information (mainly from you [1] [2]) on gadget
ConfigFS and hid, but I'm
having less luck finding information on FunctionFS and the required daemons.

On Thu, Jan 22, 2015 at 7:33 AM, Andrzej Pietrasiewicz
 wrote:


These days instead of gadgetfs one should probably use FunctionFS.
The purpose of the two is delegating actual usb function implementation
to userspace with some filesystem being the interface to the kernel.


It looks like I should be able to combine Ethernet, Mass Storage, and
HID this way.
(and it's supported all the way back to 3.10)
However I haven't found any ffs-daemon examples, including the
hid-daemon mentioned
in the kernel source.



Yeah, those are hypothetical userspace usb function drivers...
...some of which you are generously going to contribute to the open source
community ;-)

What I am aware of is a userspace program in Documentation/usb/gadget_hid.txt,
but it only uses hid's /dev node. Using hid's /dev node has nothing
to do with the way your gadget is composed. No matter how you compose
your gadget containing hid, you end up providing a /dev node and this
interface has nothing to do with FunctionFS.

By "ffs-daemon" I mean a userspace program which implements a USB function.

Example "ffs-daemons":

- tools/usb/ffs-test.c
- tools/usb/ffs-aio-example (using async io)
- https://android-review.googlesource.com/#/c/31640/ (my proof-of-concept
for converting adbd to using FunctionFS; actually as far as I know
it ended up with some changes in Android code base:
https://android.googlesource.com/platform/system/core/+/fd96db17b7f07eb6615af01fd1908b74383bf04b)
- http://www.spinics.net/lists/linux-usb/msg41962.html (PTP)
- http://www.spinics.net/lists/linux-usb/msg87093.html (another PTP)


Also I'm not quite sure how the loading g_ffs enables Ethernet in the docs.
Is g_ffs loaded after a g_multi that included Ethernet?


Not at all. You cannot have two gadgets active at the same time
unless you have at least two UDC/OTG (USB device controller) chips
in your Linux box acting as your USB gadget. And if you do have two
UDC chips then almost for sure each handles a separate USB cable
which is probably not what you want. I think you want a single
gadget which provides Ethernet _and_ FunctionFS.

Now some clarification.
g_ffs is a legacy gadget, whose composition is defined statically
in source code, as opposed to gadgets composed with configfs at runtime.
In its Kconfig entry it allows some flexibility: as a matter of fact
g_ffs is a composite gadget, which always contains FunctionFS proper and,
optionally, CDC ECM (Ethernet), and RNDIS (Ethernet).

A USB gadget contains one or more configurations, each configuration
groups a number of usb functions like Ethernet, Mass Storage, HID etc.
g_ffs can contain up to 3 configurations:

0) FunctionFS + RNDIS
1) FunctionFS + ECM
2) FunctionFS

Please note that a popular, proprietary operating system asks only
for the first configuration in any gadget.

So in order to have Ethernet and FunctionFS in g_ffs you need
to provide a suitable configuration. make menuconfig is your friend.

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

Re: [PATCH v2] usb: gadget: OS desc type unicode multi

[Andrzej Pietrasiewicz]

Hi Mario,

sorry about the delay but I was busy with other things yesterday.

W dniu 02.02.2015 o 21:37, Mario Schuknecht pisze:

Hi Andrzej,

thank you for the comment.









I'm not sure I understand the logic of the below function well.


+static inline int usb_ext_prop_put_unicode_multi(u8 *buf, int pnl,
+   const char *string, int data_len)
+{
+   int outlen = 0;
+
+   put_unaligned_le32(data_len, usb_ext_prop_data_len_ptr(buf, pnl));
+
+   while (*string && outlen < data_len - 2) {



You keep looping as long as the source *string is not '\0'
and advance the string in each loop by adding (strlen() + 1) of
what is currently available starting at *string.
For example:

string: "a\0b\0and this is past the end of your source buffer"

first loop iteration:
len = strlen(string); /* len == 1 */
string += len + 1; /* string: "b\0and this is past the end of your source
buffer" */

second loop iteration:
len = strlen(string); /* len == 1 */
string += len + 1; /* string: "and this is past the end of your source
buffer" */

so effectively the first part of the while condition rarely ever becomes
"false".
In other words when you process all the source strings from "string" you, by
design,
end up one byte past the terminating '\0' of the source buffer. The contents
of this memory can be anything, there is just 1/256 chance it is zero,
so the "while (*string" part does not make sense to me.


The assumtion is that the input string is also double Nul-terminated.
E.g. "one\0two\0three\0\0"

Should I add a parameter "inlen" which contains the input buffer length?


Ah, right. Now I see your point. You need to be able to tell
whether the current "sub"string is the last one or not.


Or can I trust that the input string is double Nul-terminated?


Extended Properties support is meant primarily to be used with the
configfs interface. The user creates a directory in /interface..
The name of the directory becomes the name of the property. Inside
the directory there are two attribute files: "data" and "type".
The user stores numeric type id into "type", in this case "7".
And then the user stores property's contents into "data".
From this perspective double termination seems awkward,
as a user I would expect that I just store a sequence of
NUL-terminated strings without additional terminator.
But this approach implies passing some additional information
to usb_ext_prop_put_unicode_multi() instead.

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

Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia

[Andrzej Pietrasiewicz]

Hello Pali,

W dniu 31.01.2015 o 10:53, Pali Rohár pisze:

This patch adds removable mass storage support to g_nokia gadget (for N900).
It means that at runtime block device can be exported or unexported.


For a hint please see this thread:
http://www.spinics.net/lists/linux-usb/msg119669.html

--
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

Re: [PATCH v2] usb: gadget: OS desc type unicode multi

[Andrzej Pietrasiewicz]

Hi Mario,

W dniu 01.02.2015 o 21:07, Mario Schuknecht pisze:

A popular proprietary operating system expects that USB devices provide extra
information via "OS descriptors". An introduction to the subject can be found
here:







This patch adds support for property data type 0x7 multiple NUL-terminated
unicode strings.


This suggests that you mean

#define USB_EXT_PROP_UNICODE_MULTI  7


Add case USB_EXT_PROP_UNICODE_MULTI in function fill_ext_prop()


And you do.



Signed-off-by: Mario Schuknecht 
---

Changes in v2:
- improve commit log

  drivers/usb/gadget/composite.c |  5 +
  drivers/usb/gadget/u_os_desc.h | 28 
  2 files changed, 33 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 6178353..bf30b73 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1422,6 +1422,11 @@ static int fill_ext_prop(struct usb_configuration *c, 
int interface, u8 *buf)
 ext_prop->data,
 ext_prop->data_len);
break;
+   case USB_EXT_PROP_UNICODE_LINK_MULTI:

Does it compile? I mean the _LINK_ infix - didn't you mean

+   case USB_EXT_PROP_UNICODE_MULTI:

instead?


+   usb_ext_prop_put_unicode_multi(buf, ret,
+ext_prop->data,
+ext_prop->data_len);
+   break;
case USB_EXT_PROP_BINARY:
usb_ext_prop_put_binary(buf, ret,
ext_prop->data,
diff --git a/drivers/usb/gadget/u_os_desc.h b/drivers/usb/gadget/u_os_desc.h
index 947b7dd..bee4274 100644
--- a/drivers/usb/gadget/u_os_desc.h
+++ b/drivers/usb/gadget/u_os_desc.h
@@ -120,4 +120,32 @@ static inline int usb_ext_prop_put_unicode(u8 *buf, int 
pnl, const char *string,
return data_len;
  }



I'm not sure I understand the logic of the below function well.


+static inline int usb_ext_prop_put_unicode_multi(u8 *buf, int pnl,
+   const char *string, int data_len)
+{
+   int outlen = 0;
+
+   put_unaligned_le32(data_len, usb_ext_prop_data_len_ptr(buf, pnl));
+
+   while (*string && outlen < data_len - 2) {


You keep looping as long as the source *string is not '\0'
and advance the string in each loop by adding (strlen() + 1) of
what is currently available starting at *string.
For example:

string: "a\0b\0and this is past the end of your source buffer"

first loop iteration:
len = strlen(string); /* len == 1 */
string += len + 1; /* string: "b\0and this is past the end of your source 
buffer" */

second loop iteration:
len = strlen(string); /* len == 1 */
string += len + 1; /* string: "and this is past the end of your source buffer" 
*/

so effectively the first part of the while condition rarely ever becomes 
"false".
In other words when you process all the source strings from "string" you, by 
design,
end up one byte past the terminating '\0' of the source buffer. The contents
of this memory can be anything, there is just 1/256 chance it is zero,
so the "while (*string" part does not make sense to me.



+   int len = strlen(string);
+   int result = utf8s_to_utf16s(string, len, UTF16_LITTLE_ENDIAN,
+   (wchar_t *) usb_ext_prop_data_ptr(buf, pnl + outlen),
+   data_len - outlen - 2);
+   if (result < 0)
+   return result;
+   outlen += result << 1;
+   string += len + 1;
+   put_unaligned_le16(0,
+   &buf[USB_EXT_PROP_B_PROPERTY_DATA + pnl + outlen]);

You put a terminating NUL "two-byte" after each destination, "wide" string.


+   outlen += 2;


Then you advance the "outlen" to reflect the above.



+   }

Suppose there is just one string in the source "string", so the loop
terminates now.

Am I correct that at this point "outlen" is supposed to be equal "data_len"?
If it is, than please see below (*).



+
+   put_unaligned_le16(0,
+   &buf[USB_EXT_PROP_B_PROPERTY_DATA + pnl + outlen]);


Why again terminate the destination string? And, since outlen value was
incremented by 2 in the meantime there will be two terminators in a row?


+   outlen += 2;


(*)
If "outlen" was supposed to be equal "data_len" above then the
return value of the whole function becomes data_len + 2.


+
+   return outlen;
+}
+
  #endif /* __U_OS_DESC_H__ */



--
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

Re: [PATCH] usb: gadget: Fix os desc test

[Andrzej Pietrasiewicz]

W dniu 26.01.2015 o 12:47, Mario Schuknecht pisze:

USB vendor type is encoded in field bmRequestType. Make test USB_TYPE_VENDOR
with bRequestType instead of bRequest.

Signed-off-by: Mario Schuknecht 


Acked-by: Andrzej Pietrasiewicz 


---
  drivers/usb/gadget/composite.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 6178353..13adfd1 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1655,7 +1655,7 @@ unknown:
 * OS descriptors handling
 */
if (cdev->use_os_string && cdev->os_desc_config &&
-   (ctrl->bRequest & USB_TYPE_VENDOR) &&
+   (ctrl->bRequestType & USB_TYPE_VENDOR) &&
ctrl->bRequest == cdev->b_vendor_code) {
struct usb_request  *req;
struct usb_configuration*os_desc_cfg;



--
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

Re: [PATCH] usb: gadget: os desc fix

[Andrzej Pietrasiewicz]

Hi Mario,

W dniu 23.01.2015 o 21:52, Mario Schuknecht pisze:

Add missing case USB_EXT_PROP_UNICODE_MULTI to fill_ext_prop.

Type vendor is coded in bRequestType. Compare USB_TYPE_VENDOR with
bRequestType instead of bRequest.



The above seem to be unrelated changes.
Please split into two separate patches, but see below.


Signed-off-by: Mario Schuknecht 
---
  drivers/usb/gadget/composite.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 6178353..3f7227b 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1418,6 +1418,7 @@ static int fill_ext_prop(struct usb_configuration *c, int 
interface, u8 *buf)
case USB_EXT_PROP_UNICODE:
case USB_EXT_PROP_UNICODE_ENV:
case USB_EXT_PROP_UNICODE_LINK:
+   case USB_EXT_PROP_UNICODE_MULTI:
usb_ext_prop_put_unicode(buf, ret,
 ext_prop->data,
 ext_prop->data_len);


While USB_EXT_PROP_UNICODE, USB_EXT_PROP_UNICODE_ENV and 
USB_EXT_PROP_UNICODE_LINK
are defined as "A NULL-terminated Unicode string", USB_EXT_PROP_UNICODE_MULTI
is defined as "Multiple NULL-terminated Unicode strings" - note the plural here.
usb_ext_prop_put_unicode() internally calls utf8s_to_utf16s() and it seems that
the latter is not designed to handle multiple strings in sequence. So I guess
that either you should add a separate case or modify usb_ext_prop_unicode().
I'd prefer the first option and there perhaps usb_ext_prop_put_unicode() should
be called  in a loop.

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

Re: f_hid, f_mass_storage, and f_rdnis via configfs on platform/intel-mid

[Andrzej Pietrasiewicz]

Hello Chris,

W dniu 22.01.2015 o 15:37, Chris McClimans pisze:

The devices in embedded space accept kernel updates at various speeds.
The raspberry pi is at 3.18, and the Intel Edison is bit lagging at 3.10.
I thought it would probably be useful to the embedded community if I at
least took a stab at back porting the hid gadgetfs function to 3.18 for use


There is no gadgetfs hid implementation, at least there are none
that I am aware of.

These days instead of gadgetfs one should probably use FunctionFS.
The purpose of the two is delegating actual usb function implementation
to userspace with some filesystem being the interface to the kernel.

And ConfigFS is a totally different thing: in the context of usb gadgets
it serves the purpose of composing gadgets of different usb functions
at runtime. The kernel versions and function names you mention below
suggest that you mean ConfigFS.


on the raspberry pi. Going farther back with more modules if there is interest.

I searched through the kernel source and I think I've found the definitive
list of gadget functions (grepping for DECLARE_USB_FUNCTION_INIT),


You should also grep for DECLARE_USB_FUNCTION

and then you would find:

SourceSink
Loopback

which, in legacy gadgets, are components of gadget zero (g_zero),
the "first" gadget by David Brownell.
The two functions serve testing purposes and their names suggest
their use. They are also available for composing gadgets with configfs.


and noted when they were introduced.



It is likely that uvc will land in 3.20, so you might also be preparing for

3.20 uvc


3.19 midi, hid
3.18 uvc, uac2, uac1
3.14 fs
3.13 mass_storage
3.11 subset, rndis, phonet, ncm, eem, ecm
3.10 serial, obex, acm

I figured this list was probably the best place to ask if there were any
caveats to taking usb/gadget/function/f_hid.c back one kernel rev.


With hid specifically it is likely you will just cherry pick a couple
of commits and solve some trivial conflicts (like in Makefile or Kconfig).



On Tue, Jan 13, 2015 at 11:49 AM, Felipe Balbi  wrote:

then you need to backport patches yourself. The community can't really

support older kernels :-)

I guess you don't have other way but keep in mind you'll, essentially,

be on your own

Any obvious dragons be aware of before I head out on my own?



If you decide to backport anything to 3.10, you must take this into
consideration:
Apart from some changes in the usb functions themselves,
at some point in time there happended refactoring of the gadget
directory. I guess it was sometime between 3.14 and 3.18.
Before the refactoring all the source code files related to gadgets
implementation had resided in drivers/usb/gadget. Since the refactoring
there are dedicated directories: functions, udc and legacy.
"functions" dir contains actual usb function implementations, "udc" dir
contains USB Device Controller implementations (except dual role devices
like dwc2 or dwc3) and "legacy" dir contains traditional gadgets,
composed at compilation time. The change also affected Makefiles
and Kconfigs. That said, git might be smart enough to treat most of the
refactoring changes merely as "renames".



Is there a better list to or talk/discuss backporting of usb gadget functions?



I guess there is no other list where people know more
about usb gadgets in Linux.
But when it comes to backporting you are on your own,
as Felipe said. If I were you, I would be targeting 3.18.
By the time you will have been done at least the 3.20 (or newer)
will be the state-of-the art kernel. My personal view is
that supporting 3.10 at that time makes no sense.

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

Re: [PATCHv2 3/4] usb: gadget: uvc: use explicit type instead of void *

[Andrzej Pietrasiewicz]

W dniu 19.01.2015 o 22:30, Laurent Pinchart pisze:

Hi Andrzej,

On Monday 19 January 2015 23:27:28 Laurent Pinchart wrote:

On Monday 19 January 2015 13:52:57 Andrzej Pietrasiewicz wrote:

The first parameter of __uvcg_iter_strm_cls() is always used in
the context of struct uvcg_streaming_header, so change the
function prototype accordingly.

Signed-off-by: Andrzej Pietrasiewicz 


Acked-by: Laurent Pinchart 


I spoke too fast. Shouldn't the first argument of the callback function be a
struct uvcg_streaming_header * as well ?



Both __uvcg_count_strm() and __uvcg_fill_strm() use their 1st parameter
in different contexts, depending whether they are called at the
streaming header, format or frame level. That is why originally
the 1st parameter of __uvcg_iter_strm_cls() was a void * - to emphasize
the fact that what is propagated to the callbacks as the 1st parameter
is not always the same type of thing.

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

[PATCHv2 1/4] Revert "usb: gadget: uvc: cleanup __uvcg_fill_strm()"

[Andrzej Pietrasiewicz]

This reverts commit c8dba1896595 ("usb: gadget: uvc: cleanup 
__uvcg_fill_strm()").
__uvcg_fill_stream() during its execution uses priv2 as a pointer
to a pointer, because it advances the current position by the amount
of data taken by each processed descriptor and the advanced position
should be visible outside this function, so that the next time it is
called, the current position corresponds to the place where it was
the last time rather than again at the beginning of some block of data.
In other words priv2 is an "out" parameter.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/uvc_configfs.c | 39 +++---
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/gadget/function/uvc_configfs.c 
b/drivers/usb/gadget/function/uvc_configfs.c
index 8a6cd61..cc2a613 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -2000,27 +2000,28 @@ static int __uvcg_cnt_strm(void *priv1, void *priv2, 
void *priv3, int n,
return 0;
 }
 
-static int __uvcg_fill_strm(void *priv1, void *dest, void *priv3, int n,
+static int __uvcg_fill_strm(void *priv1, void *priv2, void *priv3, int n,
enum uvcg_strm_type type)
 {
+   void **dest = priv2;
struct uvc_descriptor_header ***array = priv3;
size_t sz;
 
-   **array = dest;
+   **array = *dest;
++*array;
 
switch (type) {
case UVCG_HEADER: {
-   struct uvc_input_header_descriptor *ihdr = dest;
+   struct uvc_input_header_descriptor *ihdr = *dest;
struct uvcg_streaming_header *h = priv1;
struct uvcg_format_ptr *f;
 
-   memcpy(dest, &h->desc, sizeof(h->desc));
-   dest += sizeof(h->desc);
+   memcpy(*dest, &h->desc, sizeof(h->desc));
+   *dest += sizeof(h->desc);
sz = UVCG_STREAMING_CONTROL_SIZE;
list_for_each_entry(f, &h->formats, entry) {
-   memcpy(dest, f->fmt->bmaControls, sz);
-   dest += sz;
+   memcpy(*dest, f->fmt->bmaControls, sz);
+   *dest += sz;
}
ihdr->bLength = sizeof(h->desc) + h->num_fmt * sz;
ihdr->bNumFormats = h->num_fmt;
@@ -2030,22 +2031,22 @@ static int __uvcg_fill_strm(void *priv1, void *dest, 
void *priv3, int n,
struct uvcg_format *fmt = priv1;
 
if (fmt->type == UVCG_UNCOMPRESSED) {
-   struct uvc_format_uncompressed *unc = dest;
+   struct uvc_format_uncompressed *unc = *dest;
struct uvcg_uncompressed *u =
container_of(fmt, struct uvcg_uncompressed,
 fmt);
 
-   memcpy(dest, &u->desc, sizeof(u->desc));
-   dest += sizeof(u->desc);
+   memcpy(*dest, &u->desc, sizeof(u->desc));
+   *dest += sizeof(u->desc);
unc->bNumFrameDescriptors = fmt->num_frames;
unc->bFormatIndex = n + 1;
} else if (fmt->type == UVCG_MJPEG) {
-   struct uvc_format_mjpeg *mjp = dest;
+   struct uvc_format_mjpeg *mjp = *dest;
struct uvcg_mjpeg *m =
container_of(fmt, struct uvcg_mjpeg, fmt);
 
-   memcpy(dest, &m->desc, sizeof(m->desc));
-   dest += sizeof(m->desc);
+   memcpy(*dest, &m->desc, sizeof(m->desc));
+   *dest += sizeof(m->desc);
mjp->bNumFrameDescriptors = fmt->num_frames;
mjp->bFormatIndex = n + 1;
} else {
@@ -2055,15 +2056,15 @@ static int __uvcg_fill_strm(void *priv1, void *dest, 
void *priv3, int n,
break;
case UVCG_FRAME: {
struct uvcg_frame *frm = priv1;
-   struct uvc_descriptor_header *h = dest;
+   struct uvc_descriptor_header *h = *dest;
 
sz = sizeof(frm->frame);
-   memcpy(dest, &frm->frame, sz);
-   dest += sz;
+   memcpy(*dest, &frm->frame, sz);
+   *dest += sz;
sz = frm->frame.b_frame_interval_type *
sizeof(*frm->dw_frame_interval);
-   memcpy(dest, frm->dw_frame_interval, sz);
-   dest += sz;
+   memcpy(*dest, frm->dw_frame_interval, sz);
+   *dest += sz;
if (frm->fmt_type == UVCG_UNCOMPRESSED)
h->bLength

[PATCHv2 0/4] Fixes for configfs support in uvc

[Andrzej Pietrasiewicz]

This short series reverts a commit which, at a first glance,
simplifies the code but in fact makes it not work correctly.
The series applies small fixes and adds some comments to functions.
This is an updated series after Laurent's comments - thank you,
Laurent.

@Felipe: The first patch of the series should be applied
only if the patch being reverted is already in your tree.
If it is not, feel free to apply only patches 2..4.

v1..v2:

- Updated the commit log of the reverted commit - used
the correct hash-id of the reverted commit in the log
- Used explicit type instead of void * where it can be used

Andrzej Pietrasiewicz (4):
  Revert "usb: gadget: uvc: cleanup __uvcg_fill_strm()"
  usb: gadget: uvc: preserve the address passed to kfree()
  usb: gadget: uvc: use explicit type instead of void *
  usb: gadget: uvc: comments for iterating over streaming hierarchy

 drivers/usb/gadget/function/uvc_configfs.c | 87 +-
 1 file changed, 63 insertions(+), 24 deletions(-)

-- 
1.9.1

--
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

[PATCHv2 2/4] usb: gadget: uvc: preserve the address passed to kfree()

[Andrzej Pietrasiewicz]

__uvcg_fill_strm() called from __uvcg_iter_stream_cls()
might have advanced the "data" even if __uvcg_iter_stream_cls()
returns an error, so use a backup copy as an argument to kfree().

Signed-off-by: Andrzej Pietrasiewicz 
Acked-by: Laurent Pinchart 
---
 drivers/usb/gadget/function/uvc_configfs.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/uvc_configfs.c 
b/drivers/usb/gadget/function/uvc_configfs.c
index cc2a613..49f25e8 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -2086,7 +2086,7 @@ static int uvcg_streaming_class_allow_link(struct 
config_item *src,
struct mutex *su_mutex = &src->ci_group->cg_subsys->su_mutex;
struct uvc_descriptor_header ***class_array, **cl_arr;
struct uvcg_streaming_header *target_hdr;
-   void *data;
+   void *data, *data_save;
size_t size = 0, count = 0;
int ret = -EINVAL;
 
@@ -2119,7 +2119,7 @@ static int uvcg_streaming_class_allow_link(struct 
config_item *src,
goto unlock;
}
 
-   data = kzalloc(size, GFP_KERNEL);
+   data = data_save = kzalloc(size, GFP_KERNEL);
if (!data) {
kfree(*class_array);
*class_array = NULL;
@@ -2132,7 +2132,11 @@ static int uvcg_streaming_class_allow_link(struct 
config_item *src,
if (ret) {
kfree(*class_array);
*class_array = NULL;
-   kfree(data);
+   /*
+* __uvcg_fill_strm() called from __uvcg_iter_stream_cls()
+* might have advanced the "data", so use a backup copy
+*/
+   kfree(data_save);
goto unlock;
}
*cl_arr = (struct uvc_descriptor_header *)&opts->uvc_color_matching;
-- 
1.9.1

--
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

[PATCHv2 3/4] usb: gadget: uvc: use explicit type instead of void *

[Andrzej Pietrasiewicz]

The first parameter of __uvcg_iter_strm_cls() is always used in
the context of struct uvcg_streaming_header, so change the
function prototype accordingly.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/uvc_configfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/uvc_configfs.c 
b/drivers/usb/gadget/function/uvc_configfs.c
index 49f25e8..51d8e9e 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -1918,10 +1918,10 @@ enum uvcg_strm_type {
UVCG_FRAME
 };
 
-static int __uvcg_iter_strm_cls(void *priv1, void *priv2, void *priv3,
+static int __uvcg_iter_strm_cls(struct uvcg_streaming_header *h,
+   void *priv2, void *priv3,
int (*fun)(void *, void *, void *, int, enum uvcg_strm_type type))
 {
-   struct uvcg_streaming_header *h = priv1;
struct uvcg_format_ptr *f;
struct config_group *grp;
struct config_item *item;
-- 
1.9.1

--
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

[PATCHv2 4/4] usb: gadget: uvc: comments for iterating over streaming hierarchy

[Andrzej Pietrasiewicz]

The purpose of the functions and their parametrs might not be obvious
to the reader, so explain it.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/uvc_configfs.c | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/drivers/usb/gadget/function/uvc_configfs.c 
b/drivers/usb/gadget/function/uvc_configfs.c
index 51d8e9e..3c0467b 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -1918,6 +1918,25 @@ enum uvcg_strm_type {
UVCG_FRAME
 };
 
+/*
+ * Iterate over a hierarchy of streaming descriptors' config items.
+ * The items are created by the user with configfs.
+ *
+ * It "processes" the header pointed to by @priv1, then for each format
+ * that follows the header "processes" the format itself and then for
+ * each frame inside a format "processes" the frame.
+ *
+ * As a "processing" function the @fun is used.
+ *
+ * __uvcg_iter_strm_cls() is used in two context: first, to calculate
+ * the amount of memory needed for an array of streaming descriptors
+ * and second, to actually fill the array.
+ *
+ * @h: streaming header pointer
+ * @priv2: an "inout" parameter (the caller might want to see the changes to 
it)
+ * @priv3: an "inout" parameter (the caller might want to see the changes to 
it)
+ * @fun: callback function for processing each level of the hierarchy
+ */
 static int __uvcg_iter_strm_cls(struct uvcg_streaming_header *h,
void *priv2, void *priv3,
int (*fun)(void *, void *, void *, int, enum uvcg_strm_type type))
@@ -1951,6 +1970,14 @@ static int __uvcg_iter_strm_cls(struct 
uvcg_streaming_header *h,
return ret;
 }
 
+/*
+ * Count how many bytes are needed for an array of streaming descriptors.
+ *
+ * @priv1: pointer to a header, format or frame
+ * @priv2: inout parameter, accumulated size of the array
+ * @priv3: inout parameter, accumulated number of the array elements
+ * @n: unused, this function's prototype must match @fun in 
__uvcg_iter_strm_cls
+ */
 static int __uvcg_cnt_strm(void *priv1, void *priv2, void *priv3, int n,
   enum uvcg_strm_type type)
 {
@@ -2000,6 +2027,13 @@ static int __uvcg_cnt_strm(void *priv1, void *priv2, 
void *priv3, int n,
return 0;
 }
 
+/*
+ * Fill an array of streaming descriptors.
+ *
+ * @priv1: pointer to a header, format or frame
+ * @priv2: inout parameter, pointer into a block of memory
+ * @priv3: inout parameter, pointer to a 2-dimensional array
+ */
 static int __uvcg_fill_strm(void *priv1, void *priv2, void *priv3, int n,
enum uvcg_strm_type type)
 {
-- 
1.9.1

--
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

Re: [PATCH 1/3] Revert "usb: gadget: uvc: cleanup __uvcg_fill_strm()"

[Andrzej Pietrasiewicz]

W dniu 19.01.2015 o 01:05, Laurent Pinchart pisze:

Hi Andrzej,

Thank you for the patch.

On Friday 16 January 2015 15:14:26 Andrzej Pietrasiewicz wrote:

This reverts commit c0b53cb16250 ("usb: gadget: uvc: cleanup
__uvcg_fill_strm()").


I can't find that commit in Linus' master, next/master or Felipe's next
branch. If the patch hasn't been applied there's no need to revert it :-)


I (used to) have it in Felipe's testing/next.
But now the commit is visible as

c8dba18965954e7e9439376e0aa40fb5bb4c67e3.

Anyway, I will post a modified v2 series including changes resulting
from your comments.

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

[PATCH 1/3] Revert "usb: gadget: uvc: cleanup __uvcg_fill_strm()"

[Andrzej Pietrasiewicz]

This reverts commit c0b53cb16250 ("usb: gadget: uvc: cleanup 
__uvcg_fill_strm()").
__uvcg_fill_stream() during its execution uses priv2 as a pointer
to a pointer, because it advances the current position by the amount
of data taken by each processed descriptor and the advanced position
should be visible outside this function, so that the next time it is
called, the current position corresponds to the place where it was
the last time rather than again at the beginning of some block of data.
In other words priv2 is an "out" parameter.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/uvc_configfs.c | 39 +++---
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/gadget/function/uvc_configfs.c 
b/drivers/usb/gadget/function/uvc_configfs.c
index 8a6cd61..cc2a613 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -2000,27 +2000,28 @@ static int __uvcg_cnt_strm(void *priv1, void *priv2, 
void *priv3, int n,
return 0;
 }
 
-static int __uvcg_fill_strm(void *priv1, void *dest, void *priv3, int n,
+static int __uvcg_fill_strm(void *priv1, void *priv2, void *priv3, int n,
enum uvcg_strm_type type)
 {
+   void **dest = priv2;
struct uvc_descriptor_header ***array = priv3;
size_t sz;
 
-   **array = dest;
+   **array = *dest;
++*array;
 
switch (type) {
case UVCG_HEADER: {
-   struct uvc_input_header_descriptor *ihdr = dest;
+   struct uvc_input_header_descriptor *ihdr = *dest;
struct uvcg_streaming_header *h = priv1;
struct uvcg_format_ptr *f;
 
-   memcpy(dest, &h->desc, sizeof(h->desc));
-   dest += sizeof(h->desc);
+   memcpy(*dest, &h->desc, sizeof(h->desc));
+   *dest += sizeof(h->desc);
sz = UVCG_STREAMING_CONTROL_SIZE;
list_for_each_entry(f, &h->formats, entry) {
-   memcpy(dest, f->fmt->bmaControls, sz);
-   dest += sz;
+   memcpy(*dest, f->fmt->bmaControls, sz);
+   *dest += sz;
}
ihdr->bLength = sizeof(h->desc) + h->num_fmt * sz;
ihdr->bNumFormats = h->num_fmt;
@@ -2030,22 +2031,22 @@ static int __uvcg_fill_strm(void *priv1, void *dest, 
void *priv3, int n,
struct uvcg_format *fmt = priv1;
 
if (fmt->type == UVCG_UNCOMPRESSED) {
-   struct uvc_format_uncompressed *unc = dest;
+   struct uvc_format_uncompressed *unc = *dest;
struct uvcg_uncompressed *u =
container_of(fmt, struct uvcg_uncompressed,
 fmt);
 
-   memcpy(dest, &u->desc, sizeof(u->desc));
-   dest += sizeof(u->desc);
+   memcpy(*dest, &u->desc, sizeof(u->desc));
+   *dest += sizeof(u->desc);
unc->bNumFrameDescriptors = fmt->num_frames;
unc->bFormatIndex = n + 1;
} else if (fmt->type == UVCG_MJPEG) {
-   struct uvc_format_mjpeg *mjp = dest;
+   struct uvc_format_mjpeg *mjp = *dest;
struct uvcg_mjpeg *m =
container_of(fmt, struct uvcg_mjpeg, fmt);
 
-   memcpy(dest, &m->desc, sizeof(m->desc));
-   dest += sizeof(m->desc);
+   memcpy(*dest, &m->desc, sizeof(m->desc));
+   *dest += sizeof(m->desc);
mjp->bNumFrameDescriptors = fmt->num_frames;
mjp->bFormatIndex = n + 1;
} else {
@@ -2055,15 +2056,15 @@ static int __uvcg_fill_strm(void *priv1, void *dest, 
void *priv3, int n,
break;
case UVCG_FRAME: {
struct uvcg_frame *frm = priv1;
-   struct uvc_descriptor_header *h = dest;
+   struct uvc_descriptor_header *h = *dest;
 
sz = sizeof(frm->frame);
-   memcpy(dest, &frm->frame, sz);
-   dest += sz;
+   memcpy(*dest, &frm->frame, sz);
+   *dest += sz;
sz = frm->frame.b_frame_interval_type *
sizeof(*frm->dw_frame_interval);
-   memcpy(dest, frm->dw_frame_interval, sz);
-   dest += sz;
+   memcpy(*dest, frm->dw_frame_interval, sz);
+   *dest += sz;
if (frm->fmt_type == UVCG_UNCOMPRESSED)
h->bLength

[PATCH 0/3] Fixes for configfs support in uvc

[Andrzej Pietrasiewicz]

This short series reverts a commit which, at a first glance,
simplifies the code but in fact makes it not work correctly,
and then applies a small fix and adds some comments to functions.

Andrzej Pietrasiewicz (3):
  Revert "usb: gadget: uvc: cleanup __uvcg_fill_strm()"
  usb: gadget: uvc: preserve the address passed to kfree()
  usb: gadget: uvc: comments for iterating over streaming hierarchy

 drivers/usb/gadget/function/uvc_configfs.c | 83 ++
 1 file changed, 61 insertions(+), 22 deletions(-)

-- 
1.9.1

--
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

[PATCH 2/3] usb: gadget: uvc: preserve the address passed to kfree()

[Andrzej Pietrasiewicz]

__uvcg_fill_strm() called from __uvcg_iter_stream_cls()
might have advanced the "data" even if __uvcg_iter_stream_cls()
returns an error, so use a backup copy as an argument to kfree().

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/uvc_configfs.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/uvc_configfs.c 
b/drivers/usb/gadget/function/uvc_configfs.c
index cc2a613..49f25e8 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -2086,7 +2086,7 @@ static int uvcg_streaming_class_allow_link(struct 
config_item *src,
struct mutex *su_mutex = &src->ci_group->cg_subsys->su_mutex;
struct uvc_descriptor_header ***class_array, **cl_arr;
struct uvcg_streaming_header *target_hdr;
-   void *data;
+   void *data, *data_save;
size_t size = 0, count = 0;
int ret = -EINVAL;
 
@@ -2119,7 +2119,7 @@ static int uvcg_streaming_class_allow_link(struct 
config_item *src,
goto unlock;
}
 
-   data = kzalloc(size, GFP_KERNEL);
+   data = data_save = kzalloc(size, GFP_KERNEL);
if (!data) {
kfree(*class_array);
*class_array = NULL;
@@ -2132,7 +2132,11 @@ static int uvcg_streaming_class_allow_link(struct 
config_item *src,
if (ret) {
kfree(*class_array);
*class_array = NULL;
-   kfree(data);
+   /*
+* __uvcg_fill_strm() called from __uvcg_iter_stream_cls()
+* might have advanced the "data", so use a backup copy
+*/
+   kfree(data_save);
goto unlock;
}
*cl_arr = (struct uvc_descriptor_header *)&opts->uvc_color_matching;
-- 
1.9.1

--
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

[PATCH 3/3] usb: gadget: uvc: comments for iterating over streaming hierarchy

[Andrzej Pietrasiewicz]

The purpose of the functions and their parametrs might not be obvious
to the reader, so explain it.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/uvc_configfs.c | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/drivers/usb/gadget/function/uvc_configfs.c 
b/drivers/usb/gadget/function/uvc_configfs.c
index 49f25e8..6fd40c5 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -1918,6 +1918,25 @@ enum uvcg_strm_type {
UVCG_FRAME
 };
 
+/*
+ * Iterate over a hierarchy of streaming descriptors' config items.
+ * The items are created by the user with configfs.
+ *
+ * It "processes" the header pointed to by @priv1, then for each format
+ * that follows the header "processes" the format itself and then for
+ * each frame inside a format "processes" the frame.
+ *
+ * As a "processing" function the @fun is used.
+ *
+ * __uvcg_iter_strm_cls() is used in two context: first, to calculate
+ * the amount of memory needed for an array of streaming descriptors
+ * and second, to actually fill the array.
+ *
+ * @priv1: an "in" parameter
+ * @priv2: an "inout" parameter (the caller might want to see the changes to 
it)
+ * @priv3: an "inout" parameter (the caller might want to see the changes to 
it)
+ * @fun: callback function for processing each level of the hierarchy
+ */
 static int __uvcg_iter_strm_cls(void *priv1, void *priv2, void *priv3,
int (*fun)(void *, void *, void *, int, enum uvcg_strm_type type))
 {
@@ -1951,6 +1970,14 @@ static int __uvcg_iter_strm_cls(void *priv1, void 
*priv2, void *priv3,
return ret;
 }
 
+/*
+ * Count how many bytes are needed for an array of streaming descriptors.
+ *
+ * @priv1: pointer to a header, format or frame
+ * @priv2: inout parameter, accumulated size of the array
+ * @priv3: inout parameter, accumulated number of the array elements
+ * @n: unused, this function's prototype must match @fun in 
__uvcg_iter_strm_cls
+ */
 static int __uvcg_cnt_strm(void *priv1, void *priv2, void *priv3, int n,
   enum uvcg_strm_type type)
 {
@@ -2000,6 +2027,13 @@ static int __uvcg_cnt_strm(void *priv1, void *priv2, 
void *priv3, int n,
return 0;
 }
 
+/*
+ * Fill an array of streaming descriptors.
+ *
+ * @priv1: pointer to a header, format or frame
+ * @priv2: inout parameter, pointer into a block of memory
+ * @priv3: inout parameter, pointer to a 2-dimensional array
+ */
 static int __uvcg_fill_strm(void *priv1, void *priv2, void *priv3, int n,
enum uvcg_strm_type type)
 {
-- 
1.9.1

--
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

Re: [patch 2/6] usb: gadget: uvc: cleanup __uvcg_fill_strm()

[Andrzej Pietrasiewicz]

W dniu 16.01.2015 o 13:12, Dan Carpenter pisze:

Oh, yeah.  You're right.  I should have been more careful and I should
have seen that.  Sorry.  But the problem is the original code is still a
bit buggy.

We call:

data = kzalloc();

Inside __uvcg_fill_strm() we do "data += something;"

kfree(data);

We should save the orignal data pointer so that we can free it
correctly at the end in uvcg_streaming_class_allow_link().


Yeah, right. Thank you for spotting this. The kfree() is called
only if __uvcg_fill_strm() fails, though. But of course this needs
to be fixed.

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

Re: [patch 2/6] usb: gadget: uvc: cleanup __uvcg_fill_strm()

[Andrzej Pietrasiewicz]

W dniu 14.01.2015 o 22:01, Dan Carpenter pisze:

Static checkers complain about this API:

drivers/usb/gadget/function/uvc_configfs.c:2139
uvcg_streaming_class_allow_link()
warn: did you really mean to pass the address of 'data'?

Indeed, the code is cleaner when we just pass the pointer instead of the
pointer to the pointer.

Signed-off-by: Dan Carpenter 


I'm sorry but this patch breaks the code.

You should look at the broader context in which the __uvcg_fill_strm()
is called: it is used as a callback from __uvcg_iter_strm_cls().
The purpose of the latter is to iterate over a hierarchy of streaming
descriptors' config items built by the user with configfs.
This function is generic, because it needs to be called twice:
the first time to calculate the actual amount of memory needed for
an array of descriptors which is to be created, and the second time
to actually fill the said array - this is the purpose of __uvcg_fill_strm().

__uvcg_iter_strm_cls() flow of control is more or less like this:
"process" the header, then for each format that follows the header
"process" the format itself and then for each frame inside
a format "process" the frame.

__uvcg_fill_stream(), which is used to "process", during its execution
uses priv2 as a pointer to a pointer, because it advances the current
position by the amount of data taken by each processed descriptor
and the advanced position should be visible outside this function,
so that the next time it is called, the current position corresponds
to the place where it was the last time rather than again at
the beginning of some block of data. In other words priv2
is an "out" parameter.


---
Looks obvious enough to me, but I've only compiled this code and haven't
tested it.



What I suggest is to revert this patch and then I will post another
patch which adds comments to __uvcg_iterm_strm_cls(), __uvcg_cnt_strm()
and __uvcg_fill_strm() so that their purpose and meaning of parameters
is clear.

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

Re: [patch 4/6] usb: gadget: uvc: memory leak in uvcg_frame_make()

[Andrzej Pietrasiewicz]

W dniu 14.01.2015 o 22:03, Dan Carpenter pisze:

We need to add a kfree(h) on an error path.

Signed-off-by: Dan Carpenter 



Acked-by: Andrzej Pietrasiewicz 


diff --git a/drivers/usb/gadget/function/uvc_configfs.c 
b/drivers/usb/gadget/function/uvc_configfs.c
index 738d68f..1af2686 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -1300,6 +1300,7 @@ static struct config_item *uvcg_frame_make(struct 
config_group *group,
h->fmt_type = UVCG_MJPEG;
} else {
mutex_unlock(&opts->lock);
+   kfree(h);
return ERR_PTR(-EINVAL);
}
++fmt->num_frames;
--
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



--
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

Re: [patch 6/6] usb: gadget: uvc: cleanup UVCG_FRAME_ATTR macro

[Andrzej Pietrasiewicz]

W dniu 14.01.2015 o 22:06, Dan Carpenter pisze:

1) Change "conv" an "vnoc" to "to_cpu_endian" to "to_little_endian".
2) No need to check the "limit" because that is already handled in
kstrtoXX so delete that parameter along with the check.
3) By using a "bits" parameter, we can combine the "uxx" parameter and
the "str2u" parameters.
4) The kstrtou##bits() conversion does not need to be done under the
mutex so move it to the start of the function.
5) Change the name of "identity_conv" to "noop_conversion".

Signed-off-by: Dan Carpenter 



Acked-by: Andrzej Pietrasiewicz 

 ---

This file has a couple pages of Sparse endian warnings.
http://lwn.net/Articles/205624/
It's hard to review the static checker warnings when there are so many.

diff --git a/drivers/usb/gadget/function/uvc_configfs.c 
b/drivers/usb/gadget/function/uvc_configfs.c
index a0443a2..87beb8c 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -1032,7 +1032,7 @@ static struct configfs_item_operations 
uvcg_frame_item_ops = {
.store_attribute= uvcg_frame_attr_store,
  };

-#define UVCG_FRAME_ATTR(cname, aname, conv, str2u, uxx, vnoc, limit)   \
+#define UVCG_FRAME_ATTR(cname, aname, to_cpu_endian, to_little_endian, bits) \
  static ssize_t uvcg_frame_##cname##_show(struct uvcg_frame *f, char *page)\
  { \
struct f_uvc_opts *opts;\
@@ -1046,7 +1046,7 @@ static ssize_t uvcg_frame_##cname##_show(struct 
uvcg_frame *f, char *page)\
opts = to_f_uvc_opts(opts_item);\
\
mutex_lock(&opts->lock); \
-   result = sprintf(page, "%d\n", conv(f->frame.cname));  \
+   result = sprintf(page, "%d\n", to_cpu_endian(f->frame.cname)); \
mutex_unlock(&opts->lock);   \
\
mutex_unlock(su_mutex); \
@@ -1061,7 +1061,11 @@ static ssize_t  uvcg_frame_##cname##_store(struct 
uvcg_frame *f, \
struct uvcg_format *fmt;\
struct mutex *su_mutex = &f->item.ci_group->cg_subsys->su_mutex;\
int ret;\
-   uxx num;\
+   u##bits num;\
+   \
+   ret = kstrtou##bits(page, 0, &num); \
+   if (ret)\
+   return ret; \
\
mutex_lock(su_mutex); /* for navigating configfs hierarchy */   \
\
@@ -1075,15 +1079,7 @@ static ssize_t  uvcg_frame_##cname##_store(struct 
uvcg_frame *f, \
goto end;   \
}   \
\
-   ret = str2u(page, 0, &num); \
-   if (ret)\
-   goto end;   \
-   \
-   if (num > limit) {   \
-   ret = -EINVAL;  \
-   goto end;   \
-   }   \
-   f->frame.cname = vnoc(num);  \
+   f->frame.cname = to_little_endian(num);  \
ret = len;  \
  end:  \
mutex_unlock(&opts->lock);   \
@@ -1097,24 +1093,20 @@ static struct uvcg_frame_attribute  
\
uvcg_frame_##cname##_show,  \
uvcg_frame_##cname##_store)

-#define identity_conv(x) (x)
+#define noop_conversion(x) (x)

-UVCG_FRAME_ATTR(bm_capabilities, bmCapabilities, identity_conv, kstrt

Re: [patch 3/6] usb: gadget: uvc: remove an impossible condition

[Andrzej Pietrasiewicz]

W dniu 14.01.2015 o 22:02, Dan Carpenter pisze:

"num" is a u32 so "(num > 0x)" is never true.  Also the range
is already checked in kstrtou32().

Signed-off-by: Dan Carpenter 



Acked-by: Andrzej Pietrasiewicz 


diff --git a/drivers/usb/gadget/function/uvc_configfs.c 
b/drivers/usb/gadget/function/uvc_configfs.c
index 2bd0688..738d68f 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -1156,8 +1156,6 @@ static inline int __uvcg_fill_frm_intrv(char *buf, void 
*priv)
ret = kstrtou32(buf, 0, &num);
if (ret)
return ret;
-   if (num > 0x)
-   return -EINVAL;

interv = priv;
**interv = cpu_to_le32(num);
--
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



--
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

Re: [patch 1/6] usb: gadget: uvc: fix some error codes

[Andrzej Pietrasiewicz]

W dniu 14.01.2015 o 21:59, Dan Carpenter pisze:

We're basically saying ERR_CAST(NULL) and PTR_ERR(NULL) here, which is
nonsensical.

Signed-off-by: Dan Carpenter 


Acked-by: Andrzej Pietrasiewicz 



diff --git a/drivers/usb/gadget/function/uvc_configfs.c 
b/drivers/usb/gadget/function/uvc_configfs.c
index 33d92ab..d112c99 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -148,7 +148,7 @@ static struct config_item *uvcg_control_header_make(struct 
config_group *group,

h = kzalloc(sizeof(*h), GFP_KERNEL);
if (!h)
-   return ERR_CAST(h);
+   return ERR_PTR(-ENOMEM);

h->desc.bLength  = UVC_DT_HEADER_SIZE(1);
h->desc.bDescriptorType  = USB_DT_CS_INTERFACE;
@@ -840,7 +840,7 @@ static int uvcg_streaming_header_allow_link(struct 
config_item *src,

format_ptr = kzalloc(sizeof(*format_ptr), GFP_KERNEL);
if (!format_ptr) {
-   ret = PTR_ERR(format_ptr);
+   ret = -ENOMEM;
goto out;
}
ret = 0;
@@ -960,7 +960,7 @@ static struct config_item

h = kzalloc(sizeof(*h), GFP_KERNEL);
if (!h)
-   return ERR_CAST(h);
+   return ERR_PTR(-ENOMEM);

INIT_LIST_HEAD(&h->formats);
h->desc.bDescriptorType  = USB_DT_CS_INTERFACE;
@@ -1278,7 +1278,7 @@ static struct config_item *uvcg_frame_make(struct 
config_group *group,

h = kzalloc(sizeof(*h), GFP_KERNEL);
if (!h)
-   return ERR_CAST(h);
+   return ERR_PTR(-ENOMEM);

h->frame.b_descriptor_type   = USB_DT_CS_INTERFACE;
h->frame.b_frame_index   = 1;
@@ -1563,7 +1563,7 @@ static struct config_group *uvcg_uncompressed_make(struct 
config_group *group,

h = kzalloc(sizeof(*h), GFP_KERNEL);
if (!h)
-   return ERR_CAST(h);
+   return ERR_PTR(-ENOMEM);

h->desc.bLength  = UVC_DT_FORMAT_UNCOMPRESSED_SIZE;
h->desc.bDescriptorType  = USB_DT_CS_INTERFACE;
@@ -1772,7 +1772,7 @@ static struct config_group *uvcg_mjpeg_make(struct 
config_group *group,

h = kzalloc(sizeof(*h), GFP_KERNEL);
if (!h)
-   return ERR_CAST(h);
+   return ERR_PTR(-ENOMEM);

h->desc.bLength  = UVC_DT_FORMAT_MJPEG_SIZE;
h->desc.bDescriptorType  = USB_DT_CS_INTERFACE;
@@ -2124,7 +2124,7 @@ static int uvcg_streaming_class_allow_link(struct 
config_item *src,
count += 2; /* color_matching, NULL */
*class_array = kcalloc(count, sizeof(void *), GFP_KERNEL);
if (!*class_array) {
-   ret = PTR_ERR(*class_array);
+   ret = -ENOMEM;
goto unlock;
}

--
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



--
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

Re: [patch 5/6] usb: gadget: uvc: make a bunch of stuff static

[Andrzej Pietrasiewicz]

W dniu 14.01.2015 o 23:45, Felipe Balbi pisze:

On Thu, Jan 15, 2015 at 12:03:52AM +0300, Dan Carpenter wrote:

Sparse rightly complains that these things should be static since they
are only used in the one .c file.




There has already been a patch for this:

http://www.spinics.net/lists/linux-usb/msg119646.html



Signed-off-by: Dan Carpenter 


fails to apply on top of my testing/next

checking file drivers/usb/gadget/function/uvc_configfs.c
Hunk #1 FAILED at 43.
Hunk #2 FAILED at 135.
Hunk #3 FAILED at 161.
Hunk #4 FAILED at 718.
Hunk #5 FAILED at 795.
Hunk #6 FAILED at 947.
Hunk #7 FAILED at 973.
Hunk #8 FAILED at 1017.
Hunk #9 FAILED at 1260.
Hunk #10 FAILED at 1311.
Hunk #11 FAILED at 1334.
Hunk #12 FAILED at 1544.
Hunk #13 FAILED at 1582.
Hunk #14 FAILED at 1606.
Hunk #15 FAILED at 1757.
Hunk #16 FAILED at 1789.
16 out of 16 hunks FAILED

please rebase on that branch.



--
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

Re: Is g_multi with g_hid possible?

[Andrzej Pietrasiewicz]

W dniu 13.01.2015 o 16:10, Felipe Balbi pisze:

Hi,

On Tue, Jan 13, 2015 at 07:05:37AM -0800, Chris McClimans wrote:

Is it possible to appear as a USB gadget hid (kb + mouse),
mass_storage, and ethernet at the same time?


yes, that's called a composite device.


My goal is to try and create a device that when attached during boot
to a PC would be able to send keys as a hid keyboard to select booting
from a USB mass storage.

Booting from the mass storage would then load ipxe and perform dhcp
over the USB RDNIS device. The nic would probably be bridged to wifi,
or at least connected to some type of boot control process.

It has been necessary to unload g_multi in order to load g_hid on the
Edison on 3.10.17 so far and wanted to make sure the end goal was
possible at all.


Have a look at libusbg, you need to cook up your own gadget. g_multi
won't help you here.



To elaborate a bit more on Felipe's answer:

g_multi is an example of a "legacy" gadget; legacy gadgets'
composition of functions (like hid, mass storage, ethernet)
is more or less hardcoded into them and you cannot do
anything about that.

If you want a composition which is not covered by
any of the legacy gadgets, you need to create your own.
Your options for doing that are:

1) Create another "legacy"-style gadget; it is not
that difficult if you have some experience with
gadgets from the kernel side. You don't get
community support in this case and chances of
merging another "legacy"-style gadget into
upstream kernel are rather low unless you have
very good arguments but I can't think of any to
be honest.

2) Compose your gadget using configfs. This feature
has been in the kernel since late 2012 and as of now
all the functions available as
drivers/usb/gadget/function/f_xyz.c are available
for composition with configfs (with uvc being merged
in 3.20). While composing your gadget manually
(using shell) is a perfectly legal thing to do,
the bare configfs interface is intended primarily
for dedicated userspace tools and the tool you
want to look at is libusbg:

https://github.com/libusbg/libusbg

Please note that often the bleeding edge of development
can be found at:

https://github.com/kopasiak/libusbg

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

Re: [PATCH next] usb: gadget: uvc: to_uvcg_control_header() can be static

[Andrzej Pietrasiewicz]

W dniu 13.01.2015 o 09:55, kbuild test robot pisze:

drivers/usb/gadget/function/uvc_configfs.c:46:28: sparse: symbol 
'to_uvcg_control_header' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:138:25: sparse: symbol 
'uvcg_control_header_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:164:6: sparse: symbol 
'uvcg_control_header_drop' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:721:20: sparse: symbol 
'to_uvcg_format' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:798:30: sparse: symbol 
'to_uvcg_streaming_header' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:950:25: sparse: symbol 
'uvcg_streaming_header_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:976:6: sparse: symbol 
'uvcg_streaming_header_drop' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1020:19: sparse: symbol 
'to_uvcg_frame' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1265:25: sparse: symbol 
'uvcg_frame_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1315:6: sparse: symbol 
'uvcg_frame_drop' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1338:26: sparse: symbol 
'to_uvcg_uncompressed' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1548:25: sparse: symbol 
'uvcg_uncompressed_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1586:6: sparse: symbol 
'uvcg_uncompressed_drop' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1610:19: sparse: symbol 
'to_uvcg_mjpeg' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1761:25: sparse: symbol 
'uvcg_mjpeg_type' was not declared. Should it be static?
drivers/usb/gadget/function/uvc_configfs.c:1793:6: sparse: symbol 
'uvcg_mjpeg_drop' was not declared. Should it be static?

Signed-off-by: Fengguang Wu 


Reviewed-by: Andrzej Pietrasiewicz 



---
  uvc_configfs.c |   32 
  1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/gadget/function/uvc_configfs.c 
b/drivers/usb/gadget/function/uvc_configfs.c
index 33d92ab..f69f47a 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -43,7 +43,7 @@ struct uvcg_control_header {
unsignedlinked;
  };

-struct uvcg_control_header *to_uvcg_control_header(struct config_item *item)
+static struct uvcg_control_header *to_uvcg_control_header(struct config_item 
*item)
  {
return container_of(item, struct uvcg_control_header, item);
  }
@@ -135,7 +135,7 @@ static struct configfs_attribute 
*uvcg_control_header_attrs[] = {
NULL,
  };

-struct config_item_type uvcg_control_header_type = {
+static struct config_item_type uvcg_control_header_type = {
.ct_item_ops= &uvcg_control_header_item_ops,
.ct_attrs   = uvcg_control_header_attrs,
.ct_owner   = THIS_MODULE,
@@ -161,7 +161,7 @@ static struct config_item *uvcg_control_header_make(struct 
config_group *group,
return &h->item;
  }

-void uvcg_control_header_drop(struct config_group *group,
+static void uvcg_control_header_drop(struct config_group *group,
  struct config_item *item)
  {
struct uvcg_control_header *h = to_uvcg_control_header(item);
@@ -718,7 +718,7 @@ struct uvcg_format {
__u8bmaControls[UVCG_STREAMING_CONTROL_SIZE];
  };

-struct uvcg_format *to_uvcg_format(struct config_item *item)
+static struct uvcg_format *to_uvcg_format(struct config_item *item)
  {
return container_of(to_config_group(item), struct uvcg_format, group);
  }
@@ -795,7 +795,7 @@ struct uvcg_streaming_header {
unsignednum_fmt;
  };

-struct uvcg_streaming_header *to_uvcg_streaming_header(struct config_item 
*item)
+static struct uvcg_streaming_header *to_uvcg_streaming_header(struct 
config_item *item)
  {
return container_of(item, struct uvcg_streaming_header, item);
  }
@@ -947,7 +947,7 @@ static struct configfs_attribute 
*uvcg_streaming_header_attrs[] = {
NULL,
  };

-struct config_item_type uvcg_streaming_header_type = {
+static struct config_item_type uvcg_streaming_header_type = {
.ct_item_ops= &uvcg_streaming_header_item_ops,
.ct_attrs   = uvcg_streaming_header_attrs,
.ct_owner   = THIS_MODULE,
@@ -973,7 +973,7 @@ static struct config_item
return &h->item;
  }

-void uvcg_streaming_

Re: USB HID Gadget Support for Intel Edison

[Andrzej Pietrasiewicz]

W dniu 08.01.2015 o 18:09, Felipe Balbi pisze:

Hi,

On Thu, Jan 08, 2015 at 09:05:24AM -0800, Chris McClimans wrote:

I'm trying to get the g_hid module working with the Intel Edison.

I tried just compiling intel's patch(1) to 3.10.17 with
CONFIG_USB_GADGETFS=m CONFIG_USB_G_HID=m but I get an error trying to
load the module:

modprobe: ERROR: could not insert 'g_hid': No such device

According to https://www.kernel.org/doc/Documentation/usb/gadget_hid.txt:



In the very same file there is an example platform driver; well, at least
the most important hid-specific parts of it. You must add the usual
module boilerplate code and in module's init do platform_device_register(),
while in module's exit do platform_device_unregister().

You might also want to have a look at a configfs-composed gadget,
where the hid function does not require creating nor registering any
platform devices; the report descriptors are passed through
a configfs attribute (file). For more information you can have
a look here:

http://www.spinics.net/lists/linux-usb/msg116980.html

and here:

http://www.spinics.net/lists/linux-usb/msg118705.html

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

Re: State of gadget driver gadgetfs and functionfs

[Andrzej Pietrasiewicz]

W dniu 18.12.2014 o 09:16, Mario Schuknecht pisze:

Hi,

we use gadget driver gadgetfs and implement a high-level protocol over USB.
Our hardware provides an USB WCID device [1]. But the recent state of gadgetfs
is unclear. There are a couple of problems and gadgetfs does not support USB3
SuperSpeed. I guess the driver will be removed from kernel. Is this correct?

I digged around and found functionfs which has similar functions. And I found
notes [2] that it is not to hard to convert a program from gadgetfs to
functionfs.


I _think_ there are still some users of gadgetfs around, but I guess the
way to go is to use FunctionFS.



I looked at tools/usb/ffs-test.c implementation and in file f_fs.c.
If I correctly understand the driver, then the functionfs driver gets USB
descriptors only at the begining.
But I do not understand how I can add WCID specific descriptors. E.g.

- OS String Descriptor [3]
- ID Feature Descriptor [4]
- Extended Properties Feature Descriptor [5]

Can someone show me an example or give me a hint how I can add this descriptors
to functionfs driver?


This series:
http://www.spinics.net/lists/linux-usb/msg110324.html

adds support for OS descriptors (OS String, Extended Compatibility and
Extended Properties).

Use of OS descriptors is enabled for gadgets composed with configfs.
For general information about composing gadgets with configfs you
can for example look at:
http://events.linuxfoundation.org/sites/events/files/slides/LinuxConNA-Make-your-own-USB-gadget-Andrzej.Pietrasiewicz.pdf

In gadget's root directory there is os_desc subdirectory,
where OS string can be specified and enabled.
In each function's directory there are corresponding subdirectories,
where Extended Properties and Extended Compatibility can be
specified, but FunctionFS is special and does not follow this
rule: all descriptors are passed through the ep0 special file.

In order to pass Extended Properties/Extended Compatibility
descriptors with FunctionFS, you must use FUNCTIONFS_DESCRIPTORS_MAGIC_V2
in the header. You should do it anyway, because since 3.14
FUNCTIONFS_DESCRIPTORS_MAGIC is deprecated.

You can find the layout of the descriptors in
include/uapi/linux/usb/functionfs.h

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

[PATCHv3 11/19] Documentation: usb: NCM function testing

[Andrzej Pietrasiewicz]

Summary of how to test NCM function of USB gadget.

Signed-off-by: Andrzej Pietrasiewicz 
---
 Documentation/usb/gadget-testing.txt | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/Documentation/usb/gadget-testing.txt 
b/Documentation/usb/gadget-testing.txt
index c2f148d..493f79b 100644
--- a/Documentation/usb/gadget-testing.txt
+++ b/Documentation/usb/gadget-testing.txt
@@ -10,6 +10,7 @@ provided by gadgets.
 7. LOOPBACK function
 8. MASS STORAGE function
 9. MIDI function
+10. NCM function
 
 
 1. ACM function
@@ -366,3 +367,36 @@ $ aconnect 24:0 128:0 # try it on the host
 After the gadget's MIDI port is connected to timidity's MIDI port,
 whatever is played at the gadget side with aplaymidi -l is audible
 in host's speakers/headphones.
+
+10. NCM function
+
+
+The function is provided by usb_f_ncm.ko module.
+
+Function-specific configfs interface
+
+
+The function name to use when creating the function directory is "ncm".
+The NCM function provides these attributes in its function directory:
+
+   ifname  - network device interface name associated with this
+   function instance
+   qmult   - queue length multiplier for high and super speed
+   host_addr   - MAC address of host's end of this
+   Ethernet over USB link
+   dev_addr- MAC address of device's end of this
+   Ethernet over USB link
+
+and after creating the functions/ncm. they contain default
+values: qmult is 5, dev_addr and host_addr are randomly selected.
+Except for ifname they can be written to until the function is linked to a
+configuration. The ifname is read-only and contains the name of the interface
+which was assigned by the net core, e. g. usb0.
+
+Testing the NCM function
+
+
+Configure IP addresses of the device and the host. Then:
+
+On the device: ping 
+On the host: ping 
-- 
1.9.1

--
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

[PATCHv3 04/19] Documentation: usb: ECM subset function testing

[Andrzej Pietrasiewicz]

Summary of how to test ECM subset function of USB gadget.

Signed-off-by: Andrzej Pietrasiewicz 
---
 Documentation/usb/gadget-testing.txt | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/Documentation/usb/gadget-testing.txt 
b/Documentation/usb/gadget-testing.txt
index 7df8785..b40db75 100644
--- a/Documentation/usb/gadget-testing.txt
+++ b/Documentation/usb/gadget-testing.txt
@@ -3,6 +3,7 @@ provided by gadgets.
 
 1. ACM function
 2. ECM function
+3. ECM subset function
 
 
 1. ACM function
@@ -66,3 +67,36 @@ Configure IP addresses of the device and the host. Then:
 
 On the device: ping 
 On the host: ping 
+
+3. ECM subset function
+==
+
+The function is provided by usb_f_ecm_subset.ko module.
+
+Function-specific configfs interface
+
+
+The function name to use when creating the function directory is "geth".
+The ECM subset function provides these attributes in its function directory:
+
+   ifname  - network device interface name associated with this
+   function instance
+   qmult   - queue length multiplier for high and super speed
+   host_addr   - MAC address of host's end of this
+   Ethernet over USB link
+   dev_addr- MAC address of device's end of this
+   Ethernet over USB link
+
+and after creating the functions/ecm. they contain default
+values: qmult is 5, dev_addr and host_addr are randomly selected.
+Except for ifname they can be written to until the function is linked to a
+configuration. The ifname is read-only and contains the name of the interface
+which was assigned by the net core, e. g. usb0.
+
+Testing the ECM subset function
+---
+
+Configure IP addresses of the device and the host. Then:
+
+On the device: ping 
+On the host: ping 
-- 
1.9.1

--
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

[PATCHv3 17/19] Documentation: usb: UAC1 function testing

[Andrzej Pietrasiewicz]

Summary of how to test UAC1 function of USB gadget.

Signed-off-by: Andrzej Pietrasiewicz 
---
 Documentation/usb/gadget-testing.txt | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/Documentation/usb/gadget-testing.txt 
b/Documentation/usb/gadget-testing.txt
index 73a5b0c..40d22d8 100644
--- a/Documentation/usb/gadget-testing.txt
+++ b/Documentation/usb/gadget-testing.txt
@@ -16,6 +16,7 @@ provided by gadgets.
 13. RNDIS function
 14. SERIAL function
 15. SOURCESINK function
+16. UAC1 function
 
 
 1. ACM function
@@ -587,3 +588,29 @@ device: run the gadget
 host: test-usb
 
 http://www.linux-usb.org/usbtest/testusb.c
+
+16. UAC1 function
+=
+
+The function is provided by usb_f_uac1.ko module.
+
+Function-specific configfs interface
+
+
+The function name to use when creating the function directory is "uac1".
+The uac1 function provides these attributes in its function directory:
+
+   audio_buf_size - audio buffer size
+   fn_cap - capture pcm device file name
+   fn_cntl - control device file name
+   fn_play - playback pcm device file name
+   req_buf_size - ISO OUT endpoint request buffer size
+   req_count - ISO OUT endpoint request count
+
+The attributes have sane default values.
+
+Testing the UAC1 function
+-
+
+device: run the gadget
+host: aplay -l # should list our USB Audio Gadget
-- 
1.9.1

--
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

[PATCHv3 10/19] Documentation: usb: MIDI function testing

[Andrzej Pietrasiewicz]

Summary of how to test MIDI function of USB gadget.

Signed-off-by: Andrzej Pietrasiewicz 
---
 Documentation/usb/gadget-testing.txt | 84 
 1 file changed, 84 insertions(+)

diff --git a/Documentation/usb/gadget-testing.txt 
b/Documentation/usb/gadget-testing.txt
index 01b9ffe..c2f148d 100644
--- a/Documentation/usb/gadget-testing.txt
+++ b/Documentation/usb/gadget-testing.txt
@@ -9,6 +9,7 @@ provided by gadgets.
 6. HID function
 7. LOOPBACK function
 8. MASS STORAGE function
+9. MIDI function
 
 
 1. ACM function
@@ -282,3 +283,86 @@ Testing the MASS STORAGE function
 device: connect the gadget, enable it
 host: dmesg, see the USB drives appear (if system configured to automatically
 mount)
+
+9. MIDI function
+
+
+The function is provided by usb_f_midi.ko module.
+
+Function-specific configfs interface
+
+
+The function name to use when creating the function directory is "midi".
+The MIDI function provides these attributes in its function directory:
+
+   buflen  - MIDI buffer length
+   id  - ID string for the USB MIDI adapter
+   in_ports- number of MIDI input ports
+   index   - index value for the USB MIDI adapter
+   out_ports   - number of MIDI output ports
+   qlen- USB read request queue length
+
+Testing the MIDI function
+-
+
+There are two cases: playing a mid from the gadget to
+the host and playing a mid from the host to the gadget.
+
+1) Playing a mid from the gadget to the host
+host)
+
+$ arecordmidi -l
+ PortClient name  Port name
+ 14:0Midi Through Midi Through Port-0
+ 24:0MIDI Gadget  MIDI Gadget MIDI 1
+$ arecordmidi -p 24:0 from_gadget.mid
+
+gadget)
+
+$ aplaymidi -l
+ PortClient name  Port name
+ 20:0f_midi   f_midi
+
+$ aplaymidi -p 20:0 to_host.mid
+
+2) Playing a mid from the host to the gadget
+gadget)
+
+$ arecordmidi -l
+ PortClient name  Port name
+ 20:0f_midi   f_midi
+
+$ arecordmidi -p 20:0 from_host.mid
+
+host)
+
+$ aplaymidi -l
+ PortClient name  Port name
+ 14:0Midi Through Midi Through Port-0
+ 24:0MIDI Gadget  MIDI Gadget MIDI 1
+
+$ aplaymidi -p24:0 to_gadget.mid
+
+The from_gadget.mid should sound identical to the to_host.mid.
+The from_host.id should sound identical to the to_gadget.mid.
+
+MIDI files can be played to speakers/headphones with e.g. timidity installed
+
+$ aplaymidi -l
+ PortClient name  Port name
+ 14:0Midi Through Midi Through Port-0
+ 24:0MIDI Gadget  MIDI Gadget MIDI 1
+128:0TiMidity TiMidity port 0
+128:1TiMidity TiMidity port 1
+128:2TiMidity TiMidity port 2
+128:3TiMidity TiMidity port 3
+
+$ aplaymidi -p 128:0 file.mid
+
+MIDI ports can be logically connected using the aconnect utility, e.g.:
+
+$ aconnect 24:0 128:0 # try it on the host
+
+After the gadget's MIDI port is connected to timidity's MIDI port,
+whatever is played at the gadget side with aplaymidi -l is audible
+in host's speakers/headphones.
-- 
1.9.1

--
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

[PATCHv3 08/19] Documentation: usb: LOOPBACK function testing

[Andrzej Pietrasiewicz]

Summary of how to test LOOPBACK function of USB gadget.

Signed-off-by: Andrzej Pietrasiewicz 
---
 Documentation/usb/gadget-testing.txt | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/Documentation/usb/gadget-testing.txt 
b/Documentation/usb/gadget-testing.txt
index f117e5c..b13491d 100644
--- a/Documentation/usb/gadget-testing.txt
+++ b/Documentation/usb/gadget-testing.txt
@@ -7,6 +7,7 @@ provided by gadgets.
 4. EEM function
 5. FFS function
 6. HID function
+7. LOOPBACK function
 
 
 1. ACM function
@@ -205,3 +206,25 @@ $ ./hid_gadget_test /dev/hidg0 keyboard
 
 Host:
 - observe the keystrokes from the gadget
+
+7. LOOPBACK function
+
+
+The function is provided by usb_f_ss_lb.ko module.
+
+Function-specific configfs interface
+
+
+The function name to use when creating the function directory is "Loopback".
+The LOOPBACK function provides these attributes in its function directory:
+
+   qlen- depth of loopback queue
+   bulk_buflen - buffer length
+
+Testing the LOOPBACK function
+-
+
+device: run the gadget
+host: test-usb
+
+http://www.linux-usb.org/usbtest/testusb.c
-- 
1.9.1

--
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

[PATCHv3 12/19] Documentation: usb: OBEX function testing

[Andrzej Pietrasiewicz]

Summary of how to test OBEX function of USB gadget.

Signed-off-by: Andrzej Pietrasiewicz 
---
 Documentation/usb/gadget-testing.txt | 29 +
 1 file changed, 29 insertions(+)

diff --git a/Documentation/usb/gadget-testing.txt 
b/Documentation/usb/gadget-testing.txt
index 493f79b..c8ae540 100644
--- a/Documentation/usb/gadget-testing.txt
+++ b/Documentation/usb/gadget-testing.txt
@@ -11,6 +11,7 @@ provided by gadgets.
 8. MASS STORAGE function
 9. MIDI function
 10. NCM function
+11. OBEX function
 
 
 1. ACM function
@@ -400,3 +401,31 @@ Configure IP addresses of the device and the host. Then:
 
 On the device: ping 
 On the host: ping 
+
+11. OBEX function
+=
+
+The function is provided by usb_f_obex.ko module.
+
+Function-specific configfs interface
+
+
+The function name to use when creating the function directory is "obex".
+The OBEX function provides just one attribute in its function directory:
+
+   port_num
+
+The attribute is read-only.
+
+There can be at most 4 ACM/generic serial/OBEX ports in the system.
+
+Testing the OBEX function
+-
+
+On device: seriald -f /dev/ttyGS -s 1024
+On host: serialc -v  -p  -i -a1 -s1024 \
+ -t -r
+
+where seriald and serialc are Felipe's utilities found here:
+
+https://git.gitorious.org/usb/usb-tools.git master
-- 
1.9.1

--
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

[PATCHv3 05/19] Documentation: usb: EEM function testing

[Andrzej Pietrasiewicz]

Summary of how to test EEM function of USB gadget.

Signed-off-by: Andrzej Pietrasiewicz 
---
 Documentation/usb/gadget-testing.txt | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/Documentation/usb/gadget-testing.txt 
b/Documentation/usb/gadget-testing.txt
index b40db75..50b0a6c 100644
--- a/Documentation/usb/gadget-testing.txt
+++ b/Documentation/usb/gadget-testing.txt
@@ -4,6 +4,7 @@ provided by gadgets.
 1. ACM function
 2. ECM function
 3. ECM subset function
+4. EEM function
 
 
 1. ACM function
@@ -100,3 +101,36 @@ Configure IP addresses of the device and the host. Then:
 
 On the device: ping 
 On the host: ping 
+
+4. EEM function
+===
+
+The function is provided by usb_f_eem.ko module.
+
+Function-specific configfs interface
+
+
+The function name to use when creating the function directory is "eem".
+The EEM function provides these attributes in its function directory:
+
+   ifname  - network device interface name associated with this
+   function instance
+   qmult   - queue length multiplier for high and super speed
+   host_addr   - MAC address of host's end of this
+   Ethernet over USB link
+   dev_addr- MAC address of device's end of this
+   Ethernet over USB link
+
+and after creating the functions/eem. they contain default
+values: qmult is 5, dev_addr and host_addr are randomly selected.
+Except for ifname they can be written to until the function is linked to a
+configuration. The ifname is read-only and contains the name of the interface
+which was assigned by the net core, e. g. usb0.
+
+Testing the EEM function
+
+
+Configure IP addresses of the device and the host. Then:
+
+On the device: ping 
+On the host: ping 
-- 
1.9.1

--
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