[PATCH 0/9] Add Qualcomm BMC machines

[PATCH 0/9] Add Qualcomm BMC machines

[Jae Hyun Yoo]

Hello,

I'm sending a series to add Qualcomm BMC machines that are equipped with
Aspeed AST2600 SoC. Also, this series adds MAX31785 fan controller device
emulation. Please help to review.

Thanks,

Jae

Graeme Gregory (2):
  hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device
  hw/arm/aspeed: add Qualcomm Firework machine and FRU device

Jae Hyun Yoo (3):
  hw/arm/aspeed: add support for the Qualcomm EVB proto board
  hw/arm/aspeed: add support for the Qualcomm DC-SCM v1 board
  hw/arm/aspeed: firework: add I2C MUXes for VR channels

Maheswara Kurapati (4):
  hw/i2c: pmbus: Page #255 is valid page for read requests.
  hw/sensor: add Maxim MAX31785 device
  hw/arm/aspeed: firework: Add MAX31785 Fan controllers
  hw/arm/aspeed: firework: Add Thermal Diodes

 hw/arm/Kconfig        |   1 +
 hw/arm/aspeed.c       | 158 +++++++++++-
 hw/i2c/pmbus_device.c |   1 -
 hw/sensor/Kconfig     |   4 +
 hw/sensor/max31785.c  | 580 ++++++++++++++++++++++++++++++++++++++++++
 hw/sensor/meson.build |   1 +
 6 files changed, 742 insertions(+), 3 deletions(-)
 create mode 100644 hw/sensor/max31785.c

-- 
2.25.1

[PATCH 1/9] hw/arm/aspeed: add support for the Qualcomm EVB proto board

[Jae Hyun Yoo]

Add qcom-evb-proto board support.

Signed-off-by: Jae Hyun Yoo <quic_jaehyoo@quicinc.com>
---
 hw/arm/aspeed.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 98dc185acd9a..4f54721e5f1f 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1420,6 +1420,26 @@ static void aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
     amc->macs_mask = 0;
 }
 
+static void aspeed_machine_qcom_evb_proto_class_init(ObjectClass *oc,
+                                                     void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+    mc->desc       = "Qualcomm EVB Proto using AST2600 EVB (Cortex A7)";
+    amc->soc_name  = "ast2600-a3";
+    amc->hw_strap1 = AST2600_EVB_HW_STRAP1;
+    amc->hw_strap2 = AST2600_EVB_HW_STRAP2;
+    amc->fmc_model = "n25q512a";
+    amc->spi_model = "n25q512a";
+    amc->num_cs    = 2;
+    amc->macs_mask = ASPEED_MAC1_ON | ASPEED_MAC2_ON | ASPEED_MAC3_ON;
+    amc->i2c_init  = ast2600_evb_i2c_init;
+    mc->default_ram_size = 1 * GiB;
+    mc->default_cpus = mc->min_cpus = mc->max_cpus =
+        aspeed_soc_num_cpus(amc->soc_name);
+};
+
 static const TypeInfo aspeed_machine_types[] = {
     {
         .name          = MACHINE_TYPE_NAME("palmetto-bmc"),
@@ -1457,6 +1477,10 @@ static const TypeInfo aspeed_machine_types[] = {
         .name          = MACHINE_TYPE_NAME("g220a-bmc"),
         .parent        = TYPE_ASPEED_MACHINE,
         .class_init    = aspeed_machine_g220a_class_init,
+    }, {
+        .name          = MACHINE_TYPE_NAME("qcom-evb-proto-bmc"),
+        .parent        = TYPE_ASPEED_MACHINE,
+        .class_init    = aspeed_machine_qcom_evb_proto_class_init,
     }, {
         .name          = MACHINE_TYPE_NAME("fp5280g2-bmc"),
         .parent        = TYPE_ASPEED_MACHINE,
-- 
2.25.1

[PATCH 2/9] hw/arm/aspeed: add support for the Qualcomm DC-SCM v1 board

[Jae Hyun Yoo]

Add qcom-dc-scm-v1 board support.

Signed-off-by: Jae Hyun Yoo <quic_jaehyoo@quicinc.com>
---
 hw/arm/aspeed.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 4f54721e5f1f..785cc543d046 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -174,6 +174,10 @@ struct AspeedMachineState {
 #define BLETCHLEY_BMC_HW_STRAP1 AST2600_EVB_HW_STRAP1
 #define BLETCHLEY_BMC_HW_STRAP2 AST2600_EVB_HW_STRAP2
 
+/* Muvia DC-SCM hardware value */
+#define QCOM_DC_SCM_V1_BMC_HW_STRAP1  0x00000000
+#define QCOM_DC_SCM_V1_BMC_HW_STRAP2  0x00000041
+
 /*
  * The max ram region is for firmwares that scan the address space
  * with load/store to guess how much RAM the SoC has.
@@ -988,6 +992,19 @@ static void fby35_i2c_init(AspeedMachineState *bmc)
      */
 }
 
+static void qcom_dc_scm_bmc_i2c_init(AspeedMachineState *bmc)
+{
+    AspeedSoCState *soc = &bmc->soc;
+
+    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 15), "tmp105", 0x4d);
+
+    uint8_t *eeprom_buf = g_malloc0(128 * 1024);
+    if (eeprom_buf) {
+        smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53,
+                              eeprom_buf);
+    }
+}
+
 static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
 {
     return ASPEED_MACHINE(obj)->mmio_exec;
@@ -1440,6 +1457,26 @@ static void aspeed_machine_qcom_evb_proto_class_init(ObjectClass *oc,
         aspeed_soc_num_cpus(amc->soc_name);
 };
 
+static void aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
+                                                     void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+    mc->desc       = "Qualcomm DC-SCM V1 BMC (Cortex A7)";
+    amc->soc_name  = "ast2600-a3";
+    amc->hw_strap1 = QCOM_DC_SCM_V1_BMC_HW_STRAP1;
+    amc->hw_strap2 = QCOM_DC_SCM_V1_BMC_HW_STRAP2;
+    amc->fmc_model = "n25q512a";
+    amc->spi_model = "n25q512a";
+    amc->num_cs    = 2;
+    amc->macs_mask = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
+    amc->i2c_init  = qcom_dc_scm_bmc_i2c_init;
+    mc->default_ram_size = 1 * GiB;
+    mc->default_cpus = mc->min_cpus = mc->max_cpus =
+        aspeed_soc_num_cpus(amc->soc_name);
+};
+
 static const TypeInfo aspeed_machine_types[] = {
     {
         .name          = MACHINE_TYPE_NAME("palmetto-bmc"),
@@ -1481,6 +1518,10 @@ static const TypeInfo aspeed_machine_types[] = {
         .name          = MACHINE_TYPE_NAME("qcom-evb-proto-bmc"),
         .parent        = TYPE_ASPEED_MACHINE,
         .class_init    = aspeed_machine_qcom_evb_proto_class_init,
+    }, {
+        .name          = MACHINE_TYPE_NAME("qcom-dc-scm-v1-bmc"),
+        .parent        = TYPE_ASPEED_MACHINE,
+        .class_init    = aspeed_machine_qcom_dc_scm_v1_class_init,
     }, {
         .name          = MACHINE_TYPE_NAME("fp5280g2-bmc"),
         .parent        = TYPE_ASPEED_MACHINE,
-- 
2.25.1

[PATCH 3/9] hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device

[Jae Hyun Yoo]

From: Graeme Gregory <quic_ggregory@quicinc.com>

The FRU device uses the index 0 device on bus IF_NONE.

-drive file=$FRU,format=raw,if=none

file must match FRU size of 128k

Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com>
---
 hw/arm/aspeed.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 785cc543d046..36d6b2c33e48 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -992,17 +992,29 @@ static void fby35_i2c_init(AspeedMachineState *bmc)
      */
 }
 
+static void qcom_dc_scm_fru_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
+{
+    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
+    DeviceState *dev = DEVICE(i2c_dev);
+    /* Use First Index for DC-SCM FRU */
+    DriveInfo *dinfo = drive_get(IF_NONE, 0, 0);
+
+    qdev_prop_set_uint32(dev, "rom-size", rsize);
+
+    if (dinfo) {
+        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
+    }
+
+    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
+}
+
 static void qcom_dc_scm_bmc_i2c_init(AspeedMachineState *bmc)
 {
     AspeedSoCState *soc = &bmc->soc;
 
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 15), "tmp105", 0x4d);
 
-    uint8_t *eeprom_buf = g_malloc0(128 * 1024);
-    if (eeprom_buf) {
-        smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53,
-                              eeprom_buf);
-    }
+    qcom_dc_scm_fru_init(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53, 128 * 1024);
 }
 
 static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
-- 
2.25.1

Re: [PATCH 3/9] hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device

[Joel Stanley]

On Wed, 22 Jun 2022 at 17:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
>
> From: Graeme Gregory <quic_ggregory@quicinc.com>
>
> The FRU device uses the index 0 device on bus IF_NONE.
>
> -drive file=$FRU,format=raw,if=none
>
> file must match FRU size of 128k
>
> Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com>
> ---
>  hw/arm/aspeed.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 785cc543d046..36d6b2c33e48 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -992,17 +992,29 @@ static void fby35_i2c_init(AspeedMachineState *bmc)
>       */
>  }
>
> +static void qcom_dc_scm_fru_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
> +{
> +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
> +    DeviceState *dev = DEVICE(i2c_dev);
> +    /* Use First Index for DC-SCM FRU */
> +    DriveInfo *dinfo = drive_get(IF_NONE, 0, 0);
> +
> +    qdev_prop_set_uint32(dev, "rom-size", rsize);
> +
> +    if (dinfo) {
> +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
> +    }
> +
> +    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
> +}
> +
>  static void qcom_dc_scm_bmc_i2c_init(AspeedMachineState *bmc)
>  {
>      AspeedSoCState *soc = &bmc->soc;
>
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 15), "tmp105", 0x4d);
>
> -    uint8_t *eeprom_buf = g_malloc0(128 * 1024);
> -    if (eeprom_buf) {
> -        smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53,
> -                              eeprom_buf);
> -    }

Again, it's strange to see code that was just added being removed. If
you want the FRU to be in its own patch then remove the eeprom from
the previous patch.

> +    qcom_dc_scm_fru_init(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53, 128 * 1024);
>  }
>
>  static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
> --
> 2.25.1
>

Re: [PATCH 3/9] hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device

[Jae Hyun Yoo]

On 6/22/2022 10:28 PM, Joel Stanley wrote:
> On Wed, 22 Jun 2022 at 17:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
>>
>> From: Graeme Gregory <quic_ggregory@quicinc.com>
>>
>> The FRU device uses the index 0 device on bus IF_NONE.
>>
>> -drive file=$FRU,format=raw,if=none
>>
>> file must match FRU size of 128k
>>
>> Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com>
>> ---
>>   hw/arm/aspeed.c | 22 +++++++++++++++++-----
>>   1 file changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index 785cc543d046..36d6b2c33e48 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -992,17 +992,29 @@ static void fby35_i2c_init(AspeedMachineState *bmc)
>>        */
>>   }
>>
>> +static void qcom_dc_scm_fru_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
>> +{
>> +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
>> +    DeviceState *dev = DEVICE(i2c_dev);
>> +    /* Use First Index for DC-SCM FRU */
>> +    DriveInfo *dinfo = drive_get(IF_NONE, 0, 0);
>> +
>> +    qdev_prop_set_uint32(dev, "rom-size", rsize);
>> +
>> +    if (dinfo) {
>> +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
>> +    }
>> +
>> +    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
>> +}
>> +
>>   static void qcom_dc_scm_bmc_i2c_init(AspeedMachineState *bmc)
>>   {
>>       AspeedSoCState *soc = &bmc->soc;
>>
>>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 15), "tmp105", 0x4d);
>>
>> -    uint8_t *eeprom_buf = g_malloc0(128 * 1024);
>> -    if (eeprom_buf) {
>> -        smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53,
>> -                              eeprom_buf);
>> -    }
> 
> Again, it's strange to see code that was just added being removed. If
> you want the FRU to be in its own patch then remove the eeprom from
> the previous patch.

I see. I'll remove it from the 2/9 patch.

Thanks,
Jae

>> +    qcom_dc_scm_fru_init(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53, 128 * 1024);
>>   }
>>
>>   static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
>> --
>> 2.25.1
>>

Re: [PATCH 3/9] hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device

[Patrick Venture]

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

On Wed, Jun 22, 2022 at 10:48 AM Jae Hyun Yoo <quic_jaehyoo@quicinc.com>
wrote:

> From: Graeme Gregory <quic_ggregory@quicinc.com>
>
> The FRU device uses the index 0 device on bus IF_NONE.
>
> -drive file=$FRU,format=raw,if=none
>
> file must match FRU size of 128k
>
> Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com>
> ---
>  hw/arm/aspeed.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 785cc543d046..36d6b2c33e48 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -992,17 +992,29 @@ static void fby35_i2c_init(AspeedMachineState *bmc)
>       */
>  }
>
> +static void qcom_dc_scm_fru_init(I2CBus *bus, uint8_t addr, uint32_t
> rsize)
> +{
> +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
> +    DeviceState *dev = DEVICE(i2c_dev);
> +    /* Use First Index for DC-SCM FRU */
> +    DriveInfo *dinfo = drive_get(IF_NONE, 0, 0);
> +
> +    qdev_prop_set_uint32(dev, "rom-size", rsize);
> +
> +    if (dinfo) {
> +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
> +    }
> +
> +    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
> +}
>

We've sent a similar patch up for the at24c but in its own file -- but
looking at this, we could likely expand it to suite our cases as well - is
there a reason it's named qcom_dc_scm_fru_init?  Presumably that's to
handle the drive_get parameters.  If you make it use `drive_get(IF_NONE,
bus, unit);` You'll be able to associate a drive via parameters like you
aim to.


> +
>  static void qcom_dc_scm_bmc_i2c_init(AspeedMachineState *bmc)
>  {
>      AspeedSoCState *soc = &bmc->soc;
>
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 15), "tmp105",
> 0x4d);
>
> -    uint8_t *eeprom_buf = g_malloc0(128 * 1024);
> -    if (eeprom_buf) {
> -        smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53,
> -                              eeprom_buf);
> -    }
> +    qcom_dc_scm_fru_init(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53, 128 *
> 1024);
>  }
>
>  static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
> --
> 2.25.1
>
>
>

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

Re: [PATCH 3/9] hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device

[Jae Hyun Yoo]

Hello Patrick,

On 6/23/2022 8:28 AM, Patrick Venture wrote:
> 
> 
> On Wed, Jun 22, 2022 at 10:48 AM Jae Hyun Yoo <quic_jaehyoo@quicinc.com 
> <mailto:quic_jaehyoo@quicinc.com>> wrote:
> 
>     From: Graeme Gregory <quic_ggregory@quicinc.com
>     <mailto:quic_ggregory@quicinc.com>>
> 
>     The FRU device uses the index 0 device on bus IF_NONE.
> 
>     -drive file=$FRU,format=raw,if=none
> 
>     file must match FRU size of 128k
> 
>     Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com
>     <mailto:quic_ggregory@quicinc.com>>
>     ---
>       hw/arm/aspeed.c | 22 +++++++++++++++++-----
>       1 file changed, 17 insertions(+), 5 deletions(-)
> 
>     diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>     index 785cc543d046..36d6b2c33e48 100644
>     --- a/hw/arm/aspeed.c
>     +++ b/hw/arm/aspeed.c
>     @@ -992,17 +992,29 @@ static void fby35_i2c_init(AspeedMachineState
>     *bmc)
>            */
>       }
> 
>     +static void qcom_dc_scm_fru_init(I2CBus *bus, uint8_t addr,
>     uint32_t rsize)
>     +{
>     +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
>     +    DeviceState *dev = DEVICE(i2c_dev);
>     +    /* Use First Index for DC-SCM FRU */
>     +    DriveInfo *dinfo = drive_get(IF_NONE, 0, 0);
>     +
>     +    qdev_prop_set_uint32(dev, "rom-size", rsize);
>     +
>     +    if (dinfo) {
>     +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
>     +    }
>     +
>     +    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
>     +}
> 
> 
> We've sent a similar patch up for the at24c but in its own file -- but 
> looking at this, we could likely expand it to suite our cases as well - 
> is there a reason it's named qcom_dc_scm_fru_init?  Presumably that's to 
> handle the drive_get parameters.  If you make it use `drive_get(IF_NONE, 
> bus, unit);` You'll be able to associate a drive via parameters like you 
> aim to.

Okay. I agree with you that it can be more generic to be used for other
machines too. I'll rewrite this function in v2 to make it have below
shape.

static void
at24c_eeprom_init_from_drive(I2CBus *bus, uint8_t addr, uint32_t rsize,
                              int bus, int unit)

Thanks,
Jae

>     +
>       static void qcom_dc_scm_bmc_i2c_init(AspeedMachineState *bmc)
>       {
>           AspeedSoCState *soc = &bmc->soc;
> 
>           i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 15),
>     "tmp105", 0x4d);
> 
>     -    uint8_t *eeprom_buf = g_malloc0(128 * 1024);
>     -    if (eeprom_buf) {
>     -        smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53,
>     -                              eeprom_buf);
>     -    }
>     +    qcom_dc_scm_fru_init(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53,
>     128 * 1024);
>       }
> 
>       static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
>     -- 
>     2.25.1
> 
> 

Re: [PATCH 3/9] hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device

[Cédric Le Goater]

On 6/23/22 18:34, Jae Hyun Yoo wrote:
> Hello Patrick,
> 
> On 6/23/2022 8:28 AM, Patrick Venture wrote:
>>
>>
>> On Wed, Jun 22, 2022 at 10:48 AM Jae Hyun Yoo <quic_jaehyoo@quicinc.com <mailto:quic_jaehyoo@quicinc.com>> wrote:
>>
>>     From: Graeme Gregory <quic_ggregory@quicinc.com
>>     <mailto:quic_ggregory@quicinc.com>>
>>
>>     The FRU device uses the index 0 device on bus IF_NONE.
>>
>>     -drive file=$FRU,format=raw,if=none
>>
>>     file must match FRU size of 128k
>>
>>     Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com
>>     <mailto:quic_ggregory@quicinc.com>>
>>     ---
>>       hw/arm/aspeed.c | 22 +++++++++++++++++-----
>>       1 file changed, 17 insertions(+), 5 deletions(-)
>>
>>     diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>>     index 785cc543d046..36d6b2c33e48 100644
>>     --- a/hw/arm/aspeed.c
>>     +++ b/hw/arm/aspeed.c
>>     @@ -992,17 +992,29 @@ static void fby35_i2c_init(AspeedMachineState
>>     *bmc)
>>            */
>>       }
>>
>>     +static void qcom_dc_scm_fru_init(I2CBus *bus, uint8_t addr,
>>     uint32_t rsize)
>>     +{
>>     +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
>>     +    DeviceState *dev = DEVICE(i2c_dev);
>>     +    /* Use First Index for DC-SCM FRU */
>>     +    DriveInfo *dinfo = drive_get(IF_NONE, 0, 0);
>>     +
>>     +    qdev_prop_set_uint32(dev, "rom-size", rsize);
>>     +
>>     +    if (dinfo) {
>>     +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
>>     +    }
>>     +
>>     +    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
>>     +}
>>
>>
>> We've sent a similar patch up for the at24c but in its own file -- but looking at this, we could likely expand it to suite our cases as well - is there a reason it's named qcom_dc_scm_fru_init?  Presumably that's to handle the drive_get parameters.  If you make it use `drive_get(IF_NONE, bus, unit);` You'll be able to associate a drive via parameters like you aim to.
> 
> Okay. I agree with you that it can be more generic to be used for other
> machines too. I'll rewrite this function in v2 to make it have below
> shape.
> 
> static void
> at24c_eeprom_init_from_drive(I2CBus *bus, uint8_t addr, uint32_t rsize,
>                               int bus, int unit)

Yes and if other non-Aspeed machines need it one day, we could export it.
There is no need for now to multiply the interface now.

Thanks,

C.

> 
> Thanks,
> Jae
> 
>>     +
>>       static void qcom_dc_scm_bmc_i2c_init(AspeedMachineState *bmc)
>>       {
>>           AspeedSoCState *soc = &bmc->soc;
>>
>>           i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 15),
>>     "tmp105", 0x4d);
>>
>>     -    uint8_t *eeprom_buf = g_malloc0(128 * 1024);
>>     -    if (eeprom_buf) {
>>     -        smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53,
>>     -                              eeprom_buf);
>>     -    }
>>     +    qcom_dc_scm_fru_init(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53,
>>     128 * 1024);
>>       }
>>
>>       static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
>>     --     2.25.1
>>
>>

Re: [PATCH 3/9] hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device

[Cédric Le Goater]

On 6/23/22 17:28, Patrick Venture wrote:
> 
> 
> On Wed, Jun 22, 2022 at 10:48 AM Jae Hyun Yoo <quic_jaehyoo@quicinc.com <mailto:quic_jaehyoo@quicinc.com>> wrote:
> 
>     From: Graeme Gregory <quic_ggregory@quicinc.com <mailto:quic_ggregory@quicinc.com>>
> 
>     The FRU device uses the index 0 device on bus IF_NONE.
> 
>     -drive file=$FRU,format=raw,if=none
> 
>     file must match FRU size of 128k
> 
>     Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com <mailto:quic_ggregory@quicinc.com>>
>     ---
>       hw/arm/aspeed.c | 22 +++++++++++++++++-----
>       1 file changed, 17 insertions(+), 5 deletions(-)
> 
>     diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>     index 785cc543d046..36d6b2c33e48 100644
>     --- a/hw/arm/aspeed.c
>     +++ b/hw/arm/aspeed.c
>     @@ -992,17 +992,29 @@ static void fby35_i2c_init(AspeedMachineState *bmc)
>            */
>       }
> 
>     +static void qcom_dc_scm_fru_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
>     +{
>     +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
>     +    DeviceState *dev = DEVICE(i2c_dev);
>     +    /* Use First Index for DC-SCM FRU */
>     +    DriveInfo *dinfo = drive_get(IF_NONE, 0, 0);
>     +
>     +    qdev_prop_set_uint32(dev, "rom-size", rsize);
>     +
>     +    if (dinfo) {
>     +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
>     +    }
>     +
>     +    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
>     +}
> 
> 
> We've sent a similar patch up for the at24c but in its own file -- but looking at this, we could likely expand it to suite our cases as well - is there a reason it's named qcom_dc_scm_fru_init?  Presumably that's to handle the drive_get parameters.  If you make it use `drive_get(IF_NONE, bus, unit);` You'll be able to associate a drive via parameters like you aim to.


I have seen various attempts to populate the Aspeed eeproms with
data. The simplest is the g220a_bmc_i2c_init() approach. Some are
generating C code from the .bin files and compiling the result in
QEMU. Some were generating elf sections, linking them in QEMU and
passing the pointer as an eeprom buf.

The drive approach is the best I think, but the device/drive
association depends on the drive order on the command line when
devices are created by the platform.

We could may be extend the GenericLoaderState for eeproms ?
or introduce a routine which would look for a file under a (pc-bios)
per-machine directory and fill the eeprom buf with its contents ?

Any idea ?

Thanks,

C.

Re: [PATCH 3/9] hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device

[Patrick Venture]

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

On Thu, Jun 23, 2022 at 10:16 AM Cédric Le Goater <clg@kaod.org> wrote:

> On 6/23/22 17:28, Patrick Venture wrote:
> >
> >
> > On Wed, Jun 22, 2022 at 10:48 AM Jae Hyun Yoo <quic_jaehyoo@quicinc.com
> <mailto:quic_jaehyoo@quicinc.com>> wrote:
> >
> >     From: Graeme Gregory <quic_ggregory@quicinc.com <mailto:
> quic_ggregory@quicinc.com>>
> >
> >     The FRU device uses the index 0 device on bus IF_NONE.
> >
> >     -drive file=$FRU,format=raw,if=none
> >
> >     file must match FRU size of 128k
> >
> >     Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com <mailto:
> quic_ggregory@quicinc.com>>
> >     ---
> >       hw/arm/aspeed.c | 22 +++++++++++++++++-----
> >       1 file changed, 17 insertions(+), 5 deletions(-)
> >
> >     diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> >     index 785cc543d046..36d6b2c33e48 100644
> >     --- a/hw/arm/aspeed.c
> >     +++ b/hw/arm/aspeed.c
> >     @@ -992,17 +992,29 @@ static void fby35_i2c_init(AspeedMachineState
> *bmc)
> >            */
> >       }
> >
> >     +static void qcom_dc_scm_fru_init(I2CBus *bus, uint8_t addr,
> uint32_t rsize)
> >     +{
> >     +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
> >     +    DeviceState *dev = DEVICE(i2c_dev);
> >     +    /* Use First Index for DC-SCM FRU */
> >     +    DriveInfo *dinfo = drive_get(IF_NONE, 0, 0);
> >     +
> >     +    qdev_prop_set_uint32(dev, "rom-size", rsize);
> >     +
> >     +    if (dinfo) {
> >     +        qdev_prop_set_drive(dev, "drive",
> blk_by_legacy_dinfo(dinfo));
> >     +    }
> >     +
> >     +    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
> >     +}
> >
> >
> > We've sent a similar patch up for the at24c but in its own file -- but
> looking at this, we could likely expand it to suite our cases as well - is
> there a reason it's named qcom_dc_scm_fru_init?  Presumably that's to
> handle the drive_get parameters.  If you make it use `drive_get(IF_NONE,
> bus, unit);` You'll be able to associate a drive via parameters like you
> aim to.
>
>
> I have seen various attempts to populate the Aspeed eeproms with
> data. The simplest is the g220a_bmc_i2c_init() approach. Some are
> generating C code from the .bin files and compiling the result in
> QEMU. Some were generating elf sections, linking them in QEMU and
> passing the pointer as an eeprom buf.
>
> The drive approach is the best I think, but the device/drive
> association depends on the drive order on the command line when
> devices are created by the platform.
>
> We could may be extend the GenericLoaderState for eeproms ?
> or introduce a routine which would look for a file under a (pc-bios)
> per-machine directory and fill the eeprom buf with its contents ?
>
> Any idea ?
>

So we have this in our eeprom_at24c module because we use it with Aspeed
and Nuvoton:

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);
}

With this style call in the board:

snippet from downstream version of
https://github.com/qemu/qemu/blob/master/hw/arm/npcm7xx_boards.c#L305 that
we still need to finish upstreaming:

<snip>
     /* mb_fru */
    at24c_eeprom_init_one(npcm7xx_i2c_get_bus(soc, 5), 5, 0x50, 8192, 0);
