* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.