| Message ID | 20220610114732.2370242-4-marten.lindahl@axis.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon: (pmbus/ltc2978) Set voltage resolution | expand |
On 6/10/22 04:47, Mårten Lindahl wrote: > The LTC2977 regulator does not set the regulator_desc .n_voltages value > which is needed in order to let the regulator core list the regulator > voltage range. > > This patch defines a regulator_desc with a voltage range, and uses it > for defining voltage resolution for regulators LTC2972/LTC2974/LTC2975/ > LTC2977/LTC2978/LTC2979/LTC2980/LTM2987 based on that they all have a 16 > bit ADC with the same stepwise 122.07uV resolution. It also scales the > resolution to a 1mV resolution which is easier to handle. > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > --- > drivers/hwmon/pmbus/ltc2978.c | 56 ++++++++++++++++++++++++++++++++--- > 1 file changed, 52 insertions(+), 4 deletions(-) > > diff --git a/drivers/hwmon/pmbus/ltc2978.c b/drivers/hwmon/pmbus/ltc2978.c > index 531aa674a928..7d44e64c61c1 100644 > --- a/drivers/hwmon/pmbus/ltc2978.c > +++ b/drivers/hwmon/pmbus/ltc2978.c > @@ -562,7 +562,36 @@ static const struct i2c_device_id ltc2978_id[] = { > MODULE_DEVICE_TABLE(i2c, ltc2978_id); > > #if IS_ENABLED(CONFIG_SENSORS_LTC2978_REGULATOR) > +#define LTC2978_ADC_RES 0xFFFF > +#define LTC2978_N_ADC 122 > +#define LTC2978_MAX_UV (LTC2978_ADC_RES * LTC2978_N_ADC) > +#define LTC2978_UV_STEP 1000 > + > +#define PMBUS_LTC2978_REGULATOR(_name, _id) \ > + [_id] = { \ > + .name = (_name # _id), \ This needs 'supply_name = "vin"'. See commit 54cc3dbfc10dc ("hwmon: (pmbus) Add regulator supply into macro"). > + .id = (_id), \ > + .of_match = of_match_ptr(_name # _id), \ > + .regulators_node = of_match_ptr("regulators"), \ > + .ops = &pmbus_regulator_ops, \ > + .type = REGULATOR_VOLTAGE, \ > + .owner = THIS_MODULE, \ > + .n_voltages = (LTC2978_MAX_UV / LTC2978_UV_STEP) + 1, \ > + .uV_step = LTC2978_UV_STEP, \ > + } Please introduce a new generic macro PMBUS_REGULATOR_STEP() with additional parameters n_voltages and uV_step and use it here. PMBUS_REGULATOR() can then be redefined as #define PMBUS_REGULATOR(n, i) PMBUS_REGULATOR_STEP(n, i, 0, 0) Thanks, Guenter > + > static const struct regulator_desc ltc2978_reg_desc[] = { > + PMBUS_LTC2978_REGULATOR("vout", 0), > + PMBUS_LTC2978_REGULATOR("vout", 1), > + PMBUS_LTC2978_REGULATOR("vout", 2), > + PMBUS_LTC2978_REGULATOR("vout", 3), > + PMBUS_LTC2978_REGULATOR("vout", 4), > + PMBUS_LTC2978_REGULATOR("vout", 5), > + PMBUS_LTC2978_REGULATOR("vout", 6), > + PMBUS_LTC2978_REGULATOR("vout", 7), > +}; > + > +static const struct regulator_desc ltc2978_reg_desc_default[] = { > PMBUS_REGULATOR("vout", 0), > PMBUS_REGULATOR("vout", 1), > PMBUS_REGULATOR("vout", 2), > @@ -839,10 +868,29 @@ static int ltc2978_probe(struct i2c_client *client) > > #if IS_ENABLED(CONFIG_SENSORS_LTC2978_REGULATOR) > info->num_regulators = info->pages; > - info->reg_desc = ltc2978_reg_desc; > - if (info->num_regulators > ARRAY_SIZE(ltc2978_reg_desc)) { > - dev_err(&client->dev, "num_regulators too large!"); > - info->num_regulators = ARRAY_SIZE(ltc2978_reg_desc); > + switch (data->id) { > + case ltc2972: > + case ltc2974: > + case ltc2975: > + case ltc2977: > + case ltc2978: > + case ltc2979: > + case ltc2980: > + case ltm2987: > + info->reg_desc = ltc2978_reg_desc; > + if (info->num_regulators > ARRAY_SIZE(ltc2978_reg_desc)) { > + dev_warn(&client->dev, "num_regulators too large!"); > + info->num_regulators = ARRAY_SIZE(ltc2978_reg_desc); > + } > + break; > + default: > + info->reg_desc = ltc2978_reg_desc_default; > + if (info->num_regulators > ARRAY_SIZE(ltc2978_reg_desc_default)) { > + dev_warn(&client->dev, "num_regulators too large!"); > + info->num_regulators = > + ARRAY_SIZE(ltc2978_reg_desc_default); > + } > + break; > } > #endif >
On Fri, Jun 10, 2022 at 04:27:55PM +0200, Guenter Roeck wrote: > On 6/10/22 04:47, Mårten Lindahl wrote: > > The LTC2977 regulator does not set the regulator_desc .n_voltages value > > which is needed in order to let the regulator core list the regulator > > voltage range. > > > > This patch defines a regulator_desc with a voltage range, and uses it > > for defining voltage resolution for regulators LTC2972/LTC2974/LTC2975/ > > LTC2977/LTC2978/LTC2979/LTC2980/LTM2987 based on that they all have a 16 > > bit ADC with the same stepwise 122.07uV resolution. It also scales the > > resolution to a 1mV resolution which is easier to handle. > > > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > > --- > > drivers/hwmon/pmbus/ltc2978.c | 56 ++++++++++++++++++++++++++++++++--- > > 1 file changed, 52 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/hwmon/pmbus/ltc2978.c b/drivers/hwmon/pmbus/ltc2978.c > > index 531aa674a928..7d44e64c61c1 100644 > > --- a/drivers/hwmon/pmbus/ltc2978.c > > +++ b/drivers/hwmon/pmbus/ltc2978.c > > @@ -562,7 +562,36 @@ static const struct i2c_device_id ltc2978_id[] = { > > MODULE_DEVICE_TABLE(i2c, ltc2978_id); > > > > #if IS_ENABLED(CONFIG_SENSORS_LTC2978_REGULATOR) > > +#define LTC2978_ADC_RES 0xFFFF > > +#define LTC2978_N_ADC 122 > > +#define LTC2978_MAX_UV (LTC2978_ADC_RES * LTC2978_N_ADC) > > +#define LTC2978_UV_STEP 1000 > > + > > +#define PMBUS_LTC2978_REGULATOR(_name, _id) \ > > + [_id] = { \ > > + .name = (_name # _id), \ > > This needs 'supply_name = "vin"'. See commit 54cc3dbfc10dc ("hwmon: > (pmbus) Add regulator supply into macro"). > > > + .id = (_id), \ > > + .of_match = of_match_ptr(_name # _id), \ > > + .regulators_node = of_match_ptr("regulators"), \ > > + .ops = &pmbus_regulator_ops, \ > > + .type = REGULATOR_VOLTAGE, \ > > + .owner = THIS_MODULE, \ > > + .n_voltages = (LTC2978_MAX_UV / LTC2978_UV_STEP) + 1, \ > > + .uV_step = LTC2978_UV_STEP, \ > > + } > > Please introduce a new generic macro PMBUS_REGULATOR_STEP() > with additional parameters n_voltages and uV_step and use it here. > PMBUS_REGULATOR() can then be redefined as > > #define PMBUS_REGULATOR(n, i) PMBUS_REGULATOR_STEP(n, i, 0, 0) Hi Guenter! Yes, that's a good idea. I will do that, and then there wont be any risk of missing attributes like this new 'supply_name' as it is only defined at one place. Kind regards Mårten > > Thanks, > Guenter > > > + > > static const struct regulator_desc ltc2978_reg_desc[] = { > > + PMBUS_LTC2978_REGULATOR("vout", 0), > > + PMBUS_LTC2978_REGULATOR("vout", 1), > > + PMBUS_LTC2978_REGULATOR("vout", 2), > > + PMBUS_LTC2978_REGULATOR("vout", 3), > > + PMBUS_LTC2978_REGULATOR("vout", 4), > > + PMBUS_LTC2978_REGULATOR("vout", 5), > > + PMBUS_LTC2978_REGULATOR("vout", 6), > > + PMBUS_LTC2978_REGULATOR("vout", 7), > > +}; > > + > > +static const struct regulator_desc ltc2978_reg_desc_default[] = { > > PMBUS_REGULATOR("vout", 0), > > PMBUS_REGULATOR("vout", 1), > > PMBUS_REGULATOR("vout", 2), > > @@ -839,10 +868,29 @@ static int ltc2978_probe(struct i2c_client *client) > > > > #if IS_ENABLED(CONFIG_SENSORS_LTC2978_REGULATOR) > > info->num_regulators = info->pages; > > - info->reg_desc = ltc2978_reg_desc; > > - if (info->num_regulators > ARRAY_SIZE(ltc2978_reg_desc)) { > > - dev_err(&client->dev, "num_regulators too large!"); > > - info->num_regulators = ARRAY_SIZE(ltc2978_reg_desc); > > + switch (data->id) { > > + case ltc2972: > > + case ltc2974: > > + case ltc2975: > > + case ltc2977: > > + case ltc2978: > > + case ltc2979: > > + case ltc2980: > > + case ltm2987: > > + info->reg_desc = ltc2978_reg_desc; > > + if (info->num_regulators > ARRAY_SIZE(ltc2978_reg_desc)) { > > + dev_warn(&client->dev, "num_regulators too large!"); > > + info->num_regulators = ARRAY_SIZE(ltc2978_reg_desc); > > + } > > + break; > > + default: > > + info->reg_desc = ltc2978_reg_desc_default; > > + if (info->num_regulators > ARRAY_SIZE(ltc2978_reg_desc_default)) { > > + dev_warn(&client->dev, "num_regulators too large!"); > > + info->num_regulators = > > + ARRAY_SIZE(ltc2978_reg_desc_default); > > + } > > + break; > > } > > #endif > > >
diff --git a/drivers/hwmon/pmbus/ltc2978.c b/drivers/hwmon/pmbus/ltc2978.c index 531aa674a928..7d44e64c61c1 100644 --- a/drivers/hwmon/pmbus/ltc2978.c +++ b/drivers/hwmon/pmbus/ltc2978.c @@ -562,7 +562,36 @@ static const struct i2c_device_id ltc2978_id[] = { MODULE_DEVICE_TABLE(i2c, ltc2978_id); #if IS_ENABLED(CONFIG_SENSORS_LTC2978_REGULATOR) +#define LTC2978_ADC_RES 0xFFFF +#define LTC2978_N_ADC 122 +#define LTC2978_MAX_UV (LTC2978_ADC_RES * LTC2978_N_ADC) +#define LTC2978_UV_STEP 1000 + +#define PMBUS_LTC2978_REGULATOR(_name, _id) \ + [_id] = { \ + .name = (_name # _id), \ + .id = (_id), \ + .of_match = of_match_ptr(_name # _id), \ + .regulators_node = of_match_ptr("regulators"), \ + .ops = &pmbus_regulator_ops, \ + .type = REGULATOR_VOLTAGE, \ + .owner = THIS_MODULE, \ + .n_voltages = (LTC2978_MAX_UV / LTC2978_UV_STEP) + 1, \ + .uV_step = LTC2978_UV_STEP, \ + } + static const struct regulator_desc ltc2978_reg_desc[] = { + PMBUS_LTC2978_REGULATOR("vout", 0), + PMBUS_LTC2978_REGULATOR("vout", 1), + PMBUS_LTC2978_REGULATOR("vout", 2), + PMBUS_LTC2978_REGULATOR("vout", 3), + PMBUS_LTC2978_REGULATOR("vout", 4), + PMBUS_LTC2978_REGULATOR("vout", 5), + PMBUS_LTC2978_REGULATOR("vout", 6), + PMBUS_LTC2978_REGULATOR("vout", 7), +}; + +static const struct regulator_desc ltc2978_reg_desc_default[] = { PMBUS_REGULATOR("vout", 0), PMBUS_REGULATOR("vout", 1), PMBUS_REGULATOR("vout", 2), @@ -839,10 +868,29 @@ static int ltc2978_probe(struct i2c_client *client) #if IS_ENABLED(CONFIG_SENSORS_LTC2978_REGULATOR) info->num_regulators = info->pages; - info->reg_desc = ltc2978_reg_desc; - if (info->num_regulators > ARRAY_SIZE(ltc2978_reg_desc)) { - dev_err(&client->dev, "num_regulators too large!"); - info->num_regulators = ARRAY_SIZE(ltc2978_reg_desc); + switch (data->id) { + case ltc2972: + case ltc2974: + case ltc2975: + case ltc2977: + case ltc2978: + case ltc2979: + case ltc2980: + case ltm2987: + info->reg_desc = ltc2978_reg_desc; + if (info->num_regulators > ARRAY_SIZE(ltc2978_reg_desc)) { + dev_warn(&client->dev, "num_regulators too large!"); + info->num_regulators = ARRAY_SIZE(ltc2978_reg_desc); + } + break; + default: + info->reg_desc = ltc2978_reg_desc_default; + if (info->num_regulators > ARRAY_SIZE(ltc2978_reg_desc_default)) { + dev_warn(&client->dev, "num_regulators too large!"); + info->num_regulators = + ARRAY_SIZE(ltc2978_reg_desc_default); + } + break; } #endif
The LTC2977 regulator does not set the regulator_desc .n_voltages value which is needed in order to let the regulator core list the regulator voltage range. This patch defines a regulator_desc with a voltage range, and uses it for defining voltage resolution for regulators LTC2972/LTC2974/LTC2975/ LTC2977/LTC2978/LTC2979/LTC2980/LTM2987 based on that they all have a 16 bit ADC with the same stepwise 122.07uV resolution. It also scales the resolution to a 1mV resolution which is easier to handle. Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> --- drivers/hwmon/pmbus/ltc2978.c | 56 ++++++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 4 deletions(-)