[PATCH] usb: isp1760: fix peripheral/device controller chip id

[PATCH] usb: isp1760: fix peripheral/device controller chip id

[Sudeep Holla]

As per the ISP1761 data sheet, the DcChipID register represents
the hardware version number (0015h) and the chip ID (8210h) for the
Peripheral Controller.

This patch fixes the chip ID value used to verify the controller.

Cc: Felipe Balbi <balbi@ti.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/usb/isp1760/isp1760-udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi Laurent,

I found that the chip ID is wrong when I tried testing OTG on vexpress
platforms. As per Section 10.8.2 DcChipID register in [1], it should be
0x00158210.

It's still not functional, I will follow up with the logs separately.

Regards,
Sudeep
[1] http://ridl.cfd.rit.edu/products/manuals/Altera/DE3_v.1.7_CDROM/Datasheet/USB/ISP1761.pdf#G2148521

diff --git a/drivers/usb/isp1760/isp1760-udc.c b/drivers/usb/isp1760/isp1760-udc.c
index 9612d7990565..87a8dc044fce 100644
--- a/drivers/usb/isp1760/isp1760-udc.c
+++ b/drivers/usb/isp1760/isp1760-udc.c
@@ -1411,7 +1411,7 @@ static int isp1760_udc_init(struct isp1760_udc *udc)
 		return -ENODEV;
 	}
 
-	if (chipid != 0x00011582) {
+	if (chipid != 0x00158210) {
 		dev_err(udc->isp->dev, "udc: invalid chip ID 0x%08x\n", chipid);
 		return -ENODEV;
 	}
-- 
1.9.1

Re: [PATCH] usb: isp1760: fix peripheral/device controller chip id

[Laurent Pinchart]

Hi Sudeep,

Thank you for the patch.

On Tuesday 24 February 2015 17:53:42 Sudeep Holla wrote:
> As per the ISP1761 data sheet, the DcChipID register represents
> the hardware version number (0015h) and the chip ID (8210h) for the
> Peripheral Controller.
> 
> This patch fixes the chip ID value used to verify the controller.
> 
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/usb/isp1760/isp1760-udc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Hi Laurent,
> 
> I found that the chip ID is wrong when I tried testing OTG on vexpress
> platforms. As per Section 10.8.2 DcChipID register in [1], it should be
> 0x00158210.

On the other hand, http://www.nxp.com/documents/data_sheet/SAF1761.pdf#120 
uses 0x00011582. It looks like we need to accept both.

> It's still not functional, I will follow up with the logs separately.
> 
> Regards,
> Sudeep
> [1]
> http://ridl.cfd.rit.edu/products/manuals/Altera/DE3_v.1.7_CDROM/Datasheet/U
> SB/ISP1761.pdf#G2148521
> 
> diff --git a/drivers/usb/isp1760/isp1760-udc.c
> b/drivers/usb/isp1760/isp1760-udc.c index 9612d7990565..87a8dc044fce 100644
> --- a/drivers/usb/isp1760/isp1760-udc.c
> +++ b/drivers/usb/isp1760/isp1760-udc.c
> @@ -1411,7 +1411,7 @@ static int isp1760_udc_init(struct isp1760_udc *udc)
>  		return -ENODEV;
>  	}
> 
> -	if (chipid != 0x00011582) {
> +	if (chipid != 0x00158210) {
>  		dev_err(udc->isp->dev, "udc: invalid chip ID 0x%08x\n", chipid);
>  		return -ENODEV;
>  	}

-- 
Regards,

Laurent Pinchart

Re: [PATCH] usb: isp1760: fix peripheral/device controller chip id

[Sudeep Holla]

Hi Laurent,

On 25/02/15 22:27, Laurent Pinchart wrote:
> Hi Sudeep,
>
> Thank you for the patch.
>
> On Tuesday 24 February 2015 17:53:42 Sudeep Holla wrote:
>> As per the ISP1761 data sheet, the DcChipID register represents
>> the hardware version number (0015h) and the chip ID (8210h) for the
>> Peripheral Controller.
>>
>> This patch fixes the chip ID value used to verify the controller.
>>
>> Cc: Felipe Balbi <balbi@ti.com>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>   drivers/usb/isp1760/isp1760-udc.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Hi Laurent,
>>
>> I found that the chip ID is wrong when I tried testing OTG on vexpress
>> platforms. As per Section 10.8.2 DcChipID register in [1], it should be
>> 0x00158210.
>
> On the other hand, http://www.nxp.com/documents/data_sheet/SAF1761.pdf#120
> uses 0x00011582. It looks like we need to accept both.
>

Thanks for sharing this datasheet, I will update to accept both ID and
resend.

Regards,
Sudeep

[PATCH v2] usb: isp1760: add peripheral/device controller chip id

[Sudeep Holla]

As per the SAF1761 data sheet[0], the DcChipID register represents
the hardware version number (0001h) and the chip ID (1582h) for the
Peripheral Controller.

However as per the ISP1761 data sheet[1], the DcChipID register
represents the hardware version number (0015h) and the chip ID (8210h)
for the Peripheral Controller.

This patch adds support for both the chip ID values.

[0] http://www.nxp.com/documents/data_sheet/SAF1761.pdf
[1] http://pdf.datasheetcatalog.com/datasheets2/74/742102_1.pdf

Cc: Felipe Balbi <balbi@ti.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/usb/isp1760/isp1760-udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Changes v1->v2:
 - Updated to support both chip IDs: 0x00011582 and 0x00158210 instead of
   just 0x00158210 as suggested by Laurent

diff --git a/drivers/usb/isp1760/isp1760-udc.c b/drivers/usb/isp1760/isp1760-udc.c
index 9612d7990565..6d618b3fab07 100644
--- a/drivers/usb/isp1760/isp1760-udc.c
+++ b/drivers/usb/isp1760/isp1760-udc.c
@@ -1411,7 +1411,7 @@ static int isp1760_udc_init(struct isp1760_udc *udc)
 		return -ENODEV;
 	}
 
