devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [ v1] hwmon: (pmbus) Add Wistron power supply pmbus driver
@ 2019-11-29  6:02 Ben Pai
  2019-11-29 18:16 ` Guenter Roeck
  0 siblings, 1 reply; 2+ messages in thread
From: Ben Pai @ 2019-11-29  6:02 UTC (permalink / raw)
  To: linux
  Cc: robh+dt, jdelvare, linux-kernel, linux-hwmon, devicetree, corbet,
	wangat, Andy_YF_Wang, Claire_Ku, Ben Pai

Add the driver to monitor Wisreon power supplies with hwmon over pmbus.

Signed-off-by: Ben Pai <Ben_Pai@wistron.com>
---
 drivers/hwmon/pmbus/Kconfig       |   9 ++
 drivers/hwmon/pmbus/Makefile      |   1 +
 drivers/hwmon/pmbus/wistron-wps.c | 180 ++++++++++++++++++++++++++++++
 3 files changed, 190 insertions(+)
 create mode 100644 drivers/hwmon/pmbus/wistron-wps.c

diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index d62d69bb7e49..ebb7024e58ab 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -219,6 +219,15 @@ config SENSORS_UCD9200
 	  This driver can also be built as a module. If so, the module will
 	  be called ucd9200.
 
+config SENSORS_WISTRON_WPS
+	tristate "Wistron Power Supply"
+	help
+	  If you say yes here you get hardware monitoring support for the Wistron
+	  power supply.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called wistron-wps.
+
 config SENSORS_ZL6100
 	tristate "Intersil ZL6100 and compatibles"
 	help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index 03bacfcfd660..cad38f99e8c5 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -25,4 +25,5 @@ obj-$(CONFIG_SENSORS_TPS40422)	+= tps40422.o
 obj-$(CONFIG_SENSORS_TPS53679)	+= tps53679.o
 obj-$(CONFIG_SENSORS_UCD9000)	+= ucd9000.o
 obj-$(CONFIG_SENSORS_UCD9200)	+= ucd9200.o
+obj-$(CONFIG_SENSORS_WISTRON_WPS) += wistron-wps.o
 obj-$(CONFIG_SENSORS_ZL6100)	+= zl6100.o
diff --git a/drivers/hwmon/pmbus/wistron-wps.c b/drivers/hwmon/pmbus/wistron-wps.c
new file mode 100644
index 000000000000..764496aa9d4f
--- /dev/null
+++ b/drivers/hwmon/pmbus/wistron-wps.c
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2019 Wistron Corp.
+ */
+
+#include <linux/bitops.h>
+#include <linux/debugfs.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/i2c.h>
+#include <linux/jiffies.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pmbus.h>
+
+#include "pmbus.h"
+
+#define WPS_ID_CMD				0x99
+#define WPS_PN_CMD				0x9A
+#define WPS_FW_CMD				0x9B
+#define WPS_DATE_CMD				0x9D
+#define WPS_SN_CMD				0x9E
+
+enum {
+	WPS_DEBUGFS_ID,
+	WPS_DEBUGFS_PN,
+	WPS_DEBUGFS_SN,
+	WPS_DEBUGFS_FW,
+	WPS_DEBUGFS_DATE,
+	WPS_DEBUGFS_NUM_ENTRIES
+};
+
+struct wistron_wps {
+
+	struct i2c_client *client;
+
+	int debugfs_entries[WPS_DEBUGFS_NUM_ENTRIES];
+
+};
+
+#define to_psu(x, y) container_of((x), struct wistron_wps, debugfs_entries[(y)])
+
+static ssize_t wistron_wps_debugfs_op(struct file *file, char __user *buf,
+				    size_t count, loff_t *ppos)
+{
+	u8 cmd;
+	int rc;
+	int *idxp = file->private_data;
+	int idx = *idxp;
+	struct wistron_wps *psu = to_psu(idxp, idx);
+	char data[I2C_SMBUS_BLOCK_MAX] = { 0 };
+
+	switch (idx) {
+	case WPS_DEBUGFS_ID:
+		cmd = WPS_ID_CMD;
+		break;
+	case WPS_DEBUGFS_PN:
+		cmd = WPS_PN_CMD;
+		break;
+	case WPS_DEBUGFS_SN:
+		cmd = WPS_SN_CMD;
+		break;
+	case WPS_DEBUGFS_FW:
+		cmd = WPS_FW_CMD;
+		break;
+	case WPS_DEBUGFS_DATE:
+		cmd = WPS_DATE_CMD;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	rc = i2c_smbus_read_block_data(psu->client, cmd, data);
+	if (rc < 0)
+		return rc;
+
+done:
+	data[rc] = '\n';
+	rc += 2;
+
+	return simple_read_from_buffer(buf, count, ppos, data, rc);
+}
+
+static const struct file_operations wistron_wps_fops = {
+	.llseek = noop_llseek,
+	.read = wistron_wps_debugfs_op,
+	.open = simple_open,
+};
+
+static struct pmbus_driver_info wistron_wps_info = {
+	.pages = 1,
+	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT |
+		PMBUS_HAVE_PIN | PMBUS_HAVE_POUT | PMBUS_HAVE_FAN12 |
+		PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 |
+		PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT |
+		PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP |
+		PMBUS_HAVE_STATUS_FAN12,
+};
+
+static struct pmbus_platform_data wistron_wps_pdata = {
+	.flags = PMBUS_SKIP_STATUS_CHECK,
+};
+
+static int wistron_wps_probe(struct i2c_client *client,
+			   const struct i2c_device_id *id)
+{
+	int i, rc;
+	struct dentry *debugfs;
+	struct dentry *wistron_wps_dir;
+	struct wistron_wps *psu;
+
+	client->dev.platform_data = &wistron_wps_pdata;
+	rc = pmbus_do_probe(client, id, &wistron_wps_info);
+	if (rc)
+		return rc;
+
+	psu = devm_kzalloc(&client->dev, sizeof(*psu), GFP_KERNEL);
+	if (!psu)
+		return 0;
+
+	psu->client = client;
+
+	debugfs = pmbus_get_debugfs_dir(client);
+	if (!debugfs)
+		return 0;
+
+	wistron_wps_dir = debugfs_create_dir(client->name, debugfs);
+	if (!wistron_wps_dir)
+		return 0;
+
+	for (i = 0; i < WPS_DEBUGFS_NUM_ENTRIES; ++i)
+		psu->debugfs_entries[i] = i;
+
+	debugfs_create_file("fru", 0444, wistron_wps_dir,
+			    &psu->debugfs_entries[WPS_DEBUGFS_ID],
+			    &wistron_wps_fops);
+	debugfs_create_file("part_number", 0444, wistron_wps_dir,
+			    &psu->debugfs_entries[WPS_DEBUGFS_PN],
+			    &wistron_wps_fops);
+	debugfs_create_file("serial_number", 0444, wistron_wps_dir,
+			    &psu->debugfs_entries[WPS_DEBUGFS_SN],
+			    &wistron_wps_fops);
+	debugfs_create_file("fw_version", 0444, wistron_wps_dir,
+			    &psu->debugfs_entries[WPS_DEBUGFS_FW],
+			    &wistron_wps_fops);
+	debugfs_create_file("mfr_date", 0444, wistron_wps_dir,
+			    &psu->debugfs_entries[WPS_DEBUGFS_DATE],
+			    &wistron_wps_fops);
+
+	return 0;
+}
+
+static const struct i2c_device_id wistron_wps_id[] = {
+	{ "wistron_wps", 1 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, wistron_wps_id);
+
+static const struct of_device_id wistron_wps_of_match[] = {
+	{ .compatible = "wistron,wps" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, wistron_wps_of_match);
+
+static struct i2c_driver wistron_wps_driver = {
+	.driver = {
+		.name = "wistron-wps",
+		.of_match_table = wistron_wps_of_match,
+	},
+	.probe = wistron_wps_probe,
+	.remove = pmbus_do_remove,
+	.id_table = wistron_wps_id,
+};
+
+module_i2c_driver(wistron_wps_driver);
+
+MODULE_AUTHOR("Ben Pai");
+MODULE_DESCRIPTION("PMBus driver for Wistron power supplies");
+MODULE_LICENSE("GPL");
-- 
2.17.1


---------------------------------------------------------------------------------------------------------------------------------------------------------------
This email contains confidential or legally privileged information and is for the sole use of its intended recipient. 
Any unauthorized review, use, copying or distribution of this email or the content of this email is strictly prohibited.
If you are not the intended recipient, you may reply to the sender and should delete this e-mail immediately.
---------------------------------------------------------------------------------------------------------------------------------------------------------------

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [ v1] hwmon: (pmbus) Add Wistron power supply pmbus driver
  2019-11-29  6:02 [ v1] hwmon: (pmbus) Add Wistron power supply pmbus driver Ben Pai
@ 2019-11-29 18:16 ` Guenter Roeck
  0 siblings, 0 replies; 2+ messages in thread