<snip>
    /* 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);
<snip>

And then we call it with the bus and unit, the pair of those has to be
unique for the drive parameter to work:

-drive file=/export/hda3/tmp/mb_fru@50
,if=none,bus=5,unit=0,snapshot=off,format=raw
-drive file=/export/hda3/tmp/fan_fru@51
,if=none,bus=5,unit=1,snapshot=off,format=raw
-drive file=/export/hda3/tmp/hsbp_fru@52
,if=none,bus=5,unit=2,snapshot=off,format=raw

The above code snippet is what, I'm fairly certain we had sent up for
review before, and we can send it again if you want.



> Thanks,
>
> C.
>

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

Re: [PATCH 3/9] hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device

[Jae Hyun Yoo]

On 6/23/2022 10:37 AM, Patrick Venture wrote:
> 
> 
> On Thu, Jun 23, 2022 at 10:16 AM Cédric Le Goater <clg@kaod.org 
> <mailto:clg@kaod.org>> wrote:
> 
>     On 6/23/22 17:28, Patrick Venture wrote:
>      >
>      >
>      > On Wed, Jun 22, 2022 at 10:48 AM Jae Hyun Yoo
>     <quic_jaehyoo@quicinc.com <mailto:quic_jaehyoo@quicinc.com>
>     <mailto:quic_jaehyoo@quicinc.com <mailto:quic_jaehyoo@quicinc.com>>>
>     wrote:
>      >
>      >     From: Graeme Gregory <quic_ggregory@quicinc.com
>     <mailto:quic_ggregory@quicinc.com> <mailto:quic_ggregory@quicinc.com
>     <mailto:quic_ggregory@quicinc.com>>>
>      >
>      >     The FRU device uses the index 0 device on bus IF_NONE.
>      >
>      >     -drive file=$FRU,format=raw,if=none
>      >
>      >     file must match FRU size of 128k
>      >
>      >     Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com
>     <mailto:quic_ggregory@quicinc.com> <mailto:quic_ggregory@quicinc.com
>     <mailto:quic_ggregory@quicinc.com>>>
>      >     ---
>      >       hw/arm/aspeed.c | 22 +++++++++++++++++-----
>      >       1 file changed, 17 insertions(+), 5 deletions(-)
>      >
>      >     diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>      >     index 785cc543d046..36d6b2c33e48 100644
>      >     --- a/hw/arm/aspeed.c
>      >     +++ b/hw/arm/aspeed.c
>      >     @@ -992,17 +992,29 @@ static void
>     fby35_i2c_init(AspeedMachineState *bmc)
>      >            */
>      >       }
>      >
>      >     +static void qcom_dc_scm_fru_init(I2CBus *bus, uint8_t addr,
>     uint32_t rsize)
>      >     +{
>      >     +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
>      >     +    DeviceState *dev = DEVICE(i2c_dev);
>      >     +    /* Use First Index for DC-SCM FRU */
>      >     +    DriveInfo *dinfo = drive_get(IF_NONE, 0, 0);
>      >     +
>      >     +    qdev_prop_set_uint32(dev, "rom-size", rsize);
>      >     +
>      >     +    if (dinfo) {
>      >     +        qdev_prop_set_drive(dev, "drive",
>     blk_by_legacy_dinfo(dinfo));
>      >     +    }
>      >     +
>      >     +    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
>      >     +}
>      >
>      >
>      > We've sent a similar patch up for the at24c but in its own file
>     -- but looking at this, we could likely expand it to suite our cases
>     as well - is there a reason it's named qcom_dc_scm_fru_init? 
>     Presumably that's to handle the drive_get parameters.  If you make
>     it use `drive_get(IF_NONE, bus, unit);` You'll be able to associate
>     a drive via parameters like you aim to.
> 
> 
>     I have seen various attempts to populate the Aspeed eeproms with
>     data. The simplest is the g220a_bmc_i2c_init() approach. Some are
>     generating C code from the .bin files and compiling the result in
>     QEMU. Some were generating elf sections, linking them in QEMU and
>     passing the pointer as an eeprom buf.
> 
>     The drive approach is the best I think, but the device/drive
>     association depends on the drive order on the command line when
>     devices are created by the platform.
> 
>     We could may be extend the GenericLoaderState for eeproms ?
>     or introduce a routine which would look for a file under a (pc-bios)
>     per-machine directory and fill the eeprom buf with its contents ?
> 
>     Any idea ?
> 
> 
> So we have this in our eeprom_at24c module because we use it with Aspeed 
> and Nuvoton:
> 
> 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);
> }
> 
> With this style call in the board:
> 
> snippet from downstream version of 
> https://github.com/qemu/qemu/blob/master/hw/arm/npcm7xx_boards.c#L305 
> <https://github.com/qemu/qemu/blob/master/hw/arm/npcm7xx_boards.c#L305> 
> that we still need to finish upstreaming:
> 
> <snip>
>       /* mb_fru */
>      at24c_eeprom_init_one(npcm7xx_i2c_get_bus(soc, 5), 5, 0x50, 8192, 0);
> <snip>
>      /* 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);
> <snip>
> 
> And then we call it with the bus and unit, the pair of those has to be 
> unique for the drive parameter to work:
> 
> -drive 
> file=/export/hda3/tmp/mb_fru@50,if=none,bus=5,unit=0,snapshot=off,format=raw
> -drive 
> file=/export/hda3/tmp/fan_fru@51,if=none,bus=5,unit=1,snapshot=off,format=raw
> -drive 
> file=/export/hda3/tmp/hsbp_fru@52,if=none,bus=5,unit=2,snapshot=off,format=raw
> 
> The above code snippet is what, I'm fairly certain we had sent up for 
> review before, and we can send it again if you want.

Okay. I found that the above code was tried to upstream earlier by Hao
but it's not completed yet and it's pending at below thread.

https://lore.kernel.org/qemu-devel/875ystigke.fsf_-_@dusky.pond.sub.org/

Please complete the upstreaming then I'll use Hao's version.

I'll drop 3/9 patch from this series in v2.

Thanks,
Jae

> 
>     Thanks,
> 
>     C.
> 

Re: [PATCH 3/9] hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device

[Cédric Le Goater]

Adding Markus,

On 6/23/22 19:37, Patrick Venture wrote:
> 
> 
> On Thu, Jun 23, 2022 at 10:16 AM Cédric Le Goater <clg@kaod.org <mailto:clg@kaod.org>> wrote:
> 
>     On 6/23/22 17:28, Patrick Venture wrote:
>      >
>      >
>      > On Wed, Jun 22, 2022 at 10:48 AM Jae Hyun Yoo <quic_jaehyoo@quicinc.com <mailto:quic_jaehyoo@quicinc.com> <mailto:quic_jaehyoo@quicinc.com <mailto:quic_jaehyoo@quicinc.com>>> wrote:
>      >
>      >     From: Graeme Gregory <quic_ggregory@quicinc.com <mailto:quic_ggregory@quicinc.com> <mailto:quic_ggregory@quicinc.com <mailto:quic_ggregory@quicinc.com>>>
>      >
>      >     The FRU device uses the index 0 device on bus IF_NONE.
>      >
>      >     -drive file=$FRU,format=raw,if=none
>      >
>      >     file must match FRU size of 128k
>      >
>      >     Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com <mailto:quic_ggregory@quicinc.com> <mailto:quic_ggregory@quicinc.com <mailto:quic_ggregory@quicinc.com>>>
>      >     ---
>      >       hw/arm/aspeed.c | 22 +++++++++++++++++-----
>      >       1 file changed, 17 insertions(+), 5 deletions(-)
>      >
>      >     diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>      >     index 785cc543d046..36d6b2c33e48 100644
>      >     --- a/hw/arm/aspeed.c
>      >     +++ b/hw/arm/aspeed.c
>      >     @@ -992,17 +992,29 @@ static void fby35_i2c_init(AspeedMachineState *bmc)
>      >            */
>      >       }
>      >
>      >     +static void qcom_dc_scm_fru_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
>      >     +{
>      >     +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
>      >     +    DeviceState *dev = DEVICE(i2c_dev);
>      >     +    /* Use First Index for DC-SCM FRU */
>      >     +    DriveInfo *dinfo = drive_get(IF_NONE, 0, 0);
>      >     +
>      >     +    qdev_prop_set_uint32(dev, "rom-size", rsize);
>      >     +
>      >     +    if (dinfo) {
>      >     +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
>      >     +    }
>      >     +
>      >     +    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
>      >     +}
>      >
>      >
>      > We've sent a similar patch up for the at24c but in its own file -- but looking at this, we could likely expand it to suite our cases as well - is there a reason it's named qcom_dc_scm_fru_init?  Presumably that's to handle the drive_get parameters.  If you make it use `drive_get(IF_NONE, bus, unit);` You'll be able to associate a drive via parameters like you aim to.
> 
> 
>     I have seen various attempts to populate the Aspeed eeproms with
>     data. The simplest is the g220a_bmc_i2c_init() approach. Some are
>     generating C code from the .bin files and compiling the result in
>     QEMU. Some were generating elf sections, linking them in QEMU and
>     passing the pointer as an eeprom buf.
> 
>     The drive approach is the best I think, but the device/drive
>     association depends on the drive order on the command line when
>     devices are created by the platform.
> 
>     We could may be extend the GenericLoaderState for eeproms ?
>     or introduce a routine which would look for a file under a (pc-bios)
>     per-machine directory and fill the eeprom buf with its contents ?
> 
>     Any idea ?
> 
> 
> So we have this in our eeprom_at24c module because we use it with Aspeed and Nuvoton:
> 
> 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);
> }
> 
> With this style call in the board:
> 
> snippet from downstream version of https://github.com/qemu/qemu/blob/master/hw/arm/npcm7xx_boards.c#L305 <https://github.com/qemu/qemu/blob/master/hw/arm/npcm7xx_boards.c#L305> that we still need to finish upstreaming:
> 
> <snip>
>       /* mb_fru */
>      at24c_eeprom_init_one(npcm7xx_i2c_get_bus(soc, 5), 5, 0x50, 8192, 0);
> <snip>
>      /* 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);
> <snip>
> 
> And then we call it with the bus and unit, the pair of those has to be unique for the drive parameter to work:
> 
> -drive file=/export/hda3/tmp/mb_fru@50,if=none,bus=5,unit=0,snapshot=off,format=raw
> -drive file=/export/hda3/tmp/fan_fru@51,if=none,bus=5,unit=1,snapshot=off,format=raw
> -drive file=/export/hda3/tmp/hsbp_fru@52,if=none,bus=5,unit=2,snapshot=off,format=raw
> 
> The above code snippet is what, I'm fairly certain we had sent up for review before, and we can send it again if you want.

You could. I am not sure this is the good direction but it would restart
the -drive topic.


Thanks,

C.

Re: [PATCH 3/9] hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device

[Cédric Le Goater]

On 6/22/22 19:28, Jae Hyun Yoo wrote:
> From: Graeme Gregory <quic_ggregory@quicinc.com>
> 
> The FRU device uses the index 0 device on bus IF_NONE.
> 
> -drive file=$FRU,format=raw,if=none
> 
> file must match FRU size of 128k
> 
> Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com>



Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   hw/arm/aspeed.c | 22 +++++++++++++++++-----
>   1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 785cc543d046..36d6b2c33e48 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -992,17 +992,29 @@ static void fby35_i2c_init(AspeedMachineState *bmc)
>        */
>   }
>   
> +static void qcom_dc_scm_fru_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
> +{
> +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
> +    DeviceState *dev = DEVICE(i2c_dev);
> +    /* Use First Index for DC-SCM FRU */
> +    DriveInfo *dinfo = drive_get(IF_NONE, 0, 0);
> +
> +    qdev_prop_set_uint32(dev, "rom-size", rsize);
> +
> +    if (dinfo) {
> +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
> +    }
> +
> +    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
> +}
> +
>   static void qcom_dc_scm_bmc_i2c_init(AspeedMachineState *bmc)
>   {
>       AspeedSoCState *soc = &bmc->soc;
>   
>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 15), "tmp105", 0x4d);
>   
> -    uint8_t *eeprom_buf = g_malloc0(128 * 1024);
> -    if (eeprom_buf) {
> -        smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53,
> -                              eeprom_buf);
> -    }
> +    qcom_dc_scm_fru_init(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53, 128 * 1024);
>   }
>   
>   static bool aspeed_get_mmio_exec(Object *obj, Error **errp)

[PATCH 4/9] hw/arm/aspeed: add Qualcomm Firework machine and FRU device

[Jae Hyun Yoo]

From: Graeme Gregory <quic_ggregory@quicinc.com>

Add base for Qualcomm Firework machine and add its FRU device which is
defined by DC-SCM to be fixed address 0x50.

Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com>
---
 hw/arm/aspeed.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 36d6b2c33e48..0e6edd2be4fa 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1017,6 +1017,35 @@ static void qcom_dc_scm_bmc_i2c_init(AspeedMachineState *bmc)
     qcom_dc_scm_fru_init(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53, 128 * 1024);
 }
 
+static void qcom_firework_fru_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
+{
+    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
+    DeviceState *dev = DEVICE(i2c_dev);
+    /* Use First Index for DC-SCM FRU */
+    DriveInfo *dinfo = drive_get(IF_NONE, 0, 1);
+
+    qdev_prop_set_uint32(dev, "rom-size", rsize);
+
+    if (dinfo) {
+        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
+    }
+
+    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
+}
+
+static void qcom_dc_scm_firework_i2c_init(AspeedMachineState *bmc)
+{
+    AspeedSoCState *soc = &bmc->soc;
+
+    /* Create the generic DC-SCM hardware */
+    qcom_dc_scm_bmc_i2c_init(bmc);
+
+    /* Now create the Firework specific hardware */
+
+    /* I2C4 */
+    qcom_firework_fru_init(aspeed_i2c_get_bus(&soc->i2c, 4), 0x50, 128 * 1024);
+}
+
 static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
 {
     return ASPEED_MACHINE(obj)->mmio_exec;
@@ -1489,6 +1518,26 @@ static void aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
         aspeed_soc_num_cpus(amc->soc_name);
 };
 
+static void aspeed_machine_qcom_firework_class_init(ObjectClass *oc,
+                                                    void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+    mc->desc       = "Qualcomm DC-SCM V1/Firework BMC (Cortex A7)";
+    amc->soc_name  = "ast2600-a3";
+    amc->hw_strap1 = QCOM_DC_SCM_V1_BMC_HW_STRAP1;
+    amc->hw_strap2 = QCOM_DC_SCM_V1_BMC_HW_STRAP2;
+    amc->fmc_model = "n25q512a";
+    amc->spi_model = "n25q512a";
+    amc->num_cs    = 2;
+    amc->macs_mask = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
+    amc->i2c_init  = qcom_dc_scm_firework_i2c_init;
+    mc->default_ram_size = 1 * GiB;
+    mc->default_cpus = mc->min_cpus = mc->max_cpus =
+        aspeed_soc_num_cpus(amc->soc_name);
+};
+
 static const TypeInfo aspeed_machine_types[] = {
     {
         .name          = MACHINE_TYPE_NAME("palmetto-bmc"),
@@ -1534,6 +1583,10 @@ static const TypeInfo aspeed_machine_types[] = {
         .name          = MACHINE_TYPE_NAME("qcom-dc-scm-v1-bmc"),
         .parent        = TYPE_ASPEED_MACHINE,
         .class_init    = aspeed_machine_qcom_dc_scm_v1_class_init,
+    }, {
+        .name          = MACHINE_TYPE_NAME("qcom-firework"),
+        .parent        = TYPE_ASPEED_MACHINE,
+        .class_init    = aspeed_machine_qcom_firework_class_init,
     }, {
         .name          = MACHINE_TYPE_NAME("fp5280g2-bmc"),
         .parent        = TYPE_ASPEED_MACHINE,
-- 
2.25.1

Re: [PATCH 4/9] hw/arm/aspeed: add Qualcomm Firework machine and FRU device

[Cédric Le Goater]

On 6/22/22 19:28, Jae Hyun Yoo wrote:
> From: Graeme Gregory <quic_ggregory@quicinc.com>
> 
> Add base for Qualcomm Firework machine and add its FRU device which is
> defined by DC-SCM to be fixed address 0x50.
> 
> Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com>
> ---
>   hw/arm/aspeed.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 53 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 36d6b2c33e48..0e6edd2be4fa 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -1017,6 +1017,35 @@ static void qcom_dc_scm_bmc_i2c_init(AspeedMachineState *bmc)
>       qcom_dc_scm_fru_init(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53, 128 * 1024);
>   }
>   
> +static void qcom_firework_fru_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
> +{
> +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
> +    DeviceState *dev = DEVICE(i2c_dev);
> +    /* Use First Index for DC-SCM FRU */
> +    DriveInfo *dinfo = drive_get(IF_NONE, 0, 1);
> +
> +    qdev_prop_set_uint32(dev, "rom-size", rsize);
> +
> +    if (dinfo) {
> +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
> +    }
> +
> +    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
> +}
> +
> +static void qcom_dc_scm_firework_i2c_init(AspeedMachineState *bmc)
> +{
> +    AspeedSoCState *soc = &bmc->soc;
> +
> +    /* Create the generic DC-SCM hardware */
> +    qcom_dc_scm_bmc_i2c_init(bmc);
> +
> +    /* Now create the Firework specific hardware */
> +
> +    /* I2C4 */
> +    qcom_firework_fru_init(aspeed_i2c_get_bus(&soc->i2c, 4), 0x50, 128 * 1024);
> +}
> +
>   static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
>   {
>       return ASPEED_MACHINE(obj)->mmio_exec;
> @@ -1489,6 +1518,26 @@ static void aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
>           aspeed_soc_num_cpus(amc->soc_name);
>   };
>   
> +static void aspeed_machine_qcom_firework_class_init(ObjectClass *oc,
> +                                                    void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
> +
> +    mc->desc       = "Qualcomm DC-SCM V1/Firework BMC (Cortex A7)";
> +    amc->soc_name  = "ast2600-a3";
> +    amc->hw_strap1 = QCOM_DC_SCM_V1_BMC_HW_STRAP1;
> +    amc->hw_strap2 = QCOM_DC_SCM_V1_BMC_HW_STRAP2;
> +    amc->fmc_model = "n25q512a";
> +    amc->spi_model = "n25q512a";
> +    amc->num_cs    = 2;
> +    amc->macs_mask = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
> +    amc->i2c_init  = qcom_dc_scm_firework_i2c_init;
> +    mc->default_ram_size = 1 * GiB;
> +    mc->default_cpus = mc->min_cpus = mc->max_cpus =
> +        aspeed_soc_num_cpus(amc->soc_name);
> +};
> +
>   static const TypeInfo aspeed_machine_types[] = {
>       {
>           .name          = MACHINE_TYPE_NAME("palmetto-bmc"),
> @@ -1534,6 +1583,10 @@ static const TypeInfo aspeed_machine_types[] = {
>           .name          = MACHINE_TYPE_NAME("qcom-dc-scm-v1-bmc"),
>           .parent        = TYPE_ASPEED_MACHINE,
>           .class_init    = aspeed_machine_qcom_dc_scm_v1_class_init,
> +    }, {
> +        .name          = MACHINE_TYPE_NAME("qcom-firework"),

We should add the "-bmc" prefix to this machine name to be consistent
with the other BMCs. A "qcom-firework" machine would model the whole
system, host side included.

Thanks,

C.

> +        .parent        = TYPE_ASPEED_MACHINE,
> +        .class_init    = aspeed_machine_qcom_firework_class_init,
>       }, {
>           .name          = MACHINE_TYPE_NAME("fp5280g2-bmc"),
>           .parent        = TYPE_ASPEED_MACHINE,

Re: [PATCH 4/9] hw/arm/aspeed: add Qualcomm Firework machine and FRU device

[Jae Hyun Yoo]

On 6/22/2022 11:43 PM, Cédric Le Goater wrote:
> On 6/22/22 19:28, Jae Hyun Yoo wrote:
>> From: Graeme Gregory <quic_ggregory@quicinc.com>
>>
>> Add base for Qualcomm Firework machine and add its FRU device which is
>> defined by DC-SCM to be fixed address 0x50.
>>
>> Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com>
>> ---
>>   hw/arm/aspeed.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 53 insertions(+)
>>
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index 36d6b2c33e48..0e6edd2be4fa 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -1017,6 +1017,35 @@ static void 
>> qcom_dc_scm_bmc_i2c_init(AspeedMachineState *bmc)
>>       qcom_dc_scm_fru_init(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53, 
>> 128 * 1024);
>>   }
>> +static void qcom_firework_fru_init(I2CBus *bus, uint8_t addr, 
>> uint32_t rsize)
>> +{
>> +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
>> +    DeviceState *dev = DEVICE(i2c_dev);
>> +    /* Use First Index for DC-SCM FRU */
>> +    DriveInfo *dinfo = drive_get(IF_NONE, 0, 1);
>> +
>> +    qdev_prop_set_uint32(dev, "rom-size", rsize);
>> +
>> +    if (dinfo) {
>> +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
>> +    }
>> +
>> +    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
>> +}
>> +
>> +static void qcom_dc_scm_firework_i2c_init(AspeedMachineState *bmc)
>> +{
>> +    AspeedSoCState *soc = &bmc->soc;
>> +
>> +    /* Create the generic DC-SCM hardware */
>> +    qcom_dc_scm_bmc_i2c_init(bmc);
>> +
>> +    /* Now create the Firework specific hardware */
>> +
>> +    /* I2C4 */
>> +    qcom_firework_fru_init(aspeed_i2c_get_bus(&soc->i2c, 4), 0x50, 
>> 128 * 1024);
>> +}
>> +
>>   static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
>>   {
>>       return ASPEED_MACHINE(obj)->mmio_exec;
>> @@ -1489,6 +1518,26 @@ static void 
>> aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
>>           aspeed_soc_num_cpus(amc->soc_name);
>>   };
>> +static void aspeed_machine_qcom_firework_class_init(ObjectClass *oc,
>> +                                                    void *data)
>> +{
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
>> +
>> +    mc->desc       = "Qualcomm DC-SCM V1/Firework BMC (Cortex A7)";
>> +    amc->soc_name  = "ast2600-a3";
>> +    amc->hw_strap1 = QCOM_DC_SCM_V1_BMC_HW_STRAP1;
>> +    amc->hw_strap2 = QCOM_DC_SCM_V1_BMC_HW_STRAP2;
>> +    amc->fmc_model = "n25q512a";
>> +    amc->spi_model = "n25q512a";
>> +    amc->num_cs    = 2;
>> +    amc->macs_mask = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
>> +    amc->i2c_init  = qcom_dc_scm_firework_i2c_init;
>> +    mc->default_ram_size = 1 * GiB;
>> +    mc->default_cpus = mc->min_cpus = mc->max_cpus =
>> +        aspeed_soc_num_cpus(amc->soc_name);
>> +};
>> +
>>   static const TypeInfo aspeed_machine_types[] = {
>>       {
>>           .name          = MACHINE_TYPE_NAME("palmetto-bmc"),
>> @@ -1534,6 +1583,10 @@ static const TypeInfo aspeed_machine_types[] = {
>>           .name          = MACHINE_TYPE_NAME("qcom-dc-scm-v1-bmc"),
>>           .parent        = TYPE_ASPEED_MACHINE,
>>           .class_init    = aspeed_machine_qcom_dc_scm_v1_class_init,
>> +    }, {
>> +        .name          = MACHINE_TYPE_NAME("qcom-firework"),
> 
> We should add the "-bmc" prefix to this machine name to be consistent
> with the other BMCs. A "qcom-firework" machine would model the whole
> system, host side included.

Right, so I added the "-bmc" tag to "qcom-dc-scm-v1-bmc" as it's an
add-in card type board, and it can be attached to the "qcom-firework"
baseboard. The "qcom-firework" doesn't have the "-bmc" tag intentionally
since it doesn't have a bmc soc on it.

Thanks,
Jae

> Thanks,
> 
> C.
> 
>> +        .parent        = TYPE_ASPEED_MACHINE,
>> +        .class_init    = aspeed_machine_qcom_firework_class_init,
>>       }, {
>>           .name          = MACHINE_TYPE_NAME("fp5280g2-bmc"),
>>           .parent        = TYPE_ASPEED_MACHINE,
> 

Re: [PATCH 4/9] hw/arm/aspeed: add Qualcomm Firework machine and FRU device

[Cédric Le Goater]

On 6/23/22 16:11, Jae Hyun Yoo wrote:
> On 6/22/2022 11:43 PM, Cédric Le Goater wrote:
>> On 6/22/22 19:28, Jae Hyun Yoo wrote:
>>> From: Graeme Gregory <quic_ggregory@quicinc.com>
>>>
>>> Add base for Qualcomm Firework machine and add its FRU device which is
>>> defined by DC-SCM to be fixed address 0x50.
>>>
>>> Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com>
>>> ---
>>>   hw/arm/aspeed.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 53 insertions(+)
>>>
>>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>>> index 36d6b2c33e48..0e6edd2be4fa 100644
>>> --- a/hw/arm/aspeed.c
>>> +++ b/hw/arm/aspeed.c
>>> @@ -1017,6 +1017,35 @@ static void qcom_dc_scm_bmc_i2c_init(AspeedMachineState *bmc)
>>>       qcom_dc_scm_fru_init(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53, 128 * 1024);
>>>   }
>>> +static void qcom_firework_fru_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
>>> +{
>>> +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
>>> +    DeviceState *dev = DEVICE(i2c_dev);
>>> +    /* Use First Index for DC-SCM FRU */
>>> +    DriveInfo *dinfo = drive_get(IF_NONE, 0, 1);
>>> +
>>> +    qdev_prop_set_uint32(dev, "rom-size", rsize);
>>> +
>>> +    if (dinfo) {
>>> +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
>>> +    }
>>> +
>>> +    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
>>> +}
>>> +
>>> +static void qcom_dc_scm_firework_i2c_init(AspeedMachineState *bmc)
>>> +{
>>> +    AspeedSoCState *soc = &bmc->soc;
>>> +
>>> +    /* Create the generic DC-SCM hardware */
>>> +    qcom_dc_scm_bmc_i2c_init(bmc);
>>> +
>>> +    /* Now create the Firework specific hardware */
>>> +
>>> +    /* I2C4 */
>>> +    qcom_firework_fru_init(aspeed_i2c_get_bus(&soc->i2c, 4), 0x50, 128 * 1024);
>>> +}
>>> +
>>>   static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
>>>   {
>>>       return ASPEED_MACHINE(obj)->mmio_exec;
>>> @@ -1489,6 +1518,26 @@ static void aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
>>>           aspeed_soc_num_cpus(amc->soc_name);
>>>   };
>>> +static void aspeed_machine_qcom_firework_class_init(ObjectClass *oc,
>>> +                                                    void *data)
>>> +{
>>> +    MachineClass *mc = MACHINE_CLASS(oc);
>>> +    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
>>> +
>>> +    mc->desc       = "Qualcomm DC-SCM V1/Firework BMC (Cortex A7)";
>>> +    amc->soc_name  = "ast2600-a3";
>>> +    amc->hw_strap1 = QCOM_DC_SCM_V1_BMC_HW_STRAP1;
>>> +    amc->hw_strap2 = QCOM_DC_SCM_V1_BMC_HW_STRAP2;
>>> +    amc->fmc_model = "n25q512a";
>>> +    amc->spi_model = "n25q512a";
>>> +    amc->num_cs    = 2;
>>> +    amc->macs_mask = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
>>> +    amc->i2c_init  = qcom_dc_scm_firework_i2c_init;
>>> +    mc->default_ram_size = 1 * GiB;
>>> +    mc->default_cpus = mc->min_cpus = mc->max_cpus =
>>> +        aspeed_soc_num_cpus(amc->soc_name);
>>> +};
>>> +
>>>   static const TypeInfo aspeed_machine_types[] = {
>>>       {
>>>           .name          = MACHINE_TYPE_NAME("palmetto-bmc"),
>>> @@ -1534,6 +1583,10 @@ static const TypeInfo aspeed_machine_types[] = {
>>>           .name          = MACHINE_TYPE_NAME("qcom-dc-scm-v1-bmc"),
>>>           .parent        = TYPE_ASPEED_MACHINE,
>>>           .class_init    = aspeed_machine_qcom_dc_scm_v1_class_init,
>>> +    }, {
>>> +        .name          = MACHINE_TYPE_NAME("qcom-firework"),
>>
>> We should add the "-bmc" prefix to this machine name to be consistent
>> with the other BMCs. A "qcom-firework" machine would model the whole
>> system, host side included.
> 
> Right, so I added the "-bmc" tag to "qcom-dc-scm-v1-bmc" as it's an
> add-in card type board, and it can be attached to the "qcom-firework"
> baseboard. The "qcom-firework" doesn't have the "-bmc" tag intentionally
> since it doesn't have a bmc soc on it.

These are the Aspeed machines, they only model the BMC side of the
overall system.

A "qcom-firework" machine would include the host SoC, possibly the
service and management SoCs plus the BMC.

As an example, see the fb35 machine being developed by Peter :
  
   http://patchwork.ozlabs.org/project/qemu-devel/list/?series=306294

or the PowerNV machines which use an embedded or external Aspeed BMCs

Thanks,

C.

> 
> Thanks,
> Jae
> 
>> Thanks,
>>
>> C.
>>
>>> +        .parent        = TYPE_ASPEED_MACHINE,
>>> +        .class_init    = aspeed_machine_qcom_firework_class_init,
>>>       }, {
>>>           .name          = MACHINE_TYPE_NAME("fp5280g2-bmc"),
>>>           .parent        = TYPE_ASPEED_MACHINE,
>>

Re: [PATCH 4/9] hw/arm/aspeed: add Qualcomm Firework machine and FRU device

[Jae Hyun Yoo]

On 6/24/2022 12:32 AM, Cédric Le Goater wrote:
> On 6/23/22 16:11, Jae Hyun Yoo wrote:
>> On 6/22/2022 11:43 PM, Cédric Le Goater wrote:
>>> On 6/22/22 19:28, Jae Hyun Yoo wrote:
>>>> From: Graeme Gregory <quic_ggregory@quicinc.com>
>>>>
>>>> Add base for Qualcomm Firework machine and add its FRU device which is
>>>> defined by DC-SCM to be fixed address 0x50.
>>>>
>>>> Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com>
>>>> ---
>>>>   hw/arm/aspeed.c | 53 
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>>>   1 file changed, 53 insertions(+)
>>>>
>>>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>>>> index 36d6b2c33e48..0e6edd2be4fa 100644
>>>> --- a/hw/arm/aspeed.c
>>>> +++ b/hw/arm/aspeed.c
>>>> @@ -1017,6 +1017,35 @@ static void 
>>>> qcom_dc_scm_bmc_i2c_init(AspeedMachineState *bmc)
>>>>       qcom_dc_scm_fru_init(aspeed_i2c_get_bus(&soc->i2c, 15), 0x53, 
>>>> 128 * 1024);
>>>>   }
>>>> +static void qcom_firework_fru_init(I2CBus *bus, uint8_t addr, 
>>>> uint32_t rsize)
>>>> +{
>>>> +    I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
>>>> +    DeviceState *dev = DEVICE(i2c_dev);
>>>> +    /* Use First Index for DC-SCM FRU */
>>>> +    DriveInfo *dinfo = drive_get(IF_NONE, 0, 1);
>>>> +
>>>> +    qdev_prop_set_uint32(dev, "rom-size", rsize);
>>>> +
>>>> +    if (dinfo) {
>>>> +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
>>>> +    }
>>>> +
>>>> +    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
>>>> +}
>>>> +
>>>> +static void qcom_dc_scm_firework_i2c_init(AspeedMachineState *bmc)
>>>> +{
>>>> +    AspeedSoCState *soc = &bmc->soc;
>>>> +
>>>> +    /* Create the generic DC-SCM hardware */
>>>> +    qcom_dc_scm_bmc_i2c_init(bmc);
>>>> +
>>>> +    /* Now create the Firework specific hardware */
>>>> +
>>>> +    /* I2C4 */
>>>> +    qcom_firework_fru_init(aspeed_i2c_get_bus(&soc->i2c, 4), 0x50, 
>>>> 128 * 1024);
>>>> +}
>>>> +
>>>>   static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
>>>>   {
>>>>       return ASPEED_MACHINE(obj)->mmio_exec;
>>>> @@ -1489,6 +1518,26 @@ static void 
>>>> aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
>>>>           aspeed_soc_num_cpus(amc->soc_name);
>>>>   };
>>>> +static void aspeed_machine_qcom_firework_class_init(ObjectClass *oc,
>>>> +                                                    void *data)
>>>> +{
>>>> +    MachineClass *mc = MACHINE_CLASS(oc);
>>>> +    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
>>>> +
>>>> +    mc->desc       = "Qualcomm DC-SCM V1/Firework BMC (Cortex A7)";
>>>> +    amc->soc_name  = "ast2600-a3";
>>>> +    amc->hw_strap1 = QCOM_DC_SCM_V1_BMC_HW_STRAP1;
>>>> +    amc->hw_strap2 = QCOM_DC_SCM_V1_BMC_HW_STRAP2;
>>>> +    amc->fmc_model = "n25q512a";
>>>> +    amc->spi_model = "n25q512a";
>>>> +    amc->num_cs    = 2;
>>>> +    amc->macs_mask = ASPEED_MAC2_ON | ASPEED_MAC3_ON;
>>>> +    amc->i2c_init  = qcom_dc_scm_firework_i2c_init;
>>>> +    mc->default_ram_size = 1 * GiB;
>>>> +    mc->default_cpus = mc->min_cpus = mc->max_cpus =
>>>> +        aspeed_soc_num_cpus(amc->soc_name);
>>>> +};
>>>> +
>>>>   static const TypeInfo aspeed_machine_types[] = {
>>>>       {
>>>>           .name          = MACHINE_TYPE_NAME("palmetto-bmc"),
>>>> @@ -1534,6 +1583,10 @@ static const TypeInfo aspeed_machine_types[] = {
>>>>           .name          = MACHINE_TYPE_NAME("qcom-dc-scm-v1-bmc"),
>>>>           .parent        = TYPE_ASPEED_MACHINE,
>>>>           .class_init    = aspeed_machine_qcom_dc_scm_v1_class_init,
>>>> +    }, {
>>>> +        .name          = MACHINE_TYPE_NAME("qcom-firework"),
>>>
>>> We should add the "-bmc" prefix to this machine name to be consistent
>>> with the other BMCs. A "qcom-firework" machine would model the whole
>>> system, host side included.
>>
>> Right, so I added the "-bmc" tag to "qcom-dc-scm-v1-bmc" as it's an
>> add-in card type board, and it can be attached to the "qcom-firework"
>> baseboard. The "qcom-firework" doesn't have the "-bmc" tag intentionally
>> since it doesn't have a bmc soc on it.
> 
> These are the Aspeed machines, they only model the BMC side of the
> overall system.
> 
> A "qcom-firework" machine would include the host SoC, possibly the
> service and management SoCs plus the BMC.
> 
> As an example, see the fb35 machine being developed by Peter :
> 
>    http://patchwork.ozlabs.org/project/qemu-devel/list/?series=306294
> 
> or the PowerNV machines which use an embedded or external Aspeed BMCs

Okay. I'll add the '-bmc' suffix for the Firework's BMC part in this
series so that the 'qcom-firework' can be used for a machine that
has the host SoC. I'll submit v2 soon.

Thanks,
Jae

> Thanks,
> 
> C.
> 
>>
>> Thanks,
>> Jae
>>
>>> Thanks,
>>>
>>> C.
>>>
>>>> +        .parent        = TYPE_ASPEED_MACHINE,
>>>> +        .class_init    = aspeed_machine_qcom_firework_class_init,
>>>>       }, {
>>>>           .name          = MACHINE_TYPE_NAME("fp5280g2-bmc"),
>>>>           .parent        = TYPE_ASPEED_MACHINE,
>>>
> 

[PATCH 5/9] hw/i2c: pmbus: Page #255 is valid page for read requests.

[Jae Hyun Yoo]

From: Maheswara Kurapati <quic_mkurapat@quicinc.com>

Current implementation of the pmbus core driver treats the read request
for page 255 as invalid request and sets the invalid command bit (bit 7) in the
STATUS_CML register. As per the PMBus specification it is a valid request.

Refer to the PMBus specification, revision 1.3.1, section 11.10 PAGE, on the page 58:
"Setting the PAGE to FFh means that all subsequent comands are to be applied to
 all outputs.

 Some commands, such as READ_TEMPERATURE, may use a common sensor but be
 available on all pages of a device.  Such implementations are the decision of
 each device manufacturer or are specified in a PMBus Application Profile. Consult
 the manufacturer's socuments or the Applicatin Profile Specification as needed."

For e.g.,
The VOUT_MODE is a valid command for page 255 for maxim 31785 device.
refer to Table 1. PMBus Command Codes on page 14 in the datasheet.
https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf

Fixes: 38870253f1d1 ("hw/i2c: pmbus: fix error returns and guard against out of range accesses")

Signed-off-by: Maheswara Kurapati <quic_mkurapat@quicinc.com>
---
 hw/i2c/pmbus_device.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index 62885fa6a15e..7db3343a83b6 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -291,7 +291,6 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd)
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: tried to read from all pages\n",
                       __func__);
-        pmbus_cml_error(pmdev);
     } else if (pmdev->page > pmdev->num_pages - 1) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: page %d is out of range\n",
-- 
2.25.1

Re: [PATCH 5/9] hw/i2c: pmbus: Page #255 is valid page for read requests.

[Titus Rwantare]

On Wed, 22 Jun 2022 at 10:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
>
> From: Maheswara Kurapati <quic_mkurapat@quicinc.com>
>
> Current implementation of the pmbus core driver treats the read request
> for page 255 as invalid request and sets the invalid command bit (bit 7) in the
> STATUS_CML register. As per the PMBus specification it is a valid request.
>
> Refer to the PMBus specification, revision 1.3.1, section 11.10 PAGE, on the page 58:
> "Setting the PAGE to FFh means that all subsequent comands are to be applied to
>  all outputs.
>
>  Some commands, such as READ_TEMPERATURE, may use a common sensor but be
>  available on all pages of a device.  Such implementations are the decision of
>  each device manufacturer or are specified in a PMBus Application Profile. Consult
>  the manufacturer's socuments or the Applicatin Profile Specification as needed."
>
Thanks for this, the copy of the spec I used was older.


> For e.g.,
> The VOUT_MODE is a valid command for page 255 for maxim 31785 device.
> refer to Table 1. PMBus Command Codes on page 14 in the datasheet.
> https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
>
> Fixes: 38870253f1d1 ("hw/i2c: pmbus: fix error returns and guard against out of range accesses")
>
> Signed-off-by: Maheswara Kurapati <quic_mkurapat@quicinc.com>
> ---
>  hw/i2c/pmbus_device.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
> index 62885fa6a15e..7db3343a83b6 100644
> --- a/hw/i2c/pmbus_device.c
> +++ b/hw/i2c/pmbus_device.c
> @@ -291,7 +291,6 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd)
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "%s: tried to read from all pages\n",
>                        __func__);
> -        pmbus_cml_error(pmdev);
>      } else if (pmdev->page > pmdev->num_pages - 1) {
>          qemu_log_mask(LOG_GUEST_ERROR,
>                        "%s: page %d is out of range\n",
> --
> 2.25.1
>

Please also update the stale comment just above, since this is now
specified behaviour.

Reviewed-by: Titus Rwantare <titusr@google.com>

Re: [PATCH 5/9] hw/i2c: pmbus: Page #255 is valid page for read requests.

[Jae Hyun Yoo]

Hello Titus,

On 6/22/2022 1:49 PM, Titus Rwantare wrote:
> On Wed, 22 Jun 2022 at 10:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
>>
>> From: Maheswara Kurapati <quic_mkurapat@quicinc.com>
>>
>> Current implementation of the pmbus core driver treats the read request
>> for page 255 as invalid request and sets the invalid command bit (bit 7) in the
>> STATUS_CML register. As per the PMBus specification it is a valid request.
>>
>> Refer to the PMBus specification, revision 1.3.1, section 11.10 PAGE, on the page 58:
>> "Setting the PAGE to FFh means that all subsequent comands are to be applied to
>>   all outputs.
>>
>>   Some commands, such as READ_TEMPERATURE, may use a common sensor but be
>>   available on all pages of a device.  Such implementations are the decision of
>>   each device manufacturer or are specified in a PMBus Application Profile. Consult
>>   the manufacturer's socuments or the Applicatin Profile Specification as needed."
>>
> Thanks for this, the copy of the spec I used was older.
> 
> 
>> For e.g.,
>> The VOUT_MODE is a valid command for page 255 for maxim 31785 device.
>> refer to Table 1. PMBus Command Codes on page 14 in the datasheet.
>> https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
>>
>> Fixes: 38870253f1d1 ("hw/i2c: pmbus: fix error returns and guard against out of range accesses")
>>
>> Signed-off-by: Maheswara Kurapati <quic_mkurapat@quicinc.com>
>> ---
>>   hw/i2c/pmbus_device.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
>> index 62885fa6a15e..7db3343a83b6 100644
>> --- a/hw/i2c/pmbus_device.c
>> +++ b/hw/i2c/pmbus_device.c
>> @@ -291,7 +291,6 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd)
>>           qemu_log_mask(LOG_GUEST_ERROR,
>>                         "%s: tried to read from all pages\n",
>>                         __func__);
>> -        pmbus_cml_error(pmdev);
>>       } else if (pmdev->page > pmdev->num_pages - 1) {
>>           qemu_log_mask(LOG_GUEST_ERROR,
>>                         "%s: page %d is out of range\n",
>> --
>> 2.25.1
>>
> 
> Please also update the stale comment just above, since this is now
> specified behaviour.

Right. The error log printing also needs to be removed so I'll revise 
this fix like below in v2.

diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index 62885fa6a15e..749a33af827b 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -284,14 +284,10 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd)

      /*
       * Reading from all pages will return the value from page 0,
-     * this is unspecified behaviour in general.
+     * means that all subsequent commands are to be applied to all output.
       */
      if (pmdev->page == PB_ALL_PAGES) {
          index = 0;
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "%s: tried to read from all pages\n",
-                      __func__);
-        pmbus_cml_error(pmdev);
      } else if (pmdev->page > pmdev->num_pages - 1) {
          qemu_log_mask(LOG_GUEST_ERROR,
                        "%s: page %d is out of range\n",

> Reviewed-by: Titus Rwantare <titusr@google.com>

Thank you so much for your review.

Regards,
Jae

[PATCH 6/9] hw/sensor: add Maxim MAX31785 device

[Jae Hyun Yoo]

From: Maheswara Kurapati <quic_mkurapat@quicinc.com>

MAX31785 is a PMBus compliant 6-Channel fan controller. It supports 6 fan
channels, 11 temperature sensors, and 6-Channel ADC to measure the remote
voltages. Datasheet can be found here:
https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf

This initial version of the driver has skeleton and support for the
fan channels. Requests for temperature sensors, and ADC Channels the
are serviced with the default values as per the datasheet.  No additional
instrumentation is done.  NV Log feature is not supported.

Signed-off-by: Maheswara Kurapati <quic_mkurapat@quicinc.com>
---
 hw/arm/Kconfig        |   1 +
 hw/arm/aspeed.c       |   6 +-
 hw/sensor/Kconfig     |   4 +
 hw/sensor/max31785.c  | 580 ++++++++++++++++++++++++++++++++++++++++++
 hw/sensor/meson.build |   1 +
 5 files changed, 590 insertions(+), 2 deletions(-)
 create mode 100644 hw/sensor/max31785.c

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 219262a8da36..77ef0fa967b2 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -408,6 +408,7 @@ config NPCM7XX
     select SSI
     select UNIMP
     select PCA954X
+    select MAX31785
 
 config FSL_IMX25
     bool
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 0e6edd2be4fa..85630b497793 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -619,7 +619,6 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
     LEDState *led;
 
     /* Bus 3: TODO bmp280@77 */
-    /* Bus 3: TODO max31785@52 */
     dev = DEVICE(i2c_slave_new(TYPE_PCA9552, 0x60));
     qdev_prop_set_string(dev, "description", "pca1");
     i2c_slave_realize_and_unref(I2C_SLAVE(dev),
@@ -635,6 +634,8 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
                               qdev_get_gpio_in(DEVICE(led), 0));
     }
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3), "dps310", 0x76);
+    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3), "max31785",
+                            0x52);
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), "tmp423", 0x4c);
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 5), "tmp423", 0x4c);
 
@@ -779,13 +780,14 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
     create_pca9552(soc, 7, 0x31);
     create_pca9552(soc, 7, 0x32);
     create_pca9552(soc, 7, 0x33);
-    /* Bus 7: TODO max31785@52 */
     create_pca9552(soc, 7, 0x60);
     create_pca9552(soc, 7, 0x61);
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7), "dps310", 0x76);
     /* Bus 7: TODO si7021-a20@20 */
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7), TYPE_TMP105,
                      0x48);
+    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7), "max31785",
+                     0x52);
     aspeed_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 7), 0x50, 64 * KiB);
     aspeed_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 7), 0x51, 64 * KiB);
 
diff --git a/hw/sensor/Kconfig b/hw/sensor/Kconfig
index df392e786904..e03bd09b50e8 100644
--- a/hw/sensor/Kconfig
+++ b/hw/sensor/Kconfig
@@ -34,3 +34,7 @@ config LSM303DLHC_MAG
 config ISL_PMBUS_VR
     bool
     depends on PMBUS
