Re: commit ef11982dd7a657512c362242508bb4021e0d67b6 breaks musb

2015-03-04 Thread Amit Virdi

+cc Sebastian, Alan Stern

Hello Felipe,

On 2/28/2015 3:18 AM, Felipe Balbi wrote:

Hi Amit,

commit ef11982dd7a657512c362242508bb4021e0d67b6 (Add support for
interrupt EP) actually broke testusb for MUSB when MUSB is the gadget.

The reason is that we're requesting an endpoint with a 64-byte FIFO, but
later deciding to use the same endpoint with wMaxPacketSize set to 1024
and MUSB errors out because the endpoint was selected for 64-byte only.

This only happens when trying to set alternate setting 2 and it's pretty
easy to trigger.



This issue was brought up earlier by Sebastian. He even submitted a 
patch for the same and there were discussions over it.

http://www.spinics.net/lists/linux-usb/msg117962.html

However, things were never concluded. Using module parameters should get 
it working but that isn't a clean solution.


What do you suggest?

Regards
Amit Virdi
--
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] usb: gadget: zero: fix format string warnings

2015-01-23 Thread Amit Virdi

On 1/21/2015 2:36 PM, Asaf Vertz wrote:

Fixed the following warnings (reported by cppcheck):
[drivers/usb/gadget/function/f_sourcesink.c:1217]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1261]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1305]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1349]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1393]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1437]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1476]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1520]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1564]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'.
[drivers/usb/gadget/function/f_sourcesink.c:1608]: (warning) %d in format 
string (no. 1)
requires 'int' but the argument type is 'unsigned int'

Signed-off-by: Asaf Vertz
---
  drivers/usb/gadget/function/f_sourcesink.c |   20 ++--
  1 files changed, 10 insertions(+), 10 deletions(-)


Reviewed-by: Amit Virdi 
--
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 2/4] usb: dwc3: gadget: Stop TRB preparation after limit is reached

2015-01-14 Thread Amit Virdi

Alright, I just applied your patches to testing/fixes. I'll start
testing today and should be able to send a pull request to Greg by the
end of the week, hopefully.



Thanks! Just a small clarification - git failed to send patches to 
stable kernel list again (unfortunately I used the older configuration; 
so older git version - my bad).


Does these patches land up in the stable trees through maintainers or I 
should explicitly cc sta...@vger.kernel.org now?

--
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 RESEND(2) 2/2] usb: dwc3: gadget: Stop TRB preparation after limit is reached

2015-01-13 Thread Amit Virdi
DWC3 gadget sets up a pool of 32 TRBs for each EP during initialization. This
means, the max TRBs that can be submitted for an EP is fixed to 32. Since the
request queue for an EP is a linked list, any number of requests can be queued
to it by the gadget layer.  However, the dwc3 driver must not submit TRBs more
than the pool it has created for. This limit wasn't respected when SG was used
resulting in submitting more than the max TRBs, eventually leading to
non-transfer of the TRBs submitted over the max limit.

Root cause:
When SG is used, there are two loops iterating to prepare TRBs:
 - Outer loop over the request_list
 - Inner loop over the SG list
The code was missing break to get out of the outer loop.

Signed-off-by: Amit Virdi 
Cc:  # v3.9+
---
 drivers/usb/dwc3/gadget.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index cb8939134c32..6c5e344822b9 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -897,6 +897,9 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool 
starting)
if (last_one)
break;
}
+
+   if (last_one)
+   break;
} else {
dma = req->request.dma;
length = req->request.length;
-- 
1.8.0

--
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 RESEND(2) 0/2] usb: dwc3: gadget: Bug fixes

2015-01-13 Thread Amit Virdi
This is a re-submission of patches [1/4] and [2/4] from:
http://www.spinics.net/lists/linux-usb/msg118841.html

Commit log of both these patches has been modified for aided clarity. These
patches have been rebased on Balbi's testing/next.

Patches [3/4] and [4/4] were accepted as they were. 

Amit Virdi (2):
  usb: dwc3: gadget: Fix TRB preparation during SG
  usb: dwc3: gadget: Stop TRB preparation after limit is reached

 drivers/usb/dwc3/gadget.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
1.8.0

--
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 RESEND(2) 1/2] usb: dwc3: gadget: Fix TRB preparation during SG

2015-01-13 Thread Amit Virdi
When scatter gather (SG) is used, multiple TRBs are prepared from one DWC3
request (dwc3_request). So while preparing TRBs, the 'last' flag should be set
only when it is the last TRB being prepared from the last dwc3_request entry.

The current implementation uses list_is_last to check if the dwc3_request is the
last entry from the request_list. However, list_is_last returns false for the
last entry too. This is because, while preparing the first TRB from a request,
the function dwc3_prepare_one_trb modifies the request's next and prev pointers
while moving the URB to req_queued. Hence, list_is_last always returns false no
matter what.

The correct way is not to access the modified pointers of dwc3_request but to
use list_empty macro instead.

Fixes: e5ba5ec833aa4a76980b512d6a6779643516b850 ("usb: dwc3: gadget: fix scatter
gather implementation"

Signed-off-by: Amit Virdi 
Cc:  # v3.9+
---
 drivers/usb/dwc3/gadget.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 4e2593993fae..cb8939134c32 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -879,8 +879,7 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool 
starting)
 
if (i == (request->num_mapped_sgs - 1) ||
sg_is_last(s)) {
-   if (list_is_last(&req->list,
-   &dep->request_list))
+   if (list_empty(&dep->request_list))
last_one = true;
chain = false;
}
-- 
1.8.0

--
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 RESEND 0/2] usb: dwc3: gadget: Bug fixes

2015-01-13 Thread Amit Virdi

On 1/13/2015 12:10 PM, Amit VIRDI wrote:

This is a re-submission of patches [1/4] and [2/4] from:
http://www.spinics.net/lists/linux-usb/msg118841.html

Commit log of both these patches has been modified for aided clarity. These
patches have been rebased on Balbi's testing/next.

Patches [3/4] and [4/4] were accepted as they were.

Amit Virdi (2):
   usb: dwc3: gadget: Fix TRB preparation during SG
   usb: dwc3: gadget: Stop TRB preparation after limit is reached

  drivers/usb/dwc3/gadget.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)



I was using older git version, so git failed to send patches to 
sta...@vger.kernel.org


Please discard this patchset. I'm resending with updated git
--
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 RESEND 2/2] usb: dwc3: gadget: Stop TRB preparation after limit is reached

2015-01-12 Thread Amit Virdi
DWC3 gadget sets up a pool of 32 TRBs for each EP during initialization. This
means, the max TRBs that can be submitted for an EP is fixed to 32. Since the
request queue for an EP is a linked list, any number of requests can be queued
to it by the gadget layer.  However, the dwc3 driver must not submit TRBs more
than the pool it has created for. This limit wasn't respected when SG was used
resulting in submitting more than the max TRBs, eventually leading to
non-transfer of the TRBs submitted over the max limit.

Root cause:
When SG is used, there are two loops iterating to prepare TRBs:
 - Outer loop over the request_list
 - Inner loop over the SG list
The code was missing break to get out of the outer loop.

Signed-off-by: Amit Virdi 
Cc:  # v3.9+
---
 drivers/usb/dwc3/gadget.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index cb8939134c32..6c5e344822b9 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -897,6 +897,9 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool 
starting)
if (last_one)
break;
}
+
+   if (last_one)
+   break;
} else {
dma = req->request.dma;
length = req->request.length;
-- 
1.8.0

--
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 RESEND 0/2] usb: dwc3: gadget: Bug fixes

2015-01-12 Thread Amit Virdi
This is a re-submission of patches [1/4] and [2/4] from:
http://www.spinics.net/lists/linux-usb/msg118841.html

Commit log of both these patches has been modified for aided clarity. These
patches have been rebased on Balbi's testing/next.

Patches [3/4] and [4/4] were accepted as they were. 

Amit Virdi (2):
  usb: dwc3: gadget: Fix TRB preparation during SG
  usb: dwc3: gadget: Stop TRB preparation after limit is reached

 drivers/usb/dwc3/gadget.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
1.8.0

--
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 RESEND 1/2] usb: dwc3: gadget: Fix TRB preparation during SG

2015-01-12 Thread Amit Virdi
When scatter gather (SG) is used, multiple TRBs are prepared from one DWC3
request (dwc3_request). So while preparing TRBs, the 'last' flag should be set
only when it is the last TRB being prepared from the last dwc3_request entry.

The current implementation uses list_is_last to check if the dwc3_request is the
last entry from the request_list. However, list_is_last returns false for the
last entry too. This is because, while preparing the first TRB from a request,
the function dwc3_prepare_one_trb modifies the request's next and prev pointers
while moving the URB to req_queued. Hence, list_is_last always returns false no
matter what.

The correct way is not to access the modified pointers of dwc3_request but to
use list_empty macro instead.

Fixes: e5ba5ec833aa4a76980b512d6a6779643516b850 ("usb: dwc3: gadget: fix scatter
gather implementation"

Signed-off-by: Amit Virdi 
Cc:  # v3.9+
---
 drivers/usb/dwc3/gadget.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 4e2593993fae..cb8939134c32 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -879,8 +879,7 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool 
starting)
 
if (i == (request->num_mapped_sgs - 1) ||
sg_is_last(s)) {
-   if (list_is_last(&req->list,
-   &dep->request_list))
+   if (list_empty(&dep->request_list))
last_one = true;
chain = false;
}
-- 
1.8.0

--
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 2/4] usb: dwc3: gadget: Stop TRB preparation after limit is reached

2015-01-12 Thread Amit Virdi

On 1/13/2015 12:04 AM, Felipe Balbi wrote:

Hi,

On Tue, Jan 06, 2015 at 11:44:23AM +0530, Amit Virdi wrote:

I can certainly provide the dwc3 specific kernel bootup logs, full
regdump and any loglevel you want me to, if that helps


Yeah, if you can provide those, then that'll help me verifying. Full
logs from boot to failure point with VERBOSE_DEBUG enabled (considering
you're not running on anything recent).



Okay. I'll provide full verbose logs, along with the register dump,
when I'm back to the office next week, for the working and non-working
case, but how - as attachment or as inline?


Either way will do but I have a feeling mailing list attachment size
will bite you, so maybe it's best to make a tarball of both logs and
send that as attachment. Compressed text will be very small.



Attached are the dwc3 verbose logs and register dump without and with the
fixes in this patchset.


Sorry for the long delay, it has been a bit hectic.



It's okay!


I can see from your logs that we end up with a Transfer Not Ready
(Transfer Active) event and endpoint has BUSY flag set. I also see that
you're queueing 31 requests and all of them will use 2 TRBs, so we
clearly go over our TRB table.

This fix is, indeed, necessary but we need to improve commit log a bit
so it's extremely clear what the error is. Later, we can improve how we
handle requests in this driver, but that is outside of the scope of your
patchset.

Please improve commit log and resend your series so it can be applied.



Alright! I'll improve the commit messages and, also, cc stable list 
while resending the patches. As I see, you have already picked patches 
[3/4] and [4/4]. So, I'll resend only [1/4] and [2/4]



cheers



Thank you for your patience and kind understanding.
--
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 2/4] usb: dwc3: gadget: Stop TRB preparation after limit is reached

2015-01-05 Thread Amit Virdi

I can certainly provide the dwc3 specific kernel bootup logs, full
regdump and any loglevel you want me to, if that helps


Yeah, if you can provide those, then that'll help me verifying. Full
logs from boot to failure point with VERBOSE_DEBUG enabled (considering
you're not running on anything recent).



Okay. I'll provide full verbose logs, along with the register dump,
when I'm back to the office next week, for the working and non-working
case, but how - as attachment or as inline?


Either way will do but I have a feeling mailing list attachment size
will bite you, so maybe it's best to make a tarball of both logs and
send that as attachment. Compressed text will be very small.



Attached are the dwc3 verbose logs and register dump without and with 
the fixes in this patchset.


dwc3_fixes_logs.tar.gz
Description: GNU Zip compressed data


Re: [PATCH 2/4] usb: dwc3: gadget: Stop TRB preparation after limit is reached

2014-12-30 Thread Amit Virdi
>> The only reason why I have not been able to test these fixes on the
>> latest is because the customized webcam gadget is not ported on the
>> latest kernel. There have been a lot of changes in the video framework
>> lately and that is not my area of expertise. So porting the customized
>> webcam gadget to latest kernel and testing these fixes is not feasible
>> for me.
>
> right, so while I can see that both patches are very minimal and likely
> to be correct, I have no way of guaranteeing that. The only thing I can
> do, is run the tests which I already run (none of which exercise the
> cases you found though, other I'd have found them already) to make sure
> your patches don't break what's already working.
>

Okay

>> Having said all this, the fact remains that these are bugs in the dwc3
>> driver from kernel v3.9 onwards. The log messages clearly depict this.
>
> no, it doesn't :-) your log snippet is so small that I cannot see how
> the driver got to that point :-) The only thing I can see is that you
> started two requests, one for 96 bytes and another one for 2 bytes,
> but I don't know how many TRBs we had available, nor do I see a Start
> Transfer command, so I can't validate Start Transfer command parameters
> were correct or not.
>
>> These bugs are obviously present till date.
>
> Right, the code is the same :-)
>
>> I ask you to give a more deeper thought on the whole situation and not
>> undermine the importance of code review.
>
> heh
>
>>  - [Patch 2/4] fixes a bug that could have been found in the code review.
>>  - [Patch 1/4] fixes a bug which was very very subtle but a deeper
>> code review can certainly boost the confidence about the change made.
>> list_is_last won't work when SG is used because, for the last request
>> in the request_list, the TRB for the first sg entry modifies the
>> list's next and prev pointers while moving the URB to req_queued.
>>
>> I can certainly provide the dwc3 specific kernel bootup logs, full
>> regdump and any loglevel you want me to, if that helps
>
> Yeah, if you can provide those, then that'll help me verifying. Full
> logs from boot to failure point with VERBOSE_DEBUG enabled (considering
> you're not running on anything recent).
>

Okay. I'll provide full verbose logs, along with the register dump,
when I'm back to the office next week, for the working and non-working
case, but how - as attachment or as inline?
--
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 2/4] usb: dwc3: gadget: Stop TRB preparation after limit is reached

2014-12-28 Thread Amit Virdi
On Sat, Dec 27, 2014 at 11:16 PM, Felipe Balbi  wrote:
> Hi,
>
> On Sat, Dec 27, 2014 at 01:24:03PM +0530, Amit Virdi wrote:
>> On Mon, Dec 22, 2014 at 9:36 PM, Felipe Balbi  wrote:
>> > On Fri, Dec 19, 2014 at 12:40:16PM +0530, Amit Virdi wrote:
>> >> When SG is used, there are two loops iterating to prepare TRBs:
>> >>  - Outer loop over the request_list
>> >>  - Inner loop over the SG list
>> >>
>> >> The driver must stop preparing TRBs when the max TRBs have been prepared. 
>> >> The
>> >> code was missing break to get out of the outer loop.
>> >>
>> >> Signed-off-by: Amit Virdi 
>> >
>> > which bug is this fixing ? Which kernels are affected ? This need to be
>> > backported to which kernel ? Which commit introduced this bug ? How can
>> > I validate this to be a valid fix ?
>> >
>>
>> Problem description:
>> DWC3 gadget sets up a pool of 32 TRBs for each EP during
>> initialization. This means, the max TRBs that can be submitted for an
>> EP is fixed to 32. Since the request queue for an EP is a linked list,
>> any number of requests can be queued to it by the gadget layer.
>> However, the dwc3 driver must not submit TRBs more than the pool it
>> has created for. This limit wasn't respected when SG was used
>> resulting in submitting more than the max TRBs, eventually leading to
>> non-transfer of the TRBs submitted over the max limit.
>
> this would become a much, much better commit log than the one you
> provided. It's a much more verbose description of the issue.
>

Okay, I'll edit the commit message.

>> Commit that introduced this bug:
>> This bug is present from the day support for sg list was added to dwc3
>> gadget., i.e. since commit eeb720fb21d61dfc3aac780e721150998ef603af
>> ("usb: dwc3: gadget: add support for SG lists") - kernel version
>> v3.2-rc7, hence v3.2
>>
>> Kernels affected:
>> It is best to backport this fix to kernel v3.8+ as there were many
>> enhancements/bug fixes from v3.2..v3.8
>>
>> Generic setup to reproduce/test this fix:
>> This bug is reproduced whenever the number of TRBs that need to be
>> submitted to the hardware from the software queue (request_list)
>> becomes more than 32 on bulk endpoint. eg. if num_mapped_sgs is 2 and
>> the request_list has 17 entries (hence, 34 potential TRBs), the
>> scenario is reproduced.
>>
>> My setup details:
>> For reproducing and testing the patches [1/4 and 2/4], I used an
>> inhouse developed user space application that run on device side. This
>> user space application interacts with the customized uvc webcam gadget
>> to submit the video data to the DWC3 driver. The host side was running
>> standard webcam application (cheese).
>
> oh, so this is something I can't easily reproduce myself. Then I'll need
> full boot logs, register dump and full trace output of dwc3 running and
> exhibiting the problem. Also, make sure you use either v3.18 or v3.19rc1
> to validate your changes.
>

