* [PATCH 1/8] osdep: add DIV_ROUND_CLOSEST
2026-07-29 23:13 [PATCH 0/8] hw/i2c: PMBus updates and adm1266 fixes Titus Rwantare
@ 2026-07-29 23:13 ` Titus Rwantare
2026-07-30 12:28 ` Peter Maydell
2026-07-29 23:13 ` [PATCH 2/8] hw/i2c: pmbus: add milliunits linear mode functions Titus Rwantare
` (6 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Titus Rwantare @ 2026-07-29 23:13 UTC (permalink / raw)
To: peter.maydell
Cc: qemu-arm, qemu-devel, kfting, imaginos32, wuhaotsh, philmd,
fanjason, Titus Rwantare
This is taken from linux/include/uapi/linux/const.h to match the driver
behaviour when reading sensor values.
Signed-off-by: Titus Rwantare <titusr@google.com>
---
include/qemu/osdep.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 1ec5b42230..08f87c9d61 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -522,6 +522,19 @@ void QEMU_ERROR("code path is reachable")
#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
#endif
+#ifndef DIV_ROUND_CLOSEST
+#define DIV_ROUND_CLOSEST(n, d) \
+({ \
+ typeof(n) __n = n; \
+ typeof(d) __d = d; \
+ (((typeof(n))-1) > 0 || \
+ ((typeof(d))-1) > 0 || \
+ (((__n) > 0) == ((__d) > 0))) ? \
+ (((__n) + ((__d) / 2)) / (__d)) : \
+ (((__n) - ((__d) / 2)) / (__d)); \
+})
+#endif
+
/*
* &(x)[0] is always a pointer - if it's same type as x then the argument is a
* pointer, not an array.
--
2.55.0.508.g3f0d502094-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/8] osdep: add DIV_ROUND_CLOSEST
2026-07-29 23:13 ` [PATCH 1/8] osdep: add DIV_ROUND_CLOSEST Titus Rwantare
@ 2026-07-30 12:28 ` Peter Maydell
0 siblings, 0 replies; 13+ messages in thread
From: Peter Maydell @ 2026-07-30 12:28 UTC (permalink / raw)
To: Titus Rwantare
Cc: qemu-arm, qemu-devel, kfting, imaginos32, wuhaotsh, philmd,
fanjason
On Thu, 30 Jul 2026 at 00:13, Titus Rwantare <titusr@google.com> wrote:
>
> This is taken from linux/include/uapi/linux/const.h to match the driver
> behaviour when reading sensor values.
>
> Signed-off-by: Titus Rwantare <titusr@google.com>
> ---
> include/qemu/osdep.h | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 1ec5b42230..08f87c9d61 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -522,6 +522,19 @@ void QEMU_ERROR("code path is reachable")
> #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
> #endif
>
> +#ifndef DIV_ROUND_CLOSEST
> +#define DIV_ROUND_CLOSEST(n, d) \
> +({ \
> + typeof(n) __n = n; \
> + typeof(d) __d = d; \
> + (((typeof(n))-1) > 0 || \
> + ((typeof(d))-1) > 0 || \
> + (((__n) > 0) == ((__d) > 0))) ? \
> + (((__n) + ((__d) / 2)) / (__d)) : \
> + (((__n) - ((__d) / 2)) / (__d)); \
> +})
> +#endif
This is still missing the documentation comment that the kernel's
version has. (We could also mention in that comment that this is
from the kernel and so GPL-2-only.)
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/8] hw/i2c: pmbus: add milliunits linear mode functions
2026-07-29 23:13 [PATCH 0/8] hw/i2c: PMBus updates and adm1266 fixes Titus Rwantare
2026-07-29 23:13 ` [PATCH 1/8] osdep: add DIV_ROUND_CLOSEST Titus Rwantare
@ 2026-07-29 23:13 ` Titus Rwantare
2026-07-29 23:13 ` [PATCH 3/8] hw/i2c: smbus: increase MAX_DATA_LEN Titus Rwantare
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Titus Rwantare @ 2026-07-29 23:13 UTC (permalink / raw)
To: peter.maydell
Cc: qemu-arm, qemu-devel, kfting, imaginos32, wuhaotsh, philmd,
fanjason, Titus Rwantare
Because we are limited by unsigned ints, we need to be able to provide
inputs in milliunits which can get scaled up to fit in the linear mode
registers.
For example: 3.4 is a valid input, and now we can provide 3400 and scale
it into the register without losing the 0.4
Signed-off-by: Titus Rwantare <titusr@google.com>
---
hw/i2c/pmbus_device.c | 39 +++++++++++++++++++++++++++++++++++
include/hw/i2c/pmbus_device.h | 18 ++++++++++++++++
2 files changed, 57 insertions(+)
diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index 1ca117bd3c..6e477fd6cf 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -38,6 +38,26 @@ uint16_t pmbus_data2linear_mode(uint16_t value, int exp)
return value >> exp;
}
+uint16_t pmbus_milliunits2linear_mode(uint32_t value, int exp)
+{
+ uint64_t ret;
+ uint64_t val = value;
+
+ /* L = D * 2^(-e) */
+ if (exp < 0) {
+ ret = DIV_ROUND_CLOSEST((val << (-exp)), 1000);
+ } else {
+ ret = DIV_ROUND_CLOSEST((val >> exp), 1000);
+ }
+
+ /* clamp value to maximum if it exceeds representable value*/
+ if (ret > UINT16_MAX) {
+ return UINT16_MAX;
+ }
+
+ return (uint16_t)ret;
+}
+
uint16_t pmbus_linear_mode2data(uint16_t value, int exp)
{
/* D = L * 2^e */
@@ -47,6 +67,25 @@ uint16_t pmbus_linear_mode2data(uint16_t value, int exp)
return value << exp;
}
+uint32_t pmbus_linear_mode2milliunits(uint16_t value, int exp)
+{
+ /* D = L * 2^e */
+ uint64_t val = value;
+ uint64_t ret;
+
+ if (exp < 0) {
+ ret = DIV_ROUND_CLOSEST((val * 1000), 1ULL << (-exp));
+ } else {
+ ret = (val << exp) * 1000;
+ }
+
+ if (ret > UINT32_MAX) {
+ return UINT32_MAX;
+ }
+
+ return (uint32_t)ret;
+}
+
void pmbus_send(PMBusDevice *pmdev, const uint8_t *data, uint16_t len)
{
if (pmdev->out_buf_len + len > SMBUS_DATA_MAX_LEN) {
diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h
index f195c11384..9f3569e997 100644
--- a/include/hw/i2c/pmbus_device.h
+++ b/include/hw/i2c/pmbus_device.h
@@ -481,6 +481,15 @@ uint32_t pmbus_direct_mode2data(PMBusCoefficients c, uint16_t value);
*/
uint16_t pmbus_data2linear_mode(uint16_t value, int exp);
+/**
+ * Convert milliunit sensor value to linear mode format
+ *
+ * L = D * 2^(-e)
+ *
+ * @return uint16
+ */
+uint16_t pmbus_milliunits2linear_mode(uint32_t value, int exp);
+
/**
* Convert linear mode formatted data into sensor reading
*
@@ -490,6 +499,15 @@ uint16_t pmbus_data2linear_mode(uint16_t value, int exp);
*/
uint16_t pmbus_linear_mode2data(uint16_t value, int exp);
+/**
+ * Convert linear mode formatted data into sensor reading in milliunits
+ *
+ * D = L * 2^e
+ *
+ * @return uint32 value in milliunits
+ */
+uint32_t pmbus_linear_mode2milliunits(uint16_t value, int exp);
+
/**
* @brief Send a block of data over PMBus
* Assumes that the bytes in the block are already ordered correctly,
--
2.55.0.508.g3f0d502094-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/8] hw/i2c: smbus: increase MAX_DATA_LEN
2026-07-29 23:13 [PATCH 0/8] hw/i2c: PMBus updates and adm1266 fixes Titus Rwantare
2026-07-29 23:13 ` [PATCH 1/8] osdep: add DIV_ROUND_CLOSEST Titus Rwantare
2026-07-29 23:13 ` [PATCH 2/8] hw/i2c: pmbus: add milliunits linear mode functions Titus Rwantare
@ 2026-07-29 23:13 ` Titus Rwantare
2026-07-30 12:29 ` Peter Maydell
2026-07-29 23:13 ` [PATCH 4/8] hw/sensor: update adm1266 block transfers Titus Rwantare
` (4 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Titus Rwantare @ 2026-07-29 23:13 UTC (permalink / raw)
To: peter.maydell
Cc: qemu-arm, qemu-devel, kfting, imaginos32, wuhaotsh, philmd,
fanjason, Titus Rwantare
The 32 byte payload was raised to 255 for block reads in the SMBus 3.0
specification. SMBus does not provide a way for devices to declare which
version of the spec they are designed to so we must allow newer devices
to return as much data as they allow.
vsmstate_smbus_extended_data is added to avoid breaking migration.
Specification: https://smbus.org/specs/SMBus_3_3_20240512.pdf
> 6.5.7 Block Write/Read
The Block Write begins with a slave address and a write condition. After the command code the host issues a byte count which describes how many more bytes will follow in the message. If a slave has 20 bytes to send, the byte count field will have the value 20 (14h), followed by the 20 bytes of data. The byte count does not include the PEC byte. The byte count may be 0. A Block Read or Block Write is allowed to transfer a maximum of 255 data bytes.
Signed-off-by: Titus Rwantare <titusr@google.com>
---
hw/i2c/smbus_slave.c | 27 ++++++++++++++++++++++++++-
include/hw/i2c/smbus_slave.h | 2 +-
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c
index cfb61c879e..5dd2c38af7 100644
--- a/hw/i2c/smbus_slave.c
+++ b/hw/i2c/smbus_slave.c
@@ -215,6 +215,26 @@ bool smbus_vmstate_needed(SMBusDevice *dev)
return dev->mode != SMBUS_IDLE;
}
+#define SMBUS_DATA_MAX_LEN_OLD 34
+static bool smbus_extended_needed(void *opaque)
+{
+ SMBusDevice *dev = opaque;
+ return dev->data_len > SMBUS_DATA_MAX_LEN_OLD;
+}
+
+static const VMStateDescription vmstate_smbus_extended_data = {
+ .name = TYPE_SMBUS_DEVICE"/extended",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = smbus_extended_needed,
+ .fields = (const VMStateField[]) {
+ /* separately save [34..257) */
+ VMSTATE_UINT8_SUB_ARRAY(data_buf, SMBusDevice, SMBUS_DATA_MAX_LEN_OLD,
+ SMBUS_DATA_MAX_LEN - SMBUS_DATA_MAX_LEN_OLD),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
const VMStateDescription vmstate_smbus_device = {
.name = TYPE_SMBUS_DEVICE,
.version_id = 1,
@@ -223,8 +243,13 @@ const VMStateDescription vmstate_smbus_device = {
VMSTATE_I2C_SLAVE(i2c, SMBusDevice),
VMSTATE_INT32(mode, SMBusDevice),
VMSTATE_INT32(data_len, SMBusDevice),
- VMSTATE_UINT8_ARRAY(data_buf, SMBusDevice, SMBUS_DATA_MAX_LEN),
+ VMSTATE_UINT8_SUB_ARRAY(data_buf, SMBusDevice, 0,
+ SMBUS_DATA_MAX_LEN_OLD),
VMSTATE_END_OF_LIST()
+ },
+ .subsections = (const VMStateDescription * const[]) {
+ &vmstate_smbus_extended_data,
+ NULL
}
};
diff --git a/include/hw/i2c/smbus_slave.h b/include/hw/i2c/smbus_slave.h
index 86bfe0a79e..59522ff1b1 100644
--- a/include/hw/i2c/smbus_slave.h
+++ b/include/hw/i2c/smbus_slave.h
@@ -64,7 +64,7 @@ struct SMBusDeviceClass {
uint8_t (*receive_byte)(SMBusDevice *dev);
};
-#define SMBUS_DATA_MAX_LEN 34 /* command + len + 32 bytes of data. */
+#define SMBUS_DATA_MAX_LEN 257 /* command + len + 255 bytes of data. */
struct SMBusDevice {
/* The SMBus protocol is implemented on top of I2C. */
--
2.55.0.508.g3f0d502094-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/8] hw/i2c: smbus: increase MAX_DATA_LEN
2026-07-29 23:13 ` [PATCH 3/8] hw/i2c: smbus: increase MAX_DATA_LEN Titus Rwantare
@ 2026-07-30 12:29 ` Peter Maydell
0 siblings, 0 replies; 13+ messages in thread
From: Peter Maydell @ 2026-07-30 12:29 UTC (permalink / raw)
To: Titus Rwantare
Cc: qemu-arm, qemu-devel, kfting, imaginos32, wuhaotsh, philmd,
fanjason
On Thu, 30 Jul 2026 at 00:13, Titus Rwantare <titusr@google.com> wrote:
>
> The 32 byte payload was raised to 255 for block reads in the SMBus 3.0
> specification. SMBus does not provide a way for devices to declare which
> version of the spec they are designed to so we must allow newer devices
> to return as much data as they allow.
>
> vsmstate_smbus_extended_data is added to avoid breaking migration.
>
> Specification: https://smbus.org/specs/SMBus_3_3_20240512.pdf
>
> > 6.5.7 Block Write/Read
> The Block Write begins with a slave address and a write condition. After the command code the host issues a byte count which describes how many more bytes will follow in the message. If a slave has 20 bytes to send, the byte count field will have the value 20 (14h), followed by the 20 bytes of data. The byte count does not include the PEC byte. The byte count may be 0. A Block Read or Block Write is allowed to transfer a maximum of 255 data bytes.
You could line wrap this quote so it isn't one super long line.
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/8] hw/sensor: update adm1266 block transfers
2026-07-29 23:13 [PATCH 0/8] hw/i2c: PMBus updates and adm1266 fixes Titus Rwantare
` (2 preceding siblings ...)
2026-07-29 23:13 ` [PATCH 3/8] hw/i2c: smbus: increase MAX_DATA_LEN Titus Rwantare
@ 2026-07-29 23:13 ` Titus Rwantare
2026-07-29 23:13 ` [PATCH 5/8] hw/sensor: switch adm1266 to millivolts vout Titus Rwantare
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Titus Rwantare @ 2026-07-29 23:13 UTC (permalink / raw)
To: peter.maydell
Cc: qemu-arm, qemu-devel, kfting, imaginos32, wuhaotsh, philmd,
fanjason, Titus Rwantare
Fixes an issue with reading the MFR_* registers on the ADM1266, this
device has an unconventional access pattern where a block write of
length 1 is written to the MFR_* register with the length of the data
to be read back. Simultaneously, it is possible to write to the contents
of these registers so long as the block write is longer than 1.
Signed-off-by: Titus Rwantare <titusr@google.com>
---
hw/sensor/adm1266.c | 124 ++++++++++++++++++++++++++++++++-----
tests/qtest/adm1266-test.c | 25 +++++---
2 files changed, 126 insertions(+), 23 deletions(-)
diff --git a/hw/sensor/adm1266.c b/hw/sensor/adm1266.c
index 37d1cffd57..2979557309 100644
--- a/hw/sensor/adm1266.c
+++ b/hw/sensor/adm1266.c
@@ -37,13 +37,14 @@ OBJECT_DECLARE_SIMPLE_TYPE(ADM1266State, ADM1266)
#define ADM1266_CAPABILITY_NO_PEC 0x20
#define ADM1266_PMBUS_REVISION_DEFAULT 0x22
#define ADM1266_MFR_ID_DEFAULT "ADI"
-#define ADM1266_MFR_ID_DEFAULT_LEN 32
#define ADM1266_MFR_MODEL_DEFAULT "ADM1266-A1"
-#define ADM1266_MFR_MODEL_DEFAULT_LEN 32
#define ADM1266_MFR_REVISION_DEFAULT "25"
-#define ADM1266_MFR_REVISION_DEFAULT_LEN 8
+#define ADM1266_MFR_LOCATION_DEFAULT "0000"
+#define ADM1266_MFR_DATE_DEFAULT "0000"
+#define ADM1266_MFR_SERIAL_DEFAULT "0000"
-#define ADM1266_NUM_PAGES 17
+#define ADM1266_NUM_PAGES 17
+#define ADM1266_READ_LENGTH_DEFAULT 48
/**
* PAGE Index
* Page 0 VH1.
@@ -66,10 +67,14 @@ OBJECT_DECLARE_SIMPLE_TYPE(ADM1266State, ADM1266)
*/
typedef struct ADM1266State {
PMBusDevice parent;
+ uint8_t read_length;
char mfr_id[32];
char mfr_model[32];
char mfr_rev[8];
+ char mfr_location[48];
+ char mfr_date[16];
+ char mfr_serial[32];
} ADM1266State;
static const uint8_t adm1266_ic_device_id[] = {0x03, 0x41, 0x12, 0x66};
@@ -95,9 +100,34 @@ static void adm1266_exit_reset(Object *obj, ResetType type)
pmdev->pages[i].revision = ADM1266_PMBUS_REVISION_DEFAULT;
}
- strncpy(s->mfr_id, ADM1266_MFR_ID_DEFAULT, 4);
- strncpy(s->mfr_model, ADM1266_MFR_MODEL_DEFAULT, 11);
- strncpy(s->mfr_rev, ADM1266_MFR_REVISION_DEFAULT, 3);
+ memcpy(s->mfr_id, ADM1266_MFR_ID_DEFAULT, 4);
+ memcpy(s->mfr_model, ADM1266_MFR_MODEL_DEFAULT, 11);
+ memcpy(s->mfr_rev, ADM1266_MFR_REVISION_DEFAULT, 3);
+ memcpy(s->mfr_location, ADM1266_MFR_LOCATION_DEFAULT, 5);
+ memcpy(s->mfr_date, ADM1266_MFR_DATE_DEFAULT, 5);
+ memcpy(s->mfr_serial, ADM1266_MFR_SERIAL_DEFAULT, 5);
+ s->read_length = ADM1266_READ_LENGTH_DEFAULT;
+}
+
+static void adm1266_send_string(PMBusDevice *pmdev, const char *str)
+{
+ ADM1266State *s = ADM1266(pmdev);
+ size_t len = strlen(str);
+
+ if (s->read_length < len) {
+ len = s->read_length;
+ }
+
+ g_assert(len + pmdev->out_buf_len < SMBUS_DATA_MAX_LEN);
+ pmdev->out_buf[len + pmdev->out_buf_len] = len;
+
+ for (int i = len - 1; i >= 0; i--) {
+ pmdev->out_buf[i + pmdev->out_buf_len] = str[len - 1 - i];
+ }
+ pmdev->out_buf_len += len + 1;
+
+ /* reset read length */
+ s->read_length = ADM1266_READ_LENGTH_DEFAULT;
}
static uint8_t adm1266_read_byte(PMBusDevice *pmdev)
@@ -106,15 +136,27 @@ static uint8_t adm1266_read_byte(PMBusDevice *pmdev)
switch (pmdev->code) {
case PMBUS_MFR_ID: /* R/W block */
- pmbus_send_string(pmdev, s->mfr_id);
+ adm1266_send_string(pmdev, s->mfr_id);
break;
case PMBUS_MFR_MODEL: /* R/W block */
- pmbus_send_string(pmdev, s->mfr_model);
+ adm1266_send_string(pmdev, s->mfr_model);
break;
case PMBUS_MFR_REVISION: /* R/W block */
- pmbus_send_string(pmdev, s->mfr_rev);
+ adm1266_send_string(pmdev, s->mfr_rev);
+ break;
+
+ case PMBUS_MFR_LOCATION: /* R/W block */
+ adm1266_send_string(pmdev, s->mfr_location);
+ break;
+
+ case PMBUS_MFR_DATE: /* R/W block */
+ adm1266_send_string(pmdev, s->mfr_date);
+ break;
+
+ case PMBUS_MFR_SERIAL: /* R/W block */
+ adm1266_send_string(pmdev, s->mfr_serial);
break;
case PMBUS_IC_DEVICE_ID:
@@ -135,6 +177,43 @@ static uint8_t adm1266_read_byte(PMBusDevice *pmdev)
return 0;
}
+static uint8_t adm1266_receive_block(PMBusDevice *pmdev, uint8_t *dest,
+ size_t len)
+{
+ ADM1266State *s = ADM1266(pmdev);
+ uint8_t sent_len;
+
+ /* Exclude command code from return value */
+ pmdev->in_buf++;
+ pmdev->in_buf_len--;
+
+ /* The byte after the command code denotes the length */
+ sent_len = pmdev->in_buf[0];
+
+ /* Block writes with length 1 are read requests */
+ if (sent_len == 1) {
+ s->read_length = pmdev->in_buf[1];
+ return 0;
+ }
+
+ /* exclude length byte */
+ pmdev->in_buf++;
+ pmdev->in_buf_len--;
+
+ /* Be as conservative as possible on how much data to receive */
+ if (pmdev->in_buf_len < len) {
+ len = pmdev->in_buf_len;
+ }
+ if (sent_len < len) {
+ len = sent_len;
+ }
+
+ /* dest may contain data from previous writes */
+ memset(dest, 0, len);
+ memcpy(dest, pmdev->in_buf, len);
+ return len;
+}
+
static int adm1266_write_data(PMBusDevice *pmdev, const uint8_t *buf,
uint8_t len)
{
@@ -142,16 +221,31 @@ static int adm1266_write_data(PMBusDevice *pmdev, const uint8_t *buf,
switch (pmdev->code) {
case PMBUS_MFR_ID: /* R/W block */
- pmbus_receive_block(pmdev, (uint8_t *)s->mfr_id, sizeof(s->mfr_id));
+ adm1266_receive_block(pmdev, (uint8_t *)s->mfr_id, sizeof(s->mfr_id));
break;
case PMBUS_MFR_MODEL: /* R/W block */
- pmbus_receive_block(pmdev, (uint8_t *)s->mfr_model,
- sizeof(s->mfr_model));
+ adm1266_receive_block(pmdev, (uint8_t *)s->mfr_model,
+ sizeof(s->mfr_model));
break;
case PMBUS_MFR_REVISION: /* R/W block*/
- pmbus_receive_block(pmdev, (uint8_t *)s->mfr_rev, sizeof(s->mfr_rev));
+ adm1266_receive_block(pmdev, (uint8_t *)s->mfr_rev, sizeof(s->mfr_rev));
+ break;
+
+ case PMBUS_MFR_LOCATION: /* R/W block*/
+ adm1266_receive_block(pmdev, (uint8_t *)s->mfr_location,
+ sizeof(s->mfr_location));
+ break;
+
+ case PMBUS_MFR_DATE: /* R/W block*/
+ adm1266_receive_block(pmdev, (uint8_t *)s->mfr_date,
+ sizeof(s->mfr_date));
+ break;
+
+ case PMBUS_MFR_SERIAL: /* R/W block*/
+ adm1266_receive_block(pmdev, (uint8_t *)s->mfr_serial,
+ sizeof(s->mfr_serial));
break;
case ADM1266_SET_RTC: /* do nothing */
@@ -212,7 +306,7 @@ static void adm1266_init(Object *obj)
{
PMBusDevice *pmdev = PMBUS_DEVICE(obj);
uint64_t flags = PB_HAS_VOUT_MODE | PB_HAS_VOUT | PB_HAS_VOUT_MARGIN |
- PB_HAS_VOUT_RATING | PB_HAS_STATUS_MFR_SPECIFIC;
+ PB_HAS_VOUT_RATING;
for (int i = 0; i < ADM1266_NUM_PAGES; i++) {
pmbus_page_config(pmdev, i, flags);
diff --git a/tests/qtest/adm1266-test.c b/tests/qtest/adm1266-test.c
index 726e475938..fa8bbc5795 100644
--- a/tests/qtest/adm1266-test.c
+++ b/tests/qtest/adm1266-test.c
@@ -83,20 +83,28 @@ static void test_defaults(void *obj, void *data, QGuestAllocator *alloc)
compare_string(i2cdev, PMBUS_MFR_REVISION, ADM1266_MFR_REVISION_DEFAULT);
}
-/* test r/w registers */
-static void test_rw_regs(void *obj, void *data, QGuestAllocator *alloc)
+static void test_partial_reads(void *obj, void *data, QGuestAllocator *alloc)
{
QI2CDevice *i2cdev = (QI2CDevice *)obj;
+ /* 1 byte block write requesting 7 byte response */
+ uint8_t req_len[] = {0x01, 0x7};
- /* empty strings */
- i2c_set8(i2cdev, PMBUS_MFR_ID, 0);
- compare_string(i2cdev, PMBUS_MFR_ID, "");
+ i2c_write_block(i2cdev, PMBUS_MFR_MODEL, req_len, sizeof(req_len));
+ compare_string(i2cdev, PMBUS_MFR_MODEL, "ADM1266");
- i2c_set8(i2cdev, PMBUS_MFR_MODEL, 0);
+ req_len[1] = 0;
+ i2c_write_block(i2cdev, PMBUS_MFR_MODEL, req_len, sizeof(req_len));
compare_string(i2cdev, PMBUS_MFR_MODEL, "");
- i2c_set8(i2cdev, PMBUS_MFR_REVISION, 0);
- compare_string(i2cdev, PMBUS_MFR_REVISION, "");
+ req_len[1] = 100;
+ i2c_write_block(i2cdev, PMBUS_MFR_MODEL, req_len, sizeof(req_len));
+ compare_string(i2cdev, PMBUS_MFR_MODEL, ADM1266_MFR_MODEL_DEFAULT);
+}
+
+/* test r/w registers */
+static void test_rw_regs(void *obj, void *data, QGuestAllocator *alloc)
+{
+ QI2CDevice *i2cdev = (QI2CDevice *)obj;
/* test strings */
write_and_compare_string(i2cdev, PMBUS_MFR_ID, TEST_STRING_A,
@@ -118,6 +126,7 @@ static void adm1266_register_nodes(void)
qos_node_consumes("adm1266", "i2c-bus", &opts);
qos_add_test("test_defaults", "adm1266", test_defaults, NULL);
+ qos_add_test("test_partial_reads", "adm1266", test_partial_reads, NULL);
qos_add_test("test_rw_regs", "adm1266", test_rw_regs, NULL);
}
--
2.55.0.508.g3f0d502094-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 5/8] hw/sensor: switch adm1266 to millivolts vout
2026-07-29 23:13 [PATCH 0/8] hw/i2c: PMBus updates and adm1266 fixes Titus Rwantare
` (3 preceding siblings ...)
2026-07-29 23:13 ` [PATCH 4/8] hw/sensor: update adm1266 block transfers Titus Rwantare
@ 2026-07-29 23:13 ` Titus Rwantare
2026-07-30 12:38 ` Peter Maydell
2026-07-29 23:13 ` [PATCH 6/8] hw/i2c: fix VOUT_MODE representation on little-endian machines Titus Rwantare
` (2 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Titus Rwantare @ 2026-07-29 23:13 UTC (permalink / raw)
To: peter.maydell
Cc: qemu-arm, qemu-devel, kfting, imaginos32, wuhaotsh, philmd,
fanjason, Titus Rwantare
Enables storing fractional voltages for the ADM1266 over QMP
Signed-off-by: Titus Rwantare <titusr@google.com>
---
hw/sensor/adm1266.c | 27 ++++---
tests/qtest/adm1266-test.c | 150 +++++++++++++++++++++++++++++++++++++
2 files changed, 167 insertions(+), 10 deletions(-)
diff --git a/hw/sensor/adm1266.c b/hw/sensor/adm1266.c
index 2979557309..80960dc1c4 100644
--- a/hw/sensor/adm1266.c
+++ b/hw/sensor/adm1266.c
@@ -263,32 +263,39 @@ static int adm1266_write_data(PMBusDevice *pmdev, const uint8_t *buf,
static void adm1266_get(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
- uint16_t value;
+ uint32_t value, index;
PMBusDevice *pmdev = PMBUS_DEVICE(obj);
PMBusVoutMode *mode = (PMBusVoutMode *)&pmdev->pages[0].vout_mode;
- if (strcmp(name, "vout") == 0) {
- value = pmbus_linear_mode2data(*(uint16_t *)opaque, mode->exp);
+ if (strncmp(name, "vout[", 5) == 0) {
+ sscanf(name, "vout[%u]", &index);
+ mode = (PMBusVoutMode *)&pmdev->pages[index].vout_mode;
+ value = pmbus_linear_mode2milliunits(*(uint16_t *)opaque, mode->exp);
} else {
value = *(uint16_t *)opaque;
}
- visit_type_uint16(v, name, &value, errp);
+ visit_type_uint32(v, name, &value, errp);
}
static void adm1266_set(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
uint16_t *internal = opaque;
- uint16_t value;
+ uint32_t value, index;
PMBusDevice *pmdev = PMBUS_DEVICE(obj);
- PMBusVoutMode *mode = (PMBusVoutMode *)&pmdev->pages[0].vout_mode;
+ PMBusVoutMode *mode;
- if (!visit_type_uint16(v, name, &value, errp)) {
+ if (!visit_type_uint32(v, name, &value, errp)) {
return;
}
-
- *internal = pmbus_data2linear_mode(value, mode->exp);
+ if (strncmp(name, "vout[", 5) == 0) {
+ sscanf(name, "vout[%u]", &index);
+ mode = (PMBusVoutMode *)&pmdev->pages[index].vout_mode;
+ *internal = pmbus_milliunits2linear_mode(value, mode->exp);
+ } else {
+ *internal = value;
+ }
pmbus_check_limits(pmdev);
}
@@ -311,7 +318,7 @@ static void adm1266_init(Object *obj)
for (int i = 0; i < ADM1266_NUM_PAGES; i++) {
pmbus_page_config(pmdev, i, flags);
- object_property_add(obj, "vout[*]", "uint16",
+ object_property_add(obj, "vout[*]", "uint32",
adm1266_get,
adm1266_set, NULL, &pmdev->pages[i].read_vout);
}
diff --git a/tests/qtest/adm1266-test.c b/tests/qtest/adm1266-test.c
index fa8bbc5795..fd3d8079b6 100644
--- a/tests/qtest/adm1266-test.c
+++ b/tests/qtest/adm1266-test.c
@@ -16,6 +16,7 @@
#include "qobject/qdict.h"
#include "qobject/qnum.h"
#include "qemu/bitops.h"
+#include "qemu/bswap.h"
#define TEST_ID "adm1266-test"
#define TEST_ADDR (0x12)
@@ -45,6 +46,57 @@
#define TEST_STRING_B "b sample"
#define TEST_STRING_C "rev c"
+#define ADM1266_NUM_PAGES 17
+#define ADM1266_MAX_VALUE 65535000
+
+typedef union {
+ uint8_t raw;
+ PMBusVoutMode mode;
+} ADM1266VoutMode;
+
+static uint32_t qmp_adm1266_get(const char *id, const char *property)
+{
+ QDict *response;
+ uint32_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_adm1266_set(const char *id,
+ const char *property,
+ uint32_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"));
+}
+
+static uint64_t adm1266_linear_mode2milliunits(uint16_t value, int exp)
+{
+ /* D = L * 2^e */
+ uint64_t val = value;
+ uint64_t ret;
+
+ if (exp < 0) {
+ ret = DIV_ROUND_CLOSEST((val * 1000), 1ULL << (-exp));
+ } else {
+ ret = (val << exp) * 1000;
+ }
+
+ if (ret > UINT32_MAX) {
+ return UINT32_MAX;
+ }
+
+ return ret;
+}
+
static void compare_string(QI2CDevice *i2cdev, uint8_t reg,
const char *test_str)
{
@@ -67,6 +119,98 @@ static void write_and_compare_string(QI2CDevice *i2cdev, uint8_t reg,
compare_string(i2cdev, reg, test_str);
}
+static void test_vout_milliunits(void *obj, void *data, QGuestAllocator *alloc)
+{
+ uint16_t i2c_value, value;
+ uint64_t i2c_milliunits;
+ QI2CDevice *i2cdev = (QI2CDevice *)obj;
+ char *path;
+ ADM1266VoutMode m;
+
+ /* set a different value in millivolts for each page */
+ for (int i = 0; i < ADM1266_NUM_PAGES; i++) {
+ path = g_strdup_printf("vout[%d]", i);
+ qmp_adm1266_set(TEST_ID, path, (1000 * (i + 1)));
+ }
+
+ for (int i = 0; i < ADM1266_NUM_PAGES; i++) {
+ i2c_set8(i2cdev, PMBUS_PAGE, i);
+
+ m.raw = i2c_get8(i2cdev, PMBUS_VOUT_MODE);
+ i2c_value = bswap16(i2c_get16(i2cdev, PMBUS_READ_VOUT));
+ i2c_milliunits = adm1266_linear_mode2milliunits(i2c_value, m.mode.exp);
+ g_assert_cmpuint(i2c_milliunits, ==, (1000 * (i + 1)));
+
+ path = g_strdup_printf("vout[%d]", i);
+ value = qmp_adm1266_get(TEST_ID, path);
+ g_assert_cmpuint(value, ==, (1000 * (i + 1)));
+ }
+}
+
+/*
+ * Note that the exponent determines the dynamic range, large exponents can not
+ * be used with values that need to be incremented in small steps
+ */
+static void test_vout_mode_exponent(void *obj, void *data,
+ QGuestAllocator *alloc)
+{
+ uint16_t i2c_value, value, expected;
+ uint64_t i2c_milliunits;
+ QI2CDevice *i2cdev = (QI2CDevice *)obj;
+ ADM1266VoutMode m;
+ char *path;
+
+ /* set a different exponent per page and a different value */
+ for (int i = 0; i < ADM1266_NUM_PAGES; i++) {
+ i2c_set8(i2cdev, PMBUS_PAGE, i);
+ expected = 1000 * (i * 2);
+ m.mode.exp = i - 14;
+ i2c_set8(i2cdev, PMBUS_VOUT_MODE, m.raw);
+ path = g_strdup_printf("vout[%d]", i);
+ qmp_adm1266_set(TEST_ID, path, expected);
+ }
+
+ for (int i = 0; i < ADM1266_NUM_PAGES; i++) {
+ i2c_set8(i2cdev, PMBUS_PAGE, i);
+ expected = 1000 * (i * 2);
+ /* check correct value from i2c*/
+ m.raw = i2c_get8(i2cdev, PMBUS_VOUT_MODE);
+ i2c_value = bswap16(i2c_get16(i2cdev, PMBUS_READ_VOUT));
+ i2c_milliunits = adm1266_linear_mode2milliunits(i2c_value, m.mode.exp);
+ g_assert_cmpuint(i2c_milliunits, ==, expected);
+
+ /* check correct value from qmp*/
+ path = g_strdup_printf("vout[%d]", i);
+ value = qmp_adm1266_get(TEST_ID, path);
+ g_assert_cmpuint(value, ==, expected);
+ }
+}
+
+static void test_vout_clamp_to_max(void *obj, void *data,
+ QGuestAllocator *alloc)
+{
+ uint16_t i2c_value;
+ uint32_t value;
+ QI2CDevice *i2cdev = (QI2CDevice *)obj;
+ char *path;
+
+ for (int i = 0; i < ADM1266_NUM_PAGES; i++) {
+ path = g_strdup_printf("vout[%d]", i);
+ qmp_adm1266_set(TEST_ID, path, 90000000);
+ }
+
+ for (int i = 0; i < ADM1266_NUM_PAGES; i++) {
+ i2c_set8(i2cdev, PMBUS_PAGE, i);
+
+ i2c_value = bswap16(i2c_get16(i2cdev, PMBUS_READ_VOUT));
+ g_assert_cmpuint(i2c_value, ==, UINT16_MAX);
+
+ path = g_strdup_printf("vout[%d]", i);
+ value = qmp_adm1266_get(TEST_ID, path);
+ g_assert_cmpuint(value, ==, ADM1266_MAX_VALUE);
+ }
+}
+
static void test_defaults(void *obj, void *data, QGuestAllocator *alloc)
{
uint16_t i2c_value;
@@ -128,6 +272,12 @@ static void adm1266_register_nodes(void)
qos_add_test("test_defaults", "adm1266", test_defaults, NULL);
qos_add_test("test_partial_reads", "adm1266", test_partial_reads, NULL);
qos_add_test("test_rw_regs", "adm1266", test_rw_regs, NULL);
+ qos_add_test("test_vout_milliunits", "adm1266",
+ test_vout_milliunits, NULL);
+ qos_add_test("test_vout_mode_exponent", "adm1266",
+ test_vout_mode_exponent, NULL);
+ qos_add_test("test_vout_clamp_to_max", "adm1266",
+ test_vout_clamp_to_max, NULL);
}
libqos_init(adm1266_register_nodes);
--
2.55.0.508.g3f0d502094-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 5/8] hw/sensor: switch adm1266 to millivolts vout
2026-07-29 23:13 ` [PATCH 5/8] hw/sensor: switch adm1266 to millivolts vout Titus Rwantare
@ 2026-07-30 12:38 ` Peter Maydell
0 siblings, 0 replies; 13+ messages in thread
From: Peter Maydell @ 2026-07-30 12:38 UTC (permalink / raw)
To: Titus Rwantare
Cc: qemu-arm, qemu-devel, kfting, imaginos32, wuhaotsh, philmd,
fanjason
On Thu, 30 Jul 2026 at 00:13, Titus Rwantare <titusr@google.com> wrote:
>
> Enables storing fractional voltages for the ADM1266 over QMP
>
> Signed-off-by: Titus Rwantare <titusr@google.com>
> ---
> hw/sensor/adm1266.c | 27 ++++---
> tests/qtest/adm1266-test.c | 150 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 167 insertions(+), 10 deletions(-)
>
> diff --git a/hw/sensor/adm1266.c b/hw/sensor/adm1266.c
> index 2979557309..80960dc1c4 100644
> --- a/hw/sensor/adm1266.c
> +++ b/hw/sensor/adm1266.c
> @@ -263,32 +263,39 @@ static int adm1266_write_data(PMBusDevice *pmdev, const uint8_t *buf,
> static void adm1266_get(Object *obj, Visitor *v, const char *name, void *opaque,
> Error **errp)
> {
> - uint16_t value;
> + uint32_t value, index;
> PMBusDevice *pmdev = PMBUS_DEVICE(obj);
> PMBusVoutMode *mode = (PMBusVoutMode *)&pmdev->pages[0].vout_mode;
>
> - if (strcmp(name, "vout") == 0) {
> - value = pmbus_linear_mode2data(*(uint16_t *)opaque, mode->exp);
> + if (strncmp(name, "vout[", 5) == 0) {
> + sscanf(name, "vout[%u]", &index);
What is this doing? Why do we need to do it now when we didn't
need to call sscanf before? If we do need to use sscanf() we
should check the return result for errors.
> + mode = (PMBusVoutMode *)&pmdev->pages[index].vout_mode;
> + value = pmbus_linear_mode2milliunits(*(uint16_t *)opaque, mode->exp);
> } else {
> value = *(uint16_t *)opaque;
You've changed this to a uint32_t property, so leaving this as
a uint16_t* cast looks wrong now.
> }
>
> - visit_type_uint16(v, name, &value, errp);
> + visit_type_uint32(v, name, &value, errp);
> }
>
> static void adm1266_set(Object *obj, Visitor *v, const char *name, void *opaque,
> Error **errp)
> {
> uint16_t *internal = opaque;
> - uint16_t value;
> + uint32_t value, index;
> PMBusDevice *pmdev = PMBUS_DEVICE(obj);
> - PMBusVoutMode *mode = (PMBusVoutMode *)&pmdev->pages[0].vout_mode;
> + PMBusVoutMode *mode;
>
> - if (!visit_type_uint16(v, name, &value, errp)) {
> + if (!visit_type_uint32(v, name, &value, errp)) {
> return;
> }
> -
> - *internal = pmbus_data2linear_mode(value, mode->exp);
> + if (strncmp(name, "vout[", 5) == 0) {
> + sscanf(name, "vout[%u]", &index);
> + mode = (PMBusVoutMode *)&pmdev->pages[index].vout_mode;
> + *internal = pmbus_milliunits2linear_mode(value, mode->exp);
> + } else {
> + *internal = value;
> + }
> pmbus_check_limits(pmdev);
This looks like it's changing the semantics of this property,
but there's no documentation either of what the old set of
accepted strings were or what the new set are.
We shouldn't change the semantics of existing properties
in a non-backwards compatible way, generally speaking.
If we do really need to do that then we should flag that
up clearly in the commit message with the rationale and
the description of the change and its effects.
> +static void test_vout_milliunits(void *obj, void *data, QGuestAllocator *alloc)
> +{
> + uint16_t i2c_value, value;
> + uint64_t i2c_milliunits;
> + QI2CDevice *i2cdev = (QI2CDevice *)obj;
> + char *path;
> + ADM1266VoutMode m;
> +
> + /* set a different value in millivolts for each page */
> + for (int i = 0; i < ADM1266_NUM_PAGES; i++) {
> + path = g_strdup_printf("vout[%d]", i);
> + qmp_adm1266_set(TEST_ID, path, (1000 * (i + 1)));
> + }
These test functions leak the strings allocated by
g_strdup_printf() (the leaks show up if you run 'make check'
under the clang leak sanitizer). I recommend adjusting the
scope of the variables so you can do
g_autofree char *path = g_strdup_printf(....);
and have it automatically freed as it goes out of scope.
thanks
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 6/8] hw/i2c: fix VOUT_MODE representation on little-endian machines
2026-07-29 23:13 [PATCH 0/8] hw/i2c: PMBus updates and adm1266 fixes Titus Rwantare
` (4 preceding siblings ...)
2026-07-29 23:13 ` [PATCH 5/8] hw/sensor: switch adm1266 to millivolts vout Titus Rwantare
@ 2026-07-29 23:13 ` Titus Rwantare
2026-07-30 12:39 ` Peter Maydell
2026-07-29 23:13 ` [PATCH 7/8] hw/sensor: adm1266: expose vout_mode over QMP Titus Rwantare
2026-07-29 23:13 ` [PATCH 8/8] hw/sensor: adm1266: set default VOUT mode Titus Rwantare
7 siblings, 1 reply; 13+ messages in thread
From: Titus Rwantare @ 2026-07-29 23:13 UTC (permalink / raw)
To: peter.maydell
Cc: qemu-arm, qemu-devel, kfting, imaginos32, wuhaotsh, philmd,
fanjason, Titus Rwantare
From: Jason Fan <fanjason@google.com>
Even though VOUT_MODE is only one byte, bitfields are allocated in the
order specific to the host's endianness.
Signed-off-by: Titus Rwantare <titusr@google.com>
Signed-off-by: Jason Fan <fanjason@google.com>
---
include/hw/i2c/pmbus_device.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h
index 9f3569e997..03b2c21c19 100644
--- a/include/hw/i2c/pmbus_device.h
+++ b/include/hw/i2c/pmbus_device.h
@@ -449,10 +449,17 @@ typedef struct PMBusCoefficients {
/**
* VOUT_Mode bit fields
*/
+#if HOST_BIG_ENDIAN
typedef struct PMBusVoutMode {
uint8_t mode:3;
int8_t exp:5;
} PMBusVoutMode;
+# else
+typedef struct PMBusVoutMode {
+ int8_t exp:5;
+ uint8_t mode:3;
+} PMBusVoutMode;
+#endif
/**
* Convert sensor values to direct mode format
--
2.55.0.508.g3f0d502094-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 6/8] hw/i2c: fix VOUT_MODE representation on little-endian machines
2026-07-29 23:13 ` [PATCH 6/8] hw/i2c: fix VOUT_MODE representation on little-endian machines Titus Rwantare
@ 2026-07-30 12:39 ` Peter Maydell
0 siblings, 0 replies; 13+ messages in thread
From: Peter Maydell @ 2026-07-30 12:39 UTC (permalink / raw)
To: Titus Rwantare
Cc: qemu-arm, qemu-devel, kfting, imaginos32, wuhaotsh, philmd,
fanjason
On Thu, 30 Jul 2026 at 00:13, Titus Rwantare <titusr@google.com> wrote:
>
> From: Jason Fan <fanjason@google.com>
>
> Even though VOUT_MODE is only one byte, bitfields are allocated in the
> order specific to the host's endianness.
>
> Signed-off-by: Titus Rwantare <titusr@google.com>
> Signed-off-by: Jason Fan <fanjason@google.com>
> ---
> include/hw/i2c/pmbus_device.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h
> index 9f3569e997..03b2c21c19 100644
> --- a/include/hw/i2c/pmbus_device.h
> +++ b/include/hw/i2c/pmbus_device.h
> @@ -449,10 +449,17 @@ typedef struct PMBusCoefficients {
> /**
> * VOUT_Mode bit fields
> */
> +#if HOST_BIG_ENDIAN
> typedef struct PMBusVoutMode {
> uint8_t mode:3;
> int8_t exp:5;
> } PMBusVoutMode;
> +# else
> +typedef struct PMBusVoutMode {
> + int8_t exp:5;
> + uint8_t mode:3;
> +} PMBusVoutMode;
> +#endif
This is why where possible we don't use C bitfields for guest
data structures. Better to use the FIELD macros from
include/hw/core/registerfields.h.
thanks
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 7/8] hw/sensor: adm1266: expose vout_mode over QMP
2026-07-29 23:13 [PATCH 0/8] hw/i2c: PMBus updates and adm1266 fixes Titus Rwantare
` (5 preceding siblings ...)
2026-07-29 23:13 ` [PATCH 6/8] hw/i2c: fix VOUT_MODE representation on little-endian machines Titus Rwantare
@ 2026-07-29 23:13 ` Titus Rwantare
2026-07-29 23:13 ` [PATCH 8/8] hw/sensor: adm1266: set default VOUT mode Titus Rwantare
7 siblings, 0 replies; 13+ messages in thread
From: Titus Rwantare @ 2026-07-29 23:13 UTC (permalink / raw)
To: peter.maydell
Cc: qemu-arm, qemu-devel, kfting, imaginos32, wuhaotsh, philmd,
fanjason, Titus Rwantare
The vout_mode registers can now be adjusted to allow a wider value range
in read_vout.
vout_mode is per pmbus page, therefore vout_mode[n] will affect
read_vout[n].
Signed-off-by: Titus Rwantare <titusr@google.com>
---
hw/sensor/adm1266.c | 8 ++++++++
tests/qtest/adm1266-test.c | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+)
diff --git a/hw/sensor/adm1266.c b/hw/sensor/adm1266.c
index 80960dc1c4..1464307971 100644
--- a/hw/sensor/adm1266.c
+++ b/hw/sensor/adm1266.c
@@ -271,6 +271,8 @@ static void adm1266_get(Object *obj, Visitor *v, const char *name, void *opaque,
sscanf(name, "vout[%u]", &index);
mode = (PMBusVoutMode *)&pmdev->pages[index].vout_mode;
value = pmbus_linear_mode2milliunits(*(uint16_t *)opaque, mode->exp);
+ } else if (strncmp(name, "vout_mode", 9) == 0) {
+ value = *(uint8_t *)opaque;
} else {
value = *(uint16_t *)opaque;
}
@@ -293,6 +295,8 @@ static void adm1266_set(Object *obj, Visitor *v, const char *name, void *opaque,
sscanf(name, "vout[%u]", &index);
mode = (PMBusVoutMode *)&pmdev->pages[index].vout_mode;
*internal = pmbus_milliunits2linear_mode(value, mode->exp);
+ } else if (strncmp(name, "vout_mode", 9) == 0) {
+ *(uint8_t *)opaque = value;
} else {
*internal = value;
}
@@ -321,6 +325,10 @@ static void adm1266_init(Object *obj)
object_property_add(obj, "vout[*]", "uint32",
adm1266_get,
adm1266_set, NULL, &pmdev->pages[i].read_vout);
+
+ object_property_add(obj, "vout_mode[*]", "uint32",
+ adm1266_get,
+ adm1266_set, NULL, &pmdev->pages[i].vout_mode);
}
}
diff --git a/tests/qtest/adm1266-test.c b/tests/qtest/adm1266-test.c
index fd3d8079b6..9cdd206c3f 100644
--- a/tests/qtest/adm1266-test.c
+++ b/tests/qtest/adm1266-test.c
@@ -186,6 +186,40 @@ static void test_vout_mode_exponent(void *obj, void *data,
}
}
+static void test_vout_mode_qmp(void *obj, void *data,
+ QGuestAllocator *alloc)
+{
+ uint16_t i2c_value, value, expected;
+ QI2CDevice *i2cdev = (QI2CDevice *)obj;
+ ADM1266VoutMode m;
+ char *path;
+
+ /* set a different exponent per page and a different value */
+ for (int i = 0; i < ADM1266_NUM_PAGES; i++) {
+ expected = 1000 * (i * 2);
+ m.mode.exp = i - 14;
+ path = g_strdup_printf("vout_mode[%d]", i);
+ qmp_adm1266_set(TEST_ID, path, m.raw);
+ path = g_strdup_printf("vout[%d]", i);
+ qmp_adm1266_set(TEST_ID, path, expected);
+ }
+
+ for (int i = 0; i < ADM1266_NUM_PAGES; i++) {
+ i2c_set8(i2cdev, PMBUS_PAGE, i);
+ expected = 1000 * (i * 2);
+ /* check correct value from i2c*/
+ m.raw = i2c_get8(i2cdev, PMBUS_VOUT_MODE);
+ i2c_value = bswap16(i2c_get16(i2cdev, PMBUS_READ_VOUT));
+ i2c_value = adm1266_linear_mode2milliunits(i2c_value, m.mode.exp);
+ g_assert_cmpuint(i2c_value, ==, expected);
+
+ /* check correct value from qmp*/
+ path = g_strdup_printf("vout[%d]", i);
+ value = qmp_adm1266_get(TEST_ID, path);
+ g_assert_cmpuint(value, ==, expected);
+ }
+}
+
static void test_vout_clamp_to_max(void *obj, void *data,
QGuestAllocator *alloc)
{
@@ -278,6 +312,7 @@ static void adm1266_register_nodes(void)
test_vout_mode_exponent, NULL);
qos_add_test("test_vout_clamp_to_max", "adm1266",
test_vout_clamp_to_max, NULL);
+ qos_add_test("test_vout_mode_qmp", "adm1266", test_vout_mode_qmp, NULL);
}
libqos_init(adm1266_register_nodes);
--
2.55.0.508.g3f0d502094-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 8/8] hw/sensor: adm1266: set default VOUT mode
2026-07-29 23:13 [PATCH 0/8] hw/i2c: PMBus updates and adm1266 fixes Titus Rwantare
` (6 preceding siblings ...)
2026-07-29 23:13 ` [PATCH 7/8] hw/sensor: adm1266: expose vout_mode over QMP Titus Rwantare
@ 2026-07-29 23:13 ` Titus Rwantare
7 siblings, 0 replies; 13+ messages in thread
From: Titus Rwantare @ 2026-07-29 23:13 UTC (permalink / raw)
To: peter.maydell
Cc: qemu-arm, qemu-devel, kfting, imaginos32, wuhaotsh, philmd,
fanjason, Titus Rwantare
VOUT_MODE 0 represents linear mode, and the exponent is set to -10, to
allow us to fit 3.3V in the register.
Signed-off-by: Titus Rwantare <titusr@google.com>
---
hw/sensor/adm1266.c | 17 ++++++++++++-----
tests/qtest/adm1266-test.c | 2 +-
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/hw/sensor/adm1266.c b/hw/sensor/adm1266.c
index 1464307971..c0c6341876 100644
--- a/hw/sensor/adm1266.c
+++ b/hw/sensor/adm1266.c
@@ -42,6 +42,8 @@ OBJECT_DECLARE_SIMPLE_TYPE(ADM1266State, ADM1266)
#define ADM1266_MFR_LOCATION_DEFAULT "0000"
#define ADM1266_MFR_DATE_DEFAULT "0000"
#define ADM1266_MFR_SERIAL_DEFAULT "0000"
+#define ADM1266_VOUT_MODE_EXP_DEFAULT -10
+
#define ADM1266_NUM_PAGES 17
#define ADM1266_READ_LENGTH_DEFAULT 48
@@ -85,6 +87,10 @@ static void adm1266_exit_reset(Object *obj, ResetType type)
{
ADM1266State *s = ADM1266(obj);
PMBusDevice *pmdev = PMBUS_DEVICE(obj);
+ PMBusVoutMode m = {
+ .mode = 0,
+ .exp = ADM1266_VOUT_MODE_EXP_DEFAULT
+ };
pmdev->page = 0;
pmdev->capability = ADM1266_CAPABILITY_NO_PEC;
@@ -92,11 +98,12 @@ static void adm1266_exit_reset(Object *obj, ResetType type)
for (int i = 0; i < ADM1266_NUM_PAGES; i++) {
pmdev->pages[i].operation = ADM1266_OPERATION_DEFAULT;
pmdev->pages[i].revision = ADM1266_PMBUS_REVISION_DEFAULT;
- pmdev->pages[i].vout_mode = 0;
- pmdev->pages[i].read_vout = pmbus_data2linear_mode(12, 0);
- pmdev->pages[i].vout_margin_high = pmbus_data2linear_mode(15, 0);
- pmdev->pages[i].vout_margin_low = pmbus_data2linear_mode(3, 0);
- pmdev->pages[i].vout_ov_fault_limit = pmbus_data2linear_mode(16, 0);
+
+ pmdev->pages[i].vout_mode = *(uint8_t *)&m;
+ pmdev->pages[i].read_vout = pmbus_data2linear_mode(12, m.exp);
+ pmdev->pages[i].vout_margin_high = pmbus_data2linear_mode(15, m.exp);
+ pmdev->pages[i].vout_margin_low = pmbus_data2linear_mode(3, m.exp);
+ pmdev->pages[i].vout_ov_fault_limit = pmbus_data2linear_mode(16, m.exp);
pmdev->pages[i].revision = ADM1266_PMBUS_REVISION_DEFAULT;
}
diff --git a/tests/qtest/adm1266-test.c b/tests/qtest/adm1266-test.c
index 9cdd206c3f..6fcf2505cf 100644
--- a/tests/qtest/adm1266-test.c
+++ b/tests/qtest/adm1266-test.c
@@ -47,7 +47,7 @@
#define TEST_STRING_C "rev c"
#define ADM1266_NUM_PAGES 17
-#define ADM1266_MAX_VALUE 65535000
+#define ADM1266_MAX_VALUE 63999 /* UINT16_MAX * 2^-10 */
typedef union {
uint8_t raw;
--
2.55.0.508.g3f0d502094-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread