[RFC 0/2] USB gadget - configfs

[RFC 0/2] USB gadget - configfs

[Andrzej Pietrasiewicz]

Dear All,

This patch series is a follow-up to this thread:

http://marc.info/?l=linux-usb&m=132430533824884&w=2

As a prerequisite it adds an operation to configfs. The operation allows
checking if it is ok to remove a pseudo directory corresponding to a
configfs item/group.

Then, a USB Functions Gadget is added. The idea is to have a gadget, which
incorporates a number of USB functions and is fully configurable through
configfs. Here, the functions are gathered in the available_functions array.
The array cosists of struct ufg_fn instances, each of which contains its name,
pointer to the config_group associated with it and a pointer to its bind
function. As an example the f_mass_storage is adapted to the new scheme.

The structure of a mounted configfs with a configured gadget looks like this:

/cfg
 |
 +-/usb-function-gadget
    |
    +-available_functions
    |
    +-/G1
       |
       +-idVendor
       |
       +-idProduct
       |
       +-iSerialNumber
       |
       +-iManufacturer
       |
       +-iProduct
       |
       +-iSerialNumber
       |
       +-connect
       |
       +-/C1
          |
          +-maximum_power
          |
          +-number_of_interfaces
          |
          +-configuration_number
          |
          +-/F1
             |
             +-name
             |
             +/<corresponding to name>
               |
               (....) USB function-specific stuff


For mass storage, the function-specific stuff looks like this:

               +-luns
               |
               +-stall
               |
               +-/lunX
               |  |
               |  +-file
               |  |
               |  +-nofua
               |  |
               |  +-removable
               |  |
               |  +-ro
               |
               .
               .
               .

The gadget starts disconnected in USB terms. Instead, a configfs top-level
subsystem is registered. All configfs entities associated with the gadget
are hooked into the subsystem. In an example session the user issues
the following commands:

$ insmod g_usb_functions.ko
$ mount -t configfs none /cfg
$ mkdir -p /cfg/usb-function-gadget/G1/C1/F1
$ echo "f_mass_storage" > /cfg/usb-function-gadget/G1/C1/F1/name
$ mkdir /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/
$ echo 1 > /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/luns
$ mkdir /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/lun0
$ echo 1 > /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/lun0/removable

Some vendor-specific parameters can be set:

$ echo -n "ABC" > /cfg/usb-function-gadget/G1/iManufacturer
$ echo -n "DEF" > /cfg/usb-function-gadget/G1/iSerialNumber
$ echo -n "GHI" > /cfg/usb-function-gadget/G1/iProduct

The backing file may be specified now or after connecting the gadget:

$ echo 1 >  /cfg/usb-function-gadget/G1/connect
$ file.img > /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/lun0/file

Each function, after creating its corresponding directory
(/cfg/usb-function-gadget/G1/C1/F1), must be "personalized" by storing
its name in the "name" attribute. After that it is possible to create
a child item of the same name ("f_mass_storage" here). The common code
handles everything from top of the hierarchy up to the function directory.
Under the function directory a function-specific stuff provided by each
function is used. The function-specific code is abstracted by the above
mentioned struct ufg_fn. In the example, the mass storage function is
supplied with one LUN.

The "connect" attribute's store method calls the ufg_gadget_bind function,
which registers the composite gadget, then walks the configfs hierarchy
rooted at the above mentioned subsystem and does USB configurations and
functions registration.

This is a work in progress. There might be issues.

I would like to ask some questions. All answers in general, and answers
from linux-usb and Felipe and Greg KH in particular, are welcome.

1. Generally, is this the right way to go?
2. Using configfs like this calls for an interface between the generic
   configfs-related code and function-specific code. I suggested the
   struct ufg_fn. What do you think?
3. Should some parameters still be available through sysfs?
4. Do we need module parameters for USB descriptors like iManufacturer
   and similar?
5. I assumed that the configfs entities are contained in the structures
   used for configuring the USB functions, e.g. a struct config_group in
   struct fsg_common, or a struct config_item in a struct fsg_lun. This
   has implications that the lifetime of the structures is controlled by
   userspace through configfs, which, in turn, has influence on how
   the USB functions are implemented. Even though it seems natural,
   there are some issues. For example an extension to configfs was required
   in order to disable deleting the luns while the gadget is connected.
   Is this the right approach? If not, then are there any alternatives?

Andrzej Pietrasiewicz (2):
  fs: configfs: add check_rmdir operation
  usb: gadget: Add USB Functions Gadget

 drivers/usb/gadget/Kconfig          |   11 +
 drivers/usb/gadget/Makefile         |    2 +
 drivers/usb/gadget/composite.c      |   27 +-
 drivers/usb/gadget/f_mass_storage.c |  675 +++++++++++++++++---------------
 drivers/usb/gadget/storage_common.c |  376 +++++++++++-------
 drivers/usb/gadget/usb_functions.c  |  731 +++++++++++++++++++++++++++++++++++
 fs/configfs/dir.c                   |   20 +
 include/linux/configfs.h            |    2 +
 8 files changed, 1371 insertions(+), 473 deletions(-)
 create mode 100644 drivers/usb/gadget/usb_functions.c

[RFC 1/2] fs: configfs: add check_rmdir operation

[Andrzej Pietrasiewicz]

The logic behind a subsystem which uses configfs might be that it is not
desired to allow removing the pseudo directories (items or groups) in a
mounted configfs while some criterion is not met, e.g. while some
actions are in progress. This patch adds a check_rmdir operation to
configfs_item_operations and to configfs_group_operations. The operation,
if provided, is called before actual remove takes place and if the result
is nonzero, the remove is not done and the error it returns is reported.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 fs/configfs/dir.c        |   20 ++++++++++++++++++++
 include/linux/configfs.h |    2 ++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 7e6c52d..de2680f 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -1408,12 +1408,32 @@ static int configfs_rmdir(struct inode *dir, struct dentry *dentry)
 		dead_item_owner = item->ci_type->ct_owner;
 
 	if (sd->s_type & CONFIGFS_USET_DIR) {
+		if (item->ci_type && item->ci_type->ct_group_ops &&
+		    item->ci_type->ct_group_ops->check_rmdir) {
+		    	ret = item->ci_type->ct_group_ops->check_rmdir(
+				to_config_group(parent_item), item);
+			if (ret) {
+				config_item_put(item);
+				configfs_detach_rollback(dentry);
+				return ret;
+			}
+		}
 		configfs_detach_group(item);
 
 		mutex_lock(&subsys->su_mutex);
 		client_disconnect_notify(parent_item, item);
 		unlink_group(to_config_group(item));
 	} else {
+		if (item->ci_type && item->ci_type->ct_item_ops &&
+		    item->ci_type->ct_item_ops->check_rmdir) {
+		    	ret = item->ci_type->ct_item_ops->check_rmdir(
+				to_config_group(parent_item), item);
+			if (ret) {
+				config_item_put(item);
+				configfs_detach_rollback(dentry);
+				return ret;
+			}
+		}
 		configfs_detach_item(item);
 
 		mutex_lock(&subsys->su_mutex);
diff --git a/include/linux/configfs.h b/include/linux/configfs.h
index 34025df..f356a55 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -225,6 +225,7 @@ struct configfs_item_operations {
 	void (*release)(struct config_item *);
 	ssize_t	(*show_attribute)(struct config_item *, struct configfs_attribute *,char *);
 	ssize_t	(*store_attribute)(struct config_item *,struct configfs_attribute *,const char *, size_t);
+	int (*check_rmdir)(struct config_group *group, struct config_item *item);
 	int (*allow_link)(struct config_item *src, struct config_item *target);
 	int (*drop_link)(struct config_item *src, struct config_item *target);
 };
@@ -233,6 +234,7 @@ struct configfs_group_operations {
 	struct config_item *(*make_item)(struct config_group *group, const char *name);
 	struct config_group *(*make_group)(struct config_group *group, const char *name);
 	int (*commit_item)(struct config_item *item);
+	int (*check_rmdir)(struct config_group *group, struct config_item *item);
 	void (*disconnect_notify)(struct config_group *group, struct config_item *item);
 	void (*drop_item)(struct config_group *group, struct config_item *item);
 };
-- 
1.7.0.4

[RFC 2/2] usb: gadget: Add USB Functions Gadget

[Andrzej Pietrasiewicz]

Demonstrate a USB gadget configured entirely through configfs.
This is a work in progress.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/usb/gadget/Kconfig          |   11 +
 drivers/usb/gadget/Makefile         |    2 +
 drivers/usb/gadget/composite.c      |   27 +-
 drivers/usb/gadget/f_mass_storage.c |  675 +++++++++++++++++---------------
 drivers/usb/gadget/storage_common.c |  376 +++++++++++-------
 drivers/usb/gadget/usb_functions.c  |  731 +++++++++++++++++++++++++++++++++++
 6 files changed, 1349 insertions(+), 473 deletions(-)
 create mode 100644 drivers/usb/gadget/usb_functions.c

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index a167f37..40d21ba 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -556,6 +556,17 @@ choice
 
 # this first set of drivers all depend on bulk-capable hardware.
 
+config USB_FG
+	tristate "USB Functions Gadget (EXPERIMENTAL)"
+	depends on EXPERIMENTAL && CONFIGFS_FS
+	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.
+
 config USB_ZERO
 	tristate "Gadget Zero (DEVELOPMENT)"
 	help
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index 3bd3bd6..ff5ee74 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_USB_FUSB300)	+= fusb300_udc.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
@@ -53,6 +54,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
 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/composite.c b/drivers/usb/gadget/composite.c
index 390749b..ddf6390 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -40,29 +40,14 @@ static int (*composite_gadget_bind)(struct usb_composite_dev *cdev);
  */
 
 static ushort idVendor;
-module_param(idVendor, ushort, 0644);
-MODULE_PARM_DESC(idVendor, "USB Vendor ID");
-
 static ushort idProduct;
-module_param(idProduct, ushort, 0644);
-MODULE_PARM_DESC(idProduct, "USB Product ID");
-
 static ushort bcdDevice;
-module_param(bcdDevice, ushort, 0644);
-MODULE_PARM_DESC(bcdDevice, "USB Device version (BCD)");
-
-static char *iManufacturer;
-module_param(iManufacturer, charp, 0644);
-MODULE_PARM_DESC(iManufacturer, "USB Manufacturer string");
-
-static char *iProduct;
-module_param(iProduct, charp, 0644);
-MODULE_PARM_DESC(iProduct, "USB Product string");
-
-static char *iSerialNumber;
-module_param(iSerialNumber, charp, 0644);
-MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");
-
+static char i_manufacturer[256];
+static char *iManufacturer = i_manufacturer;
+static char i_product[256];
+static char *iProduct = i_product;
+static char i_serial[256];
+static char *iSerialNumber = i_serial;
 static char composite_manufacturer[50];
 
 /*-------------------------------------------------------------------------*/
diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index f67b453..8c90d37 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -55,11 +55,6 @@
  *
  *	nluns		Number of LUNs function have (anywhere from 1
  *				to FSG_MAX_LUNS which is 8).
- *	luns		An array of LUN configuration values.  This
- *				should be filled for each LUN that
- *				function will include (ie. for "nluns"
- *				LUNs).  Each element of the array has
- *				the following fields:
  *	->filename	The path to the backing file for the LUN.
  *				Required if LUN is not marked as
  *				removable.
