On Tue, Jun 10, 2014 at 02:00:13PM +0200, Andrzej Pietrasiewicz wrote:
> Hi Peter,
>
Hi Andrzej,
First, many thanks for commenting and testing it.
The reason why I changed gadget-configfs:
- The gadget-configfs will register itself (as usb_gadget_driver) when
specific udc attaches to it, with gadget bus introducing, each device
driver will be added to gadget bus for each usb_gadget_driver, and the
name for device driver need to be unique.
- The device-model bind/unbind will do probe/remove, it can cover
the functionalities of gadget_cdev_desc_UDC_store/show.
> W dniu 10.06.2014 06:32, Peter Chen pisze:
> >- Deciding configfs device_driver's name according to creating
> >sequence, it is still RFC
> >- Register gadget driver at gadgets_make
> >- Delete gadget_cdev_desc_UDC_store and gadget_cdev_desc_UDC_show
> >interface, we can use standard device-model bind/unbind entry to
> >bind specific udc to gadgetfs driver after disable gadget_bus's
> >drivers_autoprobe
>
> I don't like this solution. If udc driver is compiled in
> and drivers_autoprobe is not disabled, one cannot create gadget's
> directory in configfs because the gadget does not contain any
> configurations yet at the moment of the said directory's creation.
>
That's true, the probe will run when the gadget folder is creating,
but no configuration at that time.
> Just creating gadget's directory is only a beginning of composing
> a gadget. In other words, the gadget is under construction until
> the user decides they are done. Constructing (composing) a gadget
> involves creating directories and writing to files inside gadget's
> directory. By the very nature of this process the gadget's directory
> must be created first and by design the gadget for sure is not ready then.
> So why to attempt binding it at this moment?
> Of course one can first write 0 to drivers_autoprobe, but first
> this attribute is located somewhere totally different than the gadget's
> configfs directory and second we loose the autoprobe capability.
>
> Please also note, that the user can change their mind and delete
> a gadget currently under construction, then create some other gadget
> and so on - until the user is satisfied gadget's directories
> can come and go. And with your solution usb_gadget_probe_driver()
> is called every time a gadget's directory is created. This is more
> often than necessary - it should be called only for gadgets really
> intended for binding.
>
> What I would like is keeping a mechanism similar storing the UDC's
> name into UDC attribute in configfs. It now serves a double
> purpose: first, to say the gadget is ready and second, to bind it
> to a particular udc. I would keep the first while the second is handled
> by the gadget bus. And of course the attribute's name should be changed
> to reflect its purpose: "enable" or "commit" or something similar.
>
Some considerations for me are I want to keep auto-binding and
manual-binding at the same time, it seems Felipe only wants to
have manual-binding, then, things will be easier.
Now, three things for gadget-configfs need to be decided, I would
like to have your suggestion:
- The name of device driver for each usb_gadget_driver, it needs
to be unique, and the gadget may have multiple functions, see
test my case below, so we can't use function name.
How about the device driver name is the same with user created
gadget name
/sys/kernel/config/usb_gadget/<gadget name>
/sys/bus/usb_gadget/drivers/<gadget driver name>
<gadget name> is the same with <gadget driver name> for above?
- When to register device driver to gadget bus?
How about I create a "enable" attribute like you suggest above,
and the device driver will be registered or removed when the user
echo "1" or "0" to "enable"?
- How udc attach/detach to gadget driver?
How about like other gadget drivers that is using
/sys/bus/gadget_bus/<usb_gadget_driver>/bind
and /sys/bus/gadget_bus/<usb_gadget_driver>/unbind
>
> By the way, when I do this:
>
For your two failure test cases: I haven't meet error.
My code base:
the top of Greg's usb-next (4a95b1fce97756d0333f8232eb7ed6974e93b054)
with my seven patches, and I build in my udc driver.
Below are my test step:
- For g_ether
root@freescale ~$ modprobe g_ether
using random self ethernet address
using random host ethernet address
usb0: HOST MAC 4e:53:22:6e:d9:f6
usb0: MAC 46:e1:6c:c1:97:59
using random self ethernet address
using random host ethernet address
g_ether gadget: Ethernet Gadget, version: Memorial Day 2008
g_ether gadget: g_ether ready
root@freescale ~$ g_ether gadget: high-speed config #1: CDC Ethernet
(ECM)
- For gadgetfs
root@freescale ~$ modprobe libcomposite
root@freescale ~$ mount none /sys/kernel/config/ -t configfs
root@freescale ~$ echo 0 > /sys/bus/usb_gadget/drivers_autoprobe
root@freescale ~$
/********************** the first gadget ************/
root@freescale ~$ mkdir /sys/kernel/config/usb_gadget/g1
root@freescale ~$ cd /sys/kernel/config/usb_gadget/g1/
root@freescale /sys/kernel/config/usb_gadget/g1$ echo 0x15a2 > idVendor
root@freescale /sys/kernel/config/usb_gadget/g1$ echo 0x0054 > idProduct
root@freescale /sys/kernel/config/usb_gadget/g1$ mkdir strings/0x409
root@freescale /sys/kernel/config/usb_gadget/g1$ echo 123456ABCDEF >
strings/0x4
09/serialnumber
root@freescale /sys/kernel/config/usb_gadget/g1$ echo Freescale >
strings/0x409/
manufacturer
root@freescale /sys/kernel/config/usb_gadget/g1$ echo "FSL i.mx6q
sabreSD Board"
> strings/0x409/product
root@freescale /sys/kernel/config/usb_gadget/g1$ mkdir configs/c.1
root@freescale /sys/kernel/config/usb_gadget/g1$
root@freescale /sys/kernel/config/usb_gadget/g1$ echo 5 >
configs/c.1/MaxPower
root@freescale /sys/kernel/config/usb_gadget/g1$ mkdir functions/acm.1
root@freescale /sys/kernel/config/usb_gadget/g1$ ln -s functions/acm.1
configs/c
.1/
root@freescale /sys/kernel/config/usb_gadget/g1$ mkdir
functions/mass_storage.1
Number of LUNs=8
Mass Storage Function, version: 2009/09/11
LUN: removable file: (no medium)
root@freescale /sys/kernel/config/usb_gadget/g1$ ln -s
functions/mass_storage.1
configs/c.1/
root@freescale /sys/kernel/config/usb_gadget/g1$ echo
/dev/mmcblk0p1 > functions
/mass_storage.1/lun.0/file
root@freescale /sys/kernel/config/usb_gadget/g1$
root@freescale /sys/kernel/config/usb_gadget/g1$ echo udc-0 >
/sys/bus/usb_gadge
t/drivers/configfs-gadget.0/bind
root@freescale /sys/kernel/config/usb_gadget/g1$
configfs-gadget.0 gadget: high-speed config #1: c
/********************** the second gadget ************/
root@freescale /sys/kernel/config/usb_gadget/g1$ mkdir
/sys/kernel/config/usb_ga
dget/g2
root@freescale /sys/kernel/config/usb_gadget/g1$ cd
/sys/kernel/config/usb_gadge
t/g2/
root@freescale /sys/kernel/config/usb_gadget/g2$ echo 0x15a2 > idVendor
root@freescale /sys/kernel/config/usb_gadget/g2$ echo 0x0054 > idProduct
root@freescale /sys/kernel/config/usb_gadget/g2$ mkdir strings/0x409
root@freescale /sys/kernel/config/usb_gadget/g2$ echo 123456ABCDEF >
strings/0x4
09/serialnumber
root@freescale /sys/kernel/config/usb_gadget/g2$ echo Freescale >
strings/0x409/
manufacturer
root@freescale /sys/kernel/config/usb_gadget/g2$ echo "FSL i.mx6q
sabreSD Board"
> strings/0x409/product
root@freescale /sys/kernel/config/usb_gadget/g2$ mkdir configs/c.1
root@freescale /sys/kernel/config/usb_gadget/g2$
root@freescale /sys/kernel/config/usb_gadget/g2$ echo 5 >
configs/c.1/MaxPower
root@freescale /sys/kernel/config/usb_gadget/g2$ mkdir functions/acm.1
root@freescale /sys/kernel/config/usb_gadget/g2$ ln -s functions/acm.1
configs/c
.1/
root@freescale /sys/kernel/config/usb_gadget/g2$ mkdir
functions/mass_storage.1
Number of LUNs=8
Mass Storage Function, version: 2009/09/11
LUN: removable file: (no medium)
root@freescale /sys/kernel/config/usb_gadget/g2$ ln -s
functions/mass_storage.1
configs/c.1/
root@freescale /sys/kernel/config/usb_gadget/g2$ echo
/dev/mmcblk0p2 > functions
/mass_storage.1/lun.0/file
root@freescale /sys/kernel/config/usb_gadget/g2$ echo udc-1 >
/sys/bus/usb_gadge
t/drivers/configfs-gadget.1/bind
root@freescale /sys/kernel/config/usb_gadget/g2$
configfs-gadget.1 gadget: high-speed config #1: c
/*************************** Detach UDCs ************************/
root@freescale /sys/kernel/config/usb_gadget/g2$ echo udc-1 >
/sys/bus/usb_gadge
t/drivers/configfs-gadget.1/unbind
configfs-gadget.1 gadget: unbind function 'acm'/be9a3d80
configfs-gadget.1 gadget: unbind function 'Mass Storage
Function'/be9ea100
root@freescale /sys/kernel/config/usb_gadget/g2$
root@freescale /sys/kernel/config/usb_gadget/g2$ echo udc-0 >
/sys/bus/usb_gadge
t/drivers/configfs-gadget.0/unbind
configfs-gadget.0 gadget: unbind function 'acm'/be9a3cc0
configfs-gadget.0 gadget: unbind function 'Mass Storage
Function'/be8bd080
I even create below script to test bind/unbind for gadget-configfs,
it works ok for over night test
counter=1
while [ 1 ]
do
echo udc-0 > /sys/bus/usb_gadget/drivers/configfs-gadget.0/bind
echo udc-1 > /sys/bus/usb_gadget/drivers/configfs-gadget.1/bind
sleep 5
echo udc-0 > /sys/bus/usb_gadget/drivers/configfs-gadget.0/unbind
echo udc-1 > /sys/bus/usb_gadget/drivers/configfs-gadget.1/unbind
sleep 5
echo udc-0 > /sys/bus/usb_gadget/drivers/configfs-gadget.1/bind
echo udc-1 > /sys/bus/usb_gadget/drivers/configfs-gadget.0/bind
sleep 5
echo udc-0 > /sys/bus/usb_gadget/drivers/configfs-gadget.1/unbind
echo udc-1 > /sys/bus/usb_gadget/drivers/configfs-gadget.0/unbind
sleep 5
counter=$(( $counter + 1 ))
echo "the counter is $counter"
done
My [patch 7/7] is for gadget-configfs doc update, refer to it please
if necessary.
I also tried single udc environment, it works ok too.
Peter
> $modprobe libcomposite
> $ mount none cfg -t configfs
> $mkdir cfg/usb_gadget/g1
> $ cd cfg/usb_gadget/g1
> $mkdir configs/c.1
> $ mkdir functions/ecm.usb0
> $ mkdir strings/0x409
> $ mkdir configs/c.1/strings/0x409
> $echo 0xa4a1 > idProduct
> $ echo 0x04e8 > idVendor
> $ echo samsung123 > strings/0x409/serialnumber
> $ echo Samsung > strings/0x409/manufacturer
> $ echo ECM Gadget > strings/0x409/product
> $echo "Conf 1" > configs/c.1/strings/0x409/configuration
> $ echo 120 > configs/c.1/MaxPower
> $ ln -s functions/ecm.usb0 configs/c.1
> $ echo udc-0 > /sys/bus/usb_gadget/drivers/configfs-gadget.0/bind
>
> the gadget does bind and becomes operational.
> However, I cannot do:
>
> $ echo udc-0 > /sys/bus/usb_gadget/drivers/configfs-gadget.0/unbind
> -bash: echo: write error: No such device
>
> So I am not even able to test whether removing a gadget causes
> its sysfs entry to be removed.
>
>
> Also, the bug reported here:
> http://www.spinics.net/lists/linux-usb/msg107951.html
> still remains:
> root@target:~# modprobe g_ether
> [ 44.126086] using random self ethernet address
> [ 44.129198] using random host ethernet address
> [ 44.137323] usb0: HOST MAC 96:d9:70:ba:3e:4a
> [ 44.141969] usb0: MAC 9a:f4:fc:ff:85:96
> [ 44.144375] using random self ethernet address
> [ 44.156279] using random host ethernet address
> [ 44.161814] g_ether gadget: Ethernet Gadget, version: Memorial Day 2008
> [ 44.171013] g_ether gadget: g_ether ready
> [ 44.184948] s3c-hsotg s3c-hsotg: bound driver g_ether
> [ 44.202871] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [ 44.209534] s3c-hsotg s3c-hsotg: GINTSTS_USBSusp
> [ 44.225779] pgd = e580c000
> [ 44.227110] [00000000] *pgd=00000000
> [ 44.230575] Internal error: Oops: 5 [#1] PREEMPT ARM
> [ 44.235507] Modules linked in: usb_f_eem g_ether(+) usb_f_rndis u_ether libcomposite
> [ 44.243225] CPU: 0 PID: 2709 Comm: modprobe Not tainted 3.15.0-rc5+ #477
> [ 44.249892] task: e5828000 ti: e766a000 task.ti: e766a000
> [ 44.255276] PC is at module_add_driver+0x48/0xd0
> [ 44.259861] LR is at sysfs_do_create_link_sd+0x78/0xc8
> [ 44.264972] pc : [<c02a669c>] lr : [<c0152c5c>] psr: 80000013
> [ 44.264972] sp : e766bd58 ip : e766bd20 fp : e766bd74
> [ 44.276408] r10: 00000000 r9 : bf02a000 r8 : bf003ac8
> [ 44.281608] r7 : 00000000 r6 : c05cd720 r5 : bf008610 r4 : bf027a1c
> [ 44.288107] r3 : bf027808 r2 : 00000000 r1 : c04b6cd4 r0 : 000000d0
> [ 44.294608] Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 44.301713] Control: 10c5387d Table: 5580c019 DAC: 00000015
> [ 44.307431] Process modprobe (pid: 2709, stack limit = 0xe766a238)
> [ 44.313584] Stack: (0xe766bd58 to 0xe766c000)
> [ 44.317919] bd40: 00000000 bf027a1c
> [ 44.326068] bd60: e7694b80 c05cd720 e766bd9c e766bd78 c0297524 c02a6660 bf027808 e766bd88
> [ 44.334213] bd80: bf027a1c bf0016a4 bf0020bc bf003aa8 e766bdb4 e766bda0 c0298a24 c0297424
> [ 44.342359] bda0: bf0279d4 bf0016a4 e766bdc4 e766bdb8 c02ca118 c02989a8 e766bdec e766bdc8
> [ 44.350504] bdc0: bf0014a4 c02ca0e0 00000000 e766bf48 00000001 bf027a64 e68ae8c0 e766a000
> [ 44.358650] bde0: e766bdfc e766bdf0 bf02a014 bf001404 e766be7c e766be00 c000891c bf02a00c
> [ 44.366796] be00: e766be2c e766be10 c0058a04 c03b96c8 ffffffff c05b41fc 00000000 bf027a58
> [ 44.374942] be20: e766be3c e766be30 c0056bd4 c00589b4 e766be64 e766be40 c0047828 c0056bc8
> [ 44.383087] be40: 00000000 e766be50 e766bf48 00000001 bf027a64 e766bf48 00000001 bf027a64
> [ 44.391233] be60: e68ae8c0 00000001 bf027a58 c0077af8 e766bf3c e766be80 c007a560 c0008854
> [ 44.399379] be80: bf027a64 00007fff c0077e94 00000000 e766bebc ea9d5000 00000001 bf027a64
> [ 44.407525] bea0: 00000000 e766bed4 bf027a58 bf027aa0 e766a000 bf027bb8 ffffffff e766bf04
> [ 44.415670] bec0: e766bfa4 e766bed0 c0013d28 c009264c ea9f5000 00000000 00000000 00000000
> [ 44.423815] bee0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 44.431962] bf00: 00000000 00000000 00000000 00000000 20000013 00028a0c b6d83000 b6f22114
> [ 44.440108] bf20: 00000080 c000f948 e766a000 00000000 e766bfa4 e766bf40 c007ae34 c0078da0
> [ 44.448253] bf40: c0108eb0 00000000 ea9d5000 00028a0c ea9f359c ea9f33b1 ea9fd2ec 00000c04
> [ 44.456399] bf60: 00001084 00000000 00000000 00000000 00000031 00000032 00000019 00000000
> [ 44.464544] bf80: 00000010 00000000 00000000 b6f78308 00040000 b6f7ad00 00000000 e766bfa8
> [ 44.472691] bfa0: c000f6a0 c007ad8c b6f78308 00040000 b6d83000 00028a0c b6f22114 b6d83000
> [ 44.480836] bfc0: b6f78308 00040000 b6f7ad00 00000080 b6f783b0 00028a0c b6f22114 00000000
> [ 44.488982] bfe0: b6f2c134 bef5e968 b6f19190 b6e7bdb0 60000010 b6d83000 57ffd821 57ffdc21
> [ 44.497142] [<c02a669c>] (module_add_driver) from [<c0297524>] (bus_add_driver+0x10c/0x214)
> [ 44.505452] [<c0297524>] (bus_add_driver) from [<c0298a24>] (driver_register+0x88/0x104)
> [ 44.513516] [<c0298a24>] (driver_register) from [<c02ca118>] (usb_gadget_probe_driver+0x44/0x54)
> [ 44.522291] [<c02ca118>] (usb_gadget_probe_driver) from [<bf0014a4>] (usb_composite_probe+0xac/0xd8 [libcomposite])
> [ 44.532684] [<bf0014a4>] (usb_composite_probe [libcomposite]) from [<bf02a014>] (init+0x14/0x1c [g_ether])
> [ 44.542286] [<bf02a014>] (init [g_ether]) from [<c000891c>] (do_one_initcall+0xd4/0x188)
> [ 44.550346] [<c000891c>] (do_one_initcall) from [<c007a560>] (load_module+0x17cc/0x1fec)
> [ 44.558399] [<c007a560>] (load_module) from [<c007ae34>] (SyS_init_module+0xb4/0x120)
> [ 44.566199] [<c007ae34>] (SyS_init_module) from [<c000f6a0>] (ret_fast_syscall+0x0/0x48)
> [ 44.574253] Code: e5942004 e3a000d0 e59f107c e5943000 (e5922000)
> [ 44.580365] s3c-hsotg s3c-hsotg: new device is high-speed
> [ 44.585705] s3c-hsotg s3c-hsotg: s3c_hsotg_irq: USBRst
> [ 44.593826] s3c-hsotg s3c-hsotg: s3c_hsotg_irq: USBRst
> [ 44.606948] ---[ end trace 1973fc9dc47e3d30 ]---
> Segmentation fault
>
--
Best Regards,
Peter Chen
--
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
