[Qemu-devel] [PATCH] Extend support of SMBUS(module pm_smbus.c) HST

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [Qemu-devel] [PATCH] Extend support of SMBUS(module pm_smbus.c) HST_STS register.
@ 2013-04-28 18:26 Maksim Ratnikov
  2013-04-28 20:03 ` Peter Maydell
  0 siblings, 1 reply; 7+ messages in thread
From: Maksim Ratnikov @ 2013-04-28 18:26 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2417 bytes --]

Previous realization doesn't consider flags in the status register.
Add DS and INTR bits of HST_STS register set after transaction execution.
Update bits resetting in HST_STS register. Update error processing: if
DEV_ERR bit are set
transaction isn't execution.


Signed-off-by: Maksim_Ratnikov <m.o.ratnikov@gmail.com>
---
 hw/i2c/pm_smbus.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
index 0b5bb89..d5f5c56 100644
--- a/hw/i2c/pm_smbus.c
+++ b/hw/i2c/pm_smbus.c
@@ -50,9 +50,16 @@ static void smb_transaction(PMSMBus *s)
     i2c_bus *bus = s->smbus;

     SMBUS_DPRINTF("SMBus trans addr=0x%02x prot=0x%02x\n", addr, prot);
+    /* Transaction isn't exec if DEV_ERR bit set */
+    if ((s->smb_stat & 0x04) != 0)
+        goto error;
     switch(prot) {
     case 0x0:
         smbus_quick_command(bus, addr, read);
+        /* Set successfully transaction end:
+         * ByteDoneStatus = 1 (HST_STS bit #7) and INTR = 1 (HST_STS bit
#1)
+         */
+        s->smb_stat |= 0x82;
         break;
     case 0x1:
         if (read) {
@@ -60,6 +67,7 @@ static void smb_transaction(PMSMBus *s)
         } else {
             smbus_send_byte(bus, addr, cmd);
         }
+        s->smb_stat |= 0x82;
         break;
     case 0x2:
         if (read) {
@@ -67,6 +75,7 @@ static void smb_transaction(PMSMBus *s)
         } else {
             smbus_write_byte(bus, addr, cmd, s->smb_data0);
         }
+        s->smb_stat |= 0x82;
         break;
     case 0x3:
         if (read) {
@@ -77,6 +86,7 @@ static void smb_transaction(PMSMBus *s)
         } else {
             smbus_write_word(bus, addr, cmd, (s->smb_data1 << 8) |
s->smb_data0);
         }
+        s->smb_stat |= 0x82;
         break;
     case 0x5:
         if (read) {
@@ -84,6 +94,7 @@ static void smb_transaction(PMSMBus *s)
         } else {
             smbus_write_block(bus, addr, cmd, s->smb_data, s->smb_data0);
         }
+        s->smb_stat |= 0x82;
         break;
     default:
         goto error;
@@ -102,7 +113,7 @@ static void smb_ioport_writeb(void *opaque, hwaddr
addr, uint64_t val,
     SMBUS_DPRINTF("SMB writeb port=0x%04x val=0x%02x\n", addr, val);
     switch(addr) {
     case SMBHSTSTS:
-        s->smb_stat = 0;
+        s->smb_stat = (~(val & 0xff)) & s->smb_stat;
         s->smb_index = 0;
         break;
     case SMBHSTCNT:
-- 
1.7.3.4

[-- Attachment #2: Type: text/html, Size: 2840 bytes --]

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH] Extend support of SMBUS(module pm_smbus.c) HST_STS register.
  2013-04-28 18:26 [Qemu-devel] [PATCH] Extend support of SMBUS(module pm_smbus.c) HST_STS register Maksim Ratnikov
@ 2013-04-28 20:03 ` Peter Maydell
  2013-04-29 22:29   ` Maksim Ratnikov
  2013-04-29 22:39   ` Maksim Ratnikov
  0 siblings, 2 replies; 7+ messages in thread
From: Peter Maydell @ 2013-04-28 20:03 UTC (permalink / raw)
  To: Maksim Ratnikov; +Cc: qemu-devel

On 28 April 2013 19:26, Maksim Ratnikov <m.o.ratnikov@gmail.com> wrote:
> Previous realization doesn't consider flags in the status register.
> Add DS and INTR bits of HST_STS register set after transaction execution.
> Update bits resetting in HST_STS register. Update error processing: if
> DEV_ERR bit are set
> transaction isn't execution.

Hi; thanks for this patch. A minor comment below...

> Signed-off-by: Maksim_Ratnikov <m.o.ratnikov@gmail.com>
> ---
>  hw/i2c/pm_smbus.c |   13 ++++++++++++-
>  1 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
> index 0b5bb89..d5f5c56 100644
> --- a/hw/i2c/pm_smbus.c
> +++ b/hw/i2c/pm_smbus.c
> @@ -50,9 +50,16 @@ static void smb_transaction(PMSMBus *s)
>      i2c_bus *bus = s->smbus;
>
>      SMBUS_DPRINTF("SMBus trans addr=0x%02x prot=0x%02x\n", addr, prot);
> +    /* Transaction isn't exec if DEV_ERR bit set */
> +    if ((s->smb_stat & 0x04) != 0)
> +        goto error;

QEMU coding style requires braces on this if(). (You can run
your patch through scripts/codingstyle.pl to check for this
kind of thing.)

>      switch(prot) {
>      case 0x0:
>          smbus_quick_command(bus, addr, read);
> +        /* Set successfully transaction end:
> +         * ByteDoneStatus = 1 (HST_STS bit #7) and INTR = 1 (HST_STS bit
> #1)
> +         */
> +        s->smb_stat |= 0x82;

I think it would be nicer to define some constants for the
HST_STS bits. Then you could write
           s->smb_stat |= STS_BYTE_DONE | STS_INTR;

and you wouldn't need to have the comment here explaining
the magic numbers. (feel free to pick better constant names,
ideally ones matching whatever the datasheet uses.)
(then you can use the constants for all the smb_stat uses).

thanks
-- PMM

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH] Extend support of SMBUS(module pm_smbus.c) HST_STS register.
  2013-04-28 20:03 ` Peter Maydell
@ 2013-04-29 22:29   ` Maksim Ratnikov
  2013-04-29 22:39   ` Maksim Ratnikov
  1 sibling, 0 replies; 7+ messages in thread
From: Maksim Ratnikov @ 2013-04-29 22:29 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel


Previous realization doesn't consider flags in the status register.
Add DS and INTR bits of HST_STS register set after transaction execution.
Update bits resetting in HST_STS register. Update error processing: if 
DEV_ERR bit are set
transaction isn't execution.


Signed-off-by: Maksim_Ratnikov <m.o.ratnikov@gmail.com>
---
  hw/i2c/pm_smbus.c |   25 +++++++++++++++++++++++--
  1 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
index 0b5bb89..35f154e 100644
--- a/hw/i2c/pm_smbus.c
+++ b/hw/i2c/pm_smbus.c
@@ -32,6 +32,18 @@
  #define SMBHSTDAT1      0x06
  #define SMBBLKDAT       0x07

+#define STS_HOST_BUSY   (1)
+#define STS_INTR        (1<<1)
+#define STS_DEV_ERR     (1<<2)
+#define STS_BUS_ERR     (1<<3)
+#define STS_FAILED      (1<<4)
+#define STS_SMBALERT    (1<<5)
+#define STS_INUSE_STS   (1<<6)
+#define STS_BYTE_DONE   (1<<7)
+/* Signs of successfully transaction end :
+*  ByteDoneStatus = 1 (STS_BYTE_DONE) and INTR = 1 (STS_INTR )
+*/
+
  //#define DEBUG

  #ifdef DEBUG
@@ -50,9 +62,14 @@ static void smb_transaction(PMSMBus *s)
      i2c_bus *bus = s->smbus;

      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)  {
+            goto error;
+        }
      switch(prot) {
      case 0x0:
          smbus_quick_command(bus, addr, read);
+        s->smb_stat |= STS_BYTE_DONE | STS_INTR;
          break;
      case 0x1:
          if (read) {
@@ -60,6 +77,7 @@ static void smb_transaction(PMSMBus *s)
          } else {
              smbus_send_byte(bus, addr, cmd);
          }
+        s->smb_stat |= STS_BYTE_DONE | STS_INTR;
          break;
      case 0x2:
          if (read) {
@@ -67,6 +85,7 @@ static void smb_transaction(PMSMBus *s)
          } else {
              smbus_write_byte(bus, addr, cmd, s->smb_data0);
          }
+        s->smb_stat |= STS_BYTE_DONE | STS_INTR;
          break;
      case 0x3:
          if (read) {
@@ -77,6 +96,7 @@ static void smb_transaction(PMSMBus *s)
          } else {
              smbus_write_word(bus, addr, cmd, (s->smb_data1 << 8) | 
s->smb_data0);
          }
+        s->smb_stat |= STS_BYTE_DONE | STS_INTR;
          break;
      case 0x5:
          if (read) {
@@ -84,6 +104,7 @@ static void smb_transaction(PMSMBus *s)
          } else {
              smbus_write_block(bus, addr, cmd, s->smb_data, s->smb_data0);
          }
+        s->smb_stat |= STS_BYTE_DONE | STS_INTR;
          break;
      default:
          goto error;
@@ -91,7 +112,7 @@ static void smb_transaction(PMSMBus *s)
      return;

    error:
-    s->smb_stat |= 0x04;
+    s->smb_stat |= STS_DEV_ERR;
  }

  static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
@@ -102,7 +123,7 @@ static void smb_ioport_writeb(void *opaque, hwaddr 
addr, uint64_t val,
      SMBUS_DPRINTF("SMB writeb port=0x%04x val=0x%02x\n", addr, val);
      switch(addr) {
      case SMBHSTSTS:
-        s->smb_stat = 0;
+        s->smb_stat = (~(val & 0xff)) & s->smb_stat;
          s->smb_index = 0;
          break;
      case SMBHSTCNT:
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH] Extend support of SMBUS(module pm_smbus.c) HST_STS register.
  2013-04-28 20:03 ` Peter Maydell
  2013-04-29 22:29   ` Maksim Ratnikov
@ 2013-04-29 22:39   ` Maksim Ratnikov
  1 sibling, 0 replies; 7+ messages in thread
