Re: [Qemu-devel] Bug #757654: UHCI fails to signal stall response patch

2011-05-12 Thread Gerd Hoffmann

On 05/11/11 13:33, Jan Vesely wrote:

glad I could help

the original bug report is here: https://bugs.launchpad.net/qemu/+bug/757654
should I update it? or will it be updated when the patch reaches master?


Better wait until the stuff hits master, that is less confusing and you 
can also add the git commit hash then.


cheers,
  Gerd




Re: [Qemu-devel] Bug #757654: UHCI fails to signal stall response patch

2011-05-11 Thread Jan Vesely
glad I could help

the original bug report is here: https://bugs.launchpad.net/qemu/+bug/757654
should I update it? or will it be updated when the patch reaches master?

j.

On Mon, May 9, 2011 at 1:43 PM, Gerd Hoffmann kra...@redhat.com wrote:
 On 05/09/11 12:16, Jan Vesely wrote:

 UHCI host controller status register indicates error and
 an interrupt is triggered on BABBLE and STALL errors.

 Queued up.

 thanks,
  Gerd





Re: [Qemu-devel] Bug #757654: UHCI fails to signal stall response patch

2011-05-09 Thread Gerd Hoffmann

On 05/06/11 10:07, Jan Vesely wrote:

hi,
using the modified (single line) patch only works half-way, it sets
the value in status register (guess that's what that line does :))
but hw interrupt is not generated.

I tried adding uhci_update_irq and this patch:



+s-status |= UHCI_STS_USBERR;
+uhci_update_irq(s);



works.


Good.  Can you create a patch with a changelog entry  signed-off 
please?  I'll go queue it up then.


thanks,
  Gerd




Re: [Qemu-devel] Bug #757654: UHCI fails to signal stall response patch

2011-05-09 Thread Jan Vesely
UHCI host controller status register indicates error and
an interrupt is triggered on BABBLE and STALL errors.

Signed-off-by: Jan Vesely jano.ves...@gmail.com
---
 hw/usb-uhci.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index a65e0b3..1e9c1e7 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -702,11 +702,15 @@ out:
 case USB_RET_STALL:
 td-ctrl |= TD_CTRL_STALL;
 td-ctrl = ~TD_CTRL_ACTIVE;
+s-status |= UHCI_STS_USBERR;
+uhci_update_irq(s);
 return 1;

 case USB_RET_BABBLE:
 td-ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL;
 td-ctrl = ~TD_CTRL_ACTIVE;
+s-status |= UHCI_STS_USBERR;
+uhci_update_irq(s);
 /* frame interrupted */
 return -1;

-- 
1.7.3.4



Re: [Qemu-devel] Bug #757654: UHCI fails to signal stall response patch

2011-05-09 Thread Gerd Hoffmann

On 05/09/11 12:16, Jan Vesely wrote:

UHCI host controller status register indicates error and
an interrupt is triggered on BABBLE and STALL errors.


Queued up.

thanks,
  Gerd




Re: [Qemu-devel] Bug #757654: UHCI fails to signal stall response patch

2011-05-06 Thread Jan Vesely
hi,
using the modified (single line) patch only works half-way, it sets
the value in status register (guess that's what that line does :))
but hw interrupt is not generated.

I tried adding uhci_update_irq and this patch:

diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index a65e0b3..1e9c1e7 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -702,11 +702,15 @@ out:
 case USB_RET_STALL:
 td-ctrl |= TD_CTRL_STALL;
 td-ctrl = ~TD_CTRL_ACTIVE;
+s-status |= UHCI_STS_USBERR;
+uhci_update_irq(s);
 return 1;

 case USB_RET_BABBLE:
 td-ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL;
 td-ctrl = ~TD_CTRL_ACTIVE;
+s-status |= UHCI_STS_USBERR;
+uhci_update_irq(s);
 /* frame interrupted */
 return -1;

works.

jan

On Thu, May 5, 2011 at 2:05 PM, Gerd Hoffmann kra...@redhat.com wrote:
 diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
 index 346db3e..a51d89b 100644
 --- a/hw/usb-uhci.c
 +++ b/hw/usb-uhci.c
 @@ -732,11 +732,21 @@ out:
      case USB_RET_STALL:
          td-ctrl |= TD_CTRL_STALL;
          td-ctrl= ~TD_CTRL_ACTIVE;
 +        s-status |= UHCI_STS_USBERR;

 Just this line should be enougth.

      case USB_RET_BABBLE:
          td-ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL;
          td-ctrl= ~TD_CTRL_ACTIVE;
 +        s-status |= UHCI_STS_USBERR;

 Likewise.

 Tried that?

 cheers,
  Gerd





Re: [Qemu-devel] Bug #757654: UHCI fails to signal stall response patch

2011-05-05 Thread Gerd Hoffmann

diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index 346db3e..a51d89b 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -732,11 +732,21 @@ out:
  case USB_RET_STALL:
  td-ctrl |= TD_CTRL_STALL;
  td-ctrl= ~TD_CTRL_ACTIVE;
+s-status |= UHCI_STS_USBERR;


Just this line should be enougth.


  case USB_RET_BABBLE:
  td-ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL;
  td-ctrl= ~TD_CTRL_ACTIVE;
+s-status |= UHCI_STS_USBERR;


Likewise.

Tried that?

cheers,
  Gerd




Re: [Qemu-devel] Bug #757654: UHCI fails to signal stall response patch

2011-04-16 Thread Brad Hards
On Sat, 16 Apr 2011 06:57:00 am Jan Vesely wrote:
 +s-status |= UHCI_STS_USBERR;
This is per UHCI 1.1D Section 4.1.5. Looks good.

 +*int_mask |= 0x02;
 +if (td-ctrl  TD_CTRL_IOC)
 +*int_mask |= 0x01;
 +uhci_update_irq(s);
I see A hardware interrupt is signalled to the system, but can you provide a 
little explanation of why this particular interrupt mask?

 +s-status |= UHCI_STS_USBERR;
This is per UHCI 1.1d Section 4.1.4. Looks good.

 +*int_mask |= 0x02;
 +if (td-ctrl  TD_CTRL_IOC)
 +   *int_mask |= 0x01;
 +uhci_update_irq(s);
I see A hardware interrupt is signalled to the system, but can you provide a 
little explanation of why this particular interrupt mask?




Re: [Qemu-devel] Bug #757654: UHCI fails to signal stall response patch

2011-04-16 Thread Jan Vesely
On Sat, Apr 16, 2011 at 8:33 AM, Brad Hards br...@frogmouth.net wrote:
 On Sat, 16 Apr 2011 06:57:00 am Jan Vesely wrote:
 +        s-status |= UHCI_STS_USBERR;
 This is per UHCI 1.1D Section 4.1.5. Looks good.

 +        *int_mask |= 0x02;
 +        if (td-ctrl  TD_CTRL_IOC)
 +            *int_mask |= 0x01;
 +        uhci_update_irq(s);
 I see A hardware interrupt is signalled to the system, but can you provide a
 little explanation of why this particular interrupt mask?

I used th code I found around in that same file (hw/usb-uhci.c),
lines 705-724 contain both masks. if (td-ctrl  TD_CTRL_IOC)
*int_mask |= 0x01;, is in more places so I just copied that lines.
*int_mask |= 0x2, is used when SPD condition is detected.
that is strange, SPD should use the same interrupt as IOC, but return
value indicates that it is treated as error condition (unsuccessful
td) so I figured *int_mask |= 0x2 signals error interrupt (it does not
match bits in interrupt enable register- that was my first guess)
uhci_update_irq(s); to me it looks like a duplicate functionality to
int_mask parameter, I did not investigate further and included it just
to be sure (it's used on line 775, when error countdown reaches zero).


 +        s-status |= UHCI_STS_USBERR;
 This is per UHCI 1.1d Section 4.1.4. Looks good.

 +        *int_mask |= 0x02;
 +        if (td-ctrl  TD_CTRL_IOC)
 +           *int_mask |= 0x01;
 +        uhci_update_irq(s);
 I see A hardware interrupt is signalled to the system, but can you provide a
 little explanation of why this particular interrupt mask?