Closed shenki closed 4 years ago
Reverting 16358542f32f47f372ad48e4cdf61ab245c9f49d fixes the test case of reading fan5_input
,
I implemented https://github.com/openbmc/linux/commit/16358542f32f47f372ad48e4cdf61ab245c9f49d without this hunk, and the test case failed:
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -153,14 +153,10 @@ int pmbus_set_page(struct i2c_client *client, int page, int phase)
struct pmbus_data *data = i2c_get_clientdata(client);
int rv;
- if (page < 0)
+ if (page < 0 || page == data->currpage)
return 0;
- if (!(data->info->func[page] & PMBUS_PAGE_VIRTUAL) &&
- data->info->pages > 1 && page != data->currpage) {
- dev_dbg(&client->dev, "Want page %u, %u cached\n", page,
- data->currpage);
-
+ if (!(data->info->func[page] & PMBUS_PAGE_VIRTUAL)) {
rv = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
if (rv < 0) {
rv = i2c_smbus_write_byte_data(client, PMBUS_PAGE,
@@ -179,16 +175,8 @@ int pmbus_set_page(struct i2c_client *client, int page, int phase)
if (rv != page)
return -EIO;
}
- data->currpage = page;
- if (data->info->phases[page] && data->currphase != phase &&
- !(data->info->func[page] & PMBUS_PHASE_VIRTUAL)) {
- rv = i2c_smbus_write_byte_data(client, PMBUS_PHASE,
- phase);
- if (rv)
- return rv;
- }
- data->currphase = phase;
+ data->currpage = page;
return 0;
}
This branch has https://github.com/openbmc/linux/commit/16358542f32f47f372ad48e4cdf61ab245c9f49d implemented as a set of smaller bisectable changes: https://github.com/shenki/linux/tree/pmbus-phase-bisecting
git bisect start
# bad: [13db40844ced59228c1fc146684e4ee21f280c94] pmbus_add_sensor_attrs add implementation
git bisect bad 13db40844ced59228c1fc146684e4ee21f280c94
# good: [702572d1a052d2d280375e0a2f1beeb1a53ad335] Revert "hwmon: (pmbus) Implement multi-phase support"
git bisect good 702572d1a052d2d280375e0a2f1beeb1a53ad335
# good: [965ac0df96c6266eec3399e460e53a4289f9107a] minor things
git bisect good 965ac0df96c6266eec3399e460e53a4289f9107a
# bad: [e5448bbc95e9726d362274e6a5298040d4278f05] pmbus_add_sensor prototype
git bisect bad e5448bbc95e9726d362274e6a5298040d4278f05
# good: [03beb3950e133829c02722ba9f0a124d9d23e4b9] add_sensor_attrs_one
git bisect good 03beb3950e133829c02722ba9f0a124d9d23e4b9
# first bad commit: [e5448bbc95e9726d362274e6a5298040d4278f05] pmbus_add_sensor prototype
From e5448bbc95e9726d362274e6a5298040d4278f05 Mon Sep 17 00:00:00 2001
From: Joel Stanley <joel@jms.id.au>
Date: Mon, 20 Jul 2020 17:23:42 +0930
Subject: [PATCH] pmbus_add_sensor prototype
no internals
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/hwmon/pmbus/pmbus_core.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 46206840d6c7..86b718841fa7 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -1105,7 +1105,8 @@ static int pmbus_add_boolean(struct pmbus_data *data,
static struct pmbus_sensor *pmbus_add_sensor(struct pmbus_data *data,
const char *name, const char *type,
- int seq, int page, int reg,
+ int seq, int page, int phase,
+ int reg,
enum pmbus_sensor_classes class,
bool update, bool readonly,
bool convert)
@@ -1239,7 +1240,7 @@ static int pmbus_add_limit_attrs(struct i2c_client *client,
for (i = 0; i < nlimit; i++) {
if (pmbus_check_word_register(client, page, l->reg)) {
curr = pmbus_add_sensor(data, name, l->attr, index,
- page, l->reg, attr->class,
+ page, 0xff, l->reg, attr->class,
attr->update || l->update,
false, true);
if (!curr)
@@ -1280,8 +1281,8 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,
if (ret)
return ret;
}
- base = pmbus_add_sensor(data, name, "input", index, page, attr->reg,
- attr->class, true, true, true);
+ base = pmbus_add_sensor(data, name, "input", index, page, phase,
+ attr->reg, attr->class, true, true, true);
if (!base)
return -ENOMEM;
if (attr->sfunc) {
@@ -1861,7 +1862,7 @@ static int pmbus_add_fan_ctrl(struct i2c_client *client,
struct pmbus_sensor *sensor;
sensor = pmbus_add_sensor(data, "fan", "target", index, page,
- PMBUS_VIRT_FAN_TARGET_1 + id, PSC_FAN,
+ PMBUS_VIRT_FAN_TARGET_1 + id, 0xff, PSC_FAN,
false, false, true);
if (!sensor)
@@ -1872,14 +1873,14 @@ static int pmbus_add_fan_ctrl(struct i2c_client *client,
return 0;
sensor = pmbus_add_sensor(data, "pwm", NULL, index, page,
- PMBUS_VIRT_PWM_1 + id, PSC_PWM,
+ PMBUS_VIRT_PWM_1 + id, 0xff, PSC_PWM,
false, false, true);
if (!sensor)
return -ENOMEM;
sensor = pmbus_add_sensor(data, "pwm", "enable", index, page,
- PMBUS_VIRT_PWM_ENABLE_1 + id, PSC_PWM,
+ PMBUS_VIRT_PWM_ENABLE_1 + id, 0xff, PSC_PWM,
true, false, false);
if (!sensor)
@@ -1921,7 +1922,7 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,
continue;
if (pmbus_add_sensor(data, "fan", "input", index,
- page, pmbus_fan_registers[f],
+ page, pmbus_fan_registers[f], 0xff,
PSC_FAN, true, true, true) == NULL)
return -ENOMEM;
--
2.27.0
This patch resolves the issue by reverting the above hunk, plus a fix to still build:
0001-pmbus-core-Remove-phase-paramter-from-pmbus_add_sens..txt
However, upstream has a patch that fixes it properly. It was added to stable in v5.7.8, so if we had waited a few more days we would have never seen this failure in the openbmc tree.
b4c8af4c2a226fc9c25e1decbd26fdab1b0993ee is the upstream commit. ed0021db47b06272bd79f8c6271863280a988d33 is the v5.7 stable commit.
This was fixed in openbmc by integrating the v5.7 stable fixes into dev-5.7.
Running 5.7.7-5d6a308 (dev-5.7 HEAD as of today) on Tacoma. The logs below are from spork. This was also reported on multiple witherspoon systems.
Debugging setup: