RE: [PATCH 02/12] usb: chipidea: udc: add pullup fuction, needed by the uvc gadget

2012-09-28 Thread Alexander Shishkin 
Chen Peter-B29397 b29...@freescale.com writes:

  
   hw_write(ci, OP_USBINTR, ~0,
  
 USBi_UI|USBi_UEI|USBi_PCI|USBi_URI|USBi_SLI);
   -   hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
   } else {
   -   hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
   hw_write(ci, OP_USBINTR, ~0, 0);
   }
 
  Hi Marc, your above change break the function that load gadget before
  plug usb cable.
 
 What do you mean with that? When switching into device role, the
 otg can load every gadget-module without having the hardware pluged-in.
 

 Are you sure? In current chipidea otg design, the gadget will be freed
 when device-host, but the gadget will not be re-created when host-device.
 So, when the device connects to pc again, there will be an null pointer error.
 (I use g_serial.ko)

Hmm, gadget gets zeroed out on every gadget-whatever switch, but it
also gets reinitialized on every whatever-gadget transition. And it
really shouldn't cause a null dereference.

Are you running getty on g_serial's tty or are you just echo'ing to it?
I remember there was a problem with it assuming that there's always both
tx and rx.

Regards,
--
Alex
--
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: [PATCH 02/12] usb: chipidea: udc: add pullup fuction, needed by the uvc gadget

2012-09-28 Thread Chen Peter-B29397 
 
 
 Hmm, gadget gets zeroed out on every gadget-whatever switch, but it
 also gets reinitialized on every whatever-gadget transition. And it
 really shouldn't cause a null dereference.
 
When gadget-host, the call sequence like below:

udc_stop - usb_del_gadget_udc - usb_gadget_remove_driver -composite_unbind
- kfree(cdev);

When host-gadget (just plug out MicroB-A cable), the call sequence like below:
udc_start- usb_add_gadget_udc 
It will only create device udc, the struct usb_composite_dev *cdev does not be 
reallocated again.

When the device connects to host, the functions at composite.c will be called, 
as
cdev is freed, the null dereference will occur.

My oops like below (the oops occurs when bus reset occurs):

Unable to handle kernel NULL pointer dereference at virtual address 003c

pgd = 80004000

[003c] *pgd=

Internal error: Oops: 17 [#1] SMP ARM

Modules linked in: g_serial libcomposite

CPU: 0Not tainted  (3.6.0-rc3+ #42)

PC is at _raw_spin_lock_irqsave+0x18/0x58

LR is at composite_disconnect+0x24/0x64 [libcomposite]

pc : [804cd920]lr : [7f001000]psr: a193

sp : 80671da0  ip : 80671db0  fp : 80671dac

r10: bf8086e4  r9 : bf808680  r8 : 004b

r7 : bf833074  r6 : bf833078  r5 : 003c  r4 : 

r3 : bf3abd80  r2 : 003c  r1 : a193  r0 : a193

Flags: NzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel

Control: 10c53c7d  Table: 4ec1004a  DAC: 0017

Process swapper/0 (pid: 0, stack limit = 0x806702f0)

Stack: (0x80671da0 to 0x80672000)

1da0: 80671dcc 80671db0 7f001000 804cd914 bf833080 bf833010 bf833078 bf833074

1dc0: 80671dec 80671dd0 80366aa4 7f000fe8 bf833010 09202f20 bf833010 

1de0: 80671e4c 80671df0 80367928 803669f0 daffc000 0001 8488 0040

1e00: b7eec000 bf833014 0007 981d 80671e3c 80671e20 8004694c 80046460

1e20:  bf833010 09202f20   004b bf808680 bf8086e4

1e40: 80671e64 80671e50 80366050 80367638 bf01e100 bf8086d0 80671e9c 80671e68

1e60: 80073d48 80365ff8 2193 80679148 81257838 bf808680 bf8086d0 8066ddbc

1e80: 004b  412fc09a  80671eb4 80671ea0 80073ef4 80073d00

1ea0: bf808680 bf8086d0 80671ecc 80671eb8 80076c14 80073ea0 004b 80679148

1ec0: 80671ee4 80671ed0 80073cd8 80076b80 8067 80679148 80671f0c 80671ee8

1ee0: 8000f68c 80073cb4 f400010c 80678970 80671f30 f4000110 8067c978 412fc09a

1f00: 80671f2c 80671f10 800084bc 8000f644 8000f824 6013  80671f64

1f20: 80671f84 80671f30 8000e7c0 80008494   000f 80019720

1f40: 8067 806ae0c8 804cfd84  8067c978 412fc09a  80671f84

1f60: 80671f88 80671f78 8000f820 8000f824 6013  80671fac 80671f88

1f80: 8000fd38 8000f7f8 80678fb0 806ae000 8066063c 8fff 1000406a 412fc09a

1fa0: 80671fbc 80671fb0 804bad04 8000fc90 80671ff4 80671fc0 80624948 804bacac

1fc0:   806244c4   8066063c 10c53c7d 80678948

1fe0: 8066060c 8067c96c  80671ff8 10008044 806246b8  

Backtrace: 

[804cd908] (_raw_spin_lock_irqsave+0x0/0x58) from [7f001000] 
(composite_disconnect+0x24/0x64 [libcomposite])

[7f000fdc] (composite_disconnect+0x0/0x64 [libcomposite]) from [80366aa4] 
(_gadget_stop_activity+0xc0/0x120)

 r7:bf833074 r6:bf833078 r5:bf833010 r4:bf833080

[803669e4] (_gadget_stop_activity+0x0/0x120) from [80367928] 
(udc_irq+0x2fc/0xf90)

 r7: r6:bf833010 r5:09202f20 r4:bf833010

[8036762c] (udc_irq+0x0/0xf90) from [80366050] (ci_irq+0x64/0xdc)

[80365fec] (ci_irq+0x0/0xdc) from [80073d48] 
(handle_irq_event_percpu+0x54/0x1a0)

 r5:bf8086d0 r4:bf01e100

[80073cf4] (handle_irq_event_percpu+0x0/0x1a0) from [80073ef4] 
(handle_irq_event+0x60/0x80)

[80073e94] (handle_irq_event+0x0/0x80) from [80076c14] 
(handle_fasteoi_irq+0xa0/0x170)

 r5:bf8086d0 r4:bf808680

[80076b74] (handle_fasteoi_irq+0x0/0x170) from [80073cd8] 
(generic_handle_irq+0x30/0x38)

 r5:80679148 r4:004b

[80073ca8] (generic_handle_irq+0x0/0x38) from [8000f68c] 
(handle_IRQ+0x54/0xb4)

 r5:80679148 r4:8067

[8000f638] (handle_IRQ+0x0/0xb4) from [800084bc] (gic_handle_irq+0x34/0x68)

 r9:412fc09a r8:8067c978 r7:f4000110 r6:80671f30 r5:80678970

r4:f400010c

[80008488] (gic_handle_irq+0x0/0x68) from [8000e7c0] (__irq_svc+0x40/0x54)

Exception stack(0x80671f30 to 0x80671f78)

1f20:   000f 80019720

1f40: 8067 806ae0c8 804cfd84  8067c978 412fc09a  80671f84

1f60: 80671f88 80671f78 8000f820 8000f824 6013 

 r7:80671f64 r6: r5:6013 r4:8000f824

[8000f7ec] (default_idle+0x0/0x44) from [8000fd38] (cpu_idle+0xb4/0xf0)

[8000fc84] (cpu_idle+0x0/0xf0) from [804bad04] (rest_init+0x64/0x7c)

 r9:412fc09a r8:1000406a r7:8fff r6:8066063c r5:806ae000

r4:80678fb0

[804baca0] (rest_init+0x0/0x7c) from [80624948] (start_kernel+0x29c/0x2ec)

[806246ac] (start_kernel+0x0/0x2ec) from [10008044] (0x10008044)

Code: e24cb004

Re: [PATCH 02/12] usb: chipidea: udc: add pullup fuction, needed by the uvc gadget

2012-09-26 Thread Michael Grzeschik 
Hi,

On Thu, Sep 20, 2012 at 03:08:15PM +0800, Peter Chen wrote:
 On Wed, Sep 12, 2012 at 7:58 PM, Alexander Shishkin
 alexander.shish...@linux.intel.com wrote:
  From: Michael Grzeschik m.grzesc...@pengutronix.de
 
  Add function to physicaly enable or disable of pullup connection on the 
  USB-D+
  line. The uvc gaget will fail, if this function is not implemented.
 
  Cc: sta...@vger.kernel.org
  Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
  Acked-by: Felipe Balbi ba...@ti.com
  Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
  Signed-off-by: Alexander Shishkin alexander.shish...@linux.intel.com
  ---
   drivers/usb/chipidea/udc.c |   21 +
   1 file changed, 17 insertions(+), 4 deletions(-)
 
  diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
  index 7801a3f..32ee870 100644
  --- a/drivers/usb/chipidea/udc.c
  +++ b/drivers/usb/chipidea/udc.c
  @@ -78,8 +78,7 @@ static inline int ep_to_bit(struct ci13xxx *ci, int n)
   }
 
   /**
  - * hw_device_state: enables/disables interrupts  starts/stops device 
  (execute
  - *  without interruption)
  + * hw_device_state: enables/disables interrupts (execute without 
  interruption)
* @dma: 0 = disable, !0 = enable and set dma engine
*
* This function returns an error code
  @@ -91,9 +90,7 @@ static int hw_device_state(struct ci13xxx *ci, u32 dma)
  /* interrupt, error, port change, reset, sleep/suspend */
  hw_write(ci, OP_USBINTR, ~0,
   USBi_UI|USBi_UEI|USBi_PCI|USBi_URI|USBi_SLI);
  -   hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
  } else {
  -   hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
  hw_write(ci, OP_USBINTR, ~0, 0);
  }
 
 Hi Marc, your above change break the function that load gadget before
 plug usb cable.

What do you mean with that? When switching into device role, the
otg can load every gadget-module without having the hardware pluged-in.

 Does your change is because the set/clear usbcmd.rs twice at
 usb_gadget_probe_driver
 /usb_gadget_remove_driver? If it is, do you mind I submit  a patch to re-add 
 it?

If really needed, i suggest to use the api calls instead: 
usb_gadget_{probe,remove}_driver

 
  return 0;
  @@ -1420,6 +1417,21 @@ static int ci13xxx_vbus_draw(struct usb_gadget 
  *_gadget, unsigned mA)
  return -ENOTSUPP;
   }
 
  +/* Change Data+ pullup status
  + * this func is used by usb_gadget_connect/disconnet
  + */
  +static int ci13xxx_pullup(struct usb_gadget *_gadget, int is_on)
  +{
  +   struct ci13xxx *ci = container_of(_gadget, struct ci13xxx, gadget);
  +
  +   if (is_on)
  +   hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
  +   else
  +   hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
  +
  +   return 0;
  +}
  +
   static int ci13xxx_start(struct usb_gadget *gadget,
   struct usb_gadget_driver *driver);
   static int ci13xxx_stop(struct usb_gadget *gadget,
  @@ -1432,6 +1444,7 @@ static int ci13xxx_stop(struct usb_gadget *gadget,
   static const struct usb_gadget_ops usb_gadget_ops = {
  .vbus_session   = ci13xxx_vbus_session,
  .wakeup = ci13xxx_wakeup,
  +   .pullup = ci13xxx_pullup,
  .vbus_draw  = ci13xxx_vbus_draw,
  .udc_start  = ci13xxx_start,
  .udc_stop   = ci13xxx_stop,
  --
  1.7.10.4
 
  --
  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
 

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
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: [PATCH 02/12] usb: chipidea: udc: add pullup fuction, needed by the uvc gadget

2012-09-20 Thread Peter Chen 
On Wed, Sep 12, 2012 at 7:58 PM, Alexander Shishkin
alexander.shish...@linux.intel.com wrote:
 From: Michael Grzeschik m.grzesc...@pengutronix.de

 Add function to physicaly enable or disable of pullup connection on the USB-D+
 line. The uvc gaget will fail, if this function is not implemented.

 Cc: sta...@vger.kernel.org
 Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
 Acked-by: Felipe Balbi ba...@ti.com
 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
 Signed-off-by: Alexander Shishkin alexander.shish...@linux.intel.com
 ---
  drivers/usb/chipidea/udc.c |   21 +
  1 file changed, 17 insertions(+), 4 deletions(-)

 diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
 index 7801a3f..32ee870 100644
 --- a/drivers/usb/chipidea/udc.c
 +++ b/drivers/usb/chipidea/udc.c
 @@ -78,8 +78,7 @@ static inline int ep_to_bit(struct ci13xxx *ci, int n)
  }

  /**
 - * hw_device_state: enables/disables interrupts  starts/stops device 
 (execute
 - *  without interruption)
 + * hw_device_state: enables/disables interrupts (execute without 
 interruption)
   * @dma: 0 = disable, !0 = enable and set dma engine
   *
   * This function returns an error code
 @@ -91,9 +90,7 @@ static int hw_device_state(struct ci13xxx *ci, u32 dma)
 /* interrupt, error, port change, reset, sleep/suspend */
 hw_write(ci, OP_USBINTR, ~0,
  USBi_UI|USBi_UEI|USBi_PCI|USBi_URI|USBi_SLI);
 -   hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
 } else {
 -   hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
 hw_write(ci, OP_USBINTR, ~0, 0);
 }

Hi Marc, your above change break the function that load gadget before
plug usb cable.
Does your change is because the set/clear usbcmd.rs twice at
usb_gadget_probe_driver
/usb_gadget_remove_driver? If it is, do you mind I submit  a patch to re-add it?

 return 0;
 @@ -1420,6 +1417,21 @@ static int ci13xxx_vbus_draw(struct usb_gadget 
 *_gadget, unsigned mA)
 return -ENOTSUPP;
  }

 +/* Change Data+ pullup status
 + * this func is used by usb_gadget_connect/disconnet
 + */
 +static int ci13xxx_pullup(struct usb_gadget *_gadget, int is_on)
 +{
 +   struct ci13xxx *ci = container_of(_gadget, struct ci13xxx, gadget);
 +
 +   if (is_on)
 +   hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
 +   else
 +   hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
 +
 +   return 0;
 +}
 +
  static int ci13xxx_start(struct usb_gadget *gadget,
  struct usb_gadget_driver *driver);
  static int ci13xxx_stop(struct usb_gadget *gadget,
 @@ -1432,6 +1444,7 @@ static int ci13xxx_stop(struct usb_gadget *gadget,
  static const struct usb_gadget_ops usb_gadget_ops = {
 .vbus_session   = ci13xxx_vbus_session,
 .wakeup = ci13xxx_wakeup,
 +   .pullup = ci13xxx_pullup,
 .vbus_draw  = ci13xxx_vbus_draw,
 .udc_start  = ci13xxx_start,
 .udc_stop   = ci13xxx_stop,
 --
 1.7.10.4

 --
 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
--
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

[PATCH 02/12] usb: chipidea: udc: add pullup fuction, needed by the uvc gadget

2012-09-12 Thread Alexander Shishkin 
From: Michael Grzeschik m.grzesc...@pengutronix.de

Add function to physicaly enable or disable of pullup connection on the USB-D+
line. The uvc gaget will fail, if this function is not implemented.

Cc: sta...@vger.kernel.org
Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Acked-by: Felipe Balbi ba...@ti.com
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
Signed-off-by: Alexander Shishkin alexander.shish...@linux.intel.com
---
 drivers/usb/chipidea/udc.c |   21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 7801a3f..32ee870 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -78,8 +78,7 @@ static inline int ep_to_bit(struct ci13xxx *ci, int n)
 }
 
 /**
- * hw_device_state: enables/disables interrupts  starts/stops device (execute
- *  without interruption)
+ * hw_device_state: enables/disables interrupts (execute without interruption)
  * @dma: 0 = disable, !0 = enable and set dma engine
  *
  * This function returns an error code
@@ -91,9 +90,7 @@ static int hw_device_state(struct ci13xxx *ci, u32 dma)
/* interrupt, error, port change, reset, sleep/suspend */
hw_write(ci, OP_USBINTR, ~0,
 USBi_UI|USBi_UEI|USBi_PCI|USBi_URI|USBi_SLI);
-   hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
} else {
-   hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
hw_write(ci, OP_USBINTR, ~0, 0);
}
return 0;
@@ -1420,6 +1417,21 @@ static int ci13xxx_vbus_draw(struct usb_gadget *_gadget, 
unsigned mA)
return -ENOTSUPP;
 }
 
+/* Change Data+ pullup status
+ * this func is used by usb_gadget_connect/disconnet
+ */
+static int ci13xxx_pullup(struct usb_gadget *_gadget, int is_on)
+{
+   struct ci13xxx *ci = container_of(_gadget, struct ci13xxx, gadget);
+
+   if (is_on)
+   hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
+   else
+   hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
+
+   return 0;
+}
+
 static int ci13xxx_start(struct usb_gadget *gadget,
 struct usb_gadget_driver *driver);
 static int ci13xxx_stop(struct usb_gadget *gadget,
@@ -1432,6 +1444,7 @@ static int ci13xxx_stop(struct usb_gadget *gadget,
 static const struct usb_gadget_ops usb_gadget_ops = {
.vbus_session   = ci13xxx_vbus_session,
.wakeup = ci13xxx_wakeup,
+   .pullup = ci13xxx_pullup,
.vbus_draw  = ci13xxx_vbus_draw,
.udc_start  = ci13xxx_start,
.udc_stop   = ci13xxx_stop,
-- 
1.7.10.4

--
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