[PATCH] hwmon: (ucd9000) Add voltage monitor types

[PATCH] hwmon: (ucd9000) Add voltage monitor types

[Jim Wright]

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;
-- 
2.32.0

Re: [PATCH] hwmon: (ucd9000) Add voltage monitor types

[Guenter Roeck]

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

Re: [PATCH] hwmon: (ucd9000) Add voltage monitor types

[Jim Wright]

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

Re: [PATCH] hwmon: (ucd9000) Add voltage monitor types

[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