+
+config MAX31785
+    bool
+    depends on PMBUS
diff --git a/hw/sensor/max31785.c b/hw/sensor/max31785.c
new file mode 100644
index 000000000000..11bf9977b6fd
--- /dev/null
+++ b/hw/sensor/max31785.c
@@ -0,0 +1,580 @@
+/*
+ * Maxim MAX31785 PMBus 6-Channel Fan Controller
+ *
+ * Datasheet:
+ * https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
+ *
+ * Copyright(c) 2022 Qualcomm Innovative Center, Inc. All rights reserved.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/i2c/pmbus_device.h"
+#include "hw/irq.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+
+#define TYPE_MAX31785   "max31785"
+#define MAX31785(obj) OBJECT_CHECK(MAX31785State, (obj), TYPE_MAX31785)
+
+/*
+ * MAX31785 mfr specific PMBus commands
+ */
+#define MAX31785_MFR_MODE               0xD1
+#define MAX31785_MFR_PSEN_CONFIG        0xD2
+#define MAX31785_MFR_VOUT_PEAK          0xD4
+#define MAX31785_MFR_TEMPERATURE_PEAK   0xD6
+#define MAX31785_MFR_VOUT_MIN           0xD7
+#define MAX31785_MFR_FAULT_RESPONSE     0xD9
+#define MAX31785_MFR_NV_FAULT_LOG       0xDC
+#define MAX31785_MFR_TIME_COUNT         0xDD
+#define MAX31785_MFR_TEMP_SENSOR_CONFIG 0xF0
+#define MAX31785_MFR_FAN_CONFIG         0xF1
+#define MAX31785_MFR_FAN_LUT            0xF2
+#define MAX31785_MFR_READ_FAN_PWM       0xF3
+#define MAX31785_MFR_FAN_FAULT_LIMIT    0xF5
+#define MAX31785_MFR_FAN_WARN_LIMIT     0xF6
+#define MAX31785_MFR_FAN_RUN_TIME       0xF7
+#define MAX31785_MFR_FAN_PWM_AVG        0xF8
+#define MAX31785_MFR_FAN_PWM2RPM        0xF9
+
+/*
+ * defaults as per the data sheet
+ */
+#define MAX31785_DEFAULT_CAPABILITY         0x10
+#define MAX31785_DEFAULT_VOUT_MODE          0x40
+#define MAX31785_DEFAULT_VOUT_SCALE_MONITOR 0x7FFF
+#define MAX31785_DEFAULT_FAN_COMMAND_1      0x7FFF
+#define MAX31785_DEFAULT_OV_FAULT_LIMIT     0x7FFF
+#define MAX31785_DEFAULT_OV_WARN_LIMIT      0x7FFF
+#define MAX31785_DEFAULT_OT_FAULT_LIMIT     0x7FFF
+#define MAX31785_DEFAULT_OT_WARN_LIMIT      0x7FFF
+#define MAX31785_DEFAULT_PMBUS_REVISION     0x11
+#define MAX31785_DEFAULT_MFR_ID             0x4D
+#define MAX31785_DEFAULT_MFR_MODEL          0x53
+#define MAX31785_DEFAULT_MFR_REVISION       0x3030
+#define MAX31785A_DEFAULT_MFR_REVISION      0x3040
+#define MAX31785B_DEFAULT_MFR_REVISION      0x3061
+#define MAX31785B_DEFAULT_MFR_TEMPERATURE_PEAK   0x8000
+#define MAX31785B_DEFAULT_MFR_VOUT_MIN      0x7FFF
+#define MAX31785_DEFAULT_TEXT               0x3130313031303130
+
+/* MAX31785 pages */
+#define MAX31785_TOTAL_NUM_PAGES    23
+#define MAX31785_FAN_PAGES          6
+#define MAX31785_MIN_FAN_PAGE       0
+#define MAX31785_MAX_FAN_PAGE       5
+#define MAX31785_MIN_TEMP_PAGE      6
+#define MAX31785_MAX_TEMP_PAGE      16
+#define MAX31785_MIN_ADC_VOLTAGE_PAGE   17
+#define MAX31785_MAX_ADC_VOLTAGE_PAGE   22
+
+/* FAN_CONFIG_1_2 */
+#define MAX31785_MFR_FAN_CONFIG         0xF1
+#define MAX31785_FAN_CONFIG_ENABLE         BIT(7)
+#define MAX31785_FAN_CONFIG_RPM_PWM        BIT(6)
+#define MAX31785_FAN_CONFIG_PULSE(pulse)   (pulse << 4)
+#define MAX31785_DEFAULT_FAN_CONFIG_1_2(pulse) (MAX31785_FAN_CONFIG_ENABLE |\
+MAX31785_FAN_CONFIG_PULSE(pulse))
+#define MAX31785_DEFAULT_MFR_FAN_CONFIG     0x0000
+
+/* fan speed in RPM */
+#define MAX31785_DEFAULT_FAN_SPEED          0x7fff
+#define MAX31785_DEFAULT_FAN_STATUS         0x00
+
+#define MAX31785_DEFAULT_FAN_MAX_PWM        0x2710
+
+/*
+ * MAX31785State:
+ * @code: The command code received
+ * @page: Each page corresponds to a device monitored by the Max 31785
+ * The page register determines the available commands depending on device
+___________________________________________________________________________
+|   0   |  Fan Connected to PWM0                                            |
+|_______|___________________________________________________________________|
+|   1   |  Fan Connected to PWM1                                            |
+|_______|___________________________________________________________________|
+|   2   |  Fan Connected to PWM2                                            |
+|_______|___________________________________________________________________|
+|   3   |  Fan Connected to PWM3                                            |
+|_______|___________________________________________________________________|
+|   4   |  Fan Connected to PWM4                                            |
+|_______|___________________________________________________________________|
+|   5   |  Fan Connected to PWM5                                            |
+|_______|___________________________________________________________________|
+|   6   |  Remote Thermal Diode Connected to ADC 0                          |
+|_______|___________________________________________________________________|
+|   7   |  Remote Thermal Diode Connected to ADC 1                          |
+|_______|___________________________________________________________________|
+|   8   |  Remote Thermal Diode Connected to ADC 2                          |
+|_______|___________________________________________________________________|
+|   9   |  Remote Thermal Diode Connected to ADC 3                          |
+|_______|___________________________________________________________________|
+|  10   |  Remote Thermal Diode Connected to ADC 4                          |
+|_______|___________________________________________________________________|
+|  11   |  Remote Thermal Diode Connected to ADC 5                          |
+|_______|___________________________________________________________________|
+|  12   |  Internal Temperature Sensor                                      |
+|_______|___________________________________________________________________|
+|  13   |  Remote I2C Temperature Sensor with Address 0                     |
+|_______|___________________________________________________________________|
+|  14   |  Remote I2C Temperature Sensor with Address 1                     |
+|_______|___________________________________________________________________|
+|  15   |  Remote I2C Temperature Sensor with Address 2                     |
+|_______|___________________________________________________________________|
+|  16   |  Remote I2C Temperature Sensor with Address 3                     |
+|_______|___________________________________________________________________|
+|  17   |  Remote I2C Temperature Sensor with Address 4                     |
+|_______|___________________________________________________________________|
+|  17   |  Remote Voltage Connected to ADC0                                 |
+|_______|___________________________________________________________________|
+|  18   |  Remote Voltage Connected to ADC1                                 |
+|_______|___________________________________________________________________|
+|  19   |  Remote Voltage Connected to ADC2                                 |
+|_______|___________________________________________________________________|
+|  20   |  Remote Voltage Connected to ADC3                                 |
+|_______|___________________________________________________________________|
+|  21   |  Remote Voltage Connected to ADC4                                 |
+|_______|___________________________________________________________________|
+|  22   |  Remote Voltage Connected to ADC5                                 |
+|_______|___________________________________________________________________|
+|23-254 |  Reserved                                                         |
+|_______|___________________________________________________________________|
+|  255  |  Applies to all pages                                             |
+|_______|___________________________________________________________________|
+*/
+
+/*
+ * Place holder to save the max31785 mfr specific registers
+ */
+typedef struct MAX31785State {
+    PMBusDevice parent;
+    uint16_t mfr_mode[MAX31785_TOTAL_NUM_PAGES];
+    uint16_t vout_peak[MAX31785_TOTAL_NUM_PAGES];
+    uint16_t temperature_peak[MAX31785_TOTAL_NUM_PAGES];
+    uint16_t vout_min[MAX31785_TOTAL_NUM_PAGES];
+    uint8_t  fault_response[MAX31785_TOTAL_NUM_PAGES];
+    uint32_t time_count[MAX31785_TOTAL_NUM_PAGES];
+    uint16_t temp_sensor_config[MAX31785_TOTAL_NUM_PAGES];
+    uint16_t fan_config[MAX31785_TOTAL_NUM_PAGES];
+    uint16_t read_fan_pwm[MAX31785_TOTAL_NUM_PAGES];
+    uint16_t fan_fault_limit[MAX31785_TOTAL_NUM_PAGES];
+    uint16_t fan_warn_limit[MAX31785_TOTAL_NUM_PAGES];
+    uint16_t fan_run_time[MAX31785_TOTAL_NUM_PAGES];
+    uint16_t fan_pwm_avg[MAX31785_TOTAL_NUM_PAGES];
+    uint64_t fan_pwm2rpm[MAX31785_TOTAL_NUM_PAGES];
+    uint64_t mfr_location;
+    uint64_t mfr_date;
+    uint64_t mfr_serial;
+    uint16_t mfr_revision;
+} MAX31785State;
+
+static uint8_t max31785_read_byte(PMBusDevice *pmdev)
+{
+    MAX31785State *s = MAX31785(pmdev);
+    switch (pmdev->code) {
+
+    case PMBUS_FAN_CONFIG_1_2:
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            pmbus_send8(pmdev, pmdev->pages[pmdev->page].fan_config_1_2);
+        }
+        break;
+
+    case PMBUS_FAN_COMMAND_1:
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            pmbus_send16(pmdev, pmdev->pages[pmdev->page].fan_command_1);
+        }
+        break;
+
+    case PMBUS_READ_FAN_SPEED_1:
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            pmbus_send16(pmdev, pmdev->pages[pmdev->page].read_fan_speed_1);
+        }
+        break;
+
+    case PMBUS_STATUS_FANS_1_2:
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            pmbus_send16(pmdev, pmdev->pages[pmdev->page].status_fans_1_2);
+        }
+        break;
+
+    case PMBUS_MFR_REVISION:
+        pmbus_send16(pmdev, MAX31785_DEFAULT_MFR_REVISION);
+        break;
+
+    case PMBUS_MFR_ID:
+        pmbus_send8(pmdev, 0x4d); /* Maxim */
+        break;
+
+    case PMBUS_MFR_MODEL:
+        pmbus_send8(pmdev, 0x53);
+        break;
+
+    case PMBUS_MFR_LOCATION:
+        pmbus_send64(pmdev, s->mfr_location);
+        break;
+
+    case PMBUS_MFR_DATE:
+        pmbus_send64(pmdev, s->mfr_date);
+        break;
+
+    case PMBUS_MFR_SERIAL:
+        pmbus_send64(pmdev, s->mfr_serial);
+        break;
+
+    case MAX31785_MFR_MODE:
+        pmbus_send16(pmdev, s->mfr_mode[pmdev->page]);
+        break;
+
+    case MAX31785_MFR_VOUT_PEAK:
+        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
+            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
+            pmbus_send16(pmdev, s->vout_peak[pmdev->page]);
+        }
+        break;
+
+    case MAX31785_MFR_TEMPERATURE_PEAK:
+        if ((pmdev->page >= MAX31785_MIN_TEMP_PAGE) &&
+            (pmdev->page <= MAX31785_MAX_TEMP_PAGE)) {
+            pmbus_send16(pmdev, s->temperature_peak[pmdev->page]);
+        }
+        break;
+
+    case MAX31785_MFR_VOUT_MIN:
+        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
+            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
+            pmbus_send16(pmdev, s->vout_min[pmdev->page]);
+        }
+        break;
+
+    case MAX31785_MFR_FAULT_RESPONSE:
+        pmbus_send8(pmdev, s->fault_response[pmdev->page]);
+        break;
+
+    case MAX31785_MFR_TIME_COUNT: /* R/W 32 */
+        pmbus_send32(pmdev, s->time_count[pmdev->page]);
+        break;
+
+    case MAX31785_MFR_TEMP_SENSOR_CONFIG: /* R/W 16 */
+        if ((pmdev->page >= MAX31785_MIN_TEMP_PAGE) &&
+            (pmdev->page <= MAX31785_MAX_TEMP_PAGE)) {
+            pmbus_send16(pmdev, s->temp_sensor_config[pmdev->page]);
+        }
+        break;
+
+    case MAX31785_MFR_FAN_CONFIG: /* R/W 16 */
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            pmbus_send16(pmdev, s->fan_config[pmdev->page]);
+        }
+        break;
+
+    case MAX31785_MFR_READ_FAN_PWM: /* R/W 16 */
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            pmbus_send16(pmdev, s->read_fan_pwm[pmdev->page]);
+        }
+        break;
+
+    case MAX31785_MFR_FAN_FAULT_LIMIT: /* R/W 16 */
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            pmbus_send16(pmdev, s->fan_fault_limit[pmdev->page]);
+        }
+        break;
+
+    case MAX31785_MFR_FAN_WARN_LIMIT: /* R/W 16 */
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            pmbus_send16(pmdev, s->fan_warn_limit[pmdev->page]);
+        }
+        break;
+
+    case MAX31785_MFR_FAN_RUN_TIME: /* R/W 16 */
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            pmbus_send16(pmdev, s->fan_run_time[pmdev->page]);
+        }
+        break;
+
+    case MAX31785_MFR_FAN_PWM_AVG: /* R/W 16 */
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            pmbus_send16(pmdev, s->fan_pwm_avg[pmdev->page]);
+        }
+        break;
+
+    case MAX31785_MFR_FAN_PWM2RPM: /* R/W 64 */
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            pmbus_send64(pmdev, s->fan_pwm2rpm[pmdev->page]);
+        }
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+        "%s: reading from unsupported register: 0x%02x\n",
+        __func__, pmdev->code);
+        break;
+    }
+    return 0xFF;
+}
+
+static int max31785_write_data(PMBusDevice *pmdev, const uint8_t *buf,
+                                uint8_t len)
+{
+    MAX31785State *s = MAX31785(pmdev);
+    if (len == 0) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: writing empty data\n", __func__);
+        return -1;
+    }
+
+    pmdev->code = buf[0]; /* PMBus command code */
+
+    if (len == 1) {
+        return 0;
+    }
+
+    /* Exclude command code from buffer */
+    buf++;
+    len--;
+
+    switch (pmdev->code) {
+
+    case PMBUS_FAN_CONFIG_1_2:
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            pmdev->pages[pmdev->page].fan_config_1_2 = pmbus_receive8(pmdev);
+        }
+        break;
+
+    case PMBUS_FAN_COMMAND_1:
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            pmdev->pages[pmdev->page].fan_command_1 = pmbus_receive16(pmdev);
+            pmdev->pages[pmdev->page].read_fan_speed_1 =
+                ((MAX31785_DEFAULT_FAN_SPEED / MAX31785_DEFAULT_FAN_MAX_PWM) *
+                pmdev->pages[pmdev->page].fan_command_1);
+        }
+        break;
+
+    case PMBUS_MFR_LOCATION: /* R/W 64 */
+        s->mfr_location = pmbus_receive64(pmdev);
+        break;
+
+    case PMBUS_MFR_DATE: /* R/W 64 */
+        s->mfr_date = pmbus_receive64(pmdev);
+        break;
+
+    case PMBUS_MFR_SERIAL: /* R/W 64 */
+        s->mfr_serial = pmbus_receive64(pmdev);
+        break;
+
+    case MAX31785_MFR_MODE: /* R/W word */
+        s->mfr_mode[pmdev->page] = pmbus_receive16(pmdev);
+        break;
+
+    case MAX31785_MFR_VOUT_PEAK: /* R/W word */
+        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
+            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
+            s->vout_peak[pmdev->page] = pmbus_receive16(pmdev);
+        }
+        break;
+
+    case MAX31785_MFR_TEMPERATURE_PEAK: /* R/W word */
+        if ((pmdev->page >= 6) && (pmdev->page <= 16)) {
+            s->temperature_peak[pmdev->page] = pmbus_receive16(pmdev);
+        }
+        break;
+
+    case MAX31785_MFR_VOUT_MIN: /* R/W word */
+        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
+            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
+            s->vout_min[pmdev->page] = pmbus_receive16(pmdev);
+        }
+        break;
+
+    case MAX31785_MFR_FAULT_RESPONSE: /* R/W 8 */
+        s->fault_response[pmdev->page] = pmbus_receive8(pmdev);
+        break;
+
+    case MAX31785_MFR_TIME_COUNT: /* R/W 32 */
+        s->time_count[pmdev->page] = pmbus_receive32(pmdev);
+        break;
+
+    case MAX31785_MFR_TEMP_SENSOR_CONFIG: /* R/W 16 */
+        if ((pmdev->page >= MAX31785_MIN_TEMP_PAGE) &&
+            (pmdev->page <= MAX31785_MAX_TEMP_PAGE)) {
+            s->temp_sensor_config[pmdev->page] = pmbus_receive16(pmdev);
+        }
+        break;
+
+    case MAX31785_MFR_FAN_CONFIG: /* R/W 16 */
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            s->fan_config[pmdev->page] = pmbus_receive16(pmdev);
+        }
+        break;
+
+    case MAX31785_MFR_FAN_FAULT_LIMIT: /* R/W 16 */
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            s->fan_fault_limit[pmdev->page] = pmbus_receive16(pmdev);
+        }
+        break;
+
+    case MAX31785_MFR_FAN_WARN_LIMIT: /* R/W 16 */
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            s->fan_warn_limit[pmdev->page] = pmbus_receive16(pmdev);
+        }
+        break;
+
+    case MAX31785_MFR_FAN_RUN_TIME: /* R/W 16 */
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            s->fan_run_time[pmdev->page] = pmbus_receive16(pmdev);
+        }
+        break;
+
+    case MAX31785_MFR_FAN_PWM_AVG: /* R/W 16 */
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            s->fan_pwm_avg[pmdev->page] = pmbus_receive16(pmdev);
+        }
+        break;
+
+    case MAX31785_MFR_FAN_PWM2RPM: /* R/W 64 */
+        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
+            s->fan_pwm2rpm[pmdev->page] = pmbus_receive64(pmdev);
+        }
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: writing to unsupported register: 0x%02x\n",
+                      __func__, pmdev->code);
+        break;
+    }
+    return 0;
+}
+
+static void max31785_exit_reset(Object *obj)
+{
+    PMBusDevice *pmdev = PMBUS_DEVICE(obj);
+    MAX31785State *s = MAX31785(obj);
+
+    pmdev->capability = MAX31785_DEFAULT_CAPABILITY;
+
+    for (int i = MAX31785_MIN_FAN_PAGE; i <= MAX31785_MAX_FAN_PAGE; i++) {
+        pmdev->pages[i].vout_mode = MAX31785_DEFAULT_VOUT_MODE;
+        pmdev->pages[i].fan_command_1 = MAX31785_DEFAULT_FAN_COMMAND_1;
+        pmdev->pages[i].revision = MAX31785_DEFAULT_PMBUS_REVISION;
+        pmdev->pages[i].fan_config_1_2 = MAX31785_DEFAULT_FAN_CONFIG_1_2(0);
+        pmdev->pages[i].read_fan_speed_1 = MAX31785_DEFAULT_FAN_SPEED;
+        pmdev->pages[i].status_fans_1_2 = MAX31785_DEFAULT_FAN_STATUS;
+    }
+
+    for (int i = MAX31785_MIN_TEMP_PAGE; i <= MAX31785_MAX_TEMP_PAGE; i++) {
+        pmdev->pages[i].vout_mode = MAX31785_DEFAULT_VOUT_MODE;
+        pmdev->pages[i].revision = MAX31785_DEFAULT_PMBUS_REVISION;
+        pmdev->pages[i].ot_fault_limit = MAX31785_DEFAULT_OT_FAULT_LIMIT;
+        pmdev->pages[i].ot_warn_limit = MAX31785_DEFAULT_OT_WARN_LIMIT;
+    }
+
+    for (int i = MAX31785_MIN_ADC_VOLTAGE_PAGE;
+         i <= MAX31785_MAX_ADC_VOLTAGE_PAGE;
+         i++) {
+        pmdev->pages[i].vout_mode = MAX31785_DEFAULT_VOUT_MODE;
+        pmdev->pages[i].revision = MAX31785_DEFAULT_PMBUS_REVISION;
+        pmdev->pages[i].vout_scale_monitor =
+        MAX31785_DEFAULT_VOUT_SCALE_MONITOR;
+        pmdev->pages[i].vout_ov_fault_limit =
+        MAX31785_DEFAULT_OV_FAULT_LIMIT;
+        pmdev->pages[i].vout_ov_warn_limit =
+        MAX31785_DEFAULT_OV_WARN_LIMIT;
+    }
+
+    s->mfr_location = MAX31785_DEFAULT_TEXT;
+    s->mfr_date = MAX31785_DEFAULT_TEXT;
+    s->mfr_serial = MAX31785_DEFAULT_TEXT;
+}
+
+static const VMStateDescription vmstate_max31785 = {
+    .name = TYPE_MAX31785,
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .fields = (VMStateField[]){
+        VMSTATE_PMBUS_DEVICE(parent, MAX31785State),
+        VMSTATE_UINT16_ARRAY(mfr_mode, MAX31785State,
+        MAX31785_TOTAL_NUM_PAGES),
+        VMSTATE_UINT16_ARRAY(vout_peak, MAX31785State,
+        MAX31785_TOTAL_NUM_PAGES),
+        VMSTATE_UINT16_ARRAY(temperature_peak, MAX31785State,
+        MAX31785_TOTAL_NUM_PAGES),
+        VMSTATE_UINT16_ARRAY(vout_min, MAX31785State,
+        MAX31785_TOTAL_NUM_PAGES),
+        VMSTATE_UINT8_ARRAY(fault_response, MAX31785State,
+        MAX31785_TOTAL_NUM_PAGES),
+        VMSTATE_UINT32_ARRAY(time_count, MAX31785State,
+        MAX31785_TOTAL_NUM_PAGES),
+        VMSTATE_UINT16_ARRAY(temp_sensor_config, MAX31785State,
+        MAX31785_TOTAL_NUM_PAGES),
+        VMSTATE_UINT16_ARRAY(fan_config, MAX31785State,
+        MAX31785_TOTAL_NUM_PAGES),
+        VMSTATE_UINT16_ARRAY(read_fan_pwm, MAX31785State,
+        MAX31785_TOTAL_NUM_PAGES),
+        VMSTATE_UINT16_ARRAY(fan_fault_limit, MAX31785State,
+        MAX31785_TOTAL_NUM_PAGES),
+        VMSTATE_UINT16_ARRAY(fan_warn_limit, MAX31785State,
+        MAX31785_TOTAL_NUM_PAGES),
+        VMSTATE_UINT16_ARRAY(fan_run_time, MAX31785State,
+        MAX31785_TOTAL_NUM_PAGES),
+        VMSTATE_UINT16_ARRAY(fan_pwm_avg, MAX31785State,
+        MAX31785_TOTAL_NUM_PAGES),
+        VMSTATE_UINT64_ARRAY(fan_pwm2rpm, MAX31785State,
+        MAX31785_TOTAL_NUM_PAGES),
+        VMSTATE_UINT64(mfr_location, MAX31785State),
+        VMSTATE_UINT64(mfr_date, MAX31785State),
+        VMSTATE_UINT64(mfr_serial, MAX31785State),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void max31785_init(Object *obj)
+{
+    PMBusDevice *pmdev = PMBUS_DEVICE(obj);
+
+    for (int i = MAX31785_MIN_FAN_PAGE; i <= MAX31785_MAX_FAN_PAGE; i++) {
+        pmbus_page_config(pmdev, i, PB_HAS_VOUT_MODE);
+    }
+
+    for (int i = MAX31785_MIN_TEMP_PAGE; i <= MAX31785_MAX_TEMP_PAGE; i++) {
+        pmbus_page_config(pmdev, i, PB_HAS_VOUT_MODE | PB_HAS_TEMPERATURE);
+    }
+
+    for (int i = MAX31785_MIN_ADC_VOLTAGE_PAGE;
+        i <= MAX31785_MAX_ADC_VOLTAGE_PAGE;
+        i++) {
+        pmbus_page_config(pmdev, i, PB_HAS_VOUT_MODE | PB_HAS_VOUT |
+                                    PB_HAS_VOUT_RATING);
+    }
+}
+
+static void max31785_class_init(ObjectClass *klass, void *data)
+{
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PMBusDeviceClass *k = PMBUS_DEVICE_CLASS(klass);
+    dc->desc = "Maxim MAX31785 6-Channel Fan Controller";
+    dc->vmsd = &vmstate_max31785;
+    k->write_data = max31785_write_data;
+    k->receive_byte = max31785_read_byte;
+    k->device_num_pages = MAX31785_TOTAL_NUM_PAGES;
+    rc->phases.exit = max31785_exit_reset;
+}
+
+static const TypeInfo max31785_info = {
+    .name = TYPE_MAX31785,
+    .parent = TYPE_PMBUS_DEVICE,
+    .instance_size = sizeof(MAX31785State),
+    .instance_init = max31785_init,
+    .class_init = max31785_class_init,
+};
+
+static void max31785_register_types(void)
+{
+    type_register_static(&max31785_info);
+}
+
+type_init(max31785_register_types)
diff --git a/hw/sensor/meson.build b/hw/sensor/meson.build
index 12b6992bc845..9e9be602c349 100644
--- a/hw/sensor/meson.build
+++ b/hw/sensor/meson.build
@@ -6,3 +6,4 @@ softmmu_ss.add(when: 'CONFIG_ADM1272', if_true: files('adm1272.c'))
 softmmu_ss.add(when: 'CONFIG_MAX34451', if_true: files('max34451.c'))
 softmmu_ss.add(when: 'CONFIG_LSM303DLHC_MAG', if_true: files('lsm303dlhc_mag.c'))
 softmmu_ss.add(when: 'CONFIG_ISL_PMBUS_VR', if_true: files('isl_pmbus_vr.c'))
+softmmu_ss.add(when: 'CONFIG_MAX31785', if_true: files('max31785.c'))
-- 
2.25.1

Re: [PATCH 6/9] hw/sensor: add Maxim MAX31785 device

[Titus Rwantare]

On Wed, 22 Jun 2022 at 10:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
>
> From: Maheswara Kurapati <quic_mkurapat@quicinc.com>
>
> MAX31785 is a PMBus compliant 6-Channel fan controller. It supports 6 fan
> channels, 11 temperature sensors, and 6-Channel ADC to measure the remote
> voltages. Datasheet can be found here:
> https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
>
> This initial version of the driver has skeleton and support for the
> fan channels. Requests for temperature sensors, and ADC Channels the
> are serviced with the default values as per the datasheet.  No additional
> instrumentation is done.  NV Log feature is not supported.
>
> Signed-off-by: Maheswara Kurapati <quic_mkurapat@quicinc.com>
> ---
>  hw/arm/Kconfig        |   1 +
>  hw/arm/aspeed.c       |   6 +-
>  hw/sensor/Kconfig     |   4 +
>  hw/sensor/max31785.c  | 580 ++++++++++++++++++++++++++++++++++++++++++
>  hw/sensor/meson.build |   1 +
>  5 files changed, 590 insertions(+), 2 deletions(-)
>  create mode 100644 hw/sensor/max31785.c
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 219262a8da36..77ef0fa967b2 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -408,6 +408,7 @@ config NPCM7XX
>      select SSI
>      select UNIMP
>      select PCA954X
> +    select MAX31785
>
>  config FSL_IMX25
>      bool

As this is being used with the Aspeed 2600, you may need to select
PMBUS and MAX31785 under config ASPEED_SOC in this same file.


> diff --git a/hw/sensor/max31785.c b/hw/sensor/max31785.c
> new file mode 100644
> index 000000000000..11bf9977b6fd
> --- /dev/null
> +++ b/hw/sensor/max31785.c


Also, style nit, the checkpatch.pl script doesn't check whitespace
alignment. But the style guide
https://qemu-project.gitlab.io/qemu/devel/style.html#multiline-indent
specifies the variants we should use.

> +        pmdev->pages[i].vout_scale_monitor =
> +        MAX31785_DEFAULT_VOUT_SCALE_MONITOR;
> +        pmdev->pages[i].vout_ov_fault_limit =
> +        MAX31785_DEFAULT_OV_FAULT_LIMIT;
> +        pmdev->pages[i].vout_ov_warn_limit =
> +        MAX31785_DEFAULT_OV_WARN_LIMIT;
> +    }
> +
> +}
> +
> +static const VMStateDescription vmstate_max31785 = {
> +    .name = TYPE_MAX31785,
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .fields = (VMStateField[]){
> +        VMSTATE_PMBUS_DEVICE(parent, MAX31785State),
> +        VMSTATE_UINT16_ARRAY(mfr_mode, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(vout_peak, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(temperature_peak, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(vout_min, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT8_ARRAY(fault_response, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT32_ARRAY(time_count, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(temp_sensor_config, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_config, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(read_fan_pwm, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_fault_limit, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_warn_limit, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_run_time, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_pwm_avg, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT64_ARRAY(fan_pwm2rpm, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT64(mfr_location, MAX31785State),
> +        VMSTATE_UINT64(mfr_date, MAX31785State),
> +        VMSTATE_UINT64(mfr_serial, MAX31785State),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
There's missing indentation here for example.


Thanks,
Titus Rwantare

Re: [PATCH 6/9] hw/sensor: add Maxim MAX31785 device

[Jae Hyun Yoo]

Hello Titus,

On 6/22/2022 1:49 PM, Titus Rwantare wrote:
> On Wed, 22 Jun 2022 at 10:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
>>
>> From: Maheswara Kurapati <quic_mkurapat@quicinc.com>
>>
>> MAX31785 is a PMBus compliant 6-Channel fan controller. It supports 6 fan
>> channels, 11 temperature sensors, and 6-Channel ADC to measure the remote
>> voltages. Datasheet can be found here:
>> https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
>>
>> This initial version of the driver has skeleton and support for the
>> fan channels. Requests for temperature sensors, and ADC Channels the
>> are serviced with the default values as per the datasheet.  No additional
>> instrumentation is done.  NV Log feature is not supported.
>>
>> Signed-off-by: Maheswara Kurapati <quic_mkurapat@quicinc.com>
>> ---
>>   hw/arm/Kconfig        |   1 +
>>   hw/arm/aspeed.c       |   6 +-
>>   hw/sensor/Kconfig     |   4 +
>>   hw/sensor/max31785.c  | 580 ++++++++++++++++++++++++++++++++++++++++++
>>   hw/sensor/meson.build |   1 +
>>   5 files changed, 590 insertions(+), 2 deletions(-)
>>   create mode 100644 hw/sensor/max31785.c
>>
>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>> index 219262a8da36..77ef0fa967b2 100644
>> --- a/hw/arm/Kconfig
>> +++ b/hw/arm/Kconfig
>> @@ -408,6 +408,7 @@ config NPCM7XX
>>       select SSI
>>       select UNIMP
>>       select PCA954X
>> +    select MAX31785
>>
>>   config FSL_IMX25
>>       bool
> 
> As this is being used with the Aspeed 2600, you may need to select
> PMBUS and MAX31785 under config ASPEED_SOC in this same file.

Okay. Will fix it in v2.

>> diff --git a/hw/sensor/max31785.c b/hw/sensor/max31785.c
>> new file mode 100644
>> index 000000000000..11bf9977b6fd
>> --- /dev/null
>> +++ b/hw/sensor/max31785.c
> 
> 
> Also, style nit, the checkpatch.pl script doesn't check whitespace
> alignment. But the style guide
> https://qemu-project.gitlab.io/qemu/devel/style.html#multiline-indent
> specifies the variants we should use.

Will fix the indentation in v2.

>> +        pmdev->pages[i].vout_scale_monitor =
>> +        MAX31785_DEFAULT_VOUT_SCALE_MONITOR;
>> +        pmdev->pages[i].vout_ov_fault_limit =
>> +        MAX31785_DEFAULT_OV_FAULT_LIMIT;
>> +        pmdev->pages[i].vout_ov_warn_limit =
>> +        MAX31785_DEFAULT_OV_WARN_LIMIT;
>> +    }
>> +
>> +}
>> +
>> +static const VMStateDescription vmstate_max31785 = {
>> +    .name = TYPE_MAX31785,
>> +    .version_id = 0,
>> +    .minimum_version_id = 0,
>> +    .fields = (VMStateField[]){
>> +        VMSTATE_PMBUS_DEVICE(parent, MAX31785State),
>> +        VMSTATE_UINT16_ARRAY(mfr_mode, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(vout_peak, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(temperature_peak, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(vout_min, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT8_ARRAY(fault_response, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT32_ARRAY(time_count, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(temp_sensor_config, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(fan_config, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(read_fan_pwm, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(fan_fault_limit, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(fan_warn_limit, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(fan_run_time, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(fan_pwm_avg, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT64_ARRAY(fan_pwm2rpm, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT64(mfr_location, MAX31785State),
>> +        VMSTATE_UINT64(mfr_date, MAX31785State),
>> +        VMSTATE_UINT64(mfr_serial, MAX31785State),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
> There's missing indentation here for example.

Will fix it too.

Thanks,
Jae

Re: [PATCH 6/9] hw/sensor: add Maxim MAX31785 device

[Joel Stanley]

On Wed, 22 Jun 2022 at 17:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
>
> From: Maheswara Kurapati <quic_mkurapat@quicinc.com>
>
> MAX31785 is a PMBus compliant 6-Channel fan controller. It supports 6 fan
> channels, 11 temperature sensors, and 6-Channel ADC to measure the remote
> voltages. Datasheet can be found here:
> https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
>
> This initial version of the driver has skeleton and support for the
> fan channels. Requests for temperature sensors, and ADC Channels the
> are serviced with the default values as per the datasheet.  No additional
> instrumentation is done.  NV Log feature is not supported.
>
> Signed-off-by: Maheswara Kurapati <quic_mkurapat@quicinc.com>

An excellent contribution. Thanks Maheswara and Jae.

Reviewed-by: Joel Stanley <joel@jms.id.au>


> ---
>  hw/arm/Kconfig        |   1 +
>  hw/arm/aspeed.c       |   6 +-
>  hw/sensor/Kconfig     |   4 +
>  hw/sensor/max31785.c  | 580 ++++++++++++++++++++++++++++++++++++++++++
>  hw/sensor/meson.build |   1 +
>  5 files changed, 590 insertions(+), 2 deletions(-)
>  create mode 100644 hw/sensor/max31785.c
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 219262a8da36..77ef0fa967b2 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -408,6 +408,7 @@ config NPCM7XX
>      select SSI
>      select UNIMP
>      select PCA954X
> +    select MAX31785
>
>  config FSL_IMX25
>      bool
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 0e6edd2be4fa..85630b497793 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -619,7 +619,6 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
>      LEDState *led;
>
>      /* Bus 3: TODO bmp280@77 */
> -    /* Bus 3: TODO max31785@52 */
>      dev = DEVICE(i2c_slave_new(TYPE_PCA9552, 0x60));
>      qdev_prop_set_string(dev, "description", "pca1");
>      i2c_slave_realize_and_unref(I2C_SLAVE(dev),
> @@ -635,6 +634,8 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
>                                qdev_get_gpio_in(DEVICE(led), 0));
>      }
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3), "dps310", 0x76);
> +    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3), "max31785",
> +                            0x52);
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), "tmp423", 0x4c);
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 5), "tmp423", 0x4c);
>
> @@ -779,13 +780,14 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
>      create_pca9552(soc, 7, 0x31);
>      create_pca9552(soc, 7, 0x32);
>      create_pca9552(soc, 7, 0x33);
> -    /* Bus 7: TODO max31785@52 */
>      create_pca9552(soc, 7, 0x60);
>      create_pca9552(soc, 7, 0x61);
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7), "dps310", 0x76);
>      /* Bus 7: TODO si7021-a20@20 */
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7), TYPE_TMP105,
>                       0x48);
> +    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7), "max31785",
> +                     0x52);
>      aspeed_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 7), 0x50, 64 * KiB);
>      aspeed_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 7), 0x51, 64 * KiB);
>
> diff --git a/hw/sensor/Kconfig b/hw/sensor/Kconfig
> index df392e786904..e03bd09b50e8 100644
> --- a/hw/sensor/Kconfig
> +++ b/hw/sensor/Kconfig
> @@ -34,3 +34,7 @@ config LSM303DLHC_MAG
>  config ISL_PMBUS_VR
>      bool
>      depends on PMBUS
> +
> +config MAX31785
> +    bool
> +    depends on PMBUS
> diff --git a/hw/sensor/max31785.c b/hw/sensor/max31785.c
> new file mode 100644
> index 000000000000..11bf9977b6fd
> --- /dev/null
> +++ b/hw/sensor/max31785.c
> @@ -0,0 +1,580 @@
> +/*
> + * Maxim MAX31785 PMBus 6-Channel Fan Controller
> + *
> + * Datasheet:
> + * https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
> + *
> + * Copyright(c) 2022 Qualcomm Innovative Center, Inc. All rights reserved.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/i2c/pmbus_device.h"
> +#include "hw/irq.h"
> +#include "migration/vmstate.h"
> +#include "qapi/error.h"
> +#include "qapi/visitor.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +
> +#define TYPE_MAX31785   "max31785"
> +#define MAX31785(obj) OBJECT_CHECK(MAX31785State, (obj), TYPE_MAX31785)
> +
> +/*
> + * MAX31785 mfr specific PMBus commands
> + */
> +#define MAX31785_MFR_MODE               0xD1
> +#define MAX31785_MFR_PSEN_CONFIG        0xD2
> +#define MAX31785_MFR_VOUT_PEAK          0xD4
> +#define MAX31785_MFR_TEMPERATURE_PEAK   0xD6
> +#define MAX31785_MFR_VOUT_MIN           0xD7
> +#define MAX31785_MFR_FAULT_RESPONSE     0xD9
> +#define MAX31785_MFR_NV_FAULT_LOG       0xDC
> +#define MAX31785_MFR_TIME_COUNT         0xDD
> +#define MAX31785_MFR_TEMP_SENSOR_CONFIG 0xF0
> +#define MAX31785_MFR_FAN_CONFIG         0xF1
> +#define MAX31785_MFR_FAN_LUT            0xF2
> +#define MAX31785_MFR_READ_FAN_PWM       0xF3
> +#define MAX31785_MFR_FAN_FAULT_LIMIT    0xF5
> +#define MAX31785_MFR_FAN_WARN_LIMIT     0xF6
> +#define MAX31785_MFR_FAN_RUN_TIME       0xF7
> +#define MAX31785_MFR_FAN_PWM_AVG        0xF8
> +#define MAX31785_MFR_FAN_PWM2RPM        0xF9
> +
> +/*
> + * defaults as per the data sheet
> + */
> +#define MAX31785_DEFAULT_CAPABILITY         0x10
> +#define MAX31785_DEFAULT_VOUT_MODE          0x40
> +#define MAX31785_DEFAULT_VOUT_SCALE_MONITOR 0x7FFF
> +#define MAX31785_DEFAULT_FAN_COMMAND_1      0x7FFF
> +#define MAX31785_DEFAULT_OV_FAULT_LIMIT     0x7FFF
> +#define MAX31785_DEFAULT_OV_WARN_LIMIT      0x7FFF
> +#define MAX31785_DEFAULT_OT_FAULT_LIMIT     0x7FFF
> +#define MAX31785_DEFAULT_OT_WARN_LIMIT      0x7FFF
> +#define MAX31785_DEFAULT_PMBUS_REVISION     0x11
> +#define MAX31785_DEFAULT_MFR_ID             0x4D
> +#define MAX31785_DEFAULT_MFR_MODEL          0x53
> +#define MAX31785_DEFAULT_MFR_REVISION       0x3030
> +#define MAX31785A_DEFAULT_MFR_REVISION      0x3040
> +#define MAX31785B_DEFAULT_MFR_REVISION      0x3061
> +#define MAX31785B_DEFAULT_MFR_TEMPERATURE_PEAK   0x8000
> +#define MAX31785B_DEFAULT_MFR_VOUT_MIN      0x7FFF
> +#define MAX31785_DEFAULT_TEXT               0x3130313031303130
> +
> +/* MAX31785 pages */
> +#define MAX31785_TOTAL_NUM_PAGES    23
> +#define MAX31785_FAN_PAGES          6
> +#define MAX31785_MIN_FAN_PAGE       0
> +#define MAX31785_MAX_FAN_PAGE       5
> +#define MAX31785_MIN_TEMP_PAGE      6
> +#define MAX31785_MAX_TEMP_PAGE      16
> +#define MAX31785_MIN_ADC_VOLTAGE_PAGE   17
> +#define MAX31785_MAX_ADC_VOLTAGE_PAGE   22
> +
> +/* FAN_CONFIG_1_2 */
> +#define MAX31785_MFR_FAN_CONFIG         0xF1
> +#define MAX31785_FAN_CONFIG_ENABLE         BIT(7)
> +#define MAX31785_FAN_CONFIG_RPM_PWM        BIT(6)
> +#define MAX31785_FAN_CONFIG_PULSE(pulse)   (pulse << 4)
> +#define MAX31785_DEFAULT_FAN_CONFIG_1_2(pulse) (MAX31785_FAN_CONFIG_ENABLE |\
> +MAX31785_FAN_CONFIG_PULSE(pulse))
> +#define MAX31785_DEFAULT_MFR_FAN_CONFIG     0x0000
> +
> +/* fan speed in RPM */
> +#define MAX31785_DEFAULT_FAN_SPEED          0x7fff
> +#define MAX31785_DEFAULT_FAN_STATUS         0x00
> +
> +#define MAX31785_DEFAULT_FAN_MAX_PWM        0x2710
> +
> +/*
> + * MAX31785State:
> + * @code: The command code received
> + * @page: Each page corresponds to a device monitored by the Max 31785
> + * The page register determines the available commands depending on device
> +___________________________________________________________________________
> +|   0   |  Fan Connected to PWM0                                            |
> +|_______|___________________________________________________________________|
> +|   1   |  Fan Connected to PWM1                                            |
> +|_______|___________________________________________________________________|
> +|   2   |  Fan Connected to PWM2                                            |
> +|_______|___________________________________________________________________|
> +|   3   |  Fan Connected to PWM3                                            |
> +|_______|___________________________________________________________________|
> +|   4   |  Fan Connected to PWM4                                            |
> +|_______|___________________________________________________________________|
> +|   5   |  Fan Connected to PWM5                                            |
> +|_______|___________________________________________________________________|
> +|   6   |  Remote Thermal Diode Connected to ADC 0                          |
> +|_______|___________________________________________________________________|
> +|   7   |  Remote Thermal Diode Connected to ADC 1                          |
> +|_______|___________________________________________________________________|
> +|   8   |  Remote Thermal Diode Connected to ADC 2                          |
> +|_______|___________________________________________________________________|
> +|   9   |  Remote Thermal Diode Connected to ADC 3                          |
> +|_______|___________________________________________________________________|
> +|  10   |  Remote Thermal Diode Connected to ADC 4                          |
> +|_______|___________________________________________________________________|
> +|  11   |  Remote Thermal Diode Connected to ADC 5                          |
> +|_______|___________________________________________________________________|
> +|  12   |  Internal Temperature Sensor                                      |
> +|_______|___________________________________________________________________|
> +|  13   |  Remote I2C Temperature Sensor with Address 0                     |
> +|_______|___________________________________________________________________|
> +|  14   |  Remote I2C Temperature Sensor with Address 1                     |
> +|_______|___________________________________________________________________|
> +|  15   |  Remote I2C Temperature Sensor with Address 2                     |
> +|_______|___________________________________________________________________|
> +|  16   |  Remote I2C Temperature Sensor with Address 3                     |
> +|_______|___________________________________________________________________|
> +|  17   |  Remote I2C Temperature Sensor with Address 4                     |
> +|_______|___________________________________________________________________|
> +|  17   |  Remote Voltage Connected to ADC0                                 |
> +|_______|___________________________________________________________________|
> +|  18   |  Remote Voltage Connected to ADC1                                 |
> +|_______|___________________________________________________________________|
> +|  19   |  Remote Voltage Connected to ADC2                                 |
> +|_______|___________________________________________________________________|
> +|  20   |  Remote Voltage Connected to ADC3                                 |
> +|_______|___________________________________________________________________|
> +|  21   |  Remote Voltage Connected to ADC4                                 |
> +|_______|___________________________________________________________________|
> +|  22   |  Remote Voltage Connected to ADC5                                 |
> +|_______|___________________________________________________________________|
> +|23-254 |  Reserved                                                         |
> +|_______|___________________________________________________________________|
> +|  255  |  Applies to all pages                                             |
> +|_______|___________________________________________________________________|
> +*/
> +
> +/*
> + * Place holder to save the max31785 mfr specific registers
> + */
> +typedef struct MAX31785State {
> +    PMBusDevice parent;
> +    uint16_t mfr_mode[MAX31785_TOTAL_NUM_PAGES];
> +    uint16_t vout_peak[MAX31785_TOTAL_NUM_PAGES];
> +    uint16_t temperature_peak[MAX31785_TOTAL_NUM_PAGES];
> +    uint16_t vout_min[MAX31785_TOTAL_NUM_PAGES];
> +    uint8_t  fault_response[MAX31785_TOTAL_NUM_PAGES];
> +    uint32_t time_count[MAX31785_TOTAL_NUM_PAGES];
> +    uint16_t temp_sensor_config[MAX31785_TOTAL_NUM_PAGES];
> +    uint16_t fan_config[MAX31785_TOTAL_NUM_PAGES];
> +    uint16_t read_fan_pwm[MAX31785_TOTAL_NUM_PAGES];
> +    uint16_t fan_fault_limit[MAX31785_TOTAL_NUM_PAGES];
> +    uint16_t fan_warn_limit[MAX31785_TOTAL_NUM_PAGES];
> +    uint16_t fan_run_time[MAX31785_TOTAL_NUM_PAGES];
> +    uint16_t fan_pwm_avg[MAX31785_TOTAL_NUM_PAGES];
> +    uint64_t fan_pwm2rpm[MAX31785_TOTAL_NUM_PAGES];
> +    uint64_t mfr_location;
> +    uint64_t mfr_date;
> +    uint64_t mfr_serial;
> +    uint16_t mfr_revision;
> +} MAX31785State;
> +
> +static uint8_t max31785_read_byte(PMBusDevice *pmdev)
> +{
> +    MAX31785State *s = MAX31785(pmdev);
> +    switch (pmdev->code) {
> +
> +    case PMBUS_FAN_CONFIG_1_2:
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            pmbus_send8(pmdev, pmdev->pages[pmdev->page].fan_config_1_2);
> +        }
> +        break;
> +
> +    case PMBUS_FAN_COMMAND_1:
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            pmbus_send16(pmdev, pmdev->pages[pmdev->page].fan_command_1);
> +        }
> +        break;
> +
> +    case PMBUS_READ_FAN_SPEED_1:
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            pmbus_send16(pmdev, pmdev->pages[pmdev->page].read_fan_speed_1);
> +        }
> +        break;
> +
> +    case PMBUS_STATUS_FANS_1_2:
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            pmbus_send16(pmdev, pmdev->pages[pmdev->page].status_fans_1_2);
> +        }
> +        break;
> +
> +    case PMBUS_MFR_REVISION:
> +        pmbus_send16(pmdev, MAX31785_DEFAULT_MFR_REVISION);
> +        break;
> +
> +    case PMBUS_MFR_ID:
> +        pmbus_send8(pmdev, 0x4d); /* Maxim */
> +        break;
> +
> +    case PMBUS_MFR_MODEL:
> +        pmbus_send8(pmdev, 0x53);
> +        break;
> +
> +    case PMBUS_MFR_LOCATION:
> +        pmbus_send64(pmdev, s->mfr_location);
> +        break;
> +
> +    case PMBUS_MFR_DATE:
> +        pmbus_send64(pmdev, s->mfr_date);
> +        break;
> +
> +    case PMBUS_MFR_SERIAL:
> +        pmbus_send64(pmdev, s->mfr_serial);
> +        break;
> +
> +    case MAX31785_MFR_MODE:
> +        pmbus_send16(pmdev, s->mfr_mode[pmdev->page]);
> +        break;
> +
> +    case MAX31785_MFR_VOUT_PEAK:
> +        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
> +            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
> +            pmbus_send16(pmdev, s->vout_peak[pmdev->page]);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_TEMPERATURE_PEAK:
> +        if ((pmdev->page >= MAX31785_MIN_TEMP_PAGE) &&
> +            (pmdev->page <= MAX31785_MAX_TEMP_PAGE)) {
> +            pmbus_send16(pmdev, s->temperature_peak[pmdev->page]);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_VOUT_MIN:
> +        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
> +            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
> +            pmbus_send16(pmdev, s->vout_min[pmdev->page]);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_FAULT_RESPONSE:
> +        pmbus_send8(pmdev, s->fault_response[pmdev->page]);
> +        break;
> +
> +    case MAX31785_MFR_TIME_COUNT: /* R/W 32 */
> +        pmbus_send32(pmdev, s->time_count[pmdev->page]);
> +        break;
> +
> +    case MAX31785_MFR_TEMP_SENSOR_CONFIG: /* R/W 16 */
> +        if ((pmdev->page >= MAX31785_MIN_TEMP_PAGE) &&
> +            (pmdev->page <= MAX31785_MAX_TEMP_PAGE)) {
> +            pmbus_send16(pmdev, s->temp_sensor_config[pmdev->page]);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_FAN_CONFIG: /* R/W 16 */
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            pmbus_send16(pmdev, s->fan_config[pmdev->page]);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_READ_FAN_PWM: /* R/W 16 */
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            pmbus_send16(pmdev, s->read_fan_pwm[pmdev->page]);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_FAN_FAULT_LIMIT: /* R/W 16 */
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            pmbus_send16(pmdev, s->fan_fault_limit[pmdev->page]);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_FAN_WARN_LIMIT: /* R/W 16 */
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            pmbus_send16(pmdev, s->fan_warn_limit[pmdev->page]);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_FAN_RUN_TIME: /* R/W 16 */
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            pmbus_send16(pmdev, s->fan_run_time[pmdev->page]);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_FAN_PWM_AVG: /* R/W 16 */
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            pmbus_send16(pmdev, s->fan_pwm_avg[pmdev->page]);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_FAN_PWM2RPM: /* R/W 64 */
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            pmbus_send64(pmdev, s->fan_pwm2rpm[pmdev->page]);
> +        }
> +        break;
> +
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +        "%s: reading from unsupported register: 0x%02x\n",
> +        __func__, pmdev->code);
> +        break;
> +    }
> +    return 0xFF;
> +}
> +
> +static int max31785_write_data(PMBusDevice *pmdev, const uint8_t *buf,
> +                                uint8_t len)
> +{
> +    MAX31785State *s = MAX31785(pmdev);
> +    if (len == 0) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: writing empty data\n", __func__);
> +        return -1;
> +    }
> +
> +    pmdev->code = buf[0]; /* PMBus command code */
> +
> +    if (len == 1) {
> +        return 0;
> +    }
> +
> +    /* Exclude command code from buffer */
> +    buf++;
> +    len--;
> +
> +    switch (pmdev->code) {
> +
> +    case PMBUS_FAN_CONFIG_1_2:
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            pmdev->pages[pmdev->page].fan_config_1_2 = pmbus_receive8(pmdev);
> +        }
> +        break;
> +
> +    case PMBUS_FAN_COMMAND_1:
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            pmdev->pages[pmdev->page].fan_command_1 = pmbus_receive16(pmdev);
> +            pmdev->pages[pmdev->page].read_fan_speed_1 =
> +                ((MAX31785_DEFAULT_FAN_SPEED / MAX31785_DEFAULT_FAN_MAX_PWM) *
> +                pmdev->pages[pmdev->page].fan_command_1);
> +        }
> +        break;
> +
> +    case PMBUS_MFR_LOCATION: /* R/W 64 */
> +        s->mfr_location = pmbus_receive64(pmdev);
> +        break;
> +
> +    case PMBUS_MFR_DATE: /* R/W 64 */
> +        s->mfr_date = pmbus_receive64(pmdev);
> +        break;
> +
> +    case PMBUS_MFR_SERIAL: /* R/W 64 */
> +        s->mfr_serial = pmbus_receive64(pmdev);
> +        break;
> +
> +    case MAX31785_MFR_MODE: /* R/W word */
> +        s->mfr_mode[pmdev->page] = pmbus_receive16(pmdev);
> +        break;
> +
> +    case MAX31785_MFR_VOUT_PEAK: /* R/W word */
> +        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
> +            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
> +            s->vout_peak[pmdev->page] = pmbus_receive16(pmdev);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_TEMPERATURE_PEAK: /* R/W word */
> +        if ((pmdev->page >= 6) && (pmdev->page <= 16)) {
> +            s->temperature_peak[pmdev->page] = pmbus_receive16(pmdev);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_VOUT_MIN: /* R/W word */
> +        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
> +            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
> +            s->vout_min[pmdev->page] = pmbus_receive16(pmdev);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_FAULT_RESPONSE: /* R/W 8 */
> +        s->fault_response[pmdev->page] = pmbus_receive8(pmdev);
> +        break;
> +
> +    case MAX31785_MFR_TIME_COUNT: /* R/W 32 */
> +        s->time_count[pmdev->page] = pmbus_receive32(pmdev);
> +        break;
> +
> +    case MAX31785_MFR_TEMP_SENSOR_CONFIG: /* R/W 16 */
> +        if ((pmdev->page >= MAX31785_MIN_TEMP_PAGE) &&
> +            (pmdev->page <= MAX31785_MAX_TEMP_PAGE)) {
> +            s->temp_sensor_config[pmdev->page] = pmbus_receive16(pmdev);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_FAN_CONFIG: /* R/W 16 */
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            s->fan_config[pmdev->page] = pmbus_receive16(pmdev);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_FAN_FAULT_LIMIT: /* R/W 16 */
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            s->fan_fault_limit[pmdev->page] = pmbus_receive16(pmdev);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_FAN_WARN_LIMIT: /* R/W 16 */
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            s->fan_warn_limit[pmdev->page] = pmbus_receive16(pmdev);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_FAN_RUN_TIME: /* R/W 16 */
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            s->fan_run_time[pmdev->page] = pmbus_receive16(pmdev);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_FAN_PWM_AVG: /* R/W 16 */
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            s->fan_pwm_avg[pmdev->page] = pmbus_receive16(pmdev);
> +        }
> +        break;
> +
> +    case MAX31785_MFR_FAN_PWM2RPM: /* R/W 64 */
> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
> +            s->fan_pwm2rpm[pmdev->page] = pmbus_receive64(pmdev);
> +        }
> +        break;
> +
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: writing to unsupported register: 0x%02x\n",
> +                      __func__, pmdev->code);
> +        break;
> +    }
> +    return 0;
> +}
> +
> +static void max31785_exit_reset(Object *obj)
> +{
> +    PMBusDevice *pmdev = PMBUS_DEVICE(obj);
> +    MAX31785State *s = MAX31785(obj);
> +
> +    pmdev->capability = MAX31785_DEFAULT_CAPABILITY;
> +
> +    for (int i = MAX31785_MIN_FAN_PAGE; i <= MAX31785_MAX_FAN_PAGE; i++) {
> +        pmdev->pages[i].vout_mode = MAX31785_DEFAULT_VOUT_MODE;
> +        pmdev->pages[i].fan_command_1 = MAX31785_DEFAULT_FAN_COMMAND_1;
> +        pmdev->pages[i].revision = MAX31785_DEFAULT_PMBUS_REVISION;
> +        pmdev->pages[i].fan_config_1_2 = MAX31785_DEFAULT_FAN_CONFIG_1_2(0);
> +        pmdev->pages[i].read_fan_speed_1 = MAX31785_DEFAULT_FAN_SPEED;
> +        pmdev->pages[i].status_fans_1_2 = MAX31785_DEFAULT_FAN_STATUS;
> +    }
> +
> +    for (int i = MAX31785_MIN_TEMP_PAGE; i <= MAX31785_MAX_TEMP_PAGE; i++) {
> +        pmdev->pages[i].vout_mode = MAX31785_DEFAULT_VOUT_MODE;
> +        pmdev->pages[i].revision = MAX31785_DEFAULT_PMBUS_REVISION;
> +        pmdev->pages[i].ot_fault_limit = MAX31785_DEFAULT_OT_FAULT_LIMIT;
> +        pmdev->pages[i].ot_warn_limit = MAX31785_DEFAULT_OT_WARN_LIMIT;
> +    }
> +
> +    for (int i = MAX31785_MIN_ADC_VOLTAGE_PAGE;
> +         i <= MAX31785_MAX_ADC_VOLTAGE_PAGE;
> +         i++) {
> +        pmdev->pages[i].vout_mode = MAX31785_DEFAULT_VOUT_MODE;
> +        pmdev->pages[i].revision = MAX31785_DEFAULT_PMBUS_REVISION;
> +        pmdev->pages[i].vout_scale_monitor =
> +        MAX31785_DEFAULT_VOUT_SCALE_MONITOR;
> +        pmdev->pages[i].vout_ov_fault_limit =
> +        MAX31785_DEFAULT_OV_FAULT_LIMIT;
> +        pmdev->pages[i].vout_ov_warn_limit =
> +        MAX31785_DEFAULT_OV_WARN_LIMIT;
> +    }
> +
> +    s->mfr_location = MAX31785_DEFAULT_TEXT;
> +    s->mfr_date = MAX31785_DEFAULT_TEXT;
> +    s->mfr_serial = MAX31785_DEFAULT_TEXT;
> +}
> +
> +static const VMStateDescription vmstate_max31785 = {
> +    .name = TYPE_MAX31785,
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .fields = (VMStateField[]){
> +        VMSTATE_PMBUS_DEVICE(parent, MAX31785State),
> +        VMSTATE_UINT16_ARRAY(mfr_mode, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(vout_peak, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(temperature_peak, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(vout_min, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT8_ARRAY(fault_response, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT32_ARRAY(time_count, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(temp_sensor_config, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_config, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(read_fan_pwm, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_fault_limit, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_warn_limit, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_run_time, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT16_ARRAY(fan_pwm_avg, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT64_ARRAY(fan_pwm2rpm, MAX31785State,
> +        MAX31785_TOTAL_NUM_PAGES),
> +        VMSTATE_UINT64(mfr_location, MAX31785State),
> +        VMSTATE_UINT64(mfr_date, MAX31785State),
> +        VMSTATE_UINT64(mfr_serial, MAX31785State),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void max31785_init(Object *obj)
> +{
> +    PMBusDevice *pmdev = PMBUS_DEVICE(obj);
> +
> +    for (int i = MAX31785_MIN_FAN_PAGE; i <= MAX31785_MAX_FAN_PAGE; i++) {
> +        pmbus_page_config(pmdev, i, PB_HAS_VOUT_MODE);
> +    }
> +
> +    for (int i = MAX31785_MIN_TEMP_PAGE; i <= MAX31785_MAX_TEMP_PAGE; i++) {
> +        pmbus_page_config(pmdev, i, PB_HAS_VOUT_MODE | PB_HAS_TEMPERATURE);
> +    }
> +
> +    for (int i = MAX31785_MIN_ADC_VOLTAGE_PAGE;
> +        i <= MAX31785_MAX_ADC_VOLTAGE_PAGE;
> +        i++) {
> +        pmbus_page_config(pmdev, i, PB_HAS_VOUT_MODE | PB_HAS_VOUT |
> +                                    PB_HAS_VOUT_RATING);
> +    }
> +}
> +
> +static void max31785_class_init(ObjectClass *klass, void *data)
> +{
> +    ResettableClass *rc = RESETTABLE_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PMBusDeviceClass *k = PMBUS_DEVICE_CLASS(klass);
> +    dc->desc = "Maxim MAX31785 6-Channel Fan Controller";
> +    dc->vmsd = &vmstate_max31785;
> +    k->write_data = max31785_write_data;
> +    k->receive_byte = max31785_read_byte;
> +    k->device_num_pages = MAX31785_TOTAL_NUM_PAGES;
> +    rc->phases.exit = max31785_exit_reset;
> +}
> +
> +static const TypeInfo max31785_info = {
> +    .name = TYPE_MAX31785,
> +    .parent = TYPE_PMBUS_DEVICE,
> +    .instance_size = sizeof(MAX31785State),
> +    .instance_init = max31785_init,
> +    .class_init = max31785_class_init,
> +};
> +
> +static void max31785_register_types(void)
> +{
> +    type_register_static(&max31785_info);
> +}
> +
> +type_init(max31785_register_types)
> diff --git a/hw/sensor/meson.build b/hw/sensor/meson.build
> index 12b6992bc845..9e9be602c349 100644
> --- a/hw/sensor/meson.build
> +++ b/hw/sensor/meson.build
> @@ -6,3 +6,4 @@ softmmu_ss.add(when: 'CONFIG_ADM1272', if_true: files('adm1272.c'))
>  softmmu_ss.add(when: 'CONFIG_MAX34451', if_true: files('max34451.c'))
>  softmmu_ss.add(when: 'CONFIG_LSM303DLHC_MAG', if_true: files('lsm303dlhc_mag.c'))
>  softmmu_ss.add(when: 'CONFIG_ISL_PMBUS_VR', if_true: files('isl_pmbus_vr.c'))
> +softmmu_ss.add(when: 'CONFIG_MAX31785', if_true: files('max31785.c'))
> --
> 2.25.1
>

Re: [PATCH 6/9] hw/sensor: add Maxim MAX31785 device

[Cédric Le Goater]

On 6/23/22 07:17, Joel Stanley wrote:
> On Wed, 22 Jun 2022 at 17:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
>>
>> From: Maheswara Kurapati <quic_mkurapat@quicinc.com>
>>
>> MAX31785 is a PMBus compliant 6-Channel fan controller. It supports 6 fan
>> channels, 11 temperature sensors, and 6-Channel ADC to measure the remote
>> voltages. Datasheet can be found here:
>> https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
>>
>> This initial version of the driver has skeleton and support for the
>> fan channels. Requests for temperature sensors, and ADC Channels the
>> are serviced with the default values as per the datasheet.  No additional
>> instrumentation is done.  NV Log feature is not supported.
>>
>> Signed-off-by: Maheswara Kurapati <quic_mkurapat@quicinc.com>
> 
> An excellent contribution. Thanks Maheswara and Jae.
> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>

We could also update the models of these machines :

     aspeed-bmc-ibm-rainier.dts:	max: max31785@52 {
     aspeed-bmc-ibm-rainier.dts:		compatible = "maxim,max31785a";
     aspeed-bmc-opp-tacoma.dts:	max31785@52 {
     aspeed-bmc-opp-tacoma.dts:		compatible = "maxim,max31785a";
     aspeed-bmc-opp-witherspoon.dts:	max31785@52 {
     aspeed-bmc-opp-witherspoon.dts:		compatible = "maxim,max31785a";


For the merge, I would prefer to have separate patches. One adding the
sensor model and others updating the machines.

Thanks,
     
C.

> 
> 
>> ---
>>   hw/arm/Kconfig        |   1 +
>>   hw/arm/aspeed.c       |   6 +-
>>   hw/sensor/Kconfig     |   4 +
>>   hw/sensor/max31785.c  | 580 ++++++++++++++++++++++++++++++++++++++++++
>>   hw/sensor/meson.build |   1 +
>>   5 files changed, 590 insertions(+), 2 deletions(-)
>>   create mode 100644 hw/sensor/max31785.c
>>
>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>> index 219262a8da36..77ef0fa967b2 100644
>> --- a/hw/arm/Kconfig
>> +++ b/hw/arm/Kconfig
>> @@ -408,6 +408,7 @@ config NPCM7XX
>>       select SSI
>>       select UNIMP
>>       select PCA954X
>> +    select MAX31785
>>
>>   config FSL_IMX25
>>       bool
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index 0e6edd2be4fa..85630b497793 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -619,7 +619,6 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
>>       LEDState *led;
>>
>>       /* Bus 3: TODO bmp280@77 */
>> -    /* Bus 3: TODO max31785@52 */
>>       dev = DEVICE(i2c_slave_new(TYPE_PCA9552, 0x60));
>>       qdev_prop_set_string(dev, "description", "pca1");
>>       i2c_slave_realize_and_unref(I2C_SLAVE(dev),
>> @@ -635,6 +634,8 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
>>                                 qdev_get_gpio_in(DEVICE(led), 0));
>>       }
>>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3), "dps310", 0x76);
>> +    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3), "max31785",
>> +                            0x52);
>>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), "tmp423", 0x4c);
>>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 5), "tmp423", 0x4c);
>>
>> @@ -779,13 +780,14 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
>>       create_pca9552(soc, 7, 0x31);
>>       create_pca9552(soc, 7, 0x32);
>>       create_pca9552(soc, 7, 0x33);
>> -    /* Bus 7: TODO max31785@52 */
>>       create_pca9552(soc, 7, 0x60);
>>       create_pca9552(soc, 7, 0x61);
>>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7), "dps310", 0x76);
>>       /* Bus 7: TODO si7021-a20@20 */
>>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7), TYPE_TMP105,
>>                        0x48);
>> +    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7), "max31785",
>> +                     0x52);
>>       aspeed_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 7), 0x50, 64 * KiB);
>>       aspeed_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 7), 0x51, 64 * KiB);
>>
>> diff --git a/hw/sensor/Kconfig b/hw/sensor/Kconfig
>> index df392e786904..e03bd09b50e8 100644
>> --- a/hw/sensor/Kconfig
>> +++ b/hw/sensor/Kconfig
>> @@ -34,3 +34,7 @@ config LSM303DLHC_MAG
>>   config ISL_PMBUS_VR
>>       bool
>>       depends on PMBUS
>> +
>> +config MAX31785
>> +    bool
>> +    depends on PMBUS
>> diff --git a/hw/sensor/max31785.c b/hw/sensor/max31785.c
>> new file mode 100644
>> index 000000000000..11bf9977b6fd
>> --- /dev/null
>> +++ b/hw/sensor/max31785.c
>> @@ -0,0 +1,580 @@
>> +/*
>> + * Maxim MAX31785 PMBus 6-Channel Fan Controller
>> + *
>> + * Datasheet:
>> + * https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
>> + *
>> + * Copyright(c) 2022 Qualcomm Innovative Center, Inc. All rights reserved.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/i2c/pmbus_device.h"
>> +#include "hw/irq.h"
>> +#include "migration/vmstate.h"
>> +#include "qapi/error.h"
>> +#include "qapi/visitor.h"
>> +#include "qemu/log.h"
>> +#include "qemu/module.h"
>> +
>> +#define TYPE_MAX31785   "max31785"
>> +#define MAX31785(obj) OBJECT_CHECK(MAX31785State, (obj), TYPE_MAX31785)
>> +
>> +/*
>> + * MAX31785 mfr specific PMBus commands
>> + */
>> +#define MAX31785_MFR_MODE               0xD1
>> +#define MAX31785_MFR_PSEN_CONFIG        0xD2
>> +#define MAX31785_MFR_VOUT_PEAK          0xD4
>> +#define MAX31785_MFR_TEMPERATURE_PEAK   0xD6
>> +#define MAX31785_MFR_VOUT_MIN           0xD7
>> +#define MAX31785_MFR_FAULT_RESPONSE     0xD9
>> +#define MAX31785_MFR_NV_FAULT_LOG       0xDC
>> +#define MAX31785_MFR_TIME_COUNT         0xDD
>> +#define MAX31785_MFR_TEMP_SENSOR_CONFIG 0xF0
>> +#define MAX31785_MFR_FAN_CONFIG         0xF1
>> +#define MAX31785_MFR_FAN_LUT            0xF2
>> +#define MAX31785_MFR_READ_FAN_PWM       0xF3
>> +#define MAX31785_MFR_FAN_FAULT_LIMIT    0xF5
>> +#define MAX31785_MFR_FAN_WARN_LIMIT     0xF6
>> +#define MAX31785_MFR_FAN_RUN_TIME       0xF7
>> +#define MAX31785_MFR_FAN_PWM_AVG        0xF8
>> +#define MAX31785_MFR_FAN_PWM2RPM        0xF9
>> +
>> +/*
>> + * defaults as per the data sheet
>> + */
>> +#define MAX31785_DEFAULT_CAPABILITY         0x10
>> +#define MAX31785_DEFAULT_VOUT_MODE          0x40
>> +#define MAX31785_DEFAULT_VOUT_SCALE_MONITOR 0x7FFF
>> +#define MAX31785_DEFAULT_FAN_COMMAND_1      0x7FFF
>> +#define MAX31785_DEFAULT_OV_FAULT_LIMIT     0x7FFF
>> +#define MAX31785_DEFAULT_OV_WARN_LIMIT      0x7FFF
>> +#define MAX31785_DEFAULT_OT_FAULT_LIMIT     0x7FFF
>> +#define MAX31785_DEFAULT_OT_WARN_LIMIT      0x7FFF
>> +#define MAX31785_DEFAULT_PMBUS_REVISION     0x11
>> +#define MAX31785_DEFAULT_MFR_ID             0x4D
>> +#define MAX31785_DEFAULT_MFR_MODEL          0x53
>> +#define MAX31785_DEFAULT_MFR_REVISION       0x3030
>> +#define MAX31785A_DEFAULT_MFR_REVISION      0x3040
>> +#define MAX31785B_DEFAULT_MFR_REVISION      0x3061
>> +#define MAX31785B_DEFAULT_MFR_TEMPERATURE_PEAK   0x8000
>> +#define MAX31785B_DEFAULT_MFR_VOUT_MIN      0x7FFF
>> +#define MAX31785_DEFAULT_TEXT               0x3130313031303130
>> +
>> +/* MAX31785 pages */
>> +#define MAX31785_TOTAL_NUM_PAGES    23
>> +#define MAX31785_FAN_PAGES          6
>> +#define MAX31785_MIN_FAN_PAGE       0
>> +#define MAX31785_MAX_FAN_PAGE       5
>> +#define MAX31785_MIN_TEMP_PAGE      6
>> +#define MAX31785_MAX_TEMP_PAGE      16
>> +#define MAX31785_MIN_ADC_VOLTAGE_PAGE   17
>> +#define MAX31785_MAX_ADC_VOLTAGE_PAGE   22
>> +
>> +/* FAN_CONFIG_1_2 */
>> +#define MAX31785_MFR_FAN_CONFIG         0xF1
>> +#define MAX31785_FAN_CONFIG_ENABLE         BIT(7)
>> +#define MAX31785_FAN_CONFIG_RPM_PWM        BIT(6)
>> +#define MAX31785_FAN_CONFIG_PULSE(pulse)   (pulse << 4)
>> +#define MAX31785_DEFAULT_FAN_CONFIG_1_2(pulse) (MAX31785_FAN_CONFIG_ENABLE |\
>> +MAX31785_FAN_CONFIG_PULSE(pulse))
>> +#define MAX31785_DEFAULT_MFR_FAN_CONFIG     0x0000
>> +
>> +/* fan speed in RPM */
>> +#define MAX31785_DEFAULT_FAN_SPEED          0x7fff
>> +#define MAX31785_DEFAULT_FAN_STATUS         0x00
>> +
>> +#define MAX31785_DEFAULT_FAN_MAX_PWM        0x2710
>> +
>> +/*
>> + * MAX31785State:
>> + * @code: The command code received
>> + * @page: Each page corresponds to a device monitored by the Max 31785
>> + * The page register determines the available commands depending on device
>> +___________________________________________________________________________
>> +|   0   |  Fan Connected to PWM0                                            |
>> +|_______|___________________________________________________________________|
>> +|   1   |  Fan Connected to PWM1                                            |
>> +|_______|___________________________________________________________________|
>> +|   2   |  Fan Connected to PWM2                                            |
>> +|_______|___________________________________________________________________|
>> +|   3   |  Fan Connected to PWM3                                            |
>> +|_______|___________________________________________________________________|
>> +|   4   |  Fan Connected to PWM4                                            |
>> +|_______|___________________________________________________________________|
>> +|   5   |  Fan Connected to PWM5                                            |
>> +|_______|___________________________________________________________________|
>> +|   6   |  Remote Thermal Diode Connected to ADC 0                          |
>> +|_______|___________________________________________________________________|
>> +|   7   |  Remote Thermal Diode Connected to ADC 1                          |
>> +|_______|___________________________________________________________________|
>> +|   8   |  Remote Thermal Diode Connected to ADC 2                          |
>> +|_______|___________________________________________________________________|
>> +|   9   |  Remote Thermal Diode Connected to ADC 3                          |
>> +|_______|___________________________________________________________________|
>> +|  10   |  Remote Thermal Diode Connected to ADC 4                          |
>> +|_______|___________________________________________________________________|
>> +|  11   |  Remote Thermal Diode Connected to ADC 5                          |
>> +|_______|___________________________________________________________________|
>> +|  12   |  Internal Temperature Sensor                                      |
>> +|_______|___________________________________________________________________|
>> +|  13   |  Remote I2C Temperature Sensor with Address 0                     |
>> +|_______|___________________________________________________________________|
>> +|  14   |  Remote I2C Temperature Sensor with Address 1                     |
>> +|_______|___________________________________________________________________|
>> +|  15   |  Remote I2C Temperature Sensor with Address 2                     |
>> +|_______|___________________________________________________________________|
>> +|  16   |  Remote I2C Temperature Sensor with Address 3                     |
>> +|_______|___________________________________________________________________|
>> +|  17   |  Remote I2C Temperature Sensor with Address 4                     |
>> +|_______|___________________________________________________________________|
>> +|  17   |  Remote Voltage Connected to ADC0                                 |
>> +|_______|___________________________________________________________________|
>> +|  18   |  Remote Voltage Connected to ADC1                                 |
>> +|_______|___________________________________________________________________|
>> +|  19   |  Remote Voltage Connected to ADC2                                 |
>> +|_______|___________________________________________________________________|
>> +|  20   |  Remote Voltage Connected to ADC3                                 |
>> +|_______|___________________________________________________________________|
>> +|  21   |  Remote Voltage Connected to ADC4                                 |
>> +|_______|___________________________________________________________________|
>> +|  22   |  Remote Voltage Connected to ADC5                                 |
>> +|_______|___________________________________________________________________|
>> +|23-254 |  Reserved                                                         |
>> +|_______|___________________________________________________________________|
>> +|  255  |  Applies to all pages                                             |
>> +|_______|___________________________________________________________________|
>> +*/
>> +
>> +/*
>> + * Place holder to save the max31785 mfr specific registers
>> + */
>> +typedef struct MAX31785State {
>> +    PMBusDevice parent;
>> +    uint16_t mfr_mode[MAX31785_TOTAL_NUM_PAGES];
>> +    uint16_t vout_peak[MAX31785_TOTAL_NUM_PAGES];
>> +    uint16_t temperature_peak[MAX31785_TOTAL_NUM_PAGES];
>> +    uint16_t vout_min[MAX31785_TOTAL_NUM_PAGES];
>> +    uint8_t  fault_response[MAX31785_TOTAL_NUM_PAGES];
>> +    uint32_t time_count[MAX31785_TOTAL_NUM_PAGES];
>> +    uint16_t temp_sensor_config[MAX31785_TOTAL_NUM_PAGES];
>> +    uint16_t fan_config[MAX31785_TOTAL_NUM_PAGES];
>> +    uint16_t read_fan_pwm[MAX31785_TOTAL_NUM_PAGES];
>> +    uint16_t fan_fault_limit[MAX31785_TOTAL_NUM_PAGES];
>> +    uint16_t fan_warn_limit[MAX31785_TOTAL_NUM_PAGES];
>> +    uint16_t fan_run_time[MAX31785_TOTAL_NUM_PAGES];
>> +    uint16_t fan_pwm_avg[MAX31785_TOTAL_NUM_PAGES];
>> +    uint64_t fan_pwm2rpm[MAX31785_TOTAL_NUM_PAGES];
>> +    uint64_t mfr_location;
>> +    uint64_t mfr_date;
>> +    uint64_t mfr_serial;
>> +    uint16_t mfr_revision;
>> +} MAX31785State;
>> +
>> +static uint8_t max31785_read_byte(PMBusDevice *pmdev)
>> +{
>> +    MAX31785State *s = MAX31785(pmdev);
>> +    switch (pmdev->code) {
>> +
>> +    case PMBUS_FAN_CONFIG_1_2:
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            pmbus_send8(pmdev, pmdev->pages[pmdev->page].fan_config_1_2);
>> +        }
>> +        break;
>> +
>> +    case PMBUS_FAN_COMMAND_1:
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            pmbus_send16(pmdev, pmdev->pages[pmdev->page].fan_command_1);
>> +        }
>> +        break;
>> +
>> +    case PMBUS_READ_FAN_SPEED_1:
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            pmbus_send16(pmdev, pmdev->pages[pmdev->page].read_fan_speed_1);
>> +        }
>> +        break;
>> +
>> +    case PMBUS_STATUS_FANS_1_2:
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            pmbus_send16(pmdev, pmdev->pages[pmdev->page].status_fans_1_2);
>> +        }
>> +        break;
>> +
>> +    case PMBUS_MFR_REVISION:
>> +        pmbus_send16(pmdev, MAX31785_DEFAULT_MFR_REVISION);
>> +        break;
>> +
>> +    case PMBUS_MFR_ID:
>> +        pmbus_send8(pmdev, 0x4d); /* Maxim */
>> +        break;
>> +
>> +    case PMBUS_MFR_MODEL:
>> +        pmbus_send8(pmdev, 0x53);
>> +        break;
>> +
>> +    case PMBUS_MFR_LOCATION:
>> +        pmbus_send64(pmdev, s->mfr_location);
>> +        break;
>> +
>> +    case PMBUS_MFR_DATE:
>> +        pmbus_send64(pmdev, s->mfr_date);
>> +        break;
>> +
>> +    case PMBUS_MFR_SERIAL:
>> +        pmbus_send64(pmdev, s->mfr_serial);
>> +        break;
>> +
>> +    case MAX31785_MFR_MODE:
>> +        pmbus_send16(pmdev, s->mfr_mode[pmdev->page]);
>> +        break;
>> +
>> +    case MAX31785_MFR_VOUT_PEAK:
>> +        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
>> +            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
>> +            pmbus_send16(pmdev, s->vout_peak[pmdev->page]);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_TEMPERATURE_PEAK:
>> +        if ((pmdev->page >= MAX31785_MIN_TEMP_PAGE) &&
>> +            (pmdev->page <= MAX31785_MAX_TEMP_PAGE)) {
>> +            pmbus_send16(pmdev, s->temperature_peak[pmdev->page]);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_VOUT_MIN:
>> +        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
>> +            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
>> +            pmbus_send16(pmdev, s->vout_min[pmdev->page]);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_FAULT_RESPONSE:
>> +        pmbus_send8(pmdev, s->fault_response[pmdev->page]);
>> +        break;
>> +
>> +    case MAX31785_MFR_TIME_COUNT: /* R/W 32 */
>> +        pmbus_send32(pmdev, s->time_count[pmdev->page]);
>> +        break;
>> +
>> +    case MAX31785_MFR_TEMP_SENSOR_CONFIG: /* R/W 16 */
>> +        if ((pmdev->page >= MAX31785_MIN_TEMP_PAGE) &&
>> +            (pmdev->page <= MAX31785_MAX_TEMP_PAGE)) {
>> +            pmbus_send16(pmdev, s->temp_sensor_config[pmdev->page]);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_FAN_CONFIG: /* R/W 16 */
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            pmbus_send16(pmdev, s->fan_config[pmdev->page]);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_READ_FAN_PWM: /* R/W 16 */
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            pmbus_send16(pmdev, s->read_fan_pwm[pmdev->page]);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_FAN_FAULT_LIMIT: /* R/W 16 */
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            pmbus_send16(pmdev, s->fan_fault_limit[pmdev->page]);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_FAN_WARN_LIMIT: /* R/W 16 */
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            pmbus_send16(pmdev, s->fan_warn_limit[pmdev->page]);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_FAN_RUN_TIME: /* R/W 16 */
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            pmbus_send16(pmdev, s->fan_run_time[pmdev->page]);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_FAN_PWM_AVG: /* R/W 16 */
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            pmbus_send16(pmdev, s->fan_pwm_avg[pmdev->page]);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_FAN_PWM2RPM: /* R/W 64 */
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            pmbus_send64(pmdev, s->fan_pwm2rpm[pmdev->page]);
>> +        }
>> +        break;
>> +
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +        "%s: reading from unsupported register: 0x%02x\n",
>> +        __func__, pmdev->code);
>> +        break;
>> +    }
>> +    return 0xFF;
>> +}
>> +
>> +static int max31785_write_data(PMBusDevice *pmdev, const uint8_t *buf,
>> +                                uint8_t len)
>> +{
>> +    MAX31785State *s = MAX31785(pmdev);
>> +    if (len == 0) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: writing empty data\n", __func__);
>> +        return -1;
>> +    }
>> +
>> +    pmdev->code = buf[0]; /* PMBus command code */
>> +
>> +    if (len == 1) {
>> +        return 0;
>> +    }
>> +
>> +    /* Exclude command code from buffer */
>> +    buf++;
>> +    len--;
>> +
>> +    switch (pmdev->code) {
>> +
>> +    case PMBUS_FAN_CONFIG_1_2:
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            pmdev->pages[pmdev->page].fan_config_1_2 = pmbus_receive8(pmdev);
>> +        }
>> +        break;
>> +
>> +    case PMBUS_FAN_COMMAND_1:
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            pmdev->pages[pmdev->page].fan_command_1 = pmbus_receive16(pmdev);
>> +            pmdev->pages[pmdev->page].read_fan_speed_1 =
>> +                ((MAX31785_DEFAULT_FAN_SPEED / MAX31785_DEFAULT_FAN_MAX_PWM) *
>> +                pmdev->pages[pmdev->page].fan_command_1);
>> +        }
>> +        break;
>> +
>> +    case PMBUS_MFR_LOCATION: /* R/W 64 */
>> +        s->mfr_location = pmbus_receive64(pmdev);
>> +        break;
>> +
>> +    case PMBUS_MFR_DATE: /* R/W 64 */
>> +        s->mfr_date = pmbus_receive64(pmdev);
>> +        break;
>> +
>> +    case PMBUS_MFR_SERIAL: /* R/W 64 */
>> +        s->mfr_serial = pmbus_receive64(pmdev);
>> +        break;
>> +
>> +    case MAX31785_MFR_MODE: /* R/W word */
>> +        s->mfr_mode[pmdev->page] = pmbus_receive16(pmdev);
>> +        break;
>> +
>> +    case MAX31785_MFR_VOUT_PEAK: /* R/W word */
>> +        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
>> +            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
>> +            s->vout_peak[pmdev->page] = pmbus_receive16(pmdev);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_TEMPERATURE_PEAK: /* R/W word */
>> +        if ((pmdev->page >= 6) && (pmdev->page <= 16)) {
>> +            s->temperature_peak[pmdev->page] = pmbus_receive16(pmdev);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_VOUT_MIN: /* R/W word */
>> +        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
>> +            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
>> +            s->vout_min[pmdev->page] = pmbus_receive16(pmdev);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_FAULT_RESPONSE: /* R/W 8 */
>> +        s->fault_response[pmdev->page] = pmbus_receive8(pmdev);
>> +        break;
>> +
>> +    case MAX31785_MFR_TIME_COUNT: /* R/W 32 */
>> +        s->time_count[pmdev->page] = pmbus_receive32(pmdev);
>> +        break;
>> +
>> +    case MAX31785_MFR_TEMP_SENSOR_CONFIG: /* R/W 16 */
>> +        if ((pmdev->page >= MAX31785_MIN_TEMP_PAGE) &&
>> +            (pmdev->page <= MAX31785_MAX_TEMP_PAGE)) {
>> +            s->temp_sensor_config[pmdev->page] = pmbus_receive16(pmdev);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_FAN_CONFIG: /* R/W 16 */
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            s->fan_config[pmdev->page] = pmbus_receive16(pmdev);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_FAN_FAULT_LIMIT: /* R/W 16 */
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            s->fan_fault_limit[pmdev->page] = pmbus_receive16(pmdev);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_FAN_WARN_LIMIT: /* R/W 16 */
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            s->fan_warn_limit[pmdev->page] = pmbus_receive16(pmdev);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_FAN_RUN_TIME: /* R/W 16 */
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            s->fan_run_time[pmdev->page] = pmbus_receive16(pmdev);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_FAN_PWM_AVG: /* R/W 16 */
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            s->fan_pwm_avg[pmdev->page] = pmbus_receive16(pmdev);
>> +        }
>> +        break;
>> +
>> +    case MAX31785_MFR_FAN_PWM2RPM: /* R/W 64 */
>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>> +            s->fan_pwm2rpm[pmdev->page] = pmbus_receive64(pmdev);
>> +        }
>> +        break;
>> +
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "%s: writing to unsupported register: 0x%02x\n",
>> +                      __func__, pmdev->code);
>> +        break;
>> +    }
>> +    return 0;
>> +}
>> +
>> +static void max31785_exit_reset(Object *obj)
>> +{
>> +    PMBusDevice *pmdev = PMBUS_DEVICE(obj);
>> +    MAX31785State *s = MAX31785(obj);
>> +
>> +    pmdev->capability = MAX31785_DEFAULT_CAPABILITY;
>> +
>> +    for (int i = MAX31785_MIN_FAN_PAGE; i <= MAX31785_MAX_FAN_PAGE; i++) {
>> +        pmdev->pages[i].vout_mode = MAX31785_DEFAULT_VOUT_MODE;
>> +        pmdev->pages[i].fan_command_1 = MAX31785_DEFAULT_FAN_COMMAND_1;
>> +        pmdev->pages[i].revision = MAX31785_DEFAULT_PMBUS_REVISION;
>> +        pmdev->pages[i].fan_config_1_2 = MAX31785_DEFAULT_FAN_CONFIG_1_2(0);
>> +        pmdev->pages[i].read_fan_speed_1 = MAX31785_DEFAULT_FAN_SPEED;
>> +        pmdev->pages[i].status_fans_1_2 = MAX31785_DEFAULT_FAN_STATUS;
>> +    }
>> +
>> +    for (int i = MAX31785_MIN_TEMP_PAGE; i <= MAX31785_MAX_TEMP_PAGE; i++) {
>> +        pmdev->pages[i].vout_mode = MAX31785_DEFAULT_VOUT_MODE;
>> +        pmdev->pages[i].revision = MAX31785_DEFAULT_PMBUS_REVISION;
>> +        pmdev->pages[i].ot_fault_limit = MAX31785_DEFAULT_OT_FAULT_LIMIT;
>> +        pmdev->pages[i].ot_warn_limit = MAX31785_DEFAULT_OT_WARN_LIMIT;
>> +    }
>> +
>> +    for (int i = MAX31785_MIN_ADC_VOLTAGE_PAGE;
>> +         i <= MAX31785_MAX_ADC_VOLTAGE_PAGE;
>> +         i++) {
>> +        pmdev->pages[i].vout_mode = MAX31785_DEFAULT_VOUT_MODE;
>> +        pmdev->pages[i].revision = MAX31785_DEFAULT_PMBUS_REVISION;
>> +        pmdev->pages[i].vout_scale_monitor =
>> +        MAX31785_DEFAULT_VOUT_SCALE_MONITOR;
>> +        pmdev->pages[i].vout_ov_fault_limit =
>> +        MAX31785_DEFAULT_OV_FAULT_LIMIT;
>> +        pmdev->pages[i].vout_ov_warn_limit =
>> +        MAX31785_DEFAULT_OV_WARN_LIMIT;
>> +    }
>> +
>> +    s->mfr_location = MAX31785_DEFAULT_TEXT;
>> +    s->mfr_date = MAX31785_DEFAULT_TEXT;
>> +    s->mfr_serial = MAX31785_DEFAULT_TEXT;
>> +}
>> +
>> +static const VMStateDescription vmstate_max31785 = {
>> +    .name = TYPE_MAX31785,
>> +    .version_id = 0,
>> +    .minimum_version_id = 0,
>> +    .fields = (VMStateField[]){
>> +        VMSTATE_PMBUS_DEVICE(parent, MAX31785State),
>> +        VMSTATE_UINT16_ARRAY(mfr_mode, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(vout_peak, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(temperature_peak, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(vout_min, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT8_ARRAY(fault_response, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT32_ARRAY(time_count, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(temp_sensor_config, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(fan_config, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(read_fan_pwm, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(fan_fault_limit, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(fan_warn_limit, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(fan_run_time, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT16_ARRAY(fan_pwm_avg, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT64_ARRAY(fan_pwm2rpm, MAX31785State,
>> +        MAX31785_TOTAL_NUM_PAGES),
>> +        VMSTATE_UINT64(mfr_location, MAX31785State),
>> +        VMSTATE_UINT64(mfr_date, MAX31785State),
>> +        VMSTATE_UINT64(mfr_serial, MAX31785State),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static void max31785_init(Object *obj)
>> +{
>> +    PMBusDevice *pmdev = PMBUS_DEVICE(obj);
>> +
>> +    for (int i = MAX31785_MIN_FAN_PAGE; i <= MAX31785_MAX_FAN_PAGE; i++) {
>> +        pmbus_page_config(pmdev, i, PB_HAS_VOUT_MODE);
>> +    }
>> +
>> +    for (int i = MAX31785_MIN_TEMP_PAGE; i <= MAX31785_MAX_TEMP_PAGE; i++) {
>> +        pmbus_page_config(pmdev, i, PB_HAS_VOUT_MODE | PB_HAS_TEMPERATURE);
>> +    }
>> +
>> +    for (int i = MAX31785_MIN_ADC_VOLTAGE_PAGE;
>> +        i <= MAX31785_MAX_ADC_VOLTAGE_PAGE;
>> +        i++) {
>> +        pmbus_page_config(pmdev, i, PB_HAS_VOUT_MODE | PB_HAS_VOUT |
>> +                                    PB_HAS_VOUT_RATING);
>> +    }
>> +}
>> +
>> +static void max31785_class_init(ObjectClass *klass, void *data)
>> +{
>> +    ResettableClass *rc = RESETTABLE_CLASS(klass);
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    PMBusDeviceClass *k = PMBUS_DEVICE_CLASS(klass);
>> +    dc->desc = "Maxim MAX31785 6-Channel Fan Controller";
>> +    dc->vmsd = &vmstate_max31785;
>> +    k->write_data = max31785_write_data;
>> +    k->receive_byte = max31785_read_byte;
>> +    k->device_num_pages = MAX31785_TOTAL_NUM_PAGES;
>> +    rc->phases.exit = max31785_exit_reset;
>> +}
>> +
>> +static const TypeInfo max31785_info = {
>> +    .name = TYPE_MAX31785,
>> +    .parent = TYPE_PMBUS_DEVICE,
>> +    .instance_size = sizeof(MAX31785State),
>> +    .instance_init = max31785_init,
>> +    .class_init = max31785_class_init,
>> +};
>> +
>> +static void max31785_register_types(void)
>> +{
>> +    type_register_static(&max31785_info);
>> +}
>> +
>> +type_init(max31785_register_types)
>> diff --git a/hw/sensor/meson.build b/hw/sensor/meson.build
>> index 12b6992bc845..9e9be602c349 100644
>> --- a/hw/sensor/meson.build
>> +++ b/hw/sensor/meson.build
>> @@ -6,3 +6,4 @@ softmmu_ss.add(when: 'CONFIG_ADM1272', if_true: files('adm1272.c'))
>>   softmmu_ss.add(when: 'CONFIG_MAX34451', if_true: files('max34451.c'))
>>   softmmu_ss.add(when: 'CONFIG_LSM303DLHC_MAG', if_true: files('lsm303dlhc_mag.c'))
>>   softmmu_ss.add(when: 'CONFIG_ISL_PMBUS_VR', if_true: files('isl_pmbus_vr.c'))
>> +softmmu_ss.add(when: 'CONFIG_MAX31785', if_true: files('max31785.c'))
>> --
>> 2.25.1
>>

Re: [PATCH 6/9] hw/sensor: add Maxim MAX31785 device

[Jae Hyun Yoo]

Hello Cedric,

On 6/22/2022 10:40 PM, Cédric Le Goater wrote:
> On 6/23/22 07:17, Joel Stanley wrote:
>> On Wed, 22 Jun 2022 at 17:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> 
>> wrote:
>>>
>>> From: Maheswara Kurapati <quic_mkurapat@quicinc.com>
>>>
>>> MAX31785 is a PMBus compliant 6-Channel fan controller. It supports 6 
>>> fan
>>> channels, 11 temperature sensors, and 6-Channel ADC to measure the 
>>> remote
>>> voltages. Datasheet can be found here:
>>> https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
>>>
>>> This initial version of the driver has skeleton and support for the
>>> fan channels. Requests for temperature sensors, and ADC Channels the
>>> are serviced with the default values as per the datasheet.  No 
>>> additional
>>> instrumentation is done.  NV Log feature is not supported.
>>>
>>> Signed-off-by: Maheswara Kurapati <quic_mkurapat@quicinc.com>
>>
>> An excellent contribution. Thanks Maheswara and Jae.
>>
>> Reviewed-by: Joel Stanley <joel@jms.id.au>
> 
> We could also update the models of these machines :
> 
>      aspeed-bmc-ibm-rainier.dts:    max: max31785@52 {
>      aspeed-bmc-ibm-rainier.dts:        compatible = "maxim,max31785a";
>      aspeed-bmc-opp-tacoma.dts:    max31785@52 {
>      aspeed-bmc-opp-tacoma.dts:        compatible = "maxim,max31785a";
>      aspeed-bmc-opp-witherspoon.dts:    max31785@52 {
>      aspeed-bmc-opp-witherspoon.dts:        compatible = "maxim,max31785a";
> 
> 
> For the merge, I would prefer to have separate patches. One adding the
> sensor model and others updating the machines.

I'll split the machine updating part out from this patch in v2.

Thanks,
Jae

> 
> Thanks,
> C.
> 
>>
>>
>>> ---
>>>   hw/arm/Kconfig        |   1 +
>>>   hw/arm/aspeed.c       |   6 +-
>>>   hw/sensor/Kconfig     |   4 +
>>>   hw/sensor/max31785.c  | 580 ++++++++++++++++++++++++++++++++++++++++++
>>>   hw/sensor/meson.build |   1 +
>>>   5 files changed, 590 insertions(+), 2 deletions(-)
>>>   create mode 100644 hw/sensor/max31785.c
>>>
>>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>>> index 219262a8da36..77ef0fa967b2 100644
>>> --- a/hw/arm/Kconfig
>>> +++ b/hw/arm/Kconfig
>>> @@ -408,6 +408,7 @@ config NPCM7XX
>>>       select SSI
>>>       select UNIMP
>>>       select PCA954X
>>> +    select MAX31785
>>>
>>>   config FSL_IMX25
>>>       bool
>>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>>> index 0e6edd2be4fa..85630b497793 100644
>>> --- a/hw/arm/aspeed.c
>>> +++ b/hw/arm/aspeed.c
>>> @@ -619,7 +619,6 @@ static void 
>>> witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
>>>       LEDState *led;
>>>
>>>       /* Bus 3: TODO bmp280@77 */
>>> -    /* Bus 3: TODO max31785@52 */
>>>       dev = DEVICE(i2c_slave_new(TYPE_PCA9552, 0x60));
>>>       qdev_prop_set_string(dev, "description", "pca1");
>>>       i2c_slave_realize_and_unref(I2C_SLAVE(dev),
>>> @@ -635,6 +634,8 @@ static void 
>>> witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
>>>                                 qdev_get_gpio_in(DEVICE(led), 0));
>>>       }
>>>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3), 
>>> "dps310", 0x76);
>>> +    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 3), 
>>> "max31785",
>>> +                            0x52);
>>>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), 
>>> "tmp423", 0x4c);
>>>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 5), 
>>> "tmp423", 0x4c);
>>>
>>> @@ -779,13 +780,14 @@ static void 
>>> rainier_bmc_i2c_init(AspeedMachineState *bmc)
>>>       create_pca9552(soc, 7, 0x31);
>>>       create_pca9552(soc, 7, 0x32);
>>>       create_pca9552(soc, 7, 0x33);
>>> -    /* Bus 7: TODO max31785@52 */
>>>       create_pca9552(soc, 7, 0x60);
>>>       create_pca9552(soc, 7, 0x61);
>>>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7), 
>>> "dps310", 0x76);
>>>       /* Bus 7: TODO si7021-a20@20 */
>>>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7), 
>>> TYPE_TMP105,
>>>                        0x48);
>>> +    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7), 
>>> "max31785",
>>> +                     0x52);
>>>       aspeed_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 7), 0x50, 64 * 
>>> KiB);
>>>       aspeed_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 7), 0x51, 64 * 
>>> KiB);
>>>
>>> diff --git a/hw/sensor/Kconfig b/hw/sensor/Kconfig
>>> index df392e786904..e03bd09b50e8 100644
>>> --- a/hw/sensor/Kconfig
>>> +++ b/hw/sensor/Kconfig
>>> @@ -34,3 +34,7 @@ config LSM303DLHC_MAG
>>>   config ISL_PMBUS_VR
>>>       bool
>>>       depends on PMBUS
>>> +
>>> +config MAX31785
>>> +    bool
>>> +    depends on PMBUS
>>> diff --git a/hw/sensor/max31785.c b/hw/sensor/max31785.c
>>> new file mode 100644
>>> index 000000000000..11bf9977b6fd
>>> --- /dev/null
>>> +++ b/hw/sensor/max31785.c
>>> @@ -0,0 +1,580 @@
>>> +/*
>>> + * Maxim MAX31785 PMBus 6-Channel Fan Controller
>>> + *
>>> + * Datasheet:
>>> + * https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
>>> + *
>>> + * Copyright(c) 2022 Qualcomm Innovative Center, Inc. All rights 
>>> reserved.
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "hw/i2c/pmbus_device.h"
>>> +#include "hw/irq.h"
>>> +#include "migration/vmstate.h"
>>> +#include "qapi/error.h"
>>> +#include "qapi/visitor.h"
>>> +#include "qemu/log.h"
>>> +#include "qemu/module.h"
>>> +
>>> +#define TYPE_MAX31785   "max31785"
>>> +#define MAX31785(obj) OBJECT_CHECK(MAX31785State, (obj), TYPE_MAX31785)
>>> +
>>> +/*
>>> + * MAX31785 mfr specific PMBus commands
>>> + */
>>> +#define MAX31785_MFR_MODE               0xD1
>>> +#define MAX31785_MFR_PSEN_CONFIG        0xD2
>>> +#define MAX31785_MFR_VOUT_PEAK          0xD4
>>> +#define MAX31785_MFR_TEMPERATURE_PEAK   0xD6
>>> +#define MAX31785_MFR_VOUT_MIN           0xD7
>>> +#define MAX31785_MFR_FAULT_RESPONSE     0xD9
>>> +#define MAX31785_MFR_NV_FAULT_LOG       0xDC
>>> +#define MAX31785_MFR_TIME_COUNT         0xDD
>>> +#define MAX31785_MFR_TEMP_SENSOR_CONFIG 0xF0
>>> +#define MAX31785_MFR_FAN_CONFIG         0xF1
>>> +#define MAX31785_MFR_FAN_LUT            0xF2
>>> +#define MAX31785_MFR_READ_FAN_PWM       0xF3
>>> +#define MAX31785_MFR_FAN_FAULT_LIMIT    0xF5
>>> +#define MAX31785_MFR_FAN_WARN_LIMIT     0xF6
>>> +#define MAX31785_MFR_FAN_RUN_TIME       0xF7
>>> +#define MAX31785_MFR_FAN_PWM_AVG        0xF8
>>> +#define MAX31785_MFR_FAN_PWM2RPM        0xF9
>>> +
>>> +/*
>>> + * defaults as per the data sheet
>>> + */
>>> +#define MAX31785_DEFAULT_CAPABILITY         0x10
>>> +#define MAX31785_DEFAULT_VOUT_MODE          0x40
>>> +#define MAX31785_DEFAULT_VOUT_SCALE_MONITOR 0x7FFF
>>> +#define MAX31785_DEFAULT_FAN_COMMAND_1      0x7FFF
>>> +#define MAX31785_DEFAULT_OV_FAULT_LIMIT     0x7FFF
>>> +#define MAX31785_DEFAULT_OV_WARN_LIMIT      0x7FFF
>>> +#define MAX31785_DEFAULT_OT_FAULT_LIMIT     0x7FFF
>>> +#define MAX31785_DEFAULT_OT_WARN_LIMIT      0x7FFF
>>> +#define MAX31785_DEFAULT_PMBUS_REVISION     0x11
>>> +#define MAX31785_DEFAULT_MFR_ID             0x4D
>>> +#define MAX31785_DEFAULT_MFR_MODEL          0x53
>>> +#define MAX31785_DEFAULT_MFR_REVISION       0x3030
>>> +#define MAX31785A_DEFAULT_MFR_REVISION      0x3040
>>> +#define MAX31785B_DEFAULT_MFR_REVISION      0x3061
>>> +#define MAX31785B_DEFAULT_MFR_TEMPERATURE_PEAK   0x8000
>>> +#define MAX31785B_DEFAULT_MFR_VOUT_MIN      0x7FFF
>>> +#define MAX31785_DEFAULT_TEXT               0x3130313031303130
>>> +
>>> +/* MAX31785 pages */
>>> +#define MAX31785_TOTAL_NUM_PAGES    23
>>> +#define MAX31785_FAN_PAGES          6
>>> +#define MAX31785_MIN_FAN_PAGE       0
>>> +#define MAX31785_MAX_FAN_PAGE       5
>>> +#define MAX31785_MIN_TEMP_PAGE      6
>>> +#define MAX31785_MAX_TEMP_PAGE      16
>>> +#define MAX31785_MIN_ADC_VOLTAGE_PAGE   17
>>> +#define MAX31785_MAX_ADC_VOLTAGE_PAGE   22
>>> +
>>> +/* FAN_CONFIG_1_2 */
>>> +#define MAX31785_MFR_FAN_CONFIG         0xF1
>>> +#define MAX31785_FAN_CONFIG_ENABLE         BIT(7)
>>> +#define MAX31785_FAN_CONFIG_RPM_PWM        BIT(6)
>>> +#define MAX31785_FAN_CONFIG_PULSE(pulse)   (pulse << 4)
>>> +#define MAX31785_DEFAULT_FAN_CONFIG_1_2(pulse) 
>>> (MAX31785_FAN_CONFIG_ENABLE |\
>>> +MAX31785_FAN_CONFIG_PULSE(pulse))
>>> +#define MAX31785_DEFAULT_MFR_FAN_CONFIG     0x0000
>>> +
>>> +/* fan speed in RPM */
>>> +#define MAX31785_DEFAULT_FAN_SPEED          0x7fff
>>> +#define MAX31785_DEFAULT_FAN_STATUS         0x00
>>> +
>>> +#define MAX31785_DEFAULT_FAN_MAX_PWM        0x2710
>>> +
>>> +/*
>>> + * MAX31785State:
>>> + * @code: The command code received
>>> + * @page: Each page corresponds to a device monitored by the Max 31785
>>> + * The page register determines the available commands depending on 
>>> device
>>> +___________________________________________________________________________ 
>>>
>>> +|   0   |  Fan Connected to 
>>> PWM0                                            |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|   1   |  Fan Connected to 
>>> PWM1                                            |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|   2   |  Fan Connected to 
>>> PWM2                                            |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|   3   |  Fan Connected to 
>>> PWM3                                            |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|   4   |  Fan Connected to 
>>> PWM4                                            |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|   5   |  Fan Connected to 
>>> PWM5                                            |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|   6   |  Remote Thermal Diode Connected to ADC 
>>> 0                          |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|   7   |  Remote Thermal Diode Connected to ADC 
>>> 1                          |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|   8   |  Remote Thermal Diode Connected to ADC 
>>> 2                          |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|   9   |  Remote Thermal Diode Connected to ADC 
>>> 3                          |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  10   |  Remote Thermal Diode Connected to ADC 
>>> 4                          |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  11   |  Remote Thermal Diode Connected to ADC 
>>> 5                          |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  12   |  Internal Temperature 
>>> Sensor                                      |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  13   |  Remote I2C Temperature Sensor with Address 
>>> 0                     |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  14   |  Remote I2C Temperature Sensor with Address 
>>> 1                     |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  15   |  Remote I2C Temperature Sensor with Address 
>>> 2                     |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  16   |  Remote I2C Temperature Sensor with Address 
>>> 3                     |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  17   |  Remote I2C Temperature Sensor with Address 
>>> 4                     |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  17   |  Remote Voltage Connected to 
>>> ADC0                                 |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  18   |  Remote Voltage Connected to 
>>> ADC1                                 |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  19   |  Remote Voltage Connected to 
>>> ADC2                                 |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  20   |  Remote Voltage Connected to 
>>> ADC3                                 |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  21   |  Remote Voltage Connected to 
>>> ADC4                                 |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  22   |  Remote Voltage Connected to 
>>> ADC5                                 |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|23-254 |  
>>> Reserved                                                         |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +|  255  |  Applies to all 
>>> pages                                             |
>>> +|_______|___________________________________________________________________| 
>>>
>>> +*/
>>> +
>>> +/*
>>> + * Place holder to save the max31785 mfr specific registers
>>> + */
>>> +typedef struct MAX31785State {
>>> +    PMBusDevice parent;
>>> +    uint16_t mfr_mode[MAX31785_TOTAL_NUM_PAGES];
>>> +    uint16_t vout_peak[MAX31785_TOTAL_NUM_PAGES];
>>> +    uint16_t temperature_peak[MAX31785_TOTAL_NUM_PAGES];
>>> +    uint16_t vout_min[MAX31785_TOTAL_NUM_PAGES];
>>> +    uint8_t  fault_response[MAX31785_TOTAL_NUM_PAGES];
>>> +    uint32_t time_count[MAX31785_TOTAL_NUM_PAGES];
>>> +    uint16_t temp_sensor_config[MAX31785_TOTAL_NUM_PAGES];
>>> +    uint16_t fan_config[MAX31785_TOTAL_NUM_PAGES];
>>> +    uint16_t read_fan_pwm[MAX31785_TOTAL_NUM_PAGES];
>>> +    uint16_t fan_fault_limit[MAX31785_TOTAL_NUM_PAGES];
>>> +    uint16_t fan_warn_limit[MAX31785_TOTAL_NUM_PAGES];
>>> +    uint16_t fan_run_time[MAX31785_TOTAL_NUM_PAGES];
>>> +    uint16_t fan_pwm_avg[MAX31785_TOTAL_NUM_PAGES];
>>> +    uint64_t fan_pwm2rpm[MAX31785_TOTAL_NUM_PAGES];
>>> +    uint64_t mfr_location;
>>> +    uint64_t mfr_date;
>>> +    uint64_t mfr_serial;
>>> +    uint16_t mfr_revision;
>>> +} MAX31785State;
>>> +
>>> +static uint8_t max31785_read_byte(PMBusDevice *pmdev)
>>> +{
>>> +    MAX31785State *s = MAX31785(pmdev);
>>> +    switch (pmdev->code) {
>>> +
>>> +    case PMBUS_FAN_CONFIG_1_2:
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            pmbus_send8(pmdev, 
>>> pmdev->pages[pmdev->page].fan_config_1_2);
>>> +        }
>>> +        break;
>>> +
>>> +    case PMBUS_FAN_COMMAND_1:
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            pmbus_send16(pmdev, 
>>> pmdev->pages[pmdev->page].fan_command_1);
>>> +        }
>>> +        break;
>>> +
>>> +    case PMBUS_READ_FAN_SPEED_1:
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            pmbus_send16(pmdev, 
>>> pmdev->pages[pmdev->page].read_fan_speed_1);
>>> +        }
>>> +        break;
>>> +
>>> +    case PMBUS_STATUS_FANS_1_2:
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            pmbus_send16(pmdev, 
>>> pmdev->pages[pmdev->page].status_fans_1_2);
>>> +        }
>>> +        break;
>>> +
>>> +    case PMBUS_MFR_REVISION:
>>> +        pmbus_send16(pmdev, MAX31785_DEFAULT_MFR_REVISION);
>>> +        break;
>>> +
>>> +    case PMBUS_MFR_ID:
>>> +        pmbus_send8(pmdev, 0x4d); /* Maxim */
>>> +        break;
>>> +
>>> +    case PMBUS_MFR_MODEL:
>>> +        pmbus_send8(pmdev, 0x53);
>>> +        break;
>>> +
>>> +    case PMBUS_MFR_LOCATION:
>>> +        pmbus_send64(pmdev, s->mfr_location);
>>> +        break;
>>> +
>>> +    case PMBUS_MFR_DATE:
>>> +        pmbus_send64(pmdev, s->mfr_date);
>>> +        break;
>>> +
>>> +    case PMBUS_MFR_SERIAL:
>>> +        pmbus_send64(pmdev, s->mfr_serial);
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_MODE:
>>> +        pmbus_send16(pmdev, s->mfr_mode[pmdev->page]);
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_VOUT_PEAK:
>>> +        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
>>> +            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
>>> +            pmbus_send16(pmdev, s->vout_peak[pmdev->page]);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_TEMPERATURE_PEAK:
>>> +        if ((pmdev->page >= MAX31785_MIN_TEMP_PAGE) &&
>>> +            (pmdev->page <= MAX31785_MAX_TEMP_PAGE)) {
>>> +            pmbus_send16(pmdev, s->temperature_peak[pmdev->page]);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_VOUT_MIN:
>>> +        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
>>> +            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
>>> +            pmbus_send16(pmdev, s->vout_min[pmdev->page]);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_FAULT_RESPONSE:
>>> +        pmbus_send8(pmdev, s->fault_response[pmdev->page]);
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_TIME_COUNT: /* R/W 32 */
>>> +        pmbus_send32(pmdev, s->time_count[pmdev->page]);
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_TEMP_SENSOR_CONFIG: /* R/W 16 */
>>> +        if ((pmdev->page >= MAX31785_MIN_TEMP_PAGE) &&
>>> +            (pmdev->page <= MAX31785_MAX_TEMP_PAGE)) {
>>> +            pmbus_send16(pmdev, s->temp_sensor_config[pmdev->page]);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_FAN_CONFIG: /* R/W 16 */
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            pmbus_send16(pmdev, s->fan_config[pmdev->page]);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_READ_FAN_PWM: /* R/W 16 */
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            pmbus_send16(pmdev, s->read_fan_pwm[pmdev->page]);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_FAN_FAULT_LIMIT: /* R/W 16 */
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            pmbus_send16(pmdev, s->fan_fault_limit[pmdev->page]);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_FAN_WARN_LIMIT: /* R/W 16 */
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            pmbus_send16(pmdev, s->fan_warn_limit[pmdev->page]);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_FAN_RUN_TIME: /* R/W 16 */
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            pmbus_send16(pmdev, s->fan_run_time[pmdev->page]);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_FAN_PWM_AVG: /* R/W 16 */
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            pmbus_send16(pmdev, s->fan_pwm_avg[pmdev->page]);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_FAN_PWM2RPM: /* R/W 64 */
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            pmbus_send64(pmdev, s->fan_pwm2rpm[pmdev->page]);
>>> +        }
>>> +        break;
>>> +
>>> +    default:
>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>> +        "%s: reading from unsupported register: 0x%02x\n",
>>> +        __func__, pmdev->code);
>>> +        break;
>>> +    }
>>> +    return 0xFF;
>>> +}
>>> +
>>> +static int max31785_write_data(PMBusDevice *pmdev, const uint8_t *buf,
>>> +                                uint8_t len)
>>> +{
>>> +    MAX31785State *s = MAX31785(pmdev);
>>> +    if (len == 0) {
>>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: writing empty data\n", 
>>> __func__);
>>> +        return -1;
>>> +    }
>>> +
>>> +    pmdev->code = buf[0]; /* PMBus command code */
>>> +
>>> +    if (len == 1) {
>>> +        return 0;
>>> +    }
>>> +
>>> +    /* Exclude command code from buffer */
>>> +    buf++;
>>> +    len--;
>>> +
>>> +    switch (pmdev->code) {
>>> +
>>> +    case PMBUS_FAN_CONFIG_1_2:
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            pmdev->pages[pmdev->page].fan_config_1_2 = 
>>> pmbus_receive8(pmdev);
>>> +        }
>>> +        break;
>>> +
>>> +    case PMBUS_FAN_COMMAND_1:
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            pmdev->pages[pmdev->page].fan_command_1 = 
>>> pmbus_receive16(pmdev);
>>> +            pmdev->pages[pmdev->page].read_fan_speed_1 =
>>> +                ((MAX31785_DEFAULT_FAN_SPEED / 
>>> MAX31785_DEFAULT_FAN_MAX_PWM) *
>>> +                pmdev->pages[pmdev->page].fan_command_1);
>>> +        }
>>> +        break;
>>> +
>>> +    case PMBUS_MFR_LOCATION: /* R/W 64 */
>>> +        s->mfr_location = pmbus_receive64(pmdev);
>>> +        break;
>>> +
>>> +    case PMBUS_MFR_DATE: /* R/W 64 */
>>> +        s->mfr_date = pmbus_receive64(pmdev);
>>> +        break;
>>> +
>>> +    case PMBUS_MFR_SERIAL: /* R/W 64 */
>>> +        s->mfr_serial = pmbus_receive64(pmdev);
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_MODE: /* R/W word */
>>> +        s->mfr_mode[pmdev->page] = pmbus_receive16(pmdev);
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_VOUT_PEAK: /* R/W word */
>>> +        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
>>> +            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
>>> +            s->vout_peak[pmdev->page] = pmbus_receive16(pmdev);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_TEMPERATURE_PEAK: /* R/W word */
>>> +        if ((pmdev->page >= 6) && (pmdev->page <= 16)) {
>>> +            s->temperature_peak[pmdev->page] = pmbus_receive16(pmdev);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_VOUT_MIN: /* R/W word */
>>> +        if ((pmdev->page >= MAX31785_MIN_ADC_VOLTAGE_PAGE) &&
>>> +            (pmdev->page <= MAX31785_MAX_ADC_VOLTAGE_PAGE)) {
>>> +            s->vout_min[pmdev->page] = pmbus_receive16(pmdev);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_FAULT_RESPONSE: /* R/W 8 */
>>> +        s->fault_response[pmdev->page] = pmbus_receive8(pmdev);
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_TIME_COUNT: /* R/W 32 */
>>> +        s->time_count[pmdev->page] = pmbus_receive32(pmdev);
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_TEMP_SENSOR_CONFIG: /* R/W 16 */
>>> +        if ((pmdev->page >= MAX31785_MIN_TEMP_PAGE) &&
>>> +            (pmdev->page <= MAX31785_MAX_TEMP_PAGE)) {
>>> +            s->temp_sensor_config[pmdev->page] = 
>>> pmbus_receive16(pmdev);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_FAN_CONFIG: /* R/W 16 */
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            s->fan_config[pmdev->page] = pmbus_receive16(pmdev);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_FAN_FAULT_LIMIT: /* R/W 16 */
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            s->fan_fault_limit[pmdev->page] = pmbus_receive16(pmdev);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_FAN_WARN_LIMIT: /* R/W 16 */
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            s->fan_warn_limit[pmdev->page] = pmbus_receive16(pmdev);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_FAN_RUN_TIME: /* R/W 16 */
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            s->fan_run_time[pmdev->page] = pmbus_receive16(pmdev);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_FAN_PWM_AVG: /* R/W 16 */
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            s->fan_pwm_avg[pmdev->page] = pmbus_receive16(pmdev);
>>> +        }
>>> +        break;
>>> +
>>> +    case MAX31785_MFR_FAN_PWM2RPM: /* R/W 64 */
>>> +        if (pmdev->page <= MAX31785_MAX_FAN_PAGE) {
>>> +            s->fan_pwm2rpm[pmdev->page] = pmbus_receive64(pmdev);
>>> +        }
>>> +        break;
>>> +
>>> +    default:
>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>> +                      "%s: writing to unsupported register: 0x%02x\n",
>>> +                      __func__, pmdev->code);
>>> +        break;
>>> +    }
>>> +    return 0;
>>> +}
>>> +
>>> +static void max31785_exit_reset(Object *obj)
>>> +{
>>> +    PMBusDevice *pmdev = PMBUS_DEVICE(obj);
>>> +    MAX31785State *s = MAX31785(obj);
>>> +
>>> +    pmdev->capability = MAX31785_DEFAULT_CAPABILITY;
>>> +
>>> +    for (int i = MAX31785_MIN_FAN_PAGE; i <= MAX31785_MAX_FAN_PAGE; 
>>> i++) {
>>> +        pmdev->pages[i].vout_mode = MAX31785_DEFAULT_VOUT_MODE;
>>> +        pmdev->pages[i].fan_command_1 = MAX31785_DEFAULT_FAN_COMMAND_1;
>>> +        pmdev->pages[i].revision = MAX31785_DEFAULT_PMBUS_REVISION;
>>> +        pmdev->pages[i].fan_config_1_2 = 
>>> MAX31785_DEFAULT_FAN_CONFIG_1_2(0);
>>> +        pmdev->pages[i].read_fan_speed_1 = MAX31785_DEFAULT_FAN_SPEED;
>>> +        pmdev->pages[i].status_fans_1_2 = MAX31785_DEFAULT_FAN_STATUS;
>>> +    }
>>> +
>>> +    for (int i = MAX31785_MIN_TEMP_PAGE; i <= 
>>> MAX31785_MAX_TEMP_PAGE; i++) {
>>> +        pmdev->pages[i].vout_mode = MAX31785_DEFAULT_VOUT_MODE;
>>> +        pmdev->pages[i].revision = MAX31785_DEFAULT_PMBUS_REVISION;
>>> +        pmdev->pages[i].ot_fault_limit = 
>>> MAX31785_DEFAULT_OT_FAULT_LIMIT;
>>> +        pmdev->pages[i].ot_warn_limit = MAX31785_DEFAULT_OT_WARN_LIMIT;
>>> +    }
>>> +
>>> +    for (int i = MAX31785_MIN_ADC_VOLTAGE_PAGE;
>>> +         i <= MAX31785_MAX_ADC_VOLTAGE_PAGE;
>>> +         i++) {
>>> +        pmdev->pages[i].vout_mode = MAX31785_DEFAULT_VOUT_MODE;
>>> +        pmdev->pages[i].revision = MAX31785_DEFAULT_PMBUS_REVISION;
>>> +        pmdev->pages[i].vout_scale_monitor =
>>> +        MAX31785_DEFAULT_VOUT_SCALE_MONITOR;
>>> +        pmdev->pages[i].vout_ov_fault_limit =
>>> +        MAX31785_DEFAULT_OV_FAULT_LIMIT;
>>> +        pmdev->pages[i].vout_ov_warn_limit =
>>> +        MAX31785_DEFAULT_OV_WARN_LIMIT;
>>> +    }
>>> +
>>> +    s->mfr_location = MAX31785_DEFAULT_TEXT;
>>> +    s->mfr_date = MAX31785_DEFAULT_TEXT;
>>> +    s->mfr_serial = MAX31785_DEFAULT_TEXT;
>>> +}
>>> +
>>> +static const VMStateDescription vmstate_max31785 = {
>>> +    .name = TYPE_MAX31785,
>>> +    .version_id = 0,
>>> +    .minimum_version_id = 0,
>>> +    .fields = (VMStateField[]){
>>> +        VMSTATE_PMBUS_DEVICE(parent, MAX31785State),
>>> +        VMSTATE_UINT16_ARRAY(mfr_mode, MAX31785State,
>>> +        MAX31785_TOTAL_NUM_PAGES),
>>> +        VMSTATE_UINT16_ARRAY(vout_peak, MAX31785State,
>>> +        MAX31785_TOTAL_NUM_PAGES),
>>> +        VMSTATE_UINT16_ARRAY(temperature_peak, MAX31785State,
>>> +        MAX31785_TOTAL_NUM_PAGES),
>>> +        VMSTATE_UINT16_ARRAY(vout_min, MAX31785State,
>>> +        MAX31785_TOTAL_NUM_PAGES),
>>> +        VMSTATE_UINT8_ARRAY(fault_response, MAX31785State,
>>> +        MAX31785_TOTAL_NUM_PAGES),
>>> +        VMSTATE_UINT32_ARRAY(time_count, MAX31785State,
>>> +        MAX31785_TOTAL_NUM_PAGES),
>>> +        VMSTATE_UINT16_ARRAY(temp_sensor_config, MAX31785State,
>>> +        MAX31785_TOTAL_NUM_PAGES),
>>> +        VMSTATE_UINT16_ARRAY(fan_config, MAX31785State,
>>> +        MAX31785_TOTAL_NUM_PAGES),
>>> +        VMSTATE_UINT16_ARRAY(read_fan_pwm, MAX31785State,
>>> +        MAX31785_TOTAL_NUM_PAGES),
>>> +        VMSTATE_UINT16_ARRAY(fan_fault_limit, MAX31785State,
>>> +        MAX31785_TOTAL_NUM_PAGES),
>>> +        VMSTATE_UINT16_ARRAY(fan_warn_limit, MAX31785State,
>>> +        MAX31785_TOTAL_NUM_PAGES),
>>> +        VMSTATE_UINT16_ARRAY(fan_run_time, MAX31785State,
>>> +        MAX31785_TOTAL_NUM_PAGES),
>>> +        VMSTATE_UINT16_ARRAY(fan_pwm_avg, MAX31785State,
>>> +        MAX31785_TOTAL_NUM_PAGES),
>>> +        VMSTATE_UINT64_ARRAY(fan_pwm2rpm, MAX31785State,
>>> +        MAX31785_TOTAL_NUM_PAGES),
>>> +        VMSTATE_UINT64(mfr_location, MAX31785State),
>>> +        VMSTATE_UINT64(mfr_date, MAX31785State),
>>> +        VMSTATE_UINT64(mfr_serial, MAX31785State),
>>> +        VMSTATE_END_OF_LIST()
>>> +    }
>>> +};
>>> +
>>> +static void max31785_init(Object *obj)
>>> +{
>>> +    PMBusDevice *pmdev = PMBUS_DEVICE(obj);
>>> +
>>> +    for (int i = MAX31785_MIN_FAN_PAGE; i <= MAX31785_MAX_FAN_PAGE; 
>>> i++) {
>>> +        pmbus_page_config(pmdev, i, PB_HAS_VOUT_MODE);
>>> +    }
>>> +
>>> +    for (int i = MAX31785_MIN_TEMP_PAGE; i <= 
>>> MAX31785_MAX_TEMP_PAGE; i++) {
>>> +        pmbus_page_config(pmdev, i, PB_HAS_VOUT_MODE | 
>>> PB_HAS_TEMPERATURE);
>>> +    }
>>> +
>>> +    for (int i = MAX31785_MIN_ADC_VOLTAGE_PAGE;
>>> +        i <= MAX31785_MAX_ADC_VOLTAGE_PAGE;
>>> +        i++) {
>>> +        pmbus_page_config(pmdev, i, PB_HAS_VOUT_MODE | PB_HAS_VOUT |
>>> +                                    PB_HAS_VOUT_RATING);
>>> +    }
>>> +}
>>> +
>>> +static void max31785_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    ResettableClass *rc = RESETTABLE_CLASS(klass);
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +    PMBusDeviceClass *k = PMBUS_DEVICE_CLASS(klass);
>>> +    dc->desc = "Maxim MAX31785 6-Channel Fan Controller";
>>> +    dc->vmsd = &vmstate_max31785;
>>> +    k->write_data = max31785_write_data;
>>> +    k->receive_byte = max31785_read_byte;
>>> +    k->device_num_pages = MAX31785_TOTAL_NUM_PAGES;
>>> +    rc->phases.exit = max31785_exit_reset;
>>> +}
>>> +
>>> +static const TypeInfo max31785_info = {
>>> +    .name = TYPE_MAX31785,
>>> +    .parent = TYPE_PMBUS_DEVICE,
>>> +    .instance_size = sizeof(MAX31785State),
>>> +    .instance_init = max31785_init,
>>> +    .class_init = max31785_class_init,
>>> +};
>>> +
>>> +static void max31785_register_types(void)
>>> +{
>>> +    type_register_static(&max31785_info);
>>> +}
>>> +
>>> +type_init(max31785_register_types)
>>> diff --git a/hw/sensor/meson.build b/hw/sensor/meson.build
>>> index 12b6992bc845..9e9be602c349 100644
>>> --- a/hw/sensor/meson.build
>>> +++ b/hw/sensor/meson.build
>>> @@ -6,3 +6,4 @@ softmmu_ss.add(when: 'CONFIG_ADM1272', if_true: 
>>> files('adm1272.c'))
>>>   softmmu_ss.add(when: 'CONFIG_MAX34451', if_true: files('max34451.c'))
>>>   softmmu_ss.add(when: 'CONFIG_LSM303DLHC_MAG', if_true: 
>>> files('lsm303dlhc_mag.c'))
>>>   softmmu_ss.add(when: 'CONFIG_ISL_PMBUS_VR', if_true: 
>>> files('isl_pmbus_vr.c'))
>>> +softmmu_ss.add(when: 'CONFIG_MAX31785', if_true: files('max31785.c'))
>>> -- 
>>> 2.25.1
>>>
> 

