[PATCH 0/7] Misc NPCM7XX patches

[PATCH 0/7] Misc NPCM7XX patches

[Hao Wu]

This patch set contains a few bug fixes and I2C devices for some
NPCM7XX boards.

Patch 1~2 fix a problem that causes the SMBus module to behave
incorrectly when it's in FIFO mode and trying to receive more than
16 bytes at a time.

Patch 3 fixes a error in a register for ADC module.

Patch 4 makes the ADC input to be R/W instead of write only. It allows
a test system to read these via QMP and has no negative effect.

Patch 5 adds a new aux function for at24c EEPROM. It allows us to attach
a drive to an at24c EEPROM device that is used as the initialized content
of that device.

Patch 6 uses the function defined in patch 5 to add the EEPROM and other
I2C devices for Quanta GBS board.

Patch 7 modifies the Quanta GSJ board to use the new function defined in
patch 5.

Hao Wu (6):
  hw/i2c: Clear ACK bit in NPCM7xx SMBus module
  hw/i2c: Read FIFO during RXF_CTL change in NPCM7XX SMBus
  hw/adc: Fix CONV bit in NPCM7XX ADC CON register
  hw/adc: Make adci[*] R/W in NPCM7XX ADC
  hw/nvram: Add a new auxiliary function to init at24c eeprom
  hw/arm: Use unit number in quanta-gsj eeprom files

Patrick Venture (1):
  hw/arm: quanta-gbs-bmc add i2c devices

 hw/adc/npcm7xx_adc.c            |  4 +-
 hw/arm/npcm7xx_boards.c         | 94 ++++++++++++++++-----------------
 hw/i2c/npcm7xx_smbus.c          |  8 +--
 hw/nvram/eeprom_at24c.c         | 18 +++++++
 include/hw/nvram/eeprom_at24c.h | 13 +++++
 tests/qtest/npcm7xx_adc-test.c  |  2 +-
 6 files changed, 85 insertions(+), 54 deletions(-)
 create mode 100644 include/hw/nvram/eeprom_at24c.h

-- 
2.33.0.rc1.237.g0d66db33f3-goog

[PATCH 1/7] hw/i2c: Clear ACK bit in NPCM7xx SMBus module

[Hao Wu]

The ACK bit in NPCM7XX SMBus module should be cleared each time it
sends out a NACK signal. This patch fixes the bug that it fails to
do so.

Signed-off-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Titus Rwantare <titusr@google.com>
---
 hw/i2c/npcm7xx_smbus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i2c/npcm7xx_smbus.c b/hw/i2c/npcm7xx_smbus.c
index e7e0ba66fe..f18e311556 100644
--- a/hw/i2c/npcm7xx_smbus.c
+++ b/hw/i2c/npcm7xx_smbus.c
@@ -270,7 +270,7 @@ static void npcm7xx_smbus_recv_byte(NPCM7xxSMBusState *s)
     if (s->st & NPCM7XX_SMBCTL1_ACK) {
         trace_npcm7xx_smbus_nack(DEVICE(s)->canonical_path);
         i2c_nack(s->bus);
-        s->st &= NPCM7XX_SMBCTL1_ACK;
+        s->st &= ~NPCM7XX_SMBCTL1_ACK;
     }
     trace_npcm7xx_smbus_recv_byte((DEVICE(s)->canonical_path), s->sda);
     npcm7xx_smbus_update_irq(s);
-- 
2.33.0.rc1.237.g0d66db33f3-goog

[PATCH 2/7] hw/i2c: Read FIFO during RXF_CTL change in NPCM7XX SMBus

[Hao Wu]

Originally we read in from SMBus when RXF_STS is cleared. However,
the driver clears RXF_STS before setting RXF_CTL, causing the SM bus
module to read incorrect amount of bytes in FIFO mode when the number
of bytes read changed. This patch fixes this issue.

Signed-off-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Titus Rwantare <titusr@google.com>
---
 hw/i2c/npcm7xx_smbus.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/i2c/npcm7xx_smbus.c b/hw/i2c/npcm7xx_smbus.c
