On 11/28/2012 09:31 PM, Michal Nazarewicz wrote:
On Wed, Nov 28 2012, Sebastian Andrzej Siewior wrote:
|# modprobe dummy_hcd num=2
|# find /sys/kernel/config/ -ls
| 6547 0 drwxr-xr-x 5 root root 0 Nov 28 19:39 /sys/kernel/config/
| 532 0 drwxr-xr-x 2 root root 0 Nov 28 19:39 /sys/kernel/config/dummy_udc.1
| 531 0 drwxr-xr-x 2 root root 0 Nov 28 19:39 /sys/kernel/config/dummy_udc.0
| 6548 0 drwxr-xr-x 4 root root 0 Nov 28 19:38 /sys/kernel/config/usb_gadget
| 6550 0 drwxr-xr-x 2 root root 0 Nov 28 19:38 /sys/kernel/config/usb_gadget/gadgets
| 6549 0 drwxr-xr-x 2 root root 0 Nov 28 19:38 /sys/kernel/config/usb_gadget/functions
| # lsmod
| Module Size Used by
| dummy_hcd 20287 0
| libcomposite 17052 0
Did dummy_hcd pulled libcomposite here?
After double check, no. I had the configfs part as part of libcomposite
and my test script loaded it. After I moved it to udc where it fit
better together with the function.c file I did not update the script.
It gets loaded after "mkdir acm-bla"
| # mkdir /sys/kernel/config/usb_gadget/functions/acm_one
| # lsmod
| Module Size Used by
| f_acm 5306 0
| u_serial 9644 1 f_acm
| dummy_hcd 20287 0
| libcomposite 17052 1 f_acm
Now:
- the UDC (dummy_udc.[01]) is not within /usb_gadget/ folder where we
have /gadgets/ and /functions/ subfolder. This is what
spear13xx_pcie_gadget does as well. Is this okay or do we want to have
them enumerated below /usb_gadget/ as well? If so we got to convince
Joel to take Andrzej's patch for that.
Either way, I think they should be grouped in a “udcs” directory.
Otherwise, this, in my opinion, pollutes configfs' root directory.
Understood. So we have to create them on demand.
Whether they are under usb_gadget or outside, I don't have strong
opinions, even though I would see value in grouping USB gadget “stuff”
together. Than again, someone may argue about UDCs which support OTG.
bah. So it is part host part device. We could still put it under /udcs/
don't see a reason why not. We don't have to put there anything host
specific.
- the function name for acm is taken from the dir name. Now Michał, is
it too much of hackery?
Again, I don't really have strong opinions (on top of which, I feel like
I'm imposing my ideas without doing any coding), but I feel that using
“function/instance” rather than “function_instance” may make things
a bit easier since the name does not have to be parsed at all.
Especially since if we choose “_” than “mass_storage” would have to
change its name, right? Maybe “.” as a separator would be better? In
your example, this would definitely look consistent with what dummy_udc
does.
Okay. So the dot it is.
PS. Ah, and in general, if anyone cares about my opinion, I hate “_” and
propose to use “-” instead whenever possible. :P
This is a slow start. Other comments?
Signed-off-by: Sebastian Andrzej Siewior<bigeasy@xxxxxxxxxxxxx>
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
+struct configfs_subsystem *udc_add_configfs(struct device *dev)
+{
+ struct udc_configfs *udc_cfs;
+ struct configfs_subsystem *subsys;
+ int ret;
+
+ udc_cfs = kzalloc(sizeof(struct udc_configfs), GFP_KERNEL);
+ if (!udc_cfs)
+ return ERR_PTR(-ENOMEM);
+
+ subsys =&udc_cfs->subsys;
+ config_group_init_type_name(&subsys->su_group,
+ kobject_name(&dev->kobj),
+ &gadget_core_item);
+ ret = configfs_register_subsystem(subsys);
+ if (ret)
+ goto err;
+ return subsys;
+err:
+ kfree(udc_cfs);
+ return ERR_PTR(ret);
Maybe it's just me, but I don't like gotos where they can be avoided
(even though I have nothing against gotos in general):
The goto cleanup part is ready if new code should be added.
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
