* [PATCH v6 0/2] hwmon: (pmbus) Add support for MPS MP2949A @ 2021-07-02 7:31 ainux.wang 2021-07-02 7:31 ` [PATCH v6 1/2] " ainux.wang 2021-07-02 7:31 ` [PATCH v6 2/2] hwmon: (pmbus) Try use read_status() to read status register ainux.wang 0 siblings, 2 replies; 5+ messages in thread From: ainux.wang @ 2021-07-02 7:31 UTC (permalink / raw) To: jdelvare, linux, corbet, ainux.wang Cc: linux-hwmon, linux-doc, sterlingteng, chenhuacai, chenhuacai From: "Ainux.Wang" <ainux.wang@gmail.com> PART 1 (The chip driver): 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. v6: - Add mp2949a_read_byte_data(), mp2949a_read_word_data(), and the mp2949a do not support STATUS_BYTE, STATUS_WORD, and VOUT_MODE ,but return some random data when reading them. v5: - Moved change log to right here. v4: - Removed mp2949a_read_byte_data(). - Added space before and after '-' and fixed a bug that is '~' on line 35. v3: - Added change log here. - Errors in the document have been modified. - Fixed PMBUS_VOUT_MODE to MP2949A_MFR_VR_CONFIG. - Removed unnecessary vout_params and empty line. v2: - Reference has been added to Documentation/hwmon/index.rst. - Droped 'vendor'. - Removed change codes of the PMBus core and added mp2949a_identify() and mp2949a_read_byte_data() to this driver. v1: - Add support for MPS MP2949A. PART 2(PMbus Core): There is a case(like MP2949A) that the chip do not support STATUS_WORD and STATUS_BYTE command, but return some random data when reading. So we should call read_status() instead of i2c_smbus_read_word_data() and i2c_smbus_read_byte_data(), and the chip driver should implement a read_word_data() function and a read_byte_data() function to return -ENXIO. Ainux.Wang (2): hwmon: (pmbus) Add support for MPS MP2949A hwmon: (pmbus) Try use read_status() to read status register Documentation/hwmon/index.rst | 1 + Documentation/hwmon/mp2949a.rst | 44 ++++++++++ drivers/hwmon/pmbus/Kconfig | 9 ++ drivers/hwmon/pmbus/Makefile | 1 + drivers/hwmon/pmbus/mp2949a.c | 136 +++++++++++++++++++++++++++++++ drivers/hwmon/pmbus/pmbus_core.c | 20 +++-- 6 files changed, 206 insertions(+), 5 deletions(-) create mode 100644 Documentation/hwmon/mp2949a.rst create mode 100644 drivers/hwmon/pmbus/mp2949a.c -- 2.18.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v6 1/2] hwmon: (pmbus) Add support for MPS MP2949A 2021-07-02 7:31 [PATCH v6 0/2] hwmon: (pmbus) Add support for MPS MP2949A ainux.wang @ 2021-07-02 7:31 ` ainux.wang 2021-07-02 14:59 ` Guenter Roeck 2021-07-02 7:31 ` [PATCH v6 2/2] hwmon: (pmbus) Try use read_status() to read status register ainux.wang 1 sibling, 1 reply; 5+ messages in thread From: ainux.wang @ 2021-07-02 7:31 UTC (permalink / raw) To: jdelvare, linux, corbet, ainux.wang Cc: linux-hwmon, linux-doc, sterlingteng, chenhuacai, chenhuacai From: "Ainux.Wang" <ainux.wang@gmail.com> 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@gmail.com> --- Documentation/hwmon/index.rst | 1 + Documentation/hwmon/mp2949a.rst | 44 +++++++++++ drivers/hwmon/pmbus/Kconfig | 9 +++ drivers/hwmon/pmbus/Makefile | 1 + drivers/hwmon/pmbus/mp2949a.c | 136 ++++++++++++++++++++++++++++++++ 5 files changed, 191 insertions(+) create mode 100644 Documentation/hwmon/mp2949a.rst create mode 100644 drivers/hwmon/pmbus/mp2949a.c diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst index bc01601ea81a..e62161c0f01e 100644 --- a/Documentation/hwmon/index.rst +++ b/Documentation/hwmon/index.rst @@ -138,6 +138,7 @@ Hardware Monitoring Kernel Drivers mcp3021 menf21bmc mlxreg-fan + mp2949a mp2888 mp2975 nct6683 diff --git a/Documentation/hwmon/mp2949a.rst b/Documentation/hwmon/mp2949a.rst new file mode 100644 index 000000000000..0235dec70f63 --- /dev/null +++ b/Documentation/hwmon/mp2949a.rst @@ -0,0 +1,44 @@ +.. SPDX-License-Identifier: GPL-2.0 + +Kernel driver mp2949a +==================== + +Supported chips: + + * MPS MP2949A + + Prefix: 'mp2949a' + +Author: + + Ainux Wang <ainux.wang@gmail.com> + +Description +----------- + +This driver implements support for Monolithic Power Systems, Inc. (MPS) +triple-loop, digital, multi-phase controller MP2949A. + +This device: + +- Supports up to three power rail. +- Provides 6 pulse-width modulations (PWMs), and can be configured for + 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. + +Device supports: + +- SVID interface. +- PMBus rev 1.2 interface. + +Device supports direct format for reading output power. +Device supports linear format for reading input voltage, output current, +and temperature. +Device supports VID for reading output voltage. +The below VID modes are supported: VR12, VR13, IMVP8, IMVP9. diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig index ffb609cee3a4..d3a521bead3e 100644 --- a/drivers/hwmon/pmbus/Kconfig +++ b/drivers/hwmon/pmbus/Kconfig @@ -258,6 +258,15 @@ config SENSORS_MAX8688 This driver can also be built as a module. If so, the module will be called max8688. +config SENSORS_MP2949A + tristate "MPS MP2949A" + help + If you say yes here you get hardware monitoring support for MPS + MP2949A Triple Loop Digital Multi-Phase Controller. + + This driver can also be built as a module. If so, the module will + be called mp2949a. + config SENSORS_MP2888 tristate "MPS MP2888" help diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile index 0ed4d596a948..bfdfb599c8ba 100644 --- a/drivers/hwmon/pmbus/Makefile +++ b/drivers/hwmon/pmbus/Makefile @@ -29,6 +29,7 @@ obj-$(CONFIG_SENSORS_MAX20751) += max20751.o obj-$(CONFIG_SENSORS_MAX31785) += max31785.o obj-$(CONFIG_SENSORS_MAX34440) += max34440.o obj-$(CONFIG_SENSORS_MAX8688) += max8688.o +obj-$(CONFIG_SENSORS_MP2949A) += mp2949a.o obj-$(CONFIG_SENSORS_MP2888) += mp2888.o obj-$(CONFIG_SENSORS_MP2975) += mp2975.o obj-$(CONFIG_SENSORS_PM6764TR) += pm6764tr.o diff --git a/drivers/hwmon/pmbus/mp2949a.c b/drivers/hwmon/pmbus/mp2949a.c new file mode 100644 index 000000000000..8f8dd3235298 --- /dev/null +++ b/drivers/hwmon/pmbus/mp2949a.c @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Hardware monitoring driver for Monolithic Power Systems MP2949A + * + * Copyright (c) 2021 Lemote Technologies. All rights reserved. + * Copyright (c) 2021 Ainux <ainux.wang@gmail.com> + */ + +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include "pmbus.h" + +#define MP2949A_PAGE_NUM 3 + +#define MP2949A_MFR_VR_CONFIG 0xE4 + +static int mp2949a_read_byte_data(struct i2c_client *client, int page, int reg) +{ + switch (reg) { + /* + * This chip do not support the VOUT_MODE, STATUS_BYTE command, + * but return some random data when reading. + */ + case PMBUS_VOUT_MODE: + case PMBUS_STATUS_BYTE: + return -ENXIO; + default: + return -ENODATA; + } +} + +static int mp2949a_read_word_data(struct i2c_client *client, int page, + int phase, int reg) +{ + + switch (reg) { + /* + * This chip do not support STATUS_WORD command, + * but return some random data when reading. + */ + case PMBUS_STATUS_WORD: + return -ENXIO; + default: + return -ENODATA; + } +} + +static int mp2949a_identify(struct i2c_client *client, + struct pmbus_driver_info *info) +{ + int i, ret; + + for (i = 0; i < MP2949A_PAGE_NUM; i++) { + ret = pmbus_read_byte_data(client, i, MP2949A_MFR_VR_CONFIG); + if (ret < 0) + return ret; + + /* + * Rail A bit 5, Rail B bit 4, Rail C bit 3. + * 1'b1: 5mV (vr12/imvp8) + * 1'b0: 10mv (imvp9) + */ + info->vrm_version[i] = (ret & BIT(5 - i)) ? vr12 : imvp9; + } + + return 0; +} + +static struct pmbus_driver_info mp2949a_info = { + .pages = MP2949A_PAGE_NUM, + .format[PSC_VOLTAGE_IN] = linear, + .format[PSC_VOLTAGE_OUT] = vid, + .format[PSC_CURRENT_OUT] = linear, + .format[PSC_TEMPERATURE] = linear, + .format[PSC_POWER] = direct, + .m[PSC_POWER] = 1, + .b[PSC_POWER] = 0, + .R[PSC_POWER] = 0, + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | + PMBUS_HAVE_TEMP | PMBUS_HAVE_POUT, + .func[1] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | + PMBUS_HAVE_POUT, + .func[2] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | + PMBUS_HAVE_POUT, + .identify = mp2949a_identify, + .read_byte_data = mp2949a_read_byte_data, + .read_word_data = mp2949a_read_word_data, +}; + +static int mp2949a_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct pmbus_driver_info *info; + + info = devm_kmemdup(&client->dev, &mp2949a_info, sizeof(*info), + GFP_KERNEL); + if (!info) + return -ENOMEM; + + return pmbus_do_probe(client, info); +} + +static const struct i2c_device_id mp2949a_id[] = { + {"mp2949a", 0}, + {} +}; + +MODULE_DEVICE_TABLE(i2c, mp2949a_id); + +static const struct of_device_id mp2949a_of_match[] = { + {.compatible = "mps,mp2949a"}, + {} +}; +MODULE_DEVICE_TABLE(of, mp2949a_of_match); + +static struct i2c_driver mp2949a_driver = { + .driver = { + .name = "mp2949a", + .of_match_table = of_match_ptr(mp2949a_of_match), + }, + .probe = mp2949a_probe, + .id_table = mp2949a_id, +}; + +module_i2c_driver(mp2949a_driver); + +MODULE_AUTHOR("Ainux <ainux.wang@gmail.com>"); +MODULE_DESCRIPTION("PMBus driver for Monolithic Power Systems MP2949A"); +MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS(PMBUS); -- 2.18.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v6 1/2] hwmon: (pmbus) Add support for MPS MP2949A 2021-07-02 7:31 ` [PATCH v6 1/2] " ainux.wang @ 2021-07-02 14:59 ` Guenter Roeck 0 siblings, 0 replies; 5+ messages in thread From: Guenter Roeck @ 2021-07-02 14:59 UTC (permalink / raw) To: ainux.wang, jdelvare, corbet Cc: linux-hwmon, linux-doc, sterlingteng, chenhuacai, chenhuacai On 7/2/21 12:31 AM, ainux.wang@gmail.com wrote: > From: "Ainux.Wang" <ainux.wang@gmail.com> > > 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@gmail.com> > --- > Documentation/hwmon/index.rst | 1 + > Documentation/hwmon/mp2949a.rst | 44 +++++++++++ > drivers/hwmon/pmbus/Kconfig | 9 +++ > drivers/hwmon/pmbus/Makefile | 1 + > drivers/hwmon/pmbus/mp2949a.c | 136 ++++++++++++++++++++++++++++++++ > 5 files changed, 191 insertions(+) > create mode 100644 Documentation/hwmon/mp2949a.rst > create mode 100644 drivers/hwmon/pmbus/mp2949a.c > > diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst > index bc01601ea81a..e62161c0f01e 100644 > --- a/Documentation/hwmon/index.rst > +++ b/Documentation/hwmon/index.rst > @@ -138,6 +138,7 @@ Hardware Monitoring Kernel Drivers > mcp3021 > menf21bmc > mlxreg-fan > + mp2949a > mp2888 > mp2975 > nct6683 > diff --git a/Documentation/hwmon/mp2949a.rst b/Documentation/hwmon/mp2949a.rst > new file mode 100644 > index 000000000000..0235dec70f63 > --- /dev/null > +++ b/Documentation/hwmon/mp2949a.rst > @@ -0,0 +1,44 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +Kernel driver mp2949a > +==================== > + > +Supported chips: > + > + * MPS MP2949A > + > + Prefix: 'mp2949a' > + > +Author: > + > + Ainux Wang <ainux.wang@gmail.com> > + > +Description > +----------- > + > +This driver implements support for Monolithic Power Systems, Inc. (MPS) > +triple-loop, digital, multi-phase controller MP2949A. > + > +This device: > + > +- Supports up to three power rail. > +- Provides 6 pulse-width modulations (PWMs), and can be configured for > + 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. > + > +Device supports: > + > +- SVID interface. > +- PMBus rev 1.2 interface. > + > +Device supports direct format for reading output power. > +Device supports linear format for reading input voltage, output current, > +and temperature. > +Device supports VID for reading output voltage. > +The below VID modes are supported: VR12, VR13, IMVP8, IMVP9. > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig > index ffb609cee3a4..d3a521bead3e 100644 > --- a/drivers/hwmon/pmbus/Kconfig > +++ b/drivers/hwmon/pmbus/Kconfig > @@ -258,6 +258,15 @@ config SENSORS_MAX8688 > This driver can also be built as a module. If so, the module will > be called max8688. > > +config SENSORS_MP2949A > + tristate "MPS MP2949A" > + help > + If you say yes here you get hardware monitoring support for MPS > + MP2949A Triple Loop Digital Multi-Phase Controller. > + > + This driver can also be built as a module. If so, the module will > + be called mp2949a. > + > config SENSORS_MP2888 > tristate "MPS MP2888" > help > diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile > index 0ed4d596a948..bfdfb599c8ba 100644 > --- a/drivers/hwmon/pmbus/Makefile > +++ b/drivers/hwmon/pmbus/Makefile > @@ -29,6 +29,7 @@ obj-$(CONFIG_SENSORS_MAX20751) += max20751.o > obj-$(CONFIG_SENSORS_MAX31785) += max31785.o > obj-$(CONFIG_SENSORS_MAX34440) += max34440.o > obj-$(CONFIG_SENSORS_MAX8688) += max8688.o > +obj-$(CONFIG_SENSORS_MP2949A) += mp2949a.o > obj-$(CONFIG_SENSORS_MP2888) += mp2888.o > obj-$(CONFIG_SENSORS_MP2975) += mp2975.o > obj-$(CONFIG_SENSORS_PM6764TR) += pm6764tr.o > diff --git a/drivers/hwmon/pmbus/mp2949a.c b/drivers/hwmon/pmbus/mp2949a.c > new file mode 100644 > index 000000000000..8f8dd3235298 > --- /dev/null > +++ b/drivers/hwmon/pmbus/mp2949a.c > @@ -0,0 +1,136 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Hardware monitoring driver for Monolithic Power Systems MP2949A > + * > + * Copyright (c) 2021 Lemote Technologies. All rights reserved. > + * Copyright (c) 2021 Ainux <ainux.wang@gmail.com> > + */ > + > +#include <linux/err.h> > +#include <linux/i2c.h> > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include "pmbus.h" > + > +#define MP2949A_PAGE_NUM 3 > + > +#define MP2949A_MFR_VR_CONFIG 0xE4 > + > +static int mp2949a_read_byte_data(struct i2c_client *client, int page, int reg) > +{ > + switch (reg) { > + /* > + * This chip do not support the VOUT_MODE, STATUS_BYTE command, > + * but return some random data when reading. > + */ > + case PMBUS_VOUT_MODE: > + case PMBUS_STATUS_BYTE: > + return -ENXIO; > + default: > + return -ENODATA; > + } > +} > + > +static int mp2949a_read_word_data(struct i2c_client *client, int page, > + int phase, int reg) > +{ > + > + switch (reg) { > + /* > + * This chip do not support STATUS_WORD command, > + * but return some random data when reading. > + */ > + case PMBUS_STATUS_WORD: > + return -ENXIO; One of the status registers should be simulated. See other patch for details. > + default: > + return -ENODATA; > + } > +} > + > +static int mp2949a_identify(struct i2c_client *client, > + struct pmbus_driver_info *info) > +{ > + int i, ret; > + > + for (i = 0; i < MP2949A_PAGE_NUM; i++) { > + ret = pmbus_read_byte_data(client, i, MP2949A_MFR_VR_CONFIG); > + if (ret < 0) > + return ret; > + > + /* > + * Rail A bit 5, Rail B bit 4, Rail C bit 3. > + * 1'b1: 5mV (vr12/imvp8) > + * 1'b0: 10mv (imvp9) > + */ > + info->vrm_version[i] = (ret & BIT(5 - i)) ? vr12 : imvp9; > + } > + > + return 0; > +} > + > +static struct pmbus_driver_info mp2949a_info = { > + .pages = MP2949A_PAGE_NUM, > + .format[PSC_VOLTAGE_IN] = linear, > + .format[PSC_VOLTAGE_OUT] = vid, > + .format[PSC_CURRENT_OUT] = linear, > + .format[PSC_TEMPERATURE] = linear, > + .format[PSC_POWER] = direct, > + .m[PSC_POWER] = 1, > + .b[PSC_POWER] = 0, > + .R[PSC_POWER] = 0, > + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | > + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | > + PMBUS_HAVE_TEMP | PMBUS_HAVE_POUT, > + .func[1] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | > + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | > + PMBUS_HAVE_POUT, > + .func[2] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT | > + PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT | > + PMBUS_HAVE_POUT, > + .identify = mp2949a_identify, > + .read_byte_data = mp2949a_read_byte_data, > + .read_word_data = mp2949a_read_word_data, > +}; > + > +static int mp2949a_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct pmbus_driver_info *info; > + > + info = devm_kmemdup(&client->dev, &mp2949a_info, sizeof(*info), > + GFP_KERNEL); > + if (!info) > + return -ENOMEM; > + > + return pmbus_do_probe(client, info); > +} > + > +static const struct i2c_device_id mp2949a_id[] = { > + {"mp2949a", 0}, > + {} > +}; > + > +MODULE_DEVICE_TABLE(i2c, mp2949a_id); > + > +static const struct of_device_id mp2949a_of_match[] = { > + {.compatible = "mps,mp2949a"}, > + {} > +}; > +MODULE_DEVICE_TABLE(of, mp2949a_of_match); > + > +static struct i2c_driver mp2949a_driver = { > + .driver = { > + .name = "mp2949a", > + .of_match_table = of_match_ptr(mp2949a_of_match), > + }, > + .probe = mp2949a_probe, > + .id_table = mp2949a_id, > +}; > + > +module_i2c_driver(mp2949a_driver); > + > +MODULE_AUTHOR("Ainux <ainux.wang@gmail.com>"); > +MODULE_DESCRIPTION("PMBus driver for Monolithic Power Systems MP2949A"); > +MODULE_LICENSE("GPL"); > +MODULE_IMPORT_NS(PMBUS); > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v6 2/2] hwmon: (pmbus) Try use read_status() to read status register 2021-07-02 7:31 [PATCH v6 0/2] hwmon: (pmbus) Add support for MPS MP2949A ainux.wang 2021-07-02 7:31 ` [PATCH v6 1/2] " ainux.wang @ 2021-07-02 7:31 ` ainux.wang 2021-07-02 14:59 ` Guenter Roeck 1 sibling, 1 reply; 5+ messages in thread From: ainux.wang @ 2021-07-02 7:31 UTC (permalink / raw) To: jdelvare, linux, corbet, ainux.wang Cc: linux-hwmon, linux-doc, sterlingteng, chenhuacai, chenhuacai From: "Ainux.Wang" <ainux.wang@gmail.com> There is a case(like MP2949A) that the chip do not support STATUS_WORD and STATUS_BYTE command, but return some random data when reading. So we should call read_status() instead of i2c_smbus_read_word_data() and i2c_smbus_read_byte_data(), and the chip driver should implement a read_word_data() function and a read_byte_data() function to return -ENXIO. Signed-off-by: Ainux.Wang <ainux.wang@gmail.com> --- drivers/hwmon/pmbus/pmbus_core.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c index 776ee2237be2..d3273a20e76e 100644 --- a/drivers/hwmon/pmbus/pmbus_core.c +++ b/drivers/hwmon/pmbus/pmbus_core.c @@ -503,6 +503,9 @@ static int pmbus_check_status_cml(struct i2c_client *client) struct pmbus_data *data = i2c_get_clientdata(client); int status, status2; + if (!data->read_status) + return -EINVAL; + status = data->read_status(client, -1); if (status < 0 || (status & PB_STATUS_CML)) { status2 = _pmbus_read_byte_data(client, -1, PMBUS_STATUS_CML); @@ -534,6 +537,9 @@ static bool pmbus_check_status_register(struct i2c_client *client, int page) int status; struct pmbus_data *data = i2c_get_clientdata(client); + if (!data->read_status) + return false; + status = data->read_status(client, page); if (status >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK) && (status & PB_STATUS_CML)) { @@ -573,6 +579,8 @@ static int pmbus_get_status(struct i2c_client *client, int page, int reg) switch (reg) { case PMBUS_STATUS_WORD: + if (!data->read_status) + return -EINVAL; status = data->read_status(client, page); break; default: @@ -2306,16 +2314,15 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data, /* * Some PMBus chips don't support PMBUS_STATUS_WORD, so try * to use PMBUS_STATUS_BYTE instead if that is the case. - * Bail out if both registers are not supported. */ data->read_status = pmbus_read_status_word; - ret = i2c_smbus_read_word_data(client, PMBUS_STATUS_WORD); + ret = data->read_status(client, -1); if (ret < 0 || ret == 0xffff) { data->read_status = pmbus_read_status_byte; - ret = i2c_smbus_read_byte_data(client, PMBUS_STATUS_BYTE); + ret = data->read_status(client, -1); if (ret < 0 || ret == 0xff) { - dev_err(dev, "PMBus status register not found\n"); - return -ENODEV; + /* Both registers are not supported. */ + data->read_status = NULL; } } else { data->has_status_word = true; @@ -2484,6 +2491,9 @@ static int pmbus_debugfs_get_status(void *data, u64 *val) struct pmbus_debugfs_entry *entry = data; struct pmbus_data *pdata = i2c_get_clientdata(entry->client); + if (!pdata->read_status) + return -EINVAL; + rc = pdata->read_status(entry->client, entry->page); if (rc < 0) return rc; -- 2.18.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v6 2/2] hwmon: (pmbus) Try use read_status() to read status register 2021-07-02 7:31 ` [PATCH v6 2/2] hwmon: (pmbus) Try use read_status() to read status register ainux.wang @ 2021-07-02 14:59 ` Guenter Roeck 0 siblings, 0 replies; 5+ messages in thread From: Guenter Roeck @ 2021-07-02 14:59 UTC (permalink / raw) To: ainux.wang, jdelvare, corbet Cc: linux-hwmon, linux-doc, sterlingteng, chenhuacai, chenhuacai On 7/2/21 12:31 AM, ainux.wang@gmail.com wrote: > From: "Ainux.Wang" <ainux.wang@gmail.com> > > There is a case(like MP2949A) that the chip do not support STATUS_WORD > and STATUS_BYTE command, but return some random data when reading. > > So we should call read_status() instead of i2c_smbus_read_word_data() > and i2c_smbus_read_byte_data(), and the chip driver should implement a > read_word_data() function and a read_byte_data() function to return > -ENXIO. No, the chip driver needs to simulate at least one of the commands. That makes most of the changes below unnecessary. We should limit complexity to where it is needed, meaning the chip driver. > > Signed-off-by: Ainux.Wang <ainux.wang@gmail.com> > --- > drivers/hwmon/pmbus/pmbus_core.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c > index 776ee2237be2..d3273a20e76e 100644 > --- a/drivers/hwmon/pmbus/pmbus_core.c > +++ b/drivers/hwmon/pmbus/pmbus_core.c > @@ -503,6 +503,9 @@ static int pmbus_check_status_cml(struct i2c_client *client) > struct pmbus_data *data = i2c_get_clientdata(client); > int status, status2; > > + if (!data->read_status) > + return -EINVAL; > + Unnecessary. > status = data->read_status(client, -1); > if (status < 0 || (status & PB_STATUS_CML)) { > status2 = _pmbus_read_byte_data(client, -1, PMBUS_STATUS_CML); > @@ -534,6 +537,9 @@ static bool pmbus_check_status_register(struct i2c_client *client, int page) > int status; > struct pmbus_data *data = i2c_get_clientdata(client); > > + if (!data->read_status) > + return false; > + Unnecessary. > status = data->read_status(client, page); > if (status >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK) && > (status & PB_STATUS_CML)) { > @@ -573,6 +579,8 @@ static int pmbus_get_status(struct i2c_client *client, int page, int reg) > > switch (reg) { > case PMBUS_STATUS_WORD: > + if (!data->read_status) > + return -EINVAL; Unnecessary. > status = data->read_status(client, page); > break; > default: > @@ -2306,16 +2314,15 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data, > /* > * Some PMBus chips don't support PMBUS_STATUS_WORD, so try > * to use PMBUS_STATUS_BYTE instead if that is the case. > - * Bail out if both registers are not supported. > */ > data->read_status = pmbus_read_status_word; > - ret = i2c_smbus_read_word_data(client, PMBUS_STATUS_WORD); > + ret = data->read_status(client, -1); This ... > if (ret < 0 || ret == 0xffff) { > data->read_status = pmbus_read_status_byte; > - ret = i2c_smbus_read_byte_data(client, PMBUS_STATUS_BYTE); > + ret = data->read_status(client, -1); and this are the only two changes needed. > if (ret < 0 || ret == 0xff) { > - dev_err(dev, "PMBus status register not found\n"); > - return -ENODEV; > + /* Both registers are not supported. */ > + data->read_status = NULL; Unnecessary. > } > } else { > data->has_status_word = true; > @@ -2484,6 +2491,9 @@ static int pmbus_debugfs_get_status(void *data, u64 *val) > struct pmbus_debugfs_entry *entry = data; > struct pmbus_data *pdata = i2c_get_clientdata(entry->client); > > + if (!pdata->read_status) > + return -EINVAL; > + Unnecessary. > rc = pdata->read_status(entry->client, entry->page); > if (rc < 0) > return rc; > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-02 14:59 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-07-02 7:31 [PATCH v6 0/2] hwmon: (pmbus) Add support for MPS MP2949A ainux.wang 2021-07-02 7:31 ` [PATCH v6 1/2] " ainux.wang 2021-07-02 14:59 ` Guenter Roeck 2021-07-02 7:31 ` [PATCH v6 2/2] hwmon: (pmbus) Try use read_status() to read status register ainux.wang 2021-07-02 14:59 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).