index f18e311556..1435daea94 100644
--- a/hw/i2c/npcm7xx_smbus.c
+++ b/hw/i2c/npcm7xx_smbus.c
@@ -637,9 +637,6 @@ static void npcm7xx_smbus_write_rxf_sts(NPCM7xxSMBusState *s, uint8_t value)
 {
     if (value & NPCM7XX_SMBRXF_STS_RX_THST) {
         s->rxf_sts &= ~NPCM7XX_SMBRXF_STS_RX_THST;
-        if (s->status == NPCM7XX_SMBUS_STATUS_RECEIVING) {
-            npcm7xx_smbus_recv_fifo(s);
-        }
     }
 }
 
@@ -651,6 +648,9 @@ static void npcm7xx_smbus_write_rxf_ctl(NPCM7xxSMBusState *s, uint8_t value)
         new_ctl = KEEP_OLD_BIT(s->rxf_ctl, new_ctl, NPCM7XX_SMBRXF_CTL_LAST);
     }
     s->rxf_ctl = new_ctl;
+    if (s->status == NPCM7XX_SMBUS_STATUS_RECEIVING) {
+        npcm7xx_smbus_recv_fifo(s);
+    }
 }
 
 static uint64_t npcm7xx_smbus_read(void *opaque, hwaddr offset, unsigned size)
-- 
2.33.0.rc1.237.g0d66db33f3-goog

[PATCH 3/7] hw/adc: Fix CONV bit in NPCM7XX ADC CON register

[Hao Wu]

The correct bit for the CONV bit in NPCM7XX ADC is bit 13. This patch
fixes that in the module, and also lower the IRQ when the guest
is done handling an interrupt event from the ADC module.

Signed-off-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Patrick Venture<venture@google.com>
---
 hw/adc/npcm7xx_adc.c           | 2 +-
 tests/qtest/npcm7xx_adc-test.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/adc/npcm7xx_adc.c b/hw/adc/npcm7xx_adc.c
index 0f0a9f63e2..47fb9e5f74 100644
--- a/hw/adc/npcm7xx_adc.c
+++ b/hw/adc/npcm7xx_adc.c
@@ -36,7 +36,7 @@ REG32(NPCM7XX_ADC_DATA, 0x4)
 #define NPCM7XX_ADC_CON_INT     BIT(18)
 #define NPCM7XX_ADC_CON_EN      BIT(17)
 #define NPCM7XX_ADC_CON_RST     BIT(16)
-#define NPCM7XX_ADC_CON_CONV    BIT(14)
+#define NPCM7XX_ADC_CON_CONV    BIT(13)
 #define NPCM7XX_ADC_CON_DIV(rv) extract32(rv, 1, 8)
 
 #define NPCM7XX_ADC_MAX_RESULT      1023
diff --git a/tests/qtest/npcm7xx_adc-test.c b/tests/qtest/npcm7xx_adc-test.c
index 5ce8ce13b3..aaf127dd42 100644
--- a/tests/qtest/npcm7xx_adc-test.c
+++ b/tests/qtest/npcm7xx_adc-test.c
@@ -50,7 +50,7 @@
 #define CON_INT     BIT(18)
 #define CON_EN      BIT(17)
 #define CON_RST     BIT(16)
-#define CON_CONV    BIT(14)
+#define CON_CONV    BIT(13)
 #define CON_DIV(rv) extract32(rv, 1, 8)
 
 #define FST_RDST    BIT(1)
-- 
2.33.0.rc1.237.g0d66db33f3-goog

[PATCH 4/7] hw/adc: Make adci[*] R/W in NPCM7XX ADC

[Hao Wu]

Our sensor test requires both reading and writing from a sensor's
QOM property. So we need to make the input of ADC module R/W instead
of read only for that to work.

Signed-off-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Titus Rwantare <titusr@google.com>
---
 hw/adc/npcm7xx_adc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/adc/npcm7xx_adc.c b/hw/adc/npcm7xx_adc.c
index 47fb9e5f74..bc6f3f55e6 100644
--- a/hw/adc/npcm7xx_adc.c
+++ b/hw/adc/npcm7xx_adc.c
@@ -242,7 +242,7 @@ static void npcm7xx_adc_init(Object *obj)
 
     for (i = 0; i < NPCM7XX_ADC_NUM_INPUTS; ++i) {
         object_property_add_uint32_ptr(obj, "adci[*]",
-                &s->adci[i], OBJ_PROP_FLAG_WRITE);
+                &s->adci[i], OBJ_PROP_FLAG_READWRITE);
     }
     object_property_add_uint32_ptr(obj, "vref",
             &s->vref, OBJ_PROP_FLAG_WRITE);
-- 
2.33.0.rc1.237.g0d66db33f3-goog

[PATCH 5/7] hw/nvram: Add a new auxiliary function to init at24c eeprom

[Hao Wu]

In NPCM7xx boards, at24c eeproms are backed by drives. However,
these drives use unit number as unique identifier. So if we
specify two drives with the same unit number, error will occured:
`Device with id 'none85' exists`.

Instead of using i2c address as unit number, we now assign unique
unit numbers for each eeproms in each board. This avoids conflict
in providing multiple eeprom contents with the same address.

We add an auxiliary function in the at24c eeprom module for this.
This allows it to easily add at24c eeprom to non-nuvoton boards
like aspeed as well.

Signed-off-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Patrick Venture<venture@google.com>
---
 hw/nvram/eeprom_at24c.c         | 18 ++++++++++++++++++
 include/hw/nvram/eeprom_at24c.h | 13 +++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 include/hw/nvram/eeprom_at24c.h

diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c
index af6f5dbb99..a9e3702b00 100644
--- a/hw/nvram/eeprom_at24c.c
+++ b/hw/nvram/eeprom_at24c.c
@@ -12,9 +12,11 @@
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "hw/i2c/i2c.h"
+#include "hw/nvram/eeprom_at24c.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
 #include "sysemu/block-backend.h"
+#include "sysemu/blockdev.h"
 #include "qom/object.h"
 
 /* #define DEBUG_AT24C */
@@ -205,3 +207,19 @@ static void at24c_eeprom_register(void)
 }
 
 type_init(at24c_eeprom_register)
+
+void at24c_eeprom_init_one(I2CBus *i2c_bus, int bus, uint8_t addr,
+                           uint32_t rsize, int unit_number)
+{
+    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
+    DeviceState *dev = DEVICE(i2c_dev);
+    BlockInterfaceType type = IF_NONE;
+    DriveInfo *dinfo;
+
+    dinfo = drive_get(type, bus, unit_number);
+    if (dinfo) {
+        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
+    }
+    qdev_prop_set_uint32(dev, "rom-size", rsize);
+    i2c_slave_realize_and_unref(i2c_dev, i2c_bus, &error_abort);
+}
diff --git a/include/hw/nvram/eeprom_at24c.h b/include/hw/nvram/eeprom_at24c.h
new file mode 100644
index 0000000000..b649557a07
--- /dev/null
+++ b/include/hw/nvram/eeprom_at24c.h
@@ -0,0 +1,13 @@
+/*
+ * *AT24C* series I2C EEPROM
+ *
+ * Copyright (c) 2015 Michael Davidsaver
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the LICENSE file in the top-level directory.
+ */
+
+/* Init one at24c eeprom device */
+void at24c_eeprom_init_one(I2CBus *i2c_bus, int bus, uint8_t addr,
+                           uint32_t rsize, int unit_number);
+
-- 
2.33.0.rc1.237.g0d66db33f3-goog

[PATCH 6/7] hw/arm: quanta-gbs-bmc add i2c devices

[Hao Wu]

From: Patrick Venture <venture@google.com>

Adds supported i2c devices to the quanta-gbc-bmc board.

Signed-off-by: Patrick Venture <venture@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
---
 hw/arm/npcm7xx_boards.c | 79 +++++++++++++++++++++++------------------
 1 file changed, 45 insertions(+), 34 deletions(-)

diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
index e5a3243995..54cf9785ec 100644
--- a/hw/arm/npcm7xx_boards.c
+++ b/hw/arm/npcm7xx_boards.c
@@ -21,6 +21,7 @@
 #include "hw/i2c/i2c_mux_pca954x.h"
 #include "hw/i2c/smbus_eeprom.h"
 #include "hw/loader.h"
+#include "hw/nvram/eeprom_at24c.h"
 #include "hw/qdev-core.h"
 #include "hw/qdev-properties.h"
 #include "qapi/error.h"
@@ -252,11 +253,12 @@ static void quanta_gsj_fan_init(NPCM7xxMachine *machine, NPCM7xxState *soc)
 
 static void quanta_gbs_i2c_init(NPCM7xxState *soc)
 {
-    /*
-     * i2c-0:
-     *     pca9546@71
-     *
-     * i2c-1:
+    I2CSlave *i2c_mux;
+
+    /* i2c-0: */
+    i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 0), TYPE_PCA9546, 0x71);
+
+    /* i2c-1:
      *     pca9535@24
      *     pca9535@20
      *     pca9535@21
@@ -264,47 +266,56 @@ static void quanta_gbs_i2c_init(NPCM7xxState *soc)
      *     pca9535@23
      *     pca9535@25
      *     pca9535@26
-     *
-     * i2c-2:
-     *     sbtsi@4c
-     *
-     * i2c-5:
-     *     atmel,24c64@50 mb_fru
-     *     pca9546@71
-     *         - channel 0: max31725@54
-     *         - channel 1: max31725@55
-     *         - channel 2: max31725@5d
-     *                      atmel,24c64@51 fan_fru
-     *         - channel 3: atmel,24c64@52 hsbp_fru
-     *
-     * i2c-6:
+     */
+
+    /* i2c-2: sbtsi@4c */
+
+    /* i2c-5: */
+    /* mb_fru */
+    at24c_eeprom_init_one(npcm7xx_i2c_get_bus(soc, 5), 5, 0x50, 8192, 0);
+    i2c_mux = i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 5),
+                                      TYPE_PCA9546, 0x71);
+    /* max31725 is tmp105 compatible. */
+    i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 0), "tmp105", 0x54);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 1), "tmp105", 0x55);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 2), "tmp105", 0x5d);
+    /* fan_fru */
+    at24c_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 2), 5, 0x51, 8192, 1);
+    /* hsbp_fru */
+    at24c_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 3), 5, 0x52, 8192, 2);
+
+    /* i2c-6:
      *     pca9545@73
      *
      * i2c-7:
      *     pca9545@72
-     *
-     * i2c-8:
-     *     adi,adm1272@10
-     *
-     * i2c-9:
-     *     pca9546@71
-     *         - channel 0: isil,isl68137@60
+     */
+
+    /* i2c-8: */
+    i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 8), "adm1272", 0x10);
+
+    /* i2c-9: */
+    i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 9), TYPE_PCA9546, 0x71);
+    /*         - channel 0: isil,isl68137@60
      *         - channel 1: isil,isl68137@61
      *         - channel 2: isil,isl68137@63
      *         - channel 3: isil,isl68137@45
-     *
-     * i2c-10:
+     */
+
+    /* i2c-10:
      *     pca9545@71
      *
      * i2c-11:
      *     pca9545@76
-     *
-     * i2c-12:
-     *     maxim,max34451@4e
-     *     isil,isl68137@5d
+     */
+
+    /* i2c-12: */
+    i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 12), "max34451", 0x4e);
+    /*     isil,isl68137@5d
      *     isil,isl68137@5e
-     *
-     * i2c-14:
+     */
+
+    /* i2c-14:
      *     pca9545@70
      */
 }
-- 
2.33.0.rc1.237.g0d66db33f3-goog

[PATCH 7/7] hw/arm: Use unit number in quanta-gsj eeprom files

[Hao Wu]

Use unique unit numbers in quanta-gsj eeprom files.

Signed-off-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Patrick Venture<venture@google.com>
---
 hw/arm/npcm7xx_boards.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
index 54cf9785ec..be6c81b29d 100644
--- a/hw/arm/npcm7xx_boards.c
+++ b/hw/arm/npcm7xx_boards.c
@@ -107,17 +107,6 @@ static I2CBus *npcm7xx_i2c_get_bus(NPCM7xxState *soc, uint32_t num)
     return I2C_BUS(qdev_get_child_bus(DEVICE(&soc->smbus[num]), "i2c-bus"));
 }
 
