Rework existing implementation for calculation of direct
format conversion for TPS25990. With this implamentation
is leveraged code reusability for non standard parameters.
- Add enum for parameter
- Add m, b, R structure to hold value per device
- Add data structure to hold for pmbus_driver_info and
local_direct_values
- Conversion functions are implemented according to formula from
TPS25990 datasheet
- Remove previously used defines replace with structure
Signed-off-by: Stoyan Bogdanov <sbogdanov@baylibre.com>
---
drivers/hwmon/pmbus/tps25990.c | 115 +++++++++++++++++++++++++--------
1 file changed, 88 insertions(+), 27 deletions(-)
diff --git a/drivers/hwmon/pmbus/tps25990.c b/drivers/hwmon/pmbus/tps25990.c
index c13edd7e1abf..33f6367f797c 100644
--- a/drivers/hwmon/pmbus/tps25990.c
+++ b/drivers/hwmon/pmbus/tps25990.c
@@ -36,17 +36,58 @@
#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 tps25990_parameters {
+ TPS25990_VIN_OVF = 0, /* VIN over volatage fault */
+ TPS25990_IIN_OCF, /* IIN Over currect fault */
+ TPS25590_DIRECT_VALUES_MAX, /* Max value ensure there enough space */
+};
+
+struct local_direct_value {
+ int m[TPS25590_DIRECT_VALUES_MAX]; /* mantissa for direct data format */
+ int b[TPS25590_DIRECT_VALUES_MAX]; /* offset */
+ int R[TPS25590_DIRECT_VALUES_MAX]; /* exponent */
+};
+
+struct tps25990_data {
+ struct pmbus_driver_info *info;
+ struct local_direct_value *info_local;
+};
+
+static int tps25990_raw_to_value(struct i2c_client *client, int param, int raw)
+{
+ struct tps25990_data *data = (struct tps25990_data *)of_device_get_match_data(&client->dev);
+ struct local_direct_value *info_local = data->info_local;
+
+ /* Formula : X = (Y / 10^R - b) / m */
+ if (info_local->R[param] >= 0)
+ raw /= int_pow(10, info_local->R[param]);
+ else
+ raw *= int_pow(10, -info_local->R[param]);
+
+ return DIV_ROUND_CLOSEST(raw - info_local->b[param], info_local->m[param]);
+}
+
+static unsigned int tps25990_value_to_raw(struct i2c_client *client, int param, int val)
+{
+ struct tps25990_data *data = (struct tps25990_data *)of_device_get_match_data(&client->dev);
+ struct local_direct_value *info_local = data->info_local;
+
+ /* Formula : Y = ( m * X + b) * 10^R */
+ val = (long)val * info_local->m[param] + info_local->b[param];
+
+ if (info_local->R[param] >= 0)
+ val *= int_pow(10, info_local->R[param]);
+ else
+ val = DIV_ROUND_CLOSEST(val, int_pow(10, -info_local->R[param]));
+
+ return val;
+}
+
/*
* Arbitrary default Rimon value: 1kOhm
* This correspond to an overcurrent limit of 55A, close to the specified limit
@@ -184,9 +225,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_raw_to_value(client, TPS25990_VIN_OVF, ret);
break;
case PMBUS_IIN_OC_FAULT_LIMIT:
@@ -198,9 +237,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_raw_to_value(client, TPS25990_IIN_OCF, ret);
break;
case PMBUS_VIRT_SAMPLES:
@@ -246,17 +283,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_value_to_raw(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_value_to_raw(client, TPS25990_IIN_OCF, value);
value = clamp_val(value, 0, 0x3f);
ret = pmbus_write_byte_data(client, page, TPS25990_VIREF, value);
break;
@@ -337,7 +370,16 @@ static const struct regulator_desc tps25990_reg_desc[] = {
};
#endif
-static const struct pmbus_driver_info tps25990_base_info = {
+struct local_direct_value tps25590_local_info = {
+ .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 = {
.pages = 1,
.format[PSC_VOLTAGE_IN] = direct,
.m[PSC_VOLTAGE_IN] = 5251,
@@ -386,14 +428,19 @@ static const struct pmbus_driver_info tps25990_base_info = {
#endif
};
+struct tps25990_data data_tps25990 = {
+ .info = &tps25990_base_info,
+ .info_local = &tps25590_local_info,
+};
+
static const struct i2c_device_id tps25990_i2c_id[] = {
- { "tps25990" },
+ { .name = "tps25990", .driver_data = (kernel_ulong_t)&data_tps25990 },
{}
};
MODULE_DEVICE_TABLE(i2c, tps25990_i2c_id);
static const struct of_device_id tps25990_of_match[] = {
- { .compatible = "ti,tps25990" },
+ { .compatible = "ti,tps25990", .data = &data_tps25990 },
{}
};
MODULE_DEVICE_TABLE(of, tps25990_of_match);
@@ -401,23 +448,37 @@ 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;
u32 rimon = TPS25990_DEFAULT_RIMON;
+ struct pmbus_driver_info *info_get;
+ struct local_direct_value *info_local_get;
int ret;
ret = device_property_read_u32(dev, "ti,rimon-micro-ohms", &rimon);
if (ret < 0 && ret != -EINVAL)
return dev_err_probe(dev, ret, "failed to get rimon\n");
- info = devm_kmemdup(dev, &tps25990_base_info, sizeof(*info), GFP_KERNEL);
- if (!info)
+ data = (struct tps25990_data *)of_device_get_match_data(dev);
+ if (!data)
+ return -EOPNOTSUPP;
+
+ info_get = data->info;
+ /* Make copy of pmbus_info and replace it to preserve original values */
+ data->info = devm_kmemdup(dev, info_get, sizeof(*info_get), GFP_KERNEL);
+ if (!data->info)
+ return -ENOMEM;
+
+ info_local_get = data->info_local;
+ /* Make copy of pmbus_info and replace it to preserve original values */
+ data->info_local = devm_kmemdup(dev, info_local_get, sizeof(*info_local_get), GFP_KERNEL);
+ if (!data->info_local)
return -ENOMEM;
/* 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.34.1On Tue, Feb 17, 2026 at 10:12:01AM +0200, Stoyan Bogdanov wrote:
> Rework existing implementation for calculation of direct
> format conversion for TPS25990. With this implamentation
> is leveraged code reusability for non standard parameters.
> - Add enum for parameter
> - Add m, b, R structure to hold value per device
> - Add data structure to hold for pmbus_driver_info and
> local_direct_values
> - Conversion functions are implemented according to formula from
> TPS25990 datasheet
> - Remove previously used defines replace with structure
>
> Signed-off-by: Stoyan Bogdanov <sbogdanov@baylibre.com>
> ---
> drivers/hwmon/pmbus/tps25990.c | 115 +++++++++++++++++++++++++--------
> 1 file changed, 88 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/tps25990.c b/drivers/hwmon/pmbus/tps25990.c
> index c13edd7e1abf..33f6367f797c 100644
> --- a/drivers/hwmon/pmbus/tps25990.c
> +++ b/drivers/hwmon/pmbus/tps25990.c
> @@ -36,17 +36,58 @@
> #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 tps25990_parameters {
> + TPS25990_VIN_OVF = 0, /* VIN over volatage fault */
> + TPS25990_IIN_OCF, /* IIN Over currect fault */
> + TPS25590_DIRECT_VALUES_MAX, /* Max value ensure there enough space */
> +};
> +
> +struct local_direct_value {
> + int m[TPS25590_DIRECT_VALUES_MAX]; /* mantissa for direct data format */
> + int b[TPS25590_DIRECT_VALUES_MAX]; /* offset */
> + int R[TPS25590_DIRECT_VALUES_MAX]; /* exponent */
> +};
> +
> +struct tps25990_data {
> + struct pmbus_driver_info *info;
> + struct local_direct_value *info_local;
> +};
> +
> +static int tps25990_raw_to_value(struct i2c_client *client, int param, int raw)
> +{
> + struct tps25990_data *data = (struct tps25990_data *)of_device_get_match_data(&client->dev);
This returns the global data structure (which is corrupted on
multi-instance systems) instead of per-instance data.
> + struct local_direct_value *info_local = data->info_local;
> +
> + /* Formula : X = (Y / 10^R - b) / m */
> + if (info_local->R[param] >= 0)
> + raw /= int_pow(10, info_local->R[param]);
> + else
> + raw *= int_pow(10, -info_local->R[param]);
This will overflow if the exponent is -6 and raw is larger than ~2147.
This may not happen with current parameters, but it is brittle.
> +
> + return DIV_ROUND_CLOSEST(raw - info_local->b[param], info_local->m[param]);
> +}
> +
> +static unsigned int tps25990_value_to_raw(struct i2c_client *client, int param, int val)
> +{
> + struct tps25990_data *data = (struct tps25990_data *)of_device_get_match_data(&client->dev);
> + struct local_direct_value *info_local = data->info_local;
> +
> + /* Formula : Y = ( m * X + b) * 10^R */
> + val = (long)val * info_local->m[param] + info_local->b[param];
This will overflow on 32-bit systems (where long is a 32-bit value).
> +
> + if (info_local->R[param] >= 0)
> + val *= int_pow(10, info_local->R[param]);
> + else
> + val = DIV_ROUND_CLOSEST(val, int_pow(10, -info_local->R[param]));
> +
> + return val;
> +}
> +
> /*
> * Arbitrary default Rimon value: 1kOhm
> * This correspond to an overcurrent limit of 55A, close to the specified limit
> @@ -184,9 +225,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_raw_to_value(client, TPS25990_VIN_OVF, ret);
> break;
>
> case PMBUS_IIN_OC_FAULT_LIMIT:
> @@ -198,9 +237,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_raw_to_value(client, TPS25990_IIN_OCF, ret);
> break;
>
> case PMBUS_VIRT_SAMPLES:
> @@ -246,17 +283,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_value_to_raw(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_value_to_raw(client, TPS25990_IIN_OCF, value);
> value = clamp_val(value, 0, 0x3f);
> ret = pmbus_write_byte_data(client, page, TPS25990_VIREF, value);
> break;
> @@ -337,7 +370,16 @@ static const struct regulator_desc tps25990_reg_desc[] = {
> };
> #endif
>
> -static const struct pmbus_driver_info tps25990_base_info = {
> +struct local_direct_value tps25590_local_info = {
> + .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 = {
> .pages = 1,
> .format[PSC_VOLTAGE_IN] = direct,
> .m[PSC_VOLTAGE_IN] = 5251,
> @@ -386,14 +428,19 @@ static const struct pmbus_driver_info tps25990_base_info = {
> #endif
> };
>
> +struct tps25990_data data_tps25990 = {
> + .info = &tps25990_base_info,
> + .info_local = &tps25590_local_info,
> +};
> +
> static const struct i2c_device_id tps25990_i2c_id[] = {
> - { "tps25990" },
> + { .name = "tps25990", .driver_data = (kernel_ulong_t)&data_tps25990 },
> {}
> };
> MODULE_DEVICE_TABLE(i2c, tps25990_i2c_id);
>
> static const struct of_device_id tps25990_of_match[] = {
> - { .compatible = "ti,tps25990" },
> + { .compatible = "ti,tps25990", .data = &data_tps25990 },
> {}
> };
> MODULE_DEVICE_TABLE(of, tps25990_of_match);
> @@ -401,23 +448,37 @@ 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;
> u32 rimon = TPS25990_DEFAULT_RIMON;
> + struct pmbus_driver_info *info_get;
> + struct local_direct_value *info_local_get;
> int ret;
>
> ret = device_property_read_u32(dev, "ti,rimon-micro-ohms", &rimon);
> if (ret < 0 && ret != -EINVAL)
> return dev_err_probe(dev, ret, "failed to get rimon\n");
>
> - info = devm_kmemdup(dev, &tps25990_base_info, sizeof(*info), GFP_KERNEL);
> - if (!info)
> + data = (struct tps25990_data *)of_device_get_match_data(dev);
> + if (!data)
> + return -EOPNOTSUPP;
By calling of_device_get_match_data() which returns NULL on non-devicetree
systems, this patch breaks support for such systems.
> +
> + info_get = data->info;
> + /* Make copy of pmbus_info and replace it to preserve original values */
> + data->info = devm_kmemdup(dev, info_get, sizeof(*info_get), GFP_KERNEL);
This overwrites information in the global data structure returned by
of_device_get_match_data(). When multiple instances of the device exist,
each subsequent probe overwrites the global pointer. If any device is
removed, the global pointer now points to freed memory, leading to a
Use-After-Free when other instances access it via of_device_get_match_data
or during their own removal/re-probe.
This also completely breaks multi-instance support.
> + if (!data->info)
> + return -ENOMEM;
> +
> + info_local_get = data->info_local;
> + /* Make copy of pmbus_info and replace it to preserve original values */
> + data->info_local = devm_kmemdup(dev, info_local_get, sizeof(*info_local_get), GFP_KERNEL);
> + if (!data->info_local)
> return -ENOMEM;
>
> /* 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 = {
On Sun, Mar 8, 2026 at 7:27 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Tue, Feb 17, 2026 at 10:12:01AM +0200, Stoyan Bogdanov wrote:
> > Rework existing implementation for calculation of direct
> > format conversion for TPS25990. With this implamentation
> > is leveraged code reusability for non standard parameters.
> > - Add enum for parameter
> > - Add m, b, R structure to hold value per device
> > - Add data structure to hold for pmbus_driver_info and
> > local_direct_values
> > - Conversion functions are implemented according to formula from
> > TPS25990 datasheet
> > - Remove previously used defines replace with structure
> >
> > Signed-off-by: Stoyan Bogdanov <sbogdanov@baylibre.com>
> > ---
> > drivers/hwmon/pmbus/tps25990.c | 115 +++++++++++++++++++++++++--------
> > 1 file changed, 88 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/hwmon/pmbus/tps25990.c b/drivers/hwmon/pmbus/tps25990.c
> > index c13edd7e1abf..33f6367f797c 100644
> > --- a/drivers/hwmon/pmbus/tps25990.c
> > +++ b/drivers/hwmon/pmbus/tps25990.c
> > @@ -36,17 +36,58 @@
> > #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 tps25990_parameters {
> > + TPS25990_VIN_OVF = 0, /* VIN over volatage fault */
> > + TPS25990_IIN_OCF, /* IIN Over currect fault */
> > + TPS25590_DIRECT_VALUES_MAX, /* Max value ensure there enough space */
> > +};
> > +
> > +struct local_direct_value {
> > + int m[TPS25590_DIRECT_VALUES_MAX]; /* mantissa for direct data format */
> > + int b[TPS25590_DIRECT_VALUES_MAX]; /* offset */
> > + int R[TPS25590_DIRECT_VALUES_MAX]; /* exponent */
> > +};
> > +
> > +struct tps25990_data {
> > + struct pmbus_driver_info *info;
> > + struct local_direct_value *info_local;
> > +};
> > +
> > +static int tps25990_raw_to_value(struct i2c_client *client, int param, int raw)
> > +{
> > + struct tps25990_data *data = (struct tps25990_data *)of_device_get_match_data(&client->dev);
>
> This returns the global data structure (which is corrupted on
> multi-instance systems) instead of per-instance data.
Noted, will fix that in next revision.
> > + struct local_direct_value *info_local = data->info_local;
> > +
> > + /* Formula : X = (Y / 10^R - b) / m */
> > + if (info_local->R[param] >= 0)
> > + raw /= int_pow(10, info_local->R[param]);
> > + else
> > + raw *= int_pow(10, -info_local->R[param]);
>
> This will overflow if the exponent is -6 and raw is larger than ~2147.
> This may not happen with current parameters, but it is brittle.
>
I understand that the whole calculation is fragile
Another approach will be to break it in to phases, but it will lost precision
// make sure R is positive
u32 r_pos = info_local->R[param] >= 0 ? info_local->R[param] :
-info_local->R[param];
// if (r_pos < 9) will overflow 32bit
u32 r_pow = int_pow(10, r_pos);
if (info_local->R[param] >= 0) {
val = DIV_ROUND_CLOSEST_ULL(raw , r_pow); // result should fit in 32bit
val -= info_local->b[param]; // fit 32bit
val = DIV_ROUND_CLOSEST(val , info_local->m[param]); // fit 32bit
} else { // info_local->R[param] < 0
// val = raw * r_pos; // most likely will overflow 32 bit with big numbers
// raw * r_pow / m
// assume r_pow is bigger number
val = DIV_ROUND_CLOSEST(r_pow , info_local->m[param]); // fit 32bit
val = raw * val;
b_div = DIV_ROUND_CLOSEST(info_local->b[param],
info_local->m[param]); // fit 32bit
val -= b_div;
}
> > +
> > + return DIV_ROUND_CLOSEST(raw - info_local->b[param], info_local->m[param]);
> > +}
> > +
> > +static unsigned int tps25990_value_to_raw(struct i2c_client *client, int param, int val)
> > +{
> > + struct tps25990_data *data = (struct tps25990_data *)of_device_get_match_data(&client->dev);
> > + struct local_direct_value *info_local = data->info_local;
> > +
> > + /* Formula : Y = ( m * X + b) * 10^R */
> > + val = (long)val * info_local->m[param] + info_local->b[param];
>
> This will overflow on 32-bit systems (where long is a 32-bit value).
>
Similar approach could be used as in tps25990_raw_to_value
> > +
> > + if (info_local->R[param] >= 0)
> > + val *= int_pow(10, info_local->R[param]);
> > + else
> > + val = DIV_ROUND_CLOSEST(val, int_pow(10, -info_local->R[param]));
> > +
> > + return val;
> > +}
> > +
> > /*
> > * Arbitrary default Rimon value: 1kOhm
> > * This correspond to an overcurrent limit of 55A, close to the specified limit
> > @@ -184,9 +225,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_raw_to_value(client, TPS25990_VIN_OVF, ret);
> > break;
> >
> > case PMBUS_IIN_OC_FAULT_LIMIT:
> > @@ -198,9 +237,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_raw_to_value(client, TPS25990_IIN_OCF, ret);
> > break;
> >
> > case PMBUS_VIRT_SAMPLES:
> > @@ -246,17 +283,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_value_to_raw(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_value_to_raw(client, TPS25990_IIN_OCF, value);
> > value = clamp_val(value, 0, 0x3f);
> > ret = pmbus_write_byte_data(client, page, TPS25990_VIREF, value);
> > break;
> > @@ -337,7 +370,16 @@ static const struct regulator_desc tps25990_reg_desc[] = {
> > };
> > #endif
> >
> > -static const struct pmbus_driver_info tps25990_base_info = {
> > +struct local_direct_value tps25590_local_info = {
> > + .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 = {
> > .pages = 1,
> > .format[PSC_VOLTAGE_IN] = direct,
> > .m[PSC_VOLTAGE_IN] = 5251,
> > @@ -386,14 +428,19 @@ static const struct pmbus_driver_info tps25990_base_info = {
> > #endif
> > };
> >
> > +struct tps25990_data data_tps25990 = {
> > + .info = &tps25990_base_info,
> > + .info_local = &tps25590_local_info,
> > +};
> > +
> > static const struct i2c_device_id tps25990_i2c_id[] = {
> > - { "tps25990" },
> > + { .name = "tps25990", .driver_data = (kernel_ulong_t)&data_tps25990 },
> > {}
> > };
> > MODULE_DEVICE_TABLE(i2c, tps25990_i2c_id);
> >
> > static const struct of_device_id tps25990_of_match[] = {
> > - { .compatible = "ti,tps25990" },
> > + { .compatible = "ti,tps25990", .data = &data_tps25990 },
> > {}
> > };
> > MODULE_DEVICE_TABLE(of, tps25990_of_match);
> > @@ -401,23 +448,37 @@ 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;
> > u32 rimon = TPS25990_DEFAULT_RIMON;
> > + struct pmbus_driver_info *info_get;
> > + struct local_direct_value *info_local_get;
> > int ret;
> >
> > ret = device_property_read_u32(dev, "ti,rimon-micro-ohms", &rimon);
> > if (ret < 0 && ret != -EINVAL)
> > return dev_err_probe(dev, ret, "failed to get rimon\n");
> >
> > - info = devm_kmemdup(dev, &tps25990_base_info, sizeof(*info), GFP_KERNEL);
> > - if (!info)
> > + data = (struct tps25990_data *)of_device_get_match_data(dev);
> > + if (!data)
> > + return -EOPNOTSUPP;
>
> By calling of_device_get_match_data() which returns NULL on non-devicetree
> systems, this patch breaks support for such systems.
>
I understand , will fix that in next revision
> > +
> > + info_get = data->info;
> > + /* Make copy of pmbus_info and replace it to preserve original values */
> > + data->info = devm_kmemdup(dev, info_get, sizeof(*info_get), GFP_KERNEL);
>
> This overwrites information in the global data structure returned by
> of_device_get_match_data(). When multiple instances of the device exist,
> each subsequent probe overwrites the global pointer. If any device is
> removed, the global pointer now points to freed memory, leading to a
> Use-After-Free when other instances access it via of_device_get_match_data
> or during their own removal/re-probe.
>
> This also completely breaks multi-instance support.
>
I understand , will fix that in next revision
> > + if (!data->info)
> > + return -ENOMEM;
> > +
> > + info_local_get = data->info_local;
> > + /* Make copy of pmbus_info and replace it to preserve original values */
> > + data->info_local = devm_kmemdup(dev, info_local_get, sizeof(*info_local_get), GFP_KERNEL);
> > + if (!data->info_local)
> > return -ENOMEM;
> >
> > /* 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 = {
© 2016 - 2026 Red Hat, Inc.