-	if (chipid != 0x00011582) {
+	if (chipid != 0x00011582 && chipid != 0x00158210) {
 		dev_err(udc->isp->dev, "udc: invalid chip ID 0x%08x\n", chipid);
 		return -ENODEV;
 	}
-- 
1.9.1

Re: [PATCH v2] usb: isp1760: add peripheral/device controller chip id

[Laurent Pinchart]

Hi Sudeep,

Thank you for the patch.

On Thursday 26 February 2015 11:47:57 Sudeep Holla wrote:
> As per the SAF1761 data sheet[0], the DcChipID register represents
> the hardware version number (0001h) and the chip ID (1582h) for the
> Peripheral Controller.
> 
> However as per the ISP1761 data sheet[1], the DcChipID register
> represents the hardware version number (0015h) and the chip ID (8210h)
> for the Peripheral Controller.
> 
> This patch adds support for both the chip ID values.
> 
> [0] http://www.nxp.com/documents/data_sheet/SAF1761.pdf
> [1] http://pdf.datasheetcatalog.com/datasheets2/74/742102_1.pdf
> 
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/usb/isp1760/isp1760-udc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Changes v1->v2:
>  - Updated to support both chip IDs: 0x00011582 and 0x00158210 instead of
>    just 0x00158210 as suggested by Laurent
> 
> diff --git a/drivers/usb/isp1760/isp1760-udc.c
> b/drivers/usb/isp1760/isp1760-udc.c index 9612d7990565..6d618b3fab07 100644
> --- a/drivers/usb/isp1760/isp1760-udc.c
> +++ b/drivers/usb/isp1760/isp1760-udc.c
> @@ -1411,7 +1411,7 @@ static int isp1760_udc_init(struct isp1760_udc *udc)
>  		return -ENODEV;
>  	}
> 
> -	if (chipid != 0x00011582) {
> +	if (chipid != 0x00011582 && chipid != 0x00158210) {
>  		dev_err(udc->isp->dev, "udc: invalid chip ID 0x%08x\n", chipid);
>  		return -ENODEV;
>  	}

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] usb: isp1760: add peripheral/device controller chip id