-static void at24c_eeprom_init(NPCM7xxState *soc, int bus, uint8_t addr,
-                              uint32_t rsize)
-{
-    I2CBus *i2c_bus = npcm7xx_i2c_get_bus(soc, bus);
-    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
-    DeviceState *dev = DEVICE(i2c_dev);
-
-    qdev_prop_set_uint32(dev, "rom-size", rsize);
-    i2c_slave_realize_and_unref(i2c_dev, i2c_bus, &error_abort);
-}
-
 static void npcm7xx_init_pwm_splitter(NPCM7xxMachine *machine,
                                       NPCM7xxState *soc, const int *fan_counts)
 {
@@ -220,8 +209,8 @@ static void quanta_gsj_i2c_init(NPCM7xxState *soc)
     i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 3), "tmp105", 0x5c);
     i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 4), "tmp105", 0x5c);
 
-    at24c_eeprom_init(soc, 9, 0x55, 8192);
-    at24c_eeprom_init(soc, 10, 0x55, 8192);
+    at24c_eeprom_init_one(npcm7xx_i2c_get_bus(soc, 9), 9, 0x55, 8192, 0);
+    at24c_eeprom_init_one(npcm7xx_i2c_get_bus(soc, 10), 10, 0x55, 8192, 1);
 
     /*
      * i2c-11:
-- 
2.33.0.rc1.237.g0d66db33f3-goog

Re: [PATCH 6/7] hw/arm: quanta-gbs-bmc add i2c devices

[Peter Maydell]

On Sat, 14 Aug 2021 at 00:34, Hao Wu <wuhaotsh@google.com> wrote:
>
> From: Patrick Venture <venture@google.com>
>
> Adds supported i2c devices to the quanta-gbc-bmc board.
>
> Signed-off-by: Patrick Venture <venture@google.com>
> Reviewed-by: Hao Wu <wuhaotsh@google.com>
> ---
>  hw/arm/npcm7xx_boards.c | 79 +++++++++++++++++++++++------------------
>  1 file changed, 45 insertions(+), 34 deletions(-)
>

Does the documentation need updating to mention these newly supported devices ?

>  static void quanta_gbs_i2c_init(NPCM7xxState *soc)
>  {
> -    /*
> -     * i2c-0:
> -     *     pca9546@71
> -     *
> -     * i2c-1:
> +    I2CSlave *i2c_mux;
> +
> +    /* i2c-0: */
> +    i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 0), TYPE_PCA9546, 0x71);
> +
> +    /* i2c-1:
>       *     pca9535@24
>       *     pca9535@20
>       *     pca9535@21

This leaves this multiline block comment with the wrong format: QEMU's
coding style wants the leading "/*" on a line of its own. Similarly
for some others in this patch.

thanks
-- PMM

Re: [PATCH 5/7] hw/nvram: Add a new auxiliary function to init at24c eeprom

[Peter Maydell]

On Sat, 14 Aug 2021 at 00:34, Hao Wu <wuhaotsh@google.com> wrote:
>
> In NPCM7xx boards, at24c eeproms are backed by drives. However,
> these drives use unit number as unique identifier. So if we
> specify two drives with the same unit number, error will occured:
> `Device with id 'none85' exists`.
>
> Instead of using i2c address as unit number, we now assign unique
> unit numbers for each eeproms in each board. This avoids conflict
> in providing multiple eeprom contents with the same address.
>
> We add an auxiliary function in the at24c eeprom module for this.
> This allows it to easily add at24c eeprom to non-nuvoton boards
> like aspeed as well.
>
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> Reviewed-by: Patrick Venture<venture@google.com>
> ---
>  hw/nvram/eeprom_at24c.c         | 18 ++++++++++++++++++
>  include/hw/nvram/eeprom_at24c.h | 13 +++++++++++++
>  2 files changed, 31 insertions(+)
>  create mode 100644 include/hw/nvram/eeprom_at24c.h
>
> diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c
> index af6f5dbb99..a9e3702b00 100644
> --- a/hw/nvram/eeprom_at24c.c
> +++ b/hw/nvram/eeprom_at24c.c
> @@ -12,9 +12,11 @@
>  #include "qapi/error.h"
>  #include "qemu/module.h"
>  #include "hw/i2c/i2c.h"
> +#include "hw/nvram/eeprom_at24c.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/qdev-properties-system.h"
>  #include "sysemu/block-backend.h"
> +#include "sysemu/blockdev.h"
>  #include "qom/object.h"
>
>  /* #define DEBUG_AT24C */
> @@ -205,3 +207,19 @@ static void at24c_eeprom_register(void)
>  }
>
>  type_init(at24c_eeprom_register)
> +
> +void at24c_eeprom_init_one(I2CBus *i2c_bus, int bus, uint8_t addr,
> +                           uint32_t rsize, int unit_number)
> +{
> +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
> +    DeviceState *dev = DEVICE(i2c_dev);
> +    BlockInterfaceType type = IF_NONE;
> +    DriveInfo *dinfo;
> +
> +    dinfo = drive_get(type, bus, unit_number);
> +    if (dinfo) {
> +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
> +    }
> +    qdev_prop_set_uint32(dev, "rom-size", rsize);
> +    i2c_slave_realize_and_unref(i2c_dev, i2c_bus, &error_abort);
> +}

