diff mbox series

[1/2] esp-pci: Fix status register write erase control

Message ID 1543442171-24863-1-git-send-email-linux@roeck-us.net
State New
Headers show
Series [1/2] esp-pci: Fix status register write erase control | expand

Commit Message

Guenter Roeck Nov. 28, 2018, 9:56 p.m. UTC
Per AM53C974 datasheet, definition of "SCSI Bus and Control (SBAC)"
register:

Bit 24 – STATUS – Write Erase Control

This bit controls the Write Erase feature on bits 3:1 and bit 6 of the DMA
Status Register ((B)+54h). When this bit is programmed to ‘1’, the state
of bits 3:1 are preserved when read. Bits 3:1 are only cleared when a ‘1’
is written to the corresponding bit location. For example, to clear bit 1,
the value of ‘0000_0010b’ should be written to the register. When the DMA
Status Preserve bit is ‘0’, bits 3:1 are cleared when read.

The status register is currently defined to bit 12, not bit 24.
Also, its implementation is reversed: The status is auto-cleared if
the bit is set to 1, and must be cleared explicitly when the bit is
set to 0. This results in spurious interrupts reported by the Linux
kernel, and in some cases even results in stalled SCSI operations.

Set SBAC_STATUS to bit 24 and reverse the logic to fix the problem.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 hw/scsi/esp-pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Paolo Bonzini Nov. 29, 2018, 9:58 a.m. UTC | #1
On 28/11/18 22:56, Guenter Roeck wrote:
> Per AM53C974 datasheet, definition of "SCSI Bus and Control (SBAC)"
> register:
> 
> Bit 24 – STATUS – Write Erase Control
> 
> This bit controls the Write Erase feature on bits 3:1 and bit 6 of the DMA
> Status Register ((B)+54h). When this bit is programmed to ‘1’, the state
> of bits 3:1 are preserved when read. Bits 3:1 are only cleared when a ‘1’
> is written to the corresponding bit location. For example, to clear bit 1,
> the value of ‘0000_0010b’ should be written to the register. When the DMA
> Status Preserve bit is ‘0’, bits 3:1 are cleared when read.
> 
> The status register is currently defined to bit 12, not bit 24.
> Also, its implementation is reversed: The status is auto-cleared if
> the bit is set to 1, and must be cleared explicitly when the bit is
> set to 0. This results in spurious interrupts reported by the Linux
> kernel, and in some cases even results in stalled SCSI operations.
> 
> Set SBAC_STATUS to bit 24 and reverse the logic to fix the problem.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  hw/scsi/esp-pci.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
> index 419fc66..d956909 100644
> --- a/hw/scsi/esp-pci.c
> +++ b/hw/scsi/esp-pci.c
> @@ -59,7 +59,7 @@
>  #define DMA_STAT_SCSIINT 0x10
>  #define DMA_STAT_BCMBLT  0x20
>  
> -#define SBAC_STATUS 0x1000
> +#define SBAC_STATUS (1 << 24)
>  
>  typedef struct PCIESPState {
>      /*< private >*/
> @@ -136,7 +136,7 @@ static void esp_pci_dma_write(PCIESPState *pci, uint32_t saddr, uint32_t val)
>          pci->dma_regs[saddr] = val;
>          break;
>      case DMA_STAT:
> -        if (!(pci->sbac & SBAC_STATUS)) {
> +        if (pci->sbac & SBAC_STATUS) {
>              /* clear some bits on write */
>              uint32_t mask = DMA_STAT_ERROR | DMA_STAT_ABORT | DMA_STAT_DONE;
>              pci->dma_regs[DMA_STAT] &= ~(val & mask);
> @@ -157,7 +157,7 @@ static uint32_t esp_pci_dma_read(PCIESPState *pci, uint32_t saddr)
>          if (pci->esp.rregs[ESP_RSTAT] & STAT_INT) {
>              val |= DMA_STAT_SCSIINT;
>          }
> -        if (pci->sbac & SBAC_STATUS) {
> +        if (!(pci->sbac & SBAC_STATUS)) {
>              pci->dma_regs[DMA_STAT] &= ~(DMA_STAT_ERROR | DMA_STAT_ABORT |
>                                           DMA_STAT_DONE);
>          }
> 

Queued this one only, for now at least.

Paolo
diff mbox series

Patch

diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index 419fc66..d956909 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -59,7 +59,7 @@ 
 #define DMA_STAT_SCSIINT 0x10
 #define DMA_STAT_BCMBLT  0x20
 
-#define SBAC_STATUS 0x1000
+#define SBAC_STATUS (1 << 24)
 
 typedef struct PCIESPState {
     /*< private >*/
@@ -136,7 +136,7 @@  static void esp_pci_dma_write(PCIESPState *pci, uint32_t saddr, uint32_t val)
         pci->dma_regs[saddr] = val;
         break;
     case DMA_STAT:
-        if (!(pci->sbac & SBAC_STATUS)) {
+        if (pci->sbac & SBAC_STATUS) {
             /* clear some bits on write */
             uint32_t mask = DMA_STAT_ERROR | DMA_STAT_ABORT | DMA_STAT_DONE;
             pci->dma_regs[DMA_STAT] &= ~(val & mask);
@@ -157,7 +157,7 @@  static uint32_t esp_pci_dma_read(PCIESPState *pci, uint32_t saddr)
         if (pci->esp.rregs[ESP_RSTAT] & STAT_INT) {
             val |= DMA_STAT_SCSIINT;
         }
-        if (pci->sbac & SBAC_STATUS) {
+        if (!(pci->sbac & SBAC_STATUS)) {
             pci->dma_regs[DMA_STAT] &= ~(DMA_STAT_ERROR | DMA_STAT_ABORT |
                                          DMA_STAT_DONE);
         }