diff mbox series

[1/3] hw/sensor: add MAX31790 fan controller

Message ID 20220112002515.3991540-1-titusr@google.com (mailing list archive)
State New, archived
Headers show
Series [1/3] hw/sensor: add MAX31790 fan controller | expand

Commit Message

Titus Rwantare Jan. 12, 2022, 12:25 a.m. UTC
Signed-off-by: Titus Rwantare <titusr@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
---
 MAINTAINERS                           |   8 +-
 hw/arm/Kconfig                        |   1 +
 hw/sensor/Kconfig                     |   4 +
 hw/sensor/max31790_fan_ctrl.c         | 454 ++++++++++++++++++++++++++
 hw/sensor/meson.build                 |   1 +
 include/hw/sensor/max31790_fan_ctrl.h |  93 ++++++
 6 files changed, 560 insertions(+), 1 deletion(-)
 create mode 100644 hw/sensor/max31790_fan_ctrl.c
 create mode 100644 include/hw/sensor/max31790_fan_ctrl.h

Comments

Peter Maydell Jan. 27, 2022, 6:59 p.m. UTC | #1
On Wed, 12 Jan 2022 at 00:25, Titus Rwantare <titusr@google.com> wrote:
>
> Signed-off-by: Titus Rwantare <titusr@google.com>
> Reviewed-by: Hao Wu <wuhaotsh@google.com>

Hi; thanks for this patchset. For future revisions, could you
make sure you send a cover-letter for patchsets that have
more than one patch? It helps to keep the automated tooling
from getting confused.

> ---
>  MAINTAINERS                           |   8 +-
>  hw/arm/Kconfig                        |   1 +
>  hw/sensor/Kconfig                     |   4 +
>  hw/sensor/max31790_fan_ctrl.c         | 454 ++++++++++++++++++++++++++
>  hw/sensor/meson.build                 |   1 +
>  include/hw/sensor/max31790_fan_ctrl.h |  93 ++++++
>  6 files changed, 560 insertions(+), 1 deletion(-)
>  create mode 100644 hw/sensor/max31790_fan_ctrl.c
>  create mode 100644 include/hw/sensor/max31790_fan_ctrl.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c98a61caee..0791b6be42 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2304,6 +2304,12 @@ F: hw/timer/mips_gictimer.c
>  F: include/hw/intc/mips_gic.h
>  F: include/hw/timer/mips_gictimer.h
>
> +MAX31790 Fan controller
> +M: Titus Rwantare <titusr@google.com>
> +S: Maintained
> +F: hw/sensor/max31790_fan_ctrl.c
> +F: include/hw/sensor/max31790_fan_ctrl.h
> +
>  Subsystems
>  ----------
>  Overall Audio backends
> @@ -2798,7 +2804,7 @@ R: Paolo Bonzini <pbonzini@redhat.com>
>  R: Bandan Das <bsd@redhat.com>
>  R: Stefan Hajnoczi <stefanha@redhat.com>
>  R: Thomas Huth <thuth@redhat.com>
> -R: Darren Kenny <darren.kenny@oracle.com>
> +R: Darren Kenny <darren.kenny@oracle.com>

Why did this line get changed ? It looks like maybe there was
a trailing space that got deleted. If you want to do that kind
of cleanup it's best done as a separate patch.

>  R: Qiuhao Li <Qiuhao.Li@outlook.com>
>  S: Maintained
>  F: tests/qtest/fuzz/
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index e652590943..00bfbaf1c4 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -393,6 +393,7 @@ config NPCM7XX
>      select SMBUS
>      select AT24C  # EEPROM
>      select MAX34451
> +    select MAX31790
>      select PL310  # cache controller
>      select PMBUS
>      select SERIAL
> diff --git a/hw/sensor/Kconfig b/hw/sensor/Kconfig
> index 9c8a049b06..54d269d642 100644
> --- a/hw/sensor/Kconfig
> +++ b/hw/sensor/Kconfig
> @@ -21,3 +21,7 @@ config ADM1272
>  config MAX34451
>      bool
>      depends on I2C
> +
> +config MAX31790
> +    bool
> +    depends on I2C
> diff --git a/hw/sensor/max31790_fan_ctrl.c b/hw/sensor/max31790_fan_ctrl.c
> new file mode 100644
> index 0000000000..b5334c1130
> --- /dev/null
> +++ b/hw/sensor/max31790_fan_ctrl.c
> @@ -0,0 +1,454 @@
> +/*
> + * MAX31790 Fan controller
> + *
> + * Independently control 6 fans, up to 12 tachometer inputs,
> + * controlled through i2c
> + *
> + * This device model has read/write support for:
> + * - 9-bit pwm through i2c and qom/qmp
> + * - 11-bit tach_count through i2c
> + * - RPM through qom/qmp
> + *
> + * Copyright 2021 Google LLC
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/sensor/max31790_fan_ctrl.h"
> +#include "hw/irq.h"
> +#include "hw/qdev-properties.h"
> +#include "migration/vmstate.h"
> +#include "qapi/visitor.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +
> +static uint16_t max31790_get_sr(uint8_t fan_dynamics)
> +{
> +    uint16_t sr = 1 << ((fan_dynamics >> 5) & 0b111);
> +    return sr > 16 ? 32 : sr;
> +}
> +
> +static void max31790_place_bits(uint16_t *dest, uint16_t byte, uint8_t offset)
> +{
> +    uint16_t val = *dest;
> +    val &= ~(0x00FF << offset);
> +    val |= byte << offset;
> +    *dest = val;
> +}
> +
> +/*
> + * calculating fan speed
> + *  f_TOSC/4 is the clock, 8192Hz
> + *  NP = tachometer pulses per revolution (usually 2)
> + *  SR = number of periods(pulses) over which the clock ticks are counted
> + *  TACH Count = SR x 8192 x 60 / (NP x RPM)
> + *  RPM = SR x 8192 x 60 / (NP x TACH count)
> + *
> + *  RPM mode - desired tach count is written to TACH Target Count
> + *  PWM mode - desired duty cycle is written to PWMOUT Target Duty reg
> + */
> +static void max31790_calculate_tach_count(MAX31790State *ms, uint8_t id)
> +{
> +    uint32_t rpm;
> +    uint32_t sr = max31790_get_sr(ms->fan_dynamics[id]);
> +    ms->pwm_duty_cycle[id] = ms->pwmout[id] >> 7;
> +    rpm = (ms->max_rpm[id] * ms->pwm_duty_cycle[id]) / 0x1FF;
> +
> +    if (rpm) {
> +        ms->tach_count[id] = (sr * MAX31790_CLK_FREQ * 60) /
> +                             (MAX31790_PULSES_PER_REV * rpm);

This is all being done with 32-bit arithmetic. Is it definitely
never possible for it to overflow ?


> +    } else {
> +        ms->tach_count[id] = 0;
> +    }
> +
> +}
> +