[Sudeep Holla]

On 26/02/15 18:53, Laurent Pinchart wrote:
> Hi Sudeep,
>
> Thank you for the patch.
>
> On Thursday 26 February 2015 11:47:57 Sudeep Holla wrote:
>> As per the SAF1761 data sheet[0], the DcChipID register represents
>> the hardware version number (0001h) and the chip ID (1582h) for the
>> Peripheral Controller.
>>
>> However as per the ISP1761 data sheet[1], the DcChipID register
>> represents the hardware version number (0015h) and the chip ID (8210h)
>> for the Peripheral Controller.
>>
>> This patch adds support for both the chip ID values.
>>
>> [0] http://www.nxp.com/documents/data_sheet/SAF1761.pdf
>> [1] http://pdf.datasheetcatalog.com/datasheets2/74/742102_1.pdf
>>
>> Cc: Felipe Balbi <balbi@ti.com>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>

Thanks. I assume Felipe will pick this up.

Now about the testing in Gadget mode on Vexpress platform.
I have set
CONFIG_USB_OTG=y
CONFIG_USB_MON=y
CONFIG_USB_STORAGE=y
CONFIG_USB_ISP1760=y
CONFIG_USB_ISP1760_DUAL_ROLE=y
CONFIG_USB_GADGET=y
CONFIG_USB_MASS_STORAGE=m

When I execute:
$ modprobe g_mass_storage file=/home/linaro/backing_file

I get the following inconsistent lock state dump

--->8
Number of LUNs=8
Mass Storage Function, version: 2009/09/11
LUN: removable file: (no medium)
Number of LUNs=1
LUN: file: /home/linaro/backing_file
Number of LUNs=1
g_mass_storage gadget: Mass Storage Gadget, version: 2009/09/11
g_mass_storage gadget: userspace failed to provide iSerialNumber
g_mass_storage gadget: g_mass_storage ready

=================================
[ INFO: inconsistent lock state ]
4.0.0-rc2-00004-gf7bb2ef60173 #51 Not tainted
---------------------------------
inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
in:imklog/2118 [HC1[1]:SC0[0]:HE0:SE1] takes:
  (&(&udc->lock)->rlock){?.+...}, at: [<c0397a93>] 
isp1760_udc_irq+0x367/0x9dc
{HARDIRQ-ON-W} state was registered at:
   [<c05135b3>] _raw_spin_lock+0x23/0x30
   [<c0396b87>] isp1760_udc_start+0x23/0xf8
   [<c039dc21>] udc_bind_to_driver+0x71/0xb0
   [<c039de4f>] usb_gadget_probe_driver+0x53/0x9c
   [<bf80d0df>] usb_composite_probe+0x8a/0xa4 [libcomposite]
   [<bf8311a7>] 0xbf8311a7
   [<c00088c5>] do_one_initcall+0x8d/0x17c
   [<c050b92d>] do_init_module+0x49/0x148
   [<c0087323>] load_module+0xb7f/0xbc4
   [<c0087471>] SyS_finit_module+0x51/0x74
   [<c000d8c1>] ret_fast_syscall+0x1/0x68
irq event stamp: 4966
hardirqs last  enabled at (4965): [<c05137df>] 
_raw_spin_unlock_irq+0x1f/0x24
hardirqs last disabled at (4966): [<c00110b3>] __irq_svc+0x33/0x64
softirqs last  enabled at (4458): [<c0023475>] __do_softirq+0x23d/0x2d0
softirqs last disabled at (4389): [<c002380b>] irq_exit+0xef/0x15c

other info that might help us debug this:
  Possible unsafe locking scenario:

        CPU0
        ----
   lock(&(&udc->lock)->rlock);
   <Interrupt>
     lock(&(&udc->lock)->rlock);

  *** DEADLOCK ***

1 lock held by in:imklog/2118:
  #0:  (&f->f_pos_lock){+.+.+.}, at: [<c010a101>] __fdget_pos+0x31/0x34

stack backtrace:
CPU: 0 PID: 2118 Comm: in:imklog Not tainted 
4.0.0-rc2-00004-gf7bb2ef60173 #51
Hardware name: ARM-Versatile Express
[<c0013bc5>] (unwind_backtrace) from [<c001084d>] (show_stack+0x11/0x14)
[<c001084d>] (show_stack) from [<c050ea85>] (dump_stack+0x6d/0x78)
[<c050ea85>] (dump_stack) from [<c0054abd>] (print_usage_bug+0x1d9/0x260)
[<c0054abd>] (print_usage_bug) from [<c0054cb7>] (mark_lock+0x173/0x5d0)
[<c0054cb7>] (mark_lock) from [<c005600f>] (__lock_acquire+0xab7/0x19a4)
[<c005600f>] (__lock_acquire) from [<c0057575>] (lock_acquire+0x79/0xe8)
[<c0057575>] (lock_acquire) from [<c05135b3>] (_raw_spin_lock+0x23/0x30)
[<c05135b3>] (_raw_spin_lock) from [<c0397a93>] 
(isp1760_udc_irq+0x367/0x9dc)
[<c0397a93>] (isp1760_udc_irq) from [<c0065449>] 
(handle_irq_event_percpu+0x45/0x154)
[<c0065449>] (handle_irq_event_percpu) from [<c0065587>] 
(handle_irq_event+0x2f/0x44)
[<c0065587>] (handle_irq_event) from [<c006735b>] 
(handle_fasteoi_irq+0x6f/0xf0)
[<c006735b>] (handle_fasteoi_irq) from [<c0064d2f>] 
(generic_handle_irq+0x23/0x2c)
[<c0064d2f>] (generic_handle_irq) from [<c0064f75>] 
(__handle_domain_irq+0x45/0x84)
[<c0064f75>] (__handle_domain_irq) from [<c000856b>] 
(gic_handle_irq+0x27/0x50)
[<c000856b>] (gic_handle_irq) from [<c00110bf>] (__irq_svc+0x3f/0x64)
Exception stack(0xecc27e08 to 0xecc27e50)
7e00:                   00000001 ecd6cdd8 00000000 ecd6c7c0 ee78ff00 
c003b44a
7e20: c103a78c ecc27e68 c080d8b8 00000001 00000000 ecd6c7c0 c0992738 
ecc27e50
7e40: c0055161 c05137e0 400c0033 ffffffff
[<c00110bf>] (__irq_svc) from [<c05137e0>] (_raw_spin_unlock_irq+0x20/0x24)
[<c05137e0>] (_raw_spin_unlock_irq) from [<c003b48d>] 
(finish_task_switch+0x99/0x16c)
[<c003b48d>] (finish_task_switch) from [<c050f90d>] (__schedule+0x2fd/0x6f0)
[<c050f90d>] (__schedule) from [<c050fd2f>] (schedule+0x2f/0x64)
[<c050fd2f>] (schedule) from [<c00639a1>] (do_syslog+0x40d/0x470)
[<c00639a1>] (do_syslog) from [<c0139cd9>] (proc_reg_read+0x49/0x6c)
[<c0139cd9>] (proc_reg_read) from [<c00f44bd>] (vfs_read+0x55/0xb8)
[<c00f44bd>] (vfs_read) from [<c00f4555>] (SyS_read+0x35/0x74)
[<c00f4555>] (SyS_read) from [<c000d8c1>] (ret_fast_syscall+0x1/0x68)