[PATCH 7/9] hw/arm/aspeed: firework: Add MAX31785 Fan controllers

[Jae Hyun Yoo]

From: Maheswara Kurapati <quic_mkurapat@quicinc.com>

Firework has two MAX31785 Fan controllers at 0x52, and 0x54. Include them
in the model so that the Linux driver populates the sysfs interface.

Signed-off-by: Maheswara Kurapati <quic_mkurapat@quicinc.com>
---
 hw/arm/aspeed.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 85630b497793..08e5fc178a94 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1046,6 +1046,10 @@ static void qcom_dc_scm_firework_i2c_init(AspeedMachineState *bmc)
 
     /* I2C4 */
     qcom_firework_fru_init(aspeed_i2c_get_bus(&soc->i2c, 4), 0x50, 128 * 1024);
+
+    /* I2C-9 Fan Controller (MAX31785) */
+    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), "max31785", 0x52);
+    i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), "max31785", 0x54);
 }
 
 static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
-- 
2.25.1

[PATCH 8/9] hw/arm/aspeed: firework: Add Thermal Diodes

[Jae Hyun Yoo]

From: Maheswara Kurapati <quic_mkurapat@quicinc.com>

Add Thermal Diodes for Firework machine.

Signed-off-by: Maheswara Kurapati <quic_mkurapat@quicinc.com>
---
 hw/arm/aspeed.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 08e5fc178a94..526f3b651a9f 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1038,6 +1038,7 @@ static void qcom_firework_fru_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
 static void qcom_dc_scm_firework_i2c_init(AspeedMachineState *bmc)
 {
     AspeedSoCState *soc = &bmc->soc;
+    I2CSlave *mux;
 
     /* Create the generic DC-SCM hardware */
     qcom_dc_scm_bmc_i2c_init(bmc);
@@ -1047,6 +1048,15 @@ static void qcom_dc_scm_firework_i2c_init(AspeedMachineState *bmc)
     /* I2C4 */
     qcom_firework_fru_init(aspeed_i2c_get_bus(&soc->i2c, 4), 0x50, 128 * 1024);
 
+    /* I2C - 8 Thermal Diodes*/
+    mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8), "pca9548",
+                                  0x70);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 0), TYPE_LM75, 0x4C);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 1), TYPE_LM75, 0x4C);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 2), TYPE_TMP75, 0x48);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 3), TYPE_TMP75, 0x48);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 4), TYPE_TMP75, 0x48);
+
     /* I2C-9 Fan Controller (MAX31785) */
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), "max31785", 0x52);
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), "max31785", 0x54);
-- 
2.25.1