This is just creating a device and configuring it, right?
We tend to prefer that this is just done directly in the
board or SoC code these days, rather than providing helper functions
for it.

If you don't like what hw/arm/npcm7xx_boards.c:at24c_eeprom_init()
is currently doing, you should just patch it to do something different.

thanks
-- PMM

Re: [PATCH 0/7] Misc NPCM7XX patches

[Peter Maydell]

On Sat, 14 Aug 2021 at 00:34, Hao Wu <wuhaotsh@google.com> wrote:
>
> This patch set contains a few bug fixes and I2C devices for some
> NPCM7XX boards.
>
> Patch 1~2 fix a problem that causes the SMBus module to behave
> incorrectly when it's in FIFO mode and trying to receive more than
> 16 bytes at a time.
>
> Patch 3 fixes a error in a register for ADC module.
>
> Patch 4 makes the ADC input to be R/W instead of write only. It allows
> a test system to read these via QMP and has no negative effect.
>
> Patch 5 adds a new aux function for at24c EEPROM. It allows us to attach
> a drive to an at24c EEPROM device that is used as the initialized content
> of that device.
>
> Patch 6 uses the function defined in patch 5 to add the EEPROM and other
> I2C devices for Quanta GBS board.
>
> Patch 7 modifies the Quanta GSJ board to use the new function defined in
> patch 5.

I've left review comments on a few of the patches. The first 4 look
fine; I just haven't left r-by tags because I didn't go and look
up the specs for the hardware.

thanks
-- PMM

Re: [PATCH 4/7] hw/adc: Make adci[*] R/W in NPCM7XX ADC

[Philippe Mathieu-Daudé]

On 8/14/21 1:33 AM, Hao Wu wrote:
> Our sensor test requires both reading and writing from a sensor's
> QOM property. So we need to make the input of ADC module R/W instead
> of read only for that to work.
> 
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> Reviewed-by: Titus Rwantare <titusr@google.com>
> ---
>  hw/adc/npcm7xx_adc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/adc/npcm7xx_adc.c b/hw/adc/npcm7xx_adc.c
> index 47fb9e5f74..bc6f3f55e6 100644
> --- a/hw/adc/npcm7xx_adc.c
> +++ b/hw/adc/npcm7xx_adc.c
> @@ -242,7 +242,7 @@ static void npcm7xx_adc_init(Object *obj)
>  
>      for (i = 0; i < NPCM7XX_ADC_NUM_INPUTS; ++i) {
>          object_property_add_uint32_ptr(obj, "adci[*]",
> -                &s->adci[i], OBJ_PROP_FLAG_WRITE);
> +                &s->adci[i], OBJ_PROP_FLAG_READWRITE);

What about:

  qtest_enabled() ? OBJ_PROP_FLAG_READWRITE : OBJ_PROP_FLAG_WRITE

>      }
>      object_property_add_uint32_ptr(obj, "vref",
>              &s->vref, OBJ_PROP_FLAG_WRITE);
>