| Message ID | 20240126205714.2363-1-aladyshev22@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v3] hwmon: (pmbus/mp2975) Fix driver initialization for MP2975 device | expand |
On Fri, Jan 26, 2024 at 11:57:14PM +0300, Konstantin Aladyshev wrote: > The commit 1feb31e810b0 ("hwmon: (pmbus/mp2975) Simplify VOUT code") > has introduced a bug that makes it impossible to initialize MP2975 > device: > """ > mp2975 5-0020: Failed to identify chip capabilities > i2c i2c-5: new_device: Instantiated device mp2975 at 0x20 > i2c i2c-5: delete_device: Deleting device mp2975 at 0x20 > """ > Since the 'read_byte_data' function was removed from the > 'pmbus_driver_info ' structure the driver no longer reports correctly > that VOUT mode is direct. Therefore 'pmbus_identify_common' fails > with error, making it impossible to initialize the device. > > Restore 'read_byte_data' function to fix the issue. > > Tested: > - before: it is not possible to initialize MP2975 device with the > 'mp2975' driver, > - after: 'mp2975' correctly initializes MP2975 device and all sensor > data is correct. > > Fixes: 1feb31e810b0 ("hwmon: (pmbus/mp2975) Simplify VOUT code") > > Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com> Applied. Thanks, Guenter
Hi Konstantin, Thank you for fixing this regression. The comment is no longer true as the driver doesn't internally convert from VID to direct, but rather configures READ_VOUT using MFR_DC_LOOP_CTRL. The comment thus should read as the following: Report direct format as configured by MFR_DC_LOOP_CTRL. Unlike on MP2971/MP2973 the reported VOUT_MODE isn't automatically internally updated, but always reads as PB_VOUT_MODE_VID. Regards, Patrick On Fri, Jan 26, 2024 at 9:59 PM Konstantin Aladyshev <aladyshev22@gmail.com> wrote: > > The commit 1feb31e810b0 ("hwmon: (pmbus/mp2975) Simplify VOUT code") > has introduced a bug that makes it impossible to initialize MP2975 > device: > """ > mp2975 5-0020: Failed to identify chip capabilities > i2c i2c-5: new_device: Instantiated device mp2975 at 0x20 > i2c i2c-5: delete_device: Deleting device mp2975 at 0x20 > """ > Since the 'read_byte_data' function was removed from the > 'pmbus_driver_info ' structure the driver no longer reports correctly > that VOUT mode is direct. Therefore 'pmbus_identify_common' fails > with error, making it impossible to initialize the device. > > Restore 'read_byte_data' function to fix the issue. > > Tested: > - before: it is not possible to initialize MP2975 device with the > 'mp2975' driver, > - after: 'mp2975' correctly initializes MP2975 device and all sensor > data is correct. > > Fixes: 1feb31e810b0 ("hwmon: (pmbus/mp2975) Simplify VOUT code") > > Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com> > --- > Changes in v3: > - Drop accidentally added file from patch > > Changes in v2: > - Fix indentation issues > > drivers/hwmon/pmbus/mp2975.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/hwmon/pmbus/mp2975.c b/drivers/hwmon/pmbus/mp2975.c > index b9bb469e2d8f..5bbfdacb61a7 100644 > --- a/drivers/hwmon/pmbus/mp2975.c > +++ b/drivers/hwmon/pmbus/mp2975.c > @@ -126,6 +126,22 @@ static const struct regulator_desc __maybe_unused mp2975_reg_desc[] = { > > #define to_mp2975_data(x) container_of(x, struct mp2975_data, info) > > +static int mp2975_read_byte_data(struct i2c_client *client, int page, int reg) > +{ > + switch (reg) { > + case PMBUS_VOUT_MODE: > + /* > + * Enforce VOUT direct format, since device allows to set the > + * different formats for the different rails. Conversion from > + * VID to direct provided by driver internally, in case it is > + * necessary. > + */ > + return PB_VOUT_MODE_DIRECT; > + default: > + return -ENODATA; > + } > +} > + > static int > mp2975_read_word_helper(struct i2c_client *client, int page, int phase, u8 reg, > u16 mask) > @@ -869,6 +885,7 @@ static struct pmbus_driver_info mp2975_info = { > PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | > PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_POUT | > PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT | PMBUS_PHASE_VIRTUAL, > + .read_byte_data = mp2975_read_byte_data, > .read_word_data = mp2975_read_word_data, > #if IS_ENABLED(CONFIG_SENSORS_MP2975_REGULATOR) > .num_regulators = 1, > -- > 2.34.1 >
On 1/27/24 00:56, Patrick Rudolph wrote: > Hi Konstantin, > Thank you for fixing this regression. > > The comment is no longer true as the driver doesn't internally convert > from VID to direct, > but rather configures READ_VOUT using MFR_DC_LOOP_CTRL. > > The comment thus should read as the following: > > Report direct format as configured by MFR_DC_LOOP_CTRL. > Unlike on MP2971/MP2973 the reported VOUT_MODE isn't automatically > internally updated, > but always reads as PB_VOUT_MODE_VID. > > Regards, > Patrick Please don't top-post. This patch has already been applied. Please send a follow-up to that patch to update it. Thanks, Guenter
diff --git a/drivers/hwmon/pmbus/mp2975.c b/drivers/hwmon/pmbus/mp2975.c index b9bb469e2d8f..5bbfdacb61a7 100644 --- a/drivers/hwmon/pmbus/mp2975.c +++ b/drivers/hwmon/pmbus/mp2975.c @@ -126,6 +126,22 @@ static const struct regulator_desc __maybe_unused mp2975_reg_desc[] = { #define to_mp2975_data(x) container_of(x, struct mp2975_data, info) +static int mp2975_read_byte_data(struct i2c_client *client, int page, int reg) +{ + switch (reg) { + case PMBUS_VOUT_MODE: + /* + * Enforce VOUT direct format, since device allows to set the + * different formats for the different rails. Conversion from + * VID to direct provided by driver internally, in case it is + * necessary. + */ + return PB_VOUT_MODE_DIRECT; + default: + return -ENODATA; + } +} + static int mp2975_read_word_helper(struct i2c_client *client, int page, int phase, u8 reg, u16 mask) @@ -869,6 +885,7 @@ static struct pmbus_driver_info mp2975_info = { PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_POUT | PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT | PMBUS_PHASE_VIRTUAL, + .read_byte_data = mp2975_read_byte_data, .read_word_data = mp2975_read_word_data, #if IS_ENABLED(CONFIG_SENSORS_MP2975_REGULATOR) .num_regulators = 1,
The commit 1feb31e810b0 ("hwmon: (pmbus/mp2975) Simplify VOUT code") has introduced a bug that makes it impossible to initialize MP2975 device: """ mp2975 5-0020: Failed to identify chip capabilities i2c i2c-5: new_device: Instantiated device mp2975 at 0x20 i2c i2c-5: delete_device: Deleting device mp2975 at 0x20 """ Since the 'read_byte_data' function was removed from the 'pmbus_driver_info ' structure the driver no longer reports correctly that VOUT mode is direct. Therefore 'pmbus_identify_common' fails with error, making it impossible to initialize the device. Restore 'read_byte_data' function to fix the issue. Tested: - before: it is not possible to initialize MP2975 device with the 'mp2975' driver, - after: 'mp2975' correctly initializes MP2975 device and all sensor data is correct. Fixes: 1feb31e810b0 ("hwmon: (pmbus/mp2975) Simplify VOUT code") Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com> --- Changes in v3: - Drop accidentally added file from patch Changes in v2: - Fix indentation issues drivers/hwmon/pmbus/mp2975.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)