From: Maksim Ratnikov @ 2013-04-29 22:39 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel

29.04.2013 00:03, Peter Maydell пишет:
> On 28 April 2013 19:26, Maksim Ratnikov <m.o.ratnikov@gmail.com> wrote:
>> Previous realization doesn't consider flags in the status register.
>> Add DS and INTR bits of HST_STS register set after transaction execution.
>> Update bits resetting in HST_STS register. Update error processing: if
>> DEV_ERR bit are set
>> transaction isn't execution.
> Hi; thanks for this patch. A minor comment below...
>
>> Signed-off-by: Maksim_Ratnikov <m.o.ratnikov@gmail.com>
>> ---
>>   hw/i2c/pm_smbus.c |   13 ++++++++++++-
>>   1 files changed, 12 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
>> index 0b5bb89..d5f5c56 100644
>> --- a/hw/i2c/pm_smbus.c
>> +++ b/hw/i2c/pm_smbus.c
>> @@ -50,9 +50,16 @@ static void smb_transaction(PMSMBus *s)
>>       i2c_bus *bus = s->smbus;
>>
>>       SMBUS_DPRINTF("SMBus trans addr=0x%02x prot=0x%02x\n", addr, prot);
>> +    /* Transaction isn't exec if DEV_ERR bit set */
>> +    if ((s->smb_stat & 0x04) != 0)
>> +        goto error;
> QEMU coding style requires braces on this if(). (You can run
> your patch through scripts/codingstyle.pl to check for this
> kind of thing.)
>
>>       switch(prot) {
>>       case 0x0:
>>           smbus_quick_command(bus, addr, read);
>> +        /* Set successfully transaction end:
>> +         * ByteDoneStatus = 1 (HST_STS bit #7) and INTR = 1 (HST_STS bit
>> #1)
>> +         */
>> +        s->smb_stat |= 0x82;
> I think it would be nicer to define some constants for the
> HST_STS bits. Then you could write
>             s->smb_stat |= STS_BYTE_DONE | STS_INTR;
>
> and you wouldn't need to have the comment here explaining
> the magic numbers. (feel free to pick better constant names,
> ideally ones matching whatever the datasheet uses.)
> (then you can use the constants for all the smb_stat uses).
>
> thanks
> -- PMM
Thank you for your answer. I made some correction and sent new version 
of patch. I can't find script codingstyle.pl, but I use checkpatch.pl .  
It shows 0 error and 0 warning.

^ permalink raw reply	[flat|nested] 7+ messages in thread

[parent not found: <51CC3B2C.9070406@gmail.com>]

* Re: [Qemu-devel] [PATCH] Extend support of SMBUS(module pm_smbus.c) HST_STS register.
       [not found] <51CC3B2C.9070406@gmail.com>
@ 2013-06-27 16:33 ` Peter Maydell
  2013-06-27 21:07   ` Maksim Ratnikov
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2013-06-27 16:33 UTC (permalink / raw)
  To: Maksim Ratnikov; +Cc: Anthony Liguori, qemu-devel

On 27 June 2013 14:16, Maksim Ratnikov <m.o.ratnikov@gmail.com> wrote:
> Ping?
> I send update of my patch at April 29. Link for my patch at patchwork:
> http://patchwork.ozlabs.org/patch/240525/   . Link for my response in
> mailing-list:
> http://lists.nongnu.org/archive/html/qemu-devel/2013-04/msg05850.html
> Can you tell: is my patch in?

No, your patch doesn't seem to have been committed yet.
Since pm_smbus is a PC device, you should CC the maintainer
of the PC platform, who is Anthony (and who I've cc'd).

thanks
-- PMM

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH] Extend support of SMBUS(module pm_smbus.c) HST_STS register.
  2013-06-27 16:33 ` Peter Maydell
@ 2013-06-27 21:07   ` Maksim Ratnikov
  2013-06-27 21:48     ` Anthony Liguori
  0 siblings, 1 reply; 7+ messages in thread
From: Maksim Ratnikov @ 2013-06-27 21:07 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Anthony Liguori, qemu-devel

27.06.2013 20:33, Peter Maydell пишет:
> On 27 June 2013 14:16, Maksim Ratnikov <m.o.ratnikov@gmail.com> wrote:
>> Ping?
>> I send update of my patch at April 29. Link for my patch at patchwork:
>> http://patchwork.ozlabs.org/patch/240525/   . Link for my response in
>> mailing-list:
>> http://lists.nongnu.org/archive/html/qemu-devel/2013-04/msg05850.html
>> Can you tell: is my patch in?
> No, your patch doesn't seem to have been committed yet.
> Since pm_smbus is a PC device, you should CC the maintainer
> of the PC platform, who is Anthony (and who I've cc'd).
>
> thanks
> -- PMM
Thanks for you answer. I will wait his response.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH] Extend support of SMBUS(module pm_smbus.c) HST_STS register.
  2013-06-27 21:07   ` Maksim Ratnikov
