diff mbox

[U-Boot,1/2] usb: gadget: fotg210: add w1c interrupt status support

Message ID 1387351489-2008-2-git-send-email-dantesu@gmail.com
State Superseded
Delegated to: Marek Vasut
Headers show

Commit Message

Kuo-Jung Su Dec. 18, 2013, 7:24 a.m. UTC 
From: Kuo-Jung Su <dantesu@faraday-tech.com>

Since hardware revision 1.11.0, the following interrupt status registers
are now write-1-clear (w1c):

1. Interrupt Source Group 0 Register (0x144) (EP0 Abort: BIT5)
2. Interrupt Source Group 2 Register (0x14C) (All bits)

Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
CC: Marek Vasut <marex@denx.de>
---
 drivers/usb/gadget/fotg210.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Marek Vasut Dec. 18, 2013, 2:54 p.m. UTC | #1 
On Wednesday, December 18, 2013 at 08:24:48 AM, Kuo-Jung Su wrote:
> From: Kuo-Jung Su <dantesu@faraday-tech.com>
> 
> Since hardware revision 1.11.0, the following interrupt status registers
> are now write-1-clear (w1c):

What did they look like before ?

> 1. Interrupt Source Group 0 Register (0x144) (EP0 Abort: BIT5)
> 2. Interrupt Source Group 2 Register (0x14C) (All bits)
> 
> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
> CC: Marek Vasut <marex@denx.de>
> ---
>  drivers/usb/gadget/fotg210.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/fotg210.c b/drivers/usb/gadget/fotg210.c
> index 6e19db1..e3a61cc 100644
> --- a/drivers/usb/gadget/fotg210.c
> +++ b/drivers/usb/gadget/fotg210.c
> @@ -847,8 +847,11 @@ int usb_gadget_handle_interrupts(void)
>  	/* CX interrupts */
>  	if (gisr & GISR_GRP0) {
>  		st = readl(&regs->gisr0);
> +#ifdef CONFIG_USB_GADGET_FOTG210_ISRW1C

Can we not get rid of this ifdef somehow please ? Like detect the revision on-
the-fly and handle the bit accordingly or such ?

Best regards,
Marek Vasut
Kuo-Jung Su Dec. 19, 2013, 12:54 a.m. UTC | #2 
2013/12/18 Marek Vasut <marex@denx.de>:
> On Wednesday, December 18, 2013 at 08:24:48 AM, Kuo-Jung Su wrote:
>> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>>
>> Since hardware revision 1.11.0, the following interrupt status registers
>> are now write-1-clear (w1c):
>
> What did they look like before ?
>

They were r/w registers. i.e., software must write a zero to clear the status.
I'll add the comment in next version.

>> 1. Interrupt Source Group 0 Register (0x144) (EP0 Abort: BIT5)
>> 2. Interrupt Source Group 2 Register (0x14C) (All bits)
>>
>> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
>> CC: Marek Vasut <marex@denx.de>
>> ---
>>  drivers/usb/gadget/fotg210.c |   10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/fotg210.c b/drivers/usb/gadget/fotg210.c
>> index 6e19db1..e3a61cc 100644
>> --- a/drivers/usb/gadget/fotg210.c
>> +++ b/drivers/usb/gadget/fotg210.c
>> @@ -847,8 +847,11 @@ int usb_gadget_handle_interrupts(void)
>>       /* CX interrupts */
>>       if (gisr & GISR_GRP0) {
>>               st = readl(&regs->gisr0);
>> +#ifdef CONFIG_USB_GADGET_FOTG210_ISRW1C
>
> Can we not get rid of this ifdef somehow please ? Like detect the revision on-
> the-fly and handle the bit accordingly or such ?
>

Unfortunately there is no revision id register in this hardware, so I
have to do it manually.

> Best regards,
> Marek Vasut
Marek Vasut Dec. 19, 2013, 1:53 a.m. UTC | #3 
On Thursday, December 19, 2013 at 01:54:56 AM, Kuo-Jung Su wrote:
> 2013/12/18 Marek Vasut <marex@denx.de>:
> > On Wednesday, December 18, 2013 at 08:24:48 AM, Kuo-Jung Su wrote:
> >> From: Kuo-Jung Su <dantesu@faraday-tech.com>
> >> 
> >> Since hardware revision 1.11.0, the following interrupt status registers
> > 
> >> are now write-1-clear (w1c):
> > What did they look like before ?
> 
> They were r/w registers. i.e., software must write a zero to clear the
> status. I'll add the comment in next version.

OK.

> >> 1. Interrupt Source Group 0 Register (0x144) (EP0 Abort: BIT5)
> >> 2. Interrupt Source Group 2 Register (0x14C) (All bits)
> >> 
> >> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
> >> CC: Marek Vasut <marex@denx.de>
> >> ---
> >> 
> >>  drivers/usb/gadget/fotg210.c |   10 ++++++++--
> >>  1 file changed, 8 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/usb/gadget/fotg210.c b/drivers/usb/gadget/fotg210.c
> >> index 6e19db1..e3a61cc 100644
> >> --- a/drivers/usb/gadget/fotg210.c
> >> +++ b/drivers/usb/gadget/fotg210.c
> >> @@ -847,8 +847,11 @@ int usb_gadget_handle_interrupts(void)
> >> 
> >>       /* CX interrupts */
> >>       if (gisr & GISR_GRP0) {
> >>       
> >>               st = readl(&regs->gisr0);
> >> 
> >> +#ifdef CONFIG_USB_GADGET_FOTG210_ISRW1C
> > 
> > Can we not get rid of this ifdef somehow please ? Like detect the
> > revision on- the-fly and handle the bit accordingly or such ?
> 
> Unfortunately there is no revision id register in this hardware, so I
> have to do it manually.

So what would happen if you write 1, then write 0 into them ? ;-) Won't that 
handle both cases? Writing one will make sure the clean them on new hardware, 
writing zero afterwards will clean them on old hardware.

Best regards,
Marek Vasut
Kuo-Jung Su Dec. 19, 2013, 7:10 a.m. UTC | #4 
2013/12/19 Marek Vasut <marex@denx.de>:
> On Thursday, December 19, 2013 at 01:54:56 AM, Kuo-Jung Su wrote:
>> 2013/12/18 Marek Vasut <marex@denx.de>:
>> > On Wednesday, December 18, 2013 at 08:24:48 AM, Kuo-Jung Su wrote:
>> >> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>> >>
>> >> Since hardware revision 1.11.0, the following interrupt status registers
>> >
>> >> are now write-1-clear (w1c):
>> > What did they look like before ?
>>
>> They were r/w registers. i.e., software must write a zero to clear the
>> status. I'll add the comment in next version.
>
> OK.
>
>> >> 1. Interrupt Source Group 0 Register (0x144) (EP0 Abort: BIT5)
>> >> 2. Interrupt Source Group 2 Register (0x14C) (All bits)
>> >>
>> >> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
>> >> CC: Marek Vasut <marex@denx.de>
>> >> ---
>> >>
>> >>  drivers/usb/gadget/fotg210.c |   10 ++++++++--
>> >>  1 file changed, 8 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/drivers/usb/gadget/fotg210.c b/drivers/usb/gadget/fotg210.c
>> >> index 6e19db1..e3a61cc 100644
>> >> --- a/drivers/usb/gadget/fotg210.c
>> >> +++ b/drivers/usb/gadget/fotg210.c
>> >> @@ -847,8 +847,11 @@ int usb_gadget_handle_interrupts(void)
>> >>
>> >>       /* CX interrupts */
>> >>       if (gisr & GISR_GRP0) {
>> >>
>> >>               st = readl(&regs->gisr0);
>> >>
>> >> +#ifdef CONFIG_USB_GADGET_FOTG210_ISRW1C
>> >
>> > Can we not get rid of this ifdef somehow please ? Like detect the
>> > revision on- the-fly and handle the bit accordingly or such ?
>>
>> Unfortunately there is no revision id register in this hardware, so I
>> have to do it manually.
>
> So what would happen if you write 1, then write 0 into them ? ;-) Won't that
> handle both cases? Writing one will make sure the clean them on new hardware,
> writing zero afterwards will clean them on old hardware.
>

Good idea! I think it should work just fine.
I'll make sure if does work, and then prepare for the new patch.

> Best regards,
> Marek Vasut
Marek Vasut Dec. 19, 2013, 7:17 a.m. UTC | #5 
On Thursday, December 19, 2013 at 08:10:05 AM, Kuo-Jung Su wrote:
> 2013/12/19 Marek Vasut <marex@denx.de>:
> > On Thursday, December 19, 2013 at 01:54:56 AM, Kuo-Jung Su wrote:
> >> 2013/12/18 Marek Vasut <marex@denx.de>:
> >> > On Wednesday, December 18, 2013 at 08:24:48 AM, Kuo-Jung Su wrote:
> >> >> From: Kuo-Jung Su <dantesu@faraday-tech.com>
> >> >> 
> >> >> Since hardware revision 1.11.0, the following interrupt status
> >> >> registers
> >> > 
> >> >> are now write-1-clear (w1c):
> >> > What did they look like before ?
> >> 
> >> They were r/w registers. i.e., software must write a zero to clear the
> >> status. I'll add the comment in next version.
> > 
> > OK.
> > 
> >> >> 1. Interrupt Source Group 0 Register (0x144) (EP0 Abort: BIT5)
> >> >> 2. Interrupt Source Group 2 Register (0x14C) (All bits)
> >> >> 
> >> >> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
> >> >> CC: Marek Vasut <marex@denx.de>
> >> >> ---
> >> >> 
> >> >>  drivers/usb/gadget/fotg210.c |   10 ++++++++--
> >> >>  1 file changed, 8 insertions(+), 2 deletions(-)
> >> >> 
> >> >> diff --git a/drivers/usb/gadget/fotg210.c
> >> >> b/drivers/usb/gadget/fotg210.c index 6e19db1..e3a61cc 100644
> >> >> --- a/drivers/usb/gadget/fotg210.c
> >> >> +++ b/drivers/usb/gadget/fotg210.c
> >> >> @@ -847,8 +847,11 @@ int usb_gadget_handle_interrupts(void)
> >> >> 
> >> >>       /* CX interrupts */
> >> >>       if (gisr & GISR_GRP0) {
> >> >>       
> >> >>               st = readl(&regs->gisr0);
> >> >> 
> >> >> +#ifdef CONFIG_USB_GADGET_FOTG210_ISRW1C
> >> > 
> >> > Can we not get rid of this ifdef somehow please ? Like detect the
> >> > revision on- the-fly and handle the bit accordingly or such ?
> >> 
> >> Unfortunately there is no revision id register in this hardware, so I
> >> have to do it manually.
> > 
> > So what would happen if you write 1, then write 0 into them ? ;-) Won't
> > that handle both cases? Writing one will make sure the clean them on new
> > hardware, writing zero afterwards will clean them on old hardware.
> 
> Good idea! I think it should work just fine.
> I'll make sure if does work, and then prepare for the new patch.

Thanks :)

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/drivers/usb/gadget/fotg210.c b/drivers/usb/gadget/fotg210.c
index 6e19db1..e3a61cc 100644
--- a/drivers/usb/gadget/fotg210.c
+++ b/drivers/usb/gadget/fotg210.c
@@ -847,8 +847,11 @@  int usb_gadget_handle_interrupts(void)
 	/* CX interrupts */
 	if (gisr & GISR_GRP0) {
 		st = readl(&regs->gisr0);
+#ifdef CONFIG_USB_GADGET_FOTG210_ISRW1C
+		writel(st & GISR0_CXABORT, &regs->gisr0);
+#else
 		writel(0, &regs->gisr0);
-
+#endif
 		if (st & GISR0_CXERR)
 			printf("fotg210: cmd error\n");
 
@@ -873,8 +876,11 @@  int usb_gadget_handle_interrupts(void)
 	/* Device Status Interrupts */
 	if (gisr & GISR_GRP2) {
 		st = readl(&regs->gisr2);
+#ifdef CONFIG_USB_GADGET_FOTG210_ISRW1C
+		writel(st, &regs->gisr2);
+#else
 		writel(0, &regs->gisr2);
-
+#endif
 		if (st & GISR2_RESET)
 			printf("fotg210: reset by host\n");
 		else if (st & GISR2_SUSPEND)