Patchwork hw/esp.c mishandles commands without dma

login
register
mail settings
Submitter Martin Husemann
Date June 17, 2012, 9:55 p.m.
Message ID <20120617215520.GA7890@mail.duskware.de>
Download mbox | patch
Permalink /patch/165380/
State New
Headers show

Comments

Martin Husemann - June 17, 2012, 9:55 p.m.
The NetBSD driver sometimes uses commands without DMA (for example a simple
TEST_UNIT_READY). On esp hardware, the command has a DMA bit (if dma is to
be used), and when writing to the command register, the s->dma status is
updated accordingly.

When commands with dma are used and dma is disabled, the command is defered
untill dma is enabled again. This is all fine. However, the deferal should
not happen if the command does not use dma.

This can bee seen by booting NetBSD/sparc: for every scsi device attached
there will be a 5 second timeout and an error message.

Here is a trivial patch wich fixes this problem for me:



I filed a ticket in the bugtracker for it: #1014099.


Martin
Martin Husemann - June 17, 2012, 9:57 p.m.
On Sun, Jun 17, 2012 at 11:55:20PM +0200, Martin Husemann wrote:
> The NetBSD driver sometimes uses commands without DMA (for example a simple
> TEST_UNIT_READY). On esp hardware, the command has a DMA bit (if dma is to
> be used), and when writing to the command register, the s->dma status is
> updated accordingly.
> 
> When commands with dma are used and dma is disabled, the command is defered
> untill dma is enabled again. This is all fine. However, the deferal should
> not happen if the command does not use dma.
> 
> This can bee seen by booting NetBSD/sparc: for every scsi device attached
> there will be a 5 second timeout and an error message.
> 
> Here is a trivial patch wich fixes this problem for me:
> 
> --- hw/esp.c.orig	2012-06-01 11:13:13.000000000 +0200
> +++ hw/esp.c	2012-06-18 01:43:34.000000000 +0200
> @@ -270,7 +270,7 @@ static void handle_satn(ESPState *s)
>      uint8_t buf[32];
>      int len;
>  
> -    if (!s->dma_enabled) {
> +    if (s->dma && !s->dma_enabled) {
>          s->dma_cb = handle_satn;
>          return;
>      }
> @@ -284,7 +284,7 @@ static void handle_s_without_atn(ESPStat
>      uint8_t buf[32];
>      int len;
>  
> -    if (!s->dma_enabled) {
> +    if (s->dma && !s->dma_enabled) {
>          s->dma_cb = handle_s_without_atn;
>          return;
>      }
> @@ -296,7 +296,7 @@ static void handle_s_without_atn(ESPStat
>  
>  static void handle_satn_stop(ESPState *s)
>  {
> -    if (!s->dma_enabled) {
> +    if (s->dma && !s->dma_enabled) {
>          s->dma_cb = handle_satn_stop;
>          return;
>      }
> 
> 
> I filed a ticket in the bugtracker for it: #1014099.
> 
> 
> Martin

Forgot to add:

Signed-off-by: Martin Husemann <martin@NetBSD.org>

Patch

--- hw/esp.c.orig	2012-06-01 11:13:13.000000000 +0200
+++ hw/esp.c	2012-06-18 01:43:34.000000000 +0200
@@ -270,7 +270,7 @@  static void handle_satn(ESPState *s)
     uint8_t buf[32];
     int len;
 
-    if (!s->dma_enabled) {
+    if (s->dma && !s->dma_enabled) {
         s->dma_cb = handle_satn;
         return;
     }
@@ -284,7 +284,7 @@  static void handle_s_without_atn(ESPStat
     uint8_t buf[32];
     int len;
 
-    if (!s->dma_enabled) {
+    if (s->dma && !s->dma_enabled) {
         s->dma_cb = handle_s_without_atn;
         return;
     }
@@ -296,7 +296,7 @@  static void handle_s_without_atn(ESPStat
 
 static void handle_satn_stop(ESPState *s)
 {
-    if (!s->dma_enabled) {
+    if (s->dma && !s->dma_enabled) {
         s->dma_cb = handle_satn_stop;
         return;
     }