* [PATCH 0/7] PMBus fixes and new functions
@ 2023-03-31 0:07 Titus Rwantare
2023-03-31 0:07 ` [PATCH 1/7] hw/i2c: pmbus add support for block receive Titus Rwantare
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Titus Rwantare @ 2023-03-31 0:07 UTC (permalink / raw)
To: philmd, minyard; +Cc: qemu-arm, qemu-devel, Titus Rwantare
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.
Thanks
Changes in v2:
- Expanded commit descriptions
- Added the ADM1266 device model that uses new functions
Titus Rwantare (7):
hw/i2c: pmbus add support for block receive
hw/i2c: pmbus: add vout mode bitfields
hw/i2c: pmbus: add fan support
hw/i2c: pmbus: block uninitialised string reads
hw/i2c: pmbus: add VCAP register
hw/sensor: add ADM1266 device model
tests/qtest: add tests for ADM1266
hw/arm/Kconfig | 1 +
hw/i2c/pmbus_device.c | 221 ++++++++++++++++++++++++++++-
hw/sensor/Kconfig | 5 +
hw/sensor/adm1266.c | 255 ++++++++++++++++++++++++++++++++++
hw/sensor/meson.build | 1 +
include/hw/i2c/pmbus_device.h | 17 +++
tests/qtest/adm1266-test.c | 123 ++++++++++++++++
tests/qtest/meson.build | 1 +
8 files changed, 623 insertions(+), 1 deletion(-)
create mode 100644 hw/sensor/adm1266.c
create mode 100644 tests/qtest/adm1266-test.c
--
2.40.0.423.gd6c402a77b-goog
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/7] hw/i2c: pmbus add support for block receive
2023-03-31 0:07 [PATCH 0/7] PMBus fixes and new functions Titus Rwantare
@ 2023-03-31 0:07 ` Titus Rwantare
2023-03-31 13:47 ` Corey Minyard
2023-03-31 0:07 ` [PATCH 2/7] hw/i2c: pmbus: add vout mode bitfields Titus Rwantare
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Titus Rwantare @ 2023-03-31 0:07 UTC (permalink / raw)
To: philmd, minyard; +Cc: qemu-arm, qemu-devel, 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 c3d6046784..02647769cd 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -95,7 +95,6 @@ void pmbus_send64(PMBusDevice *pmdev, uint64_t data)
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;
@@ -105,6 +104,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.40.0.423.gd6c402a77b-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/7] hw/i2c: pmbus: add vout mode bitfields
2023-03-31 0:07 [PATCH 0/7] PMBus fixes and new functions Titus Rwantare
2023-03-31 0:07 ` [PATCH 1/7] hw/i2c: pmbus add support for block receive Titus Rwantare
@ 2023-03-31 0:07 ` Titus Rwantare
2023-03-31 13:50 ` Corey Minyard
2023-03-31 0:07 ` [PATCH 3/7] hw/i2c: pmbus: add fan support Titus Rwantare
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Titus Rwantare @ 2023-03-31 0:07 UTC (permalink / raw)
To: philmd, minyard; +Cc: qemu-arm, qemu-devel, 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.40.0.423.gd6c402a77b-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/7] hw/i2c: pmbus: add fan support
2023-03-31 0:07 [PATCH 0/7] PMBus fixes and new functions Titus Rwantare
2023-03-31 0:07 ` [PATCH 1/7] hw/i2c: pmbus add support for block receive Titus Rwantare
2023-03-31 0:07 ` [PATCH 2/7] hw/i2c: pmbus: add vout mode bitfields Titus Rwantare
@ 2023-03-31 0:07 ` Titus Rwantare
2023-03-31 13:51 ` Corey Minyard
2023-03-31 0:07 ` [PATCH 4/7] hw/i2c: pmbus: block uninitialised string reads Titus Rwantare
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Titus Rwantare @ 2023-03-31 0:07 UTC (permalink / raw)
To: philmd, minyard; +Cc: qemu-arm, qemu-devel, 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 02647769cd..bb42e410b4 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -490,6 +490,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);
@@ -800,6 +848,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);
@@ -872,6 +936,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);
@@ -1295,6 +1407,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);
@@ -1600,6 +1760,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.40.0.423.gd6c402a77b-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/7] hw/i2c: pmbus: block uninitialised string reads
2023-03-31 0:07 [PATCH 0/7] PMBus fixes and new functions Titus Rwantare
` (2 preceding siblings ...)
2023-03-31 0:07 ` [PATCH 3/7] hw/i2c: pmbus: add fan support Titus Rwantare
@ 2023-03-31 0:07 ` Titus Rwantare
2023-03-31 13:53 ` Corey Minyard
2023-03-31 0:07 ` [PATCH 5/7] hw/i2c: pmbus: add VCAP register Titus Rwantare
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Titus Rwantare @ 2023-03-31 0:07 UTC (permalink / raw)
To: philmd, minyard; +Cc: qemu-arm, qemu-devel, Titus Rwantare, Patrick Venture
Devices models calling pmbus_send_string can't be relied upon to
send a non-zero pointer. This logs an error and doesn't segfault.
Reviewed-by: Patrick Venture <venture@google.com>
Signed-off-by: Titus Rwantare <titusr@google.com>
---
hw/i2c/pmbus_device.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index bb42e410b4..18e629eaac 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -94,6 +94,13 @@ void pmbus_send64(PMBusDevice *pmdev, uint64_t data)
void pmbus_send_string(PMBusDevice *pmdev, const char *data)
{
+ if (!data) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: %s: uninitialised read from 0x%02x\n",
+ __func__, DEVICE(pmdev)->canonical_path, pmdev->code);
+ return;
+ }
+
size_t len = strlen(data);
g_assert(len + pmdev->out_buf_len < SMBUS_DATA_MAX_LEN);
pmdev->out_buf[len + pmdev->out_buf_len] = len;
--
2.40.0.423.gd6c402a77b-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/7] hw/i2c: pmbus: add VCAP register
2023-03-31 0:07 [PATCH 0/7] PMBus fixes and new functions Titus Rwantare
` (3 preceding siblings ...)
2023-03-31 0:07 ` [PATCH 4/7] hw/i2c: pmbus: block uninitialised string reads Titus Rwantare
@ 2023-03-31 0:07 ` Titus Rwantare
2023-03-31 13:53 ` Corey Minyard
2023-03-31 0:07 ` [PATCH 6/7] hw/sensor: add ADM1266 device model Titus Rwantare
2023-03-31 0:07 ` [PATCH 7/7] tests/qtest: add tests for ADM1266 Titus Rwantare
6 siblings, 1 reply; 15+ messages in thread
From: Titus Rwantare @ 2023-03-31 0:07 UTC (permalink / raw)
To: philmd, minyard; +Cc: qemu-arm, qemu-devel, 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 18e629eaac..ef0314a913 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -903,6 +903,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.40.0.423.gd6c402a77b-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 6/7] hw/sensor: add ADM1266 device model
2023-03-31 0:07 [PATCH 0/7] PMBus fixes and new functions Titus Rwantare
` (4 preceding siblings ...)
2023-03-31 0:07 ` [PATCH 5/7] hw/i2c: pmbus: add VCAP register Titus Rwantare
@ 2023-03-31 0:07 ` Titus Rwantare
2023-03-31 13:59 ` Corey Minyard
2023-03-31 0:07 ` [PATCH 7/7] tests/qtest: add tests for ADM1266 Titus Rwantare
6 siblings, 1 reply; 15+ messages in thread
From: Titus Rwantare @ 2023-03-31 0:07 UTC (permalink / raw)
To: philmd, minyard; +Cc: qemu-arm, qemu-devel, 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 | 255 ++++++++++++++++++++++++++++++++++++++++++
hw/sensor/meson.build | 1 +
4 files changed, 262 insertions(+)
create mode 100644 hw/sensor/adm1266.c
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index b5aed4aff5..4e44a7451d 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -407,6 +407,7 @@ config XLNX_VERSAL
config NPCM7XX
bool
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..0745b12b1d
--- /dev/null
+++ b/hw/sensor/adm1266.c
@@ -0,0 +1,255 @@
+/*
+ * 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 <string.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 9e9be602c3..4528ee6215 100644
--- a/hw/sensor/meson.build
+++ b/hw/sensor/meson.build
@@ -3,6 +3,7 @@ softmmu_ss.add(when: 'CONFIG_TMP421', if_true: files('tmp421.c'))
softmmu_ss.add(when: 'CONFIG_DPS310', if_true: files('dps310.c'))
softmmu_ss.add(when: 'CONFIG_EMC141X', if_true: files('emc141x.c'))
softmmu_ss.add(when: 'CONFIG_ADM1272', if_true: files('adm1272.c'))
+softmmu_ss.add(when: 'CONFIG_ADM1266', if_true: files('adm1266.c'))
softmmu_ss.add(when: 'CONFIG_MAX34451', if_true: files('max34451.c'))
softmmu_ss.add(when: 'CONFIG_LSM303DLHC_MAG', if_true: files('lsm303dlhc_mag.c'))
softmmu_ss.add(when: 'CONFIG_ISL_PMBUS_VR', if_true: files('isl_pmbus_vr.c'))
--
2.40.0.423.gd6c402a77b-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 7/7] tests/qtest: add tests for ADM1266
2023-03-31 0:07 [PATCH 0/7] PMBus fixes and new functions Titus Rwantare
` (5 preceding siblings ...)
2023-03-31 0:07 ` [PATCH 6/7] hw/sensor: add ADM1266 device model Titus Rwantare
@ 2023-03-31 0:07 ` Titus Rwantare
2023-03-31 14:01 ` Corey Minyard
6 siblings, 1 reply; 15+ messages in thread
From: Titus Rwantare @ 2023-03-31 0:07 UTC (permalink / raw)
To: philmd, minyard; +Cc: qemu-arm, qemu-devel, 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 85ea4e8d99..d3cf7b2287 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -232,6 +232,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.40.0.423.gd6c402a77b-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/7] hw/i2c: pmbus add support for block receive
2023-03-31 0:07 ` [PATCH 1/7] hw/i2c: pmbus add support for block receive Titus Rwantare
@ 2023-03-31 13:47 ` Corey Minyard
0 siblings, 0 replies; 15+ messages in thread
From: Corey Minyard @ 2023-03-31 13:47 UTC (permalink / raw)
To: Titus Rwantare; +Cc: philmd, qemu-arm, qemu-devel, Hao Wu
On Fri, Mar 31, 2023 at 12:07:50AM +0000, Titus Rwantare wrote:
> 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>
Acked-by: Corey Minyard <cminyard@mvista.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 c3d6046784..02647769cd 100644
> --- a/hw/i2c/pmbus_device.c
> +++ b/hw/i2c/pmbus_device.c
> @@ -95,7 +95,6 @@ void pmbus_send64(PMBusDevice *pmdev, uint64_t data)
> 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;
>
> @@ -105,6 +104,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.40.0.423.gd6c402a77b-goog
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/7] hw/i2c: pmbus: add vout mode bitfields
2023-03-31 0:07 ` [PATCH 2/7] hw/i2c: pmbus: add vout mode bitfields Titus Rwantare
@ 2023-03-31 13:50 ` Corey Minyard
0 siblings, 0 replies; 15+ messages in thread
From: Corey Minyard @ 2023-03-31 13:50 UTC (permalink / raw)
To: Titus Rwantare; +Cc: philmd, qemu-arm, qemu-devel, Hao Wu
On Fri, Mar 31, 2023 at 12:07:51AM +0000, Titus Rwantare wrote:
> 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>
Ok, I see the new sensor later.
Acked-by: Corey Minyard <cminyard@mvista.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.40.0.423.gd6c402a77b-goog
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/7] hw/i2c: pmbus: add fan support
2023-03-31 0:07 ` [PATCH 3/7] hw/i2c: pmbus: add fan support Titus Rwantare
@ 2023-03-31 13:51 ` Corey Minyard
0 siblings, 0 replies; 15+ messages in thread
From: Corey Minyard @ 2023-03-31 13:51 UTC (permalink / raw)
To: Titus Rwantare; +Cc: philmd, qemu-arm, qemu-devel, Stephen Longfield
On Fri, Mar 31, 2023 at 12:07:52AM +0000, Titus Rwantare wrote:
> 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>
Acked-by: Corey Minyard <cminyard@mvista.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 02647769cd..bb42e410b4 100644
> --- a/hw/i2c/pmbus_device.c
> +++ b/hw/i2c/pmbus_device.c
> @@ -490,6 +490,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);
> @@ -800,6 +848,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);
> @@ -872,6 +936,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);
> @@ -1295,6 +1407,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);
> @@ -1600,6 +1760,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.40.0.423.gd6c402a77b-goog
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/7] hw/i2c: pmbus: block uninitialised string reads
2023-03-31 0:07 ` [PATCH 4/7] hw/i2c: pmbus: block uninitialised string reads Titus Rwantare
@ 2023-03-31 13:53 ` Corey Minyard
0 siblings, 0 replies; 15+ messages in thread
From: Corey Minyard @ 2023-03-31 13:53 UTC (permalink / raw)
To: Titus Rwantare; +Cc: philmd, qemu-arm, qemu-devel, Patrick Venture
On Fri, Mar 31, 2023 at 12:07:53AM +0000, Titus Rwantare wrote:
> Devices models calling pmbus_send_string can't be relied upon to
> send a non-zero pointer. This logs an error and doesn't segfault.
>
> Reviewed-by: Patrick Venture <venture@google.com>
> Signed-off-by: Titus Rwantare <titusr@google.com>
Acked-by: Corey Minyard <cminyard@mvista.com>
> ---
> hw/i2c/pmbus_device.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
> index bb42e410b4..18e629eaac 100644
> --- a/hw/i2c/pmbus_device.c
> +++ b/hw/i2c/pmbus_device.c
> @@ -94,6 +94,13 @@ void pmbus_send64(PMBusDevice *pmdev, uint64_t data)
>
> void pmbus_send_string(PMBusDevice *pmdev, const char *data)
> {
> + if (!data) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: %s: uninitialised read from 0x%02x\n",
> + __func__, DEVICE(pmdev)->canonical_path, pmdev->code);
> + return;
> + }
> +
> size_t len = strlen(data);
> g_assert(len + pmdev->out_buf_len < SMBUS_DATA_MAX_LEN);
> pmdev->out_buf[len + pmdev->out_buf_len] = len;
> --
> 2.40.0.423.gd6c402a77b-goog
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/7] hw/i2c: pmbus: add VCAP register
2023-03-31 0:07 ` [PATCH 5/7] hw/i2c: pmbus: add VCAP register Titus Rwantare
@ 2023-03-31 13:53 ` Corey Minyard
0 siblings, 0 replies; 15+ messages in thread
From: Corey Minyard @ 2023-03-31 13:53 UTC (permalink / raw)
To: Titus Rwantare; +Cc: philmd, qemu-arm, qemu-devel, Benjamin Streb
On Fri, Mar 31, 2023 at 12:07:54AM +0000, Titus Rwantare wrote:
> 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>
Acked-by: Corey Minyard <cminyard@mvista.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 18e629eaac..ef0314a913 100644
> --- a/hw/i2c/pmbus_device.c
> +++ b/hw/i2c/pmbus_device.c
> @@ -903,6 +903,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.40.0.423.gd6c402a77b-goog
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 6/7] hw/sensor: add ADM1266 device model
2023-03-31 0:07 ` [PATCH 6/7] hw/sensor: add ADM1266 device model Titus Rwantare
@ 2023-03-31 13:59 ` Corey Minyard
0 siblings, 0 replies; 15+ messages in thread
From: Corey Minyard @ 2023-03-31 13:59 UTC (permalink / raw)
To: Titus Rwantare; +Cc: philmd, qemu-arm, qemu-devel, Hao Wu
On Fri, Mar 31, 2023 at 12:07:55AM +0000, Titus Rwantare wrote:
> The ADM1266 is a cascadable super sequencer with margin control and
> fault recording.
This sounds like serious marketing-speak :). I looked up the chip and
yes, that's what they say about it.
> 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>
Looks good.
Acked-by: Corey Minyard <cminyard@mvista.com>
> ---
> hw/arm/Kconfig | 1 +
> hw/sensor/Kconfig | 5 +
> hw/sensor/adm1266.c | 255 ++++++++++++++++++++++++++++++++++++++++++
> hw/sensor/meson.build | 1 +
> 4 files changed, 262 insertions(+)
> create mode 100644 hw/sensor/adm1266.c
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index b5aed4aff5..4e44a7451d 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -407,6 +407,7 @@ config XLNX_VERSAL
> config NPCM7XX
> bool
> 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..0745b12b1d
> --- /dev/null
> +++ b/hw/sensor/adm1266.c
> @@ -0,0 +1,255 @@
> +/*
> + * 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 <string.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 9e9be602c3..4528ee6215 100644
> --- a/hw/sensor/meson.build
> +++ b/hw/sensor/meson.build
> @@ -3,6 +3,7 @@ softmmu_ss.add(when: 'CONFIG_TMP421', if_true: files('tmp421.c'))
> softmmu_ss.add(when: 'CONFIG_DPS310', if_true: files('dps310.c'))
> softmmu_ss.add(when: 'CONFIG_EMC141X', if_true: files('emc141x.c'))
> softmmu_ss.add(when: 'CONFIG_ADM1272', if_true: files('adm1272.c'))
> +softmmu_ss.add(when: 'CONFIG_ADM1266', if_true: files('adm1266.c'))
> softmmu_ss.add(when: 'CONFIG_MAX34451', if_true: files('max34451.c'))
> softmmu_ss.add(when: 'CONFIG_LSM303DLHC_MAG', if_true: files('lsm303dlhc_mag.c'))
> softmmu_ss.add(when: 'CONFIG_ISL_PMBUS_VR', if_true: files('isl_pmbus_vr.c'))
> --
> 2.40.0.423.gd6c402a77b-goog
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 7/7] tests/qtest: add tests for ADM1266
2023-03-31 0:07 ` [PATCH 7/7] tests/qtest: add tests for ADM1266 Titus Rwantare
@ 2023-03-31 14:01 ` Corey Minyard
0 siblings, 0 replies; 15+ messages in thread
From: Corey Minyard @ 2023-03-31 14:01 UTC (permalink / raw)
To: Titus Rwantare; +Cc: philmd, qemu-arm, qemu-devel, Hao Wu
On Fri, Mar 31, 2023 at 12:07:56AM +0000, Titus Rwantare wrote:
> 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>
Acked-by: Corey Minyard <cminyard@mvista.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 85ea4e8d99..d3cf7b2287 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -232,6 +232,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.40.0.423.gd6c402a77b-goog
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-03-31 14:01 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-31 0:07 [PATCH 0/7] PMBus fixes and new functions Titus Rwantare
2023-03-31 0:07 ` [PATCH 1/7] hw/i2c: pmbus add support for block receive Titus Rwantare
2023-03-31 13:47 ` Corey Minyard
2023-03-31 0:07 ` [PATCH 2/7] hw/i2c: pmbus: add vout mode bitfields Titus Rwantare
2023-03-31 13:50 ` Corey Minyard
2023-03-31 0:07 ` [PATCH 3/7] hw/i2c: pmbus: add fan support Titus Rwantare
2023-03-31 13:51 ` Corey Minyard
2023-03-31 0:07 ` [PATCH 4/7] hw/i2c: pmbus: block uninitialised string reads Titus Rwantare
2023-03-31 13:53 ` Corey Minyard
2023-03-31 0:07 ` [PATCH 5/7] hw/i2c: pmbus: add VCAP register Titus Rwantare
2023-03-31 13:53 ` Corey Minyard
2023-03-31 0:07 ` [PATCH 6/7] hw/sensor: add ADM1266 device model Titus Rwantare
2023-03-31 13:59 ` Corey Minyard
2023-03-31 0:07 ` [PATCH 7/7] tests/qtest: add tests for ADM1266 Titus Rwantare
2023-03-31 14:01 ` Corey Minyard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).