---

Also I don't see any message on the host side. Let me know if there's
something wrong in my config or test commands.

--
Regards,
Sudeep

Re: [PATCH v2] usb: isp1760: add peripheral/device controller chip id

[Laurent Pinchart]

Hi Sudeep,

On Wednesday 04 March 2015 15:56:12 Sudeep Holla wrote:
> On 26/02/15 18:53, Laurent Pinchart wrote:
> > Hi Sudeep,
> > 
> > Thank you for the patch.
> > 
> > On Thursday 26 February 2015 11:47:57 Sudeep Holla wrote:
> >> As per the SAF1761 data sheet[0], the DcChipID register represents
> >> the hardware version number (0001h) and the chip ID (1582h) for the
> >> Peripheral Controller.
> >> 
> >> However as per the ISP1761 data sheet[1], the DcChipID register
> >> represents the hardware version number (0015h) and the chip ID (8210h)
> >> for the Peripheral Controller.
> >> 
> >> This patch adds support for both the chip ID values.
> >> 
> >> [0] http://www.nxp.com/documents/data_sheet/SAF1761.pdf
> >> [1] http://pdf.datasheetcatalog.com/datasheets2/74/742102_1.pdf
> >> 
> >> Cc: Felipe Balbi <balbi@ti.com>
> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > 
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Thanks. I assume Felipe will pick this up.
> 
> Now about the testing in Gadget mode on Vexpress platform.
> I have set
> CONFIG_USB_OTG=y
> CONFIG_USB_MON=y
> CONFIG_USB_STORAGE=y
> CONFIG_USB_ISP1760=y
> CONFIG_USB_ISP1760_DUAL_ROLE=y
> CONFIG_USB_GADGET=y
> CONFIG_USB_MASS_STORAGE=m
> 
> When I execute:
> $ modprobe g_mass_storage file=/home/linaro/backing_file
> 
> I get the following inconsistent lock state dump
> 
> --->8
> Number of LUNs=8
> Mass Storage Function, version: 2009/09/11
> LUN: removable file: (no medium)
> Number of LUNs=1
> LUN: file: /home/linaro/backing_file
> Number of LUNs=1
> g_mass_storage gadget: Mass Storage Gadget, version: 2009/09/11
> g_mass_storage gadget: userspace failed to provide iSerialNumber
> g_mass_storage gadget: g_mass_storage ready
> 
> =================================
> [ INFO: inconsistent lock state ]
> 4.0.0-rc2-00004-gf7bb2ef60173 #51 Not tainted
> ---------------------------------
> inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
> in:imklog/2118 [HC1[1]:SC0[0]:HE0:SE1] takes:
>   (&(&udc->lock)->rlock){?.+...}, at: [<c0397a93>]
> isp1760_udc_irq+0x367/0x9dc
> {HARDIRQ-ON-W} state was registered at:
>    [<c05135b3>] _raw_spin_lock+0x23/0x30
>    [<c0396b87>] isp1760_udc_start+0x23/0xf8
>    [<c039dc21>] udc_bind_to_driver+0x71/0xb0
>    [<c039de4f>] usb_gadget_probe_driver+0x53/0x9c
>    [<bf80d0df>] usb_composite_probe+0x8a/0xa4 [libcomposite]
>    [<bf8311a7>] 0xbf8311a7
>    [<c00088c5>] do_one_initcall+0x8d/0x17c
>    [<c050b92d>] do_init_module+0x49/0x148
>    [<c0087323>] load_module+0xb7f/0xbc4
>    [<c0087471>] SyS_finit_module+0x51/0x74
>    [<c000d8c1>] ret_fast_syscall+0x1/0x68
> irq event stamp: 4966
> hardirqs last  enabled at (4965): [<c05137df>]
> _raw_spin_unlock_irq+0x1f/0x24
> hardirqs last disabled at (4966): [<c00110b3>] __irq_svc+0x33/0x64
> softirqs last  enabled at (4458): [<c0023475>] __do_softirq+0x23d/0x2d0
> softirqs last disabled at (4389): [<c002380b>] irq_exit+0xef/0x15c
> 
> other info that might help us debug this:
>   Possible unsafe locking scenario:
> 
>         CPU0
>         ----
>    lock(&(&udc->lock)->rlock);
>    <Interrupt>
>      lock(&(&udc->lock)->rlock);
> 
>   *** DEADLOCK ***
> 
> 1 lock held by in:imklog/2118:
>   #0:  (&f->f_pos_lock){+.+.+.}, at: [<c010a101>] __fdget_pos+0x31/0x34
> 
> stack backtrace:
> CPU: 0 PID: 2118 Comm: in:imklog Not tainted
> 4.0.0-rc2-00004-gf7bb2ef60173 #51
> Hardware name: ARM-Versatile Express
> [<c0013bc5>] (unwind_backtrace) from [<c001084d>] (show_stack+0x11/0x14)
> [<c001084d>] (show_stack) from [<c050ea85>] (dump_stack+0x6d/0x78)
> [<c050ea85>] (dump_stack) from [<c0054abd>] (print_usage_bug+0x1d9/0x260)
> [<c0054abd>] (print_usage_bug) from [<c0054cb7>] (mark_lock+0x173/0x5d0)
> [<c0054cb7>] (mark_lock) from [<c005600f>] (__lock_acquire+0xab7/0x19a4)
> [<c005600f>] (__lock_acquire) from [<c0057575>] (lock_acquire+0x79/0xe8)
> [<c0057575>] (lock_acquire) from [<c05135b3>] (_raw_spin_lock+0x23/0x30)
> [<c05135b3>] (_raw_spin_lock) from [<c0397a93>]
> (isp1760_udc_irq+0x367/0x9dc)
> [<c0397a93>] (isp1760_udc_irq) from [<c0065449>]
> (handle_irq_event_percpu+0x45/0x154)
> [<c0065449>] (handle_irq_event_percpu) from [<c0065587>]
> (handle_irq_event+0x2f/0x44)
> [<c0065587>] (handle_irq_event) from [<c006735b>]
> (handle_fasteoi_irq+0x6f/0xf0)
> [<c006735b>] (handle_fasteoi_irq) from [<c0064d2f>]
> (generic_handle_irq+0x23/0x2c)
> [<c0064d2f>] (generic_handle_irq) from [<c0064f75>]
> (__handle_domain_irq+0x45/0x84)
> [<c0064f75>] (__handle_domain_irq) from [<c000856b>]
> (gic_handle_irq+0x27/0x50)
> [<c000856b>] (gic_handle_irq) from [<c00110bf>] (__irq_svc+0x3f/0x64)
> Exception stack(0xecc27e08 to 0xecc27e50)
> 7e00:                   00000001 ecd6cdd8 00000000 ecd6c7c0 ee78ff00
> c003b44a
> 7e20: c103a78c ecc27e68 c080d8b8 00000001 00000000 ecd6c7c0 c0992738
> ecc27e50
> 7e40: c0055161 c05137e0 400c0033 ffffffff
> [<c00110bf>] (__irq_svc) from [<c05137e0>] (_raw_spin_unlock_irq+0x20/0x24)
> [<c05137e0>] (_raw_spin_unlock_irq) from [<c003b48d>]
> (finish_task_switch+0x99/0x16c)
> [<c003b48d>] (finish_task_switch) from [<c050f90d>] (__schedule+0x2fd/0x6f0)
> [<c050f90d>] (__schedule) from [<c050fd2f>] (schedule+0x2f/0x64)
> [<c050fd2f>] (schedule) from [<c00639a1>] (do_syslog+0x40d/0x470)
> [<c00639a1>] (do_syslog) from [<c0139cd9>] (proc_reg_read+0x49/0x6c)
> [<c0139cd9>] (proc_reg_read) from [<c00f44bd>] (vfs_read+0x55/0xb8)
> [<c00f44bd>] (vfs_read) from [<c00f4555>] (SyS_read+0x35/0x74)
> [<c00f4555>] (SyS_read) from [<c000d8c1>] (ret_fast_syscall+0x1/0x68)
> 
> ---
> 
> Also I don't see any message on the host side. Let me know if there's
> something wrong in my config or test commands.

