Skip to content

Commit

Permalink
hw/i2c: pmbus: reset page register for out of range reads
Browse files Browse the repository at this point in the history
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.

Reviewed-by: Hao Wu <wuhaotsh@google.com>
Signed-off-by: Titus Rwantare <titusr@google.com>
Message-ID: <20231023-staging-pmbus-v3-v4-8-07a8cb7cd20a@google.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
  • Loading branch information
Rwantare authored and philmd committed Nov 6, 2023
1 parent 00b725f commit 909a2ce
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 9 deletions.
18 changes: 9 additions & 9 deletions hw/i2c/pmbus_device.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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) {
Expand Down
24 changes: 24 additions & 0 deletions tests/qtest/max34451-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = {
Expand All @@ -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);

0 comments on commit 909a2ce

Please sign in to comment.