[Quoting you from the thread of patch 1/4]

>> endpoint. Following is the log snippet once this bug is reproduced:
>> 
>> dwc3 dwc3.0: ep2in-bulk: Transfer Not Ready
>> dwc3 dwc3.0: queing request 24cc5780 to ep2in-bulk length 960002
>> dwc3 dwc3.0: ep2in-bulk: req 24cc5780 dma 24eb6400 length 2 chain
>> dwc3 dwc3.0: ep2in-bulk: req 24cc5780 dma 25901800 length 96
>> dwc3 dwc3.0: queing request 24cc5000 to ep2in-bulk length 960002
>> dwc3 dwc3.0: ep2in-bulk: Transfer Not Ready
>> dwc3 dwc3.0: queing request 24cc5900 to ep2in-bulk length 960002
>> -
>>
>> Without this fix, the hardware never generates "Transfer Complete"
>> event for the corresponding EP and goes into an unknown state.
>
> whiuch kernel are you using to develop this ? Most of these messages
> don't exist anymore with v3.19-rc because I have converted them into
> tracepoints, considering you're showing me logs with these messages,
> this tells me that you're sending me a patch developed/tested against a
> kernel older than v3.18. I need to see full bootup logs, a register dump
> (/sys/kernel/debug/*dwc3*/regdump) and a much larger debugging log from
> dwc3 in order to accept patches from you. Sorry, but I cannot take
> patches which were not tested against, at a minimum, the latest major
> release.
>

Yes you're right that both the fixes [1/4] and [2/4] have only been
build tested with the latest kernel. I should have mentioned this
information in the commit message or the cover-letter itself.

The only r

Re: [PATCH 1/4] usb: dwc3: gadget: Fix TRB preparation during SG

2014-12-28 Thread Amit Virdi
On Sat, Dec 27, 2014 at 11:14 PM, Felipe Balbi  wrote:
> Hi,
>
> On Sat, Dec 27, 2014 at 12:39:23PM +0530, Amit Virdi wrote:
>> On Mon, Dec 22, 2014 at 9:34 PM, Felipe Balbi  wrote:
>> > On Fri, Dec 19, 2014 at 12:40:15PM +0530, Amit Virdi wrote:
>> >> When scatter gather is used, multiple TRBs are prepared from one DWC3 
>> >> request.
>> >> Hence, we must set the 'last' flag when the SG is last as well as the TRB 
>> >> is
>> >> last. The current implementation uses list_is_last to check if the 
>> >> dwc3_request
>> >> is the last request in the request_list.
>> >>
>> >> This doesn't work when SG is used. This is because, when it is the last 
>> >> request,
>> >> the first TRB preparation (in dwc3_prepare_one_trb) modifies the 
>> >> dwc3_request
>> >> list's next and prev pointers while moving the URB to req_queued.
>> >>
>> >> Hence, list_is_last always return false no matter what. The correct way 
>> >> is not
>> >> to access the modified pointers of dwc3_request but to use list_empty 
>> >> macro
>> >> instead.
>> >>
>> >> Fixes: e5ba5ec833aa4a76980b512d6a6779643516b850 ("usb: dwc3: gadget: fix 
>> >> scatter
>> >> gather implementation"
>> >>
>> >> Signed-off-by: Amit Virdi 
>> >
>> > you need to Cc stable here and make sure you point out which kernel
>> > versions this should be backported to. Looks like this sould be:
>> >
>> > Cc:  # v3.9+
>>
>> Okay. I checked the git log. The commit ("usb: dwc3: gadget: fix
>> scatter gather implementation") was introduced in v3.8-rc5, hence
>> v3.8, so I need to
>>
>> Cc:  # v3.8+
>
> hehe, many folks get confused by this. New features will never get
> merged upstream during the -rc cycle. -rc5 is when I applied it to my
> tree so it could be merged on the following merge window, which was
> v3.9.
>

Got it, thanks!

>> > Also, how have you tested this ? I need a test case to make sure it
>> > fails here and this patch really fixes the problem.
>> >
>>
>> This bug can be easily reproduced/tested if the gadget driver submits
>> a urb having number of sg entries mapped to DMA more than 1 on bulk
>
> which gadget driver does this ?
>

I discovered the bug fixed in this patch and the other [2/4], when I
was using our customized uvc webcam gadget.

>> endpoint. Following is the log snippet once this bug is reproduced:
>> 
>> dwc3 dwc3.0: ep2in-bulk: Transfer Not Ready
>> dwc3 dwc3.0: queing request 24cc5780 to ep2in-bulk length 960002
>> dwc3 dwc3.0: ep2in-bulk: req 24cc5780 dma 24eb6400 length 2 chain
>> dwc3 dwc3.0: ep2in-bulk: req 24cc5780 dma 25901800 length 96
>> dwc3 dwc3.0: queing request 24cc5000 to ep2in-bulk length 960002
>> dwc3 dwc3.0: ep2in-bulk: Transfer Not Ready
>> dwc3 dwc3.0: queing request 24cc5900 to ep2in-bulk length 960002
>> -
>>
>> Without this fix, the hardware never generates "Transfer Complete"
>> event for the corresponding EP and goes into an unknown state.
>
> whiuch kernel are you using to develop this ? Most of these messages
> don't exist anymore with v3.19-rc because I have converted them into
> tracepoints, considering you're showing me logs with these messages,
> this tells me that you're sending me a patch developed/tested against a
> kernel older than v3.18. I need to see full bootup logs, a register dump
> (/sys/kernel/debug/*dwc3*/regdump) and a much larger debugging log from
> dwc3 in order to accept patches from you. Sorry, but I cannot take
> patches which were not tested against, at a minimum, the latest major
> release.
>
> It would be much better if you developed/tested against v3.19-rc1,
> considering we have 40 different patches which were merged since v3.18
> was tagged.
>

Your comments for this patch and the other are similar. So, I'm
quoting your comments from here to the other thread (of patch 2/4) and
replying there to avoid any duplicacy.
--
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 2/4] usb: dwc3: gadget: Stop TRB preparation after limit is reached

2014-12-26 Thread Amit Virdi
On Mon, Dec 22, 2014 at 9:36 PM, Felipe Balbi  wrote:
> On Fri, Dec 19, 2014 at 12:40:16PM +0530, Amit Virdi wrote:
>> When SG is used, there are two loops iterating to prepare TRBs:
>>  - Outer loop over the request_list
>>  - Inner loop over the SG list
>>
>> The driver must stop preparing TRBs when the max TRBs have been prepared. The
>> code was missing break to get out of the outer loop.
>>
>> Signed-off-by: Amit Virdi 
>
> which bug is this fixing ? Which kernels are affected ? This need to be
> backported to which kernel ? Which commit introduced this bug ? How can
> I validate this to be a valid fix ?
>

Problem description:
DWC3 gadget sets up a pool of 32 TRBs for each EP during
initialization. This means, the max TRBs that can be submitted for an
EP is fixed to 32. Since the request queue for an EP is a linked list,
any number of requests can be queued to it by the gadget layer.
However, the dwc3 driver must not submit TRBs more than the pool it
has created for. This limit wasn't respected when SG was used
resulting in submitting more than the max TRBs, eventually leading to
non-transfer of the TRBs submitted over the max limit.

Commit that introduced this bug:
This bug is present from the day support for sg list was added to dwc3
gadget., i.e. since commit eeb720fb21d61dfc3aac780e721150998ef603af
("usb: dwc3: gadget: add support for SG lists") - kernel version
v3.2-rc7, hence v3.2

Kernels affected:
It is best to backport this fix to kernel v3.8+ as there were many
enhancements/bug fixes from v3.2..v3.8

Generic setup to reproduce/test this fix:
This bug is reproduced whenever the number of TRBs that need to be
submitted to the hardware from the software queue (request_list)
becomes more than 32 on bulk endpoint. eg. if num_mapped_sgs is 2 and
the request_list has 17 entries (hence, 34 potential TRBs), the
scenario is reproduced.

My setup details:
For reproducing and testing the patches [1/4 and 2/4], I used an
inhouse developed user space application that run on device side. This
user space application interacts with the customized uvc webcam gadget
to submit the video data to the DWC3 driver. The host side was running
standard webcam application (cheese).
--
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 1/4] usb: dwc3: gadget: Fix TRB preparation during SG

2014-12-26 Thread Amit Virdi
On Mon, Dec 22, 2014 at 9:34 PM, Felipe Balbi  wrote:
> On Fri, Dec 19, 2014 at 12:40:15PM +0530, Amit Virdi wrote:
>> When scatter gather is used, multiple TRBs are prepared from one DWC3 
>> request.
>> Hence, we must set the 'last' flag when the SG is last as well as the TRB is
>> last. The current implementation uses list_is_last to check if the 
>> dwc3_request
>> is the last request in the request_list.
>>
>> This doesn't work when SG is used. This is because, when it is the last 
>> request,
>> the first TRB preparation (in dwc3_prepare_one_trb) modifies the dwc3_request
>> list's next and prev pointers while moving the URB to req_queued.
>>
>> Hence, list_is_last always return false no matter what. The correct way is 
>> not
>> to access the modified pointers of dwc3_request but to use list_empty macro
>> instead.
>>
>> Fixes: e5ba5ec833aa4a76980b512d6a6779643516b850 ("usb: dwc3: gadget: fix 
>> scatter
>> gather implementation"
>>
>> Signed-off-by: Amit Virdi 
>
> you need to Cc stable here and make sure you point out which kernel
> versions this should be backported to. Looks like this sould be:
>
> Cc:  # v3.9+

Okay. I checked the git log. The commit ("usb: dwc3: gadget: fix
scatter gather implementation") was introduced in v3.8-rc5, hence
v3.8, so I need to

Cc:  # v3.8+

>
> Also, how have you tested this ? I need a test case to make sure it
> fails here and this patch really fixes the problem.
>

This bug can be easily reproduced/tested if the gadget driver submits
a urb having number of sg entries mapped to DMA more than 1 on bulk
endpoint. Following is the log snippet once this bug is reproduced:

dwc3 dwc3.0: ep2in-bulk: Transfer Not Ready
dwc3 dwc3.0: queing request 24cc5780 to ep2in-bulk length 960002
dwc3 dwc3.0: ep2in-bulk: req 24cc5780 dma 24eb6400 length 2 chain
dwc3 dwc3.0: ep2in-bulk: req 24cc5780 dma 25901800 length 96
dwc3 dwc3.0: queing request 24cc5000 to ep2in-bulk length 960002
dwc3 dwc3.0: ep2in-bulk: Transfer Not Ready
dwc3 dwc3.0: queing request 24cc5900 to ep2in-bulk length 960002
-

Without this fix, the hardware never generates "Transfer Complete"
event for the corresponding EP and goes into an unknown state.
--
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 4/4] usb: dwc3: Remove current_trb as it is unused

2014-12-18 Thread Amit Virdi
This field was introduced but never used. So, remove it.

Signed-off-by: Amit Virdi 
---
 drivers/usb/dwc3/core.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 4bb9aa696ede..0842aa80976f 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -431,7 +431,6 @@ struct dwc3_event_buffer {
  * @dwc: pointer to DWC controller
  * @saved_state: ep state saved during hibernation
  * @flags: endpoint flags (wedged, stalled, ...)
- * @current_trb: index of current used trb
  * @number: endpoint number (1 - 15)
  * @type: set to bmAttributes & USB_ENDPOINT_XFERTYPE_MASK
  * @resource_index: Resource transfer index
@@ -464,8 +463,6 @@ struct dwc3_ep {
/* This last one is specific to EP0 */
 #define DWC3_EP0_DIR_IN(1 << 31)
 
-   unsignedcurrent_trb;
-
u8  number;
u8  type;
u8  resource_index;
-- 
1.8.0

--
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 3/4] usb: dwc3: gadget: Remove redundant check

2014-12-18 Thread Amit Virdi
dwc3_gadget_init_hw_endpoints calls dwc3_alloc_trb_pool only if epnum is not
equal to 0 or 1. Hence, rechecking it in the called function is redundant.

Signed-off-by: Amit Virdi 
---
 drivers/usb/dwc3/gadget.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 8f65ab3a3b92..8c2e2fbe6f39 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -352,9 +352,6 @@ static int dwc3_alloc_trb_pool(struct dwc3_ep *dep)
if (dep->trb_pool)
return 0;
 
-   if (dep->number == 0 || dep->number == 1)
-   return 0;
-
dep->trb_pool = dma_alloc_coherent(dwc->dev,
sizeof(struct dwc3_trb) * DWC3_TRB_NUM,
&dep->trb_pool_dma, GFP_KERNEL);
-- 
1.8.0

--
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 2/4] usb: dwc3: gadget: Stop TRB preparation after limit is reached

2014-12-18 Thread Amit Virdi
When SG is used, there are two loops iterating to prepare TRBs:
 - Outer loop over the request_list
 - Inner loop over the SG list

The driver must stop preparing TRBs when the max TRBs have been prepared. The
code was missing break to get out of the outer loop.

Signed-off-by: Amit Virdi 
---
 drivers/usb/dwc3/gadget.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 0eec2e917994..8f65ab3a3b92 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -900,6 +900,9 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool 
starting)
if (last_one)
break;
}
+
+   if (last_one)
+   break;
} else {
dma = req->request.dma;
length = req->request.length;
-- 
1.8.0

--
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 0/4] usb: dwc3: Fixes and code cleanup

2014-12-18 Thread Amit Virdi
The first two patches are bug fixes in TRB preparation when scatter gather is
used. The next two patches are basically trivial and part of code cleanup.

The patches are rebased on Balbi's testing/next branch.

Amit Virdi (4):
  usb: dwc3: gadget: Fix TRB preparation during SG
  usb: dwc3: gadget: Stop TRB preparation after limit is reached
  usb: dwc3: gadget: Remove redundant check
  usb: dwc3: Remove current_trb as it is unused

 drivers/usb/dwc3/core.h   | 3 ---
 drivers/usb/dwc3/gadget.c | 9 -
 2 files changed, 4 insertions(+), 8 deletions(-)

-- 
1.8.0

--
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 1/4] usb: dwc3: gadget: Fix TRB preparation during SG

2014-12-18 Thread Amit Virdi
When scatter gather is used, multiple TRBs are prepared from one DWC3 request.
Hence, we must set the 'last' flag when the SG is last as well as the TRB is
last. The current implementation uses list_is_last to check if the dwc3_request
is the last request in the request_list.

This doesn't work when SG is used. This is because, when it is the last request,
the first TRB preparation (in dwc3_prepare_one_trb) modifies the dwc3_request
list's next and prev pointers while moving the URB to req_queued.

Hence, list_is_last always return false no matter what. The correct way is not
to access the modified pointers of dwc3_request but to use list_empty macro
instead.

Fixes: e5ba5ec833aa4a76980b512d6a6779643516b850 ("usb: dwc3: gadget: fix scatter
gather implementation"

Signed-off-by: Amit Virdi 
---
 drivers/usb/dwc3/gadget.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index f03b136ecfce..0eec2e917994 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -882,8 +882,7 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool 
starting)
 
if (i == (request->num_mapped_sgs - 1) ||
sg_is_last(s)) {
-   if (list_is_last(&req->list,
-   &dep->request_list))
+   if (list_empty(&dep->request_list))
last_one = true;
chain = false;
}
-- 
1.8.0

--
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] usb: gadget: zero: fix INT endpoint assignment

2014-12-03 Thread Amit Virdi
int_maxpacket > 64 ?
+64 : int_maxpacket;
+fs_int_sink_desc.bInterval = int_interval > 255 ?
+255 : int_interval;
+
   /* support high speed hardware */
   hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
   hs_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;



Things might be working for you but this is not the correct fix, IMO.


Do you have something better?



Can't suggest anything right now. :(


Looking into the patch I feel it shall introduce other regressions.


For instance?



As you yourself correctly figured out in an another email, this shall 
break for all the FS devices wishing to use interrupt EP, since max 
would be 1024 as a result of this patch.





Did you try inserting g_zero with module parameters for musb?

If I force MPP to 64 (as Paul suggested more or less) then it works.
But this means I know about the upcoming problem.



What's the upcoming problem here? Do you mean the code isn't generic 
enough in case you have to use module_params?


Regards
Amit Virdi
--
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] usb: gadget: zero: fix INT endpoint assignment

2014-11-26 Thread Amit Virdi

Dear Sebastian,

