On Wed, Apr 27, 2022 at 03:26:56PM +0200, Guenter Roeck wrote: > On Wed, Apr 27, 2022 at 03:02:11PM +0200, Mårten Lindahl wrote: > > Pmbus drivers using the default pmbus_regulator_ops for the enable/ > > disable/is_enabled functions will use the standard pmbus core functions > > pmbus_read/write_byte_data. This could potentially influence some > > specific regulator chips that for example need a time delay before each > > data access. > > > > Lets add support for drivers to use chip specific read/write operations > > when using the standard pmbus_regulator_ops. > > The subject is misleading. It should be something like "introduce and > use write_byte_data callback". Also, existing code calling > pmbus_write_byte_data() should call _pmbus_write_byte_data() instead. > This applies to pmbus_update_fan() and pmbus_get_boolean(). Ok, I will change the subject to your suggestion, and change those two functions. Thanks! Kind regards Mårten > > Thanks, > Guenter > > > > > Signed-off-by: Mårten Lindahl <marten.lindahl@xxxxxxxx> > > --- > > drivers/hwmon/pmbus/pmbus.h | 2 ++ > > drivers/hwmon/pmbus/pmbus_core.c | 58 +++++++++++++++++++++----------- > > 2 files changed, 40 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h > > index e74b6ef070f3..c031a9700ace 100644 > > --- a/drivers/hwmon/pmbus/pmbus.h > > +++ b/drivers/hwmon/pmbus/pmbus.h > > @@ -438,6 +438,8 @@ struct pmbus_driver_info { > > int (*read_byte_data)(struct i2c_client *client, int page, int reg); > > int (*read_word_data)(struct i2c_client *client, int page, int phase, > > int reg); > > + int (*write_byte_data)(struct i2c_client *client, int page, int reg, > > + u8 byte); > > int (*write_word_data)(struct i2c_client *client, int page, int reg, > > u16 word); > > int (*write_byte)(struct i2c_client *client, int page, u8 value); > > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c > > index b2618b1d529e..1b0728c3c7d8 100644 > > --- a/drivers/hwmon/pmbus/pmbus_core.c > > +++ b/drivers/hwmon/pmbus/pmbus_core.c > > @@ -384,25 +384,6 @@ int pmbus_write_byte_data(struct i2c_client *client, int page, u8 reg, u8 value) > > } > > EXPORT_SYMBOL_NS_GPL(pmbus_write_byte_data, PMBUS); > > > > -int pmbus_update_byte_data(struct i2c_client *client, int page, u8 reg, > > - u8 mask, u8 value) > > -{ > > - unsigned int tmp; > > - int rv; > > - > > - rv = pmbus_read_byte_data(client, page, reg); > > - if (rv < 0) > > - return rv; > > - > > - tmp = (rv & ~mask) | (value & mask); > > - > > - if (tmp != rv) > > - rv = pmbus_write_byte_data(client, page, reg, tmp); > > - > > - return rv; > > -} > > -EXPORT_SYMBOL_NS_GPL(pmbus_update_byte_data, PMBUS); > > - > > /* > > * _pmbus_read_byte_data() is similar to pmbus_read_byte_data(), but checks if > > * a device specific mapping function exists and calls it if necessary. > > @@ -421,6 +402,43 @@ static int _pmbus_read_byte_data(struct i2c_client *client, int page, int reg) > > return pmbus_read_byte_data(client, page, reg); > > } > > > > +/* > > + * _pmbus_write_byte_data() is similar to pmbus_write_byte_data(), but checks if > > + * a device specific mapping function exists and calls it if necessary. > > + */ > > +static int _pmbus_write_byte_data(struct i2c_client *client, int page, int reg, u8 value) > > +{ > > + struct pmbus_data *data = i2c_get_clientdata(client); > > + const struct pmbus_driver_info *info = data->info; > > + int status; > > + > > + if (info->write_byte_data) { > > + status = info->write_byte_data(client, page, reg, value); > > + if (status != -ENODATA) > > + return status; > > + } > > + return pmbus_write_byte_data(client, page, reg, value); > > +} > > + > > +int pmbus_update_byte_data(struct i2c_client *client, int page, u8 reg, > > + u8 mask, u8 value) > > +{ > > + unsigned int tmp; > > + int rv; > > + > > + rv = _pmbus_read_byte_data(client, page, reg); > > + if (rv < 0) > > + return rv; > > + > > + tmp = (rv & ~mask) | (value & mask); > > + > > + if (tmp != rv) > > + rv = _pmbus_write_byte_data(client, page, reg, tmp); > > + > > + return rv; > > +} > > +EXPORT_SYMBOL_NS_GPL(pmbus_update_byte_data, PMBUS); > > + > > static struct pmbus_sensor *pmbus_find_sensor(struct pmbus_data *data, int page, > > int reg) > > { > > @@ -2396,7 +2414,7 @@ static int pmbus_regulator_is_enabled(struct regulator_dev *rdev) > > int ret; > > > > mutex_lock(&data->update_lock); > > - ret = pmbus_read_byte_data(client, page, PMBUS_OPERATION); > > + ret = _pmbus_read_byte_data(client, page, PMBUS_OPERATION); > > mutex_unlock(&data->update_lock); > > > > if (ret < 0)