@@ -368,7 +363,6 @@ struct fsg_common {
 
 	unsigned int		nluns;
 	unsigned int		lun;
-	struct fsg_lun		*luns;
 	struct fsg_lun		*curlun;
 
 	unsigned int		bulk_out_maxpacket;
@@ -383,7 +377,6 @@ struct fsg_common {
 	u32			usb_amount_left;
 
 	unsigned int		can_stall:1;
-	unsigned int		free_storage_on_release:1;
 	unsigned int		phase_error:1;
 	unsigned int		short_packet_received:1;
 	unsigned int		bad_lun_okay:1;
@@ -405,6 +398,10 @@ struct fsg_common {
 	char inquiry_string[8 + 16 + 4 + 1];
 
 	struct kref		ref;
+
+	const char		*lun_name_format;
+
+	struct config_group	group;
 };
 
 struct fsg_config {
@@ -430,6 +427,10 @@ struct fsg_config {
 	u16 release;
 
 	char			can_stall;
+	struct usb_configuration *usb_config;
+
+	/* configfs-related */
+	struct config_group	group;
 };
 
 struct fsg_dev {
@@ -1460,8 +1461,7 @@ static int do_start_stop(struct fsg_common *common)
 
 	/* Simulate an unload/eject */
 	if (common->ops && common->ops->pre_eject) {
-		int r = common->ops->pre_eject(common, curlun,
-					       curlun - common->luns);
+		int r = common->ops->pre_eject(common, curlun, curlun->n_lun);
 		if (unlikely(r < 0))
 			return r;
 		else if (r)
@@ -1475,8 +1475,7 @@ static int do_start_stop(struct fsg_common *common)
 	down_read(&common->filesem);
 
 	return common->ops && common->ops->post_eject
-		? min(0, common->ops->post_eject(common, curlun,
-						 curlun - common->luns))
+		? min(0, common->ops->post_eject(common, curlun, curlun->n_lun))
 		: 0;
 }
 
@@ -2255,8 +2254,7 @@ static int received_cbw(struct fsg_dev *fsg, struct fsg_buffhd *bh)
 	if (cbw->Lun >= FSG_MAX_LUNS || cbw->Flags & ~US_BULK_FLAG_IN ||
 			cbw->Length <= 0 || cbw->Length > MAX_COMMAND_SIZE) {
 		DBG(fsg, "non-meaningful CBW: lun = %u, flags = 0x%x, "
-				"cmdlen %u\n",
-				cbw->Lun, cbw->Flags, cbw->Length);
+		    "cmdlen %u\n", cbw->Lun, cbw->Flags, cbw->Length);
 
 		/*
 		 * We can do anything we want here, so let's stall the
@@ -2280,9 +2278,22 @@ static int received_cbw(struct fsg_dev *fsg, struct fsg_buffhd *bh)
 	if (common->data_size == 0)
 		common->data_dir = DATA_DIR_NONE;
 	common->lun = cbw->Lun;
-	if (common->lun >= 0 && common->lun < common->nluns)
-		common->curlun = &common->luns[common->lun];
-	else
+	if (common->lun >= 0 && common->lun < common->nluns) {
+		struct config_item *it;
+
+		mutex_lock(&common->group.cg_subsys->su_mutex);
+		list_for_each_entry(it, &common->group.cg_children, ci_entry) {
+			struct fsg_lun *lun;
+
+			lun = to_fsg_lun(it);
+			if (lun->n_lun == common->lun) {
+				common->curlun = lun;
+
+				break;
+			}
+		}
+		mutex_unlock(&common->group.cg_subsys->su_mutex);
+	} else
 		common->curlun = NULL;
 	common->tag = cbw->Tag;
 	return 0;
@@ -2342,6 +2353,7 @@ static int alloc_request(struct fsg_common *common, struct usb_ep *ep,
 /* Reset interface setting and re-init endpoint state (toggle etc). */
 static int do_set_interface(struct fsg_common *common, struct fsg_dev *new_fsg)
 {
+	struct config_item *item;
 	struct fsg_dev *fsg;
 	int i, rc = 0;
 
@@ -2426,8 +2438,14 @@ reset:
 	}
 
 	common->running = 1;
-	for (i = 0; i < common->nluns; ++i)
-		common->luns[i].unit_attention_data = SS_RESET_OCCURRED;
+	mutex_lock(&common->group.cg_subsys->su_mutex);
+	list_for_each_entry(item, &common->group.cg_children, ci_entry) {
+		struct fsg_lun *lun;
+
+		lun = to_fsg_lun(item);
+		lun->unit_attention_data = SS_RESET_OCCURRED;
+	}
+	mutex_unlock(&common->group.cg_subsys->su_mutex);
 	return rc;
 }
 
@@ -2458,7 +2476,6 @@ static void handle_exception(struct fsg_common *common)
 	int			i;
 	struct fsg_buffhd	*bh;
 	enum fsg_state		old_state;
-	struct fsg_lun		*curlun;
 	unsigned int		exception_req_tag;
 
 	/*
@@ -2526,14 +2543,20 @@ static void handle_exception(struct fsg_common *common)
 	if (old_state == FSG_STATE_ABORT_BULK_OUT)
 		common->state = FSG_STATE_STATUS_PHASE;
 	else {
-		for (i = 0; i < common->nluns; ++i) {
-			curlun = &common->luns[i];
+		struct config_item *it;
+
+		mutex_lock(&common->group.cg_subsys->su_mutex);
+		list_for_each_entry(it, &common->group.cg_children, ci_entry) {
+			struct fsg_lun *curlun;
+
+			curlun = to_fsg_lun(it);
 			curlun->prevent_medium_removal = 0;
 			curlun->sense_data = SS_NO_SENSE;
 			curlun->unit_attention_data = SS_NO_SENSE;
 			curlun->sense_data_info = 0;
 			curlun->info_valid = 0;
 		}
+		mutex_unlock(&common->group.cg_subsys->su_mutex);
 		common->state = FSG_STATE_IDLE;
 	}
 	spin_unlock_irq(&common->lock);
@@ -2666,17 +2689,25 @@ static int fsg_main_thread(void *common_)
 
 	if (!common->ops || !common->ops->thread_exits
 	 || common->ops->thread_exits(common) < 0) {
-		struct fsg_lun *curlun = common->luns;
-		unsigned i = common->nluns;
+		struct list_head *cursor;
 
 		down_write(&common->filesem);
-		for (; i--; ++curlun) {
+
+		mutex_lock(&common->group.cg_subsys->su_mutex);
+		list_for_each_prev(cursor, &common->group.cg_children) {
+			struct config_item *item;
+			struct fsg_lun *curlun;
+
+			item = list_entry(cursor, struct config_item, ci_entry);
+
+			curlun = to_fsg_lun(item);
 			if (!fsg_lun_is_open(curlun))
 				continue;
 
 			fsg_lun_close(curlun);
 			curlun->unit_attention_data = SS_MEDIUM_NOT_PRESENT;
 		}
+		mutex_unlock(&common->group.cg_subsys->su_mutex);
 		up_write(&common->filesem);
 	}
 
@@ -2684,24 +2715,10 @@ static int fsg_main_thread(void *common_)
 	complete_and_exit(&common->thread_notifier, 0);
 }
 
-
-/*************************** DEVICE ATTRIBUTES ***************************/
-
-/* Write permission is checked per LUN in store_*() functions. */
-static DEVICE_ATTR(ro, 0644, fsg_show_ro, fsg_store_ro);
-static DEVICE_ATTR(nofua, 0644, fsg_show_nofua, fsg_store_nofua);
-static DEVICE_ATTR(file, 0644, fsg_show_file, fsg_store_file);
-
-
 /****************************** FSG COMMON ******************************/
 
 static void fsg_common_release(struct kref *ref);
 
-static void fsg_lun_release(struct device *dev)
-{
-	/* Nothing needs to be done */
-}
-
 static inline void fsg_common_get(struct fsg_common *common)
 {
 	kref_get(&common->ref);
@@ -2712,49 +2729,192 @@ static inline void fsg_common_put(struct fsg_common *common)
 	kref_put(&common->ref, fsg_common_release);
 }
 
-static struct fsg_common *fsg_common_init(struct fsg_common *common,
-					  struct usb_composite_dev *cdev,
-					  struct fsg_config *cfg)
+#define DIGITS		"0123456789"
+static struct config_item *alloc_fsg_lun(struct config_group *group,
+					 const char *name)
+{
+	struct fsg_common *common;
+	struct fsg_lun *lun;
+	struct config_item *item;
+	const char *p, *r, *s;
+	int n;
+	char buf[256];
+	unsigned long tmp;
+
+	common = group ? container_of(group, struct fsg_common, group) : NULL;
+	if (!common)
+		return ERR_PTR(-ENOMEM);
+
+	/*
+	 * TODO: some of the checks should be done when
+	 *common->lun_name_format is assigned
+	 */
+	/* check if first part of the name format is good */
+	p = strchr(common->lun_name_format, '%');
+	if (!p)
+		return ERR_PTR(-EINVAL);
+	if (*(p + 1) != 'd')
+		return ERR_PTR(-EINVAL);
+	n = p - common->lun_name_format;
+	/* check if the first part of the name matches the format */
+	if (strncmp(name, common->lun_name_format, n))
+		return ERR_PTR(-EINVAL);
+	/* interpret the %d part */
+	/*
+	 * TODO: improve. Now e.g. 01 and 1 are considered equal,
+	 * which means lun1 cannot be created after lun01 is created.
+	 * Probably lun01 (number parts with leading zeros) should be
+	 * disallowed.
+	 */
+	r = name + n;
+	s = strpbrk(r, DIGITS);
+	if (s != r)
+		return ERR_PTR(-EINVAL);
+	n = strspn(s, DIGITS);
+	while (n--) {
+		buf[s - r] = *s;
+		s++;
+	}
+	buf[s - r] = '\0';
+	tmp = simple_strtoul(buf, NULL, 10);
+	if (tmp >= common->nluns)
+		return ERR_PTR(-EINVAL);
+	/* check if the second part of the name meatches the format */
+	if (strcmp(p + 2, s))
+		return ERR_PTR(-EINVAL);
+
+	list_for_each_entry(item, &common->group.cg_children, ci_entry) {
+		lun = to_fsg_lun(item);
+		if (tmp == lun->n_lun)
+			return ERR_PTR(-EBUSY);
+	}
+
+	lun = kzalloc(sizeof *lun, GFP_KERNEL);
+	if (!lun)
+		return ERR_PTR(-ENOMEM);
+
+	lun->filesem = &common->filesem;
+	lun->n_lun = tmp;
+
+	config_item_init_type_name(&lun->item, name, &fsg_lun_item_type);
+
+	LINFO(lun, "LUN: %s%s%sfile: %s\n",
+	      lun->removable ? "removable " : "",
+	      lun->ro ? "read only " : "",
+	      lun->cdrom ? "CD-ROM " : "",
+	      "(no medium)");
+
+	return &lun->item;
+}
+
+static ssize_t fsg_common_show_luns(struct fsg_common *common, char *buf)
+{
+	return sprintf(buf, "%d\n", common->nluns);
+}
+
+static ssize_t fsg_common_store_luns(struct fsg_common *common, const char *buf,
+				     size_t count)
+{
+	struct config_item *function, *config, *gadget;
+	unsigned long tmp;
+	int ret;
+	char *p = (char *)buf;
+
+	function = common->group.cg_item.ci_parent;
+	if (!function)
+		return -EBUSY;
+
+	config = function->ci_parent;
+	if (!config)
+		return -EBUSY;
+
+	gadget = config->ci_parent;
+	if (!gadget)
+		return -EBUSY;
+
+	ret = gadget_verify_connected(gadget);
+	if (ret)
+		return ret;
+
+	tmp = simple_strtoul(p, &p, 10);
+	if (!p || (*p && (*p != '\n')))
+		return -EINVAL;
+
+	if (tmp > 16383)
+		return -ERANGE;
+
+	common->nluns = tmp;
+
+	return count;
+}
+
+static ssize_t fsg_common_show_stall(struct fsg_common *common, char *buf)
+{
+	return sprintf(buf, "%d\n", common->can_stall);
+}
+
+static ssize_t fsg_common_store_stall(struct fsg_common *common,
+				      const char *buf, size_t count)
+{
+	if (buf[0] != '0' && buf[0] != '1')
+		return -EINVAL;
+
+	common->can_stall = buf[0] == '1';
+
+	return count;
+}
+
+CONFIGFS_ATTR_STRUCT(fsg_common);
+
+#define FSG_CONFIG_ATTR_RW(_name)					\
+static struct fsg_common_attribute fsg_common_##_name =			\
+	__CONFIGFS_ATTR(_name, S_IRUGO | S_IWUSR, fsg_common_show_##_name,\
+			fsg_common_store_##_name)
+
+FSG_CONFIG_ATTR_RW(luns);
+FSG_CONFIG_ATTR_RW(stall);
+
+static struct configfs_attribute *fsg_common_attrs[] = {
+	&fsg_common_luns.attr,
+	&fsg_common_stall.attr,
+	NULL,
+};
+
+static struct fsg_common *to_fsg_common(struct config_item *item)
+{
+	return item ? container_of(to_config_group(item),
+				   struct fsg_common, group) : NULL;
+}
+
+CONFIGFS_ATTR_OPS(fsg_common);
+
+static void fsg_common_release_item(struct config_item *item)
+{
+	kfree(to_fsg_common(item));
+}
+
+static struct configfs_item_operations fsg_common_item_ops = {
+	.show_attribute		= fsg_common_attr_show,
+	.store_attribute	= fsg_common_attr_store,
+	.release		= fsg_common_release_item,
+};
+
+static struct configfs_group_operations fsg_common_group_ops = {
+	.make_item	= alloc_fsg_lun,
+};
+
+static struct config_item_type fsg_common_item_type = {
+	.ct_attrs	= fsg_common_attrs,
+	.ct_item_ops	= &fsg_common_item_ops,
+	.ct_group_ops	= &fsg_common_group_ops,
+	.ct_owner	= THIS_MODULE,
+};
+
+static struct fsg_common *fsg_common_init_cdev(struct fsg_common *common,
+					       struct usb_composite_dev *cdev)
 {
 	struct usb_gadget *gadget = cdev->gadget;
-	struct fsg_buffhd *bh;
-	struct fsg_lun *curlun;
-	struct fsg_lun_config *lcfg;
-	int nluns, i, rc;
-	char *pathbuf;
-
-	rc = fsg_num_buffers_validate();
-	if (rc != 0)
-		return ERR_PTR(rc);
-
-	/* Find out how many LUNs there should be */
-	nluns = cfg->nluns;
-	if (nluns < 1 || nluns > FSG_MAX_LUNS) {
-		dev_err(&gadget->dev, "invalid number of LUNs: %u\n", nluns);
-		return ERR_PTR(-EINVAL);
-	}
-
-	/* Allocate? */
-	if (!common) {
-		common = kzalloc(sizeof *common, GFP_KERNEL);
-		if (!common)
-			return ERR_PTR(-ENOMEM);
-		common->free_storage_on_release = 1;
-	} else {
-		memset(common, 0, sizeof *common);
-		common->free_storage_on_release = 0;
-	}
-
-	common->buffhds = kcalloc(fsg_num_buffers,
-				  sizeof *(common->buffhds), GFP_KERNEL);
-	if (!common->buffhds) {
-		if (common->free_storage_on_release)
-			kfree(common);
-		return ERR_PTR(-ENOMEM);
-	}
-
-	common->ops = cfg->ops;
-	common->private_data = cfg->private_data;
+	int rc, i;
 
 	common->gadget = gadget;
 	common->ep0 = gadget->ep0;
@@ -2770,84 +2930,10 @@ static struct fsg_common *fsg_common_init(struct fsg_common *common,
 		fsg_intf_desc.iInterface = rc;
 	}
 
-	/*
-	 * Create the LUNs, open their backing files, and register the
-	 * LUN devices in sysfs.
-	 */
-	curlun = kcalloc(nluns, sizeof(*curlun), GFP_KERNEL);
-	if (unlikely(!curlun)) {
-		rc = -ENOMEM;
-		goto error_release;
-	}
-	common->luns = curlun;
-
-	init_rwsem(&common->filesem);
-
-	for (i = 0, lcfg = cfg->luns; i < nluns; ++i, ++curlun, ++lcfg) {
-		curlun->cdrom = !!lcfg->cdrom;
-		curlun->ro = lcfg->cdrom || lcfg->ro;
-		curlun->initially_ro = curlun->ro;
-		curlun->removable = lcfg->removable;
-		curlun->dev.release = fsg_lun_release;
-		curlun->dev.parent = &gadget->dev;
-		/* curlun->dev.driver = &fsg_driver.driver; XXX */
-		dev_set_drvdata(&curlun->dev, &common->filesem);
-		dev_set_name(&curlun->dev,
-			     cfg->lun_name_format
-			   ? cfg->lun_name_format
-			   : "lun%d",
-			     i);
-
-		rc = device_register(&curlun->dev);
-		if (rc) {
-			INFO(common, "failed to register LUN%d: %d\n", i, rc);
-			common->nluns = i;
-			put_device(&curlun->dev);
-			goto error_release;
-		}
-
-		rc = device_create_file(&curlun->dev, &dev_attr_ro);
-		if (rc)
-			goto error_luns;
-		rc = device_create_file(&curlun->dev, &dev_attr_file);
-		if (rc)
-			goto error_luns;
-		rc = device_create_file(&curlun->dev, &dev_attr_nofua);
-		if (rc)
-			goto error_luns;
-
-		if (lcfg->filename) {
-			rc = fsg_lun_open(curlun, lcfg->filename);
-			if (rc)
-				goto error_luns;
-		} else if (!curlun->removable) {
-			ERROR(common, "no file given for LUN%d\n", i);
-			rc = -EINVAL;
-			goto error_luns;
-		}
-	}
-	common->nluns = nluns;
-
-	/* Data buffers cyclic list */
-	bh = common->buffhds;
-	i = fsg_num_buffers;
-	goto buffhds_first_it;
-	do {
-		bh->next = bh + 1;
-		++bh;
-buffhds_first_it:
-		bh->buf = kmalloc(FSG_BUFLEN, GFP_KERNEL);
-		if (unlikely(!bh->buf)) {
-			rc = -ENOMEM;
-			goto error_release;
-		}
-	} while (--i);
-	bh->next = common->buffhds;
-
 	/* Prepare inquiryString */
-	if (cfg->release != 0xffff) {
+	/*if (cfg->release != 0xffff) {
 		i = cfg->release;
-	} else {
+	} else */{
 		i = usb_gadget_controller_number(gadget);
 		if (i >= 0) {
 			i = 0x0300 + i;
@@ -2858,28 +2944,78 @@ buffhds_first_it:
 		}
 	}
 	snprintf(common->inquiry_string, sizeof common->inquiry_string,
-		 "%-8s%-16s%04x", cfg->vendor_name ?: "Linux",
+		 "%-8s%-16s%04x", "Linux",
 		 /* Assume product name dependent on the first LUN */
-		 cfg->product_name ?: (common->luns->cdrom
-				     ? "File-Stor Gadget"
-				     : "File-CD Gadget"),
-		 i);
+		 /* TODO: actually check first child's "cdrom" flag */
+			"USB mass storage", i);
 
 	/*
 	 * Some peripheral controllers are known not to be able to
 	 * halt bulk endpoints correctly.  If one of them is present,
 	 * disable stalls.
 	 */
-	common->can_stall = cfg->can_stall &&
+	common->can_stall = common->can_stall &&
 		!(gadget_is_at91(common->gadget));
 
+	return common;
+
+error_release:
+	common->state = FSG_STATE_TERMINATED;	/* The thread is dead */
+	/* Call fsg_common_release() directly, ref might be not initialised. */
+	fsg_common_release(&common->ref);
+	return ERR_PTR(rc);
+}
+
+static struct fsg_common *fsg_common_init(struct fsg_common *common)
+{
+	struct fsg_buffhd *bh;
+	int i, rc;
+
+	rc = fsg_num_buffers_validate();
+	if (rc != 0)
+		return ERR_PTR(rc);
+
+	/* TODO: move it somewhere else */
+	/*if (common->nluns < 1 || common->nluns > FSG_MAX_LUNS) {
+		printk("invalid number of LUNs: %u\n", nluns);
+		return ERR_PTR(-EINVAL);
+	}*/
+
+	common->buffhds = kcalloc(fsg_num_buffers,
+				  sizeof *(common->buffhds), GFP_KERNEL);
+	if (!common->buffhds)
+		return ERR_PTR(-ENOMEM);
+
+	common->ops = NULL;
+	common->private_data = NULL;
+
+	init_rwsem(&common->filesem);
+
+	common->lun_name_format = common->lun_name_format ?
+			common->lun_name_format : "lun%d";
+
+	/* Data buffers cyclic list */
+	bh = common->buffhds;
+	i = fsg_num_buffers;
+	goto buffhds_first_it;
+	do {
+		bh->next = bh + 1;
+		++bh;
+buffhds_first_it:
+		bh->buf = kmalloc(FSG_BUFLEN, GFP_KERNEL);
+		if (unlikely(!bh->buf)) {
+			rc = -ENOMEM;
+			goto error_release;
+		}
+	} while (--i);
+	bh->next = common->buffhds;
+
 	spin_lock_init(&common->lock);
 	kref_init(&common->ref);
 
 	/* Tell the thread to start working */
 	common->thread_task =
-		kthread_create(fsg_main_thread, common,
-			       cfg->thread_name ?: "file-storage");
+		kthread_create(fsg_main_thread, common, "file-storage");
 	if (IS_ERR(common->thread_task)) {
 		rc = PTR_ERR(common->thread_task);
 		goto error_release;
@@ -2888,39 +3024,15 @@ buffhds_first_it:
 	init_waitqueue_head(&common->fsg_wait);
 
 	/* Information */
-	INFO(common, FSG_DRIVER_DESC ", version: " FSG_DRIVER_VERSION "\n");
-	INFO(common, "Number of LUNs=%d\n", common->nluns);
+	pr_info(FSG_DRIVER_DESC ", version: " FSG_DRIVER_VERSION "\n");
+	pr_info("Number of LUNs=%d\n", common->nluns);
 
-	pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
-	for (i = 0, nluns = common->nluns, curlun = common->luns;
-	     i < nluns;
-	     ++curlun, ++i) {
-		char *p = "(no medium)";
-		if (fsg_lun_is_open(curlun)) {
-			p = "(error)";
-			if (pathbuf) {
-				p = d_path(&curlun->filp->f_path,
-					   pathbuf, PATH_MAX);
-				if (IS_ERR(p))
-					p = "(error)";
-			}
-		}
-		LINFO(curlun, "LUN: %s%s%sfile: %s\n",
-		      curlun->removable ? "removable " : "",
-		      curlun->ro ? "read only " : "",
-		      curlun->cdrom ? "CD-ROM " : "",
-		      p);
-	}
-	kfree(pathbuf);
-
-	DBG(common, "I/O thread pid: %d\n", task_pid_nr(common->thread_task));
+	pr_info("I/O thread pid: %d\n", task_pid_nr(common->thread_task));
 
 	wake_up_process(common->thread_task);
 
 	return common;
 
-error_luns:
-	common->nluns = i + 1;
 error_release:
 	common->state = FSG_STATE_TERMINATED;	/* The thread is dead */
 	/* Call fsg_common_release() directly, ref might be not initialised. */
@@ -2928,9 +3040,38 @@ error_release:
 	return ERR_PTR(rc);
 }
 
+static struct config_group *alloc_fsg_common(struct config_group *group,
+					     const char *n)
+{
+	struct config_item *item;
+	struct fsg_common *common, *ret;
+
+	if (strcmp(name, "f_mass_storage"))
+		return ERR_PTR(-EINVAL);
+
+	list_for_each_entry(item, &group->cg_children, ci_entry)
+		if (!strcmp(name, item->ci_name))
+			return ERR_PTR(-EBUSY);
+
+	common = kzalloc(sizeof *common, GFP_KERNEL);
+	if (!common)
+		return ERR_PTR(-ENOMEM);
+
+	ret = fsg_common_init(common);
+	if (IS_ERR(ret)) {
+		kfree(common);
+		return (struct config_group *)ret;
+	}
+
+	config_group_init_type_name(&common->group, n, &fsg_common_item_type);
+
+	return &common->group;
+}
+
 static void fsg_common_release(struct kref *ref)
 {
 	struct fsg_common *common = container_of(ref, struct fsg_common, ref);
+	struct config_item *item;
 
 	/* If the thread isn't already dead, tell it to exit now */
 	if (common->state != FSG_STATE_TERMINATED) {
@@ -2938,20 +3079,9 @@ static void fsg_common_release(struct kref *ref)
 		wait_for_completion(&common->thread_notifier);
 	}
 
-	if (likely(common->luns)) {
-		struct fsg_lun *lun = common->luns;
-		unsigned i = common->nluns;
-
-		/* In error recovery common->nluns may be zero. */
-		for (; i; --i, ++lun) {
-			device_remove_file(&lun->dev, &dev_attr_nofua);
-			device_remove_file(&lun->dev, &dev_attr_ro);
-			device_remove_file(&lun->dev, &dev_attr_file);
-			fsg_lun_close(lun);
-			device_unregister(&lun->dev);
-		}
-
-		kfree(common->luns);
+	list_for_each_entry(item, &common->group.cg_children, ci_entry) {
+		struct fsg_lun *lun = to_fsg_lun(item);
+		fsg_lun_close(lun);
 	}
 
 	{
@@ -2963,11 +3093,8 @@ static void fsg_common_release(struct kref *ref)
 	}
 
 	kfree(common->buffhds);
-	if (common->free_storage_on_release)
-		kfree(common);
 }
 
-
 /*-------------------------------------------------------------------------*/
 
 static void fsg_unbind(struct usb_configuration *c, struct usb_function *f)
@@ -2983,6 +3110,8 @@ static void fsg_unbind(struct usb_configuration *c, struct usb_function *f)
 		wait_event(common->fsg_wait, common->fsg != fsg);
 	}
 
+	common->curlun = NULL;
+	common->lun = 0;
 	fsg_common_put(common);
 	usb_free_descriptors(fsg->function.descriptors);
 	usb_free_descriptors(fsg->function.hs_descriptors);
@@ -3074,26 +3203,41 @@ static struct usb_gadget_strings *fsg_strings_array[] = {
 	NULL,
 };
 
-static int fsg_bind_config(struct usb_composite_dev *cdev,
-			   struct usb_configuration *c,
-			   struct fsg_common *common)
+static int fsg_bind_function(struct usb_configuration *c,
+			     struct config_item *item, void *data)
 {
 	struct fsg_dev *fsg;
+	struct usb_composite_dev *cdev;
+	struct fsg_common *common;
+	struct list_head *cursor;
+	int luns;
 	int rc;
 
+	common = to_fsg_common(item);
+
+	/* refuse bind if some luns are not yet created */
+	luns = 0;
+	list_for_each(cursor, &common->group.cg_children)
+		luns++;
+	if (luns != common->nluns)
+		return -EAGAIN;
+
 	fsg = kzalloc(sizeof *fsg, GFP_KERNEL);
 	if (unlikely(!fsg))
 		return -ENOMEM;
 
-	fsg->function.name        = FSG_DRIVER_DESC;
-	fsg->function.strings     = fsg_strings_array;
-	fsg->function.bind        = fsg_bind;
-	fsg->function.unbind      = fsg_unbind;
-	fsg->function.setup       = fsg_setup;
-	fsg->function.set_alt     = fsg_set_alt;
-	fsg->function.disable     = fsg_disable;
+	fsg->function.name	= FSG_DRIVER_DESC;
+	fsg->function.strings	= fsg_strings_array;
+	fsg->function.bind	= fsg_bind;
+	fsg->function.unbind	= fsg_unbind;
+	fsg->function.setup	= fsg_setup;
+	fsg->function.set_alt	= fsg_set_alt;
+	fsg->function.disable	= fsg_disable;
+	fsg->common		= common;
+
+	cdev = data;
+	fsg_common_init_cdev(fsg->common, cdev);
 
-	fsg->common               = common;
 	/*
 	 * Our caller holds a reference to common structure so we
 	 * don't have to be worry about it being freed until we return
@@ -3109,98 +3253,3 @@ static int fsg_bind_config(struct usb_composite_dev *cdev,
 		fsg_common_get(fsg->common);
 	return rc;
 }
-
-
-/************************* Module parameters *************************/
-
-struct fsg_module_parameters {
-	char		*file[FSG_MAX_LUNS];
-	bool		ro[FSG_MAX_LUNS];
-	bool		removable[FSG_MAX_LUNS];
-	bool		cdrom[FSG_MAX_LUNS];
-	bool		nofua[FSG_MAX_LUNS];
-
-	unsigned int	file_count, ro_count, removable_count, cdrom_count;
-	unsigned int	nofua_count;
-	unsigned int	luns;	/* nluns */
-	bool		stall;	/* can_stall */
-};
-
-#define _FSG_MODULE_PARAM_ARRAY(prefix, params, name, type, desc)	\
-	module_param_array_named(prefix ## name, params.name, type,	\
-				 &prefix ## params.name ## _count,	\
-				 S_IRUGO);				\
-	MODULE_PARM_DESC(prefix ## name, desc)
-
-#define _FSG_MODULE_PARAM(prefix, params, name, type, desc)		\
-	module_param_named(prefix ## name, params.name, type,		\
-			   S_IRUGO);					\
-	MODULE_PARM_DESC(prefix ## name, desc)
-
-#define FSG_MODULE_PARAMETERS(prefix, params)				\
-	_FSG_MODULE_PARAM_ARRAY(prefix, params, file, charp,		\
-				"names of backing files or devices");	\
-	_FSG_MODULE_PARAM_ARRAY(prefix, params, ro, bool,		\
-				"true to force read-only");		\
-	_FSG_MODULE_PARAM_ARRAY(prefix, params, removable, bool,	\
-				"true to simulate removable media");	\
-	_FSG_MODULE_PARAM_ARRAY(prefix, params, cdrom, bool,		\
-				"true to simulate CD-ROM instead of disk"); \
-	_FSG_MODULE_PARAM_ARRAY(prefix, params, nofua, bool,		\
-				"true to ignore SCSI WRITE(10,12) FUA bit"); \
-	_FSG_MODULE_PARAM(prefix, params, luns, uint,			\
-			  "number of LUNs");				\
-	_FSG_MODULE_PARAM(prefix, params, stall, bool,			\
-			  "false to prevent bulk stalls")
-
-static void
-fsg_config_from_params(struct fsg_config *cfg,
-		       const struct fsg_module_parameters *params)
-{
-	struct fsg_lun_config *lun;
-	unsigned i;
-
-	/* Configure LUNs */
-	cfg->nluns =
-		min(params->luns ?: (params->file_count ?: 1u),
-		    (unsigned)FSG_MAX_LUNS);
-	for (i = 0, lun = cfg->luns; i < cfg->nluns; ++i, ++lun) {
-		lun->ro = !!params->ro[i];
-		lun->cdrom = !!params->cdrom[i];
-		lun->removable = /* Removable by default */
-			params->removable_count <= i || params->removable[i];
-		lun->filename =
-			params->file_count > i && params->file[i][0]
-			? params->file[i]
-			: 0;
-	}
-
-	/* Let MSF use defaults */
-	cfg->lun_name_format = 0;
-	cfg->thread_name = 0;
-	cfg->vendor_name = 0;
-	cfg->product_name = 0;
-	cfg->release = 0xffff;
-
-	cfg->ops = NULL;
-	cfg->private_data = NULL;
-
-	/* Finalise */
-	cfg->can_stall = params->stall;
-}
-
-static inline struct fsg_common *
-fsg_common_from_params(struct fsg_common *common,
-		       struct usb_composite_dev *cdev,
-		       const struct fsg_module_parameters *params)
-	__attribute__((unused));
-static inline struct fsg_common *
-fsg_common_from_params(struct fsg_common *common,
-		       struct usb_composite_dev *cdev,
-		       const struct fsg_module_parameters *params)
-{
-	struct fsg_config cfg;
-	fsg_config_from_params(&cfg, params);
-	return fsg_common_init(common, cdev, &cfg);
-}
-
diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
index 8081ca3..8ec7155 100644
--- a/drivers/usb/gadget/storage_common.c
+++ b/drivers/usb/gadget/storage_common.c
@@ -51,6 +51,7 @@
 
 
 #include <linux/usb/storage.h>
+#include <linux/configfs.h>
 #include <scsi/scsi.h>
 #include <asm/unaligned.h>
 
@@ -79,10 +80,9 @@
 #define VLDBG(lun, fmt, args...) do { } while (0)
 #endif /* VERBOSE_DEBUG */
 
-#define LDBG(lun, fmt, args...)   dev_dbg (&(lun)->dev, fmt, ## args)
-#define LERROR(lun, fmt, args...) dev_err (&(lun)->dev, fmt, ## args)
-#define LWARN(lun, fmt, args...)  dev_warn(&(lun)->dev, fmt, ## args)
-#define LINFO(lun, fmt, args...)  dev_info(&(lun)->dev, fmt, ## args)
+#define LERROR(lun, fmt, args...)  pr_err(fmt, ## args)
+#define LDBG(lun, fmt, args...)   pr_debug(fmt, ## args)
+#define LINFO(lun, fmt, args...)  pr_info(fmt, ## args)
 
 /*
  * Keep those macros in sync with those in
@@ -185,7 +185,6 @@ struct interrupt_data {
 
 /*-------------------------------------------------------------------------*/
 
-
 struct fsg_lun {
 	struct file	*filp;
 	loff_t		file_length;
@@ -206,16 +205,245 @@ struct fsg_lun {
 
 	unsigned int	blkbits;	/* Bits of logical block size of bound block device */
 	unsigned int	blksize;	/* logical block size of bound block device */
-	struct device	dev;
+
+	/* configfs-related section */
+	struct config_item	item;
+	struct rw_semaphore	*filesem;
+	unsigned int		n_lun;
 };
 
 #define fsg_lun_is_open(curlun)	((curlun)->filp != NULL)
 
-static struct fsg_lun *fsg_lun_from_dev(struct device *dev)
+static ssize_t fsg_lun_show_ro(struct fsg_lun *curlun, char *buf)
 {
-	return container_of(dev, struct fsg_lun, dev);
+	return sprintf(buf, "%d\n", fsg_lun_is_open(curlun)
+				  ? curlun->ro
+				  : curlun->initially_ro);
 }
 
+static ssize_t fsg_lun_show_nofua(struct fsg_lun *curlun, char *buf)
+{
+	return sprintf(buf, "%u\n", curlun->nofua);
+}
+
+static ssize_t fsg_lun_show_file(struct fsg_lun *curlun, char *buf)
+{
+	struct rw_semaphore	*filesem = curlun->filesem;
+	char		*p;
+	ssize_t		rc;
+
+	down_read(filesem);
+	if (fsg_lun_is_open(curlun)) {	/* Get the complete pathname */
+		p = d_path(&curlun->filp->f_path, buf, PAGE_SIZE - 1);
+		if (IS_ERR(p))
+			rc = PTR_ERR(p);
+		else {
+			rc = strlen(p);
+			memmove(buf, p, rc);
+			buf[rc] = '\n';		/* Add a newline */
+			buf[++rc] = 0;
+		}
+	} else {				/* No file, return 0 bytes */
+		*buf = 0;
+		rc = 0;
+	}
+	up_read(filesem);
+	return rc;
+}
+
+
+static ssize_t fsg_lun_store_ro(struct fsg_lun *curlun, const char *buf,
+				size_t count)
+{
+	ssize_t		rc;
+	struct rw_semaphore	*filesem = curlun->filesem;
+	unsigned	ro;
+
+	rc = kstrtouint(buf, 2, &ro);
+	if (rc)
+		return rc;
+
+	/*
+	 * Allow the write-enable status to change only while the
+	 * backing file is closed.
+	 */
+	down_read(filesem);
+	if (fsg_lun_is_open(curlun)) {
+		LDBG(curlun, "read-only status change prevented\n");
+		rc = -EBUSY;
+	} else {
+		curlun->ro = ro;
+		curlun->initially_ro = ro;
+		LDBG(curlun, "read-only status set to %d\n", curlun->ro);
+		rc = count;
+	}
+	up_read(filesem);
+	return rc;
+}
+
+static int fsg_lun_fsync_sub(struct fsg_lun *curlun);
+
+static ssize_t fsg_lun_store_nofua(struct fsg_lun *curlun, const char *buf,
+				   size_t count)
+{
+	unsigned	nofua;
+	int		ret;
+
+	ret = kstrtouint(buf, 2, &nofua);
+	if (ret)
+		return ret;
+
+	/* Sync data when switching from async mode to sync */
+	if (!nofua && curlun->nofua)
+		fsg_lun_fsync_sub(curlun);
+
+	curlun->nofua = nofua;
+
+	return count;
+}
+
+static int fsg_lun_open(struct fsg_lun *curlun, const char *filename);
+static void fsg_lun_close(struct fsg_lun *curlun);
+
+static ssize_t fsg_lun_store_file(struct fsg_lun *curlun, const char *buf,
+				  size_t count)
+{
+	struct rw_semaphore	*filesem = curlun->filesem;
+	int		rc = 0;
+
+	if (curlun->prevent_medium_removal && fsg_lun_is_open(curlun)) {
+		LDBG(curlun, "eject attempt prevented\n");
+		return -EBUSY;				/* "Door is locked" */
+	}
+
+	/* Remove a trailing newline */
+	if (count > 0 && buf[count-1] == '\n')
+		((char *) buf)[count-1] = 0;		/* Ugh! */
+
+	/* Eject current medium */
+	down_write(filesem);
+	if (fsg_lun_is_open(curlun)) {
+		fsg_lun_close(curlun);
+		curlun->unit_attention_data = SS_MEDIUM_NOT_PRESENT;
+	}
+
+	/* Load new medium */
+	if (count > 0 && buf[0]) {
+		rc = fsg_lun_open(curlun, buf);
+		if (rc == 0)
+			curlun->unit_attention_data =
+					SS_NOT_READY_TO_READY_TRANSITION;
+	}
+	up_write(filesem);
+	return (rc < 0 ? rc : count);
+}
+
+static ssize_t fsg_lun_show_removable(struct fsg_lun *curlun, char *buf)
+{
+	return sprintf(buf, "%d\n", curlun->removable);
+}
+
+static ssize_t fsg_lun_store_removable(struct fsg_lun *curlun, const char *buf,
+				       size_t count)
+{
+	if (fsg_lun_is_open(curlun)) {
+		LDBG(curlun, "media type change prevented\n");
+		return -EBUSY;
+	}
+
+	if (buf[0] != '0' && buf[0] != '1')
+		return -EINVAL;
+
+	curlun->removable = buf[0] == '1';
+
+	return count;
+}
+
+CONFIGFS_ATTR_STRUCT(fsg_lun);
+
+#define FSG_LUN_ATTR_RW(_name)						\
+static struct fsg_lun_attribute fsg_lun_##_name =			\
+	__CONFIGFS_ATTR(_name, S_IRUGO | S_IWUSR, fsg_lun_show_##_name,	\
+			fsg_lun_store_##_name)
+
+FSG_LUN_ATTR_RW(ro);
+FSG_LUN_ATTR_RW(nofua);
+FSG_LUN_ATTR_RW(file);
+FSG_LUN_ATTR_RW(removable);
+
+static struct configfs_attribute *fsg_lun_attrs[] = {
+	&fsg_lun_ro.attr,
+	&fsg_lun_nofua.attr,
+	&fsg_lun_file.attr,
+	&fsg_lun_removable.attr,
+	NULL,
+};
+
+static struct fsg_lun *to_fsg_lun(struct config_item *item)
+{
+	return item ? container_of(item, struct fsg_lun, item) : NULL;
+}
+
+CONFIGFS_ATTR_OPS(fsg_lun);
+
+static int (*gadget_verify_connected)(struct config_item *item);
+
+void fsg_set_gadget_verify_connected(int (*f)(struct config_item *item))
+{
+	gadget_verify_connected = f;
+}
+
+static int fsg_lun_check_rmdir(struct config_group *parent,
+			       struct config_item *item)
+{
+	/*
+	 * a lun item is a child of a f_mass_storage group, which is a child
+	 * of a function group, which is a child of a configuration group,
+	 * which is a child of a gadget group
+	 */
+	struct fsg_lun *lun;
+	struct config_item *function, *config, *gadget;
+
+	lun = to_fsg_lun(item);
+
+	if (fsg_lun_is_open(lun))
+		return -EBUSY;
+
+	if (!gadget_verify_connected || !parent)
+		return 0;
+
+	function = parent->cg_item.ci_parent;
+	if (!function)
+		return 0;
+
+	config = function->ci_parent;
+	if (!config)
+		return 0;
+
+	gadget = config->ci_parent;
+	if (!gadget)
+		return 0;
+
+	return gadget_verify_connected(gadget);
+}
+
+static void fsg_lun_item_release(struct config_item *item)
+{
+	kfree(to_fsg_lun(item));
+}
+
+static struct configfs_item_operations fsg_lun_ops = {
+	.show_attribute		= fsg_lun_attr_show,
+	.store_attribute	= fsg_lun_attr_store,
+	.check_rmdir		= fsg_lun_check_rmdir,
+	.release		= fsg_lun_item_release,
+};
+
+static struct config_item_type fsg_lun_item_type = {
+	.ct_attrs	= fsg_lun_attrs,
+	.ct_item_ops	= &fsg_lun_ops,
+	.ct_owner	= THIS_MODULE,
+};
 
 /* Big enough to hold our biggest descriptor */
 #define EP0_BUFSIZE	256
@@ -243,7 +471,7 @@ static inline int fsg_num_buffers_validate(void)
 	if (fsg_num_buffers >= 2 && fsg_num_buffers <= 4)
 		return 0;
 	pr_err("fsg_num_buffers %u is out of range (%d to %d)\n",
-	       fsg_num_buffers, 2 ,4);
+	       fsg_num_buffers, 2, 4);
 	return -EINVAL;
 }
 
@@ -768,133 +996,3 @@ static void store_cdrom_address(u8 *dest, int msf, u32 addr)
 /*-------------------------------------------------------------------------*/
 
 
-static ssize_t fsg_show_ro(struct device *dev, struct device_attribute *attr,
-			   char *buf)
-{
-	struct fsg_lun	*curlun = fsg_lun_from_dev(dev);
-
-	return sprintf(buf, "%d\n", fsg_lun_is_open(curlun)
-				  ? curlun->ro
-				  : curlun->initially_ro);
-}
-
-static ssize_t fsg_show_nofua(struct device *dev, struct device_attribute *attr,
-			      char *buf)
-{
-	struct fsg_lun	*curlun = fsg_lun_from_dev(dev);
-
-	return sprintf(buf, "%u\n", curlun->nofua);
-}
-
-static ssize_t fsg_show_file(struct device *dev, struct device_attribute *attr,
-			     char *buf)
-{
-	struct fsg_lun	*curlun = fsg_lun_from_dev(dev);
-	struct rw_semaphore	*filesem = dev_get_drvdata(dev);
-	char		*p;
-	ssize_t		rc;
-
-	down_read(filesem);
-	if (fsg_lun_is_open(curlun)) {	/* Get the complete pathname */
-		p = d_path(&curlun->filp->f_path, buf, PAGE_SIZE - 1);
-		if (IS_ERR(p))
-			rc = PTR_ERR(p);
-		else {
-			rc = strlen(p);
-			memmove(buf, p, rc);
-			buf[rc] = '\n';		/* Add a newline */
-			buf[++rc] = 0;
-		}
-	} else {				/* No file, return 0 bytes */
-		*buf = 0;
-		rc = 0;
-	}
-	up_read(filesem);
-	return rc;
-}
-
-
-static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
-			    const char *buf, size_t count)
-{
-	ssize_t		rc;
-	struct fsg_lun	*curlun = fsg_lun_from_dev(dev);
-	struct rw_semaphore	*filesem = dev_get_drvdata(dev);
-	unsigned	ro;
-
-	rc = kstrtouint(buf, 2, &ro);
-	if (rc)
-		return rc;
-
-	/*
-	 * Allow the write-enable status to change only while the
-	 * backing file is closed.
-	 */
-	down_read(filesem);
-	if (fsg_lun_is_open(curlun)) {
-		LDBG(curlun, "read-only status change prevented\n");
-		rc = -EBUSY;
-	} else {
-		curlun->ro = ro;
-		curlun->initially_ro = ro;
-		LDBG(curlun, "read-only status set to %d\n", curlun->ro);
-		rc = count;
-	}
-	up_read(filesem);
-	return rc;
-}
-
-static ssize_t fsg_store_nofua(struct device *dev,
-			       struct device_attribute *attr,
-			       const char *buf, size_t count)
-{
-	struct fsg_lun	*curlun = fsg_lun_from_dev(dev);
-	unsigned	nofua;
-	int		ret;
-
-	ret = kstrtouint(buf, 2, &nofua);
-	if (ret)
-		return ret;
-
-	/* Sync data when switching from async mode to sync */
-	if (!nofua && curlun->nofua)
-		fsg_lun_fsync_sub(curlun);
-
-	curlun->nofua = nofua;
-
-	return count;
-}
-
-static ssize_t fsg_store_file(struct device *dev, struct device_attribute *attr,
-			      const char *buf, size_t count)
-{
-	struct fsg_lun	*curlun = fsg_lun_from_dev(dev);
-	struct rw_semaphore	*filesem = dev_get_drvdata(dev);
-	int		rc = 0;
-
-	if (curlun->prevent_medium_removal && fsg_lun_is_open(curlun)) {
-		LDBG(curlun, "eject attempt prevented\n");
-		return -EBUSY;				/* "Door is locked" */
-	}
-
-	/* Remove a trailing newline */
-	if (count > 0 && buf[count-1] == '\n')
-		((char *) buf)[count-1] = 0;		/* Ugh! */
-
-	/* Eject current medium */
-	down_write(filesem);
-	if (fsg_lun_is_open(curlun)) {
-		fsg_lun_close(curlun);
-		curlun->unit_attention_data = SS_MEDIUM_NOT_PRESENT;
-	}
-
-	/* Load new medium */
-	if (count > 0 && buf[0]) {
-		rc = fsg_lun_open(curlun, buf);
-		if (rc == 0)
-			curlun->unit_attention_data =
-					SS_NOT_READY_TO_READY_TRANSITION;
-	}
-	up_write(filesem);
-	return (rc < 0 ? rc : count);
-}
diff --git a/drivers/usb/gadget/usb_functions.c b/drivers/usb/gadget/usb_functions.c
new file mode 100644
index 0000000..8d4b9ae
--- /dev/null
+++ b/drivers/usb/gadget/usb_functions.c
@@ -0,0 +1,731 @@
+/*
+ * USB Functions Gadget
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * Author: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
+ *
+ * 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/configfs.h>
+
+/*
+ * Directly including C files for experimental version
+ *
+ */
+#include "composite.c"
+#include "usbstring.c"
+#include "config.c"
+#include "epautoconf.c"
+
+/*
+ * Supported functions
+ */
+#include "f_mass_storage.c"
+
+/*-------------------------------------------------------------------------*/
+
+#define DRIVER_DESC		"USB Functions Gadget"
+#define DRIVER_VERSION		"Pawelek"
+
+#define VENDOR_ID		0x1d6b /* Linux Foundation */
+#define PRODUCT_ID		0x0108
+#define BCD_DEVICE		0xffff
+
+/*-----------  helper functions and macros for configfs support  ----------*/
+
+#define UFG_STR_LEN		256
+
+#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)	\
+{									\
+	grp->_member = (ushort)ufg_read_twobyte(buf);			\
+	grp->_member##_set = true;					\
+									\
+	return count;							\
+}
+
+#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)	\
+{									\
+	strlcpy(grp->_member, buf, UFG_STR_LEN);			\
+	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)
+
+typedef struct config_group *(*make_group_fn)(struct config_group *group,
+					      const char *name);
+typedef int (*bind_function_fn)(struct usb_configuration *c,
+				struct config_item *item, void *data);
+
+static ssize_t ufg_read_twobyte(const char *buf)
+{
+	unsigned long tmp;
+	char *p = (char *)buf;
+
+	tmp = simple_strtoul(p, &p, 10);
+	if (!p || (*p && (*p != '\n')))
+		return -EINVAL;
+
+	if (tmp > 16383)
+		return -ERANGE;
+
+	return tmp;
+}
+
+/*--------------------  handling of dynamic make_group --------------------*/
+
+struct ufg_fn {
+	const char		*name;
+	struct config_group	*group;
+	make_group_fn		make_group;
+	bind_function_fn	bind;
+};
+
+static struct ufg_fn f_mass_storage = {
+	.name		= "f_mass_storage",
+	.make_group	= alloc_fsg_common,
+	.bind		= fsg_bind_function,
+};
+
+static struct ufg_fn *available_functions[] = {
+	&f_mass_storage,
+	NULL,
+};
+
+struct ufg_fn *ufg_fn_by_name(const char *name)
+{
+	struct ufg_fn **f;
+
+	f = available_functions;
+
+	while (*f) {
+		if (sysfs_streq((*f)->name, name))
+			return *f;
+		f++;
+	}
+
+	return NULL;
+}
+
+struct ufg_fn *ufg_fn_by_group(struct config_group *group)
+{
+	struct ufg_fn **f;
+
+	f = available_functions;
+
+	while (*f) {
+		if ((*f)->group == group)
+			return *f;
+		f++;
+	}
+
+	return NULL;
+}
+
+static struct config_group *ufg_make_function_subgroup(
+	struct config_group *group, const char *name)
+{
+	struct ufg_fn *fn;
+
+	fn = ufg_fn_by_name(name);
+	if (!fn || !fn->make_group)
+		return ERR_PTR(-EINVAL);
+
+	if (fn->group)
+		return ERR_PTR(-EBUSY);
+
+	fn->group = group;
+
+	return fn->make_group(group, name);
+}
+
+/*------------------  USB function-level configfs group  ------------------*/
+
+#define UFG_FUNC_NAME_LEN	UFG_STR_LEN
+
+struct ufg_function_grp {
+	/* This group needs children */
+	struct config_group group;
+
+	/* attributes' values */
+	char name[UFG_FUNC_NAME_LEN];
+};
+
+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)
+{
+	struct ufg_fn *fn;
+
+	fn = ufg_fn_by_name(buf);
+	if (!fn)
+		return -EINVAL;
+
+	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 configuration-level configfs group  ----------------*/
+
+struct ufg_config_grp {
+	/* This group needs children */
+	struct config_group group;
+
+	bool	added;
+
+	/* 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  --------------------*/
+
+#define UFG_NAME_LEN		UFG_STR_LEN
+
+struct ufg_gadget_grp {
+	/* This group needs children */
+	struct config_group group;
+
+	/* attributes' values */
+	ushort	idVendor;
+	bool	idVendor_set;
+	ushort	idProduct;
+	bool	idProduct_set;
+	ushort	bcdDevice;
+	bool	bcdDevice_set;
+	char	iManufacturer[UFG_NAME_LEN];
+	bool	iManufacturer_set;
+	char	iProduct[UFG_NAME_LEN];
+	bool	iProduct_set;
+	char	iSerialNumber[UFG_NAME_LEN];
+	bool	iSerialNumber_set;
+	bool	connect;
+};
+
+static int ufg_gadget_bind(struct ufg_gadget_grp *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(connect, ufg_gadget_grp, connect);
+static ssize_t ufg_gadget_grp_store_connect(struct ufg_gadget_grp *grp,
+					    const char *buf, size_t count)
+{
+	bool connect;
+	int ret;
+
+	if (buf[0] != '0' && buf[0] != '1')
+		return -EINVAL;
+
+	connect = grp->connect;
+	grp->connect = buf[0] == '1';
+
+	if (connect && grp->connect)
+		return -EBUSY;
+
+	ret = ufg_gadget_bind(grp);
+	if (ret) {
+		grp->connect = connect;
+		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(connect, connect, 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_connect.attr,
+	NULL,
+};
+
+static 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;
+}
+
+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 *gadget;
+
+	gadget = kzalloc(sizeof *gadget, GFP_KERNEL);
+	if (!gadget)
+		return ERR_PTR(-ENOMEM);
+
+	config_group_init_type_name(&gadget->group, name, &ufg_gadget_grp_type);
+
+	return &gadget->group;
+}
+
+static int ufg_gadget_grp_check_connected(struct config_item *item)
+{
+	struct ufg_gadget_grp *gadget;
+
+	gadget = to_ufg_gadget_grp(item);
+
+	if (gadget)
+		return gadget->connect ? -EBUSY : 0;
+
+	return 0;
+}
+
+/*-------------------  configfs subsystem for ufg  ------------------------*/
+
+#define UFG_FUNC_NAMES_BUF_LEN	UFG_STR_LEN
+
+struct ufg_subsys {
+	/* This is the root of the subsystem */
+	struct configfs_subsystem subsys;
+
+	/* attributes' values */
+	char	avail_func[UFG_FUNC_NAMES_BUF_LEN];
+};
+
+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,
+};
+
+static 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 struct usb_device_descriptor ufg_device_desc = {
+	.bLength              = sizeof(ufg_device_desc),
+	.bDescriptorType      = USB_DT_DEVICE,
+	.bcdUSB               = __constant_cpu_to_le16(0x0200),
+	.bDeviceClass         = USB_CLASS_PER_INTERFACE,
+	.idVendor             = __constant_cpu_to_le16(VENDOR_ID),
+	.idProduct            = __constant_cpu_to_le16(PRODUCT_ID),
+	.bcdDevice            = __constant_cpu_to_le16(BCD_DEVICE),
+	.bNumConfigurations   = 1,
+};
+
+static struct usb_composite_driver ufg_driver = {
+	.name		= "usb_function_gadget",
+	.dev		= &ufg_device_desc,
+	.iProduct	= DRIVER_DESC,
+	.needs_serial	= 1,
+};
+
+static void ufg_unbind_config(struct usb_configuration *c);
+
+static struct usb_configuration ufg_config_driver = {
+	.label		= "ufg",
+	.unbind		= ufg_unbind_config,
+	.bConfigurationValue = 1,
+	.bmAttributes	= USB_CONFIG_ATT_ONE | USB_CONFIG_ATT_SELFPOWER,
+	.bMaxPower	= 0xFA, /* 500ma */
+};
+
+static struct usb_composite_dev *_cdev;
+
+static int ufg_bind(struct usb_composite_dev *cdev)
+{
+	struct usb_gadget	*gadget = cdev->gadget;
+	int			gcnum;
+
+	_cdev = cdev;
+	/*
+	 * Start disconnected. Userspace will connect the gadget once
+	 * it is done configuring the functions.
+	 */
+	usb_gadget_disconnect(gadget);
+
+	gcnum = usb_gadget_controller_number(gadget);
+	if (gcnum >= 0)
+		ufg_device_desc.bcdDevice = cpu_to_le16(0x0200 + gcnum);
+	else {
+		pr_warn("%s: controller '%s' not recognized\n",
+			DRIVER_DESC, gadget->name);
+		ufg_device_desc.bcdDevice = __constant_cpu_to_le16(0x9999);
+	}
+
+	usb_gadget_set_selfpowered(gadget);
+
+	pr_info("%s: version: %s\n", DRIVER_DESC, DRIVER_VERSION);
+
+	return 0;
+}
+
+static struct usb_configuration *_config;
+
+static int ufg_bind_config(struct usb_configuration *c)
+{
+	_config = c;
+
+	return 0;
+}
+
+static void ufg_unbind_config(struct usb_configuration *c)
+{
+}
+
+static void update_ufg_driver(struct ufg_gadget_grp *ufg_gadget)
+{
+	if (ufg_gadget->idVendor_set)
+		idVendor = __constant_cpu_to_le16(ufg_gadget->idVendor);
+	if (ufg_gadget->idProduct_set)
+		idProduct = __constant_cpu_to_le16(ufg_gadget->idProduct);
+	if (ufg_gadget->bcdDevice_set)
+		bcdDevice = __constant_cpu_to_le16(ufg_gadget->bcdDevice);
+	if (ufg_gadget->iManufacturer_set)
+		strlcpy(iManufacturer, ufg_gadget->iManufacturer, UFG_NAME_LEN);
+	if (ufg_gadget->iProduct_set)
+		strlcpy(iProduct, ufg_gadget->iProduct, UFG_NAME_LEN);
+	if (ufg_gadget->iSerialNumber_set)
+		strlcpy(iSerialNumber, ufg_gadget->iSerialNumber, UFG_NAME_LEN);
+}
+
+static int ufg_gadget_bind(struct ufg_gadget_grp *ufg_gadget)
+{
+	struct config_item *ci;
+	int r;
+
+	if (!ufg_gadget->connect) {
+		usb_composite_unregister(&ufg_driver);
+		return 0;
+	}
+
+	update_ufg_driver(ufg_gadget);
+	r = usb_composite_probe(&ufg_driver, ufg_bind);
+
+	if (r)
+		return r;
+
+	list_for_each_entry(ci, &ufg_gadget->group.cg_children, ci_entry) {
+		struct ufg_config_grp *config;
+		struct config_item *f;
+
+		config = to_ufg_config_grp(ci);
+
+		ufg_config_driver.bMaxPower = config->max_power;
+		r = usb_add_config(_cdev, &ufg_config_driver, ufg_bind_config);
+		if (r)
+			goto unbind;
+		config->added = true;
+
+		list_for_each_entry(f, &config->group.cg_children, ci_entry) {
+			struct ufg_fn *ufg_fn;
+
+			ufg_fn = ufg_fn_by_group(to_config_group(f));
+
+			if (ufg_fn && ufg_fn->bind) {
+				struct config_item *subgroup;
+				struct config_group *group;
+
+				group = to_config_group(f);
+				subgroup = container_of(group->cg_children.next,
+						struct config_item, ci_entry);
+				r = ufg_fn->bind(_config, subgroup, _cdev);
+				if (r)
+					goto unbind;
+			}
+		}
+
+	}
+
+	return 0;
+
+unbind:
+	list_for_each_entry(ci, &ufg_gadget->group.cg_children, ci_entry) {
+		struct ufg_config_grp *config;
+
+		config = to_ufg_config_grp(ci);
+		if (!config->added)
+			continue;
+
+		usb_remove_config(_cdev, &ufg_config_driver);
+		config->added = false;
+	}
+	usb_composite_unregister(&ufg_driver);
+	return r;
+}
+
+
+/*----------------------  general module stuff  ---------------------------*/
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_AUTHOR("Andrzej Pietrasiewicz");
+MODULE_LICENSE("GPL");
+
+static int __init ufg_init(void)
+{
+	int ret;
+
+	fsg_set_gadget_verify_connected(ufg_gadget_grp_check_connected);
+
+	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;
+
+	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);
-- 
1.7.0.4

Re: [RFC 0/2] USB gadget - configfs

[Sebastian Andrzej Siewior]

On 06/21/2012 12:55 PM, Andrzej Pietrasiewicz wrote:
> Dear All,
>
> This patch series is a follow-up to this thread:
>
> http://marc.info/?l=linux-usb&m=132430533824884&w=2

Ah, good :) The changes to composite are way smaller than I expected
them to be. I will take a deeper look, later tonight.

Sebastian

Re: [RFC 0/2] USB gadget - configfs

[Sebastian Andrzej Siewior]

On 06/21/2012 12:55 PM, Andrzej Pietrasiewicz wrote:
> $ echo 1>   /cfg/usb-function-gadget/G1/connect
> $ file.img>  /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/lun0/file

connect should be available at UDC level which triggers the
pull-up/down of the UDC. The gadget itself should be configured once
and made available / bound to the UDC. And you may have more
than once UDC at a time within a single a system.

> Each function, after creating its corresponding directory
> (/cfg/usb-function-gadget/G1/C1/F1), must be "personalized" by storing
> its name in the "name" attribute. After that it is possible to create
> a child item of the same name ("f_mass_storage" here). The common code
> handles everything from top of the hierarchy up to the function directory.
> Under the function directory a function-specific stuff provided by each
> function is used. The function-specific code is abstracted by the above
> mentioned struct ufg_fn. In the example, the mass storage function is
> supplied with one LUN.

The hierarchy looks good. C1 vs config1 is just taste.

> The "connect" attribute's store method calls the ufg_gadget_bind function,
> which registers the composite gadget, then walks the configfs hierarchy
> rooted at the above mentioned subsystem and does USB configurations and
> functions registration.


> This is a work in progress. There might be issues.
>
> I would like to ask some questions. All answers in general, and answers
> from linux-usb and Felipe and Greg KH in particular, are welcome.
>
> 1. Generally, is this the right way to go?
In the big picture I think, yes. I think you should start a little
different. We need to be able to load multiple gadgets at a time.
Therefore I would suggest to first rip the global variables out of
composite. It is probably also possible to do it after configfs is in
but I think doing it first would be less work.

> 2. Using configfs like this calls for an interface between the generic
>     configfs-related code and function-specific code. I suggested the
>     struct ufg_fn. What do you think?
> 3. Should some parameters still be available through sysfs?
We have for udc level some things like connect, power level and Felipe
planned a poll interface for connect/disconnect notifications. This is
one side. On the gadget it should configure a custom gadget as detailed
as possible / required. What kind of options would you like to export 
via sysfs? Currently I can't think of any.

> 4. Do we need module parameters for USB descriptors like iManufacturer
>     and similar?
No. No modules parameters at all. With one exception: Currently we set
those things via modprobe. As a for compatibility interface I guess we
have to keep this. In the end I would like have f_mass_storage gone as 
it. We would have a new gadget_storage which provides only the function 
and relies on configfs module and nothing else. The replacement
g_mass_storage would simply be a small module which takes the
iManufacturer parameters and others and configures the gadget the way
the old gadget did i.e. without echo this and echo that.

> 5. I assumed that the configfs entities are contained in the structures
>     used for configuring the USB functions, e.g. a struct config_group in
>     struct fsg_common, or a struct config_item in a struct fsg_lun. This
>     has implications that the lifetime of the structures is controlled by
>     userspace through configfs, which, in turn, has influence on how
>     the USB functions are implemented. Even though it seems natural,
>     there are some issues. For example an extension to configfs was required
>     in order to disable deleting the luns while the gadget is connected.
>     Is this the right approach? If not, then are there any alternatives?

Joel might be a help here. I had the same problem with my uas gadget
and I ignored it for now. I think it should be possible to forbid
unlink / rename / create at certain states of gadget. For instance it
should not be possible to change a single thing while the gadget is
connected.

Sebastian

Re: [RFC 0/2] USB gadget - configfs

[Alan Stern]

On Sun, 24 Jun 2012, Sebastian Andrzej Siewior wrote:

> On 06/21/2012 12:55 PM, Andrzej Pietrasiewicz wrote:
> > $ echo 1>   /cfg/usb-function-gadget/G1/connect
> > $ file.img>  /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/lun0/file
> 
> connect should be available at UDC level which triggers the
> pull-up/down of the UDC. The gadget itself should be configured once
> and made available / bound to the UDC. And you may have more
> than once UDC at a time within a single a system.
> 
> > Each function, after creating its corresponding directory
> > (/cfg/usb-function-gadget/G1/C1/F1), must be "personalized" by storing
> > its name in the "name" attribute. After that it is possible to create
> > a child item of the same name ("f_mass_storage" here). The common code
> > handles everything from top of the hierarchy up to the function directory.
> > Under the function directory a function-specific stuff provided by each
> > function is used. The function-specific code is abstracted by the above
> > mentioned struct ufg_fn. In the example, the mass storage function is
> > supplied with one LUN.
> 
> The hierarchy looks good. C1 vs config1 is just taste.

My taste prefers "config1".  Same for "gadget1" and "func1" (or 
"function1").  Words -- even short or abbreviated words -- are a lot 
more meaningful to people than single letters are.

> > 4. Do we need module parameters for USB descriptors like iManufacturer
> >     and similar?
> No. No modules parameters at all. With one exception: Currently we set
> those things via modprobe. As a for compatibility interface I guess we
> have to keep this. In the end I would like have f_mass_storage gone as 
> it. We would have a new gadget_storage which provides only the function 
> and relies on configfs module and nothing else. The replacement
> g_mass_storage would simply be a small module which takes the
> iManufacturer parameters and others and configures the gadget the way
> the old gadget did i.e. without echo this and echo that.

In any case, iManufacturer should not be adjustable by the user.  It 
should always be managed by the composite-gadget layer, since it 
contains nothing but the index number of the Manufacturer string 
descriptor.

Alan Stern

Re: [RFC 1/2] fs: configfs: add check_rmdir operation

[Joel Becker]

NAK.  You should be using configfs_(un)depend_item() if you require the
item to stay alive.

Joel

On Thu, Jun 21, 2012 at 12:55:28PM +0200, Andrzej Pietrasiewicz wrote:
> The logic behind a subsystem which uses configfs might be that it is not
> desired to allow removing the pseudo directories (items or groups) in a
> mounted configfs while some criterion is not met, e.g. while some
> actions are in progress. This patch adds a check_rmdir operation to
> configfs_item_operations and to configfs_group_operations. The operation,
> if provided, is called before actual remove takes place and if the result
> is nonzero, the remove is not done and the error it returns is reported.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  fs/configfs/dir.c        |   20 ++++++++++++++++++++
>  include/linux/configfs.h |    2 ++
>  2 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
> index 7e6c52d..de2680f 100644
> --- a/fs/configfs/dir.c
> +++ b/fs/configfs/dir.c
> @@ -1408,12 +1408,32 @@ static int configfs_rmdir(struct inode *dir, struct dentry *dentry)
>  		dead_item_owner = item->ci_type->ct_owner;
>  
>  	if (sd->s_type & CONFIGFS_USET_DIR) {
> +		if (item->ci_type && item->ci_type->ct_group_ops &&
> +		    item->ci_type->ct_group_ops->check_rmdir) {
> +		    	ret = item->ci_type->ct_group_ops->check_rmdir(
> +				to_config_group(parent_item), item);
> +			if (ret) {
> +				config_item_put(item);
> +				configfs_detach_rollback(dentry);
> +				return ret;
> +			}
> +		}
>  		configfs_detach_group(item);
>  
>  		mutex_lock(&subsys->su_mutex);
>  		client_disconnect_notify(parent_item, item);
>  		unlink_group(to_config_group(item));
>  	} else {
> +		if (item->ci_type && item->ci_type->ct_item_ops &&
> +		    item->ci_type->ct_item_ops->check_rmdir) {
> +		    	ret = item->ci_type->ct_item_ops->check_rmdir(
> +				to_config_group(parent_item), item);
> +			if (ret) {
> +				config_item_put(item);
> +				configfs_detach_rollback(dentry);
> +				return ret;
> +			}
> +		}
>  		configfs_detach_item(item);
>  
>  		mutex_lock(&subsys->su_mutex);
> diff --git a/include/linux/configfs.h b/include/linux/configfs.h
> index 34025df..f356a55 100644
> --- a/include/linux/configfs.h
> +++ b/include/linux/configfs.h
> @@ -225,6 +225,7 @@ struct configfs_item_operations {
>  	void (*release)(struct config_item *);
>  	ssize_t	(*show_attribute)(struct config_item *, struct configfs_attribute *,char *);
>  	ssize_t	(*store_attribute)(struct config_item *,struct configfs_attribute *,const char *, size_t);
> +	int (*check_rmdir)(struct config_group *group, struct config_item *item);
>  	int (*allow_link)(struct config_item *src, struct config_item *target);
>  	int (*drop_link)(struct config_item *src, struct config_item *target);
>  };
> @@ -233,6 +234,7 @@ struct configfs_group_operations {
>  	struct config_item *(*make_item)(struct config_group *group, const char *name);
>  	struct config_group *(*make_group)(struct config_group *group, const char *name);
>  	int (*commit_item)(struct config_item *item);
> +	int (*check_rmdir)(struct config_group *group, struct config_item *item);
>  	void (*disconnect_notify)(struct config_group *group, struct config_item *item);
>  	void (*drop_item)(struct config_group *group, struct config_item *item);
>  };
> -- 
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 

Dort wo man Bücher verbrennt, verbrennt man am Ende auch Mensch.
(Wherever they burn books, they will also end up burning people.)
	- Heinrich Heine

			http://www.jlbec.org/
			jlbec@evilplan.org

Re: [RFC 0/2] USB gadget - configfs

[Joel Becker]

On Thu, Jun 21, 2012 at 12:55:27PM +0200, Andrzej Pietrasiewicz wrote:
> Dear All,
> 
> This patch series is a follow-up to this thread:
> 
> http://marc.info/?l=linux-usb&m=132430533824884&w=2

Dear Andrzej,
	I read this thread just now, and I wanted to comment.  I think
configfs fits your needs nicely here.  I'd like to affirm your
supposition that hiding the details in a library is the right way to go.
 configfs aims to provide nice, predictable, transparent user<->kernel
API for creating and configuring objects, but it doesn't have to be what
the end user or administrator sees in everyday use.  Most of the other
configfs clients (ocfs2, dlm, target) wrap configfs usage in their
tools.

> As a prerequisite it adds an operation to configfs. The operation allows
> checking if it is ok to remove a pseudo directory corresponding to a
> configfs item/group.

	I NAK'd that patch because you should be using
configfs_depend_item().  If you have trouble with that, let's talk.

<snip>
 
> I would like to ask some questions. All answers in general, and answers
> from linux-usb and Felipe and Greg KH in particular, are welcome.
> 
> 1. Generally, is this the right way to go?

LGTM.

> 2. Using configfs like this calls for an interface between the generic
>    configfs-related code and function-specific code. I suggested the
>    struct ufg_fn. What do you think?
> 3. Should some parameters still be available through sysfs?

	If they fit, yes.  As a rule of thumb, if userspace directed the
object creation, it should be in configfs.  But if kernelspace created
the object, sysfs is the better location.  There is wiggle room here.
Do not be afraid to use both.  configfs was created because sysfs didn't
fit userspace-directed creation.  configfs is not intended to replace
all of the things sysfs does better.

> 4. Do we need module parameters for USB descriptors like iManufacturer
>    and similar?
> 5. I assumed that the configfs entities are contained in the structures
>    used for configuring the USB functions, e.g. a struct config_group in
>    struct fsg_common, or a struct config_item in a struct fsg_lun. This
>    has implications that the lifetime of the structures is controlled by
>    userspace through configfs, which, in turn, has influence on how
>    the USB functions are implemented. Even though it seems natural,
>    there are some issues. For example an extension to configfs was required
>    in order to disable deleting the luns while the gadget is connected.
>    Is this the right approach? If not, then are there any alternatives?

	Yes, embedding in the structures is correct.  These are
userspace-created objects, and thus userspace has control over their
lifetimes.  That's intentional.  See above about configfs_depend_item()
for blocking unsafe removal.

Joel

-- 

"Hey mister if you're gonna walk on water,
 Could you drop a line my way?"

			http://www.jlbec.org/
			jlbec@evilplan.org

Re: [RFC 0/2] USB gadget - configfs

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2604 bytes --]

On Mon, Jun 25, 2012 at 10:11:43AM -0400, Alan Stern wrote:
> On Sun, 24 Jun 2012, Sebastian Andrzej Siewior wrote:
> 
> > On 06/21/2012 12:55 PM, Andrzej Pietrasiewicz wrote:
> > > $ echo 1>   /cfg/usb-function-gadget/G1/connect
> > > $ file.img>  /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/lun0/file
> > 
> > connect should be available at UDC level which triggers the
> > pull-up/down of the UDC. The gadget itself should be configured once
> > and made available / bound to the UDC. And you may have more
> > than once UDC at a time within a single a system.
> > 
> > > Each function, after creating its corresponding directory
> > > (/cfg/usb-function-gadget/G1/C1/F1), must be "personalized" by storing
> > > its name in the "name" attribute. After that it is possible to create
> > > a child item of the same name ("f_mass_storage" here). The common code
> > > handles everything from top of the hierarchy up to the function directory.
> > > Under the function directory a function-specific stuff provided by each
> > > function is used. The function-specific code is abstracted by the above
> > > mentioned struct ufg_fn. In the example, the mass storage function is
> > > supplied with one LUN.
> > 
> > The hierarchy looks good. C1 vs config1 is just taste.
> 
> My taste prefers "config1".  Same for "gadget1" and "func1" (or 
> "function1").  Words -- even short or abbreviated words -- are a lot 
> more meaningful to people than single letters are.

I agree with Alan here, though I'd prefer function1 instead of func1.

> > > 4. Do we need module parameters for USB descriptors like iManufacturer
> > >     and similar?
> > No. No modules parameters at all. With one exception: Currently we set
> > those things via modprobe. As a for compatibility interface I guess we
> > have to keep this. In the end I would like have f_mass_storage gone as 
> > it. We would have a new gadget_storage which provides only the function 
> > and relies on configfs module and nothing else. The replacement
> > g_mass_storage would simply be a small module which takes the
> > iManufacturer parameters and others and configures the gadget the way
> > the old gadget did i.e. without echo this and echo that.
> 
> In any case, iManufacturer should not be adjustable by the user.  It 
> should always be managed by the composite-gadget layer, since it 
> contains nothing but the index number of the Manufacturer string 
> descriptor.

that's a misname, actually. The iManufacturer parameter is waiting for
the actual manufacturer string.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

RE: [RFC 0/2] USB gadget - configfs

[Andrzej Pietrasiewicz]

Dear Joel,

Thank you for your review.

@Sebastian, Alan, Felipe: Thank you, too.

On Monday, July 02, 2012 11:09 AM Joel Becker wrote:

<snip>

> 
> > As a prerequisite it adds an operation to configfs. The operation allows
> > checking if it is ok to remove a pseudo directory corresponding to a
> > configfs item/group.
> 
> 	I NAK'd that patch because you should be using
> configfs_depend_item().  If you have trouble with that, let's talk.
> 

Now I see the configfs_depend_item() is the way to go. I am in doubt,
though, so could you please throw some light on it? Here is why:
As an example I did a quick-and-dirty port of f_mass_storage to the new,
configfs-based approach. The business logic of this function is that
once a lun is opened, it must not be changed (deleted, in particular)
until it is closed. The moment the lun is opened is defined by a write
to a configfs "file" attribute of a lun config item:

               +-/lunX
               |  |
               |  +-file
               |  |
               |  +-nofua
               |  |
               |  +-removable
               |  |
               |  +-ro

So, the config item corresponding to the lun becomes depended on during
the write file operation, the same with undepend. Can this be expressed
with configfs_depend/undepend_item()? Your code in fs/configfs/dir.c
contains a warning not to call the configfs_depend_item()
from a configfs callback.
In this case, is store_attribute a configfs callback?

Thanks,

Andrzej

Re: [RFC 0/2] USB gadget - configfs

[Joel Becker]

On Tue, Jul 10, 2012 at 10:54:44AM +0200, Andrzej Pietrasiewicz wrote:
> Dear Joel,
> 
> Thank you for your review.
> 
> @Sebastian, Alan, Felipe: Thank you, too.
> 
> On Monday, July 02, 2012 11:09 AM Joel Becker wrote:
> 
> <snip>
> 
> > 
> > > As a prerequisite it adds an operation to configfs. The operation allows
> > > checking if it is ok to remove a pseudo directory corresponding to a
> > > configfs item/group.
> > 
> > 	I NAK'd that patch because you should be using
> > configfs_depend_item().  If you have trouble with that, let's talk.
> > 
> 
> Now I see the configfs_depend_item() is the way to go. I am in doubt,
> though, so could you please throw some light on it? Here is why:
> As an example I did a quick-and-dirty port of f_mass_storage to the new,
> configfs-based approach. The business logic of this function is that
> once a lun is opened, it must not be changed (deleted, in particular)
> until it is closed. The moment the lun is opened is defined by a write
> to a configfs "file" attribute of a lun config item:
> 
>                +-/lunX
>                |  |
>                |  +-file
>                |  |
>                |  +-nofua
>                |  |
>                |  +-removable
>                |  |
>                |  +-ro
> 
> So, the config item corresponding to the lun becomes depended on during
> the write file operation, the same with undepend. Can this be expressed
> with configfs_depend/undepend_item()? Your code in fs/configfs/dir.c
> contains a warning not to call the configfs_depend_item()
> from a configfs callback.
> In this case, is store_attribute a configfs callback?

Hey Andrzej,
	I'm sorry it took me so long to write back.  I wanted a chance
to read and understand your code, so I could make some intelligent
comments.
	But first, a small aside.  Rather than filp_open() a filename
within the kernel, with all of its attendant state problems, you should
just take a file descriptor.  You can then let userspace permissions and
other things Just Work.  See
fs/ocfs2/heartbeat.c:o2hb_region_dev_write() for an example of o2hb
doing exactly this.  It takes a file descriptor and fgets() the filp.
The process writing the fd can actually close the file right after;
the heartbeat code has its reference.
	Let's continue with that example.  Just like your code, when
o2hb is given its fd, it starts up the heartbeat infrastructure.  So not
only does it hold a reference on the filp, it is starting threads and
such.  However, it also does not block deletion of the object.  If you
rmdir() the config_item, it will shut down the threads and drop the
filp.  This is analogous to your problem.  What happens if you remove a
heartbeat device underneath a running ocfs2 filesystem?  Why, it must
crash!  We don't want that, so when the ocfs2 filesystem is mounted, it
uses configfs_depend_item() to pin that heartbeat device.  This is what
I mean by pinning outside the configfs callback.
	Yes, attribute store is a callback.  So what should you do?
This is where my understanding of your setup logic fails me.  At first I
thought fsg_bind_function() was the right place, because it is where you
expect the LUNs to already be configured.  But it is, in turn, called
underneath another configfs callback (ufg_gadget_grp_store_connect()).
	Can you help me understand the userspace steps that are used to
set up a gadget?  The way I read the code, there is some software in the
gadget that sets up the LUN mappings; that is, the host has no idea
"lun01" is backed by a file named "foo".  So, if you had a gadget that
just exposed a single LUN, it would have some userspace software at
startup that sets fua=1, removable=0, ro=0, file="foo".  At some future
point, the host connects to the gadget.  At this point, lun01 is
connected to the host, and it had better not disappear.  What part of
the code reacts to the host connect?  This is the "open" of the LUN
where I think you should be locking out.

Joel


-- 

"Only a life lived for others is a life worth while."
							-Albert Einstein  

			http://www.jlbec.org/
			jlbec@evilplan.org

RE: [RFC 0/2] USB gadget - configfs

[Andrzej Pietrasiewicz]

Hello Joel,

Thank you for your reply. Please find comments inline.

On Wednesday, August 15, 2012 10:14 AM Joel Becker wrote:

<snip>

> > So, the config item corresponding to the lun becomes depended on during
> > the write file operation, the same with undepend. Can this be expressed
> > with configfs_depend/undepend_item()? Your code in fs/configfs/dir.c
> > contains a warning not to call the configfs_depend_item()
> > from a configfs callback.
> > In this case, is store_attribute a configfs callback?
> 

<snip>

> 	Yes, attribute store is a callback.  So what should you do?
> This is where my understanding of your setup logic fails me.  At first I
> thought fsg_bind_function() was the right place, because it is where you
> expect the LUNs to already be configured.  But it is, in turn, called
> underneath another configfs callback (ufg_gadget_grp_store_connect()).
> 	Can you help me understand the userspace steps that are used to
> set up a gadget?  The way I read the code, there is some software in the
> gadget that sets up the LUN mappings; that is, the host has no idea
> "lun01" is backed by a file named "foo".  So, if you had a gadget that
> just exposed a single LUN, it would have some userspace software at
> startup that sets fua=1, removable=0, ro=0, file="foo".  At some future
> point, the host connects to the gadget.  At this point, lun01 is
> connected to the host, and it had better not disappear.  What part of
> the code reacts to the host connect?  This is the "open" of the LUN
> where I think you should be locking out.

>From userspace the procedure to set up a gadget looks like this:

(please note that for testing I use _very_ short names like
G1, C1, F1; these are not mandatory and, as already pointed out
by some, more descriptive names should be used)

$ insmod g_usb_functions.ko
$ mount -t configfs none /cfg
$ mkdir -p /cfg/usb-function-gadget/G1/C1/F1
$ echo -n <some id> > /cfg/usb-function-gadget/G1/idVendor
$ echo -n <some id> > /cfg/usb-function-gadget/G1/idProduct
$ echo -n 0xff > /cfg/usb-function-gadget/G1/bcdDevice
$ echo Manufacturer > /cfg/usb-function-gadget/G1/iManufacturer
$ echo 123abc > /cfg/usb-function-gadget/G1/iSerialNumber
$ echo Gadget > /cfg/usb-function-gadget/G1/iProduct

$ echo -n f_mass_storage > /cfg/usb-function-gadget/G1/C1/F1/name
$ mkdir /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage
$ echo -n 1 > /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/luns
$ mkdir /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/lun0
$ echo -n /root/file.img >
/cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/lun0/file

$ echo -n 1 > /cfg/usb-function-gadget/G1/connect

The mapping between luns and their backing files is maintained in
alloc_fsg_lun,
f_mass_storage.c; the function needs improvement. The intended logic is not
to
allow more than 1 lun of the same name at the same time.

A lun is "opened" on storing the "file" attribute of the lun, it is in
fsg_lun_store_file, storage_common.c. So, again, this is a configfs
callback.

On storing the connect attribute, the following happens: a composite driver
is probed, then all the configurations are iterated over, their functions
being bound in turn. After the gadget is set up this way, the host notices
connecting a new mass storage device.

So it seems the only chance to call config_item_depend is in a configfs
callback. Or is it?

Thanks,

Andrzej

Re: [RFC 0/2] USB gadget - configfs

[Sebastian Andrzej Siewior]

On 08/16/2012 03:17 PM, Andrzej Pietrasiewicz wrote:
> A lun is "opened" on storing the "file" attribute of the lun, it is in
> fsg_lun_store_file, storage_common.c. So, again, this is a configfs
> callback.
>
> On storing the connect attribute, the following happens: a composite driver
> is probed, then all the configurations are iterated over, their functions
> being bound in turn. After the gadget is set up this way, the host notices
> connecting a new mass storage device.

The configuration has to remain unchanged until the "connect" attribute
is changed (i.e. unconnected). That means the gadget can only be
reconfigured once it is not active.

> So it seems the only chance to call config_item_depend is in a configfs
> callback. Or is it?
>
> Thanks,
>
> Andrzej

Sebastian

Re: [RFC 0/2] USB gadget - configfs

[Joel Becker]

On Thu, Aug 16, 2012 at 03:47:15PM +0200, Sebastian Andrzej Siewior wrote:
> On 08/16/2012 03:17 PM, Andrzej Pietrasiewicz wrote:
> >A lun is "opened" on storing the "file" attribute of the lun, it is in
> >fsg_lun_store_file, storage_common.c. So, again, this is a configfs
> >callback.
> >
> >On storing the connect attribute, the following happens: a composite driver
> >is probed, then all the configurations are iterated over, their functions
> >being bound in turn. After the gadget is set up this way, the host notices
> >connecting a new mass storage device.
> 
> The configuration has to remain unchanged until the "connect" attribute
> is changed (i.e. unconnected). That means the gadget can only be
> reconfigured once it is not active.

	Yes, that's my understanding of the driver code.  What I'm
trying to understand is the trigger for setting connect=1.  See, all of
the other configfs steps you outlined make sense to me as "part of the
gadget's internal state".  While I don't know what C1 and F1 are (I
assume G1 is Gadget1), I don't know that it matters.
	What changes the state to "active"?  That is, what triggers
setting connect=1?  Is it just part of the flow of power-up, or is there
some separation between "setting myself up" and "seeing that a host
wants to talk to me"?

Joel

-- 

"The trouble with being punctual is that nobody's there to
 appreciate it."
	- Franklin P. Jones

			http://www.jlbec.org/
			jlbec@evilplan.org

Re: [RFC 0/2] USB gadget - configfs

[Sebastian Andrzej Siewior]

On 08/17/2012 03:46 AM, Joel Becker wrote:
>> The configuration has to remain unchanged until the "connect" attribute
>> is changed (i.e. unconnected). That means the gadget can only be
>> reconfigured once it is not active.
>
> 	Yes, that's my understanding of the driver code.  What I'm
> trying to understand is the trigger for setting connect=1.  See, all of
> the other configfs steps you outlined make sense to me as "part of the
> gadget's internal state".  While I don't know what C1 and F1 are (I
> assume G1 is Gadget1), I don't know that it matters.

That is part of the abstracted configuration from the UDC [0] side:

- You have at least one "gadget" denoted as G1 here. You could have
   more of them. The second would be named G2 in this example. Each
   gadget requires one UDC.

- each gadget has at least one configuration descriptor denoted as C1
   here. It contains information like VID/PID numbers. You could have
   two configurations, where the first one is used if you have a lot of
   current available (on host's USB port) and the second where you can
   only use the minimum current which is "required" by the USB spec.

- each configuration has at least one function or interface. Denoted as
   F1 here. A function is for instance a "mass storage" or a "network"
   gadget.

So you _could_ have C1 with F1 as storage and F2 as network and C2 with
F1 as network. Within a config (C*) the functions are independent (that
means F1 und C1 can be != to F1 in C2)

The practical use case would to have a gadget with storage and network
available but if there is not much current available (and you can't
power the disk), the host would select C2 and have only the network
function.

> 	What changes the state to "active"?  That is, what triggers
> setting connect=1?

First, the user on the UDC side (the script on UDC side) has to
configure the gadget i.e. assign the LUN property to the mass storage
gadget or the MAC address for the network gadget.
Once all configuration is complete, the gadget can be considered as
"ready". configfs will be notified once gadget is considered as ready.

 From this point on, the user (of the host) can take a USB cable, plug
into his host and the UDC (gadget) will be enumerated.

Plugging the cable prior the "ready" point would result either in
"nothing" (the host wouldn't notice that a cable has been plugged) or
with a message "the device can't be enumerated".

So back to your question: "connect=1" is triggered by the person that
has the physical power over the cable. But you should have the gadget
configured prior this point _or_ ignore the host and configure the
gadget later.

To give you another example: Your cellphone does not have a gadget in
"ready" state and you plug the cable ("connect=1") into it. The
cellphone notices the plug and gives you the following and the screen:
"Hello good sir. Please choose: a) mass storage or b) MTP something".

Once you pick a or b on your cellphone, the script would activate one
of the two gadgets (sym link it to the UDC in configfs probably) and
it would be seen by the host.

> Is it just part of the flow of power-up, or is there
> some separation between "setting myself up" and "seeing that a host
> wants to talk to me"?

There is a separation between "setting myself up" and "seeing that a
host wants to talk to me". You are a woman. And if you not done yet
done you ignore the host until you are ready. Can also be ready before
the host wants to talk to you. It depends on your mood / use case (give
the use the choice between storage & network and offer always both
because we are self powered).

However it is important that once the configuration on UDC side is done
(or ready) it remains unchanged until you remove it from UDC ("de-ready
it to configfs) and change all attributes.
You shouldn't change the LUN attribute of the mass storage device while
it is active as it has no way to tell the host so.

[0] USB Device Controller. This part where the USB gadget is running.

> Joel

Sebastian

RE: [RFC 0/2] USB gadget - configfs

[Andrzej Pietrasiewicz]

Hello Sebastian, Hello Joel,

On Friday, August 17, 2012 11:22 AM Sebastian Andrzej Siewior wrote:

<snip>

> There is a separation between "setting myself up" and "seeing that a host
> wants to talk to me". You are a woman. And if you not done yet done you
ignore
> the host until you are ready. Can also be ready before the host wants to
talk
> to you. It depends on your mood / use case (give the use the choice
between
> storage & network and offer always both because we are self powered).
> 

You never know when she is ready... ;)

Please note, that in the example implementation it is userspace
who needs to explicitly store "1" into the "connect" attribute;
if it is not set, connecting the cable will have no effect and,
conversely, if the cable is connected but the "1" is not stored,
the gadget will not be seen by the host.

Andrzej

Re: [RFC 0/2] USB gadget - configfs

[Sebastian Andrzej Siewior]

On 08/17/2012 12:30 PM, Andrzej Pietrasiewicz wrote:
>> There is a separation between "setting myself up" and "seeing that a host
>> wants to talk to me". You are a woman. And if you not done yet done you
> ignore
>> the host until you are ready. Can also be ready before the host wants to
> talk
>> to you. It depends on your mood / use case (give the use the choice
> between
>> storage&  network and offer always both because we are self powered).
>>
>
> You never know when she is ready... ;)
>
> Please note, that in the example implementation it is userspace
> who needs to explicitly store "1" into the "connect" attribute;
> if it is not set, connecting the cable will have no effect and,
> conversely, if the cable is connected but the "1" is not stored,
> the gadget will not be seen by the host.

maybe we should rename "connect" here to "ready" or "available". It
seems to cause confusion.

>
> Andrzej

Sebastian

Re: [RFC 0/2] USB gadget - configfs

[Joel Becker]

On Fri, Aug 17, 2012 at 12:34:11PM +0200, Sebastian Andrzej Siewior wrote:
> On 08/17/2012 12:30 PM, Andrzej Pietrasiewicz wrote:
> >>There is a separation between "setting myself up" and "seeing that a host
> >>wants to talk to me". You are a woman. And if you not done yet done you
> >ignore
> >>the host until you are ready. Can also be ready before the host wants to
> >talk
> >>to you. It depends on your mood / use case (give the use the choice
> >between
> >>storage&  network and offer always both because we are self powered).
> >>
> >
> >You never know when she is ready... ;)
> >
> >Please note, that in the example implementation it is userspace
> >who needs to explicitly store "1" into the "connect" attribute;
> >if it is not set, connecting the cable will have no effect and,
> >conversely, if the cable is connected but the "1" is not stored,
> >the gadget will not be seen by the host.
> 
> maybe we should rename "connect" here to "ready" or "available". It
> seems to cause confusion.

	I think what you want here is a symlink.  Create a directory
high in the tree called 'ready' or 'available', and when you want to
lock the device in place, symlink it into that directory.  This will pin
the item.  You can also make use of the ->allow_link() and ->drop_link()
callbacks to prevent symlinking an item until it is fully configured,
and prevent unlinking an item that is in use.

  $ ln -s /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/lun0 \
        /cfg/usb-function-gadget/ready/

or something like that.

Joel

-- 

"Born under a bad sign.
 I been down since I began to crawl.
 If it wasn't for bad luck,
 I wouldn't have no luck at all."

			http://www.jlbec.org/
			jlbec@evilplan.org

RE: [RFC 0/2] USB gadget - configfs

[Andrzej Pietrasiewicz]

Hello Joel,

Thanks for your reply.

On Monday, August 20, 2012 8:00 AM Joel Becker wrote:
> 
> 	I think what you want here is a symlink.  Create a directory
> high in the tree called 'ready' or 'available', and when you want to
> lock the device in place, symlink it into that directory.  This will pin
> the item.  You can also make use of the ->allow_link() and ->drop_link()
> callbacks to prevent symlinking an item until it is fully configured,
> and prevent unlinking an item that is in use.
> 
>   $ ln -s /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/lun0 \
>         /cfg/usb-function-gadget/ready/
> 

Is using the userspace program "ln" the way you suggest doing it? Or just
a suggestion about what should be the link target and the link name?

If links are to be created from userspace then some other process can race
us and rmdir "lun0" before ln is called, can't it?

And a question to you as the creator of configfs: is using symlinks to
prevent config item deletion their intended purpose? How do you like it?

Thanks,

Andrzej

Re: [RFC 0/2] USB gadget - configfs

[Sebastian Andrzej Siewior]

On 08/20/2012 07:59 AM, Joel Becker wrote:
> 	I think what you want here is a symlink.  Create a directory
> high in the tree called 'ready' or 'available', and when you want to
> lock the device in place, symlink it into that directory.  This will pin
> the item.  You can also make use of the ->allow_link() and ->drop_link()
> callbacks to prevent symlinking an item until it is fully configured,
> and prevent unlinking an item that is in use.
>
>    $ ln -s /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/lun0 \
>          /cfg/usb-function-gadget/ready/
>
> or something like that.

That would be something like

   $ ln -s /cfg/usb-function-gadget/G1 \
         /cfg/usb-function-gadget/udcs/udc1/

Where we "bind" the complete gadget (G1) including the mass storage
configuration to the UDC (udc1 in this example).

At this point we could snapshot the complete configuration so we have
it persistent and if somebody changes attributes within /cfg/usb-
function-gadget/G1 it doesn't matter because we have the copy.

Is it _this_ what you want us to do? A snapshot?

>
> Joel


Sebastian

Re: [RFC 0/2] USB gadget - configfs

[Joel Becker]

On Mon, Aug 20, 2012 at 10:53:42AM +0200, Andrzej Pietrasiewicz wrote:
> Hello Joel,
> 
> Thanks for your reply.
> 
> On Monday, August 20, 2012 8:00 AM Joel Becker wrote:
> > 
> > 	I think what you want here is a symlink.  Create a directory
> > high in the tree called 'ready' or 'available', and when you want to
> > lock the device in place, symlink it into that directory.  This will pin
> > the item.  You can also make use of the ->allow_link() and ->drop_link()
> > callbacks to prevent symlinking an item until it is fully configured,
> > and prevent unlinking an item that is in use.
> > 
> >   $ ln -s /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/lun0 \
> >         /cfg/usb-function-gadget/ready/
> > 
> 
> Is using the userspace program "ln" the way you suggest doing it? Or just
> a suggestion about what should be the link target and the link name?

You could use ln, you could use symlink(2), or some other method.  The
symlink itself is the goal.  The names are my best guess at how your
system is laid out; it's not meant to pick a name for you.

> If links are to be created from userspace then some other process can race
> us and rmdir "lun0" before ln is called, can't it?

Yes, but that same process can race the write of '1' to "connect" as well,
right?  I don't see the difference.

> And a question to you as the creator of configfs: is using symlinks to
> prevent config item deletion their intended purpose? How do you like it?

	Yes, it is an intended use.  Another intended use is
simplification of access.  For example, if you have all connected things
in the "ready" subdirectory, you don't have to go scanning all the
"connect" files to find them.

Joel

-- 

"I always thought the hardest questions were those I could not answer.
 Now I know they are the ones I can never ask."
			- Charlie Watkins

			http://www.jlbec.org/
			jlbec@evilplan.org

Re: [RFC 0/2] USB gadget - configfs

[Joel Becker]

On Mon, Aug 20, 2012 at 01:01:03PM +0200, Sebastian Andrzej Siewior wrote:
> On 08/20/2012 07:59 AM, Joel Becker wrote:
> >	I think what you want here is a symlink.  Create a directory
> >high in the tree called 'ready' or 'available', and when you want to
> >lock the device in place, symlink it into that directory.  This will pin
> >the item.  You can also make use of the ->allow_link() and ->drop_link()
> >callbacks to prevent symlinking an item until it is fully configured,
> >and prevent unlinking an item that is in use.
> >
> >   $ ln -s /cfg/usb-function-gadget/G1/C1/F1/f_mass_storage/lun0 \
> >         /cfg/usb-function-gadget/ready/
> >
> >or something like that.
> 
> That would be something like
> 
>   $ ln -s /cfg/usb-function-gadget/G1 \
>         /cfg/usb-function-gadget/udcs/udc1/
> 
> Where we "bind" the complete gadget (G1) including the mass storage
> configuration to the UDC (udc1 in this example).
> 
> At this point we could snapshot the complete configuration so we have
> it persistent and if somebody changes attributes within /cfg/usb-
> function-gadget/G1 it doesn't matter because we have the copy.
> 
> Is it _this_ what you want us to do? A snapshot?

	No, I don't see why you would snapshot.  You can, btw, reject
->store_attribute() operations once you are in a "ready" state.  That's
different from preventing the removal of items.

Joel


-- 

The Graham Corollary:

	The longer a socially-moderated news website exists, the
	probability of an old Paul Graham link appearing at the top
	approaches certainty.

			http://www.jlbec.org/
			jlbec@evilplan.org

RE: [RFC 0/2] USB gadget - configfs

[Andrzej Pietrasiewicz]

Hello Sebastian,

On Monday, August 20, 2012 1:01 PM Sebastian Andrzej Siewior wrote

<snip>

> 
> That would be something like
> 
>    $ ln -s /cfg/usb-function-gadget/G1 \
>          /cfg/usb-function-gadget/udcs/udc1/
> 
> Where we "bind" the complete gadget (G1) including the mass storage
> configuration to the UDC (udc1 in this example).
> 

As far as I understand it, Joel's intention was only to show how it is
possible to prohibit removing some items with rmdir; in this case
if a "lun0" is linked to from somewhere else it is not possible to
do "rmdir lun0"; this was the purpose of my original question.

@Joel:
However, I am wondering if it would make sense to get rid of the "connect"
(or "ready", if you will) attribute altogether and instead use symlinks:
when a user wants to make the gadget ready, they do an ln -s, when they want
to unbind the gadget they remove the link. What do you think?

There is one more thing to it, I believe; it is how to associate udc's
with gadgets. Perhaps the most convenient way is not needing to do it
explicitly at all: either some udc is found and the gadget is bound to it,
or not. However, I am wondering if something in the spirit of "1984" can
happen: all udcs are equal but some udcs are more equal than others?
So sometimes the user might be interested in binding their gadget to
a particular udc, or at least to a particular kind of udc (no matter
which one if there are more than one of its kind). And the question
is, whether we want only explicit association with udc, only implicit
association with udc, or both?

Andrzej

RE: [RFC 0/2] USB gadget - configfs

[Andrzej Pietrasiewicz]

Hello Joel,

On Monday, August 20, 2012 8:00 AM Joel Becker wrote:

<snip>

> 	I think what you want here is a symlink.  Create a directory high in
> the tree called 'ready' or 'available', and when you want to lock the
> device in place, symlink it into that directory.  This will pin the item.
> You can also make use of the ->allow_link() and ->drop_link() callbacks to
> prevent symlinking an item until it is fully configured, and prevent
> unlinking an item that is in use.
> 

I have a question to you. In fs/configfs/dir.c, in a comment before
configfs_depend_item and configfs_depend_prep you say that "configfs uses
VFS to handle all locking". I am trying to understand how this
works. Could you please elaborate a bit about it? Which locks from VFS
handle "all locking"? When does locking happen?

Thanks,

Andrzej
 

Re: [RFC 0/2] USB gadget - configfs

[Sebastian Andrzej Siewior]

On 08/21/2012 10:19 AM, Andrzej Pietrasiewicz wrote:
> @Joel:
> However, I am wondering if it would make sense to get rid of the "connect"
> (or "ready", if you will) attribute altogether and instead use symlinks:
> when a user wants to make the gadget ready, they do an ln -s, when they want
> to unbind the gadget they remove the link. What do you think?

I started the "ready" attribue and this symlink sounds reasonable.

> There is one more thing to it, I believe; it is how to associate udc's
> with gadgets. Perhaps the most convenient way is not needing to do it
> explicitly at all: either some udc is found and the gadget is bound to it,

I would prefer explicit binding. Right now first one wins which not
good.

> or not. However, I am wondering if something in the spirit of "1984" can
> happen: all udcs are equal but some udcs are more equal than others?
> So sometimes the user might be interested in binding their gadget to
> a particular udc, or at least to a particular kind of udc (no matter
> which one if there are more than one of its kind).

Do you have a use case for this? I have a simple one: A phone with two
plugs. You select on the gui storage and network on plug #1 and serial
on plug #2. If you want to obey the selection you have to always know
which UDC ends up on plug #1 and which on #2.
That means the user _always_ wants to bind it to a particular UDC.

> And the question
> is, whether we want only explicit association with udc, only implicit
> association with udc, or both?

/usb-gadget/gadgets/g1
/usb-gadget/udcs/udc1
/usb-gadget/udcs/udc2

and now symlink g1 into udc1 and or udc2

g1 is created by the user and can be named however he wants it to be.
udc1 can be the device name which is unique as well and always the same
thing. It can have a property which distinguish it from udc2 like
memory address or something.

>
> Andrzej

Sebastian