* [PATCH 1/3] hw/sensor: add MAX31790 fan controller
@ 2022-01-12 0:25 Titus Rwantare
2022-01-12 0:25 ` [PATCH 2/3] tests/qtest: add tests for " Titus Rwantare
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Titus Rwantare @ 2022-01-12 0:25 UTC (permalink / raw)
To: minyard, peter.maydell
Cc: qemu-arm, qemu-devel, f4bug, venture, Titus Rwantare, Hao Wu
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
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
--
2.34.1.575.g55b058a8bb-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] tests/qtest: add tests for MAX31790 fan controller
2022-01-12 0:25 [PATCH 1/3] hw/sensor: add MAX31790 fan controller Titus Rwantare
@ 2022-01-12 0:25 ` Titus Rwantare
2022-01-27 19:02 ` Peter Maydell
2022-01-12 0:25 ` [PATCH 3/3] hw/arm: kudo add max31790 behind bus 1 switch at 75 Titus Rwantare
2022-01-27 18:59 ` [PATCH 1/3] hw/sensor: add MAX31790 fan controller Peter Maydell
2 siblings, 1 reply; 6+ messages in thread
From: Titus Rwantare @ 2022-01-12 0:25 UTC (permalink / raw)
To: minyard, peter.maydell
Cc: qemu-arm, qemu-devel, f4bug, venture, Titus Rwantare, Hao Wu
Signed-off-by: Titus Rwantare <titusr@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
---
tests/qtest/max31790_fan_ctrl-test.c | 171 +++++++++++++++++++++++++++
tests/qtest/meson.build | 1 +
2 files changed, 172 insertions(+)
create mode 100644 tests/qtest/max31790_fan_ctrl-test.c
diff --git a/tests/qtest/max31790_fan_ctrl-test.c b/tests/qtest/max31790_fan_ctrl-test.c
new file mode 100644
index 0000000000..b0b703d018
--- /dev/null
+++ b/tests/qtest/max31790_fan_ctrl-test.c
@@ -0,0 +1,171 @@
+/*
+ * QTests for MAX31790 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
+ */
+
+#include "qemu/osdep.h"
+#include <math.h>
+#include "hw/sensor/max31790_fan_ctrl.h"
+#include "libqtest-single.h"
+#include "libqos/qgraph.h"
+#include "libqos/i2c.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qnum.h"
+#include "qemu/bitops.h"
+
+#define TEST_ID "max31790-test"
+#define TEST_ADDR (0x37)
+#define TEST_MAX_RPM 0x4000
+
+static uint16_t qmp_max31790_get(const char *id, const char *property)
+{
+ QDict *response;
+ uint64_t ret;
+
+ response = qmp("{ 'execute': 'qom-get', 'arguments': { 'path': %s, "
+ "'property': %s } }", id, property);
+ g_assert(qdict_haskey(response, "return"));
+ ret = qnum_get_uint(qobject_to(QNum, qdict_get(response, "return")));
+ qobject_unref(response);
+ return ret;
+}
+
+static void qmp_max31790_set(const char *id,
+ const char *property,
+ uint16_t value)
+{
+ QDict *response;
+
+ response = qmp("{ 'execute': 'qom-set', 'arguments': { 'path': %s, "
+ "'property': %s, 'value': %u } }", id, property, value);
+ g_assert(qdict_haskey(response, "return"));
+ qobject_unref(response);
+}
+
+static uint32_t max31790_tach_count2rpm(uint16_t tach, uint8_t sr)
+{
+ if (tach) {
+ return (sr * MAX31790_CLK_FREQ * 60) / (MAX31790_PULSES_PER_REV * tach);
+ } else {
+ return 0;
+ }
+}
+
+/* R/W Tach - 6 fans */
+static void test_defaults(void *obj, void *data, QGuestAllocator *alloc)
+{
+ QI2CDevice *i2cdev = (QI2CDevice *)obj;
+ uint8_t i2c_value;
+
+ i2c_value = i2c_get8(i2cdev, MAX31790_REG_GLOBAL_CONFIG);
+ g_assert_cmphex(i2c_value, ==, MAX31790_GLOBAL_CONFIG_DEFAULT);
+
+ i2c_value = i2c_get8(i2cdev, MAX31790_REG_PWM_FREQ);
+ g_assert_cmphex(i2c_value, ==, MAX31790_PWM_FREQ_DEFAULT);
+
+ for (int i = 0; i < MAX31790_NUM_FANS; i++) {
+ i2c_value = i2c_get8(i2cdev, MAX31790_REG_FAN_DYNAMICS(i));
+ g_assert_cmphex(i2c_value, ==, MAX31790_FAN_DYNAMICS_DEFAULT);
+ }
+
+ i2c_value = i2c_get8(i2cdev, MAX31790_REG_FAILED_FAN_OPTS_SEQ_STRT);
+ g_assert_cmphex(i2c_value, ==, MAX31790_FAILED_FAN_OPTS_SEQ_STRT_DEFAULT);
+}
+
+static void test_pwm(void *obj, void *data, QGuestAllocator *alloc)
+{
+ QI2CDevice *i2cdev = (QI2CDevice *)obj;
+ char *path;
+ int err;
+ uint16_t i2c_value, value, rpm;
+
+
+ /* init fans to different pwm duty cycles */
+ for (int i = 0; i < MAX31790_NUM_FANS; i++) {
+ path = g_strdup_printf("max_rpm[%d]", i);
+ qmp_max31790_set(TEST_ID, path, TEST_MAX_RPM); /* ~16k RPM */
+ g_free(path);
+ i2c_set8(i2cdev, MAX31790_REG_FAN_CONFIG(i), 0); /* enable PWM mode */
+ path = g_strdup_printf("pwm[%d]", i);
+ qmp_max31790_set(TEST_ID, path, i * 0x40);
+ g_free(path);
+ }
+
+ /* read and compare qmp with i2c 9-bit pwm */
+ for (int i = 0; i < MAX31790_NUM_FANS; i++) {
+ path = g_strdup_printf("pwm[%d]", i);
+ value = qmp_max31790_get(TEST_ID, path);
+ g_free(path);
+ i2c_value = i2c_get8(i2cdev, MAX31790_REG_PWMOUT_MSB(i)) << 8;
+ i2c_value |= i2c_get8(i2cdev, MAX31790_REG_PWMOUT_LSB(i));
+ i2c_value >>= MAX31790_PWM_SHAMT;
+ g_assert_cmphex(value, ==, i2c_value);
+ }
+
+ /* expect tach to match pwm scaled to max_rpm */
+ for (int i = 0; i < MAX31790_NUM_FANS; i++) {
+ i2c_value = i2c_get8(i2cdev, MAX31790_REG_TACH_COUNT_MSB(i)) << 8;
+ i2c_value |= i2c_get8(i2cdev, MAX31790_REG_TACH_COUNT_LSB(i));
+ i2c_value >>= 5;
+ value = max31790_tach_count2rpm(i2c_value, MAX31790_SR_DEFAULT);
+ rpm = (TEST_MAX_RPM * i * 0x40) / 0x1FF; /* max_rpm x pwm_duty_cycle */
+ err = value - rpm;
+ g_assert_cmpuint(abs(err), <, 163); /* ~1% of max_rpm */
+ }
+}
+
+static void test_rpm(void *obj, void *data, QGuestAllocator *alloc)
+{
+ QI2CDevice *i2cdev = (QI2CDevice *)obj;
+ char *path;
+ int err;
+ uint16_t i2c_value, value, rpm;
+
+ /* init fans to different speeds */
+ for (int i = 0; i < MAX31790_NUM_FANS; i++) {
+ i2c_set8(i2cdev, MAX31790_REG_FAN_CONFIG(i),
+ MAX31790_FAN_CFG_RPM_MODE);
+ path = g_strdup_printf("target_rpm[%d]", i);
+ qmp_max31790_set(TEST_ID, path, i * 1000);
+ g_free(path);
+ }
+
+ /* read and compare qmp with i2c 11-bit tach */
+ for (int i = 0; i < MAX31790_NUM_FANS; i++) {
+ path = g_strdup_printf("target_rpm[%d]", i);
+ value = qmp_max31790_get(TEST_ID, path);
+ g_free(path);
+
+ i2c_value = i2c_get8(i2cdev, MAX31790_REG_TACH_COUNT_MSB(i)) << 8;
+ i2c_value |= i2c_get8(i2cdev, MAX31790_REG_TACH_COUNT_LSB(i));
+ i2c_value >>= MAX31790_TACH_SHAMT;
+
+ rpm = max31790_tach_count2rpm(i2c_value, MAX31790_SR_DEFAULT);
+ err = value - rpm;
+ g_assert_cmpint(abs(err), <, 20); /* 20 RPM */
+ err = (i * 1000) - rpm;
+ g_assert_cmpint(abs(err), <, 20);
+ }
+}
+
+static void max31790_register_nodes(void)
+{
+ QOSGraphEdgeOptions opts = {
+ .extra_device_opts = "id=" TEST_ID ",address=0x37"
+ };
+ add_qi2c_address(&opts, &(QI2CAddress) { TEST_ADDR });
+
+ qos_node_create_driver("max31790", i2c_device_create);
+ qos_node_consumes("max31790", "i2c-bus", &opts);
+
+ qos_add_test("test_defaults", "max31790", test_defaults, NULL);
+ qos_add_test("test_pwm", "max31790", test_pwm, NULL);
+ qos_add_test("test_rpm", "max31790", test_rpm, NULL);
+}
+libqos_init(max31790_register_nodes);
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 37e1eaa449..45694a26ba 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -243,6 +243,7 @@ qos_test_ss.add(
'es1370-test.c',
'ipoctal232-test.c',
'max34451-test.c',
+ 'max31790_fan_ctrl-test.c',
'megasas-test.c',
'ne2000-test.c',
'tulip-test.c',
--
2.34.1.575.g55b058a8bb-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] hw/arm: kudo add max31790 behind bus 1 switch at 75
2022-01-12 0:25 [PATCH 1/3] hw/sensor: add MAX31790 fan controller Titus Rwantare
2022-01-12 0:25 ` [PATCH 2/3] tests/qtest: add tests for " Titus Rwantare
@ 2022-01-12 0:25 ` Titus Rwantare
2022-01-27 19:02 ` Peter Maydell
2022-01-27 18:59 ` [PATCH 1/3] hw/sensor: add MAX31790 fan controller Peter Maydell
2 siblings, 1 reply; 6+ messages in thread
From: Titus Rwantare @ 2022-01-12 0:25 UTC (permalink / raw)
To: minyard, peter.maydell
Cc: qemu-arm, qemu-devel, f4bug, venture, Titus Rwantare
From: Patrick Venture <venture@google.com>
Signed-off-by: Patrick Venture <venture@google.com>
Reviewed-by: Titus Rwantare <titusr@google.com>
---
hw/arm/npcm7xx_boards.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
index 7d0f3148be..6fea2e161f 100644
--- a/hw/arm/npcm7xx_boards.c
+++ b/hw/arm/npcm7xx_boards.c
@@ -342,6 +342,7 @@ static void kudo_bmc_i2c_init(NPCM7xxState *soc)
i2c_mux = i2c_slave_create_simple(npcm7xx_i2c_get_bus(soc, 13),
TYPE_PCA9548, 0x77);
+ i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 2), "max31790", 0x2c);
/* tmp105 is compatible with the lm75 */
i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 2), "tmp105", 0x48);
i2c_slave_create_simple(pca954x_i2c_get_bus(i2c_mux, 3), "tmp105", 0x49);
--
2.34.1.575.g55b058a8bb-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] hw/sensor: add MAX31790 fan controller
2022-01-12 0:25 [PATCH 1/3] hw/sensor: add MAX31790 fan controller Titus Rwantare
2022-01-12 0:25 ` [PATCH 2/3] tests/qtest: add tests for " Titus Rwantare
2022-01-12 0:25 ` [PATCH 3/3] hw/arm: kudo add max31790 behind bus 1 switch at 75 Titus Rwantare
@ 2022-01-27 18:59 ` Peter Maydell
2 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2022-01-27 18:59 UTC (permalink / raw)
To: Titus Rwantare; +Cc: minyard, venture, qemu-devel, f4bug, Hao Wu, qemu-arm
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] tests/qtest: add tests for MAX31790 fan controller
2022-01-12 0:25 ` [PATCH 2/3] tests/qtest: add tests for " Titus Rwantare
@ 2022-01-27 19:02 ` Peter Maydell
0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2022-01-27 19:02 UTC (permalink / raw)
To: Titus Rwantare; +Cc: minyard, venture, qemu-devel, f4bug, Hao Wu, qemu-arm
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>
> ---
> tests/qtest/max31790_fan_ctrl-test.c | 171 +++++++++++++++++++++++++++
> tests/qtest/meson.build | 1 +
> 2 files changed, 172 insertions(+)
> create mode 100644 tests/qtest/max31790_fan_ctrl-test.c
Tests look OK to me, so
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Is it worth adding a test of the address auto-increment logic,
given that the 'increases modulo 8' behaviour is not completely
trivial ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] hw/arm: kudo add max31790 behind bus 1 switch at 75
2022-01-12 0:25 ` [PATCH 3/3] hw/arm: kudo add max31790 behind bus 1 switch at 75 Titus Rwantare
@ 2022-01-27 19:02 ` Peter Maydell
0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2022-01-27 19:02 UTC (permalink / raw)
To: Titus Rwantare; +Cc: venture, qemu-arm, qemu-devel, minyard, f4bug
On Wed, 12 Jan 2022 at 00:25, Titus Rwantare <titusr@google.com> wrote:
>
> From: Patrick Venture <venture@google.com>
>
> Signed-off-by: Patrick Venture <venture@google.com>
> Reviewed-by: Titus Rwantare <titusr@google.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-27 19:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-12 0:25 [PATCH 1/3] hw/sensor: add MAX31790 fan controller Titus Rwantare
2022-01-12 0:25 ` [PATCH 2/3] tests/qtest: add tests for " Titus Rwantare
2022-01-27 19:02 ` Peter Maydell
2022-01-12 0:25 ` [PATCH 3/3] hw/arm: kudo add max31790 behind bus 1 switch at 75 Titus Rwantare
2022-01-27 19:02 ` Peter Maydell
2022-01-27 18:59 ` [PATCH 1/3] hw/sensor: add MAX31790 fan controller Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).