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

[Marten Lindahl <martenli@axis.com>]

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);
> >   
>