On 11/25/2014 10:56 PM, Sebastian Andrzej Siewior wrote:

This fixes the test case mentioned for musb which is a regression.
Other things that I noticed:

- if ISOC is not available, we won't have INT as well.


I didn't understand this. The original patch added a new alternate 
setting (2) that has two interrupt endpoints. ISOC EP is available 
through alternate setting 1.



- wMaxPacketSize is supposed to be LE. The assignments within the code
   does not use cpu_to_le16().


Can you please point to the code where it should have been and is missing?


- the module Parameter for INT says max packet size is 0…1023 for FS.


Yes, I'll send a patch to rectify this.


   This is clearly a copy/paste bug.

Amit could you please look at this and fix it?

Sebastian



Regards
Amit Virdi
--
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] usb: gadget: zero: fix INT endpoint assignment

2014-11-26 Thread Amit Virdi
 is not the correct fix, IMO. 
Looking into the patch I feel it shall introduce other regressions.


Did you try inserting g_zero with module parameters for musb?

Regards
Amit Virdi
--
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


XHCI host dies with LPM + webcam gadget

2014-09-25 Thread Amit Virdi

Hello Mathias,

I'm trying to connect a webcam gadget on USB3.0 interface with xHCI host 
when LPM is enabled. My xHCI host is TI's TUSB7340EVM. xHCI host stops 
responding as soon as the webcam gadget module is inserted on the device 
side (The device is also running linux)


Host machine kernel version is 3.16. I have enabled quirk for LPM enable 
for my XHCI card.


The detailed description and a host kernel logs can be found from bugzila:
https://bugzilla.kernel.org/show_bug.cgi?id=85141

Could you please help in rectifying this bug? Please let me know if 
there's any more information required.


Thanks
Amit Virdi
--
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: usb: gadget: zero: Add support for interrupt EP

2014-09-10 Thread Amit Virdi

Hello Dan,

On 9/9/2014 2:32 PM, Dan Carpenter wrote:

Hello Amit Virdi,

The patch ef11982dd7a6: "usb: gadget: zero: Add support for interrupt
EP" from Aug 22, 2014, leads to the following static checker warning:

drivers/usb/gadget/function/f_sourcesink.c:1498 
f_ss_opts_int_interval_store()
warn: impossible condition '(num > 4096) => (0-255 > 4096)'

drivers/usb/gadget/function/f_sourcesink.c
   1482  static ssize_t f_ss_opts_int_interval_store(struct f_ss_opts *opts,
   1483 const char *page, size_t len)
   1484  {
   1485  int ret;
   1486  u8 num;
   1487
   1488  mutex_lock(&opts->lock);
   1489  if (opts->refcnt) {
   1490  ret = -EBUSY;
   1491  goto end;
   1492  }
   1493
   1494  ret = kstrtou8(page, 0, &num);
   1495  if (ret)
   1496  goto end;
   1497
   1498  if (num > 4096) {

This limit doesn't make sense because num is an 8 bit number.

   1499  ret = -EINVAL;
   1500  goto end;
   1501  }
   1502
   1503  opts->int_interval = num;

opts->int_interval is 32 bit so probably num should be declared as a u32
as well.



Yes, you're right. Infact, I submitted a patch [1] yesterday itself 
doing the change which you're suggesting here.


Regards
Amit Virdi

[1] 
https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/next&id=70aacc5777d1f1ca0a88067c9121ce86441bc4e0

--
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 V2] usb: gadget: zero: Fix warning generated by kbuild

2014-09-08 Thread Amit Virdi
The kbuild test bot generated the warning:
drivers/usb/gadget/function/f_sourcesink.c:1498: warning: comparison is
always false due to limited range of data type

This patch fixes it.

Reported-by: kbuild test robot 
Signed-off-by: Amit Virdi 
CC: Felipe Balbi 
---
 drivers/usb/gadget/function/f_sourcesink.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
b/drivers/usb/gadget/function/f_sourcesink.c
index 7c091a328228..80be25b32cd7 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -1483,7 +1483,7 @@ static ssize_t f_ss_opts_int_interval_store(struct 
f_ss_opts *opts,
   const char *page, size_t len)
 {
int ret;
-   u8 num;
+   u32 num;
 
mutex_lock(&opts->lock);
if (opts->refcnt) {
@@ -1491,7 +1491,7 @@ static ssize_t f_ss_opts_int_interval_store(struct 
f_ss_opts *opts,
goto end;
}
 
-   ret = kstrtou8(page, 0, &num);
+   ret = kstrtou32(page, 0, &num);
if (ret)
goto end;
 
-- 
1.8.0

--
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] usb: gadget: zero: Fix warning generated by kbuild

2014-09-08 Thread Amit Virdi

On 9/8/2014 7:18 PM, Felipe Balbi wrote:

Hi,

On Mon, Sep 08, 2014 at 04:05:33PM +0530, Amit Virdi wrote:

The kbuild test bot generated the warning:
drivers/usb/gadget/function/f_sourcesink.c:1498: warning: comparison is
always false due to limited range of data type

This patch fixes it.

Reported-by: kbuild test robot 
Signed-off-by: Amit Virdi 
CC: Felipe Balbi 


did you *really* build test this patch ?



Yes, I did. This seems to be an issue with my compiler, otherwise I 
wouldn't have required to send this patch altogether.


I have build tested the patch on the following two compilers, yet I 
didn't receive any warning:
   1. armv7-linux-gcc (GCC) 4.6.2 20110813 (STMicroelectronics/Linux 
Base 4.6.2-99)

   2. gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2

Anyways, sorry for the inconvenience. I'll be more careful from now 
onwards. I'm sending V2, build tested using the above two compilers, I 
hope the code is warning free now.


Regards
Amit Virdi
--
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] usb: gadget: zero: Fix warning generated by kbuild

2014-09-08 Thread Amit Virdi
The kbuild test bot generated the warning:
drivers/usb/gadget/function/f_sourcesink.c:1498: warning: comparison is
always false due to limited range of data type

This patch fixes it.

Reported-by: kbuild test robot 
Signed-off-by: Amit Virdi 
CC: Felipe Balbi 
---
 drivers/usb/gadget/function/f_sourcesink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
b/drivers/usb/gadget/function/f_sourcesink.c
index 7c091a328228..065e9d4e4785 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -1483,7 +1483,7 @@ static ssize_t f_ss_opts_int_interval_store(struct 
f_ss_opts *opts,
   const char *page, size_t len)
 {
int ret;
-   u8 num;
+   u32 num;
 
mutex_lock(&opts->lock);
if (opts->refcnt) {
-- 
1.8.0

--
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 V5 2/2] usbtest: Add interrupt EP testcases

2014-08-28 Thread Amit Virdi

Felipe,

I just checked your testing/next branch. 'git log' of that branch 
doesn't show the complete commit log of this patch.


On 8/22/2014 2:36 PM, Amit VIRDI wrote:

Two simple test cases for interrupt endpoints are added to the usbtest.c file.
These are simple non-queued interrupt IN and interrupt OUT transfers. Currently,
only gadget zero is capable of executing the interrupt EP test cases. However,
extending the same to other gadgets is extremely simple and can be done
on-demand.

The two new tests added are
   - Test 25: To verify Interrupt OUT transfer
   - Test 26: To verify Interrupt IN transfer

Since the default value of wMaxPacketSize is set as 1024, so interrupt
IN transfers must be specified with the size parameter = multiple of
1024. Otherwise the default value (512) in the usbtest application fails
the transfer. See [RUN 4] for sample logs

The application logs (usbtest) and corresponding kernel logs are as
following:


It only shows up to here. Rest of the commit log is missing.


---


I guess this is the culprit...


[Run 1]
./testusb -a -c 10 -s 2048 -t 26 -v 511
Jul 17 10:31:13 dlhl1014 kernel: [72056.950910] usbtest 7-1:3.0: TEST
26: read 2048 bytes 10 times

[Run 2]
./testusb -a -c 10 -s 1024 -t 25 -v 511
Jul 17 10:31:29 dlhl1014 kernel: [72072.834853] usbtest 7-1:3.0: TEST
25: write 1024 bytes 10 times

[Run 3]
./testusb -a -c 10 -s 1098 -t 25 -v 511
Jul 17 10:31:39 dlhl1014 kernel: [72082.322219] usbtest 7-1:3.0: TEST
25: write 1098 bytes 10 times

[Run 4 - Failure case scenario]
./testusb -a  -t 26
unknown speed   /dev/bus/usb/007/0040
/dev/bus/usb/007/004 test 26 --> 75 (Value too large for defined data
type)

Jul 17 11:11:30 dlhl1014 kernel: [74473.347219] usbtest 7-1:3.0: TEST
26: read 512 bytes 1000 times
Jul 17 11:11:30 dlhl1014 kernel: [74473.348959] usb 7-1: test26 failed,
iterations left 999, status -75 (not 0)
---

This code has been tested only with gadget zero and care has been taken so as to
not break the existing functionality. However, if anyone can test with other
gadgets then that would be great!

Signed-off-by: Amit Virdi 
---
  drivers/usb/misc/usbtest.c | 113 +++--
  1 file changed, 98 insertions(+), 15 deletions(-)



Please let me know if there's anything I can help with.

Regards
Amit Virdi
--
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 V5 1/2] usb: gadget: zero: Add support for interrupt EP

2014-08-22 Thread Amit Virdi
Interrupt endpoints behave quite similar to the bulk endpoints with the
difference that the endpoints expect data sending/reception request at
particular intervals till the whole data has not been transmitted.

The interrupt EP support is added to gadget zero. A new alternate setting (=2)
has been added. It has 2 interrupt endpoints. The default parameters are set as:
bInterval: 1 ms for FS and 8 uFrames (implying 1 ms) for HS/SS
wMaxPacketSize: 64 bytes for FS and 1024 bytes for HS/SS
However, the same can be overridden through the module parameter interface.

The code is tested for HS and SS on a platform having DWC3 controller.

Signed-off-by: Amit Virdi 
---
 drivers/usb/gadget/function/f_loopback.c   |   3 +-
 drivers/usb/gadget/function/f_sourcesink.c | 511 +++--
 drivers/usb/gadget/function/g_zero.h   |  13 +-
 drivers/usb/gadget/legacy/zero.c   |  21 ++
 4 files changed, 526 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/gadget/function/f_loopback.c 
b/drivers/usb/gadget/function/f_loopback.c
index 4557cd03f0b1..bf04389137e6 100644
--- a/drivers/usb/gadget/function/f_loopback.c
+++ b/drivers/usb/gadget/function/f_loopback.c
@@ -298,7 +298,8 @@ static void disable_loopback(struct f_loopback *loop)
struct usb_composite_dev*cdev;
 
cdev = loop->function.config->cdev;
-   disable_endpoints(cdev, loop->in_ep, loop->out_ep, NULL, NULL);
+   disable_endpoints(cdev, loop->in_ep, loop->out_ep, NULL, NULL, NULL,
+   NULL);
VDBG(cdev, "%s disabled\n", loop->function.name);
 }
 
diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
b/drivers/usb/gadget/function/f_sourcesink.c
index d3cd52db78fe..7c091a328228 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -23,6 +23,15 @@
 #include "gadget_chips.h"
 #include "u_f.h"
 
+#define USB_MS_TO_SS_INTERVAL(x) USB_MS_TO_HS_INTERVAL(x)
+
+enum eptype {
+   EP_CONTROL = 0,
+   EP_BULK,
+   EP_ISOC,
+   EP_INTERRUPT,
+};
+
 /*
  * SOURCE/SINK FUNCTION ... a primary testing vehicle for USB peripheral
  * controller drivers.
@@ -55,6 +64,8 @@ struct f_sourcesink {
struct usb_ep   *out_ep;
struct usb_ep   *iso_in_ep;
struct usb_ep   *iso_out_ep;
+   struct usb_ep   *int_in_ep;
+   struct usb_ep   *int_out_ep;
int cur_alt;
 };
 
@@ -68,6 +79,10 @@ static unsigned isoc_interval;
 static unsigned isoc_maxpacket;
 static unsigned isoc_mult;
 static unsigned isoc_maxburst;
+static unsigned int_interval; /* In ms */
+static unsigned int_maxpacket;
+static unsigned int_mult;
+static unsigned int_maxburst;
 static unsigned buflen;
 
 /*-*/
@@ -92,6 +107,16 @@ static struct usb_interface_descriptor 
source_sink_intf_alt1 = {
/* .iInterface  = DYNAMIC */
 };
 
