On Tue, 12 May 2026 12:20:39 +0200 Alexander Hansen <[email protected]> wrote:
> Product: [1] > Datasheet: [2] > > Sensor readings can be provided upon creation of the device. In case no > readings are provided the ADC reads a pre-defined arbitrary value. > > root@yosemite4:~# cat /sys/bus/iio/devices/iio:device2/name > max11615 > root@yosemite4:~# cat /sys/bus/iio/devices/iio:device2/in_voltage0_raw > 1922 > root@yosemite4:~# cat /sys/bus/iio/devices/iio:device2/in_voltage_scale > 0.500000000 > Great to see emulation for a sensor in this family - I've not had working actual hardware for quite some time - not because the ADC isn't still common, but just because the arm board was a PXA271 one that I long ago killed off kernel support for ;) A few comments inline. Whilst I understand the motivation is to emulate something, it is a bit minimal and the linux driver is going to trip over the gaps if anyone tries to do much beyond the minimum stuff you have in the test. I think min changes to make are to log a bit more carefully when attempts are made to do things that aren't emulated. In some cases it might be easier to just emulate a bit more though. It's not a particularly complex device but I understand you probably don't want to spend that much time on it. > trace: > less /tmp/qemu-trace.log | grep -i max116 > max11615_realize i2c_addr: 0x33 > max11615_realize i2c_addr: 0x33 > max11615_event i2c_addr: 0x33, event: 0x01 > max11615_write_setup i2c_addr: 0x33, data: 0xd2 > max11615_write_config i2c_addr: 0x33, data: 0x0f > max11615_event i2c_addr: 0x33, event: 0x03 > max11615_event i2c_addr: 0x33, event: 0x01 > max11615_write_setup i2c_addr: 0x33, data: 0xd2 > max11615_write_config i2c_addr: 0x33, data: 0x0f > max11615_event i2c_addr: 0x33, event: 0x03 > max11615_event i2c_addr: 0x33, event: 0x01 > max11615_write_setup i2c_addr: 0x33, data: 0xd2 > max11615_write_config i2c_addr: 0x33, data: 0x61 > max11615_event i2c_addr: 0x33, event: 0x03 > max11615_event i2c_addr: 0x33, event: 0x00 > max11615_recv i2c_addr: 0x33, reg_addr: 0x00 > max11615_recv_return i2c_addr: 0x33, returns: 0xfa > max11615_recv i2c_addr: 0x33, reg_addr: 0x00 > max11615_recv_return i2c_addr: 0x33, returns: 0xd2 > max11615_event i2c_addr: 0x33, event: 0x04 > max11615_event i2c_addr: 0x33, event: 0x03 > > References: > [1] https://www.analog.com/en/products/MAX11615.html > [2] > https://www.analog.com/media/en/technical-documentation/data-sheets/MAX11612-MAX11617.pdf > > Cc: Titus Rwantare <[email protected]> > Cc: "Cédric Le Goater" <[email protected]> (maintainer:ASPEED BMCs) > Cc: "Philippe Mathieu-Daudé" <[email protected]> (odd fixer:Overall sensors) > Cc: Paolo Bonzini <[email protected]> (maintainer:Kconfig) > Cc: Peter Maydell <[email protected]> (supporter:ARM TCG CPUs) > Cc: [email protected] (open list:All patches CC here) > Cc: [email protected] (open list:ARM TCG CPUs) > Signed-off-by: Alexander Hansen <[email protected]> > --- > MAINTAINERS | 1 + > hw/arm/Kconfig | 1 + > hw/arm/aspeed_ast2600_fby4.c | 8 +- > hw/sensor/Kconfig | 4 + > hw/sensor/max11615.c | 202 +++++++++++++++++++++++ > hw/sensor/meson.build | 1 + > hw/sensor/trace-events | 8 + > include/hw/sensor/max11615.h | 20 +++ > tests/functional/arm/test_aspeed_fby4.py | 8 + > 9 files changed, 252 insertions(+), 1 deletion(-) > create mode 100644 hw/sensor/max11615.c > create mode 100644 include/hw/sensor/max11615.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 9c991f8e70..a9c88996a2 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3981,6 +3981,7 @@ S: Maintained > F: hw/i2c/pmbus_device.c > F: hw/sensor/adm1272.c > F: hw/sensor/isl_pmbus_vr.c > +F: hw/sensor/max11615.c > F: hw/sensor/max31790.c > F: hw/sensor/max34451.c > F: include/hw/i2c/pmbus_device.h > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig > index 99864eb878..76a7d327a9 100644 > --- a/hw/arm/Kconfig > +++ b/hw/arm/Kconfig > @@ -553,6 +553,7 @@ config ASPEED_SOC > select PMBUS > select MAX31785 > select MAX31790 > + select MAX11615 > select FSI_APB2OPB_ASPEED > select AT24C > select PCI_EXPRESS > diff --git a/hw/arm/aspeed_ast2600_fby4.c b/hw/arm/aspeed_ast2600_fby4.c > index a7c2dc09ea..036f5b9e38 100644 > --- a/hw/arm/aspeed_ast2600_fby4.c > +++ b/hw/arm/aspeed_ast2600_fby4.c > @@ -12,6 +12,7 @@ > #include "hw/arm/aspeed_soc.h" > #include "hw/nvram/eeprom_at24c.h" > #include "hw/sensor/max31790.h" > +#include "hw/sensor/max11615.h" > #include "hw/i2c/i2c_mux_pca954x.h" > #include "hw/gpio/pca9552.h" > > @@ -186,7 +187,12 @@ static void fby4_i2c_init_fanboard(I2CSlave *fan_mux, > size_t eepromSize) > i2c_slave_create_simple(bus, TYPE_MAX31790, 0x2f); > > /* maxim,max11615 @ 0x33 (adc) */ > - /* TODO */ > + static const uint16_t adc_values[8] = { > + 0b011110000010, 0b010100011000, > + 0b001000110100, 0b100000101001, > + 0b011110000010, 0b010100011000, > + 0b001000110100, 0b100000101001}; Given these correspond to the values read out in the test, maybe just but them in decimal? I'm not sure what the value is in using binary here. > + max11615_init_with_values(bus, 0x33, adc_values, 8); > > at24c_eeprom_init_rom( > bus, 0x52, eepromSize, > diff --git a/hw/sensor/Kconfig b/hw/sensor/Kconfig > index ece2f2b167..84eede9d84 100644 > --- a/hw/sensor/Kconfig > +++ b/hw/sensor/Kconfig > @@ -47,3 +47,7 @@ config MAX31785 > config MAX31790 > bool > depends on PMBUS > + > +config MAX11615 > + bool > + depends on I2C > diff --git a/hw/sensor/max11615.c b/hw/sensor/max11615.c > new file mode 100644 > index 0000000000..7950e00e33 > --- /dev/null > +++ b/hw/sensor/max11615.c > @@ -0,0 +1,202 @@ > +/* > + * Maxim MAX11615 Low-Power 12 bit ADC > + * Models MAX11612,MAX11613,MAX11614,MAX11615,MAX11616,MAX11617 > + * > + * Datasheet: > + * > https://www.analog.com/media/en/technical-documentation/data-sheets/MAX11612-MAX11617.pdf > + * > + * Copyright 2026 9elements > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > +#include "qemu/osdep.h" > +#include "hw/i2c/i2c.h" > +#include "migration/vmstate.h" > +#include "qapi/error.h" > +#include "qapi/visitor.h" > +#include "qemu/log.h" > +#include "qemu/module.h" > +#include "trace.h" > +#include "hw/sensor/max11615.h" > + > +#define MAX11615_NUM_CHANNELS 8 > + > +struct MAX11615State { > + I2CSlave i2c; > + > + uint16_t channels[MAX11615_NUM_CHANNELS]; > + > + /* output buffer */ > + uint8_t outlen; > + uint8_t outbuf[2]; > + > + /* selected channel for read/write operation */ > + uint8_t pointer; > +}; > + > +struct MAX11615Class { > + I2CSlaveClass parent_class; > +}; > + > +OBJECT_DECLARE_TYPE(MAX11615State, MAX11615Class, MAX11615) > + > +static void max11615_read(MAX11615State *s) > +{ > + /* read an ADC channel, first 4 bits must be high */ > + uint8_t msb = s->channels[s->pointer] >> 8; > + uint8_t lsb = s->channels[s->pointer] & 0xff; > + s->outbuf[0] = 0b11110000 | (msb & 0b00001111); > + s->outbuf[1] = lsb; > +} > + > +static void max11615_write_config_byte(MAX11615State *s, uint8_t data) > +{ > + trace_max11615_write_config(s->i2c.address, data); > + > + uint8_t channelSelect = (data >> 1) & 0b1111; It shouldn't be too horrible to support differential mode. Maybe for now just log something if bit 0 is set or implement as the appropriate subtraction of the single ended values. Also good to log a print if the scan mode is set to anything other b11 as output will be garbage (and the linux driver supports all that fun stuff). > + > + /* Table 3. Channel Selection (AIN0 ... AIN11) */ > + if (channelSelect > 11) { > + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid channel select", > __func__); > + channelSelect = 11; > + } > + s->pointer = channelSelect; > +} > + > +static void max11615_write_setup_byte(MAX11615State *s, uint8_t data) > +{ > + trace_max11615_write_setup(s->i2c.address, data); > + /* we ignore the setup byte, not implemented */ Maybe should log on any attempt to set it to values that the emulation will generate wrong answers for? Mind you without differential support only reference is going to make much difference I think. Probably just a print on external reference being set? > +} > + > +static int max11615_send(I2CSlave *i2c, uint8_t data) > +{ > + MAX11615State *s = MAX11615(i2c); > + const uint8_t msb = (data >> 7) & 0b1; > + > + if (msb) { > + max11615_write_setup_byte(s, data); > + } else { > + max11615_write_config_byte(s, data); > + } > + > + s->outlen = 0; > + return 0; > +} > + > +static uint8_t max11615_recv(I2CSlave *i2c) > +{ > + MAX11615State *s = MAX11615(i2c); > + trace_max11615_recv(s->i2c.address, s->pointer); > + > + max11615_read(s); > + > + if (s->outlen >= 2) { > + qemu_log_mask(LOG_GUEST_ERROR, "%s: too many bytes read", __func__); Is that right error type? It's not a guest error to do this, it's unimp emulation. From what I recall large reads are how we get multichannel scans. > + s->outlen = 0; > + } > + > + const uint8_t data = s->outbuf[s->outlen++]; bonus space after = > + > + trace_max11615_recv_return(s->i2c.address, data); > + return data; > +} > + > +static int max11615_event(I2CSlave *i2c, enum i2c_event event) > +{ > + MAX11615State *s = MAX11615(i2c); > + > + trace_max11615_event(s->i2c.address, event); > + > + switch (event) { > + case I2C_START_RECV: > + s->outlen = 0; > + break; > + default: > + break; > + } > + > + return 0; > +} > + > +static const VMStateDescription vmstate_max11615 = { > + .name = TYPE_MAX11615, > + .version_id = 0, > + .minimum_version_id = 0, > + .fields = (const VMStateField[]){ > + VMSTATE_UINT16_ARRAY(channels, MAX11615State, MAX11615_NUM_CHANNELS), > + VMSTATE_UINT8(outlen, MAX11615State), > + VMSTATE_UINT8_ARRAY(outbuf, MAX11615State, 2), > + VMSTATE_UINT8(pointer, MAX11615State), > + VMSTATE_I2C_SLAVE(i2c, MAX11615State), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static void max11615_init(Object *obj) > +{ > + /* Nothing to do */ > +} > + > +I2CSlave *max11615_init_with_values(I2CBus *bus, uint8_t address, > + const uint16_t *init_values, uint32_t init_values_size) > +{ > + MAX11615State *s; > + > + s = MAX11615(i2c_slave_new(TYPE_MAX11615, address)); > + > + for (int i = 0; i < MAX11615_NUM_CHANNELS && i < init_values_size; i++) { > + > + /* arbitrary value */ > + uint16_t value = 0b0000101011010010; > + > + if (i < init_values_size) { > + value = init_values[i]; > + } > + s->channels[i] = value; > + } > + > + i2c_slave_realize_and_unref(I2C_SLAVE(s), bus, &error_abort); > + > + return I2C_SLAVE(s); > +} > + > +static void max11615_realize(DeviceState *dev, Error **errp) > +{ > + MAX11615State *s = MAX11615(dev); > + > + trace_max11615_realize(s->i2c.address); > + > + s->pointer = 0; > + s->outlen = 0; > +} > diff --git a/tests/functional/arm/test_aspeed_fby4.py > b/tests/functional/arm/test_aspeed_fby4.py > index d29423add7..319f1a7672 100755 > --- a/tests/functional/arm/test_aspeed_fby4.py > +++ b/tests/functional/arm/test_aspeed_fby4.py > @@ -51,6 +51,14 @@ def do_test_arm_aspeed_openbmc_no_network(self, machine, > image, uboot, > exec_command_and_wait_for_pattern(self, > "cat /sys/class/hwmon/hwmon2/fan1_fault", "0"); > > + # MAX11615 test > + exec_command_and_wait_for_pattern(self, > + "cat /sys/bus/iio/devices/iio:device2/name", "max11615"); > + exec_command_and_wait_for_pattern(self, > + "cat /sys/bus/iio/devices/iio:device2/in_voltage0_raw", "1922"); > + exec_command_and_wait_for_pattern(self, > + "cat /sys/bus/iio/devices/iio:device2/in_voltage_scale", > "0.500000000"); > + The file naming is probably not stable. But I guess it might work well enough. If you want stable file names go via the bus device and a wild card for the iio:device? bit Jonathan