> +        if ((ms->command + 1) % 8) {
> +            ms->command++;
> +        } else {
> +            ms->command -= 7;
> +        }

You could write this:
   ms->command = (ms->command & ~7) | ((ms->command + 1) & 7);

Maybe that's clearer, maybe not -- your choice whether to change it or not.


> +/* assumes that the fans have the same speed range (SR) */
> +static void max31790_get_rpm(Object *obj, Visitor *v, const char *name,
> +                             void *opaque, Error **errp)

For get/set functions for object properties, can you include
a comment somewhere that clearly describes what the property
is and what units are being used, please? (We really ought to
document this but we don't currently have a clear place for that;
for the moment at least we can write it down in the source code...)

> +{
> +    MAX31790State *ms = MAX31790(obj);
> +    uint16_t tach_count = *(uint16_t *)opaque;
> +    uint32_t sr = max31790_get_sr(ms->fan_dynamics[0]);
> +    uint16_t rpm;
> +
> +    max31790_update_tach_count(ms);
> +    tach_count >>= MAX31790_TACH_SHAMT;
> +
> +    if (tach_count) {
> +        rpm = (sr * MAX31790_CLK_FREQ * 60) /
> +              (MAX31790_PULSES_PER_REV * tach_count);

More 32-bit integer arithmetic, same "can this overflow"
question. It may be worth abstracting the "rpm-to-tach-count"
and "tach-count-to-rpm" calculations out into functions
(I think you used at least one of them open-coded earlier.)

> +    }
> +
> +    visit_type_uint16(v, name, &rpm, errp);
> +}

