diff mbox series

Adding a driver for the INA260 chip of Texas Instrument. It follows the hardware monitoring kernel API.

Message ID ZNtWl_Jyj2PWBYpf@lguegan-thinkpad (mailing list archive)
State Changes Requested
Headers show
Series Adding a driver for the INA260 chip of Texas Instrument. It follows the hardware monitoring kernel API. | expand

Commit Message

Loic Guegan Aug. 15, 2023, 10:42 a.m. UTC
Signed-off-by: Loic Guegan <manzerbredes@mailbox.org>
---
 drivers/staging/Kconfig         |   2 +
 drivers/staging/Makefile        |   1 +
 drivers/staging/ina260/Kconfig  |   6 +
 drivers/staging/ina260/Makefile |   6 +
 drivers/staging/ina260/TODO     |   2 +
 drivers/staging/ina260/ina260.c | 261 ++++++++++++++++++++++++++++++++
 6 files changed, 278 insertions(+)
 create mode 100644 drivers/staging/ina260/Kconfig
 create mode 100644 drivers/staging/ina260/Makefile
 create mode 100644 drivers/staging/ina260/TODO
 create mode 100755 drivers/staging/ina260/ina260.c

Comments

Guenter Roeck Aug. 15, 2023, 3:45 p.m. UTC | #1
On Tue, Aug 15, 2023 at 12:42:31PM +0200, Loic Guegan wrote:
> Signed-off-by: Loic Guegan <manzerbredes@mailbox.org>

Subject and description are all wrong. I would suggest to read
and understand the documentation in Documentation/process
as well as Documentation/hwmon/submitting-patches.rst
before resubmitting.

> ---
>  drivers/staging/Kconfig         |   2 +
>  drivers/staging/Makefile        |   1 +
>  drivers/staging/ina260/Kconfig  |   6 +
>  drivers/staging/ina260/Makefile |   6 +
>  drivers/staging/ina260/TODO     |   2 +
>  drivers/staging/ina260/ina260.c | 261 ++++++++++++++++++++++++++++++++

No hwmon patches in drivers/staging/, please.

>  6 files changed, 278 insertions(+)
>  create mode 100644 drivers/staging/ina260/Kconfig
>  create mode 100644 drivers/staging/ina260/Makefile
>  create mode 100644 drivers/staging/ina260/TODO
>  create mode 100755 drivers/staging/ina260/ina260.c
> 
> diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
> index f9aef39ca..e173e4353 100644
> --- a/drivers/staging/Kconfig
> +++ b/drivers/staging/Kconfig
> @@ -78,4 +78,6 @@ source "drivers/staging/qlge/Kconfig"
>  
>  source "drivers/staging/vme_user/Kconfig"
>  
> +source "drivers/staging/ina260/Kconfig"
> +
>  endif # STAGING
> diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
> index ffa70dda4..a1d7e1ddb 100644
> --- a/drivers/staging/Makefile
> +++ b/drivers/staging/Makefile
> @@ -28,3 +28,4 @@ obj-$(CONFIG_PI433)		+= pi433/
>  obj-$(CONFIG_XIL_AXIS_FIFO)	+= axis-fifo/
>  obj-$(CONFIG_FIELDBUS_DEV)     += fieldbus/
>  obj-$(CONFIG_QLGE)		+= qlge/
> +obj-$(CONFIG_INA260)		+= ina260/
> \ No newline at end of file
> diff --git a/drivers/staging/ina260/Kconfig b/drivers/staging/ina260/Kconfig
> new file mode 100644
> index 000000000..e873abc9d
> --- /dev/null
> +++ b/drivers/staging/ina260/Kconfig
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +config INA260
> +	tristate "Support for INA260 Power Monitoring i2c chip"
> +	depends on I2C && REGMAP_I2C
> +	help
> +	  Support for the Texas Instrument INA260 power monitoring chip with precision integrated shunt.
> diff --git a/drivers/staging/ina260/Makefile b/drivers/staging/ina260/Makefile
> new file mode 100644
> index 000000000..d4eeba95e
> --- /dev/null
> +++ b/drivers/staging/ina260/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for the Texas Instruments INA260 drivers
> +#
> +
> +obj-$(CONFIG_INA260) += ina260.o
> diff --git a/drivers/staging/ina260/TODO b/drivers/staging/ina260/TODO
> new file mode 100644
> index 000000000..2ed5b80c3
> --- /dev/null
> +++ b/drivers/staging/ina260/TODO
> @@ -0,0 +1,2 @@
> +Created on: 15 August 2023
> +Contact: Loic GUEGAN <loic.guegan@mailbox.org>
> \ No newline at end of file
> diff --git a/drivers/staging/ina260/ina260.c b/drivers/staging/ina260/ina260.c
> new file mode 100755
> index 000000000..f827236b8
> --- /dev/null
> +++ b/drivers/staging/ina260/ina260.c
> @@ -0,0 +1,261 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for Texas Instruments INA260 power monitor chip
> + * with precision integrated shunt.
> + * Datasheet: https://www.ti.com/lit/gpn/INA260
> + *
> + * Copyright (C) 2023 GUEGAN Loic <loic.guegan@mailbox.org>
> + */
> +
> +#include "linux/module.h"
> +#include "linux/uaccess.h"
> +#include "linux/i2c.h"
> +#include "linux/kobject.h"
> +#include "linux/slab.h"
> +#include "linux/kernel.h"
> +#include <linux/sysfs.h>
> +#include <linux/hwmon.h>
> +#include <linux/regmap.h>
> +
> +// INA260 registers
> +#define INA260_REG_CONFIGURATION  0x00
> +#define INA260_REG_CURRENT        0x01
> +#define INA260_REG_VOLTAGE        0x02
> +#define INA260_REG_POWER          0x03
> +#define INA260_REG_MASKENABLE     0x06
> +#define INA260_REG_ALERTLIMIT     0x07
> +#define INA260_REG_MANUFACTURER   0xFE
> +#define INA260_REG_DIE            0xFF
> +