From: Guenter Roeck @ 2019-11-29 18:16 UTC (permalink / raw)
  To: Ben Pai
  Cc: robh+dt, jdelvare, linux-kernel, linux-hwmon, devicetree, corbet,
	wangat, Andy_YF_Wang, Claire_Ku

On Fri, Nov 29, 2019 at 02:02:30PM +0800, Ben Pai wrote:
> Add the driver to monitor Wisreon power supplies with hwmon over pmbus.

Wistron ?

> 
> Signed-off-by: Ben Pai <Ben_Pai@wistron.com>
> ---
>  drivers/hwmon/pmbus/Kconfig       |   9 ++
>  drivers/hwmon/pmbus/Makefile      |   1 +
>  drivers/hwmon/pmbus/wistron-wps.c | 180 ++++++++++++++++++++++++++++++
>  3 files changed, 190 insertions(+)
>  create mode 100644 drivers/hwmon/pmbus/wistron-wps.c
> 
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index d62d69bb7e49..ebb7024e58ab 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -219,6 +219,15 @@ config SENSORS_UCD9200
>  	  This driver can also be built as a module. If so, the module will
>  	  be called ucd9200.
>  
> +config SENSORS_WISTRON_WPS
> +	tristate "Wistron Power Supply"
> +	help
> +	  If you say yes here you get hardware monitoring support for the Wistron
> +	  power supply.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called wistron-wps.
> +
>  config SENSORS_ZL6100
>  	tristate "Intersil ZL6100 and compatibles"
>  	help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 03bacfcfd660..cad38f99e8c5 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -25,4 +25,5 @@ obj-$(CONFIG_SENSORS_TPS40422)	+= tps40422.o
>  obj-$(CONFIG_SENSORS_TPS53679)	+= tps53679.o
>  obj-$(CONFIG_SENSORS_UCD9000)	+= ucd9000.o
>  obj-$(CONFIG_SENSORS_UCD9200)	+= ucd9200.o
> +obj-$(CONFIG_SENSORS_WISTRON_WPS) += wistron-wps.o
>  obj-$(CONFIG_SENSORS_ZL6100)	+= zl6100.o
> diff --git a/drivers/hwmon/pmbus/wistron-wps.c b/drivers/hwmon/pmbus/wistron-wps.c
> new file mode 100644
> index 000000000000..764496aa9d4f
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/wistron-wps.c
> @@ -0,0 +1,180 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2019 Wistron Corp.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/debugfs.h>
> +#include <linux/device.h>
> +#include <linux/fs.h>
> +#include <linux/i2c.h>
> +#include <linux/jiffies.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pmbus.h>
> +
> +#include "pmbus.h"
> +
> +#define WPS_ID_CMD				0x99
> +#define WPS_PN_CMD				0x9A
> +#define WPS_FW_CMD				0x9B
> +#define WPS_DATE_CMD				0x9D
> +#define WPS_SN_CMD				0x9E
> +
> +enum {
> +	WPS_DEBUGFS_ID,
> +	WPS_DEBUGFS_PN,
> +	WPS_DEBUGFS_SN,
> +	WPS_DEBUGFS_FW,
> +	WPS_DEBUGFS_DATE,
> +	WPS_DEBUGFS_NUM_ENTRIES
> +};
> +
> +struct wistron_wps {
> +
> +	struct i2c_client *client;
> +
> +	int debugfs_entries[WPS_DEBUGFS_NUM_ENTRIES];
> +
> +};
> +
> +#define to_psu(x, y) container_of((x), struct wistron_wps, debugfs_entries[(y)])