[PATCH 9/9] hw/arm/aspeed: firework: add I2C MUXes for VR channels

[Jae Hyun Yoo]

Add 2-level cascaded I2C MUXes for SOC VR channels into the Firework
machine.

Signed-off-by: Jae Hyun Yoo <quic_jaehyoo@quicinc.com>
---
 hw/arm/aspeed.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 526f3b651a9f..866a60cf7b4e 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1038,7 +1038,7 @@ static void qcom_firework_fru_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
 static void qcom_dc_scm_firework_i2c_init(AspeedMachineState *bmc)
 {
     AspeedSoCState *soc = &bmc->soc;
-    I2CSlave *mux;
+    I2CSlave *therm_mux, *cpuvr_mux;
 
     /* Create the generic DC-SCM hardware */
     qcom_dc_scm_bmc_i2c_init(bmc);
@@ -1048,16 +1048,24 @@ static void qcom_dc_scm_firework_i2c_init(AspeedMachineState *bmc)
     /* I2C4 */
     qcom_firework_fru_init(aspeed_i2c_get_bus(&soc->i2c, 4), 0x50, 128 * 1024);
 
-    /* I2C - 8 Thermal Diodes*/
-    mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8), "pca9548",
-                                  0x70);
-    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 0), TYPE_LM75, 0x4C);
-    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 1), TYPE_LM75, 0x4C);
-    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 2), TYPE_TMP75, 0x48);
-    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 3), TYPE_TMP75, 0x48);
-    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 4), TYPE_TMP75, 0x48);
-
-    /* I2C-9 Fan Controller (MAX31785) */
+    /* I2C7 CPUVR MUX */
+    cpuvr_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7),
+                                        "pca9546", 0x70);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(cpuvr_mux, 0), "pca9548", 0x72);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(cpuvr_mux, 1), "pca9548", 0x72);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(cpuvr_mux, 2), "pca9548", 0x72);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(cpuvr_mux, 3), "pca9548", 0x72);
+
+    /* I2C8 Thermal Diodes*/
+    therm_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8),
+                                        "pca9548", 0x70);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 0), TYPE_LM75, 0x4C);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 1), TYPE_LM75, 0x4C);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 2), TYPE_LM75, 0x48);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 3), TYPE_LM75, 0x48);
+    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 4), TYPE_LM75, 0x48);
+
+    /* I2C9 Fan Controller (MAX31785) */
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), "max31785", 0x52);
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), "max31785", 0x54);
 }
