* [PATCH v3 1/8] hw/i2c: pmbus add support for block receive
2023-10-23 18:08 [PATCH v3 0/8] PMBus fixes and new functions Titus Rwantare
@ 2023-10-23 18:08 ` Titus Rwantare
2023-10-23 18:08 ` [PATCH v3 2/8] hw/i2c: pmbus: add vout mode bitfields Titus Rwantare
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Titus Rwantare @ 2023-10-23 18:08 UTC (permalink / raw)
To: qemu-arm, qemu-devel, minyard, philmd; +Cc: Titus Rwantare, Hao Wu
PMBus devices can send and receive variable length data using the
block read and write format, with the first byte in the payload
denoting the length.
This is mostly used for strings and on-device logs. Devices can
respond to a block read with an empty string.
Reviewed-by: Hao Wu <wuhaotsh@google.com>
Signed-off-by: Titus Rwantare <titusr@google.com>
---
hw/i2c/pmbus_device.c | 30 +++++++++++++++++++++++++++++-
include/hw/i2c/pmbus_device.h | 7 +++++++
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index cef51663d0..ea15490720 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -102,7 +102,6 @@ void pmbus_send_string(PMBusDevice *pmdev, const char *data)
}
size_t len = strlen(data);
- g_assert(len > 0);
g_assert(len + pmdev->out_buf_len < SMBUS_DATA_MAX_LEN);
pmdev->out_buf[len + pmdev->out_buf_len] = len;
@@ -112,6 +111,35 @@ void pmbus_send_string(PMBusDevice *pmdev, const char *data)
pmdev->out_buf_len += len + 1;
}
+uint8_t pmbus_receive_block(PMBusDevice *pmdev, uint8_t *dest, size_t len)
+{
+ /* dest may contain data from previous writes */
+ memset(dest, 0, len);
+
+ /* Exclude command code from return value */
+ pmdev->in_buf++;
+ pmdev->in_buf_len--;
+
+ /* The byte after the command code denotes the length */
+ uint8_t sent_len = pmdev->in_buf[0];
+
+ if (sent_len != pmdev->in_buf_len - 1) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: length mismatch. Expected %d bytes, got %d bytes\n",
+ __func__, sent_len, pmdev->in_buf_len - 1);
+ }
+
+ /* exclude length byte */
+ pmdev->in_buf++;
+ pmdev->in_buf_len--;
+
+ if (pmdev->in_buf_len < len) {
+ len = pmdev->in_buf_len;
+ }
+ memcpy(dest, pmdev->in_buf, len);
+ return len;
+}
+
static uint64_t pmbus_receive_uint(PMBusDevice *pmdev)
{
diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h
index 93f5d57c9d..7dc00cc4d9 100644
--- a/include/hw/i2c/pmbus_device.h
+++ b/include/hw/i2c/pmbus_device.h
@@ -501,6 +501,13 @@ void pmbus_send64(PMBusDevice *state, uint64_t data);
*/
void pmbus_send_string(PMBusDevice *state, const char *data);
+/**
+ * @brief Receive data sent with Block Write.
+ * @param dest - memory with enough capacity to receive the write
+ * @param len - the capacity of dest
+ */
+uint8_t pmbus_receive_block(PMBusDevice *pmdev, uint8_t *dest, size_t len);
+
/**
* @brief Receive data over PMBus
* These methods help track how much data is being received over PMBus
--
2.42.0.758.gaed0368e0e-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 2/8] hw/i2c: pmbus: add vout mode bitfields
2023-10-23 18:08 [PATCH v3 0/8] PMBus fixes and new functions Titus Rwantare
2023-10-23 18:08 ` [PATCH v3 1/8] hw/i2c: pmbus add support for block receive Titus Rwantare
@ 2023-10-23 18:08 ` Titus Rwantare
2023-10-23 18:08 ` [PATCH v3 3/8] hw/i2c: pmbus: add fan support Titus Rwantare
` (6 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Titus Rwantare @ 2023-10-23 18:08 UTC (permalink / raw)
To: qemu-arm, qemu-devel, minyard, philmd; +Cc: Titus Rwantare, Hao Wu
The VOUT_MODE command is described in the PMBus Specification,
Part II, Ver 1.3 Section 8.3
VOUT_MODE has a three bit mode and 4 bit parameter, the three bit
mode determines whether voltages are formatted as uint16, uint16,
VID, and Direct modes. VID and Direct modes use the remaining 5 bits
to scale the voltage readings.
Reviewed-by: Hao Wu <wuhaotsh@google.com>
Signed-off-by: Titus Rwantare <titusr@google.com>
---
include/hw/i2c/pmbus_device.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h
index 7dc00cc4d9..2e95164aa1 100644
--- a/include/hw/i2c/pmbus_device.h
+++ b/include/hw/i2c/pmbus_device.h
@@ -444,6 +444,14 @@ typedef struct PMBusCoefficients {
int32_t R; /* exponent */
} PMBusCoefficients;
+/**
+ * VOUT_Mode bit fields
+ */
+typedef struct PMBusVoutMode {
+ uint8_t mode:3;
+ int8_t exp:5;
+} PMBusVoutMode;
+
/**
* Convert sensor values to direct mode format
*
--
2.42.0.758.gaed0368e0e-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 3/8] hw/i2c: pmbus: add fan support
2023-10-23 18:08 [PATCH v3 0/8] PMBus fixes and new functions Titus Rwantare
2023-10-23 18:08 ` [PATCH v3 1/8] hw/i2c: pmbus add support for block receive Titus Rwantare
2023-10-23 18:08 ` [PATCH v3 2/8] hw/i2c: pmbus: add vout mode bitfields Titus Rwantare
@ 2023-10-23 18:08 ` Titus Rwantare
2023-10-23 18:08 ` [PATCH v3 4/8] hw/i2c: pmbus: add VCAP register Titus Rwantare
` (5 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Titus Rwantare @ 2023-10-23 18:08 UTC (permalink / raw)
To: qemu-arm, qemu-devel, minyard, philmd; +Cc: Titus Rwantare, Stephen Longfield
PMBus devices may integrate fans whose operation is configurable
over PMBus. This commit allows the driver to read and write the
fan control registers but does not model the operation of fans.
Reviewed-by: Stephen Longfield <slongfield@google.com>
Signed-off-by: Titus Rwantare <titusr@google.com>
---
hw/i2c/pmbus_device.c | 176 ++++++++++++++++++++++++++++++++++
include/hw/i2c/pmbus_device.h | 1 +
2 files changed, 177 insertions(+)
diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index ea15490720..c1d8c93056 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -500,6 +500,54 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd)
}
break;
+ case PMBUS_FAN_CONFIG_1_2: /* R/W byte */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send8(pmdev, pmdev->pages[index].fan_config_1_2);
+ } else {
+ goto passthough;
+ }
+ break;
+
+ case PMBUS_FAN_COMMAND_1: /* R/W word */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send16(pmdev, pmdev->pages[index].fan_command_1);
+ } else {
+ goto passthough;
+ }
+ break;
+
+ case PMBUS_FAN_COMMAND_2: /* R/W word */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send16(pmdev, pmdev->pages[index].fan_command_2);
+ } else {
+ goto passthough;
+ }
+ break;
+
+ case PMBUS_FAN_CONFIG_3_4: /* R/W byte */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send8(pmdev, pmdev->pages[index].fan_config_3_4);
+ } else {
+ goto passthough;
+ }
+ break;
+
+ case PMBUS_FAN_COMMAND_3: /* R/W word */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send16(pmdev, pmdev->pages[index].fan_command_3);
+ } else {
+ goto passthough;
+ }
+ break;
+
+ case PMBUS_FAN_COMMAND_4: /* R/W word */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send16(pmdev, pmdev->pages[index].fan_command_4);
+ } else {
+ goto passthough;
+ }
+ break;
+
case PMBUS_VOUT_OV_FAULT_LIMIT: /* R/W word */
if (pmdev->pages[index].page_flags & PB_HAS_VOUT) {
pmbus_send16(pmdev, pmdev->pages[index].vout_ov_fault_limit);
@@ -810,6 +858,22 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd)
pmbus_send8(pmdev, pmdev->pages[index].status_mfr_specific);
break;
+ case PMBUS_STATUS_FANS_1_2: /* R/W byte */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send8(pmdev, pmdev->pages[index].status_fans_1_2);
+ } else {
+ goto passthough;
+ }
+ break;
+
+ case PMBUS_STATUS_FANS_3_4: /* R/W byte */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send8(pmdev, pmdev->pages[index].status_fans_3_4);
+ } else {
+ goto passthough;
+ }
+ break;
+
case PMBUS_READ_EIN: /* Read-Only block 5 bytes */
if (pmdev->pages[index].page_flags & PB_HAS_EIN) {
pmbus_send(pmdev, pmdev->pages[index].read_ein, 5);
@@ -882,6 +946,54 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd)
}
break;
+ case PMBUS_READ_FAN_SPEED_1: /* Read-Only word */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send16(pmdev, pmdev->pages[index].read_fan_speed_1);
+ } else {
+ goto passthough;
+ }
+ break;
+
+ case PMBUS_READ_FAN_SPEED_2: /* Read-Only word */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send16(pmdev, pmdev->pages[index].read_fan_speed_2);
+ } else {
+ goto passthough;
+ }
+ break;
+
+ case PMBUS_READ_FAN_SPEED_3: /* Read-Only word */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send16(pmdev, pmdev->pages[index].read_fan_speed_3);
+ } else {
+ goto passthough;
+ }
+ break;
+
+ case PMBUS_READ_FAN_SPEED_4: /* Read-Only word */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send16(pmdev, pmdev->pages[index].read_fan_speed_4);
+ } else {
+ goto passthough;
+ }
+ break;
+
+ case PMBUS_READ_DUTY_CYCLE: /* Read-Only word */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send16(pmdev, pmdev->pages[index].read_duty_cycle);
+ } else {
+ goto passthough;
+ }
+ break;
+
+ case PMBUS_READ_FREQUENCY: /* Read-Only word */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send16(pmdev, pmdev->pages[index].read_frequency);
+ } else {
+ goto passthough;
+ }
+ break;
+
case PMBUS_READ_POUT: /* Read-Only word */
if (pmdev->pages[index].page_flags & PB_HAS_POUT) {
pmbus_send16(pmdev, pmdev->pages[index].read_pout);
@@ -1305,6 +1417,54 @@ static int pmbus_write_data(SMBusDevice *smd, uint8_t *buf, uint8_t len)
}
break;
+ case PMBUS_FAN_CONFIG_1_2: /* R/W byte */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmdev->pages[index].fan_config_1_2 = pmbus_receive8(pmdev);
+ } else {
+ goto passthrough;
+ }
+ break;
+
+ case PMBUS_FAN_COMMAND_1: /* R/W word */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmdev->pages[index].fan_command_1 = pmbus_receive16(pmdev);
+ } else {
+ goto passthrough;
+ }
+ break;
+
+ case PMBUS_FAN_COMMAND_2: /* R/W word */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmdev->pages[index].fan_command_2 = pmbus_receive16(pmdev);
+ } else {
+ goto passthrough;
+ }
+ break;
+
+ case PMBUS_FAN_CONFIG_3_4: /* R/W byte */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmdev->pages[index].fan_config_3_4 = pmbus_receive8(pmdev);
+ } else {
+ goto passthrough;
+ }
+ break;
+
+ case PMBUS_FAN_COMMAND_3: /* R/W word */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmdev->pages[index].fan_command_3 = pmbus_receive16(pmdev);
+ } else {
+ goto passthrough;
+ }
+ break;
+
+ case PMBUS_FAN_COMMAND_4: /* R/W word */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmdev->pages[index].fan_command_4 = pmbus_receive16(pmdev);
+ } else {
+ goto passthrough;
+ }
+ break;
+
case PMBUS_VOUT_OV_FAULT_LIMIT: /* R/W word */
if (pmdev->pages[index].page_flags & PB_HAS_VOUT) {
pmdev->pages[index].vout_ov_fault_limit = pmbus_receive16(pmdev);
@@ -1610,6 +1770,22 @@ static int pmbus_write_data(SMBusDevice *smd, uint8_t *buf, uint8_t len)
pmdev->pages[index].status_mfr_specific = pmbus_receive8(pmdev);
break;
+ case PMBUS_STATUS_FANS_1_2: /* R/W byte */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send8(pmdev, pmdev->pages[index].status_fans_1_2);
+ } else {
+ goto passthrough;
+ }
+ break;
+
+ case PMBUS_STATUS_FANS_3_4: /* R/W byte */
+ if (pmdev->pages[index].page_flags & PB_HAS_FAN) {
+ pmbus_send8(pmdev, pmdev->pages[index].status_fans_3_4);
+ } else {
+ goto passthrough;
+ }
+ break;
+
case PMBUS_PAGE_PLUS_READ: /* Block Read-only */
case PMBUS_CAPABILITY: /* Read-Only byte */
case PMBUS_COEFFICIENTS: /* Read-only block 5 bytes */
diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h
index 2e95164aa1..ad431bdc7c 100644
--- a/include/hw/i2c/pmbus_device.h
+++ b/include/hw/i2c/pmbus_device.h
@@ -258,6 +258,7 @@ OBJECT_DECLARE_TYPE(PMBusDevice, PMBusDeviceClass,
#define PB_HAS_TEMP2 BIT_ULL(41)
#define PB_HAS_TEMP3 BIT_ULL(42)
#define PB_HAS_TEMP_RATING BIT_ULL(43)
+#define PB_HAS_FAN BIT_ULL(44)
#define PB_HAS_MFR_INFO BIT_ULL(50)
#define PB_HAS_STATUS_MFR_SPECIFIC BIT_ULL(51)
--
2.42.0.758.gaed0368e0e-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 4/8] hw/i2c: pmbus: add VCAP register
2023-10-23 18:08 [PATCH v3 0/8] PMBus fixes and new functions Titus Rwantare
` (2 preceding siblings ...)
2023-10-23 18:08 ` [PATCH v3 3/8] hw/i2c: pmbus: add fan support Titus Rwantare
@ 2023-10-23 18:08 ` Titus Rwantare
2023-10-23 18:08 ` [PATCH v3 5/8] hw/sensor: add ADM1266 device model Titus Rwantare
` (4 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Titus Rwantare @ 2023-10-23 18:08 UTC (permalink / raw)
To: qemu-arm, qemu-devel, minyard, philmd; +Cc: Titus Rwantare, Benjamin Streb
VCAP is a register for devices with energy storage capacitors.
Reviewed-by: Benjamin Streb <bstreb@google.com>
Signed-off-by: Titus Rwantare <titusr@google.com>
---
hw/i2c/pmbus_device.c | 8 ++++++++
include/hw/i2c/pmbus_device.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index c1d8c93056..3bce39e84e 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -906,6 +906,14 @@ static uint8_t pmbus_receive_byte(SMBusDevice *smd)
}
break;
+ case PMBUS_READ_VCAP: /* Read-Only word */
+ if (pmdev->pages[index].page_flags & PB_HAS_VCAP) {
+ pmbus_send16(pmdev, pmdev->pages[index].read_vcap);
+ } else {
+ goto passthough;
+ }
+ break;
+
case PMBUS_READ_VOUT: /* Read-Only word */
if (pmdev->pages[index].page_flags & PB_HAS_VOUT) {
pmbus_send16(pmdev, pmdev->pages[index].read_vout);
diff --git a/include/hw/i2c/pmbus_device.h b/include/hw/i2c/pmbus_device.h
index ad431bdc7c..f195c11384 100644
--- a/include/hw/i2c/pmbus_device.h
+++ b/include/hw/i2c/pmbus_device.h
@@ -243,6 +243,7 @@ OBJECT_DECLARE_TYPE(PMBusDevice, PMBusDeviceClass,
#define PB_HAS_VIN_RATING BIT_ULL(13)
#define PB_HAS_VOUT_RATING BIT_ULL(14)
#define PB_HAS_VOUT_MODE BIT_ULL(15)
+#define PB_HAS_VCAP BIT_ULL(16)
#define PB_HAS_IOUT BIT_ULL(21)
#define PB_HAS_IIN BIT_ULL(22)
#define PB_HAS_IOUT_RATING BIT_ULL(23)
--
2.42.0.758.gaed0368e0e-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 5/8] hw/sensor: add ADM1266 device model
2023-10-23 18:08 [PATCH v3 0/8] PMBus fixes and new functions Titus Rwantare
` (3 preceding siblings ...)
2023-10-23 18:08 ` [PATCH v3 4/8] hw/i2c: pmbus: add VCAP register Titus Rwantare
@ 2023-10-23 18:08 ` Titus Rwantare
2023-10-23 18:08 ` [PATCH v3 6/8] tests/qtest: add tests for ADM1266 Titus Rwantare
` (3 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Titus Rwantare @ 2023-10-23 18:08 UTC (permalink / raw)
To: qemu-arm, qemu-devel, minyard, philmd; +Cc: Titus Rwantare, Hao Wu
The ADM1266 is a cascadable super sequencer with margin control and
fault recording.
This commit adds basic support for its PMBus commands and models
the identification registers that can be modified in a firmware
update.
Reviewed-by: Hao Wu <wuhaotsh@google.com>
Signed-off-by: Titus Rwantare <titusr@google.com>
---
hw/arm/Kconfig | 1 +
hw/sensor/Kconfig | 5 +
hw/sensor/adm1266.c | 254 ++++++++++++++++++++++++++++++++++++++++++
hw/sensor/meson.build | 1 +
4 files changed, 261 insertions(+)
create mode 100644 hw/sensor/adm1266.c
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 7e68348440..b1e8c0e2ac 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -488,6 +488,7 @@ config NPCM7XX
default y
depends on TCG && ARM
select A9MPCORE
+ select ADM1266
select ADM1272
select ARM_GIC
select SMBUS
diff --git a/hw/sensor/Kconfig b/hw/sensor/Kconfig
index e03bd09b50..bc6331b4ab 100644
--- a/hw/sensor/Kconfig
+++ b/hw/sensor/Kconfig
@@ -22,6 +22,11 @@ config ADM1272
bool
depends on I2C
+config ADM1266
+ bool
+ depends on PMBUS
+ default y if PMBUS
+
config MAX34451
bool
depends on I2C
diff --git a/hw/sensor/adm1266.c b/hw/sensor/adm1266.c
new file mode 100644
index 0000000000..5ae4f82ba1
--- /dev/null
+++ b/hw/sensor/adm1266.c
@@ -0,0 +1,254 @@
+/*
+ * Analog Devices ADM1266 Cascadable Super Sequencer with Margin Control and
+ * Fault Recording with PMBus
+ *
+ * https://www.analog.com/media/en/technical-documentation/data-sheets/adm1266.pdf
+ *
+ * Copyright 2023 Google LLC
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/i2c/pmbus_device.h"
+#include "hw/irq.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+
+#define TYPE_ADM1266 "adm1266"
+OBJECT_DECLARE_SIMPLE_TYPE(ADM1266State, ADM1266)
+
+#define ADM1266_BLACKBOX_CONFIG 0xD3
+#define ADM1266_PDIO_CONFIG 0xD4
+#define ADM1266_READ_STATE 0xD9
+#define ADM1266_READ_BLACKBOX 0xDE
+#define ADM1266_SET_RTC 0xDF
+#define ADM1266_GPIO_SYNC_CONFIGURATION 0xE1
+#define ADM1266_BLACKBOX_INFORMATION 0xE6
+#define ADM1266_PDIO_STATUS 0xE9
+#define ADM1266_GPIO_STATUS 0xEA
+
+/* Defaults */
+#define ADM1266_OPERATION_DEFAULT 0x80
+#define ADM1266_CAPABILITY_DEFAULT 0xA0
+#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_NUM_PAGES 17
+/**
+ * PAGE Index
+ * Page 0 VH1.
+ * Page 1 VH2.
+ * Page 2 VH3.
+ * Page 3 VH4.
+ * Page 4 VP1.
+ * Page 5 VP2.
+ * Page 6 VP3.
+ * Page 7 VP4.
+ * Page 8 VP5.
+ * Page 9 VP6.
+ * Page 10 VP7.
+ * Page 11 VP8.
+ * Page 12 VP9.
+ * Page 13 VP10.
+ * Page 14 VP11.
+ * Page 15 VP12.
+ * Page 16 VP13.
+ */
+typedef struct ADM1266State {
+ PMBusDevice parent;
+
+ char mfr_id[32];
+ char mfr_model[32];
+ char mfr_rev[8];
+} ADM1266State;
+
+static const uint8_t adm1266_ic_device_id[] = {0x03, 0x41, 0x12, 0x66};
+static const uint8_t adm1266_ic_device_rev[] = {0x08, 0x01, 0x08, 0x07, 0x0,
+ 0x0, 0x07, 0x41, 0x30};
+
+static void adm1266_exit_reset(Object *obj)
+{
+ ADM1266State *s = ADM1266(obj);
+ PMBusDevice *pmdev = PMBUS_DEVICE(obj);
+
+ pmdev->page = 0;
+ pmdev->capability = ADM1266_CAPABILITY_NO_PEC;
+
+ 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].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);
+}
+
+static uint8_t adm1266_read_byte(PMBusDevice *pmdev)
+{
+ ADM1266State *s = ADM1266(pmdev);
+
+ switch (pmdev->code) {
+ case PMBUS_MFR_ID: /* R/W block */
+ pmbus_send_string(pmdev, s->mfr_id);
+ break;
+
+ case PMBUS_MFR_MODEL: /* R/W block */
+ pmbus_send_string(pmdev, s->mfr_model);
+ break;
+
+ case PMBUS_MFR_REVISION: /* R/W block */
+ pmbus_send_string(pmdev, s->mfr_rev);
+ break;
+
+ case PMBUS_IC_DEVICE_ID:
+ pmbus_send(pmdev, adm1266_ic_device_id, sizeof(adm1266_ic_device_id));
+ break;
+
+ case PMBUS_IC_DEVICE_REV:
+ pmbus_send(pmdev, adm1266_ic_device_rev, sizeof(adm1266_ic_device_rev));
+ break;
+
+ default:
+ qemu_log_mask(LOG_UNIMP,
+ "%s: reading from unimplemented register: 0x%02x\n",
+ __func__, pmdev->code);
+ return 0xFF;
+ }
+
+ return 0;
+}
+
+static int adm1266_write_data(PMBusDevice *pmdev, const uint8_t *buf,
+ uint8_t len)
+{
+ ADM1266State *s = ADM1266(pmdev);
+
+ switch (pmdev->code) {
+ case PMBUS_MFR_ID: /* R/W block */
+ pmbus_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));
+ break;
+
+ case PMBUS_MFR_REVISION: /* R/W block*/
+ pmbus_receive_block(pmdev, (uint8_t *)s->mfr_rev, sizeof(s->mfr_rev));
+ break;
+
+ case ADM1266_SET_RTC: /* do nothing */
+ break;
+
+ default:
+ qemu_log_mask(LOG_UNIMP,
+ "%s: writing to unimplemented register: 0x%02x\n",
+ __func__, pmdev->code);
+ break;
+ }
+ return 0;
+}
+
+static void adm1266_get(Object *obj, Visitor *v, const char *name, void *opaque,
+ Error **errp)
+{
+ uint16_t value;
+ 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);
+ } else {
+ value = *(uint16_t *)opaque;
+ }
+
+ visit_type_uint16(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;
+ PMBusDevice *pmdev = PMBUS_DEVICE(obj);
+ PMBusVoutMode *mode = (PMBusVoutMode *)&pmdev->pages[0].vout_mode;
+
+ if (!visit_type_uint16(v, name, &value, errp)) {
+ return;
+ }
+
+ *internal = pmbus_data2linear_mode(value, mode->exp);
+ pmbus_check_limits(pmdev);
+}
+
+static const VMStateDescription vmstate_adm1266 = {
+ .name = "ADM1266",
+ .version_id = 0,
+ .minimum_version_id = 0,
+ .fields = (VMStateField[]){
+ VMSTATE_PMBUS_DEVICE(parent, ADM1266State),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+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;
+
+ for (int i = 0; i < ADM1266_NUM_PAGES; i++) {
+ pmbus_page_config(pmdev, i, flags);
+
+ object_property_add(obj, "vout[*]", "uint16",
+ adm1266_get,
+ adm1266_set, NULL, &pmdev->pages[i].read_vout);
+ }
+}
+
+static void adm1266_class_init(ObjectClass *klass, void *data)
+{
+ ResettableClass *rc = RESETTABLE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ PMBusDeviceClass *k = PMBUS_DEVICE_CLASS(klass);
+
+ dc->desc = "Analog Devices ADM1266 Hot Swap controller";
+ dc->vmsd = &vmstate_adm1266;
+ k->write_data = adm1266_write_data;
+ k->receive_byte = adm1266_read_byte;
+ k->device_num_pages = 17;
+
+ rc->phases.exit = adm1266_exit_reset;
+}
+
+static const TypeInfo adm1266_info = {
+ .name = TYPE_ADM1266,
+ .parent = TYPE_PMBUS_DEVICE,
+ .instance_size = sizeof(ADM1266State),
+ .instance_init = adm1266_init,
+ .class_init = adm1266_class_init,
+};
+
+static void adm1266_register_types(void)
+{
+ type_register_static(&adm1266_info);
+}
+
+type_init(adm1266_register_types)
diff --git a/hw/sensor/meson.build b/hw/sensor/meson.build
index 30e20e27b8..420fdc3359 100644
--- a/hw/sensor/meson.build
+++ b/hw/sensor/meson.build
@@ -2,6 +2,7 @@ system_ss.add(when: 'CONFIG_TMP105', if_true: files('tmp105.c'))
system_ss.add(when: 'CONFIG_TMP421', if_true: files('tmp421.c'))
system_ss.add(when: 'CONFIG_DPS310', if_true: files('dps310.c'))
system_ss.add(when: 'CONFIG_EMC141X', if_true: files('emc141x.c'))
+system_ss.add(when: 'CONFIG_ADM1266', if_true: files('adm1266.c'))
system_ss.add(when: 'CONFIG_ADM1272', if_true: files('adm1272.c'))
system_ss.add(when: 'CONFIG_MAX34451', if_true: files('max34451.c'))
system_ss.add(when: 'CONFIG_LSM303DLHC_MAG', if_true: files('lsm303dlhc_mag.c'))
--
2.42.0.758.gaed0368e0e-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 6/8] tests/qtest: add tests for ADM1266
2023-10-23 18:08 [PATCH v3 0/8] PMBus fixes and new functions Titus Rwantare
` (4 preceding siblings ...)
2023-10-23 18:08 ` [PATCH v3 5/8] hw/sensor: add ADM1266 device model Titus Rwantare
@ 2023-10-23 18:08 ` Titus Rwantare
2023-10-23 18:08 ` [PATCH v3 7/8] hw/i2c: pmbus: immediately clear faults on request Titus Rwantare
` (2 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Titus Rwantare @ 2023-10-23 18:08 UTC (permalink / raw)
To: qemu-arm, qemu-devel, minyard, philmd; +Cc: Titus Rwantare, Hao Wu
The ADM1266 can have string fields written by the driver, so
it's worth specifically testing.
Reviewed-by: Hao Wu <wuhaotsh@google.com>
Signed-off-by: Titus Rwantare <titusr@google.com>
---
tests/qtest/adm1266-test.c | 123 +++++++++++++++++++++++++++++++++++++
tests/qtest/meson.build | 1 +
2 files changed, 124 insertions(+)
create mode 100644 tests/qtest/adm1266-test.c
diff --git a/tests/qtest/adm1266-test.c b/tests/qtest/adm1266-test.c
new file mode 100644
index 0000000000..6431a21de6
--- /dev/null
+++ b/tests/qtest/adm1266-test.c
@@ -0,0 +1,123 @@
+/*
+ * Analog Devices ADM1266 Cascadable Super Sequencer with Margin Control and
+ * Fault Recording with PMBus
+ *
+ * Copyright 2022 Google LLC
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include <math.h>
+#include "hw/i2c/pmbus_device.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 "adm1266-test"
+#define TEST_ADDR (0x12)
+
+#define ADM1266_BLACKBOX_CONFIG 0xD3
+#define ADM1266_PDIO_CONFIG 0xD4
+#define ADM1266_READ_STATE 0xD9
+#define ADM1266_READ_BLACKBOX 0xDE
+#define ADM1266_SET_RTC 0xDF
+#define ADM1266_GPIO_SYNC_CONFIGURATION 0xE1
+#define ADM1266_BLACKBOX_INFORMATION 0xE6
+#define ADM1266_PDIO_STATUS 0xE9
+#define ADM1266_GPIO_STATUS 0xEA
+
+/* Defaults */
+#define ADM1266_OPERATION_DEFAULT 0x80
+#define ADM1266_CAPABILITY_DEFAULT 0xA0
+#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 TEST_STRING_A "a sample"
+#define TEST_STRING_B "b sample"
+#define TEST_STRING_C "rev c"
+
+static void compare_string(QI2CDevice *i2cdev, uint8_t reg,
+ const char *test_str)
+{
+ uint8_t len = i2c_get8(i2cdev, reg);
+ char i2c_str[SMBUS_DATA_MAX_LEN] = {0};
+
+ i2c_read_block(i2cdev, reg, (uint8_t *)i2c_str, len);
+ g_assert_cmpstr(i2c_str, ==, test_str);
+}
+
+static void write_and_compare_string(QI2CDevice *i2cdev, uint8_t reg,
+ const char *test_str, uint8_t len)
+{
+ char buf[SMBUS_DATA_MAX_LEN] = {0};
+ buf[0] = len;
+ strncpy(buf + 1, test_str, len);
+ i2c_write_block(i2cdev, reg, (uint8_t *)buf, len + 1);
+ compare_string(i2cdev, reg, test_str);
+}
+
+static void test_defaults(void *obj, void *data, QGuestAllocator *alloc)
+{
+ uint16_t i2c_value;
+ QI2CDevice *i2cdev = (QI2CDevice *)obj;
+
+ i2c_value = i2c_get8(i2cdev, PMBUS_OPERATION);
+ g_assert_cmphex(i2c_value, ==, ADM1266_OPERATION_DEFAULT);
+
+ i2c_value = i2c_get8(i2cdev, PMBUS_REVISION);
+ g_assert_cmphex(i2c_value, ==, ADM1266_PMBUS_REVISION_DEFAULT);
+
+ compare_string(i2cdev, PMBUS_MFR_ID, ADM1266_MFR_ID_DEFAULT);
+ compare_string(i2cdev, PMBUS_MFR_MODEL, ADM1266_MFR_MODEL_DEFAULT);
+ 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)
+{
+ QI2CDevice *i2cdev = (QI2CDevice *)obj;
+
+ /* empty strings */
+ i2c_set8(i2cdev, PMBUS_MFR_ID, 0);
+ compare_string(i2cdev, PMBUS_MFR_ID, "");
+
+ i2c_set8(i2cdev, PMBUS_MFR_MODEL, 0);
+ compare_string(i2cdev, PMBUS_MFR_MODEL, "");
+
+ i2c_set8(i2cdev, PMBUS_MFR_REVISION, 0);
+ compare_string(i2cdev, PMBUS_MFR_REVISION, "");
+
+ /* test strings */
+ write_and_compare_string(i2cdev, PMBUS_MFR_ID, TEST_STRING_A,
+ sizeof(TEST_STRING_A));
+ write_and_compare_string(i2cdev, PMBUS_MFR_ID, TEST_STRING_B,
+ sizeof(TEST_STRING_B));
+ write_and_compare_string(i2cdev, PMBUS_MFR_ID, TEST_STRING_C,
+ sizeof(TEST_STRING_C));
+}
+
+static void adm1266_register_nodes(void)
+{
+ QOSGraphEdgeOptions opts = {
+ .extra_device_opts = "id=" TEST_ID ",address=0x12"
+ };
+ add_qi2c_address(&opts, &(QI2CAddress) { TEST_ADDR });
+
+ qos_node_create_driver("adm1266", i2c_device_create);
+ qos_node_consumes("adm1266", "i2c-bus", &opts);
+
+ qos_add_test("test_defaults", "adm1266", test_defaults, NULL);
+ qos_add_test("test_rw_regs", "adm1266", test_rw_regs, NULL);
+}
+
+libqos_init(adm1266_register_nodes);
+
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index d6022ebd64..7899537e78 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -241,6 +241,7 @@ qos_test_ss = ss.source_set()
qos_test_ss.add(
'ac97-test.c',
'adm1272-test.c',
+ 'adm1266-test.c',
'ds1338-test.c',
'e1000-test.c',
'eepro100-test.c',
--
2.42.0.758.gaed0368e0e-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 7/8] hw/i2c: pmbus: immediately clear faults on request
2023-10-23 18:08 [PATCH v3 0/8] PMBus fixes and new functions Titus Rwantare
` (5 preceding siblings ...)
2023-10-23 18:08 ` [PATCH v3 6/8] tests/qtest: add tests for ADM1266 Titus Rwantare
@ 2023-10-23 18:08 ` Titus Rwantare
2023-10-23 18:08 ` [PATCH v3 8/8] hw/i2c: pmbus: reset page register for out of range reads Titus Rwantare
2023-10-23 19:10 ` [PATCH v3 0/8] PMBus fixes and new functions Alex Bennée
8 siblings, 0 replies; 15+ messages in thread
From: Titus Rwantare @ 2023-10-23 18:08 UTC (permalink / raw)
To: qemu-arm, qemu-devel, minyard, philmd; +Cc: Titus Rwantare, Patrick Venture
The probing process of the generic pmbus driver generates
faults to determine if functions are available. These faults
were not always cleared resulting in probe failures.
Signed-off-by: Titus Rwantare <titusr@google.com>
Reviewed-by: Patrick Venture <venture@google.com>
---
hw/i2c/pmbus_device.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index 3bce39e84e..481e158380 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -1244,6 +1244,11 @@ static int pmbus_write_data(SMBusDevice *smd, uint8_t *buf, uint8_t len)
pmdev->in_buf = buf;
pmdev->code = buf[0]; /* PMBus command code */
+
+ if (pmdev->code == PMBUS_CLEAR_FAULTS) {
+ pmbus_clear_faults(pmdev);
+ }
+
if (len == 1) { /* Single length writes are command codes only */
return 0;
}
--
2.42.0.758.gaed0368e0e-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 8/8] hw/i2c: pmbus: reset page register for out of range reads
2023-10-23 18:08 [PATCH v3 0/8] PMBus fixes and new functions Titus Rwantare
` (6 preceding siblings ...)
2023-10-23 18:08 ` [PATCH v3 7/8] hw/i2c: pmbus: immediately clear faults on request Titus Rwantare
@ 2023-10-23 18:08 ` Titus Rwantare
2023-10-23 19:10 ` [PATCH v3 0/8] PMBus fixes and new functions Alex Bennée
8 siblings, 0 replies; 15+ messages in thread
From: Titus Rwantare @ 2023-10-23 18:08 UTC (permalink / raw)
To: qemu-arm, qemu-devel, minyard, philmd; +Cc: Titus Rwantare, Hao Wu
The linux pmbus driver scans all possible pages and does not reset the
current page after the scan, making all future page reads fail as out of range
on devices with a single page.
This change resets out of range pages immediately on write.
Also added a qtest for simultaneous writes to all pages.
Signed-off-by: Titus Rwantare <titusr@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
---
hw/i2c/pmbus_device.c | 18 +++++++++---------
tests/qtest/max34451-test.c | 24 ++++++++++++++++++++++++
2 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index 481e158380..1b978e588f 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -1255,6 +1255,15 @@ static int pmbus_write_data(SMBusDevice *smd, uint8_t *buf, uint8_t len)
if (pmdev->code == PMBUS_PAGE) {
pmdev->page = pmbus_receive8(pmdev);
+
+ if (pmdev->page > pmdev->num_pages - 1 && pmdev->page != PB_ALL_PAGES) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: page %u is out of range\n",
+ __func__, pmdev->page);
+ pmdev->page = 0; /* undefined behaviour - reset to page 0 */
+ pmbus_cml_error(pmdev);
+ return PMBUS_ERR_BYTE;
+ }
return 0;
}
@@ -1268,15 +1277,6 @@ static int pmbus_write_data(SMBusDevice *smd, uint8_t *buf, uint8_t len)
return 0;
}
- if (pmdev->page > pmdev->num_pages - 1) {
- qemu_log_mask(LOG_GUEST_ERROR,
- "%s: page %u is out of range\n",
- __func__, pmdev->page);
- pmdev->page = 0; /* undefined behaviour - reset to page 0 */
- pmbus_cml_error(pmdev);
- return PMBUS_ERR_BYTE;
- }
-
index = pmdev->page;
switch (pmdev->code) {
diff --git a/tests/qtest/max34451-test.c b/tests/qtest/max34451-test.c
index 0c98d0764c..dbf6ddc829 100644
--- a/tests/qtest/max34451-test.c
+++ b/tests/qtest/max34451-test.c
@@ -18,6 +18,7 @@
#define TEST_ID "max34451-test"
#define TEST_ADDR (0x4e)
+#define MAX34451_MFR_MODE 0xD1
#define MAX34451_MFR_VOUT_PEAK 0xD4
#define MAX34451_MFR_IOUT_PEAK 0xD5
#define MAX34451_MFR_TEMPERATURE_PEAK 0xD6
@@ -315,6 +316,28 @@ static void test_ot_faults(void *obj, void *data, QGuestAllocator *alloc)
}
}
+#define RAND_ON_OFF_CONFIG 0x12
+#define RAND_MFR_MODE 0x3456
+
+/* test writes to all pages */
+static void test_all_pages(void *obj, void *data, QGuestAllocator *alloc)
+{
+ uint16_t i2c_value;
+ QI2CDevice *i2cdev = (QI2CDevice *)obj;
+
+ i2c_set8(i2cdev, PMBUS_PAGE, PB_ALL_PAGES);
+ i2c_set8(i2cdev, PMBUS_ON_OFF_CONFIG, RAND_ON_OFF_CONFIG);
+ max34451_i2c_set16(i2cdev, MAX34451_MFR_MODE, RAND_MFR_MODE);
+
+ for (int i = 0; i < MAX34451_NUM_TEMP_DEVICES + MAX34451_NUM_PWR_DEVICES;
+ i++) {
+ i2c_value = i2c_get8(i2cdev, PMBUS_ON_OFF_CONFIG);
+ g_assert_cmphex(i2c_value, ==, RAND_ON_OFF_CONFIG);
+ i2c_value = max34451_i2c_get16(i2cdev, MAX34451_MFR_MODE);
+ g_assert_cmphex(i2c_value, ==, RAND_MFR_MODE);
+ }
+}
+
static void max34451_register_nodes(void)
{
QOSGraphEdgeOptions opts = {
@@ -332,5 +355,6 @@ static void max34451_register_nodes(void)
qos_add_test("test_ro_regs", "max34451", test_ro_regs, NULL);
qos_add_test("test_ov_faults", "max34451", test_ov_faults, NULL);
qos_add_test("test_ot_faults", "max34451", test_ot_faults, NULL);
+ qos_add_test("test_all_pages", "max34451", test_all_pages, NULL);
}
libqos_init(max34451_register_nodes);
--
2.42.0.758.gaed0368e0e-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v3 0/8] PMBus fixes and new functions
2023-10-23 18:08 [PATCH v3 0/8] PMBus fixes and new functions Titus Rwantare
` (7 preceding siblings ...)
2023-10-23 18:08 ` [PATCH v3 8/8] hw/i2c: pmbus: reset page register for out of range reads Titus Rwantare
@ 2023-10-23 19:10 ` Alex Bennée
2023-10-23 23:53 ` Titus Rwantare
8 siblings, 1 reply; 15+ messages in thread
From: Alex Bennée @ 2023-10-23 19:10 UTC (permalink / raw)
To: Titus Rwantare; +Cc: qemu-arm, minyard, philmd, qemu-devel
Titus Rwantare <titusr@google.com> writes:
> This patch series contains fixes and improvements to PMBus support in QEMU.
>
> The following has been added:
> - Support for block receive
> - Support for devices with fans
> - Support for the VCAP register for devices with onboard energy storage
> - A bitfield struct for the vout mode register, whose bits determine the formatting of several read commands in PMBus
> Fixes:
> - String read now handles now logs an error when passed an empty string
>
> This series is in preparation for some additional sensors that exercise
> this functionality that will be incoming shortly.
You seem to have missed a number of tags from previous postings:
https://qemu.readthedocs.io/en/master/devel/submitting-a-patch.html#proper-use-of-reviewed-by-tags-can-aid-review
(although I notice we only mention Reviewed-by in the docs)
You can use a tool like b4 to apply a series and collect the tags. It
will also collect the Message-Id's which are also useful.
Once you've fixed up your tags I think as the maintainer you can submit
a pull request.
>
> Thanks
>
> Changes in v3:
> - Added fixes to PMBus: page resets and fault clearing
>
> Changes in v2:
> - Expanded commit descriptions
> - Added the ADM1266 device model that uses new functions
>
> Titus Rwantare (8):
> hw/i2c: pmbus add support for block receive
> hw/i2c: pmbus: add vout mode bitfields
> hw/i2c: pmbus: add fan support
> hw/i2c: pmbus: add VCAP register
> hw/sensor: add ADM1266 device model
> tests/qtest: add tests for ADM1266
> hw/i2c: pmbus: immediately clear faults on request
> hw/i2c: pmbus: reset page register for out of range reads
>
> hw/arm/Kconfig | 1 +
> hw/i2c/pmbus_device.c | 237 +++++++++++++++++++++++++++++--
> hw/sensor/Kconfig | 5 +
> hw/sensor/adm1266.c | 254 ++++++++++++++++++++++++++++++++++
> hw/sensor/meson.build | 1 +
> include/hw/i2c/pmbus_device.h | 17 +++
> tests/qtest/adm1266-test.c | 123 ++++++++++++++++
> tests/qtest/max34451-test.c | 24 ++++
> tests/qtest/meson.build | 1 +
> 9 files changed, 653 insertions(+), 10 deletions(-)
> create mode 100644 hw/sensor/adm1266.c
> create mode 100644 tests/qtest/adm1266-test.c
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 0/8] PMBus fixes and new functions
2023-10-23 19:10 ` [PATCH v3 0/8] PMBus fixes and new functions Alex Bennée
@ 2023-10-23 23:53 ` Titus Rwantare
2023-10-24 10:06 ` Alex Bennée
0 siblings, 1 reply; 15+ messages in thread
From: Titus Rwantare @ 2023-10-23 23:53 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-arm, minyard, philmd, qemu-devel
On Mon, 23 Oct 2023 at 12:16, Alex Bennée <alex.bennee@linaro.org> wrote:
> You seem to have missed a number of tags from previous postings:
>
> https://qemu.readthedocs.io/en/master/devel/submitting-a-patch.html#proper-use-of-reviewed-by-tags-can-aid-review
>
> (although I notice we only mention Reviewed-by in the docs)
>
> You can use a tool like b4 to apply a series and collect the tags. It
> will also collect the Message-Id's which are also useful.
>
> Once you've fixed up your tags I think as the maintainer you can submit
> a pull request.
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
Thanks for the tip about b4, I spent some time getting to grips with
it and it's a huge timesaver.
I haven't quite figured out how to get it to produce and send a signed
pull request, but I don't need that just yet.
-Titus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 0/8] PMBus fixes and new functions
2023-10-23 23:53 ` Titus Rwantare
@ 2023-10-24 10:06 ` Alex Bennée
2023-10-24 11:50 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 15+ messages in thread
From: Alex Bennée @ 2023-10-24 10:06 UTC (permalink / raw)
To: Titus Rwantare; +Cc: qemu-arm, minyard, philmd, qemu-devel
Titus Rwantare <titusr@google.com> writes:
> On Mon, 23 Oct 2023 at 12:16, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>> You seem to have missed a number of tags from previous postings:
>>
>> https://qemu.readthedocs.io/en/master/devel/submitting-a-patch.html#proper-use-of-reviewed-by-tags-can-aid-review
>>
>> (although I notice we only mention Reviewed-by in the docs)
>>
>> You can use a tool like b4 to apply a series and collect the tags. It
>> will also collect the Message-Id's which are also useful.
>>
>> Once you've fixed up your tags I think as the maintainer you can submit
>> a pull request.
>>
>> --
>> Alex Bennée
>> Virtualisation Tech Lead @ Linaro
>
> Thanks for the tip about b4, I spent some time getting to grips with
> it and it's a huge timesaver.
> I haven't quite figured out how to get it to produce and send a signed
> pull request, but I don't need that just yet.
A pull request is really just a GPG signed tag that you push to a repo.
You can use the existing git tooling to create the cover letter for it.
I've included my exact steps at the end of the email but really it comes
down to:
git tag --sign your-pr-tag
git push your-pr-tag
git format-patch <series details>
git request-pull origin/master your_repo_details your-pr-tag
and finally
git send-email
My personal exact steps are integrated with my editor but are:
1 Create a branch (optional)
════════════════════════════
┌────
│ git fetch origin
│ branch="pr/$(date +%d%m%y)-${series}-${version}"
│ if test -z "$prefix" ; then
│ git checkout -b $branch origin/master
│ else
│ git checkout -b $branch HEAD
│ fi
└────
2 Apply patches from mailing list
═════════════════════════════════
┌────
│ if test -z $prefix; then
│ mkdir -p "${series}.patches"
│ b4 am -S -t -o "${series}.patches" "${id}"
│ else
│ echo "Built tree by hand"
│ fi
└────
┌────
│ if test -z $prefix; then
│ git am ${series}.patches/*.mbx
│ rm -rf ${series}.patches
│ else
│ git log --oneline origin/master..
│ fi
└────
3 Check if we are missing any review/comments or tags
═════════════════════════════════════════════════════
4 Check for unreviewed patches
══════════════════════════════
5 Check all sign-offs are there
═══════════════════════════════
6 Check we have only 1 Message-Id per commit
════════════════════════════════════════════
7 Check commits are good
════════════════════════
We need to ensure we have added our signoff and there is no — ephemera
left from commit history.
┌────
│ errors=0
│ commits=0
│ while read rev; do
│ author=$(git show -s --format='%an <%ae>' $rev)
│ body=$(git show -s --format='%B' $rev)
│ title=$(git show -s --format='%s' $rev)
│
│ # Check for Author signoff
│ if ! grep -q "^Signed-off-by: $author" <<< "$body"; then
│ errors=$((errors+1))
│ echo $(git log -1 --pretty=format:"missing author signoff - %h - %an: %s" $rev)
│ fi
│
│ # Check for my signoff (fix for quotes)
│ if ! grep -q "^Signed-off-by: $signoff" <<< "$body"; then
│ errors=$((errors+1))
│ echo $(git log -1 --pretty=format:"missing my signoff - %h - %an: %s" $rev)
│ fi
│
│ if ! grep -q "^Message-Id: " <<< "$body"; then
│ errors=$((errors+1))
│ echo $(git log -1 --pretty=format:"missing message id - %h - %an: %s" $rev)
│ fi
│
│ # check for unreviewed patches for patches I authored
│ if [ "$author" = "$signoff" ]; then
│ if ! grep -q "^Reviewed-by:" <<< "$body"; then
│ echo $(git log -1 --pretty=format:"unreviewed - %h - %an: %s" $rev)
│ fi
│ fi
│
│ # Check for stray Based-on tags
│ if grep -q "^Based-on: " <<< "$body"; then
│ errors=$((errors+1))
│ echo $(git log -1 --pretty=format:"has based-on tag - %h - %an: %s" $rev)
│ fi
│
│ # Check for stray history
│ if grep -q "^--" <<< "$body"; then
│ errors=$((errors+1))
│ echo $(git log -1 --pretty=format:"has commit history - %h - %an: %s" $rev)
│ fi
│
│ commits=$((commits+1))
│ done < <(git rev-list "origin/master..HEAD")
│
│ echo "Found $errors errors over $commits commits"
└────
8 Preparing a QEMU Pull Request
═══════════════════════════════
We have two properties here, tversion for the tag and pversion for the
iteration of the PULL. This is to account for re-rolls where we detect
and issue after tagging but before we send the PR itself.
┌────
│ (let ((tag (format
│ "pull-%s-%s-%d"
│ series
│ (format-time-string "%d%m%y")
│ tversion)))
│ (magit-tag-create tag "HEAD" "--sign")
│ tag)
└────
┌────
│ set -e
│ tag=$(git describe)
│ git push github $tag
│ if test -z $prefix; then
│ git push-ci-now gitlab $tag
│ else
│ git push-ci gitlab $tag
│ fi
│ echo "pushed $tag"
└────
┌────
│ (if (= 1 pversion)
│ "PULL"
│ (format "PULL v%d" pversion))
└────
┌────
│ if [ -d "${series}.pull" ]; then
│ rm -rf ${series}.pull
│ fi
│ git format-patch --subject-prefix="$subjprefix" --cover-letter origin/master..HEAD -p -o ${series}.pull
└────
You can use the $pull macro to fill in the details
9 And send the pull request
═══════════════════════════
Using the prefix will limit the send to just the cover letter, useful
for v2+ pull requests
┌────
│ if test -z "$prefix" ; then
│ git send-email --confirm=never --dry-run --quiet ${mailto} ${series}.pull/*
│ else
│ git send-email --confirm=never --dry-run --quiet ${mailto} ${series}.pull/0000*
│ fi
└────
┌────
│ if test -z "$prefix" ; then
│ git send-email --confirm=never --quiet ${mailto} ${series}.pull/*
│ else
│ git send-email --confirm=never --quiet ${mailto} ${series}.pull/0000*
│ fi
│ rm -rf ${series}.pull
└────
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 0/8] PMBus fixes and new functions
2023-10-24 10:06 ` Alex Bennée
@ 2023-10-24 11:50 ` Philippe Mathieu-Daudé
2023-10-24 16:07 ` Titus Rwantare
0 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-24 11:50 UTC (permalink / raw)
To: Alex Bennée, Titus Rwantare; +Cc: qemu-arm, minyard, qemu-devel
On 24/10/23 12:06, Alex Bennée wrote:
> A pull request is really just a GPG signed tag that you push to a repo.
> You can use the existing git tooling to create the cover letter for it.
>
> I've included my exact steps at the end of the email but really it comes
> down to:
>
> git tag --sign your-pr-tag
> git push your-pr-tag
> git format-patch <series details>
> git request-pull origin/master your_repo_details your-pr-tag
>
> and finally
>
> git send-email
>
> My personal exact steps are integrated with my editor but are:
> 8 Preparing a QEMU Pull Request
> ═══════════════════════════════
> 9 And send the pull request
> ═══════════════════════════
For these steps I just do:
$ git publish -b origin/master \
--pull-request --sign-pull --keyid=0xMYKEY
which uses .gitpublish from commit 08bb160e02,
calling get_maintainer.pl for each patch.
Using GSuite, I also have in ~/.gitconfig:
[sendemail]
smtpServer = smtp.gmail.com
smtpBatchSize = 1
smtpReloginDelay = 3
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 0/8] PMBus fixes and new functions
2023-10-24 11:50 ` Philippe Mathieu-Daudé
@ 2023-10-24 16:07 ` Titus Rwantare
2023-10-25 5:56 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 15+ messages in thread
From: Titus Rwantare @ 2023-10-24 16:07 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Alex Bennée, qemu-arm, minyard, qemu-devel
On Tue, 24 Oct 2023 at 04:50, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 24/10/23 12:06, Alex Bennée wrote:
>
> > A pull request is really just a GPG signed tag that you push to a repo.
> > You can use the existing git tooling to create the cover letter for it.
> >
> > I've included my exact steps at the end of the email but really it comes
> > down to:
> >
> > git tag --sign your-pr-tag
> > git push your-pr-tag
> > git format-patch <series details>
> > git request-pull origin/master your_repo_details your-pr-tag
> >
> > and finally
> >
> > git send-email
> >
> > My personal exact steps are integrated with my editor but are:
>
>
> > 8 Preparing a QEMU Pull Request
> > ═══════════════════════════════
>
> > 9 And send the pull request
> > ═══════════════════════════
>
> For these steps I just do:
>
> $ git publish -b origin/master \
> --pull-request --sign-pull --keyid=0xMYKEY
>
> which uses .gitpublish from commit 08bb160e02,
> calling get_maintainer.pl for each patch.
>
> Using GSuite, I also have in ~/.gitconfig:
>
> [sendemail]
> smtpServer = smtp.gmail.com
> smtpBatchSize = 1
> smtpReloginDelay = 3
Thanks all, I'll do some dry runs to walk through these approaches.
-Titus
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 0/8] PMBus fixes and new functions
2023-10-24 16:07 ` Titus Rwantare
@ 2023-10-25 5:56 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-25 5:56 UTC (permalink / raw)
To: Titus Rwantare; +Cc: Alex Bennée, qemu-arm, minyard, qemu-devel
On 24/10/23 18:07, Titus Rwantare wrote:
> On Tue, 24 Oct 2023 at 04:50, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> On 24/10/23 12:06, Alex Bennée wrote:
>>
>>> A pull request is really just a GPG signed tag that you push to a repo.
>>> You can use the existing git tooling to create the cover letter for it.
>>>
>>> I've included my exact steps at the end of the email but really it comes
>>> down to:
>>>
>>> git tag --sign your-pr-tag
>>> git push your-pr-tag
>>> git format-patch <series details>
>>> git request-pull origin/master your_repo_details your-pr-tag
>>>
>>> and finally
>>>
>>> git send-email
>>>
>>> My personal exact steps are integrated with my editor but are:
>>
>>
>>> 8 Preparing a QEMU Pull Request
>>> ═══════════════════════════════
>>
>>> 9 And send the pull request
>>> ═══════════════════════════
>>
>> For these steps I just do:
>>
>> $ git publish -b origin/master \
>> --pull-request --sign-pull --keyid=0xMYKEY
>>
>> which uses .gitpublish from commit 08bb160e02,
>> calling get_maintainer.pl for each patch.
>>
>> Using GSuite, I also have in ~/.gitconfig:
>>
>> [sendemail]
>> smtpServer = smtp.gmail.com
>> smtpBatchSize = 1
>> smtpReloginDelay = 3
>
> Thanks all, I'll do some dry runs to walk through these approaches.
Tell me if you want me to unqueue your v4, otherwise I'll send a PR
with it in a few days.
Regards,
Phil.
^ permalink raw reply [flat|nested] 15+ messages in thread