container_of() doesn't really need the extra ().

> +
> +static ssize_t wistron_wps_debugfs_op(struct file *file, char __user *buf,
> +				    size_t count, loff_t *ppos)

Please align continuation lines with '('.

> +{
> +	u8 cmd;
> +	int rc;
> +	int *idxp = file->private_data;
> +	int idx = *idxp;
> +	struct wistron_wps *psu = to_psu(idxp, idx);
> +	char data[I2C_SMBUS_BLOCK_MAX] = { 0 };
> +
> +	switch (idx) {
> +	case WPS_DEBUGFS_ID:
> +		cmd = WPS_ID_CMD;
> +		break;
> +	case WPS_DEBUGFS_PN:
> +		cmd = WPS_PN_CMD;
> +		break;
> +	case WPS_DEBUGFS_SN:
> +		cmd = WPS_SN_CMD;
> +		break;
> +	case WPS_DEBUGFS_FW:
> +		cmd = WPS_FW_CMD;
> +		break;
> +	case WPS_DEBUGFS_DATE:
> +		cmd = WPS_DATE_CMD;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	rc = i2c_smbus_read_block_data(psu->client, cmd, data);
> +	if (rc < 0)
> +		return rc;
> +
> +done:
> +	data[rc] = '\n';

The block command can return up to 32 bytes. If it does, the above code
writes beyond the end of the buffer.

> +	rc += 2;

Why += 2 ? This will report the trailing '\0' to userspace (and require
an even larger buffer). Is that intentional ?

> +
> +	return simple_read_from_buffer(buf, count, ppos, data, rc);
> +}
> +
> +static const struct file_operations wistron_wps_fops = {
> +	.llseek = noop_llseek,
> +	.read = wistron_wps_debugfs_op,
> +	.open = simple_open,
> +};
> +
> +static struct pmbus_driver_info wistron_wps_info = {
> +	.pages = 1,
> +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT |
> +		PMBUS_HAVE_PIN | PMBUS_HAVE_POUT | PMBUS_HAVE_FAN12 |
> +		PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 |
> +		PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT |
> +		PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP |
> +		PMBUS_HAVE_STATUS_FAN12,
> +};
> +
> +static struct pmbus_platform_data wistron_wps_pdata = {
> +	.flags = PMBUS_SKIP_STATUS_CHECK,

This should be explained if it is indeed needed.

> +};
> +
> +static int wistron_wps_probe(struct i2c_client *client,
> +			   const struct i2c_device_id *id)

Please match ( in continuation lines.

> +{
> +	int i, rc;
> +	struct dentry *debugfs;
> +	struct dentry *wistron_wps_dir;
> +	struct wistron_wps *psu;
> +
> +	client->dev.platform_data = &wistron_wps_pdata;
> +	rc = pmbus_do_probe(client, id, &wistron_wps_info);
> +	if (rc)
> +		return rc;
> +
> +	psu = devm_kzalloc(&client->dev, sizeof(*psu), GFP_KERNEL);
> +	if (!psu)
> +		return 0;
> +
> +	psu->client = client;
> +
> +	debugfs = pmbus_get_debugfs_dir(client);
> +	if (!debugfs)
> +		return 0;
> +
> +	wistron_wps_dir = debugfs_create_dir(client->name, debugfs);
> +	if (!wistron_wps_dir)
> +		return 0;
> +
> +	for (i = 0; i < WPS_DEBUGFS_NUM_ENTRIES; ++i)
> +		psu->debugfs_entries[i] = i;
> +
> +	debugfs_create_file("fru", 0444, wistron_wps_dir,
> +			    &psu->debugfs_entries[WPS_DEBUGFS_ID],
> +			    &wistron_wps_fops);
> +	debugfs_create_file("part_number", 0444, wistron_wps_dir,
> +			    &psu->debugfs_entries[WPS_DEBUGFS_PN],
> +			    &wistron_wps_fops);
> +	debugfs_create_file("serial_number", 0444, wistron_wps_dir,
> +			    &psu->debugfs_entries[WPS_DEBUGFS_SN],
> +			    &wistron_wps_fops);
> +	debugfs_create_file("fw_version", 0444, wistron_wps_dir,
> +			    &psu->debugfs_entries[WPS_DEBUGFS_FW],
> +			    &wistron_wps_fops);
> +	debugfs_create_file("mfr_date", 0444, wistron_wps_dir,
> +			    &psu->debugfs_entries[WPS_DEBUGFS_DATE],
> +			    &wistron_wps_fops);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id wistron_wps_id[] = {
> +	{ "wistron_wps", 1 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, wistron_wps_id);
> +
> +static const struct of_device_id wistron_wps_of_match[] = {
> +	{ .compatible = "wistron,wps" },

This will need to be documented. It is also probably not the best name
for a devicetree property (what is "wps" ?), but that will be up to
a DT maintainer to decide.

> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, wistron_wps_of_match);
> +
> +static struct i2c_driver wistron_wps_driver = {
> +	.driver = {
> +		.name = "wistron-wps",
> +		.of_match_table = wistron_wps_of_match,
> +	},
> +	.probe = wistron_wps_probe,
> +	.remove = pmbus_do_remove,
> +	.id_table = wistron_wps_id,
> +};
> +
> +module_i2c_driver(wistron_wps_driver);
> +
> +MODULE_AUTHOR("Ben Pai");
> +MODULE_DESCRIPTION("PMBus driver for Wistron power supplies");
> +MODULE_LICENSE("GPL");

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-11-29 18:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-29  6:02 [ v1] hwmon: (pmbus) Add Wistron power supply pmbus driver Ben Pai
2019-11-29 18:16 ` 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).