Re: [v3,1/2] hwmon: add MP2891 driver

[Bjorn Helgaas]

On Fri, May 31, 2024 at 03:26:01PM +0800, Noah Wang wrote:
> Add support for MPS VR controller mp2891. This driver exposes
> telemetry and limit value readings and writings.

> +++ b/Documentation/hwmon/index.rst
> @@ -168,6 +168,7 @@ Hardware Monitoring Kernel Drivers
>     mp2975
>     mp5023
>     mp5990
> +   mp2891

Add in alpha order.

>     mpq8785
>     nct6683
>     nct6775

> +++ b/Documentation/hwmon/mp2891.rst

> +Device supports direct and linear format for reading input voltage,
> +output voltage, input currect, output current, input power, output

s/currect/current/

> +++ b/MAINTAINERS
> @@ -22683,6 +22683,13 @@ S:	Maintained
>  F:	Documentation/hwmon/tps546d24.rst
>  F:	drivers/hwmon/pmbus/tps546d24.c
>  
> ++MPS MP2891 DRIVER

Should be added in alpha order.

> ++M:	Noah Wang <noahwang.wang@xxxxxxxxxxx>
> ++L:	linux-hwmon@xxxxxxxxxxxxxxx
> ++S:	Maintained
> ++F:	Documentation/hwmon/mp2891.rst
> ++F:	drivers/hwmon/pmbus/mp2891.c
> +
>  TQ SYSTEMS BOARD & DRIVER SUPPORT

> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -39,6 +39,7 @@ obj-$(CONFIG_SENSORS_MP2888)	+= mp2888.o
>  obj-$(CONFIG_SENSORS_MP2975)	+= mp2975.o
>  obj-$(CONFIG_SENSORS_MP5023)	+= mp5023.o
>  obj-$(CONFIG_SENSORS_MP5990)	+= mp5990.o
> +obj-$(CONFIG_SENSORS_MP2891)	+= mp2891.o

This list as a whole isn't sorted, but I would move this so the MPxxxx
entries remain sorted.

> +++ b/drivers/hwmon/pmbus/mp2891.c

> + * Vender specific registers, the register MFR_SVI3_IOUT_PRT(0x65),
> + * MFR_VOUT_LOOP_CTRL(0xBD), READ_PIN_EST(0x94)and READ_IIN_EST(0x95)
> + * redefine the standard PMBUS register. The MFR_SVI3_IOUT_PRT(0x65)
> + * is used to identify the iout scale and the MFR_VOUT_LOOP_CTRL(0xBD)
> + * is used to identify the vout scale. The READ_PIN_EST(0x94) is used
> + * to read input power of per rail. The MP2891 does not have standard

s/of per rail/per rail/ ?

> +	 * The output voltage is equal to the READ_VOUT(0x8B) register value multiply
> +	 * by vout_scale.

s/multiply by/multiplied by/

> +	 * The output current is equal to the READ_IOUT(0x8C) register value
> +	 * multiply by iout_scale.

s/multiply by/multiplied by/

> +		 * The MP2891 does not follow standard PMBus protocol completely, the
> +		 * PMBUS_VOUT_MODE(0x20) in MP2891 is reserved and 0x00 is always be
> +		 * returned when the register is read. But the calculation of vout in

s/always be/always/

> +		 * The MP2891 has standard PMBUS_READ_PIN register(0x97), but this
> +		 * is not used to read the input power of per rail. The input power

s/of per rail/per rail/ ?

> +		 * of per rail is read through the vender redefined register

s/of per rail/per rail/ ?

> +		 * The MP2891 PMBUS_VIN_OV_FAULT_LIMIT scale is 125mV/Lsb.
> +		 * but the vin scale is set to 31.25mV/Lsb(using r/m/b scale).
> +		 * As a result, the limit value should multiply by 4.

s/multiply by/be multiplied by/

> +		 * The scale of PMBUS_IIN_OC_WARN_LIMIT is 0.5A/Lsb, but the iin scale
> +		 * is set to 1A/Lsb(using r/m/b scale), so the word data should divide
> +		 * by 2.

s/divide by/be divided by/

> +		 * The scale of PMBUS_PIN_OP_WARN_LIMIT is 2W/Lsb, but the pin scale
> +		 * is set to 1W/Lsb(using r/m/b scale), so the word data should multiply
> +		 * by 2.

s/multiply by/be multiplied by/

> +		 * The PMBUS_VIN_OV_FAULT_LIMIT[7:0] is the limit value, and bit8-bit15
> +		 * should not be changed. The scale of PMBUS_VIN_OV_FAULT_LIMIT is 125mV/Lsb,
> +		 * but the vin scale is set to 31.25mV/Lsb(using r/m/b scale), so the word data
> +		 * should divide by 4.

s/divide by/be divided by/

> +		 * The scale of PMBUS_IIN_OC_WARN_LIMIT is 0.5A/Lsb, but the iin scale
> +		 * is set to 1A/Lsb(using r/m/b scale), so the word data should multiply
> +		 * by 2.

s/multiply by/be multiplied by/

> +		 * The scale of PMBUS_PIN_OP_WARN_LIMIT is 2W/Lsb, but the pin scale
> +		 * is set to 1W/Lsb(using r/m/b scale), so the word data should divide
> +		 * by 2.

s/divide by/be divided by/