Re: [PATCH v3 0/2] hwmon: Add support for MPS mp2985

[Guenter Roeck]

On 3/22/26 23:19, wenswang@xxxxxxxx wrote:
> From: Wensheng Wang <wenswang@xxxxxxxx>
>
> Add mp2985 driver in hwmon and add dt-bindings for it.

AI feedback is here:

https://sashiko.dev/#/patchset/20260323062104.827263-1-wenswang%40yeah.net

Couple of comments:

- If the mantissa overflow can not happen, please add a comment into the code
  explaining the reason.

- EINVAL returns need to be explained. They should only be used
  if the chip can not handle reading unsupported registers, and a comment
  in the code needs to explain that.

- Ignore the concern about the chip being left on page 2 after an error
  reading MFR_VR_MULTI_CONFIG_R[12] for now. It makes me wonder if
  pmbus_init_common() should explicitly select page 0 for multi-page
  chips.

- The alignment of

+		ret = i2c_smbus_read_word_data(client, page == 0 ?
+									MFR_VR_MULTI_CONFIG_R1 :
+									MFR_VR_MULTI_CONFIG_R2);

still seems odd. Why the large indentation ?

Thanks,
Guenter