> +static void max31790_init(Object *obj)
> +{
> +    MAX31790State *ms = MAX31790(obj);
> +
> +    ms->global_config = MAX31790_GLOBAL_CONFIG_DEFAULT;
> +    ms->pwm_freq = MAX31790_PWM_FREQ_DEFAULT;
> +    ms->failed_fan_opts_seq_strt = MAX31790_FAILED_FAN_OPTS_SEQ_STRT_DEFAULT;
> +
> +    for (int i = 0; i < MAX31790_NUM_FANS; i++) {
> +        ms->max_rpm[i] = MAX31790_MAX_RPM_DEFAULT;
> +        ms->fan_config[i] = 0;
> +        ms->fan_dynamics[i] = MAX31790_FAN_DYNAMICS_DEFAULT;
> +        ms->pwmout[i] = MAX31790_PWMOUT_DEFAULT;
> +        ms->target_count[i] = MAX31790_TARGET_COUNT_DEFAULT;
> +    }

A lot of this looks like it's initialization of guest-writeable
data. For guest-writeable data:
 * initialize it in a reset method, not in device init
 * you need to migrate it, which means it needs a line in
   a VMStateDescription

> +    max31790_update_tach_count(ms);
> +    for (int i = 0; i < MAX31790_NUM_FANS; i++) {
> +        object_property_add(obj, "target_rpm[*]", "uint16",
> +                            max31790_get_rpm,
> +                            max31790_set_rpm, NULL, &ms->target_count[i]);
> +
> +        /* 9-bit PWM on this device */
> +        object_property_add(obj, "pwm[*]", "uint16",
> +                            max31790_get,
> +                            max31790_set, NULL, &ms->pwmout[i]);
> +
> +        /* used to calculate rpm for a given pwm duty cycle */
> +        object_property_add(obj, "max_rpm[*]", "uint16",
> +                            max31790_get,
> +                            max31790_set, NULL, &ms->max_rpm[i]);
> +    }
> +}
> +
> +static void max31790_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
> +
> +    dc->desc = "Maxim MAX31790 fan controller";
> +
> +    k->event = max31790_event;
> +    k->recv = max31790_recv;
> +    k->send = max31790_send;
> +}
> +
> +static const TypeInfo max31790_info = {
> +    .name = TYPE_MAX31790,
> +    .parent = TYPE_I2C_SLAVE,
> +    .instance_size = sizeof(MAX31790State),
> +    .instance_init = max31790_init,
> +    .class_init = max31790_class_init,
> +};
> +
> +static void max31790_register_types(void)
> +{
> +    type_register_static(&max31790_info);
> +}
> +
> +type_init(max31790_register_types)
> diff --git a/hw/sensor/meson.build b/hw/sensor/meson.build
> index 059c4ca935..4ce68cfc89 100644
> --- a/hw/sensor/meson.build
> +++ b/hw/sensor/meson.build
> @@ -4,3 +4,4 @@ softmmu_ss.add(when: 'CONFIG_DPS310', if_true: files('dps310.c'))
>  softmmu_ss.add(when: 'CONFIG_EMC141X', if_true: files('emc141x.c'))
>  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_MAX31790', if_true: files('max31790_fan_ctrl.c'))
> diff --git a/include/hw/sensor/max31790_fan_ctrl.h b/include/hw/sensor/max31790_fan_ctrl.h
> new file mode 100644
> index 0000000000..74ff7bb5a0
> --- /dev/null
> +++ b/include/hw/sensor/max31790_fan_ctrl.h
> @@ -0,0 +1,93 @@
> +/*
> + * Max 31790 Fan controller
> + *
> + * Independently control 6 fans, up to 12 tachometer inputs,
> + * controlled through i2c
> + *
> + * Copyright 2021 Google LLC
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef MAX31790_FAN_CTRL_H
> +#define MAX31790_FAN_CTRL_H
> +
> +#include "hw/i2c/i2c.h"
> +#include "qom/object.h"
> +
> +#define MAX31790_NUM_FANS 6
> +#define MAX31790_NUM_TACHS 12
> +
> +typedef struct MAX31790State {
> +    I2CSlave parent;
> +
> +    /* Registers */
> +    uint8_t global_config;
> +    uint8_t pwm_freq;
> +    uint8_t fan_config[MAX31790_NUM_FANS];
> +    uint8_t fan_dynamics[MAX31790_NUM_FANS];
> +    uint8_t fan_fault_status_2;
> +    uint8_t fan_fault_status_1;
> +    uint8_t fan_fault_mask_2;
> +    uint8_t fan_fault_mask_1;
> +    uint8_t failed_fan_opts_seq_strt;
> +    uint16_t tach_count[MAX31790_NUM_TACHS];
> +    uint16_t pwm_duty_cycle[MAX31790_NUM_FANS];
> +    uint16_t pwmout[MAX31790_NUM_FANS];
> +    uint16_t target_count[MAX31790_NUM_FANS];
> +    uint8_t window[MAX31790_NUM_FANS];
> +
> +    /* config */
> +    uint16_t max_rpm[MAX31790_NUM_FANS];
> +
> +    /* i2c transaction state */
> +    uint8_t command;
> +    bool i2c_cmd_event;
> +    bool cmd_is_new;
> +} MAX31790State;
> +
> +#define TYPE_MAX31790 "max31790"
> +#define MAX31790(obj) OBJECT_CHECK(MAX31790State, (obj), TYPE_MAX31790)

Prefer the OBJECT_DECLARE_SIMPLE_TYPE() macro rather than
hand-defining a cast macro. (It also provides the typedef for
you, so you only need to define the struct, without the "typedef"
keyword.)

> +
> +#define MAX31790_REG_GLOBAL_CONFIG             0x00                 /* R/W */
> +#define MAX31790_REG_PWM_FREQ                  0x01                 /* R/W */
> +#define MAX31790_REG_FAN_CONFIG(ch)           (0x02 + (ch))         /* R/W */
> +#define MAX31790_REG_FAN_DYNAMICS(ch)         (0x08 + (ch))         /* R/W */
> +#define MAX31790_REG_FAN_FAULT_STATUS_2        0x10                 /* R/W */
> +#define MAX31790_REG_FAN_FAULT_STATUS_1        0x11                 /* R/W */
> +#define MAX31790_REG_FAN_FAULT_MASK_2          0x12                 /* R/W */
> +#define MAX31790_REG_FAN_FAULT_MASK_1          0x13                 /* R/W */
> +#define MAX31790_REG_FAILED_FAN_OPTS_SEQ_STRT  0x14                 /* R/W */
> +#define MAX31790_REG_TACH_COUNT_MSB(ch)       (0x18 + (ch) * 2)     /* R */
> +#define MAX31790_REG_TACH_COUNT_LSB(ch)       (0x19 + (ch) * 2)     /* R */
> +#define MAX31790_REG_PWM_DUTY_CYCLE_MSB(ch)   (0x30 + (ch) * 2)     /* R */
> +#define MAX31790_REG_PWM_DUTY_CYCLE_LSB(ch)   (0x31 + (ch) * 2)     /* R */
> +#define MAX31790_REG_PWMOUT_MSB(ch)           (0x40 + (ch) * 2)     /* R/W */
> +#define MAX31790_REG_PWMOUT_LSB(ch)           (0x41 + (ch) * 2)     /* R/W */
> +#define MAX31790_REG_TARGET_COUNT_MSB(ch)     (0x50 + (ch) * 2)     /* R/W */
> +#define MAX31790_REG_TARGET_COUNT_LSB(ch)     (0x51 + (ch) * 2)     /* R/W */
> +#define MAX31790_REG_WINDOW(ch)               (0x60 + (ch))         /* R/W */
> +
> +#define MAX31790_GLOBAL_CONFIG_DEFAULT                0x20
> +#define MAX31790_PWM_FREQ_DEFAULT                     0x44 /* 125Hz */
> +#define MAX31790_FAN_DYNAMICS_DEFAULT                 0x4C
> +#define MAX31790_FAILED_FAN_OPTS_SEQ_STRT_DEFAULT     0x45
> +#define MAX31790_PWMOUT_DEFAULT                       (128 << 7) /* 25% */
> +#define MAX31790_TARGET_COUNT_DEFAULT                 0x3D60
> +
> +/* Fan Config register bits */
> +#define MAX31790_FAN_CFG_RPM_MODE             BIT(7)
> +#define MAX31790_FAN_CFG_MONITOR_ONLY         BIT(4)
> +#define MAX31790_FAN_CFG_TACH_INPUT_EN        BIT(3)
> +#define MAX31790_FAN_CFG_TACH_INPUT           BIT(0)
> +
> +/* Tachometer calculation constants */
> +#define MAX31790_PULSES_PER_REV             2
> +#define MAX31790_SR_DEFAULT                 4
> +#define MAX31790_CLK_FREQ                   8192
> +#define MAX31790_MAX_RPM_DEFAULT            16500
> +
> +/* reg alignment amounts */
> +#define MAX31790_PWM_SHAMT                  7
> +#define MAX31790_TACH_SHAMT                 5
> +#endif

Consider whether some of these constants would be better in the .c file.

thanks
-- PMM
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index c98a61caee..0791b6be42 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2304,6 +2304,12 @@  F: hw/timer/mips_gictimer.c
 F: include/hw/intc/mips_gic.h
 F: include/hw/timer/mips_gictimer.h
 
+MAX31790 Fan controller
+M: Titus Rwantare <titusr@google.com>
+S: Maintained
+F: hw/sensor/max31790_fan_ctrl.c
+F: include/hw/sensor/max31790_fan_ctrl.h
+
 Subsystems
 ----------
 Overall Audio backends
@@ -2798,7 +2804,7 @@  R: Paolo Bonzini <pbonzini@redhat.com>
 R: Bandan Das <bsd@redhat.com>
 R: Stefan Hajnoczi <stefanha@redhat.com>
 R: Thomas Huth <thuth@redhat.com>
-R: Darren Kenny <darren.kenny@oracle.com> 
+R: Darren Kenny <darren.kenny@oracle.com>
 R: Qiuhao Li <Qiuhao.Li@outlook.com>
 S: Maintained
 F: tests/qtest/fuzz/
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index e652590943..00bfbaf1c4 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -393,6 +393,7 @@  config NPCM7XX
     select SMBUS
     select AT24C  # EEPROM
     select MAX34451
+    select MAX31790
     select PL310  # cache controller
     select PMBUS
     select SERIAL
diff --git a/hw/sensor/Kconfig b/hw/sensor/Kconfig
index 9c8a049b06..54d269d642 100644
--- a/hw/sensor/Kconfig
+++ b/hw/sensor/Kconfig
@@ -21,3 +21,7 @@  config ADM1272
 config MAX34451
     bool
     depends on I2C