@ 2013-06-27 21:48     ` Anthony Liguori
  0 siblings, 0 replies; 7+ messages in thread
From: Anthony Liguori @ 2013-06-27 21:48 UTC (permalink / raw)
  To: Maksim Ratnikov, Peter Maydell; +Cc: qemu-devel

Maksim Ratnikov <m.o.ratnikov@gmail.com> writes:

> 27.06.2013 20:33, Peter Maydell пишет:
>> On 27 June 2013 14:16, Maksim Ratnikov <m.o.ratnikov@gmail.com> wrote:
>>> Ping?
>>> I send update of my patch at April 29. Link for my patch at patchwork:
>>> http://patchwork.ozlabs.org/patch/240525/   . Link for my response in
>>> mailing-list:
>>> http://lists.nongnu.org/archive/html/qemu-devel/2013-04/msg05850.html
>>> Can you tell: is my patch in?
>> No, your patch doesn't seem to have been committed yet.
>> Since pm_smbus is a PC device, you should CC the maintainer
>> of the PC platform, who is Anthony (and who I've cc'd).
>>
>> thanks
>> -- PMM
> Thanks for you answer. I will wait his response.

Please resubmit as a top level patch rebased against latest git HEAD.

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-06-27 21:48 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-28 18:26 [Qemu-devel] [PATCH] Extend support of SMBUS(module pm_smbus.c) HST_STS register Maksim Ratnikov
2013-04-28 20:03 ` Peter Maydell
2013-04-29 22:29   ` Maksim Ratnikov
2013-04-29 22:39   ` Maksim Ratnikov
     [not found] <51CC3B2C.9070406@gmail.com>
2013-06-27 16:33 ` Peter Maydell
2013-06-27 21:07   ` Maksim Ratnikov
2013-06-27 21:48     ` Anthony Liguori

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).