* Andrzej Pietrasiewicz | 2012-11-22 13:06:56 [+0100]:
>insmod libcomposite.ko
>insmod g_usb_functions.ko
You should use modprobe. Atleast g_usb_functions should pull in
libcomposite if requried.
>echo <some file>.img > /cfg/usb-function-gadget/G1/C1/F1/MassStorage/lun0/file
>
>Do the similar thing to other functions, then
>
>echo 1 > /cfg/usb-function-gadget/G1/ready
The upper part (left out) sounds okay. I don't comment on the names. One
thing you miss: Lets say you have C1 and C2. How do you configure the
same F1 in C1 and C2 _and_ how do you configure a different F1 in F1 and
F2 in C2. I guess the latter will work just now but the former example
won't. The former example is used by the nokia gadget, the latter by
g_serial.
>to actually probe and bind the gadget.
>
>In this case, under F1 there will be automatically created interface00
>directory with the contents similar to this:
>
>/cfg/usb-function-gadget/G1/C1/F1/interface00
>/cfg/usb-function-gadget/G1/C1/F1/interface00/altsetting
>/cfg/usb-function-gadget/G1/C1/F1/interface00/interface_class
>/cfg/usb-function-gadget/G1/C1/F1/interface00/interface_number
>/cfg/usb-function-gadget/G1/C1/F1/interface00/interface_protocol
>/cfg/usb-function-gadget/G1/C1/F1/interface00/interface_subclass
>/cfg/usb-function-gadget/G1/C1/F1/interface00/n_endpoints
>/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint02
>/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint02/attributes
>/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint02/endpoint_address
>/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint02/interval
>/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint02/max_packet_size
>/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81
>/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81/attributes
>/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81/endpoint_address
>/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81/interval
>/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81/max_packet_size
Interresting. I have no idea if I do like or I don't. Some minor things:
You do have endpoint02 and you have endpoint_address. I guess
endpoint_address here reads 2. So I think this attribute is read only.
Same goes for max_packet_size since this is usually protocol specific.
>In order to unload the gadget:
>
>echo 0 > /cfg/usb-function-gadget/G1/ready
>echo > /cfg/usb-function-gadget/G1/C1/F1/MassStorage/lun0/file
>rmdir /cfg/usb-function-gadget/G1/C1/F1/MassStorage/lun0
>rmdir /cfg/usb-function-gadget/G1/C1/F1/MassStorage
>rmdir /cfg/usb-function-gadget/G1/C1/F1
>rmdir /cfg/usb-function-gadget/G1/C1
>rmdir /cfg/usb-function-gadget/G1
>umount cfg
>rmmod g_usb_functions.ko
>rmmod f_mass_storage.ko
>rmmod libcomposite.ko
>
>Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
>Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
>---
> drivers/usb/gadget/Kconfig | 12 +
> drivers/usb/gadget/Makefile | 2 +
> drivers/usb/gadget/usb_functions.c | 1067 ++++++++++++++++++++++++++++++++++++
> drivers/usb/gadget/usb_functions.h | 100 ++++
> 4 files changed, 1181 insertions(+), 0 deletions(-)
> create mode 100644 drivers/usb/gadget/usb_functions.c
> create mode 100644 drivers/usb/gadget/usb_functions.h
>
>diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
>index 14625fd..8530279 100644
>--- a/drivers/usb/gadget/Kconfig
>+++ b/drivers/usb/gadget/Kconfig
>@@ -521,6 +521,18 @@ choice
>
> # this first set of drivers all depend on bulk-capable hardware.
>
>+config USB_FG
>+ tristate "USB Functions Gadget (EXPERIMENTAL)"
>+ select USB_LIBCOMPOSITE
>+ depends on EXPERIMENTAL && CONFIGFS_FS
EXPERIMENTAL is going away. It is no longer under drivers/usb.
>+ help
>+ USB Functions Gadget is a device which aggregates a number of
>+ USB functions. The gadget is composed by userspace through a
>+ configfs interface, which enables specifying what USB
>+ configurations the gadget is composed of, what USB functions
>+ a USB configuration is composed of and enabling/disabling
>+ the gadget.
It would be nice to point to Documentation/usb/file-with-examples.txt
>diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
>index fef41f5..09faa34 100644
>--- a/drivers/usb/gadget/Makefile
>+++ b/drivers/usb/gadget/Makefile
>@@ -38,6 +38,7 @@ obj-$(CONFIG_USB_MV_U3D) += mv_u3d_core.o
> #
> # USB gadget drivers
> #
>+g_usb_functions-y := usb_functions.o
> g_zero-y := zero.o
> g_audio-y := audio.o
> g_ether-y := ether.o
>@@ -56,6 +57,7 @@ g_ncm-y := ncm.o
> g_acm_ms-y := acm_ms.o
> g_tcm_usb_gadget-y := tcm_usb_gadget.o
>
>+obj-$(CONFIG_USB_FG) += g_usb_functions.o
Don't use this indirection and I don't see a reason why you shouldn't
drop the g_ at the beginning. After all this will not be a gadget but an
interface to create gadgets.
> obj-$(CONFIG_USB_ZERO) += g_zero.o
> obj-$(CONFIG_USB_AUDIO) += g_audio.o
> obj-$(CONFIG_USB_ETH) += g_ether.o
>diff --git a/drivers/usb/gadget/usb_functions.c b/drivers/usb/gadget/usb_functions.c
>new file mode 100644
>index 0000000..93d8345
>--- /dev/null
>+++ b/drivers/usb/gadget/usb_functions.c
>@@ -0,0 +1,1067 @@
>+/*
>+ * USB Functions Gadget
>+ *
>+ * Copyright (C) 2012 Samsung Electronics
>+ * Author: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
>+ *
>+ * This software is licensed under the terms of the GNU General Public
>+ * License version 2, as published by the Free Software Foundation, and
>+ * may be copied, distributed, and modified under those terms.
>+ *
>+ * This program is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+ * GNU General Public License for more details.
>+ *
>+ */
>+
>+#include <linux/kernel.h>
>+#include <linux/module.h>
>+#include <linux/usb/ch9.h>
>+#include <linux/usb/composite.h>
>+#include <linux/configfs.h>
>+#include <linux/dcache.h>
>+#include <linux/fs.h>
>+
>+#include "usb_functions.h"
>+
>+/*-------------------------------------------------------------------------*/
>+
>+#define DRIVER_DESC "USB Functions Gadget"
This define can go.
>+
>+USB_GADGET_COMPOSITE_OPTIONS();
No, no modules parameters please. This will be configsfs only.
>+
>+/*----------- helper functions and macros for configfs support ----------*/
>+
>+#define UFG_SHOW_USHORT_ATTR(_name, _type, _member) \
>+static ssize_t _type##_show_##_name(struct _type *grp, char *buf) \
>+{ \
>+ return sprintf(buf, "%d\n", grp->_member); \
>+}
>+
>+#define UFG_STORE_USHORT_ATTR(_name, _type, _member) \
>+static ssize_t _type##_store_##_name(struct _type *grp, \
>+ const char *buf, size_t count) \
>+{ \
>+ u16 tmp; \
>+ int res; \
>+ char *p = (char *)buf; \
>+ \
>+ res = kstrtou16(p, 16, &tmp); \
>+ if (res < 0) \
>+ return res; \
>+ grp->_member = (ushort)tmp; \
>+ grp->_member##_set = true; \
>+ \
>+ return count; \
>+}
Please try to keep the \ and the end of line in a row.
>+
>+#define UFG_SHOW_STR_ATTR(_name, _type, _member) \
>+static ssize_t _type##_show_##_name(struct _type *grp, char *buf) \
>+{ \
>+ return strlcpy(buf, grp->_member, UFG_STR_LEN); \
>+}
>+
>+#define UFG_STORE_STR_ATTR(_name, _type, _member) \
>+static ssize_t _type##_store_##_name(struct _type *grp, \
>+ const char *buf, size_t count) \
>+{ \
>+ const char *end = strchr(buf, '\n'); \
>+ \
>+ if (!end) \
>+ end = buf + count; \
>+ if (end - buf >= sizeof(grp->_member)) \
>+ return -EINVAL; \
>+ memcpy(grp->_member, buf, end - buf); \
>+ grp->_member[end - buf] = 0; \
>+ grp->_member##_set = true; \
>+ \
>+ return count; \
>+}
>+
>+#define UFG_ATTR_RW(_name, _attr, _type) \
>+static struct _type##_attribute _type##_##_name = \
>+ __CONFIGFS_ATTR(_attr, S_IRUGO | S_IWUSR, _type##_show_##_name, \
>+ _type##_store_##_name)
>+
>+#define UFG_ATTR_RO(_name, _attr, _type) \
>+static struct _type##_attribute _type##_##_name = \
>+ __CONFIGFS_ATTR(_attr, S_IRUGO | S_IWUSR, _type##_show_##_name, NULL)
>+
>+#define ufg_hdr(ptr) container_of((ptr), struct ufg_grp_hdr, group)
>+
>+/*-------------------- handling of dynamic make_group --------------------*/
>+
>+static struct config_group *make_ufg_interface(struct config_group *group,
>+ const char *name);
>+
>+static struct config_group *ufg_make_function_subgroup(
>+ struct config_group *group, const char *name)
>+{
>+ struct ufg_function_grp *f_grp;
>+ struct config_group *result;
>+
>+ /*
>+ * TODO:
>+ *
>+ * This makes "interface" an illegal name for a function.
>+ */
>+ if (!strncmp(name, "interface", 9)) {
Can't you use something else to match between function & interface?
After all you know that gadget's parent has to be a "config"-folder,
followed by an interface, followed by the function.
>+ result = make_ufg_interface(group, name);
>+
>+ if (result) {
>+ struct ufg_grp_hdr *h;
>+
>+ h = container_of(result, struct ufg_grp_hdr, group);
>+ h->type = UFG_INTERFACE;
>+ }
>+
>+ return result;
>+ }
>+
>+ f_grp = container_of(group, struct ufg_function_grp, group);
>+ if (IS_ERR_OR_NULL(f_grp->f))
>+ return ERR_PTR(-ENODEV);
>+ result = f_grp->f->make_group(group, name);
>+
>+ if (!IS_ERR_OR_NULL(result)) {
>+ struct ufg_grp_hdr *h;
>+
>+ h = ufg_hdr(result);
>+ h->type = UFG_FUNCTION;
>+ }
>+
>+ return result;
>+}
>+
>+/*------------------ USB function-level configfs group ------------------*/
>+
>+UFG_SHOW_STR_ATTR(name, ufg_function_grp, name);
>+
>+static ssize_t ufg_function_grp_store_name(struct ufg_function_grp *grp,
>+ const char *buf, size_t count)
>+{
>+ int rc;
>+ struct dentry *parent, *new;
>+ struct qstr name;
>+ char name_buf[UFG_STR_LEN];
Try to put the complex types and the top, the simple at the bottom.
>+
>+ if (count >= sizeof(name_buf))
>+ return -ENODEV;
>+
>+ sscanf(buf, "%s", name_buf);
>+ grp->f = usb_get_function(name_buf);
>+ if (IS_ERR_OR_NULL(grp->f))
>+ return -ENODEV;
>+
>+ name.name = name_buf;
>+ name.len = strlen(name_buf);
>+ name.hash = full_name_hash(name.name, name.len);
>+
>+ parent = grp->group.cg_item.ci_dentry;
>+ new = d_alloc(parent, &name);
>+ if (IS_ERR_OR_NULL(new))
Why err_or_null? d_alloc() never returns an ERR ptr.
>+ return -ENOMEM;
>+ d_add(new, NULL);
>+ rc = ufg_mkdir(parent, new);
>+ if (rc) {
>+ d_drop(new);
>+ dput(new);
>+
>+ return rc;
>+ }
>+ dput(new); /* make the refcount 1 */
Looking at this: full_name_hash(), d_alloc(), d_add(), d_drop(), dput().
Do you write a filesystem? I don't see any one of this functions beeing
use in drivers/target? Why do you need them? I think you do something
more complicated than it needs to be because target made it without it.
>+
>+ strlcpy(grp->name, buf, UFG_FUNC_NAME_LEN);
>+
>+ return count;
>+}
>+
>+CONFIGFS_ATTR_STRUCT(ufg_function_grp);
>+
>+UFG_ATTR_RW(name, name, ufg_function_grp);
>+
>+static struct configfs_attribute *ufg_function_grp_attrs[] = {
>+ &ufg_function_grp_name.attr,
>+ NULL,
>+};
>+
>+static struct ufg_function_grp *to_ufg_function_grp(struct config_item *item)
>+{
>+ return item ? container_of(to_config_group(item),
>+ struct ufg_function_grp, group) : NULL;
>+}
>+
>+CONFIGFS_ATTR_OPS(ufg_function_grp);
>+
>+static void ufg_function_grp_release(struct config_item *item)
>+{
>+ kfree(to_ufg_function_grp(item));
>+}
>+
>+
>+static struct configfs_item_operations ufg_function_grp_item_ops = {
>+ .show_attribute = ufg_function_grp_attr_show,
>+ .store_attribute = ufg_function_grp_attr_store,
>+ .release = ufg_function_grp_release,
>+};
>+
>+static struct configfs_group_operations ufg_function_grp_group_ops = {
>+ .make_group = ufg_make_function_subgroup,
>+};
>+
>+static struct config_item_type ufg_function_grp_type = {
>+ .ct_attrs = ufg_function_grp_attrs,
>+ .ct_item_ops = &ufg_function_grp_item_ops,
>+ .ct_group_ops = &ufg_function_grp_group_ops,
>+ .ct_owner = THIS_MODULE,
>+};
>+
>+static struct config_group *make_ufg_function(struct config_group *group,
>+ const char *name)
>+{
>+ struct ufg_function_grp *function;
>+
>+ function = kzalloc(sizeof(*function), GFP_KERNEL);
>+ if (!function)
>+ return ERR_PTR(-ENOMEM);
>+
>+ config_group_init_type_name(&function->group, name,
>+ &ufg_function_grp_type);
>+
>+ return &function->group;
>+}
>+
>+/*------------------ USB endpoint-level configfs group ------------------*/
>+
>+struct ufg_endpoint_grp {
>+ /* This group needs children */
>+ struct config_group group;
>+
>+ /* attributes' values */
>+ int endpoint_address;
>+ int attributes;
>+ int max_packet_sz;
>+ int interval;
>+};
>+
>+UFG_SHOW_USHORT_ATTR(endpoint_address, ufg_endpoint_grp, endpoint_address);
>+UFG_SHOW_USHORT_ATTR(attributes, ufg_endpoint_grp, attributes);
>+UFG_SHOW_USHORT_ATTR(max_packet_sz, ufg_endpoint_grp, max_packet_sz);
>+UFG_SHOW_USHORT_ATTR(interval, ufg_endpoint_grp, interval);
>+
>+CONFIGFS_ATTR_STRUCT(ufg_endpoint_grp);
>+
>+UFG_ATTR_RO(endpoint_address, endpoint_address, ufg_endpoint_grp);
>+UFG_ATTR_RO(attributes, attributes, ufg_endpoint_grp);
>+UFG_ATTR_RO(max_packet_sz, max_packet_size, ufg_endpoint_grp);
>+UFG_ATTR_RO(interval, interval, ufg_endpoint_grp);
>+
>+static struct configfs_attribute *ufg_endpoint_grp_attrs[] = {
>+ &ufg_endpoint_grp_endpoint_address.attr,
>+ &ufg_endpoint_grp_attributes.attr,
>+ &ufg_endpoint_grp_max_packet_sz.attr,
>+ &ufg_endpoint_grp_interval.attr,
>+ NULL,
>+};
>+
>+static struct ufg_endpoint_grp *to_ufg_endpoint_grp(struct config_item *item)
>+{
>+ return item ? container_of(to_config_group(item),
>+ struct ufg_endpoint_grp, group) : NULL;
>+}
>+
>+CONFIGFS_ATTR_OPS(ufg_endpoint_grp);
>+
>+static void ufg_endpoint_grp_release(struct config_item *item)
>+{
>+ kfree(to_ufg_endpoint_grp(item));
>+}
>+
>+
>+static struct configfs_item_operations ufg_endpoint_grp_item_ops = {
>+ .show_attribute = ufg_endpoint_grp_attr_show,
>+ //.store_attribute = ufg_endpoint_grp_attr_store,
>+ .release = ufg_endpoint_grp_release,
>+};
>+
>+static struct configfs_group_operations ufg_endpoint_grp_group_ops = {
>+ .make_group = NULL, /* TODO: implement a make endpoint group */
>+};
>+
>+static struct config_item_type ufg_endpoint_grp_type = {
>+ .ct_attrs = ufg_endpoint_grp_attrs,
>+ .ct_item_ops = &ufg_endpoint_grp_item_ops,
>+ .ct_group_ops = &ufg_endpoint_grp_group_ops,
>+ .ct_owner = THIS_MODULE,
>+};
>+
>+static struct config_group *make_ufg_endpoint(struct config_group *group,
>+ const char *name)
>+{
>+ struct ufg_endpoint_grp *endpoint;
>+
>+ endpoint = kzalloc(sizeof(*endpoint), GFP_KERNEL);
>+ if (!endpoint)
>+ return ERR_PTR(-ENOMEM);
>+
>+ config_group_init_type_name(&endpoint->group, name,
>+ &ufg_endpoint_grp_type);
>+
>+ return &endpoint->group;
>+}
>+
>+/*------------------ USB interface-level configfs group ------------------*/
>+
>+struct ufg_interface_grp {
>+ /* This group needs children */
>+ struct config_group group;
>+
>+ enum ufg_hdr_type type;
>+
>+ /* attributes' values */
>+ int interface_nr;
>+ int alt_setting;
>+ int num_endpoints;
>+ int intf_class;
>+ int intf_subclass;
>+ int intf_protocol;
>+};
>+
>+UFG_SHOW_USHORT_ATTR(interface_nr, ufg_interface_grp, interface_nr);
>+UFG_SHOW_USHORT_ATTR(alt_setting, ufg_interface_grp, alt_setting);
>+UFG_SHOW_USHORT_ATTR(num_endpoints, ufg_interface_grp, num_endpoints);
>+UFG_SHOW_USHORT_ATTR(intf_class, ufg_interface_grp, intf_class);
>+UFG_SHOW_USHORT_ATTR(intf_subclass, ufg_interface_grp, intf_subclass);
>+UFG_SHOW_USHORT_ATTR(intf_protocol, ufg_interface_grp, intf_protocol);
>+
>+CONFIGFS_ATTR_STRUCT(ufg_interface_grp);
>+
>+UFG_ATTR_RO(interface_nr, interface_number, ufg_interface_grp);
>+UFG_ATTR_RO(alt_setting, altsetting, ufg_interface_grp);
>+UFG_ATTR_RO(num_endpoints, n_endpoints, ufg_interface_grp);
>+UFG_ATTR_RO(intf_class, interface_class, ufg_interface_grp);
>+UFG_ATTR_RO(intf_subclass, interface_subclass, ufg_interface_grp);
>+UFG_ATTR_RO(intf_protocol, interface_protocol, ufg_interface_grp);
>+
>+static struct configfs_attribute *ufg_interface_grp_attrs[] = {
>+ &ufg_interface_grp_interface_nr.attr,
>+ &ufg_interface_grp_alt_setting.attr,
>+ &ufg_interface_grp_num_endpoints.attr,
>+ &ufg_interface_grp_intf_class.attr,
>+ &ufg_interface_grp_intf_subclass.attr,
>+ &ufg_interface_grp_intf_protocol.attr,
>+ NULL,
>+};
>+
>+static struct ufg_interface_grp *to_ufg_interface_grp(struct config_item *item)
>+{
>+ return item ? container_of(to_config_group(item),
>+ struct ufg_interface_grp, group) : NULL;
>+}
>+
>+CONFIGFS_ATTR_OPS(ufg_interface_grp);
>+
>+static void ufg_interface_grp_release(struct config_item *item)
>+{
>+ kfree(to_ufg_interface_grp(item));
>+}
>+
>+
>+static struct configfs_item_operations ufg_interface_grp_item_ops = {
>+ .show_attribute = ufg_interface_grp_attr_show,
>+ //.store_attribute = ufg_interface_grp_attr_store,
>+ .release = ufg_interface_grp_release,
>+};
>+
>+static struct configfs_group_operations ufg_interface_grp_group_ops = {
>+ .make_group = make_ufg_endpoint,
>+};
>+
>+static struct config_item_type ufg_interface_grp_type = {
>+ .ct_attrs = ufg_interface_grp_attrs,
>+ .ct_item_ops = &ufg_interface_grp_item_ops,
>+ .ct_group_ops = &ufg_interface_grp_group_ops,
>+ .ct_owner = THIS_MODULE,
>+};
>+
>+static struct config_group *make_ufg_interface(struct config_group *group,
>+ const char *name)
>+{
>+ struct ufg_interface_grp *interface;
>+
>+ interface = kzalloc(sizeof(*interface), GFP_KERNEL);
>+ if (!interface)
>+ return ERR_PTR(-ENOMEM);
>+
>+ interface->type = UFG_INTERFACE;
>+ config_group_init_type_name(&interface->group, name,
>+ &ufg_interface_grp_type);
>+
>+ return &interface->group;
>+}
>+
>+/*--------------- USB configuration-level configfs group ----------------*/
>+
>+struct ufg_config_grp {
>+ /* This group needs children */
>+ struct config_group group;
>+
>+ struct usb_configuration *usb_configuration;
>+
>+ /* attributes' values */
>+ ushort max_power;
>+ bool max_power_set;
>+ ushort num_interfaces;
>+ ushort conf_number;
>+};
>+
>+UFG_SHOW_USHORT_ATTR(max_power, ufg_config_grp, max_power);
>+UFG_STORE_USHORT_ATTR(max_power, ufg_config_grp, max_power);
>+
>+UFG_SHOW_USHORT_ATTR(num_interfaces, ufg_config_grp, num_interfaces);
>+
>+UFG_SHOW_USHORT_ATTR(conf_number, ufg_config_grp, conf_number);
>+
>+CONFIGFS_ATTR_STRUCT(ufg_config_grp);
>+
>+UFG_ATTR_RW(max_power, maximum_power, ufg_config_grp);
>+
>+UFG_ATTR_RO(num_interfaces, number_of_interfaces, ufg_config_grp);
>+
>+UFG_ATTR_RO(conf_number, configuration_number, ufg_config_grp);
>+
>+static struct configfs_attribute *ufg_config_grp_attrs[] = {
>+ &ufg_config_grp_max_power.attr,
>+ &ufg_config_grp_num_interfaces.attr,
>+ &ufg_config_grp_conf_number.attr,
>+ NULL,
>+};
>+
>+static struct ufg_config_grp *to_ufg_config_grp(struct config_item *item)
>+{
>+ return item ? container_of(to_config_group(item), struct ufg_config_grp,
>+ group) : NULL;
>+}
>+
>+CONFIGFS_ATTR_OPS(ufg_config_grp);
>+
>+static void ufg_config_grp_release(struct config_item *item)
>+{
>+ kfree(to_ufg_config_grp(item));
>+}
>+
>+static struct configfs_item_operations ufg_config_grp_item_ops = {
>+ .show_attribute = ufg_config_grp_attr_show,
>+ .store_attribute = ufg_config_grp_attr_store,
>+ .release = ufg_config_grp_release,
>+};
>+
>+static struct configfs_group_operations ufg_config_grp_group_ops = {
>+ .make_group = make_ufg_function,
>+};
>+
>+static struct config_item_type ufg_config_grp_type = {
>+ .ct_attrs = ufg_config_grp_attrs,
>+ .ct_item_ops = &ufg_config_grp_item_ops,
>+ .ct_group_ops = &ufg_config_grp_group_ops,
>+ .ct_owner = THIS_MODULE,
>+};
>+
>+static struct config_group *make_ufg_config(struct config_group *group,
>+ const char *name)
>+{
>+ struct ufg_config_grp *config;
>+
>+ config = kzalloc(sizeof(*config), GFP_KERNEL);
>+ if (!config)
>+ return ERR_PTR(-ENOMEM);
>+
>+ config_group_init_type_name(&config->group, name, &ufg_config_grp_type);
>+
>+ return &config->group;
>+}
>+
>+/*------------------ USB gadget-level configfs group --------------------*/
>+
>+UFG_SHOW_USHORT_ATTR(id_vendor, ufg_gadget_grp, idVendor);
>+UFG_STORE_USHORT_ATTR(id_vendor, ufg_gadget_grp, idVendor);
>+
>+UFG_SHOW_USHORT_ATTR(id_product, ufg_gadget_grp, idProduct);
>+UFG_STORE_USHORT_ATTR(id_product, ufg_gadget_grp, idProduct);
>+
>+UFG_SHOW_USHORT_ATTR(bcd_device, ufg_gadget_grp, bcdDevice);
>+UFG_STORE_USHORT_ATTR(bcd_device, ufg_gadget_grp, bcdDevice);
>+
>+UFG_SHOW_STR_ATTR(i_manufacturer, ufg_gadget_grp, iManufacturer);
>+UFG_STORE_STR_ATTR(i_manufacturer, ufg_gadget_grp, iManufacturer);
>+
>+UFG_SHOW_STR_ATTR(i_product, ufg_gadget_grp, iProduct);
>+UFG_STORE_STR_ATTR(i_product, ufg_gadget_grp, iProduct);
>+
>+UFG_SHOW_STR_ATTR(i_serial_number, ufg_gadget_grp, iSerialNumber);
>+UFG_STORE_STR_ATTR(i_serial_number, ufg_gadget_grp, iSerialNumber);
>+
>+UFG_SHOW_USHORT_ATTR(ready, ufg_gadget_grp, ready);
>+
>+static int ufg_gadget_ready(struct ufg_gadget_grp *group);
PLease no forward declaration if they can be avoided.
>+
>+static ssize_t ufg_gadget_grp_store_ready(struct ufg_gadget_grp *ufg_gadget_grp,
>+ const char *buf, size_t count)
>+{
>+ bool ready;
>+ int ret;
>+
>+ if (buf[0] != '0' && buf[0] != '1')
>+ return -EINVAL;
>+
>+ ready = ufg_gadget_grp->ready;
>+ ufg_gadget_grp->ready = buf[0] == '1';
>+
>+ if (ready && ufg_gadget_grp->ready)
>+ return -EBUSY;
>+
>+ ret = ufg_gadget_ready(ufg_gadget_grp);
>+ if (ret) {
>+ ufg_gadget_grp->ready = ready;
>+ return ret;
>+ }
>+
>+ return count;
>+}
>+
>+CONFIGFS_ATTR_STRUCT(ufg_gadget_grp);
>+
>+UFG_ATTR_RW(id_vendor, idVendor, ufg_gadget_grp);
>+UFG_ATTR_RW(id_product, idProduct, ufg_gadget_grp);
>+UFG_ATTR_RW(bcd_device, bcdDevice, ufg_gadget_grp);
>+UFG_ATTR_RW(i_manufacturer, iManufacturer, ufg_gadget_grp);
>+UFG_ATTR_RW(i_product, iProduct, ufg_gadget_grp);
>+UFG_ATTR_RW(i_serial_number, iSerialNumber, ufg_gadget_grp);
>+UFG_ATTR_RW(ready, ready, ufg_gadget_grp);
>+
>+static struct configfs_attribute *ufg_gadget_grp_attrs[] = {
>+ &ufg_gadget_grp_id_vendor.attr,
>+ &ufg_gadget_grp_id_product.attr,
>+ &ufg_gadget_grp_bcd_device.attr,
>+ &ufg_gadget_grp_i_manufacturer.attr,
>+ &ufg_gadget_grp_i_product.attr,
>+ &ufg_gadget_grp_i_serial_number.attr,
>+ &ufg_gadget_grp_ready.attr,
>+ NULL,
>+};
>+
>+CONFIGFS_ATTR_OPS(ufg_gadget_grp);
>+
>+static void ufg_gadget_grp_release(struct config_item *item)
>+{
>+ kfree(to_ufg_gadget_grp(item));
>+}
>+
>+static struct configfs_item_operations ufg_gadget_grp_item_ops = {
>+ .show_attribute = ufg_gadget_grp_attr_show,
>+ .store_attribute = ufg_gadget_grp_attr_store,
>+ .release = ufg_gadget_grp_release,
>+};
>+
>+static struct configfs_group_operations ufg_gadget_grp_group_ops = {
>+ .make_group = make_ufg_config,
>+};
>+
>+static struct config_item_type ufg_gadget_grp_type = {
>+ .ct_attrs = ufg_gadget_grp_attrs,
>+ .ct_item_ops = &ufg_gadget_grp_item_ops,
>+ .ct_group_ops = &ufg_gadget_grp_group_ops,
>+ .ct_owner = THIS_MODULE,
>+};
>+
>+static struct config_group *make_ufg_gadget(struct config_group *group,
>+ const char *name)
>+{
>+ struct ufg_gadget_grp *ufg_gadget_grp;
>+
>+ ufg_gadget_grp = kzalloc(sizeof(*ufg_gadget_grp), GFP_KERNEL);
>+ if (!ufg_gadget_grp)
>+ return ERR_PTR(-ENOMEM);
>+
>+ config_group_init_type_name(&ufg_gadget_grp->group, name, &ufg_gadget_grp_type);
>+
>+ return &ufg_gadget_grp->group;
>+}
>+
>+/*------------------- configfs subsystem for ufg ------------------------*/
>+
>+static ssize_t ufg_subsys_show_avail_func(struct ufg_subsys *grp, char *buf)
>+{
>+ return sprintf(buf, "%s\n", grp->avail_func);
>+}
>+
>+CONFIGFS_ATTR_STRUCT(ufg_subsys);
>+
>+UFG_ATTR_RO(avail_func, available_functions, ufg_subsys);
>+
>+static struct configfs_attribute *ufg_subsys_attrs[] = {
>+ &ufg_subsys_avail_func.attr,
>+ NULL,
>+};
>+
>+static struct ufg_subsys *to_ufg_subsys(struct config_item *item)
>+{
>+ return item ? container_of(to_configfs_subsystem(to_config_group(item)),
>+ struct ufg_subsys, subsys) : NULL;
>+}
>+
>+CONFIGFS_ATTR_OPS(ufg_subsys);
>+
>+static struct configfs_item_operations ufg_subsys_item_ops = {
>+ .show_attribute = ufg_subsys_attr_show,
>+};
>+
>+static struct configfs_group_operations ufg_subsys_group_ops = {
>+ .make_group = make_ufg_gadget,
>+};
>+
>+static struct config_item_type ufg_subsys_type = {
>+ .ct_attrs = ufg_subsys_attrs,
>+ .ct_item_ops = &ufg_subsys_item_ops,
>+ .ct_group_ops = &ufg_subsys_group_ops,
>+ .ct_owner = THIS_MODULE,
>+};
>+
>+struct ufg_subsys ufg_subsystem = {
>+ .subsys = {
>+ .su_group = {
>+ .cg_item = {
>+ .ci_namebuf = "usb-function-gadget",
>+ .ci_type = &ufg_subsys_type,
>+ },
>+ },
>+ },
>+};
>+
>+/*------------------- USB composite handling code -----------------------*/
>+
>+static const char longname[] = "USB Functions Gadget";
>+static const char serial[] = "0123456789.0123456789.0123456789";
>+static struct usb_string strings_dev[] = {
>+ [USB_GADGET_MANUFACTURER_IDX].s = "",
>+ [USB_GADGET_PRODUCT_IDX].s = longname,
>+ [USB_GADGET_SERIAL_IDX].s = serial,
>+ { } /* end of list */
I would keep them all empty. If the serial number has not been specified
there should be none, same goes for product. So the user has to set them
if he wants it to be set.
>+};
>+
>+static struct usb_gadget_strings stringtab_dev = {
>+ .language = 0x0409, /* en-us */
>+ .strings = strings_dev,
>+};
>+
>+static struct usb_gadget_strings *dev_strings[] = {
>+ &stringtab_dev,
>+ NULL,
>+};
>+
>+static void ufg_unbind_config(struct usb_configuration *c)
>+{
>+ kfree(c);
>+}
>+
>+static void ufg_fill_config_driver(struct usb_configuration *u_cfg,
>+ struct ufg_config_grp *c_grp, int n)
>+{
>+ u_cfg->label = "ufg";
>+ u_cfg->unbind = ufg_unbind_config;
>+ u_cfg->bConfigurationValue = c_grp->conf_number = n;
>+ u_cfg->bmAttributes = USB_CONFIG_ATT_ONE |
>+ USB_CONFIG_ATT_SELFPOWER;
>+ u_cfg->bMaxPower = c_grp->max_power;
>+}
>+
>+#define ufg_for_each_child(cursor, grp) \
>+ list_for_each_entry((cursor), &(grp)->cg_children, ci_entry)
>+
>+#define ufg_for_each_child_safe(cursor, tmp, grp) \
>+ list_for_each_entry_safe((cursor), (tmp), &(grp)->cg_children, ci_entry)
>+
>+#define ufg_rmdir_item(i, d) ufg_rmdir((i)->ci_dentry, (d)->ci_dentry)
>+
>+static void ufg_rmdirs(struct ufg_gadget_grp *ufg_gadget_grp)
>+{
>+ struct config_item *ci;
>+
>+ /* configurations */
>+ ufg_for_each_child(ci, &ufg_gadget_grp->group) {
>+ struct ufg_config_grp *ufg_cgrp = to_ufg_config_grp(ci);
>+ struct config_item *f;
>+
>+ /* functions' main groups */
>+ ufg_for_each_child(f, &ufg_cgrp->group) {
>+ struct config_group *f_grp = to_config_group(f);
>+ struct config_item *i, *i_tmp;
>+
>+ /* interfaces */
>+ ufg_for_each_child_safe(i, i_tmp, f_grp) {
>+ struct config_group *i_grp = to_config_group(i);
>+ struct config_item *e, *e_tmp;
>+ struct ufg_grp_hdr *h = ufg_hdr(i_grp);
>+
>+ if (h->type != UFG_INTERFACE)
>+ continue;
>+
>+ /* endpoints */
>+ ufg_for_each_child_safe(e, e_tmp, i_grp)
>+ ufg_rmdir_item(i, e);
>+
>+ ufg_rmdir_item(f, i);
>+ }
>+ }
>+ }
>+}
>+
>+static struct usb_descriptor_header **ufg_get_function_descriptors(
>+ struct usb_configuration *u_cfg, struct ufg_dev *ufg_dev)
>+{
>+ struct usb_function *u_fn;
>+
>+ /* last added function */
>+ u_fn = list_entry(u_cfg->functions.prev, struct usb_function, list);
>+ if (!u_fn)
>+ return NULL;
>+
>+ switch (ufg_dev->cdev->gadget->speed) {
>+ case USB_SPEED_SUPER:
>+ if (gadget_is_superspeed(ufg_dev->cdev->gadget))
>+ return u_fn->ss_descriptors;
>+ /* else: fall trough */
>+ case USB_SPEED_HIGH:
>+ if (gadget_is_dualspeed(ufg_dev->cdev->gadget))
>+ return u_fn->hs_descriptors;
>+ /* else: fall through */
>+ default:
>+ return u_fn->fs_descriptors;
>+ }
>+
>+ return NULL;
>+}
>+
>+#define INTF_NAME_TEMPL "interface00"
>+#define INTF_NR 9
>+
>+static struct config_group *ufg_process_interface_desc(
>+ struct usb_descriptor_header *d, struct config_group *f_grp)
>+{
>+ struct usb_interface_descriptor *id =
>+ (struct usb_interface_descriptor *)d;
>+ struct qstr name;
>+ struct dentry *parent, *new;
>+ struct config_item *i;
>+ struct ufg_interface_grp *igrp;
>+ int r;
>+
>+ parent = f_grp->cg_item.ci_dentry;
>+ name.name = INTF_NAME_TEMPL;
>+ sprintf((char *)(name.name + INTF_NR), "%02x", id->bInterfaceNumber);
>+ name.len = strlen(name.name);
>+ name.hash = full_name_hash(name.name, name.len);
>+ new = d_alloc(parent, &name);
>+ if (IS_ERR_OR_NULL(new))
>+ return NULL;
>+
>+ d_add(new, NULL);
>+ r = ufg_mkdir(parent, new);
>+ if (r) {
>+ d_drop(new);
>+ dput(new);
>+
>+ return ERR_PTR(r);
>+ }
>+ dput(new); /* make the refcount 1 */
>+
>+ i = config_group_find_item(f_grp, name.name);
>+ igrp = to_ufg_interface_grp(i);
>+ if (!igrp) {
>+ d_drop(new);
>+ dput(new);
>+
>+ return NULL;
>+ }
>+
>+ igrp->interface_nr = id->bInterfaceNumber;
>+ igrp->alt_setting = id->bAlternateSetting;
>+ igrp->num_endpoints = id->bNumEndpoints;
>+ igrp->intf_class = id->bInterfaceClass;
>+ igrp->intf_subclass = id->bInterfaceSubClass;
>+ igrp->intf_protocol = id->bInterfaceProtocol;
>+
>+ return to_config_group(i);
>+}
>+
>+#define ENDP_NAME_TEMPL "endpoint00"
>+#define ENDP_NR 8
>+
>+static struct config_group *ufg_process_endpoint_desc(
>+ struct usb_descriptor_header *d, struct config_group *interface_grp)
>+
>+{
>+ struct usb_endpoint_descriptor *ed = (struct usb_endpoint_descriptor *)d;
>+ struct qstr name;
>+ struct dentry *parent, *new;
>+ struct config_item *i;
>+ struct ufg_endpoint_grp *egrp;
>+ int r;
>+
>+ parent = interface_grp->cg_item.ci_dentry;
>+ name.name = ENDP_NAME_TEMPL;
>+ sprintf((char *)(name.name + ENDP_NR), "%02x", ed->bEndpointAddress);
Now, this confusing. Please use this variant:
sprintf(name.name, "endpoint%02x", ed->bEndpointAddress);
and allocate the qstr.name on stack and not global + anonym.
The same comment goes for the "interface00" case. And since both
function share so much code, what about merging them? Would that work?
>+ name.len = strlen(name.name);
So now you rely on strlen()? What about the return val of sprintf?
>+ name.hash = full_name_hash(name.name, name.len);
>+ new = d_alloc(parent, &name);
>+ if (IS_ERR_OR_NULL(new))
>+ return NULL;
>+
>+ d_add(new, NULL);
>+ r = ufg_mkdir(parent, new);
>+ if (r) {
>+ d_drop(new);
>+ dput(new);
please (re)use the "goto err;" technique.
>+ return ERR_PTR(r);
>+ }
>+ dput(new); /* make the refcount 1 */
>+
>+ i = config_group_find_item(interface_grp, name.name);
>+ egrp = to_ufg_endpoint_grp(i);
>+ if (!egrp) {
>+ d_drop(new);
>+ dput(new);
>+
>+ return NULL;
>+ }
>+
>+ egrp->endpoint_address = ed->bEndpointAddress;
>+ egrp->attributes = ed->bmAttributes;
>+ egrp->max_packet_sz = ed->wMaxPacketSize;
>+ egrp->interval = ed->bInterval;
>+
>+ return to_config_group(i);
>+}
>+
>+static int ufg_add_f(struct ufg_dev *ufg_dev, struct usb_configuration *u_cfg,
>+ struct config_item *f, ushort /*out*/ *num_interfaces)
>+{
>+ struct config_group *f_grp = to_config_group(f);
>+ struct ufg_function_grp *function_grp =
>+ container_of(f_grp, struct ufg_function_grp, group);
>+ struct usb_function *fn = function_grp->f;
>+
>+ if (IS_ERR_OR_NULL(fn))
>+ return PTR_ERR(fn);
ach. So fn maybe NULL or an ERR-PTR. And if it is NULL you return 0 and
everythink is fine?
>+
>+ if (fn && fn->add_function) {
But fn exisits.
>+ struct config_item *fn_grp;
>+ struct usb_descriptor_header **descriptors;
>+ struct usb_descriptor_header **d;
>+ struct config_group *i_grp = NULL;
>+ struct config_group *e_grp = NULL;
>+ int r;
>+
>+ fn_grp = list_entry(f_grp->cg_children.next,
>+ struct config_item, ci_entry);
>+ r = fn->add_function(u_cfg, fn, fn_grp, ufg_dev->cdev);
>+ if (r)
>+ return -1;
-EPERM?
>+
>+ descriptors = ufg_get_function_descriptors(u_cfg, ufg_dev);
>+ if (!descriptors)
>+ return -1;
>+
>+ for (d = descriptors; *d; d++)
>+ switch ((*d)->bDescriptorType) {
>+ case USB_DT_INTERFACE:
>+ i_grp = ufg_process_interface_desc(*d, f_grp);
>+ if (IS_ERR_OR_NULL(i_grp))
>+ return -1;
>+ /* keep gcc quiet about a value not being used*/
If it is not used, why do we have it?
>+ *num_interfaces = *num_interfaces + 1;
>+ break;
>+ case USB_DT_ENDPOINT:
>+ e_grp = ufg_process_endpoint_desc(*d, i_grp);
>+ if (IS_ERR_OR_NULL(e_grp))
>+ return -1;
>+ break;
>+ }
>+ }
>+
>+ return 0;
>+}
>+
>+static int ufg_bind(struct usb_composite_dev *cdev)
>+{
>+ struct ufg_dev *ufg_dev;
>+ struct usb_gadget *gadget = cdev->gadget;
>+ struct config_item *ci;
>+ int gcnum;
>+ int status;
>+ int n = 1;
>+
>+ ufg_dev = container_of(cdev->driver, struct ufg_dev, ufg_driver);
>+ ufg_dev->cdev = cdev;
>+ status = usb_string_ids_tab(cdev, strings_dev);
>+ if (status < 0)
>+ return status;
>+
>+ ufg_dev->ufg_device_desc.iManufacturer =
>+ strings_dev[USB_GADGET_MANUFACTURER_IDX].id;
>+ ufg_dev->ufg_device_desc.iProduct =
>+ strings_dev[USB_GADGET_PRODUCT_IDX].id;
>+ ufg_dev->ufg_device_desc.iSerialNumber =
>+ strings_dev[USB_GADGET_SERIAL_IDX].id;
>+
>+ /*
>+ * Start disconnected. Userspace will connect the gadget once
>+ * it is done configuring the functions.
>+ */
>+ usb_gadget_disconnect(gadget);
>+
>+ gcnum = get_default_bcdDevice();
>+ if (gcnum >= 0)
>+ ufg_dev->ufg_device_desc.bcdDevice =
>+ cpu_to_le16(0x0200 + gcnum);
>+ else {
>+ pr_warn("%s: controller '%s' not recognized\n",
>+ DRIVER_DESC, gadget->name);
>+ ufg_dev->ufg_device_desc.bcdDevice = cpu_to_le16(0x9999);
>+ }
This is an old chunk and should go. It will be auto-allocated based on
kernel's version.
>+
>+ usb_gadget_set_selfpowered(gadget);
>+
>+ usb_composite_overwrite_options(cdev, &coverwrite);
>+ pr_info("%s\n", DRIVER_DESC);
Why would you want to print this string over and over again? It is
static after all.
>+
>+ /* configurations */
>+ ufg_for_each_child(ci, &ufg_dev->g_grp->group) {
>+ struct ufg_config_grp *c_grp = to_ufg_config_grp(ci);
>+ struct usb_configuration *u_cfg;
>+ struct config_item *f;
>+
>+ u_cfg = kzalloc(sizeof(*u_cfg), GFP_KERNEL);
>+ if (!u_cfg)
>+ goto unbind;
>+ ufg_fill_config_driver(u_cfg, c_grp, n++);
>+ status = usb_add_config_only(ufg_dev->cdev, u_cfg);
>+ if (status)
>+ goto unbind;
>+
>+ /* functions' main groups */
>+ ufg_for_each_child(f, &c_grp->group) {
>+ status = ufg_add_f(ufg_dev, u_cfg, f, &c_grp->num_interfaces);
>+ if (status)
>+ goto unbind;
>+ }
>+
>+ }
>+
>+ return 0;
>+
>+unbind:
>+ ufg_rmdirs(ufg_dev->g_grp);
>+ return status;
>+}
>+
>+static void ufg_fill_ufg_driver(struct usb_composite_driver *ufgd,
>+ struct ufg_gadget_grp *g_grp)
>+{
>+ struct usb_device_descriptor *desc;
>+
>+ desc = &container_of(ufgd, struct ufg_dev, ufg_driver)->ufg_device_desc;
>+ desc->bLength = sizeof(desc); /* TODO: *desc ? */
>+ desc->bDescriptorType = USB_DT_DEVICE;
>+ desc->bcdUSB = cpu_to_le16(0x0200);
>+ desc->bDeviceClass = USB_CLASS_PER_INTERFACE;
>+ desc->bNumConfigurations = 1;
You had config items for those. Where did they go? And what about multi
configs?
>+
>+ ufgd->bind = ufg_bind;
>+ ufgd->strings = dev_strings;
>+ ufgd->name = "usb_function_gadget";
>+ ufgd->dev = desc;
>+ ufgd->needs_serial = 1;
>+
>+ if (g_grp->idVendor_set)
>+ coverwrite.idVendor = g_grp->idVendor;
>+ if (g_grp->idProduct_set)
>+ coverwrite.idProduct = g_grp->idProduct;
>+ if (g_grp->bcdDevice_set)
>+ coverwrite.bcdDevice = g_grp->bcdDevice;
>+ if (g_grp->iManufacturer_set)
>+ coverwrite.manufacturer = g_grp->iManufacturer;
>+ if (g_grp->iProduct_set)
>+ coverwrite.product = g_grp->iProduct;
>+ if (g_grp->iSerialNumber_set)
>+ coverwrite.serial_number = g_grp->iSerialNumber;
>+}
>+
>+static int ufg_gadget_ready(struct ufg_gadget_grp *g_grp)
>+{
>+ struct ufg_dev *ufg_dev;
>+ int r;
>+
>+ if (!g_grp->ready) {
>+ ufg_dev = g_grp->gadget_grp_data;
>+
>+ ufg_rmdirs(g_grp);
>+ usb_composite_unregister(&ufg_dev->ufg_driver);
>+ kfree(ufg_dev);
>+ return 0;
>+ }
>+
>+ ufg_dev = kzalloc(sizeof(*ufg_dev), GFP_KERNEL);
>+ if (!ufg_dev)
>+ return -ENOMEM;
>+
>+ ufg_fill_ufg_driver(&ufg_dev->ufg_driver, g_grp);
>+ g_grp->gadget_grp_data = ufg_dev;
>+ ufg_dev->g_grp = g_grp;
>+ r = usb_composite_probe(&ufg_dev->ufg_driver);
>+ if (r) {
>+ usb_composite_unregister(&ufg_dev->ufg_driver);
>+ kfree(ufg_dev);
>+ }
>+
>+ return r;
>+}
>+
>+/*---------------------- general module stuff ---------------------------*/
>+
>+MODULE_DESCRIPTION(DRIVER_DESC);
>+MODULE_AUTHOR("Andrzej Pietrasiewicz");
>+MODULE_ALIAS("usb_functions");
>+MODULE_LICENSE("GPL");
Please keep this things (MODULE_*) at the bottom. Why an ALIAS?
>+
>+struct ufg_subsys *UFG_SUBSYSTEM;
>+EXPORT_SYMBOL(UFG_SUBSYSTEM);
>+
>+static int __init ufg_init(void)
>+{
>+ int ret;
>+
>+ config_group_init(&ufg_subsystem.subsys.su_group);
>+ mutex_init(&ufg_subsystem.subsys.su_mutex);
>+ ret = configfs_register_subsystem(&ufg_subsystem.subsys);
>+ if (ret)
>+ goto unregister;
>+
>+ UFG_SUBSYSTEM = &ufg_subsystem;
>+ return 0;
>+
>+unregister:
>+ configfs_unregister_subsystem(&ufg_subsystem.subsys);
>+
>+ return ret;
>+}
>+module_init(ufg_init);
>+
>+static void ufg_cleanup(void)
>+{
>+ configfs_unregister_subsystem(&ufg_subsystem.subsys);
>+}
>+module_exit(ufg_cleanup);
>diff --git a/drivers/usb/gadget/usb_functions.h b/drivers/usb/gadget/usb_functions.h
>new file mode 100644
>index 0000000..678f4b8
>--- /dev/null
>+++ b/drivers/usb/gadget/usb_functions.h
>@@ -0,0 +1,100 @@
>+/*
>+ * USB Functions Gadget
>+ *
>+ * Copyright (C) 2012 Samsung Electronics
>+ * Author: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
>+ *
>+ * This software is licensed under the terms of the GNU General Public
>+ * License version 2, as published by the Free Software Foundation, and
>+ * may be copied, distributed, and modified under those terms.
>+ *
>+ * This program is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+ * GNU General Public License for more details.
>+ *
>+ */
>+
>+#ifndef __LINUX_USB_USB_FUNCTIONS__
>+#define __LINUX_USB_USB_FUNCTIONS__
>+
>+#include <linux/configfs.h>
>+#include <linux/usb/composite.h>
>+
>+#define UFG_STR_LEN 256
>+#define UFG_FUNC_NAMES_BUF_LEN 256
>+#define UFG_NAME_LEN 256
>+#define UFG_FUNC_NAME_LEN 256
>+
>+
>+#define ufg_mkdir(parent, new) (parent)->d_inode->i_op->mkdir(NULL, (new), 0)
>+#define ufg_rmdir(p, d) (p)->d_inode->i_op->rmdir((p)->d_inode, (d))
NO. For both. Either static inline or you type it.
>+
>+enum ufg_hdr_type {
>+ UFG_FUNCTION,
>+ UFG_INTERFACE,
>+};
>+
>+struct ufg_grp_hdr {
>+ struct config_group group;
>+
>+ enum ufg_hdr_type type;
>+};
>+
>+struct ufg_function_grp {
>+ /* This group needs children */
>+ struct config_group group;
>+
>+ /* attributes' values */
>+ char name[UFG_FUNC_NAME_LEN];
>+
>+ struct usb_function *f;
>+};
>+
>+struct ufg_gadget_grp {
>+ /* This group needs children */
>+ struct config_group group;
>+
>+ /* attributes' values */
>+ ushort idVendor;
>+ ushort idVendor_set:1;
>+ ushort idProduct;
>+ ushort idProduct_set:1;
>+ ushort bcdDevice;
>+ ushort bcdDevice_set:1;
If you put the :1 variables after each other then the compiler will
merge them. I think that was the plan because otherwise it is confusing.
>+ char iManufacturer[UFG_NAME_LEN];
>+ ushort iManufacturer_set:1;
>+ char iProduct[UFG_NAME_LEN];
>+ ushort iProduct_set:1;
>+ char iSerialNumber[UFG_NAME_LEN];
>+ ushort iSerialNumber_set:1;
So you decided to use the max number of bytes USB allowes. You waste
around half a KiB because most will be smaller. Please use a kmalloc()
buffer here.
>+ bool ready;
>+
>+ void *gadget_grp_data;
>+};
>+
>+static inline struct ufg_gadget_grp *to_ufg_gadget_grp(struct config_item *item)
>+{
>+ return item ? container_of(to_config_group(item), struct ufg_gadget_grp,
>+ group) : NULL;
>+}
>+
>+struct ufg_subsys {
>+ /* This is the root of the subsystem */
>+ struct configfs_subsystem subsys;
>+
>+ /* attributes' values */
>+ char avail_func[UFG_FUNC_NAMES_BUF_LEN];
>+};
>+
>+struct ufg_dev {
>+ struct usb_device_descriptor ufg_device_desc;
>+ struct usb_composite_driver ufg_driver;
>+ struct usb_composite_dev *cdev;
>+ struct ufg_gadget_grp *g_grp;
>+};
>+
>+extern struct usb_composite_driver *UFG_COMPOSITE;
>+extern struct ufg_subsys *UFG_SUBSYSTEM;
>+
>+#endif /* __LINUX_USB_USB_FUNCTIONS__ */
>--
Sesbastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