The register map is very much identical to ina219, ina220, ina226, ina230,
and ina231, all of which are supported by drivers/hwmon/ina2xx.c.
Please consider adding support for ina260 to that driver.

Guenter
Bagas Sanjaya Aug. 16, 2023, 3:32 a.m. UTC | #2
On Tue, Aug 15, 2023 at 08:45:39AM -0700, Guenter Roeck wrote:
> On Tue, Aug 15, 2023 at 12:42:31PM +0200, Loic Guegan wrote:
> > Signed-off-by: Loic Guegan <manzerbredes@mailbox.org>
> 
> Subject and description are all wrong. I would suggest to read
> and understand the documentation in Documentation/process
> as well as Documentation/hwmon/submitting-patches.rst
> before resubmitting.

And when submitting patches, do not GPG-sign them like ordinary emails,
but rather use patatt instead.
diff mbox series

Patch

diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index f9aef39ca..e173e4353 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -78,4 +78,6 @@  source "drivers/staging/qlge/Kconfig"
 
 source "drivers/staging/vme_user/Kconfig"
 
+source "drivers/staging/ina260/Kconfig"
+
 endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index ffa70dda4..a1d7e1ddb 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -28,3 +28,4 @@  obj-$(CONFIG_PI433)		+= pi433/
 obj-$(CONFIG_XIL_AXIS_FIFO)	+= axis-fifo/
 obj-$(CONFIG_FIELDBUS_DEV)     += fieldbus/
 obj-$(CONFIG_QLGE)		+= qlge/