Looks like a driver bug to me, .udc_start() and .udc_stop() should use 
spin_(un)lock_irq(). Would you like to submit a patch ? I can also do it.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] usb: isp1760: add peripheral/device controller chip id

[Sudeep Holla]

On 04/03/15 16:11, Laurent Pinchart wrote:
> Hi Sudeep,
>
> On Wednesday 04 March 2015 15:56:12 Sudeep Holla wrote:

[...]

>>
>> Also I don't see any message on the host side. Let me know if there's
>> something wrong in my config or test commands.
>
> Looks like a driver bug to me, .udc_start() and .udc_stop() should use
> spin_(un)lock_irq(). Would you like to submit a patch ? I can also do it.
>

Thanks for the quick response. Yes I can submit that as I do have the
patch already. Though it removes above dump for possible deadlock, it's
still not working. So I was not sure if that's the right fix. Anyways I
will post that patch and you can review :)

BTW, if you see above the log also contains "g_mass_storage gadget:
userspace failed to provide iSerialNumber". Is this OK ? or is this the
reason for not seeing any activity on host side.

Regards,
Sudeep

Re: [PATCH v2] usb: isp1760: add peripheral/device controller chip id

[Laurent Pinchart]

Hi Sudeep,

On Wednesday 04 March 2015 17:04:43 Sudeep Holla wrote:
> On 04/03/15 16:11, Laurent Pinchart wrote:
> > Hi Sudeep,
> 
> > On Wednesday 04 March 2015 15:56:12 Sudeep Holla wrote:
> [...]
> 
> >> Also I don't see any message on the host side. Let me know if there's
> >> something wrong in my config or test commands.
> > 
> > Looks like a driver bug to me, .udc_start() and .udc_stop() should use
> > spin_(un)lock_irq(). Would you like to submit a patch ? I can also do it.
> 
> Thanks for the quick response. Yes I can submit that as I do have the
> patch already. Though it removes above dump for possible deadlock, it's
> still not working. So I was not sure if that's the right fix. Anyways I
> will post that patch and you can review :)
> 
> BTW, if you see above the log also contains "g_mass_storage gadget:
> userspace failed to provide iSerialNumber". Is this OK ? or is this the
> reason for not seeing any activity on host side.