+static struct usb_interface_descriptor source_sink_intf_alt2 = {
+   .bLength =  USB_DT_INTERFACE_SIZE,
+   .bDescriptorType =  USB_DT_INTERFACE,
+
+   .bAlternateSetting =2,
+   .bNumEndpoints =2,
+   .bInterfaceClass =  USB_CLASS_VENDOR_SPEC,
+   /* .iInterface  = DYNAMIC */
+};
+
 /* full speed support: */
 
 static struct usb_endpoint_descriptor fs_source_desc = {
@@ -130,6 +155,26 @@ static struct usb_endpoint_descriptor fs_iso_sink_desc = {
.bInterval =4,
 };
 
+static struct usb_endpoint_descriptor fs_int_source_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_IN,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(64),
+   .bInterval =GZERO_INT_INTERVAL,
+};
+
+static struct usb_endpoint_descriptor fs_int_sink_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_OUT,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(64),
+   .bInterval =GZERO_INT_INTERVAL,
+};
+
 static struct usb_descriptor_header *fs_source_sink_descs[] = {
(struct usb_descriptor_header *) &source_sink_intf_alt0,
(struct usb_descriptor_header *) &fs_sink_desc,
@@ -140,6 +185,10 @@ static struct usb_descriptor_header 
*fs_source_sink_descs[] = {
(struct usb_descriptor_header *) &fs_source_desc,
(struct usb_descriptor_header *) &fs_iso_sink_desc,
(struct usb_descriptor_header *) &fs_iso_source_desc,
+   (struct usb_descriptor_header *) &source_sink_intf_alt2,
+#define FS_ALT_IFC_2_OFFSET8
+   (struct usb_descriptor_header *) &

[PATCH V5 2/2] usbtest: Add interrupt EP testcases

2014-08-22 Thread Amit Virdi
Two simple test cases for interrupt endpoints are added to the usbtest.c file.
These are simple non-queued interrupt IN and interrupt OUT transfers. Currently,
only gadget zero is capable of executing the interrupt EP test cases. However,
extending the same to other gadgets is extremely simple and can be done
on-demand.

The two new tests added are
  - Test 25: To verify Interrupt OUT transfer
  - Test 26: To verify Interrupt IN transfer

Since the default value of wMaxPacketSize is set as 1024, so interrupt
IN transfers must be specified with the size parameter = multiple of
1024. Otherwise the default value (512) in the usbtest application fails
the transfer. See [RUN 4] for sample logs

The application logs (usbtest) and corresponding kernel logs are as
following:
---
[Run 1]
./testusb -a -c 10 -s 2048 -t 26 -v 511
Jul 17 10:31:13 dlhl1014 kernel: [72056.950910] usbtest 7-1:3.0: TEST
26: read 2048 bytes 10 times

[Run 2]
./testusb -a -c 10 -s 1024 -t 25 -v 511
Jul 17 10:31:29 dlhl1014 kernel: [72072.834853] usbtest 7-1:3.0: TEST
25: write 1024 bytes 10 times

[Run 3]
./testusb -a -c 10 -s 1098 -t 25 -v 511
Jul 17 10:31:39 dlhl1014 kernel: [72082.322219] usbtest 7-1:3.0: TEST
25: write 1098 bytes 10 times

[Run 4 - Failure case scenario]
./testusb -a  -t 26
unknown speed   /dev/bus/usb/007/0040
/dev/bus/usb/007/004 test 26 --> 75 (Value too large for defined data
type)

Jul 17 11:11:30 dlhl1014 kernel: [74473.347219] usbtest 7-1:3.0: TEST
26: read 512 bytes 1000 times
Jul 17 11:11:30 dlhl1014 kernel: [74473.348959] usb 7-1: test26 failed,
iterations left 999, status -75 (not 0)
---

This code has been tested only with gadget zero and care has been taken so as to
not break the existing functionality. However, if anyone can test with other
gadgets then that would be great!

Signed-off-by: Amit Virdi 
---
 drivers/usb/misc/usbtest.c | 113 +++--
 1 file changed, 98 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 829f446064ea..90e6644dc886 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -54,6 +54,7 @@ struct usbtest_info {
unsignedautoconf:1;
unsignedctrl_out:1;
unsignediso:1;  /* try iso in/out */
+   unsignedintr:1; /* try interrupt in/out */
int alt;
 };
 
@@ -70,7 +71,10 @@ struct usbtest_dev {
int out_pipe;
int in_iso_pipe;
int out_iso_pipe;
+   int in_int_pipe;
+   int out_int_pipe;
struct usb_endpoint_descriptor  *iso_in, *iso_out;
+   struct usb_endpoint_descriptor  *int_in, *int_out;
struct mutexlock;
 
 #define TBUF_SIZE  256
@@ -101,6 +105,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
struct usb_host_interface   *alt;
struct usb_host_endpoint*in, *out;
struct usb_host_endpoint*iso_in, *iso_out;
+   struct usb_host_endpoint*int_in, *int_out;
struct usb_device   *udev;
 
for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
@@ -108,6 +113,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
 
in = out = NULL;
iso_in = iso_out = NULL;
+   int_in = int_out = NULL;
alt = intf->altsetting + tmp;
 
if (override_alt >= 0 &&
@@ -124,6 +130,9 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
switch (usb_endpoint_type(&e->desc)) {
case USB_ENDPOINT_XFER_BULK:
break;
+   case USB_ENDPOINT_XFER_INT:
+   if (dev->info->intr)
+   goto try_intr;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
goto try_iso;
@@ -139,6 +148,15 @@ get_endpoints(struct usbtest_dev *dev, struct 
usb_interface *intf)
out = e;
}
continue;
+try_intr:
+   if (usb_endpoint_dir_in(&e->desc)) {
+   if (!int_in)
+   int_in = e;
+   } else {
+   if (!int_out)
+   int_out = e;
+   }
+   continue;
 try_iso:
   

[PATCH V5 0/2] usb: gadget: zero: Add support for interrupt EP

2014-08-22 Thread Amit Virdi
This patchset adds support for interrupt EP and the corresponding test cases to
gadget zero. The code has been rebased and tested on Kernel v3.15-rc5

V4 -> V5
 - Rebased on Felipe Balbi's testing/next
 - Build tested
 - No other change

V3 -> V4
 - Edited the commit message to provide more information regarding the new test
   cases added
 - Rebased the code in Kernel v3.16-rc5
 - No change in the code

V2 -> V3
 - Rectified wMaxPacketSize for FS from 1023 to 64
 - Modified default value of interrupt interval for FS to 1 ms

V1 -> V2
 - Modified the alternate interface from having 6 EPs (2 - Interrupt, 2 Bulk, 2
   Isoc) to 2 EPs (Interrupt only)

RFC -> V1
 - Added support for configuring interrupt EP attributes from configfs interface

Amit Virdi (2):
  usb: gadget: zero: Add support for interrupt EP
  usbtest: Add interrupt EP testcases

 drivers/usb/gadget/function/f_loopback.c   |   3 +-
 drivers/usb/gadget/function/f_sourcesink.c | 511 +++--
 drivers/usb/gadget/function/g_zero.h   |  13 +-
 drivers/usb/gadget/legacy/zero.c   |  21 ++
 drivers/usb/misc/usbtest.c | 113 ++-
 5 files changed, 624 insertions(+), 37 deletions(-)

-- 
1.8.0

--
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 V4 0/2] usb: gadget: zero: Add support for interrupt EP

2014-07-20 Thread Amit Virdi
This patchset adds support for interrupt EP and the corresponding test cases to
gadget zero. The code has been rebased and tested on Kernel v3.15-rc5

V3->V4
 - Edited the commit message to provide more information regarding the new test
   cases added
 - Rebased the code in Kernel v3.16-rc5
 - No change in the code

V2 -> V3
 - Rectified wMaxPacketSize for FS from 1023 to 64
 - Modified default value of interrupt interval for FS to 1 ms

V1 -> V2
 - Modified the alternate interface from having 6 EPs (2 - Interrupt, 2 Bulk, 2
   Isoc) to 2 EPs (Interrupt only)

RFC -> V1
 - Added support for configuring interrupt EP attributes from configfs interface

Amit Virdi (2):
  usb: gadget: zero: Add support for interrupt EP
  usbtest: Add interrupt EP testcases

 drivers/usb/gadget/f_loopback.c   |   3 +-
 drivers/usb/gadget/f_sourcesink.c | 511 --
 drivers/usb/gadget/g_zero.h   |  13 +-
 drivers/usb/gadget/zero.c |  21 ++
 drivers/usb/misc/usbtest.c| 113 +++--
 5 files changed, 624 insertions(+), 37 deletions(-)

-- 
1.8.0

--
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 V4 2/2] usbtest: Add interrupt EP testcases

2014-07-20 Thread Amit Virdi
Two simple test cases for interrupt endpoints are added to the usbtest.c file.
These are simple non-queued interrupt IN and interrupt OUT transfers. Currently,
only gadget zero is capable of executing the interrupt EP test cases. However,
extending the same to other gadgets is extremely simple and can be done
on-demand.

The two new tests added are
  - Test 25: To verify Interrupt OUT transfer
  - Test 26: To verify Interrupt IN transfer

Since the default value of wMaxPacketSize is set as 1024, so interrupt
IN transfers must be specified with the size parameter = multiple of
1024. Otherwise the default value (512) in the usbtest application fails
the transfer. See [RUN 4] for sample logs

The application logs (usbtest) and corresponding kernel logs are as
following:
---
[Run 1]
./testusb -a -c 10 -s 2048 -t 26 -v 511
Jul 17 10:31:13 dlhl1014 kernel: [72056.950910] usbtest 7-1:3.0: TEST
26: read 2048 bytes 10 times

[Run 2]
./testusb -a -c 10 -s 1024 -t 25 -v 511
Jul 17 10:31:29 dlhl1014 kernel: [72072.834853] usbtest 7-1:3.0: TEST
25: write 1024 bytes 10 times

[Run 3]
./testusb -a -c 10 -s 1098 -t 25 -v 511
Jul 17 10:31:39 dlhl1014 kernel: [72082.322219] usbtest 7-1:3.0: TEST
25: write 1098 bytes 10 times

[Run 4 - Failure case scenario]
./testusb -a  -t 26
unknown speed   /dev/bus/usb/007/0040
/dev/bus/usb/007/004 test 26 --> 75 (Value too large for defined data
type)

Jul 17 11:11:30 dlhl1014 kernel: [74473.347219] usbtest 7-1:3.0: TEST
26: read 512 bytes 1000 times
Jul 17 11:11:30 dlhl1014 kernel: [74473.348959] usb 7-1: test26 failed,
iterations left 999, status -75 (not 0)
---

This code has been tested only with gadget zero and care has been taken so as to
not break the existing functionality. However, if anyone can test with other
gadgets then that would be great!

Signed-off-by: Amit Virdi 
---
 drivers/usb/misc/usbtest.c | 113 +++--
 1 file changed, 98 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 829f446064ea..90e6644dc886 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -54,6 +54,7 @@ struct usbtest_info {
unsignedautoconf:1;
unsignedctrl_out:1;
unsignediso:1;  /* try iso in/out */
+   unsignedintr:1; /* try interrupt in/out */
int alt;
 };
 
@@ -70,7 +71,10 @@ struct usbtest_dev {
int out_pipe;
int in_iso_pipe;
int out_iso_pipe;
+   int in_int_pipe;
+   int out_int_pipe;
struct usb_endpoint_descriptor  *iso_in, *iso_out;
+   struct usb_endpoint_descriptor  *int_in, *int_out;
struct mutexlock;
 
 #define TBUF_SIZE  256
@@ -101,6 +105,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
struct usb_host_interface   *alt;
struct usb_host_endpoint*in, *out;
struct usb_host_endpoint*iso_in, *iso_out;
+   struct usb_host_endpoint*int_in, *int_out;
struct usb_device   *udev;
 
for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
@@ -108,6 +113,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
 
in = out = NULL;
iso_in = iso_out = NULL;
+   int_in = int_out = NULL;
alt = intf->altsetting + tmp;
 
if (override_alt >= 0 &&
@@ -124,6 +130,9 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
switch (usb_endpoint_type(&e->desc)) {
case USB_ENDPOINT_XFER_BULK:
break;
+   case USB_ENDPOINT_XFER_INT:
+   if (dev->info->intr)
+   goto try_intr;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
goto try_iso;
@@ -139,6 +148,15 @@ get_endpoints(struct usbtest_dev *dev, struct 
usb_interface *intf)
out = e;
}
continue;
+try_intr:
+   if (usb_endpoint_dir_in(&e->desc)) {
+   if (!int_in)
+   int_in = e;
+   } else {
+   if (!int_out)
+   int_out = e;
+   }
+   continue;
 try_iso:
   

[PATCH V4 1/2] usb: gadget: zero: Add support for interrupt EP

2014-07-20 Thread Amit Virdi
Interrupt endpoints behave quite similar to the bulk endpoints with the
difference that the endpoints expect data sending/reception request at
particular intervals till the whole data has not been transmitted.

The interrupt EP support is added to gadget zero. A new alternate setting (=2)
has been added. It has 2 interrupt endpoints. The default parameters are set as:
bInterval: 1 ms for FS and 8 uFrames (implying 1 ms) for HS/SS
wMaxPacketSize: 64 bytes for FS and 1024 bytes for HS/SS
However, the same can be overridden through the module parameter interface.

The code is tested for HS and SS on a platform having DWC3 controller.

Signed-off-by: Amit Virdi 
---
 drivers/usb/gadget/f_loopback.c   |   3 +-
 drivers/usb/gadget/f_sourcesink.c | 511 --
 drivers/usb/gadget/g_zero.h   |  13 +-
 drivers/usb/gadget/zero.c |  21 ++
 4 files changed, 526 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c
index 4557cd03f0b1..bf04389137e6 100644
--- a/drivers/usb/gadget/f_loopback.c
+++ b/drivers/usb/gadget/f_loopback.c
@@ -298,7 +298,8 @@ static void disable_loopback(struct f_loopback *loop)
struct usb_composite_dev*cdev;
 
cdev = loop->function.config->cdev;
-   disable_endpoints(cdev, loop->in_ep, loop->out_ep, NULL, NULL);
+   disable_endpoints(cdev, loop->in_ep, loop->out_ep, NULL, NULL, NULL,
+   NULL);
VDBG(cdev, "%s disabled\n", loop->function.name);
 }
 
diff --git a/drivers/usb/gadget/f_sourcesink.c 
b/drivers/usb/gadget/f_sourcesink.c
index d3cd52db78fe..7c091a328228 100644
--- a/drivers/usb/gadget/f_sourcesink.c
+++ b/drivers/usb/gadget/f_sourcesink.c
@@ -23,6 +23,15 @@
 #include "gadget_chips.h"
 #include "u_f.h"
 
+#define USB_MS_TO_SS_INTERVAL(x) USB_MS_TO_HS_INTERVAL(x)
+
+enum eptype {
+   EP_CONTROL = 0,
+   EP_BULK,
+   EP_ISOC,
+   EP_INTERRUPT,
+};
+
 /*
  * SOURCE/SINK FUNCTION ... a primary testing vehicle for USB peripheral
  * controller drivers.
@@ -55,6 +64,8 @@ struct f_sourcesink {
struct usb_ep   *out_ep;
struct usb_ep   *iso_in_ep;
struct usb_ep   *iso_out_ep;
+   struct usb_ep   *int_in_ep;
+   struct usb_ep   *int_out_ep;
int cur_alt;
 };
 
@@ -68,6 +79,10 @@ static unsigned isoc_interval;
 static unsigned isoc_maxpacket;
 static unsigned isoc_mult;
 static unsigned isoc_maxburst;
+static unsigned int_interval; /* In ms */
+static unsigned int_maxpacket;
+static unsigned int_mult;
+static unsigned int_maxburst;
 static unsigned buflen;
 
 /*-*/
@@ -92,6 +107,16 @@ static struct usb_interface_descriptor 
source_sink_intf_alt1 = {
/* .iInterface  = DYNAMIC */
 };
 
+static struct usb_interface_descriptor source_sink_intf_alt2 = {
+   .bLength =  USB_DT_INTERFACE_SIZE,
+   .bDescriptorType =  USB_DT_INTERFACE,
+
+   .bAlternateSetting =2,
+   .bNumEndpoints =2,
+   .bInterfaceClass =  USB_CLASS_VENDOR_SPEC,
+   /* .iInterface  = DYNAMIC */
+};
+
 /* full speed support: */
 
 static struct usb_endpoint_descriptor fs_source_desc = {
@@ -130,6 +155,26 @@ static struct usb_endpoint_descriptor fs_iso_sink_desc = {
.bInterval =4,
 };
 
+static struct usb_endpoint_descriptor fs_int_source_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_IN,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(64),
+   .bInterval =GZERO_INT_INTERVAL,
+};
+
+static struct usb_endpoint_descriptor fs_int_sink_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_OUT,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(64),
+   .bInterval =GZERO_INT_INTERVAL,
+};
+
 static struct usb_descriptor_header *fs_source_sink_descs[] = {
(struct usb_descriptor_header *) &source_sink_intf_alt0,
(struct usb_descriptor_header *) &fs_sink_desc,
@@ -140,6 +185,10 @@ static struct usb_descriptor_header 
*fs_source_sink_descs[] = {
(struct usb_descriptor_header *) &fs_source_desc,
(struct usb_descriptor_header *) &fs_iso_sink_desc,
(struct usb_descriptor_header *) &fs_iso_source_desc,
+   (struct usb_descriptor_header *) &source_sink_intf_alt2,
+#define FS_ALT_IFC_2_OFFSET8
+   (struct usb_descriptor_header *) &fs_int_sink_desc,
+   (struct usb_descriptor_header *) &fs_int_source_desc,
NULL

[PATCH V2] usb: core: allow zero packet flag for interrupt urbs

2014-07-20 Thread Amit Virdi
Section 4.4.7.2 "Interrupt Transfer Bandwidth Requirements" of the USB3.0 spec
says:
A zero-length data payload is a valid transfer and may be useful for
some implementations.

So, extend the logic of allowing URB_ZERO_PACKET to interrupt urbs too.
Otherwise, the kernel throws warning of BOGUS transfer flags.

Signed-off-by: Amit Virdi 
Acked-by: Hans de Goede 
---
 drivers/usb/core/urb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index 991386ceb4ec..c9e8ee81b6b7 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -454,6 +454,7 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
URB_FREE_BUFFER);
switch (xfertype) {
case USB_ENDPOINT_XFER_BULK:
+   case USB_ENDPOINT_XFER_INT:
if (is_out)
allowed |= URB_ZERO_PACKET;
/* FALLTHROUGH */
-- 
1.8.0

--
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] usb: core: allow zero packet flag for interrupt urbs

2014-07-20 Thread Amit Virdi

On 7/18/2014 8:09 PM, Alan Stern wrote:

On Fri, 18 Jul 2014, Amit Virdi wrote:


On 7/17/2014 8:25 PM, Alan Stern wrote:

I can't say this is actually wrong, but have you ever encountered a
situation where this would be needed?  How often does anyone need to do
a multi-packet transfer over an interrupt endpoint?


Honestly, I haven't found any such real device yet. I did this change
while I was going through the code when adding support for interrupt
transfers in gadget zero. I'm a no expert, but the spec says it should
be supported so this code should be added.


I just remembered, the HID spec requires that all reports (except the
longest) end with a short packet.  And since output reports are allowed
to be sent over an Interrupt-OUT endpoint, we have to accept the
URB_ZERO_PACKET flag for interrupt URBs.



Ok, great!


However, I messed up a little in this patch. It should have been
---
  case USB_ENDPOINT_XFER_ISOC:
  allowed |= URB_ISO_ASAP;
  break;
+   case USB_ENDPOINT_XFER_INT:
+   if (is_out)
+   allowed |= URB_ZERO_PACKET;
+   else
+   allowed |= URB_SHORT_NOT_OK;
+   break;
  }
  allowed &= urb->transfer_flags;

---
Otherwise, it sets zero packet flag for control out transfers too.

I'll send a V2 with this change if you agree to setting of the zero
packet flag for interrupt transfers.


How about this change instead?

URB_FREE_BUFFER);
switch (xfertype) {
case USB_ENDPOINT_XFER_BULK:
+   case USB_ENDPOINT_XFER_INT:


Looks better.
I'll submit a V2 with this change.

Regards
Amit Virdi
--
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] usb: core: allow zero packet flag for interrupt urbs

2014-07-18 Thread Amit Virdi

On 7/17/2014 8:25 PM, Alan Stern wrote:

I can't say this is actually wrong, but have you ever encountered a
situation where this would be needed?  How often does anyone need to do
a multi-packet transfer over an interrupt endpoint?


Honestly, I haven't found any such real device yet. I did this change 
while I was going through the code when adding support for interrupt 
transfers in gadget zero. I'm a no expert, but the spec says it should 
be supported so this code should be added.


However, I messed up a little in this patch. It should have been
---
case USB_ENDPOINT_XFER_ISOC:
allowed |= URB_ISO_ASAP;
break;
+   case USB_ENDPOINT_XFER_INT:
+   if (is_out)
+   allowed |= URB_ZERO_PACKET;
+   else
+   allowed |= URB_SHORT_NOT_OK;
+   break;
}
allowed &= urb->transfer_flags;

