diff mbox series

[v2,07/10] Don't delay host status register busy bit when interrupts are enabled

Message ID 1534796770-10295-8-git-send-email-minyard@acm.org
State New
Headers show
Series [v2,01/10] i2c:pm_smbus: Clean up some style issues | expand

Commit Message

Corey Minyard Aug. 20, 2018, 8:26 p.m. UTC
From: Corey Minyard <cminyard@mvista.com>


Change 880b1ffe6ec2f0ae "smbus: do not immediately complete commands"
changed pm_smbus to delay setting the host busy bit until the status
register was read, to work around a bug in AMIBIOS.  Unfortunately,
when interrupts are enabled, the status register will never get read
and the processing will never happen.

Modify the code to only delay setting the host busy bit if interrupts
are not enabled.

Signed-off-by: Corey Minyard <cminyard@mvista.com>

Cc: Hervé Poussineau <hpoussin@reactos.org>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/i2c/pm_smbus.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

-- 
2.7.4

Comments

Philippe Mathieu-Daudé Aug. 21, 2018, 6:26 a.m. UTC | #1
On 08/20/2018 05:26 PM, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>

> 

> Change 880b1ffe6ec2f0ae "smbus: do not immediately complete commands"

> changed pm_smbus to delay setting the host busy bit until the status

> register was read, to work around a bug in AMIBIOS.  Unfortunately,

> when interrupts are enabled, the status register will never get read

> and the processing will never happen.

> 

> Modify the code to only delay setting the host busy bit if interrupts

> are not enabled.

> 

> Signed-off-by: Corey Minyard <cminyard@mvista.com>

> Cc: Hervé Poussineau <hpoussin@reactos.org>

> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>


Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


> ---

>  hw/i2c/pm_smbus.c | 19 +++++++++++++------

>  1 file changed, 13 insertions(+), 6 deletions(-)

> 

> diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c

> index 664a6b1..10ba208 100644

> --- a/hw/i2c/pm_smbus.c

> +++ b/hw/i2c/pm_smbus.c

> @@ -80,9 +80,6 @@ static void smb_transaction(PMSMBus *s)

>      I2CBus *bus = s->smbus;

>      int ret;

>  

> -    assert(s->smb_stat & STS_HOST_BUSY);

> -    s->smb_stat &= ~STS_HOST_BUSY;

> -

>      SMBUS_DPRINTF("SMBus trans addr=0x%02x prot=0x%02x\n", addr, prot);

>      /* Transaction isn't exec if STS_DEV_ERR bit set */

>      if ((s->smb_stat & STS_DEV_ERR) != 0)  {

> @@ -209,9 +206,18 @@ error:

>  

>  static void smb_transaction_start(PMSMBus *s)

>  {

> -    /* Do not execute immediately the command ; it will be

> -     * executed when guest will read SMB_STAT register */

> -    s->smb_stat |= STS_HOST_BUSY;

> +    if (s->smb_ctl & CTL_INTREN) {

> +        smb_transaction(s);

> +    } else {

> +        /* Do not execute immediately the command; it will be

> +         * executed when guest will read SMB_STAT register.  This

> +         * is to work around a bug in AMIBIOS (that is working

> +         * around another bug in some specific hardware) where

> +         * it waits for STS_HOST_BUSY to be set before waiting

> +         * checking for status.  If STS_HOST_BUSY doesn't get

> +         * set, it gets stuck. */

> +        s->smb_stat |= STS_HOST_BUSY;

> +    }

>  }

>  

>  static bool

> @@ -330,6 +336,7 @@ static uint64_t smb_ioport_readb(void *opaque, hwaddr addr, unsigned width)

>          val = s->smb_stat;

>          if (s->smb_stat & STS_HOST_BUSY) {

>              /* execute command now */

> +            s->smb_stat &= ~STS_HOST_BUSY;

>              smb_transaction(s);

>          }

>          break;

>
diff mbox series

Patch

diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
index 664a6b1..10ba208 100644
--- a/hw/i2c/pm_smbus.c
+++ b/hw/i2c/pm_smbus.c
@@ -80,9 +80,6 @@  static void smb_transaction(PMSMBus *s)
     I2CBus *bus = s->smbus;
     int ret;
 
-    assert(s->smb_stat & STS_HOST_BUSY);
-    s->smb_stat &= ~STS_HOST_BUSY;
-
     SMBUS_DPRINTF("SMBus trans addr=0x%02x prot=0x%02x\n", addr, prot);
     /* Transaction isn't exec if STS_DEV_ERR bit set */
     if ((s->smb_stat & STS_DEV_ERR) != 0)  {
@@ -209,9 +206,18 @@  error:
 
 static void smb_transaction_start(PMSMBus *s)
 {
-    /* Do not execute immediately the command ; it will be
-     * executed when guest will read SMB_STAT register */
-    s->smb_stat |= STS_HOST_BUSY;
+    if (s->smb_ctl & CTL_INTREN) {
+        smb_transaction(s);
+    } else {
+        /* Do not execute immediately the command; it will be
+         * executed when guest will read SMB_STAT register.  This
+         * is to work around a bug in AMIBIOS (that is working
+         * around another bug in some specific hardware) where
+         * it waits for STS_HOST_BUSY to be set before waiting
+         * checking for status.  If STS_HOST_BUSY doesn't get
+         * set, it gets stuck. */
+        s->smb_stat |= STS_HOST_BUSY;
+    }
 }
 
 static bool
@@ -330,6 +336,7 @@  static uint64_t smb_ioport_readb(void *opaque, hwaddr addr, unsigned width)
         val = s->smb_stat;
         if (s->smb_stat & STS_HOST_BUSY) {
             /* execute command now */
+            s->smb_stat &= ~STS_HOST_BUSY;
             smb_transaction(s);
         }
         break;