On 4/28/22 07:40, 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@xxxxxxxx>
---
drivers/hwmon/pmbus/pmbus_core.c | 63 ++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index bd143ca0c320..fe7dbb496e3b 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -1531,6 +1531,11 @@ static const struct pmbus_sensor_attr voltage_attributes[] = {
.gbit = PB_STATUS_VOUT_OV,
.limit = vout_limit_attrs,
.nlimit = ARRAY_SIZE(vout_limit_attrs),
+ }, {
+ .reg = PMBUS_VOUT_COMMAND,
+ .class = PSC_VOLTAGE_OUT,
+ .paged = true,
+ .func = PMBUS_HAVE_VOUT,
}
Ok, you lost me here. This adds an inX_input attribute. Why ? This is completely
unrelated to the intended scope of this patch. It also doesn't report a measured
voltage, but a configuration value. If anything, it would have to be a separate
patch, and you'd have to argue hard why it makes sense to report it as measured
voltage.
};
@@ -2563,11 +2568,69 @@ 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);
+ struct pmbus_data *data = i2c_get_clientdata(client);
+ struct pmbus_sensor *sensor;
+ u8 page = rdev_get_id(rdev);
+ int ret;
+
+ sensor = pmbus_find_sensor(data, page, PMBUS_READ_VOUT);
+ if (IS_ERR(sensor))
+ return -ENODATA;
+
+ mutex_lock(&data->update_lock);
+ pmbus_update_sensor_data(client, sensor);
+ if (sensor->data < 0)
+ ret = sensor->data;
+ else
+ ret = (int)pmbus_reg2data(data, sensor) * 1000; /* unit is uV */
+ mutex_unlock(&data->update_lock);
+
Same question. Why ?
+ return ret;
+}
+
+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);
+ struct pmbus_data *data = i2c_get_clientdata(client);
+ struct pmbus_sensor *sensor;
+ u8 page = rdev_get_id(rdev);
+ s64 tmp = DIV_ROUND_CLOSEST_ULL(min_uV, 1000); /* convert to mV */
+ u16 val;
+ int ret;
+ *selector = 0;
+
+ sensor = pmbus_find_sensor(data, page, PMBUS_VOUT_COMMAND);
+ if (IS_ERR(sensor))
+ return -ENODATA;
+
+ ret = _pmbus_read_word_data(client, page, 0xff, PMBUS_VOUT_MARGIN_LOW);
+ if (ret < 0)
+ return ret;
+
That actually makes me wonder: What about VOUT_MARGIN_HIGH ?
Also, there are optional MFR_VOUT_MIN and MFR_VOUT_MAX registers.
Would it possibly make sense to determine the valid range once
during probe and then compare against it ?
Thanks,
Guenter
+ val = pmbus_data2reg(data, sensor, tmp);
+
+ /* Do not fall shorter than low margin */
+ if (ret > val)
+ val = ret;
+
+ ret = _pmbus_write_word_data(client, page, PMBUS_VOUT_COMMAND, 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);
