Re: [PATCH v5] hwmon: (pmbus) Add support for MPS MP2949A

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/28/21 6:26 PM, Ainux Wang wrote:
On Thu, 24 Jun 2021 at 21:17, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:

On Wed, Jun 23, 2021 at 08:54:26AM +0800, ainux.wang@xxxxxxxxx wrote:
From: "Ainux.Wang" <ainux.wang@xxxxxxxxx>

Add support for MP2949A device from Monolithic Power Systems, Inc. (MPS).
This is a triple-loop, digital, multi-phase controller.
This device:
- Supports up to three power rail.
- Provides 6 pulse-width modulations (PWMs), and can be configured up
   to 6-phase operation for Rail A , up to 2-phase operation for Rail B
   and up to 1-phase operation for Rail C.
- The PMBus registers are distributed into three pages: Page 0, Page 1,
   Page 2. Page 0 contains the registers for Rail A and most of the common
   settings for all of the rails. Page 1 contains register information for
   Rail B. Page 2 contains register information for Rail C.
- The MP2949A supports both 5mV VID step and 10mv VID step for IMVP8 and
   IMVP9.

Signed-off-by: Ainux.Wang <ainux.wang@xxxxxxxxx>
---
v5:
- Moved change log to right here.
v4:
- Removed mp2949a_read_byte_data().

Your other question left me confused. I had previously asked to provide
a rationale for filtering out the PMBUS_VOUT_MODE command, and I had
stated that "the chip does not support it" is not a valid reason. However,
"the chip does not support it but does not report an error when reading
it" _is_ a valid reason. So what happens when the PMBus core reads
PMBUS_VOUT_MODE ? Does the chip return an error, or some random data ?

Thanks,
Guenter

Hi, Guenter

I have not been clear about the cause of this problem that
"the chip does not support it is not a valid reason".

However, i have added some "printk", i found the chip will return some
random data,
when the PMbus core reads PMBUS_STATUS and PMBUS_VOUT_MODE.

So, I do not known what should i do, when the PMbus core reads PMBUS_STATUS.
and i have knwon that use "This chip do not support the VOUT_MODE command,
the chip does not support it but return some random data when reading
" instead of
"
                /*
                 * This chip do not support the VOUT_MODE command.
                 * There is not VOUT_MODE command in MP2949A datasheet P29~P31.
                 * So this is EINVAL in here.
                */
"
Now, there is only one question left, what should the PMbus do, the
chip return random data,
when it reads PMBUS_STATUS by i2c_smbus_read_word_data() or
i2c_smbus_read_byte_data()?
Can the PMbus core use  pmbus_read_status_byte() and
pmbus_read_status_word() instead of
i2c_smbus_read_byte_data() and i2c_smbus_read_word_data() ?


The driver should implement a read_byte_data() function and either
simulate PMBUS_VOUT_MODE and let it return whatever makes sense for
the driver, or have it return -EINVAL.

For the status register, handling is a bit more difficult. For this, we will
need an introductory patch. That patch needs to change pmbus_init_common()
to call pmbus_read_status_word() and pmbus_read_status_byte() instead
of i2c_smbus_read_word_data() and i2c_smbus_read_byte_data() (with page set
to -1).
The driver patch then needs to implement a read_word_data() function and
have it simulate the PMBUS_STATUS_WORD command. Or, alternatively,
have the read_word_data() function return -EINVAL for PMBUS_STATUS_WORD,
and simulate PMBUS_STATUS_BYTE in the read_byte_data callback.

Thanks,
Guenter



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux