linux-kernel - Re: [PATCH] Adding a driver for the INA260 chip of Texas Instrument. It follows the hardware monitoring kernel API.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2ee4bac8-8114-4c72-9b6c-6038869fb9f2@roeck-us.net>
Date:   Tue, 15 Aug 2023 08:45:39 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Loic Guegan <manzerbredes@...lbox.org>
Cc:     jdelvare@...e.com, linux-kernel@...r.kernel.org,
        linux-hwmon@...r.kernel.org
Subject: Re: [PATCH] Adding a driver for the INA260 chip of Texas Instrument.
 It follows the hardware monitoring kernel API.

On Tue, Aug 15, 2023 at 12:42:31PM +0200, Loic Guegan wrote:
> Signed-off-by: Loic Guegan <manzerbredes@...lbox.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@...lbox.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@...lbox.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