Re: linux-next and usb gadget composite BUG

2013-02-25 Thread Robert Jarzmik 
Felipe Balbi ba...@ti.com writes:

 this is not caused by current pull request. Well, you could argue that
 commit 70189a63d408d4ea0cddbf0ff0afe6020844e813 (usb: gadget:
 pxa27x_udc: convert to udc_start/udc_stop) is the culprit, but the fact
 is that what pxa27x is doing is pretty wrong.
No need to argue, I'm only looking for a fix :)
I just hope this change will still enable to change the gadget (ie. switch from
g_ether to g_zero by rmmoding/modprobing).

 The bug is that pxa27x_udc tries to register gadget-dev at udc_start()
 time and that's only called after gadget driver has already been bound
 to the controller. Since commit above converted pxa27x to
 udc_start()/udc_stop() calls, it exposed the issue.

 Anyway, this patch should fix it (I didn't even compile test it, since
 pxa27x still depends on its architecture, which needs to be fixed).

I suppose you made that patch against your own tree, not linux-next nor Linus's
tree.
But anyway, I took your changes into linux-next and it solves the problem.

So please take my :
Tested-by: Robert Jarzmik robert.jarz...@free.fr

I think the same problem occurs on other drivers. Out of my head I see :
  - pxa25x_udc.c
  - s3c2410_udc.c
  - imx_udc_start.c

Cheers.

--
Robert
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: linux-next and usb gadget composite BUG

2013-02-25 Thread Felipe Balbi 
On Mon, Feb 25, 2013 at 07:16:26PM +0100, Robert Jarzmik wrote:
 Felipe Balbi ba...@ti.com writes:
 
  this is not caused by current pull request. Well, you could argue that
  commit 70189a63d408d4ea0cddbf0ff0afe6020844e813 (usb: gadget:
  pxa27x_udc: convert to udc_start/udc_stop) is the culprit, but the fact
  is that what pxa27x is doing is pretty wrong.
 No need to argue, I'm only looking for a fix :)
 I just hope this change will still enable to change the gadget (ie. switch 
 from
 g_ether to g_zero by rmmoding/modprobing).
 
  The bug is that pxa27x_udc tries to register gadget-dev at udc_start()
  time and that's only called after gadget driver has already been bound
  to the controller. Since commit above converted pxa27x to
  udc_start()/udc_stop() calls, it exposed the issue.
 
  Anyway, this patch should fix it (I didn't even compile test it, since
  pxa27x still depends on its architecture, which needs to be fixed).
 
 I suppose you made that patch against your own tree, not linux-next nor 
 Linus's
 tree.
 But anyway, I took your changes into linux-next and it solves the problem.
 
 So please take my :
 Tested-by: Robert Jarzmik robert.jarz...@free.fr

thanks, will add your tested-by for sure.

 I think the same problem occurs on other drivers. Out of my head I see :
   - pxa25x_udc.c
   - s3c2410_udc.c
   - imx_udc_start.c

Thank you, I'll patch those too unless someone beats me to it ;-)

-- 
balbi


signature.asc
Description: Digital signature

Re: linux-next and usb gadget composite BUG

2013-02-24 Thread Felipe Balbi 
Hi,

On Sun, Feb 24, 2013 at 12:06:35AM +0100, Robert Jarzmik wrote:
 Hi Felipe,
 
 I tried today's version of linux-next tree (commit id
 b1714a88bee1ae, Add linux-next specific files for 20130222).
 
 My PXA based platform bugs on bootup. I have no console available on my
 platform, just a JTAG, so this is the info I have :
  - I'm using a Mitac MIO A701 board (arch/arm/mach-pxa/mioa701.c)
  - on gadget side, I'm using g_ether + pxa27x_udc
 
 So far, I didn't have any composite driver enabled (as the PXA suffer from a
 severe hardware defect, see comment in header of pxa27x_udc.c).
 
 Now, what I gathered from the JTAG is in [1].
 You can see that in sysfs_create_file(), the first argument has :
 dev-kobj-sd == NULL.
 
 And of course in fs/sysfs/file.c :
   BUG_ON(!kobj || !kobj-sd || !attr);
 
 Therefore, I hit a BUG.
 
 Could you tell me what changed recently that triggers this ? I saw in my 
 .config
 CONFIG_USB_LIBCOMPOSITE=y which looks a bit suspect to me, but I'm sure you'll
 tell me what recent change prevents my platform from booting.

this is not caused by current pull request. Well, you could argue that
commit 70189a63d408d4ea0cddbf0ff0afe6020844e813 (usb: gadget:
pxa27x_udc: convert to udc_start/udc_stop) is the culprit, but the fact
is that what pxa27x is doing is pretty wrong.

The bug is that pxa27x_udc tries to register gadget-dev at udc_start()
time and that's only called after gadget driver has already been bound
to the controller. Since commit above converted pxa27x to
udc_start()/udc_stop() calls, it exposed the issue.