-- 
2.25.1

Re: [PATCH 9/9] hw/arm/aspeed: firework: add I2C MUXes for VR channels

[Joel Stanley]

On Wed, 22 Jun 2022 at 17:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
>
> Add 2-level cascaded I2C MUXes for SOC VR channels into the Firework
> machine.
>
> Signed-off-by: Jae Hyun Yoo <quic_jaehyoo@quicinc.com>
> ---
>  hw/arm/aspeed.c | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 526f3b651a9f..866a60cf7b4e 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -1038,7 +1038,7 @@ static void qcom_firework_fru_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
>  static void qcom_dc_scm_firework_i2c_init(AspeedMachineState *bmc)
>  {
>      AspeedSoCState *soc = &bmc->soc;
> -    I2CSlave *mux;
> +    I2CSlave *therm_mux, *cpuvr_mux;
>
>      /* Create the generic DC-SCM hardware */
>      qcom_dc_scm_bmc_i2c_init(bmc);
> @@ -1048,16 +1048,24 @@ static void qcom_dc_scm_firework_i2c_init(AspeedMachineState *bmc)
>      /* I2C4 */
>      qcom_firework_fru_init(aspeed_i2c_get_bus(&soc->i2c, 4), 0x50, 128 * 1024);
>
> -    /* I2C - 8 Thermal Diodes*/
> -    mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8), "pca9548",
> -                                  0x70);
> -    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 0), TYPE_LM75, 0x4C);
> -    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 1), TYPE_LM75, 0x4C);
> -    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 2), TYPE_TMP75, 0x48);
> -    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 3), TYPE_TMP75, 0x48);
> -    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 4), TYPE_TMP75, 0x48);
> -

