On Tue, Apr 16, 2024 at 05:32:46PM -0700, Guenter Roeck wrote: > On Wed, Apr 17, 2024 at 08:07:22AM +0800, Jose Ramon San Buenaventura wrote: > > Adding support for adm1281 which is similar to adm1275 > > > > ADM1281 has STATUS_CML read support which is also being added. > > > > Signed-off-by: Jose Ramon San Buenaventura <jose.sanbuenaventura@xxxxxxxxxx> > > --- > > Documentation/hwmon/adm1275.rst | 14 +++++++++++--- > > drivers/hwmon/pmbus/Kconfig | 4 ++-- > > drivers/hwmon/pmbus/adm1275.c | 27 +++++++++++++++++++++++++-- > > 3 files changed, 38 insertions(+), 7 deletions(-) > > > > diff --git a/Documentation/hwmon/adm1275.rst b/Documentation/hwmon/adm1275.rst > > index 804590eea..467daf8ce 100644 > > --- a/Documentation/hwmon/adm1275.rst > > +++ b/Documentation/hwmon/adm1275.rst > > @@ -43,6 +43,14 @@ Supported chips: > > > > Datasheet: www.analog.com/static/imported-files/data_sheets/ADM1278.pdf > > > > + * Analog Devices ADM1281 > > + > > + Prefix: 'adm1281' > > + > > + Addresses scanned: - > > + > > + Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adm1281.pdf > > + > > * Analog Devices ADM1293/ADM1294 > > > > Prefix: 'adm1293', 'adm1294' > > @@ -58,10 +66,10 @@ Description > > ----------- > > > > This driver supports hardware monitoring for Analog Devices ADM1075, ADM1272, > > -ADM1275, ADM1276, ADM1278, ADM1293, and ADM1294 Hot-Swap Controller and > > +ADM1275, ADM1276, ADM1278, ADM1281, ADM1293, and ADM1294 Hot-Swap Controller and > > Digital Power Monitors. > > > > -ADM1075, ADM1272, ADM1275, ADM1276, ADM1278, ADM1293, and ADM1294 are hot-swap > > +ADM1075, ADM1272, ADM1275, ADM1276, ADM1278, ADM1281, ADM1293, and ADM1294 are hot-swap > > controllers that allow a circuit board to be removed from or inserted into > > a live backplane. They also feature current and voltage readback via an > > integrated 12 bit analog-to-digital converter (ADC), accessed using a > > @@ -144,5 +152,5 @@ temp1_highest Highest observed temperature. > > temp1_reset_history Write any value to reset history. > > > > Temperature attributes are supported on ADM1272 and > > - ADM1278. > > + ADM1278, and ADM1281. > > ======================= ======================================================= > > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig > > index 557ae0c41..9c1d0d7d5 100644 > > --- a/drivers/hwmon/pmbus/Kconfig > > +++ b/drivers/hwmon/pmbus/Kconfig > > @@ -51,8 +51,8 @@ config SENSORS_ADM1275 > > tristate "Analog Devices ADM1275 and compatibles" > > help > > If you say yes here you get hardware monitoring support for Analog > > - Devices ADM1075, ADM1272, ADM1275, ADM1276, ADM1278, ADM1293, > > - and ADM1294 Hot-Swap Controller and Digital Power Monitors. > > + Devices ADM1075, ADM1272, ADM1275, ADM1276, ADM1278, ADM1281, > > + ADM1293, and ADM1294 Hot-Swap Controller and Digital Power Monitors. > > > > This driver can also be built as a module. If so, the module will > > be called adm1275. > > diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c > > index e2c61d6fa..6c3e8840f 100644 > > --- a/drivers/hwmon/pmbus/adm1275.c > > +++ b/drivers/hwmon/pmbus/adm1275.c > > @@ -18,7 +18,7 @@ > > #include <linux/log2.h> > > #include "pmbus.h" > > > > -enum chips { adm1075, adm1272, adm1275, adm1276, adm1278, adm1293, adm1294 }; > > +enum chips { adm1075, adm1272, adm1275, adm1276, adm1278, adm1281, adm1293, adm1294 }; > > > > #define ADM1275_MFR_STATUS_IOUT_WARN2 BIT(0) > > #define ADM1293_MFR_STATUS_VAUX_UV_WARN BIT(5) > > @@ -101,6 +101,7 @@ struct adm1275_data { > > bool have_pin_max; > > bool have_temp_max; > > bool have_power_sampling; > > + bool have_status_cml; > > struct pmbus_driver_info info; > > }; > > > > @@ -469,6 +470,22 @@ static int adm1275_read_byte_data(struct i2c_client *client, int page, int reg) > > ret |= PB_VOLTAGE_UV_WARNING; > > } > > break; > > + case PMBUS_STATUS_CML: > > + if (!data->have_status_cml) > > + return -ENXIO; > > + > > + ret = pmbus_read_byte_data(client, page, PMBUS_STATUS_BYTE); > > + if (ret < 0) > > + break; > > You'll have to explain why this additional status byte read > is necessary (while it isn't necessary for all other chips supporting > PMBUS_STATUS_CML). > After looking more into the existing PMBus code and into this patch, I really fail to understand why the above change would be needed. The PMBus core code already reads the status register to check if there is a communication error. I fail to see why it would be necessary to do it again, and why it would be necessary to change behavior for the other chips supported by this driver. Guenter