Anyway, this patch should fix it (I didn't even compile test it, since
pxa27x still depends on its architecture, which needs to be fixed).

commit de7dedf26d906e3a2d6bf60385f8a55b9938ae16
Author: Felipe Balbi ba...@ti.com
Date:   Mon Feb 25 08:49:05 2013 +0200

usb: gadget: pxa27x: fix gadget-dev registration

Whenever -udc_start() gets called, gadet driver
has already being bound to the udc controller, which
means that gadget-dev had to be already initialized
and added to driver model.

This patch fixes pxa27x mistake.

Signed-off-by: Felipe Balbi ba...@ti.com

diff --git a/drivers/usb/gadget/pxa27x_udc.c b/drivers/usb/gadget/pxa27x_udc.c
index 2b3b01d..8f39db2 100644
--- a/drivers/usb/gadget/pxa27x_udc.c
+++ b/drivers/usb/gadget/pxa27x_udc.c
@@ -1821,11 +1821,6 @@ static int pxa27x_udc_start(struct usb_gadget_driver 
*driver,
udc-gadget.dev.driver = driver-driver;
dplus_pullup(udc, 1);
 
-   retval = device_add(udc-gadget.dev);
-   if (retval) {
-   dev_err(udc-dev, device_add error %d\n, retval);
-   goto add_fail;
-   }
retval = bind(udc-gadget, driver);
if (retval) {
dev_err(udc-dev, bind to driver %s -- error %d\n,
@@ -1852,8 +1847,6 @@ transceiver_fail:
if (driver-unbind)
driver-unbind(udc-gadget);
 bind_fail:
-   device_del(udc-gadget.dev);
-add_fail:
udc-driver = NULL;
udc-gadget.dev.driver = NULL;
return retval;
@@ -1905,7 +1898,6 @@ static int pxa27x_udc_stop(struct usb_gadget_driver 
*driver)
driver-unbind(udc-gadget);
udc-driver = NULL;
 
-   device_del(udc-gadget.dev);
dev_info(udc-dev, unregistered gadget driver '%s'\n,
 driver-driver.name);
 
@@ -2511,13 +2503,24 @@ static int __init pxa_udc_probe(struct platform_device 
*pdev)
driver_name, udc-irq, retval);
goto err_irq;
}
+
+   retval = device_add(udc-gadget.dev);
+   if (retval) {
+   dev_err(udc-dev, device_add error %d\n, retval);
+   goto err_dev_add;
+   }
+
retval = usb_add_gadget_udc(pdev-dev, udc-gadget);
if (retval)
goto err_add_udc;
 
pxa_init_debugfs(udc);
+
return 0;
+
 err_add_udc:
+   device_unregister(udc-gadget.dev);
+err_dev_add:
free_irq(udc-irq, udc);
 err_irq:
iounmap(udc-regs);
@@ -2538,6 +2541,7 @@ static int __exit pxa_udc_remove(struct platform_device 
*_dev)
int gpio = udc-mach-gpio_pullup;
 
usb_del_gadget_udc(udc-gadget);
+   device_del(udc-gadget.dev);
usb_gadget_unregister_driver(udc-driver);
free_irq(udc-irq, udc);
pxa_cleanup_debugfs(udc);

-- 
balbi


signature.asc
Description: Digital signature

linux-next and usb gadget composite BUG

2013-02-23 Thread Robert Jarzmik 
Hi Felipe,

I tried today's version of linux-next tree (commit id
b1714a88bee1ae, Add linux-next specific files for 20130222).

My PXA based platform bugs on bootup. I have no console available on my
platform, just a JTAG, so this is the info I have :
 - I'm using a Mitac MIO A701 board (arch/arm/mach-pxa/mioa701.c)
 - on gadget side, I'm using g_ether + pxa27x_udc

So far, I didn't have any composite driver enabled (as the PXA suffer from a
severe hardware defect, see comment in header of pxa27x_udc.c).

Now, what I gathered from the JTAG is in [1].
You can see that in sysfs_create_file(), the first argument has :
dev-kobj-sd == NULL.

And of course in fs/sysfs/file.c :
BUG_ON(!kobj || !kobj-sd || !attr);

Therefore, I hit a BUG.

Could you tell me what changed recently that triggers this ? I saw in my .config
CONFIG_USB_LIBCOMPOSITE=y which looks a bit suspect to me, but I'm sure you'll
tell me what recent change prevents my platform from booting.

Cheers.

--
Robert

[1]: JTAG + GDB analysis
1597ret = device_create_file(gadget-dev, dev_attr_suspended);
(gdb) bt
#0  composite_dev_prepare (composite=0xc04fdb6c, cdev=0xc39a8660) at 
drivers/usb/gadget/composite.c:1597
#1  0xc024cf80 in composite_bind (gadget=0xc04f4020, gdriver=0xc04fdb94) at 
drivers/usb/gadget/composite.c:1660
#2  0xc024ae2c in udc_bind_to_driver (udc=0xc3a10ec0, driver=0xc39a85a0) at 
drivers/usb/gadget/udc-core.c:272
#3  0xc024af40 in usb_gadget_probe_driver (driver=0xc04fdb94) at 
drivers/usb/gadget/udc-core.c:337
#4  0xc024c718 in usb_composite_probe (driver=0xc04fdb6c) at 
drivers/usb/gadget/composite.c:1793
#5  0xc04aac68 in init () at drivers/usb/gadget/ether.c:366
#6  0xc0008608 in do_one_initcall (fn=0xc04aac54 init) at init/main.c:690
#7  0xc0496318 in do_initcall_level () at init/main.c:761
#8  do_initcalls () at init/main.c:769
#9  do_basic_setup () at init/main.c:788
#10 kernel_init_freeable () at init/main.c:890
#11 0xc038e2e4 in kernel_init (unused=0xc39fe800) at init/main.c:822
#12 0xc0009320 in ret_from_fork () at arch/arm/kernel/entry-common.S:92
Backtrace stopped: frame did not save the PC
(gdb) p composite
$1 = (struct usb_composite_driver *) 0xc04fdb6c
(gdb) p *composite
$2 = {name = 0xc045e2d0 g_ether, dev = 0xc04f46e0, strings = 0xc04f4988, 
max_speed = USB_SPEED_SUPER, needs_serial = 0, bind = 0xc04aac70 eth_bind, 
unbind = 0, disconnect = 0, 
  suspend = 0, resume = 0, gadget_driver = {function = 0xc045e2d0 g_ether, 
max_speed = USB_SPEED_SUPER, bind = 0xc024cf04 composite_bind, unbind = 
0xc024cdc4 composite_unbind, 
setup = 0xc024d498 composite_setup, disconnect = 0xc024c8ac 
composite_disconnect, suspend = 0xc024c7f8 composite_suspend, resume = 
0xc024c740 composite_resume, driver = {
  name = 0xc045e2d0 g_ether, bus = 0x0, owner = 0x0, mod_name = 0x0, 
suppress_bind_attrs = false, of_match_table = 0x0, acpi_match_table = 0x0, 
probe = 0, remove = 0, shutdown = 0, 
  suspend = 0, resume = 0, groups = 0x0, pm = 0x0, p = 0x0}}}
(gdb) p *cdev
$3 = {gadget = 0xc04f4020, req = 0xc39a85a0, config = 0x0, suspended = 0, desc 
= {bLength = 0 '\000', bDescriptorType = 0 '\000', bcdUSB = 0, bDeviceClass = 0 
'\000', 
bDeviceSubClass = 0 '\000', bDeviceProtocol = 0 '\000', bMaxPacketSize0 = 0 
'\000', idVendor = 0, idProduct = 0, bcdDevice = 0, iManufacturer = 0 '\000', 
iProduct = 0 '\000', 
iSerialNumber = 0 '\000', bNumConfigurations = 0 '\000'}, configs = {next = 
0xc39a8680, prev = 0xc39a8680}, gstrings = {next = 0xc39a8688, prev = 
0xc39a8688}, driver = 0x0, 
  next_string_id = 0 '\000', def_manufacturer = 0x0, deactivations = 0, 
delayed_status = 0, lock = {{rlock = {raw_lock = {No data fields}
(gdb) p *gadget
$4 = {ops = 0xc03ba528, ep0 = 0xc04f413c, ep_list = {next = 0xc04f4180, prev = 
0xc04f4260}, speed = USB_SPEED_FULL, max_speed = USB_SPEED_UNKNOWN, 
sg_supported = 0, is_otg = 0, 
  is_a_peripheral = 0, b_hnp_enable = 0, a_hnp_support = 0, a_alt_hnp_support = 
0, name = 0xc03ba370 pxa27x_udc, dev = {parent = 0xc04d4328, p = 0xc39a8600, 
kobj = {name = 0x0, entry = {
next = 0xc04f404c, prev = 0xc04f404c}, parent = 0x0, kset = 0xc3822be0, 
ktype = 0xc04f2078, sd = 0x0, kref = {refcount = {counter = 1}}, 
state_initialized = 1, state_in_sysfs = 0, 
  state_add_uevent_sent = 0, state_remove_uevent_sent = 0, uevent_suppress 
= 0}, init_name = 0xc045e180 gadget, type = 0x0, mutex = {count = {counter = 
1}, wait_lock = {{rlock = {
raw_lock = {No data fields, wait_list = {next = 0xc04f4078, 
prev = 0xc04f4078}}, bus = 0x0, driver = 0x0, platform_data = 0x0, power = 
{power_state = {event = -1}, 
  can_wakeup = 0, async_suspend = 0, is_prepared = false, is_suspended = 
false, ignore_children = false, early_init = true, lock = {{rlock = {raw_lock = 
{No data fields, entry = {
next = 0xc04f4094, prev = 0xc04f4094}, completion = {done = 2147483647, 
wait = {lock = {{rlock = {raw_lock = {No data fields, task_list = {next =