Re: [PATCH v3 3/3] hwmon: (pmbus) Add get_voltage/set_voltage ops

Marten Lindahl
2022-04-28T13:04:21Z

On Wed, Apr 27, 2022 at 03:10:30PM +0200, Guenter Roeck wrote:
> On 4/27/22 06:02, Mårten Lindahl wrote:
> > The pmbus core does not have operations for getting or setting voltage.
> > Add functions get/set voltage for the dynamic regulator framework.
> >
> > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
> > ---
> > drivers/hwmon/pmbus/pmbus_core.c | 43 ++++++++++++++++++++++++++++++++
> > 1 file changed, 43 insertions(+)
> >
> > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> > index 1b0728c3c7d8..afc238faa8ce 100644
> > --- a/drivers/hwmon/pmbus/pmbus_core.c
> > +++ b/drivers/hwmon/pmbus/pmbus_core.c
> > @@ -2563,11 +2563,54 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
> > return 0;
> > }
> >
> > +static int pmbus_regulator_get_voltage(struct regulator_dev *rdev)
> > +{
> > + struct device *dev = rdev_get_dev(rdev);
> > + struct i2c_client *client = to_i2c_client(dev->parent);
> > + u8 page = rdev_get_id(rdev);
> > + int ret;
> > +
> > + ret = _pmbus_read_word_data(client, page, 0xff, PMBUS_READ_VOUT);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret *= 1000;
> > +
> > + return ((ret >> 13) * 1000);
>
> That will need to use voltage conversion functions. VOUT isn't the same
> for all chips.
>
Ok, I will fix that. Will change it to use pmbus_reg2data()

> > +}
> > +
> > +static int pmbus_regulator_set_voltage(struct regulator_dev *rdev, int min_uV,
> > + int max_uV, unsigned int *selector)
> > +{
> > + struct device *dev = rdev_get_dev(rdev);
> > + struct i2c_client *client = to_i2c_client(dev->parent);
> > + u8 page = rdev_get_id(rdev);
> > + long tmp = DIV_ROUND_CLOSEST(min_uV, 1000);
> > + u32 val = DIV_ROUND_CLOSEST(tmp << 13, 1000);
>
> Same as above.

I will make it use pmbus_data2reg() for conversion.

Kind regards
Mårten
>
> > + int ret;
> > + *selector = 0;
> > +
> > + ret = _pmbus_read_word_data(client, page, 0xff, PMBUS_VOUT_MARGIN_LOW);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* Select the voltage closest to min_uV */
> > + if (ret > val)
> > + val = ret;
> > +
> > + ret = _pmbus_write_word_data(client, page, PMBUS_VOUT_COMMAND,
> > + (u16)val);
> > +
> > + return ret;
> > +}
> > +
> > const struct regulator_ops pmbus_regulator_ops = {
> > .enable = pmbus_regulator_enable,
> > .disable = pmbus_regulator_disable,
> > .is_enabled = pmbus_regulator_is_enabled,
> > .get_error_flags = pmbus_regulator_get_error_flags,
> > + .get_voltage = pmbus_regulator_get_voltage,
> > + .set_voltage = pmbus_regulator_set_voltage,
> > };
> > EXPORT_SYMBOL_NS_GPL(pmbus_regulator_ops, PMBUS);
> >
>