diff mbox series

[v2,4/9] hw/i2c: pmbus: refactor uint handling and update MAINTAINERS

Message ID 20220302002307.1895616-5-titusr@google.com (mailing list archive)
State New, archived
Headers show
Series Fixups for PMBus and new sensors | expand

Commit Message

Titus Rwantare March 2, 2022, 12:23 a.m. UTC
Signed-off-by: Titus Rwantare <titusr@google.com>
---
 MAINTAINERS           | 10 ++++++++++
 hw/i2c/pmbus_device.c | 18 +++++++++---------
 2 files changed, 19 insertions(+), 9 deletions(-)

Comments

Corey Minyard March 2, 2022, 12:38 a.m. UTC | #1
On Tue, Mar 01, 2022 at 04:23:02PM -0800, Titus Rwantare wrote:
> Signed-off-by: Titus Rwantare <titusr@google.com>
> ---
>  MAINTAINERS           | 10 ++++++++++
>  hw/i2c/pmbus_device.c | 18 +++++++++---------
>  2 files changed, 19 insertions(+), 9 deletions(-)

This makes sense, but can you split it into two patches and give a bit
more explaination about the uint handling.  Usually patches without
explaination are frowned upon, even if it's kind of obvious.

-corey

> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fa8adc2618..3601984b5d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3135,6 +3135,16 @@ F: include/hw/i2c/smbus_master.h
>  F: include/hw/i2c/smbus_slave.h
>  F: include/hw/i2c/smbus_eeprom.h
>  
> +PMBus
> +M: Titus Rwantare <titusr@google.com>
> +S: Maintained
> +F: hw/i2c/pmbus_device.c
> +F: hw/sensor/adm1272.c
> +F: hw/sensor/max34451.c
> +F: include/hw/i2c/pmbus_device.h
> +F: tests/qtest/adm1272-test.c
> +F: tests/qtest/max34451-test.c
> +
>  Firmware schema specifications
>  M: Philippe Mathieu-Daudé <f4bug@amsat.org>
>  R: Daniel P. Berrange <berrange@redhat.com>
> diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
> index 6eeb0731d7..3beb02afad 100644
> --- a/hw/i2c/pmbus_device.c
> +++ b/hw/i2c/pmbus_device.c
> @@ -89,16 +89,16 @@ void pmbus_send_string(PMBusDevice *pmdev, const char *data)
>  }
>  
>  
> -static uint64_t pmbus_receive_uint(const uint8_t *buf, uint8_t len)
> +static uint64_t pmbus_receive_uint(PMBusDevice *pmdev)
>  {
>      uint64_t ret = 0;
>  
>      /* Exclude command code from return value */
> -    buf++;
> -    len--;
> +    pmdev->in_buf++;
> +    pmdev->in_buf_len--;
>  
> -    for (int i = len - 1; i >= 0; i--) {
> -        ret = ret << 8 | buf[i];
> +    for (int i = pmdev->in_buf_len - 1; i >= 0; i--) {
> +        ret = ret << 8 | pmdev->in_buf[i];
>      }
>      return ret;
>  }
> @@ -110,7 +110,7 @@ uint8_t pmbus_receive8(PMBusDevice *pmdev)
>                        "%s: length mismatch. Expected 1 byte, got %d bytes\n",
>                        __func__, pmdev->in_buf_len - 1);
>      }
> -    return pmbus_receive_uint(pmdev->in_buf, pmdev->in_buf_len);
> +    return pmbus_receive_uint(pmdev);
>  }
>  
>  uint16_t pmbus_receive16(PMBusDevice *pmdev)
> @@ -120,7 +120,7 @@ uint16_t pmbus_receive16(PMBusDevice *pmdev)
>                        "%s: length mismatch. Expected 2 bytes, got %d bytes\n",
>                        __func__, pmdev->in_buf_len - 1);
>      }
> -    return pmbus_receive_uint(pmdev->in_buf, pmdev->in_buf_len);
> +    return pmbus_receive_uint(pmdev);
>  }
>  
>  uint32_t pmbus_receive32(PMBusDevice *pmdev)
> @@ -130,7 +130,7 @@ uint32_t pmbus_receive32(PMBusDevice *pmdev)
>                        "%s: length mismatch. Expected 4 bytes, got %d bytes\n",
>                        __func__, pmdev->in_buf_len - 1);
>      }
> -    return pmbus_receive_uint(pmdev->in_buf, pmdev->in_buf_len);
> +    return pmbus_receive_uint(pmdev);
>  }
>  
>  uint64_t pmbus_receive64(PMBusDevice *pmdev)
> @@ -140,7 +140,7 @@ uint64_t pmbus_receive64(PMBusDevice *pmdev)
>                        "%s: length mismatch. Expected 8 bytes, got %d bytes\n",
>                        __func__, pmdev->in_buf_len - 1);
>      }
> -    return pmbus_receive_uint(pmdev->in_buf, pmdev->in_buf_len);
> +    return pmbus_receive_uint(pmdev);
>  }
>  
>  static uint8_t pmbus_out_buf_pop(PMBusDevice *pmdev)
> -- 
> 2.35.1.616.g0bdcbb4464-goog
>
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index fa8adc2618..3601984b5d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3135,6 +3135,16 @@  F: include/hw/i2c/smbus_master.h
 F: include/hw/i2c/smbus_slave.h
 F: include/hw/i2c/smbus_eeprom.h
 
+PMBus
+M: Titus Rwantare <titusr@google.com>
+S: Maintained
+F: hw/i2c/pmbus_device.c
+F: hw/sensor/adm1272.c
+F: hw/sensor/max34451.c
+F: include/hw/i2c/pmbus_device.h
+F: tests/qtest/adm1272-test.c
+F: tests/qtest/max34451-test.c
+
 Firmware schema specifications
 M: Philippe Mathieu-Daudé <f4bug@amsat.org>
 R: Daniel P. Berrange <berrange@redhat.com>
diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index 6eeb0731d7..3beb02afad 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -89,16 +89,16 @@  void pmbus_send_string(PMBusDevice *pmdev, const char *data)
 }
 
 
-static uint64_t pmbus_receive_uint(const uint8_t *buf, uint8_t len)
+static uint64_t pmbus_receive_uint(PMBusDevice *pmdev)
 {
     uint64_t ret = 0;
 
     /* Exclude command code from return value */
-    buf++;
-    len--;
+    pmdev->in_buf++;
+    pmdev->in_buf_len--;
 
-    for (int i = len - 1; i >= 0; i--) {
-        ret = ret << 8 | buf[i];
+    for (int i = pmdev->in_buf_len - 1; i >= 0; i--) {
+        ret = ret << 8 | pmdev->in_buf[i];
     }
     return ret;
 }
@@ -110,7 +110,7 @@  uint8_t pmbus_receive8(PMBusDevice *pmdev)
                       "%s: length mismatch. Expected 1 byte, got %d bytes\n",
                       __func__, pmdev->in_buf_len - 1);
     }
-    return pmbus_receive_uint(pmdev->in_buf, pmdev->in_buf_len);
+    return pmbus_receive_uint(pmdev);
 }
 
 uint16_t pmbus_receive16(PMBusDevice *pmdev)
@@ -120,7 +120,7 @@  uint16_t pmbus_receive16(PMBusDevice *pmdev)
                       "%s: length mismatch. Expected 2 bytes, got %d bytes\n",
                       __func__, pmdev->in_buf_len - 1);
     }
-    return pmbus_receive_uint(pmdev->in_buf, pmdev->in_buf_len);
+    return pmbus_receive_uint(pmdev);
 }
 
 uint32_t pmbus_receive32(PMBusDevice *pmdev)
@@ -130,7 +130,7 @@  uint32_t pmbus_receive32(PMBusDevice *pmdev)
                       "%s: length mismatch. Expected 4 bytes, got %d bytes\n",
                       __func__, pmdev->in_buf_len - 1);
     }
-    return pmbus_receive_uint(pmdev->in_buf, pmdev->in_buf_len);
+    return pmbus_receive_uint(pmdev);
 }
 
 uint64_t pmbus_receive64(PMBusDevice *pmdev)
@@ -140,7 +140,7 @@  uint64_t pmbus_receive64(PMBusDevice *pmdev)
                       "%s: length mismatch. Expected 8 bytes, got %d bytes\n",
                       __func__, pmdev->in_buf_len - 1);
     }
-    return pmbus_receive_uint(pmdev->in_buf, pmdev->in_buf_len);
+    return pmbus_receive_uint(pmdev);
 }
 
 static uint8_t pmbus_out_buf_pop(PMBusDevice *pmdev)