---
Otherwise, it sets zero packet flag for control out transfers too.

I'll send a V2 with this change if you agree to setting of the zero 
packet flag for interrupt transfers.


Regards
Amit Virdi
--
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 V3 0/2] usb: gadget: zero: Add support for interrupt EP

2014-07-17 Thread Amit Virdi

On 7/17/2014 8:06 PM, Felipe Balbi wrote:

that's good, thanks. But my tree is now closed for v3.17, once v3.17-rc1
is out I'll start queueing for v3.18.


No probs!
I'll send the patch with updated commit msg.

BTW, I think you mean you'll queue for kernel v3.17 (v3.16 is yet to be 
released as v3.16-rc5 is the latest). Pardon my ignorance otherwise...


Regards
Amit Virdi
--
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] usb: core: allow zero packet flag for interrupt urbs

2014-07-17 Thread Amit Virdi
Section 4.4.7.2 of the USB3.0 spec says:
A zero-length data payload is a valid transfer and may be useful for
some implementations.

So, extend the logic of allowing URB_ZERO_PACKET to interrupt urbs too.
Otherwise, the kernel throws error of BOGUS transfer flags.

Signed-off-by: Amit Virdi 
---
 drivers/usb/core/urb.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index 991386c..a136246 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -460,6 +460,10 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
case USB_ENDPOINT_XFER_CONTROL:
allowed |= URB_NO_FSBR; /* only affects UHCI */
/* FALLTHROUGH */
+   case USB_ENDPOINT_XFER_INT:
+   if (is_out)
+   allowed |= URB_ZERO_PACKET;
+   /* FALLTHROUGH */
default:/* all non-iso endpoints */
if (!is_out)
allowed |= URB_SHORT_NOT_OK;
-- 
1.8.0

--
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 V3 0/2] usb: gadget: zero: Add support for interrupt EP

2014-07-16 Thread Amit Virdi

can you send some logs of this patchset working ? What output should we
expect ? How to run it ?

(sure, we could just read the source code and figure it out, but it's
good 'documentation' to put on the commit log)



In case the newly added tests pass, there's no log that the test passed
(although kernel reports that it is going to execute the Test#). This
has been the convention followed in the usbtest.c for all other tests as
well.

But surely, I can document how to execute the new tests and what to
expect for failure scenarios.



Let me know if you expect something like this in the commit logs:

The two new tests added are
 - Test 25: To verify Interrupt OUT transfer
 - Test 26: To verify Interrupt IN transfer

Since the default value of wMaxPacketSize is set as 1024, so interrupt 
IN transfers must be specified with the size parameter = multiple of 
1024. Otherwise the default value (512) in the usbtest application fails 
the transfer. See [RUN 4] for sample logs


The application logs (usbtest) and corresponding kernel logs are as 
following:

---
[Run 1]
./testusb -a -c 10 -s 2048 -t 26 -v 511
Jul 17 10:31:13 dlhl1014 kernel: [72056.950910] usbtest 7-1:3.0: TEST 
26: read 2048 bytes 10 times


[Run 2]
./testusb -a -c 10 -s 1024 -t 25 -v 511
Jul 17 10:31:29 dlhl1014 kernel: [72072.834853] usbtest 7-1:3.0: TEST 
25: write 1024 bytes 10 times


[Run 3]
./testusb -a -c 10 -s 1098 -t 25 -v 511
Jul 17 10:31:39 dlhl1014 kernel: [72082.322219] usbtest 7-1:3.0: TEST 
25: write 1098 bytes 10 times


[Run 4 - Failure case scenario]
./testusb -a  -t 26
unknown speed   /dev/bus/usb/007/0040
/dev/bus/usb/007/004 test 26 --> 75 (Value too large for defined data type)

Jul 17 11:11:30 dlhl1014 kernel: [74473.347219] usbtest 7-1:3.0: TEST 
26: read 512 bytes 1000 times
Jul 17 11:11:30 dlhl1014 kernel: [74473.348959] usb 7-1: test26 failed, 
iterations left 999, status -75 (not 0)


---

Regards
Amit Virdi
--
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 V3 0/2] usb: gadget: zero: Add support for interrupt EP

2014-07-16 Thread Amit Virdi

On 7/1/2014 12:01 AM, Felipe Balbi wrote:

Hi,

On Mon, Jun 09, 2014 at 10:44:38AM +0530, Amit Virdi wrote:

Felipe, Alan,

On 5/23/2014 12:01 PM, Amit VIRDI wrote:

This patchset adds support for interrupt EP and the corresponding test cases to
gadget zero. The code has been rebased and tested on Kernel v3.15-rc5

V2 -> V3
  - Rectified wMaxPacketSize for FS from 1023 to 64
  - Modified default value of interrupt interval for FS to 1 ms

V1 -> V2
  - Modified the alternate interface from having 6 EPs (2 - Interrupt, 2 Bulk, 2
Isoc) to 2 EPs (Interrupt only)

RFC -> V1
  - Added support for configuring interrupt EP attributes from configfs 
interface

Amit Virdi (2):
   usb: gadget: zero: Add support for interrupt EP
   usbtest: Add interrupt EP testcases



Any comment on this patchset?


can you send some logs of this patchset working ? What output should we
expect ? How to run it ?

(sure, we could just read the source code and figure it out, but it's
good 'documentation' to put on the commit log)



In case the newly added tests pass, there's no log that the test passed 
(although kernel reports that it is going to execute the Test#). This 
has been the convention followed in the usbtest.c for all other tests as 
well.


But surely, I can document how to execute the new tests and what to 
expect for failure scenarios.


I'll rebase the patchset on latest kernel version, and re-send with 
modified commit logs.


Regards
Amit Virdi
--
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 V3 0/2] usb: gadget: zero: Add support for interrupt EP

2014-06-08 Thread Amit Virdi

Felipe, Alan,

On 5/23/2014 12:01 PM, Amit VIRDI wrote:

This patchset adds support for interrupt EP and the corresponding test cases to
gadget zero. The code has been rebased and tested on Kernel v3.15-rc5

V2 -> V3
  - Rectified wMaxPacketSize for FS from 1023 to 64
  - Modified default value of interrupt interval for FS to 1 ms

V1 -> V2
  - Modified the alternate interface from having 6 EPs (2 - Interrupt, 2 Bulk, 2
Isoc) to 2 EPs (Interrupt only)

RFC -> V1
  - Added support for configuring interrupt EP attributes from configfs 
interface

Amit Virdi (2):
   usb: gadget: zero: Add support for interrupt EP
   usbtest: Add interrupt EP testcases



Any comment on this patchset?

Regards
Amit Virdi
--
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 V3 2/2] usbtest: Add interrupt EP testcases

2014-05-27 Thread Amit Virdi

On 5/23/2014 8:14 PM, Daniele Forsi wrote:

2014-05-23 8:32 GMT+02:00 Amit Virdi:


@@ -124,6 +130,9 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
 switch (usb_endpoint_type(&e->desc)) {
 case USB_ENDPOINT_XFER_BULK:
 break;
+   case USB_ENDPOINT_XFER_INT:
+   if (dev->info->intr)
+   goto try_intr;
 case USB_ENDPOINT_XFER_ISOC:
 if (dev->info->iso)
 goto try_iso;
@@ -139,6 +148,15 @@ get_endpoints(struct usbtest_dev *dev, struct 
usb_interface *intf)


I don't think you really mean to fall through to case
USB_ENDPOINT_XFER_ISOC if the test is false, but the logic of that


I do mean to fall through to check whether the EP is ISOC type if it 
isn't INTERRUPT.



for-loop is becoming harder to follow

in pseudo code, the switch statement is like this?
case USB_ENDPOINT_XFER_BULK:
   set in or out;
   break;
case USB_ENDPOINT_XFER_INT:
   set int_in or int_out;
   break;
case USB_ENDPOINT_XFER_ISOC:
   set iso_in or iso_out;
   break;
default:
  do nothing;

it would be easier to follow even if it adds and indentation level



I do agree that the goto statements don't look visually pleasing here, 
but the approach you suggested would end up in upto 7 levels of 
indentation and that would make the code look even more ugly (see below 
sample code snippet)


---
for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
unsigned ep;
.
.
.

for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) {
struct usb_host_endpoint*e;
e = alt->endpoint + ep;
switch (usb_endpoint_type(&e->desc)) {
.
.
.

  case USB_ENDPOINT_XFER_INT:
 if (dev->info->intr) {
 if usb_endpoint_dir_in(&e->desc) {
 if (!int_in)
 int_in = e;
 }
 else {
 if (!int_out)
 int_out = e;
 }
 }
 break;
.
.
.
.

---
--
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 V3 0/2] usb: gadget: zero: Add support for interrupt EP

2014-05-22 Thread Amit Virdi
This patchset adds support for interrupt EP and the corresponding test cases to
gadget zero. The code has been rebased and tested on Kernel v3.15-rc5

V2 -> V3
 - Rectified wMaxPacketSize for FS from 1023 to 64
 - Modified default value of interrupt interval for FS to 1 ms

V1 -> V2
 - Modified the alternate interface from having 6 EPs (2 - Interrupt, 2 Bulk, 2
   Isoc) to 2 EPs (Interrupt only)

RFC -> V1
 - Added support for configuring interrupt EP attributes from configfs interface

Amit Virdi (2):
  usb: gadget: zero: Add support for interrupt EP
  usbtest: Add interrupt EP testcases

 drivers/usb/gadget/f_loopback.c   |   3 +-
 drivers/usb/gadget/f_sourcesink.c | 511 --
 drivers/usb/gadget/g_zero.h   |  13 +-
 drivers/usb/gadget/zero.c |  21 ++
 drivers/usb/misc/usbtest.c| 113 +++--
 5 files changed, 624 insertions(+), 37 deletions(-)

-- 
1.8.0

--
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 V3 1/2] usb: gadget: zero: Add support for interrupt EP

2014-05-22 Thread Amit Virdi
Interrupt endpoints behave quite similar to the bulk endpoints with the
difference that the endpoints expect data sending/reception request at
particular intervals till the whole data has not been transmitted.

The interrupt EP support is added to gadget zero. A new alternate setting (=2)
has been added. It has 2 interrupt endpoints. The default parameters are set as:
bInterval: 1 ms for FS and 8 uFrames (implying 1 ms) for HS/SS
wMaxPacketSize: 64 bytes for FS and 1024 bytes for HS/SS
However, the same can be overridden through the module parameter interface.

The code is tested for HS and SS on a platform having DWC3 controller.

Signed-off-by: Amit Virdi 
---
 drivers/usb/gadget/f_loopback.c   |   3 +-
 drivers/usb/gadget/f_sourcesink.c | 511 --
 drivers/usb/gadget/g_zero.h   |  13 +-
 drivers/usb/gadget/zero.c |  21 ++
 4 files changed, 526 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c
index 4557cd0..bf04389 100644
--- a/drivers/usb/gadget/f_loopback.c
+++ b/drivers/usb/gadget/f_loopback.c
@@ -298,7 +298,8 @@ static void disable_loopback(struct f_loopback *loop)
struct usb_composite_dev*cdev;
 
cdev = loop->function.config->cdev;
-   disable_endpoints(cdev, loop->in_ep, loop->out_ep, NULL, NULL);
+   disable_endpoints(cdev, loop->in_ep, loop->out_ep, NULL, NULL, NULL,
+   NULL);
VDBG(cdev, "%s disabled\n", loop->function.name);
 }
 
diff --git a/drivers/usb/gadget/f_sourcesink.c 
b/drivers/usb/gadget/f_sourcesink.c
index d3cd52d..7c091a3 100644
--- a/drivers/usb/gadget/f_sourcesink.c
+++ b/drivers/usb/gadget/f_sourcesink.c
@@ -23,6 +23,15 @@
 #include "gadget_chips.h"
 #include "u_f.h"
 
+#define USB_MS_TO_SS_INTERVAL(x) USB_MS_TO_HS_INTERVAL(x)
+
+enum eptype {
+   EP_CONTROL = 0,
+   EP_BULK,
+   EP_ISOC,
+   EP_INTERRUPT,
+};
+
 /*
  * SOURCE/SINK FUNCTION ... a primary testing vehicle for USB peripheral
  * controller drivers.
@@ -55,6 +64,8 @@ struct f_sourcesink {
struct usb_ep   *out_ep;
struct usb_ep   *iso_in_ep;
struct usb_ep   *iso_out_ep;
+   struct usb_ep   *int_in_ep;
+   struct usb_ep   *int_out_ep;
int cur_alt;
 };
 
@@ -68,6 +79,10 @@ static unsigned isoc_interval;
 static unsigned isoc_maxpacket;
 static unsigned isoc_mult;
 static unsigned isoc_maxburst;
+static unsigned int_interval; /* In ms */
+static unsigned int_maxpacket;
+static unsigned int_mult;
+static unsigned int_maxburst;
 static unsigned buflen;
 
 /*-*/
@@ -92,6 +107,16 @@ static struct usb_interface_descriptor 
source_sink_intf_alt1 = {
/* .iInterface  = DYNAMIC */
 };
 
+static struct usb_interface_descriptor source_sink_intf_alt2 = {
+   .bLength =  USB_DT_INTERFACE_SIZE,
+   .bDescriptorType =  USB_DT_INTERFACE,
+
+   .bAlternateSetting =2,
+   .bNumEndpoints =2,
+   .bInterfaceClass =  USB_CLASS_VENDOR_SPEC,
+   /* .iInterface  = DYNAMIC */
+};
+
 /* full speed support: */
 
 static struct usb_endpoint_descriptor fs_source_desc = {
@@ -130,6 +155,26 @@ static struct usb_endpoint_descriptor fs_iso_sink_desc = {
.bInterval =4,
 };
 
+static struct usb_endpoint_descriptor fs_int_source_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_IN,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(64),
+   .bInterval =GZERO_INT_INTERVAL,
+};
+
+static struct usb_endpoint_descriptor fs_int_sink_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_OUT,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(64),
+   .bInterval =GZERO_INT_INTERVAL,
+};
+
 static struct usb_descriptor_header *fs_source_sink_descs[] = {
(struct usb_descriptor_header *) &source_sink_intf_alt0,
(struct usb_descriptor_header *) &fs_sink_desc,
@@ -140,6 +185,10 @@ static struct usb_descriptor_header 
*fs_source_sink_descs[] = {
(struct usb_descriptor_header *) &fs_source_desc,
(struct usb_descriptor_header *) &fs_iso_sink_desc,
(struct usb_descriptor_header *) &fs_iso_source_desc,
+   (struct usb_descriptor_header *) &source_sink_intf_alt2,
+#define FS_ALT_IFC_2_OFFSET8
+   (struct usb_descriptor_header *) &fs_int_sink_desc,
+   (struct usb_descriptor_header *) &fs_int_source_desc,
NULL,
 };
 
@@ -179,6 +228,

[Patch V3 2/2] usbtest: Add interrupt EP testcases

2014-05-22 Thread Amit Virdi
Two simple test cases for interrupt endpoints are added to the usbtest.c file.
These are simple non-queued interrupt IN and interrupt OUT transfers. Currently,
only gadget zero is capable of executing the interrupt EP test cases. However,
extending the same to other gadgets is extremely simple and can be done
on-demand.

This code has been tested only with gadget zero and care has been taken so as to
not break the existing functionality. However, if anyone can test with other
gadgets then that would be great!

Signed-off-by: Amit Virdi 
---
 drivers/usb/misc/usbtest.c | 113 +++--
 1 file changed, 98 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index f6568b5..5c6baaa 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -54,6 +54,7 @@ struct usbtest_info {
unsignedautoconf:1;
unsignedctrl_out:1;
unsignediso:1;  /* try iso in/out */
+   unsignedintr:1; /* try interrupt in/out */
int alt;
 };
 
@@ -70,7 +71,10 @@ struct usbtest_dev {
int out_pipe;
int in_iso_pipe;
int out_iso_pipe;
+   int in_int_pipe;
+   int out_int_pipe;
struct usb_endpoint_descriptor  *iso_in, *iso_out;
+   struct usb_endpoint_descriptor  *int_in, *int_out;
struct mutexlock;
 
 #define TBUF_SIZE  256
@@ -101,6 +105,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
struct usb_host_interface   *alt;
struct usb_host_endpoint*in, *out;
struct usb_host_endpoint*iso_in, *iso_out;
+   struct usb_host_endpoint*int_in, *int_out;
struct usb_device   *udev;
 
for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
@@ -108,6 +113,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
 
in = out = NULL;
iso_in = iso_out = NULL;
+   int_in = int_out = NULL;
alt = intf->altsetting + tmp;
 
if (override_alt >= 0 &&
@@ -124,6 +130,9 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
switch (usb_endpoint_type(&e->desc)) {
case USB_ENDPOINT_XFER_BULK:
break;
+   case USB_ENDPOINT_XFER_INT:
+   if (dev->info->intr)
+   goto try_intr;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
goto try_iso;
@@ -139,6 +148,15 @@ get_endpoints(struct usbtest_dev *dev, struct 
usb_interface *intf)
out = e;
}
continue;
+try_intr:
+   if (usb_endpoint_dir_in(&e->desc)) {
+   if (!int_in)
+   int_in = e;
+   } else {
+   if (!int_out)
+   int_out = e;
+   }
+   continue;
 try_iso:
if (usb_endpoint_dir_in(&e->desc)) {
if (!iso_in)
@@ -148,7 +166,7 @@ try_iso:
iso_out = e;
}
}
-   if ((in && out)  ||  iso_in || iso_out)
+   if ((in && out)  ||  iso_in || iso_out || int_in || int_out)
goto found;
}
return -EINVAL;
@@ -183,6 +201,20 @@ found:
iso_out->desc.bEndpointAddress
& USB_ENDPOINT_NUMBER_MASK);
}
+
+   if (int_in) {
+   dev->int_in = &int_in->desc;
+   dev->in_int_pipe = usb_rcvintpipe(udev,
+   int_in->desc.bEndpointAddress
+   & USB_ENDPOINT_NUMBER_MASK);
+   }
+
+   if (int_out) {
+   dev->int_out = &int_out->desc;
+   dev->out_int_pipe = usb_sndintpipe(udev,
+   int_out->desc.bEndpointAddress
+   & USB_ENDPOINT_NUMBER_MASK);
+   }
return 0;
 }
 
