Mailing List Archive

[PATCH] fusb300_udc: modify stall clear and idma reset procedure
From: Yuan-Hsin Chen <yuanlmm@gmail.com>

Due to fusb300 controller modification, stall clear procedure should be
modified consistantly. This patch also fixes software bugs: only
enter IDMA_RESET when the condition matched and disable corresponding
PRD interrupt in IDMA_RESET.


Signed-off-by: Yuan-Hsin Chen <yhchen@faraday-tech.com>
---
drivers/usb/gadget/fusb300_udc.c | 9 ++++++---
drivers/usb/gadget/fusb300_udc.h | 2 +-
2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/fusb300_udc.c b/drivers/usb/gadget/fusb300_udc.c
index 72cd5e6..109cab1 100644
--- a/drivers/usb/gadget/fusb300_udc.c
+++ b/drivers/usb/gadget/fusb300_udc.c
@@ -394,7 +394,7 @@ static void fusb300_clear_epnstall(struct fusb300 *fusb300, u8 ep)

if (reg & FUSB300_EPSET0_STL) {
printk(KERN_DEBUG "EP%d stall... Clear!!\n", ep);
- reg &= ~FUSB300_EPSET0_STL;
+ reg |= FUSB300_EPSET0_STL_CLR;
iowrite32(reg, fusb300->reg + FUSB300_OFFSET_EPSET0(ep));
}
}
@@ -930,9 +930,12 @@ static void fusb300_wait_idma_finished(struct fusb300_ep *ep)

fusb300_clear_int(ep->fusb300, FUSB300_OFFSET_IGR0,
FUSB300_IGR0_EPn_PRD_INT(ep->epnum));
+ return;
+
IDMA_RESET:
- fusb300_clear_int(ep->fusb300, FUSB300_OFFSET_IGER0,
- FUSB300_IGER0_EEPn_PRD_INT(ep->epnum));
+ reg = ioread32(ep->fusb300->reg + FUSB300_OFFSET_IGER0);
+ reg &= ~FUSB300_IGER0_EEPn_PRD_INT(ep->epnum);
+ iowrite32(reg, ep->fusb300->reg + FUSB300_OFFSET_IGER0);
}

static void fusb300_set_idma(struct fusb300_ep *ep,
diff --git a/drivers/usb/gadget/fusb300_udc.h b/drivers/usb/gadget/fusb300_udc.h
index 542cd83..ccae1b5 100644
--- a/drivers/usb/gadget/fusb300_udc.h
+++ b/drivers/usb/gadget/fusb300_udc.h
@@ -111,8 +111,8 @@
/*
* * EPn Setting 0 (EPn_SET0, offset = 020H+(n-1)*30H, n=1~15 )
* */
+#define FUSB300_EPSET0_STL_CLR (1 << 3)
#define FUSB300_EPSET0_CLRSEQNUM (1 << 2)
-#define FUSB300_EPSET0_EPn_TX0BYTE (1 << 1)
#define FUSB300_EPSET0_STL (1 << 0)

/*
--
1.7.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fusb300_udc: modify stall clear and idma reset procedure [ In reply to ]
Hi,

On Fri, Feb 22, 2013 at 07:09:39AM +0000, Yuan-Hsin Chen wrote:
> From: Yuan-Hsin Chen <yuanlmm@gmail.com>
>
> Due to fusb300 controller modification, stall clear procedure should be
> modified consistantly. This patch also fixes software bugs: only
> enter IDMA_RESET when the condition matched and disable corresponding
> PRD interrupt in IDMA_RESET.
>
>
> Signed-off-by: Yuan-Hsin Chen <yhchen@faraday-tech.com>

I have been trying to understand if this is a bug fix or if it can wait
until v3.10.

Also, according to your commit log, it seems like this patch is doing
more than one thing, which is really frowned upon. Please split it up
into separate logically self-contained changes. While doing that, make
sure that your commit log explains the problem and the solution very
well. Saying that because of a controller modification you need to
change stall clear consistently isn't enough.

Don't forget to mention how and with which platforms you tested.

--
balbi
Re: [PATCH] fusb300_udc: modify stall clear and idma reset procedure [ In reply to ]
Hi,

On Wed, Feb 27, 2013 at 1:59 AM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Fri, Feb 22, 2013 at 07:09:39AM +0000, Yuan-Hsin Chen wrote:
>> From: Yuan-Hsin Chen <yuanlmm@gmail.com>
>>
>> Due to fusb300 controller modification, stall clear procedure should be
>> modified consistantly. This patch also fixes software bugs: only
>> enter IDMA_RESET when the condition matched and disable corresponding
>> PRD interrupt in IDMA_RESET.
>>
>>
>> Signed-off-by: Yuan-Hsin Chen <yhchen@faraday-tech.com>
>
> I have been trying to understand if this is a bug fix or if it can wait
> until v3.10.

There are a software bug fix and two corresponding fixes to hw register
modification in the patch. Should it wait until v3.10?
>
> Also, according to your commit log, it seems like this patch is doing
> more than one thing, which is really frowned upon. Please split it up
> into separate logically self-contained changes. While doing that, make
> sure that your commit log explains the problem and the solution very
> well. Saying that because of a controller modification you need to
> change stall clear consistently isn't enough.

I will split it up and explain more clearly.
Thanks.
>
> Don't forget to mention how and with which platforms you tested.
>
> --
> balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/