That should be ok.

To rule out the possible low-level issues, have you checked that the ISP1761 
OC1_N/VBUS pin is properly connected to the VBUS power supply coming from the 
host ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] usb: isp1760: add peripheral/device controller chip id

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1132 bytes --]

On Wed, Mar 04, 2015 at 03:56:12PM +0000, Sudeep Holla wrote:
> 
> 
> On 26/02/15 18:53, Laurent Pinchart wrote:
> >Hi Sudeep,
> >
> >Thank you for the patch.
> >
> >On Thursday 26 February 2015 11:47:57 Sudeep Holla wrote:
> >>As per the SAF1761 data sheet[0], the DcChipID register represents
> >>the hardware version number (0001h) and the chip ID (1582h) for the
> >>Peripheral Controller.
> >>
> >>However as per the ISP1761 data sheet[1], the DcChipID register
> >>represents the hardware version number (0015h) and the chip ID (8210h)
> >>for the Peripheral Controller.
> >>
> >>This patch adds support for both the chip ID values.
> >>
> >>[0] http://www.nxp.com/documents/data_sheet/SAF1761.pdf
> >>[1] http://pdf.datasheetcatalog.com/datasheets2/74/742102_1.pdf
> >>
> >>Cc: Felipe Balbi <balbi@ti.com>
> >>Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> >
> >Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> 
> Thanks. I assume Felipe will pick this up.