+
+config MAX31790
+    bool
+    depends on I2C
diff --git a/hw/sensor/max31790_fan_ctrl.c b/hw/sensor/max31790_fan_ctrl.c
new file mode 100644
index 0000000000..b5334c1130
--- /dev/null
+++ b/hw/sensor/max31790_fan_ctrl.c
@@ -0,0 +1,454 @@ 
+/*
+ * MAX31790 Fan controller
+ *
+ * Independently control 6 fans, up to 12 tachometer inputs,
+ * controlled through i2c
+ *
+ * This device model has read/write support for:
+ * - 9-bit pwm through i2c and qom/qmp
+ * - 11-bit tach_count through i2c
+ * - RPM through qom/qmp
+ *
+ * Copyright 2021 Google LLC
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sensor/max31790_fan_ctrl.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "qapi/visitor.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+
+static uint16_t max31790_get_sr(uint8_t fan_dynamics)
+{
+    uint16_t sr = 1 << ((fan_dynamics >> 5) & 0b111);
+    return sr > 16 ? 32 : sr;
+}
+
+static void max31790_place_bits(uint16_t *dest, uint16_t byte, uint8_t offset)
+{
+    uint16_t val = *dest;
+    val &= ~(0x00FF << offset);
+    val |= byte << offset;
+    *dest = val;
+}
+
+/*
+ * calculating fan speed
+ *  f_TOSC/4 is the clock, 8192Hz
+ *  NP = tachometer pulses per revolution (usually 2)
+ *  SR = number of periods(pulses) over which the clock ticks are counted
+ *  TACH Count = SR x 8192 x 60 / (NP x RPM)
+ *  RPM = SR x 8192 x 60 / (NP x TACH count)
+ *
+ *  RPM mode - desired tach count is written to TACH Target Count
+ *  PWM mode - desired duty cycle is written to PWMOUT Target Duty reg
+ */
+static void max31790_calculate_tach_count(MAX31790State *ms, uint8_t id)
+{
+    uint32_t rpm;
+    uint32_t sr = max31790_get_sr(ms->fan_dynamics[id]);
+    ms->pwm_duty_cycle[id] = ms->pwmout[id] >> 7;
+    rpm = (ms->max_rpm[id] * ms->pwm_duty_cycle[id]) / 0x1FF;
+
+    if (rpm) {
+        ms->tach_count[id] = (sr * MAX31790_CLK_FREQ * 60) /
+                             (MAX31790_PULSES_PER_REV * rpm);
+    } else {
+        ms->tach_count[id] = 0;
+    }
+
+}
+
+static void max31790_update_tach_count(MAX31790State *ms)
+{
+    for (int i = 0; i < MAX31790_NUM_FANS; i++) {
+        if (ms->fan_config[i] &
+            (MAX31790_FAN_CFG_RPM_MODE | MAX31790_FAN_CFG_TACH_INPUT_EN)) {
+                ms->tach_count[i] = ms->target_count[i] >> 5;
+        } else { /* PWM mode */
+            max31790_calculate_tach_count(ms, i);
+        }
+    }
+}
+
+/* consecutive reads can increment the address up to 0xFF then wrap to 0 */
+/* slave to master */
+static uint8_t max31790_recv(I2CSlave *i2c)
+{
+    MAX31790State *ms = MAX31790(i2c);
+    uint8_t data, index, rem;
+
+    max31790_update_tach_count(ms);
+
+    if (ms->cmd_is_new) {
+        ms->cmd_is_new = false;
+    } else {
+        ms->command++;
+    }
+
+    switch (ms->command) {
+    case MAX31790_REG_GLOBAL_CONFIG:
+        data = ms->global_config;
+        break;
+
+    case MAX31790_REG_PWM_FREQ:
+        data = ms->pwm_freq;
+        break;
+
+    case MAX31790_REG_FAN_CONFIG(0) ...
+         MAX31790_REG_FAN_CONFIG(MAX31790_NUM_FANS - 1):
+        data = ms->fan_config[ms->command - MAX31790_REG_FAN_CONFIG(0)];
+        break;
+
+    case MAX31790_REG_FAN_DYNAMICS(0) ...
+         MAX31790_REG_FAN_DYNAMICS(MAX31790_NUM_FANS - 1):
+        data = ms->fan_dynamics[ms->command - MAX31790_REG_FAN_DYNAMICS(0)];
+        break;
+
+    case MAX31790_REG_FAN_FAULT_STATUS_2:
+        data = ms->fan_fault_status_2;
+        break;
+
+    case MAX31790_REG_FAN_FAULT_STATUS_1:
+        data = ms->fan_fault_status_1;
+        break;
+
+    case MAX31790_REG_FAN_FAULT_MASK_2:
+        data = ms->fan_fault_mask_2;
+        break;
+
+    case MAX31790_REG_FAN_FAULT_MASK_1:
+        data = ms->fan_fault_mask_1;
+        break;
+
+    case MAX31790_REG_FAILED_FAN_OPTS_SEQ_STRT:
+        data = ms->failed_fan_opts_seq_strt;
+        break;
+
+    case MAX31790_REG_TACH_COUNT_MSB(0) ...
+         MAX31790_REG_TACH_COUNT_LSB(MAX31790_NUM_TACHS - 1):
+        index = (ms->command - MAX31790_REG_TACH_COUNT_MSB(0)) / 2;
+        rem = (ms->command - MAX31790_REG_TACH_COUNT_MSB(0)) % 2;
+        if (rem) {
+            data = ms->tach_count[index] << 5;
+        } else {
+            data = ms->tach_count[index] >> 3;
+        }
+        break;
+
+    /*
+     * PWM_DUTY_CYCLE is meant to be the current duty cycle while
+     * PWMOUT is the requested duty cycle
+     */
+    case MAX31790_REG_PWM_DUTY_CYCLE_MSB(0) ...
+         MAX31790_REG_PWM_DUTY_CYCLE_LSB(MAX31790_NUM_FANS - 1):
+        index = (ms->command - MAX31790_REG_PWM_DUTY_CYCLE_MSB(0)) / 2;
+        rem = (ms->command - MAX31790_REG_PWM_DUTY_CYCLE_MSB(0)) % 2;
+
+        if (rem) {
+            data = ms->pwm_duty_cycle[index] << 7;
+        } else {
+            data = ms->pwm_duty_cycle[index] >> 1;
+        }
+        break;
+
+    case MAX31790_REG_PWMOUT_MSB(0) ...
+         MAX31790_REG_PWMOUT_LSB(MAX31790_NUM_FANS - 1):
+        index = (ms->command - MAX31790_REG_PWMOUT_MSB(0)) / 2;
+        rem = (ms->command - MAX31790_REG_PWMOUT_MSB(0)) % 2;
+        if (rem) {
+            data = ms->pwmout[index];
+        } else {
+            data = ms->pwmout[index] >> 8;
+        }
+        break;
+
+    case MAX31790_REG_TARGET_COUNT_MSB(0) ...
+         MAX31790_REG_TARGET_COUNT_LSB(MAX31790_NUM_FANS - 1):
+        index = (ms->command - MAX31790_REG_TARGET_COUNT_MSB(0)) / 2;
+        rem = (ms->command - MAX31790_REG_TARGET_COUNT_MSB(0)) % 2;
+        if (rem) {
+            data = ms->target_count[index];
+        } else {
+            data = ms->target_count[index] >> 8;
+        }
+        break;
+
+    case MAX31790_REG_WINDOW(0) ...
+         MAX31790_REG_WINDOW(MAX31790_NUM_FANS - 1):
+        data = ms->window[ms->command - MAX31790_REG_WINDOW(0)];
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                "%s: reading from unsupported register 0x%02x",
+                __func__, ms->command);
+        data = 0xFF;
+        break;
+    }
+
+    return data;
+}
+
+/*
+ * The device’s control registers are organized in rows of 8 bytes.
+ * The I2C master can read or write individual bytes, or can read or write
+ * multiple bytes. When writing consecutive bytes, all writes are to the same
+ * row. When the final byte in the row is reached, the next byte written is the
+ * row’s first byte.
+ * For example, a write that starts with 02h (Fan 1 Configuration) can write to
+ * 02h, 03h, 04h, 05h, 06h, and 07h. If writes continue, the next byte written
+ * is 00h, and so on.
+ */
+/* master to slave */
+static int max31790_send(I2CSlave *i2c, uint8_t data)
+{
+    MAX31790State *ms = MAX31790(i2c);
+    uint8_t index, rem;
+
+    if (ms->i2c_cmd_event) {
+        ms->command = data;
+        ms->i2c_cmd_event = false;
+        ms->cmd_is_new = true;
+        return 0;
+    }
+
+    if (ms->cmd_is_new) {
+        ms->cmd_is_new = false;
+    } else {
+        if ((ms->command + 1) % 8) {
+            ms->command++;
+        } else {
+            ms->command -= 7;
+        }
+    }
+
+    switch (ms->command) {
+    case MAX31790_REG_GLOBAL_CONFIG:
+        ms->global_config = data;
+        break;
+
+    case MAX31790_REG_PWM_FREQ:
+        ms->pwm_freq = data;
+        break;
+
+    case MAX31790_REG_FAN_CONFIG(0) ...
+         MAX31790_REG_FAN_CONFIG(MAX31790_NUM_FANS - 1):
+        ms->fan_config[ms->command - MAX31790_REG_FAN_CONFIG(0)] = data;
+        break;
+
+    case MAX31790_REG_FAN_DYNAMICS(0) ...
+         MAX31790_REG_FAN_DYNAMICS(MAX31790_NUM_FANS - 1):
+        ms->fan_dynamics[ms->command - MAX31790_REG_FAN_DYNAMICS(0)] = data;
+        break;
+
+    case MAX31790_REG_FAN_FAULT_STATUS_2:
+        ms->fan_fault_status_2 = data;
+        break;
+
+    case MAX31790_REG_FAN_FAULT_STATUS_1:
+        ms->fan_fault_status_1 = data;
+        break;
+
+    case MAX31790_REG_FAN_FAULT_MASK_2:
+        ms->fan_fault_mask_2 = data;
+        break;
+
+    case MAX31790_REG_FAN_FAULT_MASK_1:
+        ms->fan_fault_mask_1 = data;
+        break;
+
+    case MAX31790_REG_FAILED_FAN_OPTS_SEQ_STRT:
+        ms->failed_fan_opts_seq_strt = data;
+        break;
+
+    case MAX31790_REG_PWMOUT_MSB(0) ...
+         MAX31790_REG_PWMOUT_LSB(MAX31790_NUM_FANS - 1):
+        index = (ms->command - MAX31790_REG_PWMOUT_MSB(0)) / 2;
+        rem = (ms->command - MAX31790_REG_PWMOUT_MSB(0)) % 2;
+        if (rem) {
+            max31790_place_bits(&ms->pwmout[index], data, 0);
+        } else {
+            max31790_place_bits(&ms->pwmout[index], data, 8);
+        }
+        break;
+
+    case MAX31790_REG_TARGET_COUNT_MSB(0) ...
+         MAX31790_REG_TARGET_COUNT_LSB(MAX31790_NUM_FANS - 1):
+        index = (ms->command - MAX31790_REG_TARGET_COUNT_MSB(0)) / 2;
+        rem = (ms->command - MAX31790_REG_TARGET_COUNT_MSB(0)) % 2;
+        if (rem) {
+            max31790_place_bits(&ms->target_count[index], data, 0);
+        } else {
+            max31790_place_bits(&ms->target_count[index], data, 8);
+        }
+        break;
+
+    case MAX31790_REG_WINDOW(0) ...
+         MAX31790_REG_WINDOW(MAX31790_NUM_FANS - 1):
+        ms->window[ms->command - MAX31790_REG_WINDOW(0)] = data;
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: writing to unsupported register 0x%02x",
+                      __func__, ms->command);
+        return -1;
+    }
+
+    return 0;
+}
+
+static int max31790_event(I2CSlave *i2c, enum i2c_event event)
+{
+    MAX31790State *ms = MAX31790(i2c);
+
+    if (event == I2C_START_SEND) {
+        ms->i2c_cmd_event = true;
+    }
+
+    return 0;
+}
+
+/* assumes that the fans have the same speed range (SR) */
+static void max31790_get_rpm(Object *obj, Visitor *v, const char *name,
+                             void *opaque, Error **errp)
+{
+    MAX31790State *ms = MAX31790(obj);
+    uint16_t tach_count = *(uint16_t *)opaque;
+    uint32_t sr = max31790_get_sr(ms->fan_dynamics[0]);
+    uint16_t rpm;
+
+    max31790_update_tach_count(ms);
+    tach_count >>= MAX31790_TACH_SHAMT;
+
+    if (tach_count) {
+        rpm = (sr * MAX31790_CLK_FREQ * 60) /
+              (MAX31790_PULSES_PER_REV * tach_count);
+    }
+
+    visit_type_uint16(v, name, &rpm, errp);
+}
+
+static void max31790_set_rpm(Object *obj, Visitor *v, const char *name,
+                             void *opaque, Error **errp)
+{
+    MAX31790State *ms = MAX31790(obj);
+    uint16_t *internal = opaque;
+    uint32_t sr = max31790_get_sr(ms->fan_dynamics[0]);
+    uint16_t rpm, tach_count;
+
+    if (!visit_type_uint16(v, name, &rpm, errp)) {
+        return;
+    }
+
+    if (rpm) {
+        tach_count = (sr * MAX31790_CLK_FREQ * 60) /
+                     (MAX31790_PULSES_PER_REV * rpm);
+    } else {
+        tach_count = 0;
+    }
+
+    *internal = tach_count << MAX31790_TACH_SHAMT;
+}
+
+static void max31790_get(Object *obj, Visitor *v, const char *name,
+                             void *opaque, Error **errp)
+{
+    MAX31790State *ms = MAX31790(obj);
+    uint16_t value;
+
+    max31790_update_tach_count(ms);
+
+    if (strncmp(name, "pwm", 3) == 0) {
+        value = *(uint16_t *)opaque >> 7;
+    } else {
+        value = *(uint16_t *)opaque;
+    }
+
+    visit_type_uint16(v, name, &value, errp);
+}
+
+static void max31790_set(Object *obj, Visitor *v, const char *name,
+                             void *opaque, Error **errp)
+{
+    uint16_t *internal = opaque;
+    uint16_t value;
+
+    if (!visit_type_uint16(v, name, &value, errp)) {
+        return;
+    }
+
+    if (strncmp(name, "pwm", 3) == 0) {
+        *internal = value << MAX31790_PWM_SHAMT;
+    } else {
+        *internal = value;
+    }
+
+}
+
+static void max31790_init(Object *obj)
+{
+    MAX31790State *ms = MAX31790(obj);
+
+    ms->global_config = MAX31790_GLOBAL_CONFIG_DEFAULT;
+    ms->pwm_freq = MAX31790_PWM_FREQ_DEFAULT;
+    ms->failed_fan_opts_seq_strt = MAX31790_FAILED_FAN_OPTS_SEQ_STRT_DEFAULT;
+
+    for (int i = 0; i < MAX31790_NUM_FANS; i++) {
+        ms->max_rpm[i] = MAX31790_MAX_RPM_DEFAULT;
+        ms->fan_config[i] = 0;
+        ms->fan_dynamics[i] = MAX31790_FAN_DYNAMICS_DEFAULT;
+        ms->pwmout[i] = MAX31790_PWMOUT_DEFAULT;
+        ms->target_count[i] = MAX31790_TARGET_COUNT_DEFAULT;
+    }
+
+    max31790_update_tach_count(ms);
+    for (int i = 0; i < MAX31790_NUM_FANS; i++) {
+        object_property_add(obj, "target_rpm[*]", "uint16",
+                            max31790_get_rpm,
+                            max31790_set_rpm, NULL, &ms->target_count[i]);
+
+        /* 9-bit PWM on this device */
+        object_property_add(obj, "pwm[*]", "uint16",
+                            max31790_get,
+                            max31790_set, NULL, &ms->pwmout[i]);
+
+        /* used to calculate rpm for a given pwm duty cycle */
+        object_property_add(obj, "max_rpm[*]", "uint16",
+                            max31790_get,
+                            max31790_set, NULL, &ms->max_rpm[i]);
+    }
+}
+
+static void max31790_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
+
+    dc->desc = "Maxim MAX31790 fan controller";
+
+    k->event = max31790_event;
+    k->recv = max31790_recv;
+    k->send = max31790_send;
+}
+
+static const TypeInfo max31790_info = {
+    .name = TYPE_MAX31790,
+    .parent = TYPE_I2C_SLAVE,
+    .instance_size = sizeof(MAX31790State),
+    .instance_init = max31790_init,
+    .class_init = max31790_class_init,
+};
+
+static void max31790_register_types(void)
+{
+    type_register_static(&max31790_info);
+}
+
+type_init(max31790_register_types)
diff --git a/hw/sensor/meson.build b/hw/sensor/meson.build
index 059c4ca935..4ce68cfc89 100644
--- a/hw/sensor/meson.build
+++ b/hw/sensor/meson.build
@@ -4,3 +4,4 @@  softmmu_ss.add(when: 'CONFIG_DPS310', if_true: files('dps310.c'))
 softmmu_ss.add(when: 'CONFIG_EMC141X', if_true: files('emc141x.c'))
 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_MAX31790', if_true: files('max31790_fan_ctrl.c'))
diff --git a/include/hw/sensor/max31790_fan_ctrl.h b/include/hw/sensor/max31790_fan_ctrl.h
new file mode 100644
index 0000000000..74ff7bb5a0
--- /dev/null
+++ b/include/hw/sensor/max31790_fan_ctrl.h
@@ -0,0 +1,93 @@ 
+/*
+ * Max 31790 Fan controller
+ *
+ * Independently control 6 fans, up to 12 tachometer inputs,
+ * controlled through i2c
+ *
+ * Copyright 2021 Google LLC
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef MAX31790_FAN_CTRL_H
+#define MAX31790_FAN_CTRL_H
+
+#include "hw/i2c/i2c.h"
+#include "qom/object.h"
+
+#define MAX31790_NUM_FANS 6
+#define MAX31790_NUM_TACHS 12
+
+typedef struct MAX31790State {
+    I2CSlave parent;
+
+    /* Registers */
+    uint8_t global_config;
+    uint8_t pwm_freq;
+    uint8_t fan_config[MAX31790_NUM_FANS];
+    uint8_t fan_dynamics[MAX31790_NUM_FANS];
+    uint8_t fan_fault_status_2;
+    uint8_t fan_fault_status_1;
+    uint8_t fan_fault_mask_2;
+    uint8_t fan_fault_mask_1;
+    uint8_t failed_fan_opts_seq_strt;
+    uint16_t tach_count[MAX31790_NUM_TACHS];
+    uint16_t pwm_duty_cycle[MAX31790_NUM_FANS];
+    uint16_t pwmout[MAX31790_NUM_FANS];
+    uint16_t target_count[MAX31790_NUM_FANS];
+    uint8_t window[MAX31790_NUM_FANS];
+
+    /* config */
+    uint16_t max_rpm[MAX31790_NUM_FANS];
+
+    /* i2c transaction state */
+    uint8_t command;
+    bool i2c_cmd_event;
+    bool cmd_is_new;
+} MAX31790State;
+
+#define TYPE_MAX31790 "max31790"
+#define MAX31790(obj) OBJECT_CHECK(MAX31790State, (obj), TYPE_MAX31790)
+
+#define MAX31790_REG_GLOBAL_CONFIG             0x00                 /* R/W */
+#define MAX31790_REG_PWM_FREQ                  0x01                 /* R/W */
+#define MAX31790_REG_FAN_CONFIG(ch)           (0x02 + (ch))         /* R/W */
+#define MAX31790_REG_FAN_DYNAMICS(ch)         (0x08 + (ch))         /* R/W */
+#define MAX31790_REG_FAN_FAULT_STATUS_2        0x10                 /* R/W */
+#define MAX31790_REG_FAN_FAULT_STATUS_1        0x11                 /* R/W */
+#define MAX31790_REG_FAN_FAULT_MASK_2          0x12                 /* R/W */
+#define MAX31790_REG_FAN_FAULT_MASK_1          0x13                 /* R/W */
+#define MAX31790_REG_FAILED_FAN_OPTS_SEQ_STRT  0x14                 /* R/W */
+#define MAX31790_REG_TACH_COUNT_MSB(ch)       (0x18 + (ch) * 2)     /* R */
+#define MAX31790_REG_TACH_COUNT_LSB(ch)       (0x19 + (ch) * 2)     /* R */
+#define MAX31790_REG_PWM_DUTY_CYCLE_MSB(ch)   (0x30 + (ch) * 2)     /* R */
+#define MAX31790_REG_PWM_DUTY_CYCLE_LSB(ch)   (0x31 + (ch) * 2)     /* R */
+#define MAX31790_REG_PWMOUT_MSB(ch)           (0x40 + (ch) * 2)     /* R/W */
+#define MAX31790_REG_PWMOUT_LSB(ch)           (0x41 + (ch) * 2)     /* R/W */
+#define MAX31790_REG_TARGET_COUNT_MSB(ch)     (0x50 + (ch) * 2)     /* R/W */
+#define MAX31790_REG_TARGET_COUNT_LSB(ch)     (0x51 + (ch) * 2)     /* R/W */
+#define MAX31790_REG_WINDOW(ch)               (0x60 + (ch))         /* R/W */
+
+#define MAX31790_GLOBAL_CONFIG_DEFAULT                0x20
+#define MAX31790_PWM_FREQ_DEFAULT                     0x44 /* 125Hz */
+#define MAX31790_FAN_DYNAMICS_DEFAULT                 0x4C
+#define MAX31790_FAILED_FAN_OPTS_SEQ_STRT_DEFAULT     0x45
+#define MAX31790_PWMOUT_DEFAULT                       (128 << 7) /* 25% */
+#define MAX31790_TARGET_COUNT_DEFAULT                 0x3D60
+
+/* Fan Config register bits */
+#define MAX31790_FAN_CFG_RPM_MODE             BIT(7)
+#define MAX31790_FAN_CFG_MONITOR_ONLY         BIT(4)
+#define MAX31790_FAN_CFG_TACH_INPUT_EN        BIT(3)
+#define MAX31790_FAN_CFG_TACH_INPUT           BIT(0)
+
+/* Tachometer calculation constants */
+#define MAX31790_PULSES_PER_REV             2
+#define MAX31790_SR_DEFAULT                 4
+#define MAX31790_CLK_FREQ                   8192
+#define MAX31790_MAX_RPM_DEFAULT            16500
+
+/* reg alignment amounts */
+#define MAX31790_PWM_SHAMT                  7
+#define MAX31790_TACH_SHAMT                 5
+#endif