* [PATCH v4 1/4] hwmon: (pmbus) Add and export direct conversion calculation helpers
2026-05-22 8:23 [PATCH v4 0/4] Rework TPS25990 direct conversions and add TPS1689 support Stoyan Bogdanov
@ 2026-05-22 8:23 ` Stoyan Bogdanov
2026-05-22 8:38 ` sashiko-bot
2026-05-22 8:23 ` [PATCH v4 2/4] hwmon: (pmbus/tps25990): Rework TPS25990 direct conversion handling Stoyan Bogdanov
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Stoyan Bogdanov @ 2026-05-22 8:23 UTC (permalink / raw)
To: jbrunet, linux, robh, krzk+dt, conor+dt, corbet, skhan
Cc: linux-hwmon, devicetree, linux-doc, linux-kernel, Stoyan Bogdanov
TPS25990 and upcoming TPS1689 need common computation APIs but
the current implementation is static to TPS25990. As a preparation for
TPS1689 support, split the math-only parts of pmbus_reg2data_direct() and
pmbus_data2reg_direct() into separate helper functions:
- pmbus_reg2data_direct_calc()
- pmbus_data2reg_direct_calc()
export them so the upcoming TPS1689 can use the same APIs.
This has no behavioral change on TPS25990 while allowing TPS1689
to use the same.
Signed-off-by: Stoyan Bogdanov <sbogdanov@baylibre.com>
---
drivers/hwmon/pmbus/pmbus.h | 2 ++
drivers/hwmon/pmbus/pmbus_core.c | 59 +++++++++++++++++++-------------
2 files changed, 38 insertions(+), 23 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 23e3eda58870..dd4fe7d9821d 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -578,5 +578,7 @@ DEFINE_GUARD(pmbus_lock, struct i2c_client *, pmbus_lock(_T), pmbus_unlock(_T))
int pmbus_update_fan(struct i2c_client *client, int page, int id,
u8 config, u8 mask, u16 command);
struct dentry *pmbus_get_debugfs_dir(struct i2c_client *client);
+s64 pmbus_reg2data_direct_calc(s64 val, s64 b, s32 m, s32 R);
+u16 pmbus_data2reg_direct_calc(s64 val, s64 b, s32 m, s32 R);
#endif /* PMBUS_H */
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index e8fdd799c71c..2eaac337eeab 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -817,6 +817,22 @@ static s64 pmbus_reg2data_linear(struct pmbus_data *data,
return val;
}
+s64 pmbus_reg2data_direct_calc(s64 val, s64 b, s32 m, s32 R)
+{
+ while (R > 0) {
+ val *= 10;
+ R--;
+ }
+ while (R < 0) {
+ val = div_s64(val + 5LL, 10L); /* round closest */
+ R++;
+ }
+
+ val = div_s64(val - b, m);
+ return val;
+}
+EXPORT_SYMBOL_NS_GPL(pmbus_reg2data_direct_calc, "PMBUS");
+
/*
* Convert direct sensor values to milli- or micro-units
* depending on sensor type.
@@ -824,7 +840,7 @@ static s64 pmbus_reg2data_linear(struct pmbus_data *data,
static s64 pmbus_reg2data_direct(struct pmbus_data *data,
struct pmbus_sensor *sensor)
{
- s64 b, val = (s16)sensor->data;
+ s64 b;
s32 m, R;
m = data->info->m[sensor->class];
@@ -848,17 +864,7 @@ static s64 pmbus_reg2data_direct(struct pmbus_data *data,
b *= 1000;
}
- while (R > 0) {
- val *= 10;
- R--;
- }
- while (R < 0) {
- val = div_s64(val + 5LL, 10L); /* round closest */
- R++;
- }
-
- val = div_s64(val - b, m);
- return val;
+ return pmbus_reg2data_direct_calc((s16)sensor->data, b, m, R);
}
/*
@@ -1057,6 +1063,23 @@ static u16 pmbus_data2reg_linear(struct pmbus_data *data,
return (mantissa & 0x7ff) | ((exponent << 11) & 0xf800);
}
+u16 pmbus_data2reg_direct_calc(s64 val, s64 b, s32 m, s32 R)
+{
+ val = val * m + b;
+
+ while (R > 0) {
+ val *= 10;
+ R--;
+ }
+ while (R < 0) {
+ val = div_s64(val + 5LL, 10L); /* round closest */
+ R++;
+ }
+
+ return (u16)clamp_val(val, S16_MIN, S16_MAX);
+}
+EXPORT_SYMBOL_NS_GPL(pmbus_data2reg_direct_calc, "PMBUS");
+
static u16 pmbus_data2reg_direct(struct pmbus_data *data,
struct pmbus_sensor *sensor, s64 val)
{
@@ -1078,18 +1101,8 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
R -= 3; /* Adjust R and b for data in milli-units */
b *= 1000;
}
- val = val * m + b;
- while (R > 0) {
- val *= 10;
- R--;
- }
- while (R < 0) {
- val = div_s64(val + 5LL, 10L); /* round closest */
- R++;
- }
-
- return (u16)clamp_val(val, S16_MIN, S16_MAX);
+ return pmbus_data2reg_direct_calc(val, b, m, R);
}
static u16 pmbus_data2reg_vid(struct pmbus_data *data,
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v4 1/4] hwmon: (pmbus) Add and export direct conversion calculation helpers
2026-05-22 8:23 ` [PATCH v4 1/4] hwmon: (pmbus) Add and export direct conversion calculation helpers Stoyan Bogdanov
@ 2026-05-22 8:38 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-05-22 8:38 UTC (permalink / raw)
To: Stoyan Bogdanov; +Cc: conor+dt, linux-hwmon, robh, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Unchecked arithmetic operations in the exported helpers can silently overflow, violating the hardware monitoring subsystem guidelines.
--
commit 4b7c4efa13e04ccc1fad0afa9ecc064091eda152
Author: Stoyan Bogdanov <sbogdanov@baylibre.com>
hwmon: (pmbus) Add and export direct conversion calculation helpers
This commit splits the math-only parts of pmbus_reg2data_direct() and
pmbus_data2reg_direct() into separate helper functions. It exports these
helpers so they can be reused by upcoming drivers.
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index e8fdd799c71c0..2eaac337eeaba 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
[ ... ]
> @@ -1057,6 +1063,23 @@ static u16 pmbus_data2reg_linear(struct pmbus_data *data,
> return (mantissa & 0x7ff) | ((exponent << 11) & 0xf800);
> }
>
> +u16 pmbus_data2reg_direct_calc(s64 val, s64 b, s32 m, s32 R)
> +{
> + val = val * m + b;
[Severity: High]
This is a pre-existing issue, but does this calculation need overflow checks
to conform with the hardware monitoring subsystem guidelines?
Since sysfs handlers can parse arbitrary 64-bit values from user space without
prior bounds checking, writing a very large positive value could overflow s64
during multiplication.
If an overflow wraps the value to a negative number, it would bypass the
intended clamping logic later in this function and result in writing S16_MIN
instead of S16_MAX to the hardware limit register.
For power management controllers, this unintended minimum limit could
immediately trip hardware protection and shut down the system.
> +
> + while (R > 0) {
> + val *= 10;
> + R--;
> + }
> + while (R < 0) {
> + val = div_s64(val + 5LL, 10L); /* round closest */
> + R++;
> + }
> +
> + return (u16)clamp_val(val, S16_MIN, S16_MAX);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522082349.2749970-1-sbogdanov@baylibre.com?part=1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 2/4] hwmon: (pmbus/tps25990): Rework TPS25990 direct conversion handling
2026-05-22 8:23 [PATCH v4 0/4] Rework TPS25990 direct conversions and add TPS1689 support Stoyan Bogdanov
2026-05-22 8:23 ` [PATCH v4 1/4] hwmon: (pmbus) Add and export direct conversion calculation helpers Stoyan Bogdanov
@ 2026-05-22 8:23 ` Stoyan Bogdanov
2026-05-22 9:12 ` sashiko-bot
2026-05-22 8:23 ` [PATCH v4 3/4] dt-bindings: hwmon: pmbus/tps25990: Add TPS1689 Stoyan Bogdanov
2026-05-22 8:23 ` [PATCH v4 4/4] hwmon: (pmbus/tps25990): Add TPS1689 support Stoyan Bogdanov
3 siblings, 1 reply; 9+ messages in thread
From: Stoyan Bogdanov @ 2026-05-22 8:23 UTC (permalink / raw)
To: jbrunet, linux, robh, krzk+dt, conor+dt, corbet, skhan
Cc: linux-hwmon, devicetree, linux-doc, linux-kernel, Stoyan Bogdanov
Rework the existing implementation of direct format conversion for
TPS25990 non-standard parameters to improve code reusability and
integration with the PMBus direct conversion helpers.
Changes include:
- Add an enum describing the supported parameters
- Add structure to hold m, b, R per-device coefficients
- Add data structures for pmbus_driver_info and local direct values
- Use the generic PMBus conversion helpers:
pmbus_reg2data_direct_calc()
pmbus_data2reg_direct_calc()
- Replace previously used defines with structured data
This reduces duplicated conversion logic and makes handling of
non-standard parameters more maintainable.
Signed-off-by: Stoyan Bogdanov <sbogdanov@baylibre.com>
---
drivers/hwmon/pmbus/tps25990.c | 197 +++++++++++++++++++++------------
1 file changed, 127 insertions(+), 70 deletions(-)
diff --git a/drivers/hwmon/pmbus/tps25990.c b/drivers/hwmon/pmbus/tps25990.c
index 05c6288ecafc..1e252844217b 100644
--- a/drivers/hwmon/pmbus/tps25990.c
+++ b/drivers/hwmon/pmbus/tps25990.c
@@ -36,17 +36,63 @@
#define TPS25990_UNLOCKED BIT(7)
#define TPS25990_8B_SHIFT 2
-#define TPS25990_VIN_OVF_NUM 525100
-#define TPS25990_VIN_OVF_DIV 10163
-#define TPS25990_VIN_OVF_OFF 155
-#define TPS25990_IIN_OCF_NUM 953800
-#define TPS25990_IIN_OCF_DIV 129278
-#define TPS25990_IIN_OCF_OFF 157
#define PK_MIN_AVG_RST_MASK (PK_MIN_AVG_RST_PEAK | \
PK_MIN_AVG_RST_AVG | \
PK_MIN_AVG_RST_MIN)
+enum chips {
+ tps25990,
+};
+
+enum tps25990_parameters {
+ TPS25990_VIN_OVF = 0, /* VIN over volatage fault */
+ TPS25990_IIN_OCF, /* IIN Over currect fault */
+ TPS25990_DIRECT_VALUES_COUNT,
+};
+
+struct tps25990_local_direct_value {
+ int m[TPS25990_DIRECT_VALUES_COUNT]; /* mantissa */
+ int b[TPS25990_DIRECT_VALUES_COUNT]; /* offset */
+ int R[TPS25990_DIRECT_VALUES_COUNT]; /* exponent */
+};
+
+struct tps25990_data {
+ struct pmbus_driver_info info;
+ struct tps25990_local_direct_value info_local;
+};
+
+static s64 tps25990_reg2data_direct(struct i2c_client *client, int param, s32 raw)
+{
+ struct pmbus_driver_info *info = i2c_get_clientdata(client);
+ struct tps25990_data *data = container_of(info, struct tps25990_data, info);
+ struct tps25990_local_direct_value *info_local = &data->info_local;
+ s64 b, val;
+ s32 m, R;
+
+ val = (s16)raw;
+ m = info_local->m[param];
+ b = info_local->b[param];
+ R = info_local->R[param];
+
+ return pmbus_reg2data_direct_calc(val, b, m, R);
+}
+
+static u16 tps25990_data2reg_direct(struct i2c_client *client, int param, s64 val)
+{
+ struct pmbus_driver_info *info = i2c_get_clientdata(client);
+ struct tps25990_data *data = container_of(info, struct tps25990_data, info);
+ struct tps25990_local_direct_value *info_local = &data->info_local;
+ s32 m, R;
+ s64 b;
+
+ m = info_local->m[param];
+ b = info_local->b[param];
+ R = info_local->R[param];
+
+ return pmbus_data2reg_direct_calc(val, b, m, R);
+}
+
/*
* Arbitrary default Rimon value: 1kOhm
* This correspond to an overcurrent limit of 55A, close to the specified limit
@@ -184,9 +230,7 @@ static int tps25990_read_word_data(struct i2c_client *client,
ret = pmbus_read_word_data(client, page, phase, reg);
if (ret < 0)
break;
- ret = DIV_ROUND_CLOSEST(ret * TPS25990_VIN_OVF_NUM,
- TPS25990_VIN_OVF_DIV);
- ret += TPS25990_VIN_OVF_OFF;
+ ret = tps25990_reg2data_direct(client, TPS25990_VIN_OVF, ret);
break;
case PMBUS_IIN_OC_FAULT_LIMIT:
@@ -198,9 +242,7 @@ static int tps25990_read_word_data(struct i2c_client *client,
ret = pmbus_read_byte_data(client, page, TPS25990_VIREF);
if (ret < 0)
break;
- ret = DIV_ROUND_CLOSEST(ret * TPS25990_IIN_OCF_NUM,
- TPS25990_IIN_OCF_DIV);
- ret += TPS25990_IIN_OCF_OFF;
+ ret = tps25990_reg2data_direct(client, TPS25990_IIN_OCF, ret);
break;
case PMBUS_VIRT_SAMPLES:
@@ -246,17 +288,13 @@ static int tps25990_write_word_data(struct i2c_client *client,
break;
case PMBUS_VIN_OV_FAULT_LIMIT:
- value -= TPS25990_VIN_OVF_OFF;
- value = DIV_ROUND_CLOSEST(((unsigned int)value) * TPS25990_VIN_OVF_DIV,
- TPS25990_VIN_OVF_NUM);
+ value = tps25990_data2reg_direct(client, TPS25990_VIN_OVF, value);
value = clamp_val(value, 0, 0xf);
ret = pmbus_write_word_data(client, page, reg, value);
break;
case PMBUS_IIN_OC_FAULT_LIMIT:
- value -= TPS25990_IIN_OCF_OFF;
- value = DIV_ROUND_CLOSEST(((unsigned int)value) * TPS25990_IIN_OCF_DIV,
- TPS25990_IIN_OCF_NUM);
+ value = tps25990_data2reg_direct(client, TPS25990_IIN_OCF, value);
value = clamp_val(value, 0, 0x3f);
ret = pmbus_write_byte_data(client, page, TPS25990_VIREF, value);
break;
@@ -337,63 +375,76 @@ static const struct regulator_desc tps25990_reg_desc[] = {
};
#endif
-static const struct pmbus_driver_info tps25990_base_info = {
- .pages = 1,
- .format[PSC_VOLTAGE_IN] = direct,
- .m[PSC_VOLTAGE_IN] = 5251,
- .b[PSC_VOLTAGE_IN] = 0,
- .R[PSC_VOLTAGE_IN] = -2,
- .format[PSC_VOLTAGE_OUT] = direct,
- .m[PSC_VOLTAGE_OUT] = 5251,
- .b[PSC_VOLTAGE_OUT] = 0,
- .R[PSC_VOLTAGE_OUT] = -2,
- .format[PSC_TEMPERATURE] = direct,
- .m[PSC_TEMPERATURE] = 140,
- .b[PSC_TEMPERATURE] = 32100,
- .R[PSC_TEMPERATURE] = -2,
- /*
- * Current and Power measurement depends on the ohm value
- * of Rimon. m is multiplied by 1000 below to have an integer
- * and -3 is added to R to compensate.
- */
- .format[PSC_CURRENT_IN] = direct,
- .m[PSC_CURRENT_IN] = 9538,
- .b[PSC_CURRENT_IN] = 0,
- .R[PSC_CURRENT_IN] = -6,
- .format[PSC_POWER] = direct,
- .m[PSC_POWER] = 4901,
- .b[PSC_POWER] = 0,
- .R[PSC_POWER] = -7,
- .func[0] = (PMBUS_HAVE_VIN |
- PMBUS_HAVE_VOUT |
- PMBUS_HAVE_VMON |
- PMBUS_HAVE_IIN |
- PMBUS_HAVE_PIN |
- PMBUS_HAVE_TEMP |
- PMBUS_HAVE_STATUS_VOUT |
- PMBUS_HAVE_STATUS_IOUT |
- PMBUS_HAVE_STATUS_INPUT |
- PMBUS_HAVE_STATUS_TEMP |
- PMBUS_HAVE_SAMPLES),
- .read_word_data = tps25990_read_word_data,
- .write_word_data = tps25990_write_word_data,
- .read_byte_data = tps25990_read_byte_data,
- .write_byte_data = tps25990_write_byte_data,
+static struct tps25990_local_direct_value tps25990_local_info[] = {
+ [tps25990] = {
+ .m[TPS25990_VIN_OVF] = 10163,
+ .b[TPS25990_VIN_OVF] = -30081,
+ .R[TPS25990_VIN_OVF] = -4,
+ .m[TPS25990_IIN_OCF] = 9538,
+ .b[TPS25990_IIN_OCF] = 0,
+ .R[TPS25990_IIN_OCF] = -6,
+ },
+};
+
+static struct pmbus_driver_info tps25990_base_info[] = {
+ [tps25990] = {
+ .pages = 1,
+ .format[PSC_VOLTAGE_IN] = direct,
+ .m[PSC_VOLTAGE_IN] = 5251,
+ .b[PSC_VOLTAGE_IN] = 0,
+ .R[PSC_VOLTAGE_IN] = -2,
+ .format[PSC_VOLTAGE_OUT] = direct,
+ .m[PSC_VOLTAGE_OUT] = 5251,
+ .b[PSC_VOLTAGE_OUT] = 0,
+ .R[PSC_VOLTAGE_OUT] = -2,
+ .format[PSC_TEMPERATURE] = direct,
+ .m[PSC_TEMPERATURE] = 140,
+ .b[PSC_TEMPERATURE] = 32100,
+ .R[PSC_TEMPERATURE] = -2,
+ /*
+ * Current and Power measurement depends on the ohm value
+ * of Rimon. m is multiplied by 1000 below to have an integer
+ * and -3 is added to R to compensate.
+ */
+ .format[PSC_CURRENT_IN] = direct,
+ .m[PSC_CURRENT_IN] = 9538,
+ .b[PSC_CURRENT_IN] = 0,
+ .R[PSC_CURRENT_IN] = -6,
+ .format[PSC_POWER] = direct,
+ .m[PSC_POWER] = 4901,
+ .b[PSC_POWER] = 0,
+ .R[PSC_POWER] = -7,
+ .func[0] = (PMBUS_HAVE_VIN |
+ PMBUS_HAVE_VOUT |
+ PMBUS_HAVE_VMON |
+ PMBUS_HAVE_IIN |
+ PMBUS_HAVE_PIN |
+ PMBUS_HAVE_TEMP |
+ PMBUS_HAVE_STATUS_VOUT |
+ PMBUS_HAVE_STATUS_IOUT |
+ PMBUS_HAVE_STATUS_INPUT |
+ PMBUS_HAVE_STATUS_TEMP |
+ PMBUS_HAVE_SAMPLES),
+ .read_word_data = tps25990_read_word_data,
+ .write_word_data = tps25990_write_word_data,
+ .read_byte_data = tps25990_read_byte_data,
+ .write_byte_data = tps25990_write_byte_data,
#if IS_ENABLED(CONFIG_SENSORS_TPS25990_REGULATOR)
- .reg_desc = tps25990_reg_desc,
- .num_regulators = ARRAY_SIZE(tps25990_reg_desc),
+ .reg_desc = tps25990_reg_desc,
+ .num_regulators = ARRAY_SIZE(tps25990_reg_desc),
#endif
+ },
};
static const struct i2c_device_id tps25990_i2c_id[] = {
- { "tps25990" },
+ { "tps25990", tps25990 },
{}
};
MODULE_DEVICE_TABLE(i2c, tps25990_i2c_id);
static const struct of_device_id tps25990_of_match[] = {
- { .compatible = "ti,tps25990" },
+ { .compatible = "ti,tps25990", .data = (void *)tps25990 },
{}
};
MODULE_DEVICE_TABLE(of, tps25990_of_match);
@@ -401,8 +452,9 @@ MODULE_DEVICE_TABLE(of, tps25990_of_match);
static int tps25990_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
- struct pmbus_driver_info *info;
+ struct tps25990_data *data;
const char *propname;
+ enum chips chip_id;
u32 rimon;
int ret;
@@ -415,15 +467,20 @@ static int tps25990_probe(struct i2c_client *client)
rimon = TPS25990_DEFAULT_RIMON;
}
- info = devm_kmemdup(dev, &tps25990_base_info, sizeof(*info), GFP_KERNEL);
- if (!info)
+ chip_id = (enum chips)(unsigned long)i2c_get_match_data(client);
+
+ data = devm_kzalloc(dev, sizeof(struct tps25990_data), GFP_KERNEL);
+ if (!data)
return -ENOMEM;
+ data->info = tps25990_base_info[chip_id];
+ data->info_local = tps25990_local_info[chip_id];
+
/* Adapt the current and power scale for each instance */
- tps25990_set_m(&info->m[PSC_CURRENT_IN], rimon);
- tps25990_set_m(&info->m[PSC_POWER], rimon);
+ tps25990_set_m(&data->info.m[PSC_CURRENT_IN], rimon);
+ tps25990_set_m(&data->info.m[PSC_POWER], rimon);
- return pmbus_do_probe(client, info);
+ return pmbus_do_probe(client, &data->info);
}
static struct i2c_driver tps25990_driver = {
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v4 2/4] hwmon: (pmbus/tps25990): Rework TPS25990 direct conversion handling
2026-05-22 8:23 ` [PATCH v4 2/4] hwmon: (pmbus/tps25990): Rework TPS25990 direct conversion handling Stoyan Bogdanov
@ 2026-05-22 9:12 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-05-22 9:12 UTC (permalink / raw)
To: Stoyan Bogdanov; +Cc: devicetree, linux-hwmon, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [Critical] The custom conversion functions incorrectly retrieve I2C client data, leading to severe type confusion and out-of-bounds heap reads.
- [High] Replacing ratio-based math with direct conversion helpers inside `read_word_data` and `write_word_data` introduces a double-conversion bug that completely corrupts sysfs output and configuration limits.
- [High] Calling `pmbus_reg2data_direct_calc()` directly with negative `R` coefficients and small register values leads to a total loss of precision due to premature integer division.
- [High] The patch silently drops the correct translation ratios for the Over-Current Fault Limit (`PMBUS_IIN_OC_FAULT_LIMIT`) and bypasses dynamic resistor scaling.
- [Low] Spelling mistakes in the newly introduced `tps25990_parameters` enum comments.
Pre-existing issues:
- [High] When setting fault limits, an integer underflow on low user inputs leads to an unsigned wrap-around, causing `clamp_val()` to clamp to the maximum limit instead of the minimum limit, dangerously defeating hardware protection.
--
commit 11131ea034ce953233470df60e4f6a1c34cca609
Author: Stoyan Bogdanov <sbogdanov@baylibre.com>
Date: Fri May 22 08:23:37 2026 +0000
hwmon: (pmbus/tps25990): Rework TPS25990 direct conversion handling
Rework the existing implementation of direct format conversion for
TPS25990 non-standard parameters to improve code reusability and
integration with the PMBus direct conversion helpers.
> diff --git a/drivers/hwmon/pmbus/tps25990.c b/drivers/hwmon/pmbus/tps25990.c
> index 05c6288ecafcc..1e252844217b8 100644
> --- a/drivers/hwmon/pmbus/tps25990.c
> +++ b/drivers/hwmon/pmbus/tps25990.c
[ ... ]
> +enum tps25990_parameters {
> + TPS25990_VIN_OVF = 0, /* VIN over volatage fault */
> + TPS25990_IIN_OCF, /* IIN Over currect fault */
[Severity: Low]
This isn't a bug, but there are minor typos in the comments: volatage and
currect.
> + TPS25990_DIRECT_VALUES_COUNT,
> +};
[ ... ]
> +static s64 tps25990_reg2data_direct(struct i2c_client *client, int param, s32 raw)
> +{
> + struct pmbus_driver_info *info = i2c_get_clientdata(client);
> + struct tps25990_data *data = container_of(info, struct tps25990_data, info);
[Severity: Critical]
Does this correctly retrieve the driver info?
Looking at pmbus_do_probe(), i2c_get_clientdata(client) is set to a
struct pmbus_data * object, not a struct pmbus_driver_info * object.
Using container_of() on the wrong type calculates a 0 offset, but improperly
casts the pmbus_data object into a struct tps25990_data pointer, causing the
subsequent read of info_local to fetch completely out-of-bounds heap memory.
Should this use pmbus_get_driver_info(client) instead?
> + struct tps25990_local_direct_value *info_local = &data->info_local;
> + s64 b, val;
> + s32 m, R;
> +
> + val = (s16)raw;
> + m = info_local->m[param];
> + b = info_local->b[param];
> + R = info_local->R[param];
> +
> + return pmbus_reg2data_direct_calc(val, b, m, R);
[Severity: High]
Can this result in a total loss of precision due to integer division?
For TPS25990_VIN_OVF, R is -4 and the raw register value is a small 4-bit
integer (0 to 15). The generic pmbus_reg2data_direct_calc() handles negative
R by dividing the input by 10 for each R:
pmbus_core.c:pmbus_reg2data_direct_calc() {
...
while (R < 0) {
val = div_s64(val + 5LL, 10L);
R++;
}
...
}
Dividing a small value like 15 by 10000 via integer division would truncate
it to 0, permanently destroying the underlying data. Does this need
pre-scaling before the calculation loop?
[ ... ]
> @@ -184,9 +230,7 @@ static int tps25990_read_word_data(struct i2c_client *client,
> ret = pmbus_read_word_data(client, page, phase, reg);
> if (ret < 0)
> break;
> - ret = DIV_ROUND_CLOSEST(ret * TPS25990_VIN_OVF_NUM,
> - TPS25990_VIN_OVF_DIV);
> - ret += TPS25990_VIN_OVF_OFF;
> + ret = tps25990_reg2data_direct(client, TPS25990_VIN_OVF, ret);
[Severity: High]
Does this introduce a double-conversion bug?
The driver's .read_word_data hook is expected to convert between the
chip-specific raw register value and the standard PMBus raw format (Y_std).
The PMBus core then converts Y_std to physical units.
By calling pmbus_reg2data_direct_calc() directly here, does it return the
physical value (e.g. milliVolts) rather than Y_std, causing the PMBus core
to apply its standard conversion a second time to an already converted value?
[ ... ]
> @@ -246,17 +288,13 @@ static int tps25990_write_word_data(struct i2c_client *client,
> break;
>
> case PMBUS_VIN_OV_FAULT_LIMIT:
> - value -= TPS25990_VIN_OVF_OFF;
> - value = DIV_ROUND_CLOSEST(((unsigned int)value) * TPS25990_VIN_OVF_DIV,
> - TPS25990_VIN_OVF_NUM);
> + value = tps25990_data2reg_direct(client, TPS25990_VIN_OVF, value);
> value = clamp_val(value, 0, 0xf);
[Severity: High]
This is a pre-existing issue, but does this lead to an integer underflow
that defeats hardware protection?
The value parameter in tps25990_write_word_data() is an unsigned u16. If a
sufficiently low limit (like 0) is converted, the mathematical result could
be negative. This would wrap around the unsigned u16 to a large value
(e.g., 65530).
Since clamp_val() compares using the type of the passed variable, it would see
the wrapped value as much greater than 15 and incorrectly clamp it to the
maximum hardware limit (0xf) instead of the minimum limit.
> ret = pmbus_write_word_data(client, page, reg, value);
> break;
>
> case PMBUS_IIN_OC_FAULT_LIMIT:
> - value -= TPS25990_IIN_OCF_OFF;
> - value = DIV_ROUND_CLOSEST(((unsigned int)value) * TPS25990_IIN_OCF_DIV,
> - TPS25990_IIN_OCF_NUM);
> + value = tps25990_data2reg_direct(client, TPS25990_IIN_OCF, value);
[Severity: High]
Are we losing the dynamic resistor scaling here?
The original code mapped TPS25990_VIREF to the standard Y_std format,
allowing the PMBus core to dynamically convert it using the PSC_CURRENT_IN
coefficients, which are adjusted in tps25990_probe() based on the
ti,rimon-micro-ohms property.
The new tps25990_data2reg_direct() call uses static coefficients from
tps25990_local_info which are never updated by the rimon logic. Will this
break the over-current limits for boards using a non-default rimon
configuration?
> value = clamp_val(value, 0, 0x3f);
> ret = pmbus_write_byte_data(client, page, TPS25990_VIREF, value);
> break;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522082349.2749970-1-sbogdanov@baylibre.com?part=2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 3/4] dt-bindings: hwmon: pmbus/tps25990: Add TPS1689
2026-05-22 8:23 [PATCH v4 0/4] Rework TPS25990 direct conversions and add TPS1689 support Stoyan Bogdanov
2026-05-22 8:23 ` [PATCH v4 1/4] hwmon: (pmbus) Add and export direct conversion calculation helpers Stoyan Bogdanov
2026-05-22 8:23 ` [PATCH v4 2/4] hwmon: (pmbus/tps25990): Rework TPS25990 direct conversion handling Stoyan Bogdanov
@ 2026-05-22 8:23 ` Stoyan Bogdanov
2026-05-22 9:24 ` sashiko-bot
2026-05-22 8:23 ` [PATCH v4 4/4] hwmon: (pmbus/tps25990): Add TPS1689 support Stoyan Bogdanov
3 siblings, 1 reply; 9+ messages in thread
From: Stoyan Bogdanov @ 2026-05-22 8:23 UTC (permalink / raw)
To: jbrunet, linux, robh, krzk+dt, conor+dt, corbet, skhan
Cc: linux-hwmon, devicetree, linux-doc, linux-kernel, Stoyan Bogdanov,
Krzysztof Kozlowski
Add device compatible support for TPS1689
Signed-off-by: Stoyan Bogdanov <sbogdanov@baylibre.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
.../devicetree/bindings/hwmon/pmbus/ti,tps25990.yaml | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,tps25990.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,tps25990.yaml
index f4115870e450..63ccb67576df 100644
--- a/Documentation/devicetree/bindings/hwmon/pmbus/ti,tps25990.yaml
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,tps25990.yaml
@@ -5,18 +5,20 @@
$id: http://devicetree.org/schemas/hwmon/pmbus/ti,tps25990.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: Texas Instruments TPS25990 Stackable eFuse
+title: Texas Instruments Stackable eFuses
maintainers:
- Jerome Brunet <jbrunet@baylibre.com>
description:
- The TI TPS25990 is an integrated, high-current circuit
+ The TI TPS25990 and TPS1689 are integrated, high-current circuit
protection and power management device with PMBUS interface
properties:
compatible:
- const: ti,tps25990
+ enum:
+ - ti,tps1689
+ - ti,tps25990
reg:
maxItems: 1
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v4 3/4] dt-bindings: hwmon: pmbus/tps25990: Add TPS1689
2026-05-22 8:23 ` [PATCH v4 3/4] dt-bindings: hwmon: pmbus/tps25990: Add TPS1689 Stoyan Bogdanov
@ 2026-05-22 9:24 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-05-22 9:24 UTC (permalink / raw)
To: Stoyan Bogdanov; +Cc: conor+dt, robh, devicetree, linux-hwmon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The driver incorrectly uses `i2c_get_clientdata()` to retrieve the `pmbus_driver_info` pointer, resulting in a type mismatch and out-of-bounds memory access.
--
commit 11131ea034ce953233470df60e4f6a1c34cca609
Author: Stoyan Bogdanov <sbogdanov@baylibre.com>
hwmon: (pmbus/tps25990): Rework TPS25990 direct conversion handling
Rework the existing implementation of direct format conversion for TPS25990
non-standard parameters to improve code reusability. It introduces structures
for per-device coefficients and uses generic PMBus conversion helpers.
> diff --git a/drivers/hwmon/pmbus/tps25990.c b/drivers/hwmon/pmbus/tps25990.c
> index 05c6288ecafcc..1e252844217b8 100644
> --- a/drivers/hwmon/pmbus/tps25990.c
> +++ b/drivers/hwmon/pmbus/tps25990.c
[ ... ]
> @@ -36,17 +36,63 @@
> #define TPS25990_UNLOCKED BIT(7)
>
> #define TPS25990_8B_SHIFT 2
[ ... ]
> +struct tps25990_data {
> + struct pmbus_driver_info info;
> + struct tps25990_local_direct_value info_local;
> +};
> +
> +static s64 tps25990_reg2data_direct(struct i2c_client *client, int param, s32 raw)
> +{
> + struct pmbus_driver_info *info = i2c_get_clientdata(client);
> + struct tps25990_data *data = container_of(info, struct tps25990_data, info);
[Severity: Critical]
Does this code correctly retrieve the pmbus_driver_info pointer?
In pmbus_core.c, pmbus_do_probe() sets the I2C client data to a newly
allocated struct pmbus_data object, not struct pmbus_driver_info.
Because info is the first member of struct tps25990_data, container_of()
evaluates to the same pointer, causing the driver to mistakenly interpret the
pmbus_data object as a tps25990_data object.
When accessing the info_local member, it reads from an offset, yielding garbage
values for the calculation constants m, b, and R. If the garbage m value
happens to be zero, reading sensors via sysfs could cause a divide-by-zero
kernel panic in pmbus_reg2data_direct_calc().
Should this use pmbus_get_driver_info(client) instead of i2c_get_clientdata()?
> + struct tps25990_local_direct_value *info_local = &data->info_local;
> + s64 b, val;
> + s32 m, R;
> +
> + val = (s16)raw;
> + m = info_local->m[param];
> + b = info_local->b[param];
> + R = info_local->R[param];
> +
> + return pmbus_reg2data_direct_calc(val, b, m, R);
> +}
> +
> +static u16 tps25990_data2reg_direct(struct i2c_client *client, int param, s64 val)
> +{
> + struct pmbus_driver_info *info = i2c_get_clientdata(client);
> + struct tps25990_data *data = container_of(info, struct tps25990_data, info);
[Severity: Critical]
Does this face the same type mismatch regression?
Since the I2C client data points to a struct pmbus_data, using
i2c_get_clientdata() here will also misinterpret the object and result in
out-of-bounds memory access when reading info_local.
Could this be updated to use pmbus_get_driver_info(client) as well?
> + struct tps25990_local_direct_value *info_local = &data->info_local;
> + s32 m, R;
> + s64 b;
> +
> + m = info_local->m[param];
> + b = info_local->b[param];
> + R = info_local->R[param];
> +
> + return pmbus_data2reg_direct_calc(val, b, m, R);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522082349.2749970-1-sbogdanov@baylibre.com?part=3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 4/4] hwmon: (pmbus/tps25990): Add TPS1689 support
2026-05-22 8:23 [PATCH v4 0/4] Rework TPS25990 direct conversions and add TPS1689 support Stoyan Bogdanov
` (2 preceding siblings ...)
2026-05-22 8:23 ` [PATCH v4 3/4] dt-bindings: hwmon: pmbus/tps25990: Add TPS1689 Stoyan Bogdanov
@ 2026-05-22 8:23 ` Stoyan Bogdanov
2026-05-22 9:45 ` sashiko-bot
3 siblings, 1 reply; 9+ messages in thread
From: Stoyan Bogdanov @ 2026-05-22 8:23 UTC (permalink / raw)
To: jbrunet, linux, robh, krzk+dt, conor+dt, corbet, skhan
Cc: linux-hwmon, devicetree, linux-doc, linux-kernel, Stoyan Bogdanov
Extend the existing TPS25990 driver to support the TPS1689 eFuse,
as both devices share the same command interface and functionality.
Update the documentation to include TPS1689 support.
Signed-off-by: Stoyan Bogdanov <sbogdanov@baylibre.com>
---
Documentation/hwmon/tps25990.rst | 15 +++++---
drivers/hwmon/pmbus/tps25990.c | 66 +++++++++++++++++++++++++++++++-
2 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/Documentation/hwmon/tps25990.rst b/Documentation/hwmon/tps25990.rst
index 04faec780d26..e8bc9a550bda 100644
--- a/Documentation/hwmon/tps25990.rst
+++ b/Documentation/hwmon/tps25990.rst
@@ -9,26 +9,31 @@ Supported chips:
Prefix: 'tps25990'
- * Datasheet
+ Datasheet: Publicly available at Texas Instruments website: https://www.ti.com/lit/gpn/tps25990
- Publicly available at Texas Instruments website: https://www.ti.com/lit/gpn/tps25990
+ * TI TPS1689
+
+ Prefix: 'tps1689'
+
+ Datasheet: Publicly available at Texas Instruments website: https://www.ti.com/lit/gpn/tps1689
Author:
Jerome Brunet <jbrunet@baylibre.com>
+ Stoyan Bogdanov <sbogdanov@baylibre.com>
Description
-----------
-This driver implements support for TI TPS25990 eFuse.
+This driver implements support for TI TPS25990 and TI TPS1689 eFuse chips.
This is an integrated, high-current circuit protection and power
management device with PMBUS interface
-Device compliant with:
+Devices are compliant with:
- PMBus rev 1.3 interface.
-Device supports direct format for reading input voltages,
+Devices supports direct format for reading input voltages,
output voltage, input current, input power and temperature.
Due to the specificities of the chip, all history reset attributes
diff --git a/drivers/hwmon/pmbus/tps25990.c b/drivers/hwmon/pmbus/tps25990.c
index 1e252844217b..720e134f1892 100644
--- a/drivers/hwmon/pmbus/tps25990.c
+++ b/drivers/hwmon/pmbus/tps25990.c
@@ -42,6 +42,7 @@
PK_MIN_AVG_RST_MIN)
enum chips {
+ tps1689,
tps25990,
};
@@ -286,7 +287,6 @@ static int tps25990_write_word_data(struct i2c_client *client,
value = clamp_val(value, 0, 0xff);
ret = pmbus_write_word_data(client, page, reg, value);
break;
-
case PMBUS_VIN_OV_FAULT_LIMIT:
value = tps25990_data2reg_direct(client, TPS25990_VIN_OVF, value);
value = clamp_val(value, 0, 0xf);
@@ -376,6 +376,14 @@ static const struct regulator_desc tps25990_reg_desc[] = {
#endif
static struct tps25990_local_direct_value tps25990_local_info[] = {
+ [tps1689] = {
+ .m[TPS25990_VIN_OVF] = 3984,
+ .b[TPS25990_VIN_OVF] = -63750,
+ .R[TPS25990_VIN_OVF] = -3,
+ .m[TPS25990_IIN_OCF] = 7111,
+ .b[TPS25990_IIN_OCF] = -2133,
+ .R[TPS25990_IIN_OCF] = -2,
+ },
[tps25990] = {
.m[TPS25990_VIN_OVF] = 10163,
.b[TPS25990_VIN_OVF] = -30081,
@@ -387,6 +395,59 @@ static struct tps25990_local_direct_value tps25990_local_info[] = {
};
static struct pmbus_driver_info tps25990_base_info[] = {
+ [tps1689] = {
+ .pages = 1,
+ .format[PSC_VOLTAGE_IN] = direct,
+ .m[PSC_VOLTAGE_IN] = 1166,
+ .b[PSC_VOLTAGE_IN] = 0,
+ .R[PSC_VOLTAGE_IN] = -2,
+ .format[PSC_VOLTAGE_OUT] = direct,
+ .m[PSC_VOLTAGE_OUT] = 1166,
+ .b[PSC_VOLTAGE_OUT] = 0,
+ .R[PSC_VOLTAGE_OUT] = -2,
+ .format[PSC_TEMPERATURE] = direct,
+ .m[PSC_TEMPERATURE] = 140,
+ .b[PSC_TEMPERATURE] = 32103,
+ .R[PSC_TEMPERATURE] = -2,
+ /*
+ * Current and Power measurement depends on the ohm value
+ * of Rimon. m is multiplied by 1000 below to have an integer
+ * and -3 is added to R to compensate.
+ */
+ .format[PSC_CURRENT_IN] = direct,
+ .m[PSC_CURRENT_IN] = 9548,
+ .b[PSC_CURRENT_IN] = 0,
+ .R[PSC_CURRENT_IN] = -6,
+ .format[PSC_CURRENT_OUT] = direct,
+ .m[PSC_CURRENT_OUT] = 24347,
+ .b[PSC_CURRENT_OUT] = 0,
+ .R[PSC_CURRENT_OUT] = -3,
+ .format[PSC_POWER] = direct,
+ .m[PSC_POWER] = 2775,
+ .b[PSC_POWER] = 0,
+ .R[PSC_POWER] = -4,
+ .func[0] = (PMBUS_HAVE_VIN |
+ PMBUS_HAVE_VOUT |
+ PMBUS_HAVE_VMON |
+ PMBUS_HAVE_IIN |
+ PMBUS_HAVE_PIN |
+ PMBUS_HAVE_TEMP |
+ PMBUS_HAVE_STATUS_VOUT |
+ PMBUS_HAVE_STATUS_IOUT |
+ PMBUS_HAVE_STATUS_INPUT |
+ PMBUS_HAVE_STATUS_TEMP |
+ PMBUS_HAVE_SAMPLES),
+
+ .read_word_data = tps25990_read_word_data,
+ .write_word_data = tps25990_write_word_data,
+ .read_byte_data = tps25990_read_byte_data,
+ .write_byte_data = tps25990_write_byte_data,
+
+#if IS_ENABLED(CONFIG_SENSORS_TPS25990_REGULATOR)
+ .reg_desc = tps25990_reg_desc,
+ .num_regulators = ARRAY_SIZE(tps25990_reg_desc),
+#endif
+ },
[tps25990] = {
.pages = 1,
.format[PSC_VOLTAGE_IN] = direct,
@@ -429,7 +490,6 @@ static struct pmbus_driver_info tps25990_base_info[] = {
.write_word_data = tps25990_write_word_data,
.read_byte_data = tps25990_read_byte_data,
.write_byte_data = tps25990_write_byte_data,
-
#if IS_ENABLED(CONFIG_SENSORS_TPS25990_REGULATOR)
.reg_desc = tps25990_reg_desc,
.num_regulators = ARRAY_SIZE(tps25990_reg_desc),
@@ -438,12 +498,14 @@ static struct pmbus_driver_info tps25990_base_info[] = {
};
static const struct i2c_device_id tps25990_i2c_id[] = {
+ { "tps1689", tps1689 },
{ "tps25990", tps25990 },
{}
};
MODULE_DEVICE_TABLE(i2c, tps25990_i2c_id);
static const struct of_device_id tps25990_of_match[] = {
+ { .compatible = "ti,tps1689", .data = (void *)tps1689 },
{ .compatible = "ti,tps25990", .data = (void *)tps25990 },
{}
};
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v4 4/4] hwmon: (pmbus/tps25990): Add TPS1689 support
2026-05-22 8:23 ` [PATCH v4 4/4] hwmon: (pmbus/tps25990): Add TPS1689 support Stoyan Bogdanov
@ 2026-05-22 9:45 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-05-22 9:45 UTC (permalink / raw)
To: Stoyan Bogdanov; +Cc: linux-hwmon, conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Medium] The output current (`PSC_CURRENT_OUT`) parameters for TPS1689 are configured, but the feature is rendered inactive due to a missing capability flag, and its mantissa is not dynamically scaled by the sense resistor.
Pre-existing issues:
- [Critical] The driver incorrectly uses `i2c_get_clientdata()` to retrieve the `pmbus_driver_info` pointer in `tps25990_reg2data_direct` and `tps25990_data2reg_direct`, resulting in a type confusion and out-of-bounds read.
- [High] The mantissa for the local `TPS25990_IIN_OCF` parameter is not dynamically scaled by the `rimon` resistor value, leading to incorrect over-current fault limits.
--
commit 7807fad0cb3cb2d485b85493ccab6a9ca7c6bdde
Author: Stoyan Bogdanov <sbogdanov@baylibre.com>
hwmon: (pmbus/tps25990): Add TPS1689 support
Extend the existing TPS25990 driver to support the TPS1689 eFuse,
as both devices share the same command interface and functionality.
> diff --git a/drivers/hwmon/pmbus/tps25990.c b/drivers/hwmon/pmbus/tps25990.c
> index 1e252844217b8..720e134f18925 100644
> --- a/drivers/hwmon/pmbus/tps25990.c
> +++ b/drivers/hwmon/pmbus/tps25990.c
[ ... ]
> @@ -286,7 +287,6 @@ static int tps25990_write_word_data(struct i2c_client *client,
> value = clamp_val(value, 0, 0xff);
> ret = pmbus_write_word_data(client, page, reg, value);
> break;
> -
> case PMBUS_VIN_OV_FAULT_LIMIT:
> value = tps25990_data2reg_direct(client, TPS25990_VIN_OVF, value);
[Severity: Critical]
This is a pre-existing issue, but does tps25990_data2reg_direct() retrieve
the correct pointer type here?
Looking at tps25990_data2reg_direct(), it uses i2c_get_clientdata() to
get struct pmbus_driver_info *:
static u16 tps25990_data2reg_direct(struct i2c_client *client, int param, s64 val)
{
struct pmbus_driver_info *info = i2c_get_clientdata(client);
struct tps25990_data *data = container_of(info, struct tps25990_data, info);
Since pmbus_do_probe() sets the client data to its internal
struct pmbus_data *, wouldn't this cast read past the end of
struct pmbus_data into arbitrary heap memory when accessing data->info_local?
Should this use pmbus_get_driver_info(client) to retrieve the correct
pointer instead?
> value = clamp_val(value, 0, 0xf);
> ret = pmbus_write_word_data(client, page, reg, value);
[ ... ]
> @@ -376,6 +376,14 @@ static const struct regulator_desc tps25990_reg_desc[] = {
> #endif
>
> static struct tps25990_local_direct_value tps25990_local_info[] = {
> + [tps1689] = {
> + .m[TPS25990_VIN_OVF] = 3984,
> + .b[TPS25990_VIN_OVF] = -63750,
> + .R[TPS25990_VIN_OVF] = -3,
> + .m[TPS25990_IIN_OCF] = 7111,
[Severity: High]
This is a pre-existing issue, but is the mantissa for TPS25990_IIN_OCF
scaled correctly for custom sense resistors?
Because PMBUS_IIN_OC_FAULT_LIMIT corresponds to a physical current passing
through the sense resistor, shouldn't the mantissa be dynamically scaled by
the hardware rimon resistor value in tps25990_probe(), similar to
PSC_CURRENT_IN?
Without scaling, will writing to PMBUS_IIN_OC_FAULT_LIMIT configure the
over-current protection based on the unscaled default resistor value?
> + .b[TPS25990_IIN_OCF] = -2133,
> + .R[TPS25990_IIN_OCF] = -2,
> + },
> [tps25990] = {
[ ... ]
> @@ -387,6 +395,59 @@ static struct pmbus_driver_info tps25990_base_info[] = {
> + [tps1689] = {
> + .pages = 1,
[ ... ]
> + .format[PSC_CURRENT_OUT] = direct,
> + .m[PSC_CURRENT_OUT] = 24347,
> + .b[PSC_CURRENT_OUT] = 0,
> + .R[PSC_CURRENT_OUT] = -3,
> + .format[PSC_POWER] = direct,
> + .m[PSC_POWER] = 2775,
> + .b[PSC_POWER] = 0,
> + .R[PSC_POWER] = -4,
> + .func[0] = (PMBUS_HAVE_VIN |
> + PMBUS_HAVE_VOUT |
> + PMBUS_HAVE_VMON |
> + PMBUS_HAVE_IIN |
> + PMBUS_HAVE_PIN |
> + PMBUS_HAVE_TEMP |
> + PMBUS_HAVE_STATUS_VOUT |
> + PMBUS_HAVE_STATUS_IOUT |
> + PMBUS_HAVE_STATUS_INPUT |
> + PMBUS_HAVE_STATUS_TEMP |
> + PMBUS_HAVE_SAMPLES),
[Severity: Medium]
Are we missing the PMBUS_HAVE_IOUT capability flag here?
The format parameters for PSC_CURRENT_OUT are configured above, but
without the capability flag, isn't the feature inaccessible?
Additionally, does tps25990_probe() need to be updated to dynamically scale
the PSC_CURRENT_OUT mantissa with the rimon sense resistor, just like it
does for PSC_CURRENT_IN and PSC_POWER?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522082349.2749970-1-sbogdanov@baylibre.com?part=4
^ permalink raw reply [flat|nested] 9+ messages in thread