You only just added this. If you modify the previous patch to call the
"mux" variable "therm_mux" then you don't need to modify it in this
patch.

or just squash them both together. I don't think there's much value in
having two separate patches.

> -    /* I2C-9 Fan Controller (MAX31785) */
> +    /* I2C7 CPUVR MUX */
> +    cpuvr_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7),
> +                                        "pca9546", 0x70);
> +    i2c_slave_create_simple(pca954x_i2c_get_bus(cpuvr_mux, 0), "pca9548", 0x72);
> +    i2c_slave_create_simple(pca954x_i2c_get_bus(cpuvr_mux, 1), "pca9548", 0x72);
> +    i2c_slave_create_simple(pca954x_i2c_get_bus(cpuvr_mux, 2), "pca9548", 0x72);
> +    i2c_slave_create_simple(pca954x_i2c_get_bus(cpuvr_mux, 3), "pca9548", 0x72);
> +
> +    /* I2C8 Thermal Diodes*/
> +    therm_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8),
> +                                        "pca9548", 0x70);
> +    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 0), TYPE_LM75, 0x4C);
> +    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 1), TYPE_LM75, 0x4C);
> +    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 2), TYPE_LM75, 0x48);
> +    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 3), TYPE_LM75, 0x48);
> +    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 4), TYPE_LM75, 0x48);
> +
> +    /* I2C9 Fan Controller (MAX31785) */
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), "max31785", 0x52);
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), "max31785", 0x54);
>  }
> --
> 2.25.1
>

Re: [PATCH 9/9] hw/arm/aspeed: firework: add I2C MUXes for VR channels

[Jae Hyun Yoo]

Hello Joel,

On 6/22/2022 10:27 PM, Joel Stanley wrote:
> On Wed, 22 Jun 2022 at 17:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
>>
>> Add 2-level cascaded I2C MUXes for SOC VR channels into the Firework
>> machine.
>>
>> Signed-off-by: Jae Hyun Yoo <quic_jaehyoo@quicinc.com>
>> ---
>>   hw/arm/aspeed.c | 30 +++++++++++++++++++-----------
>>   1 file changed, 19 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index 526f3b651a9f..866a60cf7b4e 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -1038,7 +1038,7 @@ static void qcom_firework_fru_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
>>   static void qcom_dc_scm_firework_i2c_init(AspeedMachineState *bmc)
>>   {
>>       AspeedSoCState *soc = &bmc->soc;
>> -    I2CSlave *mux;
>> +    I2CSlave *therm_mux, *cpuvr_mux;
>>
>>       /* Create the generic DC-SCM hardware */
>>       qcom_dc_scm_bmc_i2c_init(bmc);
>> @@ -1048,16 +1048,24 @@ static void qcom_dc_scm_firework_i2c_init(AspeedMachineState *bmc)
>>       /* I2C4 */
>>       qcom_firework_fru_init(aspeed_i2c_get_bus(&soc->i2c, 4), 0x50, 128 * 1024);
>>
>> -    /* I2C - 8 Thermal Diodes*/
>> -    mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8), "pca9548",
>> -                                  0x70);
>> -    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 0), TYPE_LM75, 0x4C);
>> -    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 1), TYPE_LM75, 0x4C);
>> -    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 2), TYPE_TMP75, 0x48);
>> -    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 3), TYPE_TMP75, 0x48);
>> -    i2c_slave_create_simple(pca954x_i2c_get_bus(mux, 4), TYPE_TMP75, 0x48);
>> -
> 
> You only just added this. If you modify the previous patch to call the
> "mux" variable "therm_mux" then you don't need to modify it in this
> patch.
> 
> or just squash them both together. I don't think there's much value in
> having two separate patches.

Okay. I'll fix the 8/9 patch to call the variable "therm_mux" instead.

Thanks,
Jae

>> -    /* I2C-9 Fan Controller (MAX31785) */
>> +    /* I2C7 CPUVR MUX */
>> +    cpuvr_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 7),
>> +                                        "pca9546", 0x70);
>> +    i2c_slave_create_simple(pca954x_i2c_get_bus(cpuvr_mux, 0), "pca9548", 0x72);
>> +    i2c_slave_create_simple(pca954x_i2c_get_bus(cpuvr_mux, 1), "pca9548", 0x72);
>> +    i2c_slave_create_simple(pca954x_i2c_get_bus(cpuvr_mux, 2), "pca9548", 0x72);
>> +    i2c_slave_create_simple(pca954x_i2c_get_bus(cpuvr_mux, 3), "pca9548", 0x72);
>> +
>> +    /* I2C8 Thermal Diodes*/
>> +    therm_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8),
>> +                                        "pca9548", 0x70);
>> +    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 0), TYPE_LM75, 0x4C);
>> +    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 1), TYPE_LM75, 0x4C);
>> +    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 2), TYPE_LM75, 0x48);
>> +    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 3), TYPE_LM75, 0x48);
>> +    i2c_slave_create_simple(pca954x_i2c_get_bus(therm_mux, 4), TYPE_LM75, 0x48);
>> +
>> +    /* I2C9 Fan Controller (MAX31785) */
>>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), "max31785", 0x52);
>>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 9), "max31785", 0x54);
>>   }
>> --
>> 2.25.1
>>

Re: [PATCH 0/9] Add Qualcomm BMC machines

[Joel Stanley]

On Wed, 22 Jun 2022 at 17:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
>
> Hello,
>
> I'm sending a series to add Qualcomm BMC machines that are equipped with
> Aspeed AST2600 SoC. Also, this series adds MAX31785 fan controller device
> emulation. Please help to review.

Thanks for the MAX31785 model, that's handy to have.

I'm all for more emulation and testing using Qemu models, but I wonder
if you need to add all three of your boards. They seem to be a
progression (evb-proto -> dc-scm -> firework). Could you get away with
just one or two of those?


>
> Thanks,
>
> Jae
>
> Graeme Gregory (2):
>   hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device
>   hw/arm/aspeed: add Qualcomm Firework machine and FRU device
>
> Jae Hyun Yoo (3):
>   hw/arm/aspeed: add support for the Qualcomm EVB proto board
>   hw/arm/aspeed: add support for the Qualcomm DC-SCM v1 board
>   hw/arm/aspeed: firework: add I2C MUXes for VR channels
>
> Maheswara Kurapati (4):
>   hw/i2c: pmbus: Page #255 is valid page for read requests.
>   hw/sensor: add Maxim MAX31785 device
>   hw/arm/aspeed: firework: Add MAX31785 Fan controllers
>   hw/arm/aspeed: firework: Add Thermal Diodes
>
>  hw/arm/Kconfig        |   1 +
>  hw/arm/aspeed.c       | 158 +++++++++++-
>  hw/i2c/pmbus_device.c |   1 -
>  hw/sensor/Kconfig     |   4 +
>  hw/sensor/max31785.c  | 580 ++++++++++++++++++++++++++++++++++++++++++
>  hw/sensor/meson.build |   1 +
>  6 files changed, 742 insertions(+), 3 deletions(-)
>  create mode 100644 hw/sensor/max31785.c
>
> --
> 2.25.1
>

Re: [PATCH 0/9] Add Qualcomm BMC machines

[Cédric Le Goater]

On 6/23/22 07:25, Joel Stanley wrote:
> On Wed, 22 Jun 2022 at 17:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
>>
>> Hello,
>>
>> I'm sending a series to add Qualcomm BMC machines that are equipped with
>> Aspeed AST2600 SoC. Also, this series adds MAX31785 fan controller device
>> emulation. Please help to review.
> 
> Thanks for the MAX31785 model, that's handy to have.
> 
> I'm all for more emulation and testing using Qemu models, but I wonder
> if you need to add all three of your boards. They seem to be a
> progression (evb-proto -> dc-scm -> firework). Could you get away with
> just one or two of those?

I am not sure the evb-proto-bmc is useful to upstream. The other two
are fine.

Thanks,

C.



> 
> 
>>
>> Thanks,
>>
>> Jae
>>
>> Graeme Gregory (2):
>>    hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device
>>    hw/arm/aspeed: add Qualcomm Firework machine and FRU device
>>
>> Jae Hyun Yoo (3):
>>    hw/arm/aspeed: add support for the Qualcomm EVB proto board
>>    hw/arm/aspeed: add support for the Qualcomm DC-SCM v1 board
>>    hw/arm/aspeed: firework: add I2C MUXes for VR channels
>>
>> Maheswara Kurapati (4):
>>    hw/i2c: pmbus: Page #255 is valid page for read requests.
>>    hw/sensor: add Maxim MAX31785 device
>>    hw/arm/aspeed: firework: Add MAX31785 Fan controllers
>>    hw/arm/aspeed: firework: Add Thermal Diodes
>>
>>   hw/arm/Kconfig        |   1 +
>>   hw/arm/aspeed.c       | 158 +++++++++++-
>>   hw/i2c/pmbus_device.c |   1 -
>>   hw/sensor/Kconfig     |   4 +
>>   hw/sensor/max31785.c  | 580 ++++++++++++++++++++++++++++++++++++++++++
>>   hw/sensor/meson.build |   1 +
>>   6 files changed, 742 insertions(+), 3 deletions(-)
>>   create mode 100644 hw/sensor/max31785.c
>>
>> --
>> 2.25.1
>>

Re: [PATCH 0/9] Add Qualcomm BMC machines

[Graeme Gregory]

On Thu, Jun 23, 2022 at 08:48:49AM +0200, Cédric Le Goater wrote:
> On 6/23/22 07:25, Joel Stanley wrote:
> > On Wed, 22 Jun 2022 at 17:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
> > > 
> > > Hello,
> > > 
> > > I'm sending a series to add Qualcomm BMC machines that are equipped with
> > > Aspeed AST2600 SoC. Also, this series adds MAX31785 fan controller device
> > > emulation. Please help to review.
> > 
> > Thanks for the MAX31785 model, that's handy to have.
> > 
> > I'm all for more emulation and testing using Qemu models, but I wonder
> > if you need to add all three of your boards. They seem to be a
> > progression (evb-proto -> dc-scm -> firework). Could you get away with
> > just one or two of those?
> 
> I am not sure the evb-proto-bmc is useful to upstream. The other two
> are fine.
> 
> Thanks,
> 

I am happy with dropping the evb-proto-bmc machine. We used that
internally before actual hardware was available.

Graeme

> C.
> 
> 
> 
> > 
> > 
> > > 
> > > Thanks,
> > > 
> > > Jae
> > > 
> > > Graeme Gregory (2):
> > >    hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device
> > >    hw/arm/aspeed: add Qualcomm Firework machine and FRU device
> > > 
> > > Jae Hyun Yoo (3):
> > >    hw/arm/aspeed: add support for the Qualcomm EVB proto board
> > >    hw/arm/aspeed: add support for the Qualcomm DC-SCM v1 board
> > >    hw/arm/aspeed: firework: add I2C MUXes for VR channels
> > > 
> > > Maheswara Kurapati (4):
> > >    hw/i2c: pmbus: Page #255 is valid page for read requests.
> > >    hw/sensor: add Maxim MAX31785 device
> > >    hw/arm/aspeed: firework: Add MAX31785 Fan controllers
> > >    hw/arm/aspeed: firework: Add Thermal Diodes
> > > 
> > >   hw/arm/Kconfig        |   1 +
> > >   hw/arm/aspeed.c       | 158 +++++++++++-
> > >   hw/i2c/pmbus_device.c |   1 -
> > >   hw/sensor/Kconfig     |   4 +
> > >   hw/sensor/max31785.c  | 580 ++++++++++++++++++++++++++++++++++++++++++
> > >   hw/sensor/meson.build |   1 +
> > >   6 files changed, 742 insertions(+), 3 deletions(-)
> > >   create mode 100644 hw/sensor/max31785.c
> > > 
> > > --
> > > 2.25.1
> > > 
> 

Re: [PATCH 0/9] Add Qualcomm BMC machines

[Jae Hyun Yoo]

On 6/23/2022 3:24 AM, Graeme Gregory wrote:
> On Thu, Jun 23, 2022 at 08:48:49AM +0200, Cédric Le Goater wrote:
>> On 6/23/22 07:25, Joel Stanley wrote:
>>> On Wed, 22 Jun 2022 at 17:29, Jae Hyun Yoo <quic_jaehyoo@quicinc.com> wrote:
>>>>
>>>> Hello,
>>>>
>>>> I'm sending a series to add Qualcomm BMC machines that are equipped with
>>>> Aspeed AST2600 SoC. Also, this series adds MAX31785 fan controller device
>>>> emulation. Please help to review.
>>>
>>> Thanks for the MAX31785 model, that's handy to have.
>>>
>>> I'm all for more emulation and testing using Qemu models, but I wonder
>>> if you need to add all three of your boards. They seem to be a
>>> progression (evb-proto -> dc-scm -> firework). Could you get away with
>>> just one or two of those?
>>
>> I am not sure the evb-proto-bmc is useful to upstream. The other two
>> are fine.
>>
>> Thanks,
>>
> 
> I am happy with dropping the evb-proto-bmc machine. We used that
> internally before actual hardware was available.

Okay. I'll drop the evb-proto-bmc in v2.

Thanks,
Jae

> Graeme
> 
>> C.
>>
>>
>>
>>>
>>>
>>>>
>>>> Thanks,
>>>>
>>>> Jae
>>>>
>>>> Graeme Gregory (2):
>>>>     hw/arm/aspeed: qcom-dc-scm-v1: add block backed FRU device
>>>>     hw/arm/aspeed: add Qualcomm Firework machine and FRU device
>>>>
>>>> Jae Hyun Yoo (3):
>>>>     hw/arm/aspeed: add support for the Qualcomm EVB proto board
>>>>     hw/arm/aspeed: add support for the Qualcomm DC-SCM v1 board
>>>>     hw/arm/aspeed: firework: add I2C MUXes for VR channels
>>>>
>>>> Maheswara Kurapati (4):
>>>>     hw/i2c: pmbus: Page #255 is valid page for read requests.
>>>>     hw/sensor: add Maxim MAX31785 device
>>>>     hw/arm/aspeed: firework: Add MAX31785 Fan controllers
>>>>     hw/arm/aspeed: firework: Add Thermal Diodes
>>>>
>>>>    hw/arm/Kconfig        |   1 +
>>>>    hw/arm/aspeed.c       | 158 +++++++++++-
>>>>    hw/i2c/pmbus_device.c |   1 -
>>>>    hw/sensor/Kconfig     |   4 +
>>>>    hw/sensor/max31785.c  | 580 ++++++++++++++++++++++++++++++++++++++++++
>>>>    hw/sensor/meson.build |   1 +
>>>>    6 files changed, 742 insertions(+), 3 deletions(-)
>>>>    create mode 100644 hw/sensor/max31785.c
>>>>
>>>> --
>>>> 2.25.1
>>>>
>>