| Message ID | 20220607205306.145636-1-wrightj@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon: (ucd9000) Add voltage monitor types | expand |
On 6/7/22 13:53, Jim Wright wrote: > The UCD90240 and UCD90320 devices support monitor types "Input Voltage", > "Voltage With AVS", and "Input Voltage With AVS", add support to driver > to recognize these types as voltage monitors. > > Signed-off-by: Jim Wright <wrightj@linux.ibm.com> > --- > drivers/hwmon/pmbus/ucd9000.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c > index 75fc770c9e40..28cc214d4d6b 100644 > --- a/drivers/hwmon/pmbus/ucd9000.c > +++ b/drivers/hwmon/pmbus/ucd9000.c > @@ -45,6 +45,9 @@ enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd90320, ucd9090, > #define UCD9000_MON_TEMPERATURE 2 > #define UCD9000_MON_CURRENT 3 > #define UCD9000_MON_VOLTAGE_HW 4 > +#define UCD9000_MON_INPUT_VOLTAGE 5 > +#define UCD9000_MON_VOLTAGE_AVS 6 > +#define UCD9000_MON_INPUT_VOLTAGE_AVS 7 > > #define UCD9000_NUM_FAN 4 > > @@ -566,6 +569,9 @@ static int ucd9000_probe(struct i2c_client *client) > switch (UCD9000_MON_TYPE(block_buffer[i])) { > case UCD9000_MON_VOLTAGE: > case UCD9000_MON_VOLTAGE_HW: > + case UCD9000_MON_INPUT_VOLTAGE: > + case UCD9000_MON_VOLTAGE_AVS: > + case UCD9000_MON_INPUT_VOLTAGE_AVS: > info->func[page] |= PMBUS_HAVE_VOUT > | PMBUS_HAVE_STATUS_VOUT; > break; I don't think it makes sense to claim VOUT support if the chip is configured to monitor input voltages. This should probably be something like ... > + case UCD9000_MON_VOLTAGE_AVS: > info->func[page] |= PMBUS_HAVE_VOUT > | PMBUS_HAVE_STATUS_VOUT; > break; case UCD9000_MON_INPUT_VOLTAGE: case UCD9000_MON_INPUT_VOLTAGE_AVS: info->func[page] |= PMBUS_HAVE_VIN; break; with appropriate mapping code to map the READ_VIN command for the affected pages to READ_VOUT. Question is if the limit registers on those pages are also reporting the limits using the vout limit commands; if so, those should be mapped as well. Guenter
On 6/7/2022 8:13 PM, Guenter Roeck wrote: > > I don't think it makes sense to claim VOUT support if the chip is > configured to monitor input voltages. This should probably be something > like > > ... > > + case UCD9000_MON_VOLTAGE_AVS: > > info->func[page] |= PMBUS_HAVE_VOUT > > | PMBUS_HAVE_STATUS_VOUT; > > break; > case UCD9000_MON_INPUT_VOLTAGE: > case UCD9000_MON_INPUT_VOLTAGE_AVS: > info->func[page] |= PMBUS_HAVE_VIN; > break; > > with appropriate mapping code to map the READ_VIN command for the > affected pages to READ_VOUT. Question is if the limit registers on > those pages are also reporting the limits using the vout limit > commands; if so, those should be mapped as well. > > Guenter Hi Guenter, Thank you for the review. I'll drop adding the input voltage types and resend the patch. Jim Wright
On 6/7/2022 8:26 PM, Jim Wright wrote: > On 6/7/2022 8:13 PM, Guenter Roeck wrote: >> >> I don't think it makes sense to claim VOUT support if the chip is >> configured to monitor input voltages. This should probably be something >> like >> >> ... >> > + case UCD9000_MON_VOLTAGE_AVS: >> > info->func[page] |= PMBUS_HAVE_VOUT >> > | PMBUS_HAVE_STATUS_VOUT; >> > break; >> case UCD9000_MON_INPUT_VOLTAGE: >> case UCD9000_MON_INPUT_VOLTAGE_AVS: >> info->func[page] |= PMBUS_HAVE_VIN; >> break; >> >> with appropriate mapping code to map the READ_VIN command for the >> affected pages to READ_VOUT. Question is if the limit registers on >> those pages are also reporting the limits using the vout limit >> commands; if so, those should be mapped as well. >> >> Guenter > > Hi Guenter, > > Thank you for the review. I'll drop adding the input voltage types and > resend the patch. > > Jim Wright After a second look, it's the input voltage type that I need. Will revise as suggested and resubmit. Thanks, Jim Wright
diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c index 75fc770c9e40..28cc214d4d6b 100644 --- a/drivers/hwmon/pmbus/ucd9000.c +++ b/drivers/hwmon/pmbus/ucd9000.c @@ -45,6 +45,9 @@ enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd90320, ucd9090, #define UCD9000_MON_TEMPERATURE 2 #define UCD9000_MON_CURRENT 3 #define UCD9000_MON_VOLTAGE_HW 4 +#define UCD9000_MON_INPUT_VOLTAGE 5 +#define UCD9000_MON_VOLTAGE_AVS 6 +#define UCD9000_MON_INPUT_VOLTAGE_AVS 7 #define UCD9000_NUM_FAN 4 @@ -566,6 +569,9 @@ static int ucd9000_probe(struct i2c_client *client) switch (UCD9000_MON_TYPE(block_buffer[i])) { case UCD9000_MON_VOLTAGE: case UCD9000_MON_VOLTAGE_HW: + case UCD9000_MON_INPUT_VOLTAGE: + case UCD9000_MON_VOLTAGE_AVS: + case UCD9000_MON_INPUT_VOLTAGE_AVS: info->func[page] |= PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT; break;
The UCD90240 and UCD90320 devices support monitor types "Input Voltage", "Voltage With AVS", and "Input Voltage With AVS", add support to driver to recognize these types as voltage monitors. Signed-off-by: Jim Wright <wrightj@linux.ibm.com> --- drivers/hwmon/pmbus/ucd9000.c | 6 ++++++ 1 file changed, 6 insertions(+)