On 11/23/2012 12:52 PM, Andrzej Pietrasiewicz wrote:
The general idea is that:
echo "some_name"> /cfg/usb-function-gadget/G1/C1/F1/name
echo "some_name"> /cfg/usb-function-gadget/G1/C2/F1/name
creates two separate instances of F1 in C1 and C2. Did you mean
the same instance of F1 used both in C1 and C2?
Anyway, since Michał proposed some alternative this scheme is
likely to be revised.
I posted something as well, please take a look at it.
/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81
/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81/attributes
/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81/endpoint_addre
ss /cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81/interval
/cfg/usb-function-gadget/G1/C1/F1/interface00/endpoint81/max_packet_siz
e
Interresting. I have no idea if I do like or I don't. Some minor things:
You do have endpoint02 and you have endpoint_address. I guess
endpoint_address here reads 2. So I think this attribute is read only.
Same goes for max_packet_size since this is usually protocol specific.
Once upon a time Felipe expressed interest in having all these accessible
from userspace (e.g. for debugging purposes) so here it is.
Meant to be read-only.
Okay.
+config USB_FG
+ tristate "USB Functions Gadget (EXPERIMENTAL)"
+ select USB_LIBCOMPOSITE
+ depends on EXPERIMENTAL&& CONFIGFS_FS
EXPERIMENTAL is going away. It is no longer under drivers/usb.
Anything else instead perhaps?
Nothing actually.
+ help
+ USB Functions Gadget is a device which aggregates a number of
+ USB functions. The gadget is composed by userspace through a
+ configfs interface, which enables specifying what USB
+ configurations the gadget is composed of, what USB functions
+ a USB configuration is composed of and enabling/disabling
+ the gadget.
It would be nice to point to Documentation/usb/file-with-examples.txt
You can expect it in the next version.
Yeah but easy. I have no problem with adding that file once we have
something file as you better things to do than updating the
documentation every second.
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index fef41f5..09faa34 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_USB_MV_U3D) += mv_u3d_core.o
#
# USB gadget drivers
#
+g_usb_functions-y := usb_functions.o
g_zero-y := zero.o
g_audio-y := audio.o
g_ether-y := ether.o
@@ -56,6 +57,7 @@ g_ncm-y := ncm.o
g_acm_ms-y := acm_ms.o
g_tcm_usb_gadget-y := tcm_usb_gadget.o
+obj-$(CONFIG_USB_FG) += g_usb_functions.o
Don't use this indirection and I don't see a reason why you shouldn't drop
the g_ at the beginning. After all this will not be a gadget but an
interface to create gadgets.
This calls for a discussion: where actually this kind of code should be
located? usb_functions.c or similar? composite.c?
Keep a separate file next to composite/functions but in the end I would
merge it some place if it fits. The same goes for the config option
because ideally there is no way without configfs :)
+static struct config_group *ufg_make_function_subgroup(
+ struct config_group *group, const char *name) {
+ struct ufg_function_grp *f_grp;
+ struct config_group *result;
+
+ /*
+ * TODO:
+ *
+ * This makes "interface" an illegal name for a function.
+ */
+ if (!strncmp(name, "interface", 9)) {
Can't you use something else to match between function& interface?
After all you know that gadget's parent has to be a "config"-folder,
followed by an interface, followed by the function.
I could have used another level of nesting, where by default there are
/cfg...../C1/F1/name
/cfg...../C1/F1/function/
/cfg...../C1/F1/interfaces/
and
echo "some_name"> /cfg...../C1/F1/name
creates
/cfg...../C1/F1/function/some_name
/cfg...../C1/F1/function/some_name/function-specific-stuff
and later binding the gadget creates
/cfg...../C1/F1/interfaces/interface00
In such a situation the distinction between a function and
an interface is made by construction: a subdirectory of
/cfg...../C1/F1/interfaces must be an interface
While a subdirectory of
/cfg...../C1/F1/function must be a function
This can be done. However, I was not sure whether it was
desirable to have one more directory level.
Now it is
/cfg...../C1/F1/name
/cfg...../C1/F1/some_name
/cfg...../C1/F1/some_name/function-specific-stuff
/cfg...../C1/F1/interface00
I would put the interfaces as a child of function and not merge them in
one folder.
<snip>
+static ssize_t ufg_function_grp_store_name(struct ufg_function_grp *grp,
+ const char *buf, size_t count) {
+ int rc;
+ struct dentry *parent, *new;
+ struct qstr name;
+ char name_buf[UFG_STR_LEN];
<snip>
+ return -ENOMEM;
+ d_add(new, NULL);
+ rc = ufg_mkdir(parent, new);
+ if (rc) {
+ d_drop(new);
+ dput(new);
+
+ return rc;
+ }
+ dput(new); /* make the refcount 1 */
Looking at this: full_name_hash(), d_alloc(), d_add(), d_drop(), dput().
Do you write a filesystem? I don't see any one of this functions beeing
use in drivers/target? Why do you need them? I think you do something more
complicated than it needs to be because target made it without it.
This again calls for a discussion about configfs in general.
There is no interface to configfs to operate on the directories
and files programmatically from the kernel, and this is why I use
all these filesystem-specific stuff here: only operation from
userspace through syscalls is supported in configfs so I need to
simulate it.
It is becoming more and more evident that we _will_ need a way
to create/manipulate configfs entries programmatically from the kernel.
The way I implement it now in ufg_mkdir is something which eventually
evaluates to a line of this kind:
grp->group.cg_item.ci_dentry->d_inode->i_op->mkdir()
where grp is a pointer to the parent's config_group container.
When mkdir() operation is called, it calls back to configfs's
make_group from config_group_operations. An important note:
Once make_group is called there is no way to tell whether the
call originates from userspace/syscall, or was initiated
programmatically.
@Joel: can you suggest something else for programmatically creating
configfs directories and reading/writing configfs files?
I'm not convinced because others made it. So far I see the USB
interface mostly in the same way as the target interface. Please take a
look at proposal I made for the hierarchy. Maybe it makes some of the
requirements go away.
+ for (d = descriptors; *d; d++)
+ switch ((*d)->bDescriptorType) {
+ case USB_DT_INTERFACE:
+ i_grp = ufg_process_interface_desc(*d,
f_grp);
+ if (IS_ERR_OR_NULL(i_grp))
+ return -1;
+ /* keep gcc quiet about a value not being
used*/
If it is not used, why do we have it?
Ok, it should be
(*num_interfaces)++;
+ *num_interfaces = *num_interfaces + 1;
I see. So you change something :) Okay then.
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_AUTHOR("Andrzej Pietrasiewicz"); MODULE_ALIAS("usb_functions");
+MODULE_LICENSE("GPL");
Please keep this things (MODULE_*) at the bottom. Why an ALIAS?
ALIAS() to have a string which can be referred to in adapters, which
provide the old modprobe + parameters (+ e.g. sysfs) interface and
should be able to request_module("usb_functions");
The string could be defined in some header file included both by
this module and adapter modules. Simpler solutions welcome.
I don't get. No ever did "modprobe usb_functions" at least on an
mainline kernel so I don't understand "old modprobe + prameters"
interface. So why again?
Andrzej
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