I will, but I need Laurent's Ack.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2] usb: isp1760: add peripheral/device controller chip id

[Sudeep Holla]

Hi Filipe,

On 09/03/15 15:49, Felipe Balbi wrote:
> On Wed, Mar 04, 2015 at 03:56:12PM +0000, Sudeep Holla wrote:
>>
>>
>> On 26/02/15 18:53, Laurent Pinchart wrote:
>>> Hi Sudeep,
>>>
>>> Thank you for the patch.
>>>
>>> On Thursday 26 February 2015 11:47:57 Sudeep Holla wrote:
>>>> As per the SAF1761 data sheet[0], the DcChipID register represents
>>>> the hardware version number (0001h) and the chip ID (1582h) for the
>>>> Peripheral Controller.
>>>>
>>>> However as per the ISP1761 data sheet[1], the DcChipID register
>>>> represents the hardware version number (0015h) and the chip ID (8210h)
>>>> for the Peripheral Controller.
>>>>
>>>> This patch adds support for both the chip ID values.
>>>>
>>>> [0] http://www.nxp.com/documents/data_sheet/SAF1761.pdf
>>>> [1] http://pdf.datasheetcatalog.com/datasheets2/74/742102_1.pdf
>>>>
>>>> Cc: Felipe Balbi <balbi@ti.com>
>>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>
>>> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>
>>
>> Thanks. I assume Felipe will pick this up.
>
> I will, but I need Laurent's Ack.
>

I am confused, Laurent has already Ack-ed(see above) this patch[0]
as well as [1].

Regards,
Sudeep

[0] https://lkml.org/lkml/2015/2/26/525
[1] https://lkml.org/lkml/2015/3/5/209

Re: [PATCH v2] usb: isp1760: add peripheral/device controller chip id

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1484 bytes --]

On Mon, Mar 09, 2015 at 03:58:06PM +0000, Sudeep Holla wrote:
> Hi Filipe,
> 
> On 09/03/15 15:49, Felipe Balbi wrote:
> >On Wed, Mar 04, 2015 at 03:56:12PM +0000, Sudeep Holla wrote:
> >>
> >>
> >>On 26/02/15 18:53, Laurent Pinchart wrote:
> >>>Hi Sudeep,
> >>>
> >>>Thank you for the patch.
> >>>
> >>>On Thursday 26 February 2015 11:47:57 Sudeep Holla wrote:
> >>>>As per the SAF1761 data sheet[0], the DcChipID register represents
> >>>>the hardware version number (0001h) and the chip ID (1582h) for the
> >>>>Peripheral Controller.
> >>>>
> >>>>However as per the ISP1761 data sheet[1], the DcChipID register
> >>>>represents the hardware version number (0015h) and the chip ID (8210h)
> >>>>for the Peripheral Controller.
> >>>>
> >>>>This patch adds support for both the chip ID values.
> >>>>
> >>>>[0] http://www.nxp.com/documents/data_sheet/SAF1761.pdf
> >>>>[1] http://pdf.datasheetcatalog.com/datasheets2/74/742102_1.pdf
> >>>>
> >>>>Cc: Felipe Balbi <balbi@ti.com>
> >>>>Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> >>>
> >>>Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>
> >>
> >>Thanks. I assume Felipe will pick this up.
> >
> >I will, but I need Laurent's Ack.
> >
> 
> I am confused, Laurent has already Ack-ed(see above) this patch[0]
> as well as [1].

yeah, just came back from vacations, going through my inbox.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]