On Tue, 29 Jun 2021 at 10:20, Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > > 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 Hi, Guenter Very nice, that is what i mean, we have the same idea, I am going to revise pmbus_init_common(), pmbus_check_register and so on. Best regards, Ainux Wang.
