+ help
+ If you say yes here you get six integrated buck converter
regulator
+ with hardware monitoring functionality support for power
management
+ IC MPS MPQ7932.
That description is more appropriate for the second configuration option.
Primarily one gets hardware monitoring support for the chip.
+
+ This driver can also be built as a module. If so, the module will
+ be called mpq7932.
+
config SENSORS_PIM4328
tristate "Flex PIM4328 and compatibles"
help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index 0002dbe22d52..28a534629cc3 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_SENSORS_MAX8688) += max8688.o
obj-$(CONFIG_SENSORS_MP2888) += mp2888.o
obj-$(CONFIG_SENSORS_MP2975) += mp2975.o
obj-$(CONFIG_SENSORS_MP5023) += mp5023.o
+obj-$(CONFIG_SENSORS_MPQ7932_REGULATOR) += mpq7932.o
obj-$(CONFIG_SENSORS_PLI1209BC) += pli1209bc.o
obj-$(CONFIG_SENSORS_PM6764TR) += pm6764tr.o
obj-$(CONFIG_SENSORS_PXE1610) += pxe1610.o
diff --git a/drivers/hwmon/pmbus/mpq7932.c
b/drivers/hwmon/pmbus/mpq7932.c
new file mode 100644
index 000000000000..3747d7862afd
--- /dev/null
+++ b/drivers/hwmon/pmbus/mpq7932.c
@@ -0,0 +1,144 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0+
The SPDX license must be in the first line and be a C++ style comment.
Please run checkpatch --strict and fix what it reports (including the
various continuation line misalignments and unnecessary empty lines).
+ *
+ * mpq7932.c - regulator driver for mps mpq7932
+ * Copyright 2022 Monolithic Power Systems, Inc
This is a hwmon driver with optional regulator functionality.
+ *
+ * Author: Saravanan Sekar <saravanan@xxxxxxxxxxx>
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/pmbus.h>
+#include "pmbus.h"
+
+#define MPQ7932_BUCK_UV_MIN 206250
+#define MPQ7932_UV_STEP 6250
+#define MPQ7932_N_VOLTAGES 0xFF
+#define MPQ7932_NUM_PAGES 6
+
+#define MPQ7932_TON_DELAY 0x60
+#define MPQ7932_VOUT_STARTUP_SLEW 0xA3
+#define MPQ7932_VOUT_SHUTDOWN_SLEW 0xA5
+#define MPQ7932_VOUT_SLEW_MASK GENMASK(1, 0)
+#define MPQ7932_TON_DELAY_MASK GENMASK(4, 0)
Please include the appropriate include file defining GENMASK.
+
+struct mpq7932_data {
+ struct pmbus_driver_info info;
+ struct pmbus_platform_data pdata;
+};
+
+static struct regulator_desc mpq7932_regulators_desc[] = {
+ PMBUS_REGULATOR_STEP("buck", 0, MPQ7932_N_VOLTAGES,
+ MPQ7932_UV_STEP, MPQ7932_BUCK_UV_MIN),
+ PMBUS_REGULATOR_STEP("buck", 1, MPQ7932_N_VOLTAGES,
+ MPQ7932_UV_STEP, MPQ7932_BUCK_UV_MIN),
+ PMBUS_REGULATOR_STEP("buck", 2, MPQ7932_N_VOLTAGES,
+ MPQ7932_UV_STEP, MPQ7932_BUCK_UV_MIN),
+ PMBUS_REGULATOR_STEP("buck", 3, MPQ7932_N_VOLTAGES,
+ MPQ7932_UV_STEP, MPQ7932_BUCK_UV_MIN),
+ PMBUS_REGULATOR_STEP("buck", 4, MPQ7932_N_VOLTAGES,
+ MPQ7932_UV_STEP, MPQ7932_BUCK_UV_MIN),
+ PMBUS_REGULATOR_STEP("buck", 5, MPQ7932_N_VOLTAGES,
+ MPQ7932_UV_STEP, MPQ7932_BUCK_UV_MIN),
+};
+
+static int mpq7932_write_word_data(struct i2c_client *client, int
page, int reg,
+ u16 word)
+{
+
+ switch (reg) {
+ case PMBUS_VOUT_COMMAND:
This needs a comment explaining why it is needed.
+ return pmbus_write_byte_data(client, page, reg, (u8)word);
word should be clamped to [0, 255], not cut off.
+
+ default:
+ return -ENODATA;
+ }
+}
+
+static int mpq7932_read_word_data(struct i2c_client *client, int page,
+ int phase, int reg)
+{
+
+ switch (reg) {
+ case PMBUS_MFR_VOUT_MIN:
+ return 0;
+
+ case PMBUS_MFR_VOUT_MAX:
+ return MPQ7932_N_VOLTAGES;
The above need comments. Also, MPQ7932_N_VOLTAGES is inappropriate. This
is not the
number of voltages, it is the maximum voltage. Even if the values happen
to be the
same, the content is different.
Also, with PMBUS_MFR_VOUT_MIN=0 and PMBUS_MFR_VOUT_MAX=0xff, the number
of voltages
would actually be 256, not 255.
+
+ case PMBUS_READ_VOUT:
+ return pmbus_read_byte_data(client, page, PMBUS_VOUT_COMMAND);
+
Needs same comment as above.
+ default:
+ return -ENODATA;
+ }
+}
+
+static int mpq7932_probe(struct i2c_client *client)
+{
+ struct mpq7932_data *data;
+ struct pmbus_driver_info *info;
+ struct device *dev = &client->dev;
+ int i;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_READ_WORD_DATA))
Unnecessary check. This code doesn't use it, and pmbus_do_probe()
does its own check.
+ return -ENODEV;
+
+ data = devm_kzalloc(&client->dev, sizeof(struct mpq7932_data),
Use dev.
+ GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ info = &data->info;
+ info->pages = MPQ7932_NUM_PAGES;
+ info->num_regulators = ARRAY_SIZE(mpq7932_regulators_desc);
+ info->reg_desc = mpq7932_regulators_desc;
+ info->format[PSC_VOLTAGE_OUT] = direct;
+ info->m[PSC_VOLTAGE_OUT] = 160;
+ info->b[PSC_VOLTAGE_OUT] = -33;
+ for (i = 0; i < info->pages; i++) {
+ info->func[i] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
+ | PMBUS_HAVE_STATUS_TEMP;
I think I already asked: Is this really all telemetry supported by the
chip ?
I keep asking because that would be highly unusual.