@@ -205,14 +237,22 @@ static struct urb *usbtest_alloc_urb(
int pipe,
unsigned long   bytes,
unsignedtransfer_flags,
-   unsignedoffset)
+   unsignedoffset,

Re: [Patch V2 1/2] usb: gadget: zero: Add support for interrupt EP

2014-05-14 Thread Amit Virdi

On 5/10/2014 1:04 AM, Alan Stern wrote:

On Mon, 5 May 2014, Amit Virdi wrote:


Interrupt endpoints behave quite similar to the bulk endpoints with the
difference that the endpoints expect data sending/reception request at
particular intervals till the whole data has not been transmitted.

The interrupt EP support is added to gadget zero. A new alternate setting (=2)
has been added. It has 2 interrupt endpoints. The default parameters are set as:
bInterval: 4
wMaxPacketSize: 1024


The default size should be the maximum allowed for the current speed.



Yes, I understand. I'll modify the code for FS and change this commit 
message.



The code is tested for HS and SS on a platform having DWC3 controller.
+static struct usb_endpoint_descriptor fs_int_source_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_IN,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(1023),
+   .bInterval =4,
+};
+
+static struct usb_endpoint_descriptor fs_int_sink_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_OUT,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(1023),
+   .bInterval =4,
+};


The maximum interrupt packet size for full speed is 64, not 1023.



I missed it, thanks for pointing out. I'll rectify the code.


+   /* sanity check the interrupt module parameters */
+   if (int_interval < 1)
+   int_interval = 1;
+   if (int_interval > 16)
+   int_interval = 16;


Interrupt intervals for full-speed are specified in frames, not in
exponential form.  The maximum allowed value is 255.



Agreed. I'll send V2.

Regards
Amit Virdi
--
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 8/8] usb: dwc3: gadget: always enable IOC on bulk/interrupt transfers

2014-05-05 Thread Amit Virdi

On 5/6/2014 12:56 AM, Felipe Balbi wrote:

I understand that enabling XferInProgress event for bulk/interrupt
>transfers will completely
>change the design of the dwc3 driver and hence is not the viable solution
>to the issue here.

just send a patch enabling XferInProgress.. I haven't gotten to it yet.
If you beat me to it, more power for you;-)


Enabling XferInProgress event for bulk and interrupt would incur 
significant testing efforts. Till it is done can we revert this patch as 
it isn't correct conceptually?

--
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 V2 1/2] usb: gadget: zero: Add support for interrupt EP

2014-05-05 Thread Amit Virdi
Interrupt endpoints behave quite similar to the bulk endpoints with the
difference that the endpoints expect data sending/reception request at
particular intervals till the whole data has not been transmitted.

The interrupt EP support is added to gadget zero. A new alternate setting (=2)
has been added. It has 2 interrupt endpoints. The default parameters are set as:
bInterval: 4
wMaxPacketSize: 1024
However, the same can be overridden through the module parameter interface.

The code is tested for HS and SS on a platform having DWC3 controller.

Signed-off-by: Amit Virdi 
---
 drivers/usb/gadget/f_loopback.c   |   3 +-
 drivers/usb/gadget/f_sourcesink.c | 507 --
 drivers/usb/gadget/g_zero.h   |  13 +-
 drivers/usb/gadget/zero.c |  21 ++
 4 files changed, 522 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c
index 4557cd0..bf04389 100644
--- a/drivers/usb/gadget/f_loopback.c
+++ b/drivers/usb/gadget/f_loopback.c
@@ -298,7 +298,8 @@ static void disable_loopback(struct f_loopback *loop)
struct usb_composite_dev*cdev;
 
cdev = loop->function.config->cdev;
-   disable_endpoints(cdev, loop->in_ep, loop->out_ep, NULL, NULL);
+   disable_endpoints(cdev, loop->in_ep, loop->out_ep, NULL, NULL, NULL,
+   NULL);
VDBG(cdev, "%s disabled\n", loop->function.name);
 }
 
diff --git a/drivers/usb/gadget/f_sourcesink.c 
b/drivers/usb/gadget/f_sourcesink.c
index d3cd52d..4c96b99 100644
--- a/drivers/usb/gadget/f_sourcesink.c
+++ b/drivers/usb/gadget/f_sourcesink.c
@@ -23,6 +23,13 @@
 #include "gadget_chips.h"
 #include "u_f.h"
 
+enum eptype {
+   EP_CONTROL = 0,
+   EP_BULK,
+   EP_ISOC,
+   EP_INTERRUPT,
+};
+
 /*
  * SOURCE/SINK FUNCTION ... a primary testing vehicle for USB peripheral
  * controller drivers.
@@ -55,6 +62,8 @@ struct f_sourcesink {
struct usb_ep   *out_ep;
struct usb_ep   *iso_in_ep;
struct usb_ep   *iso_out_ep;
+   struct usb_ep   *int_in_ep;
+   struct usb_ep   *int_out_ep;
int cur_alt;
 };
 
@@ -68,6 +77,10 @@ static unsigned isoc_interval;
 static unsigned isoc_maxpacket;
 static unsigned isoc_mult;
 static unsigned isoc_maxburst;
+static unsigned int_interval;
+static unsigned int_maxpacket;
+static unsigned int_mult;
+static unsigned int_maxburst;
 static unsigned buflen;
 
 /*-*/
@@ -92,6 +105,16 @@ static struct usb_interface_descriptor 
source_sink_intf_alt1 = {
/* .iInterface  = DYNAMIC */
 };
 
+static struct usb_interface_descriptor source_sink_intf_alt2 = {
+   .bLength =  USB_DT_INTERFACE_SIZE,
+   .bDescriptorType =  USB_DT_INTERFACE,
+
+   .bAlternateSetting =2,
+   .bNumEndpoints =2,
+   .bInterfaceClass =  USB_CLASS_VENDOR_SPEC,
+   /* .iInterface  = DYNAMIC */
+};
+
 /* full speed support: */
 
 static struct usb_endpoint_descriptor fs_source_desc = {
@@ -130,6 +153,26 @@ static struct usb_endpoint_descriptor fs_iso_sink_desc = {
.bInterval =4,
 };
 
+static struct usb_endpoint_descriptor fs_int_source_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_IN,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(1023),
+   .bInterval =4,
+};
+
+static struct usb_endpoint_descriptor fs_int_sink_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_OUT,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(1023),
+   .bInterval =4,
+};
+
 static struct usb_descriptor_header *fs_source_sink_descs[] = {
(struct usb_descriptor_header *) &source_sink_intf_alt0,
(struct usb_descriptor_header *) &fs_sink_desc,
@@ -140,6 +183,10 @@ static struct usb_descriptor_header 
*fs_source_sink_descs[] = {
(struct usb_descriptor_header *) &fs_source_desc,
(struct usb_descriptor_header *) &fs_iso_sink_desc,
(struct usb_descriptor_header *) &fs_iso_source_desc,
+   (struct usb_descriptor_header *) &source_sink_intf_alt2,
+#define FS_ALT_IFC_2_OFFSET8
+   (struct usb_descriptor_header *) &fs_int_sink_desc,
+   (struct usb_descriptor_header *) &fs_int_source_desc,
NULL,
 };
 
@@ -179,6 +226,24 @@ static struct usb_endpoint_descriptor hs_iso_sink_desc = {
.bInterval =4,
 };
 
+static struct usb_endpoint_descriptor hs_int_source_desc = {
+   .bLength =

[Patch V2 0/2] usb: gadget: zero: Add support for interrupt EP

2014-05-05 Thread Amit Virdi
This patchset adds support for interrupt EP and the corresponding test cases to
gadget zero. The code has been rebased and tested on Kernel v3.15-rc3

V1 -> V2
 - Modified the alternate interface from having 6 EPs (2 - Interrupt, 2 Bulk, 2
   Isoc) to 2 EPs (Interrupt only)

RFC -> V1
 - Added support for configuring interrupt EP attributes from configfs interface

Amit Virdi (2):
  usb: gadget: zero: Add support for interrupt EP
  usbtest: Add interrupt EP testcases

 drivers/usb/gadget/f_loopback.c   |   3 +-
 drivers/usb/gadget/f_sourcesink.c | 507 --
 drivers/usb/gadget/g_zero.h   |  13 +-
 drivers/usb/gadget/zero.c |  21 ++
 drivers/usb/misc/usbtest.c| 113 +++--
 5 files changed, 620 insertions(+), 37 deletions(-)

-- 
1.8.0

--
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 V2 2/2] usbtest: Add interrupt EP testcases

2014-05-05 Thread Amit Virdi
Two simple test cases for interrupt endpoints are added to the usbtest.c file.
These are simple non-queued interrupt IN and interrupt OUT transfers. Currently,
only gadget zero is capable of executing the interrupt EP test cases. However,
extending the same to other gadgets is extremely simple and can be done
on-demand.

This code has been tested only with gadget zero and care has been taken so as to
not break the existing functionality. However, if anyone can test with other
gadgets then that would be great!

Signed-off-by: Amit Virdi 
---
 drivers/usb/misc/usbtest.c | 113 +++--
 1 file changed, 98 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index f6568b5..5c6baaa 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -54,6 +54,7 @@ struct usbtest_info {
unsignedautoconf:1;
unsignedctrl_out:1;
unsignediso:1;  /* try iso in/out */
+   unsignedintr:1; /* try interrupt in/out */
int alt;
 };
 
@@ -70,7 +71,10 @@ struct usbtest_dev {
int out_pipe;
int in_iso_pipe;
int out_iso_pipe;
+   int in_int_pipe;
+   int out_int_pipe;
struct usb_endpoint_descriptor  *iso_in, *iso_out;
+   struct usb_endpoint_descriptor  *int_in, *int_out;
struct mutexlock;
 
 #define TBUF_SIZE  256
@@ -101,6 +105,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
struct usb_host_interface   *alt;
struct usb_host_endpoint*in, *out;
struct usb_host_endpoint*iso_in, *iso_out;
+   struct usb_host_endpoint*int_in, *int_out;
struct usb_device   *udev;
 
for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
@@ -108,6 +113,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
 
in = out = NULL;
iso_in = iso_out = NULL;
+   int_in = int_out = NULL;
alt = intf->altsetting + tmp;
 
if (override_alt >= 0 &&
@@ -124,6 +130,9 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
switch (usb_endpoint_type(&e->desc)) {
case USB_ENDPOINT_XFER_BULK:
break;
+   case USB_ENDPOINT_XFER_INT:
+   if (dev->info->intr)
+   goto try_intr;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
goto try_iso;
@@ -139,6 +148,15 @@ get_endpoints(struct usbtest_dev *dev, struct 
usb_interface *intf)
out = e;
}
continue;
+try_intr:
+   if (usb_endpoint_dir_in(&e->desc)) {
+   if (!int_in)
+   int_in = e;
+   } else {
+   if (!int_out)
+   int_out = e;
+   }
+   continue;
 try_iso:
if (usb_endpoint_dir_in(&e->desc)) {
if (!iso_in)
@@ -148,7 +166,7 @@ try_iso:
iso_out = e;
}
}
-   if ((in && out)  ||  iso_in || iso_out)
+   if ((in && out)  ||  iso_in || iso_out || int_in || int_out)
goto found;
}
return -EINVAL;
@@ -183,6 +201,20 @@ found:
iso_out->desc.bEndpointAddress
& USB_ENDPOINT_NUMBER_MASK);
}
+
+   if (int_in) {
+   dev->int_in = &int_in->desc;
+   dev->in_int_pipe = usb_rcvintpipe(udev,
+   int_in->desc.bEndpointAddress
+   & USB_ENDPOINT_NUMBER_MASK);
+   }
+
+   if (int_out) {
+   dev->int_out = &int_out->desc;
+   dev->out_int_pipe = usb_sndintpipe(udev,
+   int_out->desc.bEndpointAddress
+   & USB_ENDPOINT_NUMBER_MASK);
+   }
return 0;
 }
 
@@ -205,14 +237,22 @@ static struct urb *usbtest_alloc_urb(
int pipe,
unsigned long   bytes,
unsignedtransfer_flags,
-   unsignedoffset)
+   unsignedoffset,

Re: [Patch V1 1/2] usb: gadget: zero: Add support for interrupt EP

2014-04-30 Thread Amit Virdi

On 4/30/2014 9:36 PM, Felipe Balbi wrote:

On Fri, Mar 14, 2014 at 11:41:47AM -0400, Alan Stern wrote:

On Fri, 14 Mar 2014, Amit Virdi wrote:


Felipe, Alan,

On 2/25/2014 10:43 AM, Amit Virdi wrote:

ccing: Felipe Balbi, Alen Stern

On 2/24/2014 3:55 PM, Amit VIRDI wrote:

Interrupt endpoints behave quite similar to the bulk endpoints with the
difference that the endpoints expect data sending/reception request at
particular intervals till the whole data has not been transmitted.

The interrupt EP support is added to gadget zero. A new alternate
setting (=2)
has been added. It has 6 endpoints (2-BULK, 2-ISOC, 2-INTERRUPT). The
default
parameters are set as:
  bInterval: 4
  wMaxPacketSize: 1024
However, the same can be overridden through the module parameter
interface.

The code is tested for HS and SS on a platform having DWC3 controller.

Signed-off-by: Amit Virdi 


I'm sorry, I simply have not had time to look at either of these
patches.  The basic idea is okay; there's no good reason not to have
testing available for interrupt endpoints.

What happens if you try to run this gadget but the UDC can't provide
more than 4 endpoints?  Would it make more sense to have the new
altsetting provide only the two interrupt endpoints?


makes sense to me.



Thanks Alan and Felipe,

I'll send V2 with modified implementation that would have fewer 
endpoints - 2 interrupt endpoints apart from the default 2 control EPs - 
 in alternate interface 2.


Regards
Amit Virdi
--
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 V1 2/2] usbtest: Add interrupt EP testcases

2014-03-14 Thread Amit Virdi

Felipe/Alan,

On 2/25/2014 10:44 AM, Amit Virdi wrote:

ccing: Felipe Balbi, Alan Stern

On 2/24/2014 3:55 PM, Amit VIRDI wrote:

Two simple test cases for interrupt endpoints are added to the
usbtest.c file.
These are simple non-queued interrupt IN and interrupt OUT transfers.
Currently,
only gadget zero is capable of executing the interrupt EP test cases.
However,
extending the same to other gadgets is extremely simple and can be done
on-demand.

This code has been tested only with gadget zero and care has been
taken so as to
not break the existing functionality. However, if anyone can test with
other
gadgets then that would be great!

Signed-off-by: Amit Virdi 
---
  drivers/usb/misc/usbtest.c | 112
+++--
  1 file changed, 97 insertions(+), 15 deletions(-)



Any comments?
--
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 V1 1/2] usb: gadget: zero: Add support for interrupt EP

2014-03-14 Thread Amit Virdi

Felipe, Alan,

On 2/25/2014 10:43 AM, Amit Virdi wrote:

ccing: Felipe Balbi, Alen Stern

On 2/24/2014 3:55 PM, Amit VIRDI wrote:

Interrupt endpoints behave quite similar to the bulk endpoints with the
difference that the endpoints expect data sending/reception request at
particular intervals till the whole data has not been transmitted.

