[PATCH v3] USB: gadget: s3c-hsotg: add isochronous transfers support

[PATCH v3] USB: gadget: s3c-hsotg: add isochronous transfers support

[Robert Baldyga]

This patch adds isochronous transfer support. It adds few modifications:
- Modify s3c_hsotg_epint() function. Some interrupts are ignored for
  isochronous endpoints, (e.g. INTknTXFEmpMsk) becouse isochronous request is
  always transfered in single transaction, which ends with XferCompl interrupt.
- Add Odd/Even microframe toggle to allow data transfering in each microframe
  in s3c_hsotg_epint() function.
- Fix s3c_hsotg_ep_enable() function by supporting isochronous endpoint type.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---

Hello,
This is third version of patch adding isochronous transfers support in
s3c-hsotg driver. From last version I have separated one bugfix for
s3c_hsotg_set_ep_maxpacket() function into another patch. I also have splitted
modifications into two patches: this patch, adding isochronous support base, and
the second patch adding support for multiple transfers per microframe, which will
be sent soon.

Best regards
Robert Baldyga
Samsung R&D Institute Poland

Changelog:
v3:
- separated bugfix into another patch
- moved additional functionality into another patch

v2: https://lkml.org/lkml/2013/9/24/154
- moved bugfix affecting to the other features to separated patch
- changed conditions order in request length checking in s3c_hsotg_start_req
  function, as Bartlomiej Zolnierkiewicz suggested
- fixed typos

v1: https://lkml.org/lkml/2013/9/23/72
- initial proposal


 drivers/usb/gadget/s3c-hsotg.c |   27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 139122a..e3ffbc0 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -83,9 +83,11 @@ struct s3c_hsotg_req;
  * @dir_in: Set to true if this endpoint is of the IN direction, which
  *	    means that it is sending data to the Host.
  * @index: The index for the endpoint registers.
+ * @interval - Interval for periodic endpoints
  * @name: The name array passed to the USB core.
  * @halted: Set if the endpoint has been halted.
  * @periodic: Set if this is a periodic ep, such as Interrupt
+ * @isochronous: Set if this is a isochronous ep
  * @sent_zlp: Set if we've sent a zero-length packet.
  * @total_data: The total number of data bytes done.
  * @fifo_size: The size of the FIFO (for periodic IN endpoints)