+obj-$(CONFIG_INA260)		+= ina260/
\ No newline at end of file
diff --git a/drivers/staging/ina260/Kconfig b/drivers/staging/ina260/Kconfig
new file mode 100644
index 000000000..e873abc9d
--- /dev/null
+++ b/drivers/staging/ina260/Kconfig
@@ -0,0 +1,6 @@ 
+# SPDX-License-Identifier: GPL-2.0
+config INA260
+	tristate "Support for INA260 Power Monitoring i2c chip"
+	depends on I2C && REGMAP_I2C
+	help
+	  Support for the Texas Instrument INA260 power monitoring chip with precision integrated shunt.
diff --git a/drivers/staging/ina260/Makefile b/drivers/staging/ina260/Makefile
new file mode 100644
index 000000000..d4eeba95e
--- /dev/null
+++ b/drivers/staging/ina260/Makefile
@@ -0,0 +1,6 @@ 
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for the Texas Instruments INA260 drivers
+#
+
+obj-$(CONFIG_INA260) += ina260.o
diff --git a/drivers/staging/ina260/TODO b/drivers/staging/ina260/TODO
new file mode 100644
index 000000000..2ed5b80c3
--- /dev/null
+++ b/drivers/staging/ina260/TODO
@@ -0,0 +1,2 @@ 
+Created on: 15 August 2023
+Contact: Loic GUEGAN <loic.guegan@mailbox.org>
\ No newline at end of file
diff --git a/drivers/staging/ina260/ina260.c b/drivers/staging/ina260/ina260.c
new file mode 100755
index 000000000..f827236b8
--- /dev/null
+++ b/drivers/staging/ina260/ina260.c
@@ -0,0 +1,261 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for Texas Instruments INA260 power monitor chip
+ * with precision integrated shunt.
+ * Datasheet: https://www.ti.com/lit/gpn/INA260
+ *
+ * Copyright (C) 2023 GUEGAN Loic <loic.guegan@mailbox.org>
+ */
+
+#include "linux/module.h"
+#include "linux/uaccess.h"
+#include "linux/i2c.h"
+#include "linux/kobject.h"
+#include "linux/slab.h"
+#include "linux/kernel.h"
+#include <linux/sysfs.h>
+#include <linux/hwmon.h>
+#include <linux/regmap.h>
+
+// INA260 registers
+#define INA260_REG_CONFIGURATION  0x00
+#define INA260_REG_CURRENT        0x01
+#define INA260_REG_VOLTAGE        0x02
+#define INA260_REG_POWER          0x03
+#define INA260_REG_MASKENABLE     0x06
+#define INA260_REG_ALERTLIMIT     0x07
+#define INA260_REG_MANUFACTURER   0xFE
+#define INA260_REG_DIE            0xFF
+
+static struct regmap_config ina260_regmap_config = {
+	.max_register = INA260_REG_DIE,
+	.reg_bits = 8,
+	.val_bits = 16,
+};
+
+#define INA260_REG_SHOW(_attr, _reg) \
+static ssize_t _attr##_show(struct device *dev, struct device_attribute *attr, char *buf) \
+{ \
+	unsigned int rvalue; \
+	int err; \
+	struct client_data *cdata = dev_get_drvdata(dev); \
+	err = regmap_read(cdata->regmap, (_reg), &rvalue); \
+	if (err > 0) \
+		return err; \
+	return sprintf(buf, "0x%x\n", rvalue); \
+}
+
+#define INA260_REG_STORE(_attr, _reg) \
+static ssize_t _attr##_store(struct device *dev, struct device_attribute *attr, \
+const char *buf, size_t count) \
+{ \
+	int uvalue, err; \
+	struct client_data *cdata = dev_get_drvdata(dev); \
+	if (kstrtoint(buf, 0, &uvalue)) \
+		return -EINVAL; \
+	err = regmap_write(cdata->regmap, (_reg), uvalue); \
+	if (err > 0) \
+		return err; \
+	return count; \
+}
+
+/**
+ * @brief Embedded user data
+ */
+struct client_data {
+	struct i2c_client *client;
+	struct regmap *regmap;
+};
+
+static int ina260_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+			     u32 attr, int channel, long *val)
+{
+	int rvalue, reg, err, rem;
+	struct client_data *cdata = dev_get_drvdata(dev);
+
+	switch (type) {
+	case hwmon_power:
+		reg = INA260_REG_POWER;
+		break;
+	case hwmon_curr:
+		reg = INA260_REG_CURRENT;
+		break;
+	case hwmon_in:
+		reg = INA260_REG_VOLTAGE;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+	err = regmap_read(cdata->regmap, reg, &rvalue);
+	if (err < 0)
+		return err;
+	else if (type == hwmon_power)
+		*val = rvalue * 10000;
+	else
+		*val = div_u64_rem(rvalue * 25, 100, &rem) + rvalue + div_u64(rem, 10);
+	return 0;
+}
+
+static int ina260_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
+			      u32 attr, int channel, long val)
+{
+	return -EOPNOTSUPP;
+}
+
+INA260_REG_SHOW(configuration, INA260_REG_CONFIGURATION)
+INA260_REG_SHOW(curr, INA260_REG_CURRENT)
+INA260_REG_SHOW(bus_voltage, INA260_REG_VOLTAGE)
+INA260_REG_SHOW(power, INA260_REG_POWER)
+INA260_REG_SHOW(mask_enable, INA260_REG_MASKENABLE)
+INA260_REG_SHOW(alert_limit, INA260_REG_ALERTLIMIT)
+INA260_REG_SHOW(manufacturer_id, INA260_REG_MANUFACTURER)
+INA260_REG_SHOW(die_id, INA260_REG_DIE)
+
+INA260_REG_STORE(configuration, INA260_REG_CONFIGURATION)
+INA260_REG_STORE(curr, INA260_REG_CURRENT)
+INA260_REG_STORE(bus_voltage, INA260_REG_VOLTAGE)
+INA260_REG_STORE(power, INA260_REG_POWER)
+INA260_REG_STORE(mask_enable, INA260_REG_MASKENABLE)
+INA260_REG_STORE(alert_limit, INA260_REG_ALERTLIMIT)
+
+static umode_t ina260_hwmon_is_visible(const void *drvdata,
+				       enum hwmon_sensor_types type, u32 attr, int channel)
+{
+	switch (type) {
+	case hwmon_in:
+		switch (attr) {
+		case hwmon_in_input:
+			return 0444;
+		default:
+			return 0;
+		}
+	case hwmon_curr:
+		switch (attr) {
+		case hwmon_curr_input:
+			return 0444;
+		default:
+			return 0;
+		}
+	case hwmon_power:
+		switch (attr) {
+		case hwmon_power_input:
+			return 0444;
+		default:
+			return 0;
+		}
+	default:
+		return 0;
+	}
+}
+
+static const struct hwmon_channel_info *ina260_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
+	HWMON_CHANNEL_INFO(power, HWMON_P_INPUT),
+	HWMON_CHANNEL_INFO(curr, HWMON_C_INPUT),
+	NULL
+};
+
+static const struct hwmon_ops ina260_hwmon_ops = {
+	.is_visible = ina260_hwmon_is_visible,
+	.read = ina260_hwmon_read,
+	.write = ina260_hwmon_write,
+};
+
+static const struct hwmon_chip_info ina260_chip_info = {
+	.ops = &ina260_hwmon_ops,
+	.info = ina260_hwmon_info,
+};
+
+// ----- Registers -----
+static DEVICE_ATTR_RW(configuration);
+static DEVICE_ATTR_RW(curr);
+static DEVICE_ATTR_RW(bus_voltage);
+static DEVICE_ATTR_RW(power);
+static DEVICE_ATTR_RW(mask_enable);
+static DEVICE_ATTR_RW(alert_limit);
+static DEVICE_ATTR_RO(manufacturer_id);
+static DEVICE_ATTR_RO(die_id);
+static struct attribute *registers_attrs[] = {
+	&dev_attr_configuration.attr,
+	&dev_attr_curr.attr,
+	&dev_attr_bus_voltage.attr,
+	&dev_attr_power.attr,
+	&dev_attr_mask_enable.attr,
+	&dev_attr_alert_limit.attr,
+	&dev_attr_manufacturer_id.attr,
+	&dev_attr_die_id.attr,
+	NULL,
+};
+
+static const struct attribute_group registers_group = {
+	.attrs = registers_attrs,
+	.name = "registers"
+};
+
+const struct attribute_group *extra_groups[] = {
+	&registers_group,
+	NULL
+};
+
+static int ina260_probe_new(struct i2c_client *client)
+{
+	struct client_data *p;
+	struct device *hwmon_dev;
+
+	// Initialize client data:
+	dev_dbg(&client->dev, "Adding ina260 [bus=%d address=0x%02x]\n",
+		client->adapter->nr, client->addr);
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	p->client = client;
+	p->regmap = devm_regmap_init_i2c(client, &ina260_regmap_config);
+
+	hwmon_dev = hwmon_device_register_with_info(&client->dev, client->name, p,
+						    &ina260_chip_info, extra_groups);
+	if (IS_ERR(hwmon_dev))
+		return PTR_ERR(hwmon_dev);
+	return 0;
+}
+
+static void ina260_remove(struct i2c_client *client)
+{
+	struct client_data *p = i2c_get_clientdata(client);
+
+	kfree(p);
+	hwmon_device_unregister(&client->dev);
+	dev_dbg(&client->dev, "Removing ina260 [bus=%d address=0x%02x]\n",
+		client->adapter->nr, client->addr);
+}
+
+static const struct i2c_device_id ina260_ids[] = {
+	{ "ina260", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ina260_ids);
+
+static struct i2c_driver ina260_driver = {
+	.class = I2C_CLASS_HWMON,
+	.driver = {
+		.name = "ina260"
+	},
+	.probe_new = ina260_probe_new,
+	.remove = ina260_remove,
+	.id_table = ina260_ids
+};
+
+static int __init ina260_init(void)
+{
+	i2c_add_driver(&ina260_driver);
+	return 0;
+}
+
+static void __exit ina260_exit(void)
+{
+	i2c_del_driver(&ina260_driver);
+}
+
+module_init(ina260_init);
+module_exit(ina260_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Loïc Guegan");
+MODULE_DESCRIPTION("INA260 Texas Instruments");
+MODULE_VERSION("1.0");