The interrupt EP support is added to gadget zero. A new alternate
setting (=2)
has been added. It has 6 endpoints (2-BULK, 2-ISOC, 2-INTERRUPT). The
default
parameters are set as:
 bInterval: 4
 wMaxPacketSize: 1024
However, the same can be overridden through the module parameter
interface.

The code is tested for HS and SS on a platform having DWC3 controller.

Signed-off-by: Amit Virdi 
---
  drivers/usb/gadget/f_loopback.c   |   3 +-
  drivers/usb/gadget/f_sourcesink.c | 519
--
  drivers/usb/gadget/g_zero.h   |  13 +-
  drivers/usb/gadget/zero.c |  21 ++
  4 files changed, 533 insertions(+), 23 deletions(-)


Any comments?
--
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 V1 1/2] usb: gadget: zero: Add support for interrupt EP

2014-02-24 Thread Amit Virdi

ccing: Felipe Balbi, Alen Stern

On 2/24/2014 3:55 PM, Amit VIRDI wrote:

Interrupt endpoints behave quite similar to the bulk endpoints with the
difference that the endpoints expect data sending/reception request at
particular intervals till the whole data has not been transmitted.

The interrupt EP support is added to gadget zero. A new alternate setting (=2)
has been added. It has 6 endpoints (2-BULK, 2-ISOC, 2-INTERRUPT). The default
parameters are set as:
 bInterval: 4
 wMaxPacketSize: 1024
However, the same can be overridden through the module parameter interface.

The code is tested for HS and SS on a platform having DWC3 controller.

Signed-off-by: Amit Virdi 
---
  drivers/usb/gadget/f_loopback.c   |   3 +-
  drivers/usb/gadget/f_sourcesink.c | 519 --
  drivers/usb/gadget/g_zero.h   |  13 +-
  drivers/usb/gadget/zero.c |  21 ++
  4 files changed, 533 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c
index 4557cd0..bf04389 100644
--- a/drivers/usb/gadget/f_loopback.c
+++ b/drivers/usb/gadget/f_loopback.c
@@ -298,7 +298,8 @@ static void disable_loopback(struct f_loopback *loop)
 struct usb_composite_dev*cdev;

 cdev = loop->function.config->cdev;
-   disable_endpoints(cdev, loop->in_ep, loop->out_ep, NULL, NULL);
+   disable_endpoints(cdev, loop->in_ep, loop->out_ep, NULL, NULL, NULL,
+   NULL);
 VDBG(cdev, "%s disabled\n", loop->function.name);
  }

diff --git a/drivers/usb/gadget/f_sourcesink.c 
b/drivers/usb/gadget/f_sourcesink.c
index d3cd52d..306de39 100644
--- a/drivers/usb/gadget/f_sourcesink.c
+++ b/drivers/usb/gadget/f_sourcesink.c
@@ -23,6 +23,13 @@
  #include "gadget_chips.h"
  #include "u_f.h"

+enum eptype {
+   EP_CONTROL = 0,
+   EP_BULK,
+   EP_ISOC,
+   EP_INTERRUPT,
+};
+
  /*
   * SOURCE/SINK FUNCTION ... a primary testing vehicle for USB peripheral
   * controller drivers.
@@ -55,6 +62,8 @@ struct f_sourcesink {
 struct usb_ep   *out_ep;
 struct usb_ep   *iso_in_ep;
 struct usb_ep   *iso_out_ep;
+   struct usb_ep   *int_in_ep;
+   struct usb_ep   *int_out_ep;
 int cur_alt;
  };

@@ -68,6 +77,10 @@ static unsigned isoc_interval;
  static unsigned isoc_maxpacket;
  static unsigned isoc_mult;
  static unsigned isoc_maxburst;
+static unsigned int_interval;
+static unsigned int_maxpacket;
+static unsigned int_mult;
+static unsigned int_maxburst;
  static unsigned buflen;

  /*-*/
@@ -92,6 +105,16 @@ static struct usb_interface_descriptor 
source_sink_intf_alt1 = {
 /* .iInterface  = DYNAMIC */
  };

+static struct usb_interface_descriptor source_sink_intf_alt2 = {
+   .bLength =  USB_DT_INTERFACE_SIZE,
+   .bDescriptorType =  USB_DT_INTERFACE,
+
+   .bAlternateSetting =2,
+   .bNumEndpoints =6,
+   .bInterfaceClass =  USB_CLASS_VENDOR_SPEC,
+   /* .iInterface  = DYNAMIC */
+};
+
  /* full speed support: */

  static struct usb_endpoint_descriptor fs_source_desc = {
@@ -130,6 +153,26 @@ static struct usb_endpoint_descriptor fs_iso_sink_desc = {
 .bInterval =4,
  };

+static struct usb_endpoint_descriptor fs_int_source_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_IN,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(1023),
+   .bInterval =4,
+};
+
+static struct usb_endpoint_descriptor fs_int_sink_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_OUT,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(1023),
+   .bInterval =4,
+};
+
  static struct usb_descriptor_header *fs_source_sink_descs[] = {
 (struct usb_descriptor_header *) &source_sink_intf_alt0,
 (struct usb_descriptor_header *) &fs_sink_desc,
@@ -140,6 +183,14 @@ static struct usb_descriptor_header 
*fs_source_sink_descs[] = {
 (struct usb_descriptor_header *) &fs_source_desc,
 (struct usb_descriptor_header *) &fs_iso_sink_desc,
 (struct usb_descriptor_header *) &fs_iso_source_desc,
+   (struct usb_descriptor_header *) &source_sink_intf_alt2,
+#define FS_ALT_IFC_2_OFFSET8
+   (struct usb_descriptor_header *) &fs_sink_desc,
+   (struct usb_descriptor_header *) &fs_source_desc,
+   (struct usb_descriptor_header *) &fs_iso_sink_desc,
+   (struct usb_descriptor_header *) &fs_i

Re: [Patch V1 2/2] usbtest: Add interrupt EP testcases

2014-02-24 Thread Amit Virdi

ccing: Felipe Balbi, Alan Stern

On 2/24/2014 3:55 PM, Amit VIRDI wrote:

Two simple test cases for interrupt endpoints are added to the usbtest.c file.
These are simple non-queued interrupt IN and interrupt OUT transfers. Currently,
only gadget zero is capable of executing the interrupt EP test cases. However,
extending the same to other gadgets is extremely simple and can be done
on-demand.

This code has been tested only with gadget zero and care has been taken so as to
not break the existing functionality. However, if anyone can test with other
gadgets then that would be great!

Signed-off-by: Amit Virdi 
---
  drivers/usb/misc/usbtest.c | 112 +++--
  1 file changed, 97 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index f6568b5..8b96235 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -54,6 +54,7 @@ struct usbtest_info {
unsignedautoconf:1;
unsignedctrl_out:1;
unsignediso:1;  /* try iso in/out */
+   unsignedintr:1; /* try interrupt in/out */
int alt;
  };

@@ -70,7 +71,10 @@ struct usbtest_dev {
int out_pipe;
int in_iso_pipe;
int out_iso_pipe;
+   int in_int_pipe;
+   int out_int_pipe;
struct usb_endpoint_descriptor  *iso_in, *iso_out;
+   struct usb_endpoint_descriptor  *int_in, *int_out;
struct mutexlock;

  #define TBUF_SIZE 256
@@ -101,6 +105,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
struct usb_host_interface   *alt;
struct usb_host_endpoint*in, *out;
struct usb_host_endpoint*iso_in, *iso_out;
+   struct usb_host_endpoint*int_in, *int_out;
struct usb_device   *udev;

for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
@@ -108,6 +113,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)

in = out = NULL;
iso_in = iso_out = NULL;
+   int_in = int_out = NULL;
alt = intf->altsetting + tmp;

if (override_alt >= 0 &&
@@ -124,6 +130,9 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
switch (usb_endpoint_type(&e->desc)) {
case USB_ENDPOINT_XFER_BULK:
break;
+   case USB_ENDPOINT_XFER_INT:
+   if (dev->info->intr)
+   goto try_intr;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
goto try_iso;
@@ -139,6 +148,14 @@ get_endpoints(struct usbtest_dev *dev, struct 
usb_interface *intf)
out = e;
}
continue;
+try_intr:
+   if (usb_endpoint_dir_in(&e->desc)) {
+   if (!int_in)
+   int_in = e;
+   } else {
+   if (!int_out)
+   int_out = e;
+   }
  try_iso:
if (usb_endpoint_dir_in(&e->desc)) {
if (!iso_in)
@@ -148,7 +165,7 @@ try_iso:
iso_out = e;
}
}
-   if ((in && out)  ||  iso_in || iso_out)
+   if ((in && out)  ||  iso_in || iso_out || int_in || int_out)
goto found;
}
return -EINVAL;
@@ -183,6 +200,20 @@ found:
iso_out->desc.bEndpointAddress
& USB_ENDPOINT_NUMBER_MASK);
}
+
+   if (int_in) {
+   dev->int_in = &int_in->desc;
+   dev->in_int_pipe = usb_rcvintpipe(udev,
+   int_in->desc.bEndpointAddress
+   & USB_ENDPOINT_NUMBER_MASK);
+   }
+
+   if (int_out) {
+   dev->int_out = &int_out->desc;
+   dev->out_int_pipe = usb_sndintpipe(udev,
+   int_out->desc.bEndpointAddress
+   & USB_ENDPOINT_NUMBER_MASK);
+   }
return 0;
  }

@@ -205,14 +236,22 @@ static struct urb *usbtest_alloc_urb(
int pipe,
unsigned long   bytes,
unsignedtransfer_flags,
-   unsignedoffset)
+  

[Patch V1 1/2] usb: gadget: zero: Add support for interrupt EP

2014-02-24 Thread Amit Virdi
Interrupt endpoints behave quite similar to the bulk endpoints with the
difference that the endpoints expect data sending/reception request at
particular intervals till the whole data has not been transmitted.

The interrupt EP support is added to gadget zero. A new alternate setting (=2)
has been added. It has 6 endpoints (2-BULK, 2-ISOC, 2-INTERRUPT). The default
parameters are set as:
bInterval: 4
wMaxPacketSize: 1024
However, the same can be overridden through the module parameter interface.

The code is tested for HS and SS on a platform having DWC3 controller.

Signed-off-by: Amit Virdi 
---
 drivers/usb/gadget/f_loopback.c   |   3 +-
 drivers/usb/gadget/f_sourcesink.c | 519 --
 drivers/usb/gadget/g_zero.h   |  13 +-
 drivers/usb/gadget/zero.c |  21 ++
 4 files changed, 533 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c
index 4557cd0..bf04389 100644
--- a/drivers/usb/gadget/f_loopback.c
+++ b/drivers/usb/gadget/f_loopback.c
@@ -298,7 +298,8 @@ static void disable_loopback(struct f_loopback *loop)
struct usb_composite_dev*cdev;
 
cdev = loop->function.config->cdev;
-   disable_endpoints(cdev, loop->in_ep, loop->out_ep, NULL, NULL);
+   disable_endpoints(cdev, loop->in_ep, loop->out_ep, NULL, NULL, NULL,
+   NULL);
VDBG(cdev, "%s disabled\n", loop->function.name);
 }
 
diff --git a/drivers/usb/gadget/f_sourcesink.c 
b/drivers/usb/gadget/f_sourcesink.c
index d3cd52d..306de39 100644
--- a/drivers/usb/gadget/f_sourcesink.c
+++ b/drivers/usb/gadget/f_sourcesink.c
@@ -23,6 +23,13 @@
 #include "gadget_chips.h"
 #include "u_f.h"
 
+enum eptype {
+   EP_CONTROL = 0,
+   EP_BULK,
+   EP_ISOC,
+   EP_INTERRUPT,
+};
+
 /*
  * SOURCE/SINK FUNCTION ... a primary testing vehicle for USB peripheral
  * controller drivers.
@@ -55,6 +62,8 @@ struct f_sourcesink {
struct usb_ep   *out_ep;
struct usb_ep   *iso_in_ep;
struct usb_ep   *iso_out_ep;
+   struct usb_ep   *int_in_ep;
+   struct usb_ep   *int_out_ep;
int cur_alt;
 };
 
@@ -68,6 +77,10 @@ static unsigned isoc_interval;
 static unsigned isoc_maxpacket;
 static unsigned isoc_mult;
 static unsigned isoc_maxburst;
+static unsigned int_interval;
+static unsigned int_maxpacket;
+static unsigned int_mult;
+static unsigned int_maxburst;
 static unsigned buflen;
 
 /*-*/
@@ -92,6 +105,16 @@ static struct usb_interface_descriptor 
source_sink_intf_alt1 = {
/* .iInterface  = DYNAMIC */
 };
 
+static struct usb_interface_descriptor source_sink_intf_alt2 = {
+   .bLength =  USB_DT_INTERFACE_SIZE,
+   .bDescriptorType =  USB_DT_INTERFACE,
+
+   .bAlternateSetting =2,
+   .bNumEndpoints =6,
+   .bInterfaceClass =  USB_CLASS_VENDOR_SPEC,
+   /* .iInterface  = DYNAMIC */
+};
+
 /* full speed support: */
 
 static struct usb_endpoint_descriptor fs_source_desc = {
@@ -130,6 +153,26 @@ static struct usb_endpoint_descriptor fs_iso_sink_desc = {
.bInterval =4,
 };
 
+static struct usb_endpoint_descriptor fs_int_source_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_IN,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(1023),
+   .bInterval =4,
+};
+
+static struct usb_endpoint_descriptor fs_int_sink_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_OUT,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(1023),
+   .bInterval =4,
+};
+
 static struct usb_descriptor_header *fs_source_sink_descs[] = {
(struct usb_descriptor_header *) &source_sink_intf_alt0,
(struct usb_descriptor_header *) &fs_sink_desc,
@@ -140,6 +183,14 @@ static struct usb_descriptor_header 
*fs_source_sink_descs[] = {
(struct usb_descriptor_header *) &fs_source_desc,
(struct usb_descriptor_header *) &fs_iso_sink_desc,
(struct usb_descriptor_header *) &fs_iso_source_desc,
+   (struct usb_descriptor_header *) &source_sink_intf_alt2,
+#define FS_ALT_IFC_2_OFFSET8
+   (struct usb_descriptor_header *) &fs_sink_desc,
+   (struct usb_descriptor_header *) &fs_source_desc,
+   (struct usb_descriptor_header *) &fs_iso_sink_desc,
+   (struct usb_descriptor_header *) &fs_iso_source_desc,
+   (struct usb_descriptor_header *) &fs_int_sink_desc,
+   (struct usb_d

[Patch V1 2/2] usbtest: Add interrupt EP testcases

2014-02-24 Thread Amit Virdi
Two simple test cases for interrupt endpoints are added to the usbtest.c file.
These are simple non-queued interrupt IN and interrupt OUT transfers. Currently,
only gadget zero is capable of executing the interrupt EP test cases. However,
extending the same to other gadgets is extremely simple and can be done
on-demand.

This code has been tested only with gadget zero and care has been taken so as to
not break the existing functionality. However, if anyone can test with other
gadgets then that would be great!

Signed-off-by: Amit Virdi 
---
 drivers/usb/misc/usbtest.c | 112 +++--
 1 file changed, 97 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index f6568b5..8b96235 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -54,6 +54,7 @@ struct usbtest_info {
unsignedautoconf:1;
unsignedctrl_out:1;
unsignediso:1;  /* try iso in/out */
+   unsignedintr:1; /* try interrupt in/out */
int alt;
 };
 
@@ -70,7 +71,10 @@ struct usbtest_dev {
int out_pipe;
int in_iso_pipe;
int out_iso_pipe;
+   int in_int_pipe;
+   int out_int_pipe;
struct usb_endpoint_descriptor  *iso_in, *iso_out;
+   struct usb_endpoint_descriptor  *int_in, *int_out;
struct mutexlock;
 
 #define TBUF_SIZE  256
@@ -101,6 +105,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
struct usb_host_interface   *alt;
struct usb_host_endpoint*in, *out;
struct usb_host_endpoint*iso_in, *iso_out;
+   struct usb_host_endpoint*int_in, *int_out;
struct usb_device   *udev;
 
for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
@@ -108,6 +113,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
 
in = out = NULL;
iso_in = iso_out = NULL;
+   int_in = int_out = NULL;
alt = intf->altsetting + tmp;
 
if (override_alt >= 0 &&
@@ -124,6 +130,9 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
switch (usb_endpoint_type(&e->desc)) {
case USB_ENDPOINT_XFER_BULK:
break;
+   case USB_ENDPOINT_XFER_INT:
+   if (dev->info->intr)
+   goto try_intr;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
goto try_iso;
@@ -139,6 +148,14 @@ get_endpoints(struct usbtest_dev *dev, struct 
usb_interface *intf)
out = e;
}
continue;
+try_intr:
+   if (usb_endpoint_dir_in(&e->desc)) {
+   if (!int_in)
+   int_in = e;
+   } else {
+   if (!int_out)
+   int_out = e;
+   }
 try_iso:
if (usb_endpoint_dir_in(&e->desc)) {
if (!iso_in)
@@ -148,7 +165,7 @@ try_iso:
iso_out = e;
}
}
-   if ((in && out)  ||  iso_in || iso_out)
+   if ((in && out)  ||  iso_in || iso_out || int_in || int_out)
goto found;
}
return -EINVAL;
@@ -183,6 +200,20 @@ found:
iso_out->desc.bEndpointAddress
& USB_ENDPOINT_NUMBER_MASK);
}
+
+   if (int_in) {
+   dev->int_in = &int_in->desc;
+   dev->in_int_pipe = usb_rcvintpipe(udev,
+   int_in->desc.bEndpointAddress
+   & USB_ENDPOINT_NUMBER_MASK);
+   }
+
+   if (int_out) {
+   dev->int_out = &int_out->desc;
+   dev->out_int_pipe = usb_sndintpipe(udev,
+   int_out->desc.bEndpointAddress
+   & USB_ENDPOINT_NUMBER_MASK);
+   }
return 0;
 }
 