@@ -121,9 +123,11 @@ struct s3c_hsotg_ep {
 
 	unsigned char		dir_in;
 	unsigned char		index;
+	unsigned char		interval;
 
 	unsigned int		halted:1;
 	unsigned int		periodic:1;
+	unsigned int		isochronous:1;
 	unsigned int		sent_zlp:1;
 
 	char			name[10];
@@ -1915,8 +1919,10 @@ static void s3c_hsotg_epint(struct s3c_hsotg *hsotg, unsigned int idx,
 	u32 epctl_reg = dir_in ? DIEPCTL(idx) : DOEPCTL(idx);
 	u32 epsiz_reg = dir_in ? DIEPTSIZ(idx) : DOEPTSIZ(idx);
 	u32 ints;
+	u32 ctrl;
 
 	ints = readl(hsotg->regs + epint_reg);
+	ctrl = readl(hsotg->regs + epctl_reg);
 
 	/* Clear endpoint interrupts */
 	writel(ints, hsotg->regs + epint_reg);
@@ -1925,6 +1931,14 @@ static void s3c_hsotg_epint(struct s3c_hsotg *hsotg, unsigned int idx,
 		__func__, idx, dir_in ? "in" : "out", ints);
 
 	if (ints & DxEPINT_XferCompl) {
+		if (hs_ep->isochronous && hs_ep->interval == 1) {
+			if (ctrl & DxEPCTL_EOFrNum)
+				ctrl |= DxEPCTL_SetEvenFr;
+			else
+				ctrl |= DxEPCTL_SetOddFr;
+			writel(ctrl, hsotg->regs + epctl_reg);
+		}
+
 		dev_dbg(hsotg->dev,
 			"%s: XferCompl: DxEPCTL=0x%08x, DxEPTSIZ=%08x\n",
 			__func__, readl(hsotg->regs + epctl_reg),
@@ -1991,7 +2005,7 @@ static void s3c_hsotg_epint(struct s3c_hsotg *hsotg, unsigned int idx,
 	if (ints & DxEPINT_Back2BackSetup)
 		dev_dbg(hsotg->dev, "%s: B2BSetup/INEPNakEff\n", __func__);
 
-	if (dir_in) {
+	if (dir_in && !hs_ep->isochronous) {
 		/* not sure if this is important, but we'll clear it anyway */
 		if (ints & DIEPMSK_INTknTXFEmpMsk) {
 			dev_dbg(hsotg->dev, "%s: ep%d: INTknTXFEmpMsk\n",
@@ -2616,14 +2630,19 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
 	s3c_hsotg_set_ep_maxpacket(hsotg, hs_ep->index, mps);
 
 	/* default, set to non-periodic */
+	hs_ep->isochronous = 0;
 	hs_ep->periodic = 0;
 	hs_ep->halted = 0;
+	hs_ep->interval = desc->bInterval;
 
 	switch (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) {
 	case USB_ENDPOINT_XFER_ISOC:
-		dev_err(hsotg->dev, "no current ISOC support\n");
-		ret = -EINVAL;
-		goto out;
+		epctrl |= DxEPCTL_EPType_Iso;
+		epctrl |= DxEPCTL_SetEvenFr;
+		hs_ep->isochronous = 1;
+		if (dir_in)
+			hs_ep->periodic = 1;
+		break;
 
 	case USB_ENDPOINT_XFER_BULK:
 		epctrl |= DxEPCTL_EPType_Bulk;
-- 
1.7.9.5

Re: [PATCH v3] USB: gadget: s3c-hsotg: add isochronous transfers support

[Felipe Balbi]

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

Hi,

On Wed, Oct 09, 2013 at 08:41:57AM +0200, Robert Baldyga wrote:
> @@ -2616,14 +2630,19 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
>  	s3c_hsotg_set_ep_maxpacket(hsotg, hs_ep->index, mps);
>  
>  	/* default, set to non-periodic */
> +	hs_ep->isochronous = 0;
>  	hs_ep->periodic = 0;
>  	hs_ep->halted = 0;

this hunk doesn't apply because of this hs_ep->halted line, which patch
does this depend on ?

-- 
balbi

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

Re: [PATCH v3] USB: gadget: s3c-hsotg: add isochronous transfers support

[Robert Baldyga]

Hello,

On 10/10/2013 06:04 PM, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Oct 09, 2013 at 08:41:57AM +0200, Robert Baldyga wrote:
>> @@ -2616,14 +2630,19 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
>>  	s3c_hsotg_set_ep_maxpacket(hsotg, hs_ep->index, mps);
>>  
>>  	/* default, set to non-periodic */
>> +	hs_ep->isochronous = 0;
>>  	hs_ep->periodic = 0;
>>  	hs_ep->halted = 0;
> 
> this hunk doesn't apply because of this hs_ep->halted line, which patch
> does this depend on ?
> 
it depends on http://www.spinics.net/lists/linux-usb/msg94025.html

Best regards
Robert Baldyga
Samsung R&D Institute Poland

Re: [PATCH v3] USB: gadget: s3c-hsotg: add isochronous transfers support

[Felipe Balbi]

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

Hi,

On Fri, Oct 11, 2013 at 08:23:35AM +0200, Robert Baldyga wrote:
> Hello,
> 
> On 10/10/2013 06:04 PM, Felipe Balbi wrote:
> > Hi,
> > 
> > On Wed, Oct 09, 2013 at 08:41:57AM +0200, Robert Baldyga wrote:
> >> @@ -2616,14 +2630,19 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
> >>  	s3c_hsotg_set_ep_maxpacket(hsotg, hs_ep->index, mps);
> >>  
> >>  	/* default, set to non-periodic */
> >> +	hs_ep->isochronous = 0;
> >>  	hs_ep->periodic = 0;
> >>  	hs_ep->halted = 0;
> > 
> > this hunk doesn't apply because of this hs_ep->halted line, which patch
> > does this depend on ?
> > 
> it depends on http://www.spinics.net/lists/linux-usb/msg94025.html

I'll fix it up by hand this time, but new features shouldn't (ideally)
directly depend on fixes which you just wrote.

cheers

-- 
balbi

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