Re: [PATCH v1] hwmon: Add driver for MPS MPQ8785 Synchronous Step-Down Converter

From: Guenter Roeck
Date: Sun Jan 28 2024 - 12:13:25 EST


On 1/27/24 16:00, Guenter Roeck wrote:
On Fri, Jan 26, 2024 at 03:52:13PM +0800, Charles Hsu wrote:
Add support for mpq8785 device from Monolithic Power Systems, Inc.
(MPS) vendor. This is synchronous step-down controller.


"(MPS) vendor" above has no value.

I find no reference that this chip actually exists. Sorry, but I can not
apply such patches without confirmation that the chip actually exists.


I since learned that the chip does exist, so this is no longer a concern.

Signed-off-by: Charles Hsu <ythsu0511@xxxxxxxxx>
---
Change in v1:
Initial patchset.

A change log or v1 tag is not needed for the first version of a patch
or patch series.

---
...
+ PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_TEMP | PMBUS_HAVE_IOUT |
+ PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP,

I am not too happy that all those drivers claim to have no output status
registers. It always makes me wonder if the definitions are just copied
from one driver to the next.

I also learned that the chip supports additional status registers. Please
list all supported status registers.

I also learned that the chips voltage output mode is configurable. As such,

+ .format[PSC_CURRENT_OUT] = direct,

won't do because it cause instantiation to fail due to mode mismatch
in pmbus_identify_common() if the mode is configured to linear or vid mode.
This will need to be addressed.

Thanks,
Guenter