@@ -205,14 +236,22 @@ static struct urb *usbtest_alloc_urb(
int pipe,
unsigned long   bytes,
unsignedtransfer_flags,
-   unsignedoffset)
+   unsignedoffset,
+   u8

[Patch V1 0/2] usb: g_zero: Add support for interrupt EP

2014-02-24 Thread Amit Virdi
This patchset adds support for interrupt EP and the corresponding test cases to
gadget zero. The code has been rebased and tested on Kernel v3.14-rc3

RFC -> V1
 - Added support for configuring interrupt EP attributes from configfs interface

Amit Virdi (2):
  usb: gadget: zero: Add support for interrupt EP
  usbtest: Add interrupt EP testcases

 drivers/usb/gadget/f_loopback.c   |   3 +-
 drivers/usb/gadget/f_sourcesink.c | 519 --
 drivers/usb/gadget/g_zero.h   |  13 +-
 drivers/usb/gadget/zero.c |  21 ++
 drivers/usb/misc/usbtest.c| 112 ++--
 5 files changed, 630 insertions(+), 38 deletions(-)

-- 
1.8.0

--
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: [RFC 1/2] usb: gadget: zero: Add support for interrupt EP

2014-02-18 Thread Amit Virdi

Dear Andrezej,

On 2/10/2014 7:15 PM, Andrzej Pietrasiewicz wrote:

W dniu 10.02.2014 14:16, Amit Virdi pisze:

Interrupt endpoints behave quite similar to the bulk endpoints with the
difference that the endpoints expect data sending/reception request at
particular intervals till the whole data has not been transmitted.

The interrupt EP support is added to gadget zero. A new alternate setting (=2)
has been added. It has 6 endpoints (2-BULK, 2-ISOC, 2-INTERRUPT). The default
parameters are set as:
bInterval: 4
wMaxPacketSize: 1024
However, the same can be overridden through the module parameter interface.



The module parameter interface is available only in legacy mode,
that is using g_zero.ko. Both sourcesink and loopback now support
configfs.



Thanks for the enlightenment. I'll implement parameter passing through 
configfs.



The code is tested for HS and SS on a platform having DWC3 controller.

Signed-off-by: Amit Virdi 





+static unsigned int_interval;
+static unsigned int_maxpacket;
+static unsigned int_mult;
+static unsigned int_maxburst;


For these you need appropriate configfs attributes (files).

Below there is a typical way to create the attributes.

/*
   * "show" means to copy data from kernel to user;
   * you get the opts pointer and copy the relevant data to the page
   */
static ssize_t f_ss_opts_pattern_show(struct f_ss_opts *opts, char *page)
{
int result;

mutex_lock(&opts->lock);
result = sprintf(page, "%d", opts->pattern);
mutex_unlock(&opts->lock);

return result;
}

/*
   * "store" means to copy data from user to the kernel;
   * you take data from the page and interpret it;
   * if it is ok, you store it in the opts
   */
static ssize_t f_ss_opts_pattern_store(struct f_ss_opts *opts,
   const char *page, size_t len)
{
int ret;
u8 num;

mutex_lock(&opts->lock);
if (opts->refcnt) {
ret = -EBUSY;
goto end;
}

ret = kstrtou8(page, 0, &num);
if (ret)
goto end;

if (num != 0 && num != 1 && num != 2) {
ret = -EINVAL;
goto end;
}

opts->pattern = num;
ret = len;
end:
mutex_unlock(&opts->lock);
return ret;
}

static struct f_ss_opts_attribute f_ss_opts_pattern =
__CONFIGFS_ATTR(pattern, S_IRUGO | S_IWUSR,
f_ss_opts_pattern_show,
f_ss_opts_pattern_store);

/*
   * definitions of f_ss_opts_isoc_interval & co follow
   */

...


/*
   * hereyou should create your implementations of
   * f_ss_opts_int_interval_show/store & co
   */

/*
   *and then attach the attributes to the config item;
   * which translates to making new files appear in their
   * directory
   */
static struct configfs_attribute *ss_attrs[] = {
&f_ss_opts_pattern.attr,
&f_ss_opts_isoc_interval.attr,
&f_ss_opts_isoc_maxpacket.attr,
&f_ss_opts_isoc_mult.attr,
&f_ss_opts_isoc_maxburst.attr,
&f_ss_opts_bulk_buflen.attr,

/* HERE */
&f_ss_opts_isoc_interval.attr,

NULL,
};



Ok, I got it.

I'll incorporate your comments and send V1 in sometime.

Regards
Amit Virdi
--
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


[RFC 2/2] usbtest: Add interrupt EP testcases

2014-02-10 Thread Amit Virdi
Two simple test cases for interrupt endpoints are added to the usbtest.c file.
These are simple non-queued interrupt IN and interrupt OUT transfers. Currently,
only gadget zero is capable of executing the interrupt EP test cases. However,
extending the same to other gadgets is extremely simple and can be done
on-demand.

This code has been tested only with gadget zero and care has been taken so as to
not break the existing functionality. However, if anyone can test with other
gadgets then that would be great!

Signed-off-by: Amit Virdi 
---
 drivers/usb/misc/usbtest.c | 112 +++--
 1 file changed, 97 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index f6568b5..8b96235 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -54,6 +54,7 @@ struct usbtest_info {
unsignedautoconf:1;
unsignedctrl_out:1;
unsignediso:1;  /* try iso in/out */
+   unsignedintr:1; /* try interrupt in/out */
int alt;
 };
 
@@ -70,7 +71,10 @@ struct usbtest_dev {
int out_pipe;
int in_iso_pipe;
int out_iso_pipe;
+   int in_int_pipe;
+   int out_int_pipe;
struct usb_endpoint_descriptor  *iso_in, *iso_out;
+   struct usb_endpoint_descriptor  *int_in, *int_out;
struct mutexlock;
 
 #define TBUF_SIZE  256
@@ -101,6 +105,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
struct usb_host_interface   *alt;
struct usb_host_endpoint*in, *out;
struct usb_host_endpoint*iso_in, *iso_out;
+   struct usb_host_endpoint*int_in, *int_out;
struct usb_device   *udev;
 
for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
@@ -108,6 +113,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
 
in = out = NULL;
iso_in = iso_out = NULL;
+   int_in = int_out = NULL;
alt = intf->altsetting + tmp;
 
if (override_alt >= 0 &&
@@ -124,6 +130,9 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
switch (usb_endpoint_type(&e->desc)) {
case USB_ENDPOINT_XFER_BULK:
break;
+   case USB_ENDPOINT_XFER_INT:
+   if (dev->info->intr)
+   goto try_intr;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
goto try_iso;
@@ -139,6 +148,14 @@ get_endpoints(struct usbtest_dev *dev, struct 
usb_interface *intf)
out = e;
}
continue;
+try_intr:
+   if (usb_endpoint_dir_in(&e->desc)) {
+   if (!int_in)
+   int_in = e;
+   } else {
+   if (!int_out)
+   int_out = e;
+   }
 try_iso:
if (usb_endpoint_dir_in(&e->desc)) {
if (!iso_in)
@@ -148,7 +165,7 @@ try_iso:
iso_out = e;
}
}
-   if ((in && out)  ||  iso_in || iso_out)
+   if ((in && out)  ||  iso_in || iso_out || int_in || int_out)
goto found;
}
return -EINVAL;
@@ -183,6 +200,20 @@ found:
iso_out->desc.bEndpointAddress
& USB_ENDPOINT_NUMBER_MASK);
}
+
+   if (int_in) {
+   dev->int_in = &int_in->desc;
+   dev->in_int_pipe = usb_rcvintpipe(udev,
+   int_in->desc.bEndpointAddress
+   & USB_ENDPOINT_NUMBER_MASK);
+   }
+
+   if (int_out) {
+   dev->int_out = &int_out->desc;
+   dev->out_int_pipe = usb_sndintpipe(udev,
+   int_out->desc.bEndpointAddress
+   & USB_ENDPOINT_NUMBER_MASK);
+   }
return 0;
 }
 
@@ -205,14 +236,22 @@ static struct urb *usbtest_alloc_urb(
int pipe,
unsigned long   bytes,
unsignedtransfer_flags,
-   unsignedoffset)
+   unsignedoffset,
+   u8

[RFC 0/2] usb: g_zero: Add support for interrupt EP

2014-02-10 Thread Amit Virdi
This patchset adds interrupt EP support to gadget zero and subsequently adds
corresponding test cases to usbtest.c

Amit Virdi (2):
  usb: gadget: zero: Add support for interrupt EP
  usbtest: Add interrupt EP testcases

 drivers/usb/gadget/f_loopback.c   |   3 +-
 drivers/usb/gadget/f_sourcesink.c | 339 +++---
 drivers/usb/gadget/g_zero.h   |  13 +-
 drivers/usb/gadget/zero.c |  21 +++
 drivers/usb/misc/usbtest.c| 112 +++--
 5 files changed, 450 insertions(+), 38 deletions(-)

-- 
1.8.0

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


[RFC 1/2] usb: gadget: zero: Add support for interrupt EP

2014-02-10 Thread Amit Virdi
Interrupt endpoints behave quite similar to the bulk endpoints with the
difference that the endpoints expect data sending/reception request at
particular intervals till the whole data has not been transmitted.

The interrupt EP support is added to gadget zero. A new alternate setting (=2)
has been added. It has 6 endpoints (2-BULK, 2-ISOC, 2-INTERRUPT). The default
parameters are set as:
bInterval: 4
wMaxPacketSize: 1024
However, the same can be overridden through the module parameter interface.

The code is tested for HS and SS on a platform having DWC3 controller.

Signed-off-by: Amit Virdi 
---
 drivers/usb/gadget/f_loopback.c   |   3 +-
 drivers/usb/gadget/f_sourcesink.c | 339 +++---
 drivers/usb/gadget/g_zero.h   |  13 +-
 drivers/usb/gadget/zero.c |  21 +++
 4 files changed, 353 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c
index 4557cd0..bf04389 100644
--- a/drivers/usb/gadget/f_loopback.c
+++ b/drivers/usb/gadget/f_loopback.c
@@ -298,7 +298,8 @@ static void disable_loopback(struct f_loopback *loop)
struct usb_composite_dev*cdev;
 
cdev = loop->function.config->cdev;
-   disable_endpoints(cdev, loop->in_ep, loop->out_ep, NULL, NULL);
+   disable_endpoints(cdev, loop->in_ep, loop->out_ep, NULL, NULL, NULL,
+   NULL);
VDBG(cdev, "%s disabled\n", loop->function.name);
 }
 
diff --git a/drivers/usb/gadget/f_sourcesink.c 
b/drivers/usb/gadget/f_sourcesink.c
index d3cd52d..89e 100644
--- a/drivers/usb/gadget/f_sourcesink.c
+++ b/drivers/usb/gadget/f_sourcesink.c
@@ -23,6 +23,13 @@
 #include "gadget_chips.h"
 #include "u_f.h"
 
+enum eptype {
+   EP_CONTROL = 0,
+   EP_BULK,
+   EP_ISOC,
+   EP_INTERRUPT,
+};
+
 /*
  * SOURCE/SINK FUNCTION ... a primary testing vehicle for USB peripheral
  * controller drivers.
@@ -55,6 +62,8 @@ struct f_sourcesink {
struct usb_ep   *out_ep;
struct usb_ep   *iso_in_ep;
struct usb_ep   *iso_out_ep;
+   struct usb_ep   *int_in_ep;
+   struct usb_ep   *int_out_ep;
int cur_alt;
 };
 
@@ -68,6 +77,10 @@ static unsigned isoc_interval;
 static unsigned isoc_maxpacket;
 static unsigned isoc_mult;
 static unsigned isoc_maxburst;
+static unsigned int_interval;
+static unsigned int_maxpacket;
+static unsigned int_mult;
+static unsigned int_maxburst;
 static unsigned buflen;
 
 /*-*/
@@ -92,6 +105,16 @@ static struct usb_interface_descriptor 
source_sink_intf_alt1 = {
/* .iInterface  = DYNAMIC */
 };
 
+static struct usb_interface_descriptor source_sink_intf_alt2 = {
+   .bLength =  USB_DT_INTERFACE_SIZE,
+   .bDescriptorType =  USB_DT_INTERFACE,
+
+   .bAlternateSetting =2,
+   .bNumEndpoints =6,
+   .bInterfaceClass =  USB_CLASS_VENDOR_SPEC,
+   /* .iInterface  = DYNAMIC */
+};
+
 /* full speed support: */
 
 static struct usb_endpoint_descriptor fs_source_desc = {
@@ -130,6 +153,26 @@ static struct usb_endpoint_descriptor fs_iso_sink_desc = {
.bInterval =4,
 };
 
+static struct usb_endpoint_descriptor fs_int_source_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_IN,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(1023),
+   .bInterval =4,
+};
+
+static struct usb_endpoint_descriptor fs_int_sink_desc = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_OUT,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(1023),
+   .bInterval =4,
+};
+
 static struct usb_descriptor_header *fs_source_sink_descs[] = {
(struct usb_descriptor_header *) &source_sink_intf_alt0,
(struct usb_descriptor_header *) &fs_sink_desc,
@@ -140,6 +183,14 @@ static struct usb_descriptor_header 
*fs_source_sink_descs[] = {
(struct usb_descriptor_header *) &fs_source_desc,
(struct usb_descriptor_header *) &fs_iso_sink_desc,
(struct usb_descriptor_header *) &fs_iso_source_desc,
+   (struct usb_descriptor_header *) &source_sink_intf_alt2,
+#define FS_ALT_IFC_2_OFFSET8
+   (struct usb_descriptor_header *) &fs_sink_desc,
+   (struct usb_descriptor_header *) &fs_source_desc,
+   (struct usb_descriptor_header *) &fs_iso_sink_desc,
+   (struct usb_descriptor_header *) &fs_iso_source_desc,
+   (struct usb_descriptor_header *) &fs_int_sink_desc,
+   (struct usb_d

Re: Testing glue layer for DWC3 device controller

2013-11-18 Thread Amit Virdi

Dear Felipe,

On 11/15/2013 11:04 PM, Felipe Balbi wrote:

please send your glue layer sources and any logs you might have. PHY
drivers are loading at subsys_initcall and gadget drivers are loading at
module_init(). I don't see how that could happen :-s


Thanks for your inputs. I was using module_init for my PHY driver. 
Although DWC3 probe was deferred until PHY was registered, udc_start was 
not called and, hence, the device didn't change state to "running".


Using subsys_initcall, I see that the PHY is probed first and then the 
gadget driver is loaded thereby, resolving the problem.


Regards
Amit Virdi
--
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


Testing glue layer for DWC3 device controller

2013-11-15 Thread Amit Virdi

Hi All,

I'm new to this USB driver development so posting this email. Currently, 
I'm using Kernel version 3.12-rc5


In my SoC, there's Synopsys' DWC3 USB controller configured in device 
mode. This is integrated with our internal USB3.0 PHY and USB 2.0 PHY 
taken from synopsys. I'm done with the initial coding and now I want to 
test the glue layer logic implemented. I have compiled and statically 
linked the drivers.


When I boot my kernel, I see that the PHY and the DWC3 driver is probed 
successfully. However, the device doesn't enumerate. I debugged this and 
found that the device specific registers are not configured at all. More 
debugging followed and I figured out that the probe for zero gadget is 
called before the phy and the device is probed.


So, how can I test the driver by *not* modularizing the phy and the 
controller drivers.


I would be thankful for any help.

Regards
Amit Virdi
--
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