All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/10] Add per channel properies support in tmp421
@ 2021-10-12  9:14 Krzysztof Adamski
  2021-10-12  9:14 ` [PATCH v4 01/10] dt-bindings: hwmon: add missing tmp421 binding Krzysztof Adamski
                   ` (9 more replies)
  0 siblings, 10 replies; 26+ messages in thread
From: Krzysztof Adamski @ 2021-10-12  9:14 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

Hi,

This series adds support for defining per-channel properies (like
n-factor and label) to the TMP421 driver. It starts by adding the
missing DT binding for tmp421, in the form that was there before any of
my changes. Then I do the changes to the driver and finally adjust the
bindings to my changes.

The precedence for this case is:
[PATCH v9 2/2] hwmon: (ina3221) Read channel input source info from DT
Which can be found here:
https://lkml.org/lkml/2018/10/2/136

My patches does similar thing but to tmp422 - we need a way to define
the labels for specific channels as well as to define the n-factor (that
is board specific as it depends on the diodes used for remote sensing).
A possibility to disable unused channels seems like a good idea too.

Here comes v4. Changes compared to v3:
- fixed of by one error in the tmp421_probe_child_from_dt
- fixed errors in binding description
  - there are maximum 3 remote sensors
  - the n-factor range is -128..127
- changed "input@X" nodes into "channel@X" in DT
- changed "n-factor" property into "ti,n-factor" in DT

Changes compared to v2:
- fixed the $id path in DT
- moved the tmp421_enable_channels() call inside of tmp421_init_client()
- fixed some unneeded line brakes
- added "ignore non input related DT nodes" patch for skipping all
  subnodes that are not input@X without raising an error. I'm not
  completly convinced this is what we should do so I made it a separate
  patch so that you can easily skip it, if you decide to

Changes compared to v1:
- fixed sparse warnings about making function declarations static
- changed the policy for broken DT - in case of errors, the probe will
  return an error instead of continuing
- in addition to disabling the channels specified in DT, the probe
  function will also enable all the others in case they were disabled by
  some other code earlier
  NOTE: this may be a backwards incompatible change - if some channels
  were disabled by some bootloader code previously the channels would
  stay like that and now they would be enabled during probe, even if
  nothing is specified in DT. Is this what we want?
- added support for HWMON_T_ENABLE
- updated documentation
- NOTE: I haven't changed anything related to DT as the discussion has
  no clear conclusion yet.


Krzysztof Adamski (10):
  dt-bindings: hwmon: add missing tmp421 binding
  hwmon: (tmp421) introduce a channel struct
  hwmon: (tmp421) add support for defining labels from DT
  hwmon: (tmp421) support disabling channels from DT
  hwmon: (tmp421) support specifying n-factor via DT
  hwmon: (tmp421) really disable channels
  hwmon: (tmp421) support HWMON_T_ENABLE
  hwmon: (tmp421) update documentation
  hwmon: (tmp421) ignore non-channel related DT nodes
  dt-bindings: hwmon: allow specifying channels for tmp421

 .../devicetree/bindings/hwmon/ti,tmp421.yaml  | 110 ++++++++++++
 Documentation/hwmon/tmp421.rst                |  10 ++
 drivers/hwmon/tmp421.c                        | 168 ++++++++++++++++--
 3 files changed, 278 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml

-- 
2.31.1


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

* [PATCH v4 01/10] dt-bindings: hwmon: add missing tmp421 binding
  2021-10-12  9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
@ 2021-10-12  9:14 ` Krzysztof Adamski
  2021-10-12  9:26 ` [PATCH v4 02/10] hwmon: (tmp421) introduce a channel struct Krzysztof Adamski
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Krzysztof Adamski @ 2021-10-12  9:14 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

Add basic description of the tmp421 driver DT bindings.

Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
 .../devicetree/bindings/hwmon/ti,tmp421.yaml  | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml b/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml
new file mode 100644
index 000000000000..47040ace4f73
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/ti,tmp421.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TMP42x/TMP44x temperature sensor
+
+maintainers:
+  - Guenter Roeck <linux@roeck-us.net>
+
+description: |
+  ±1°C Remote and Local temperature sensor
+  https://www.ti.com/lit/ds/symlink/tmp422.pdf
+
+properties:
+  compatible:
+    enum:
+      - ti,tmp421
+      - ti,tmp422
+      - ti,tmp423
+      - ti,tmp441
+      - ti,tmp442
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      sensor@4c {
+        compatible = "ti,tmp422";
+        reg = <0x4c>;
+      };
+    };
-- 
2.31.1


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

* [PATCH v4 02/10] hwmon: (tmp421) introduce a channel struct
  2021-10-12  9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
  2021-10-12  9:14 ` [PATCH v4 01/10] dt-bindings: hwmon: add missing tmp421 binding Krzysztof Adamski
@ 2021-10-12  9:26 ` Krzysztof Adamski
  2021-10-12 14:25   ` Guenter Roeck
  2021-10-12  9:27 ` [PATCH v4 03/10] hwmon: (tmp421) add support for defining labels from DT Krzysztof Adamski
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Krzysztof Adamski @ 2021-10-12  9:26 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

This is a preparatory change. Upcoming patches will introduce more
per-channel parameters so it's worth organizing them into a struct.

Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
 drivers/hwmon/tmp421.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 356c96c3588b..707310d616a4 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -87,6 +87,10 @@ static const struct of_device_id __maybe_unused tmp421_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tmp421_of_match);
 
+struct tmp421_channel {
+	s16 temp;
+};
+
 struct tmp421_data {
 	struct i2c_client *client;
 	struct mutex update_lock;
@@ -98,7 +102,7 @@ struct tmp421_data {
 	unsigned long last_updated;
 	unsigned long channels;
 	u8 config;
-	s16 temp[MAX_CHANNELS];
+	struct tmp421_channel channel[MAX_CHANNELS];
 };
 
 static int temp_from_raw(u16 reg, bool extended)
@@ -133,12 +137,12 @@ static int tmp421_update_device(struct tmp421_data *data)
 			ret = i2c_smbus_read_byte_data(client, TMP421_TEMP_MSB[i]);
 			if (ret < 0)
 				goto exit;
-			data->temp[i] = ret << 8;
+			data->channel[i].temp = ret << 8;
 
 			ret = i2c_smbus_read_byte_data(client, TMP421_TEMP_LSB[i]);
 			if (ret < 0)
 				goto exit;
-			data->temp[i] |= ret;
+			data->channel[i].temp |= ret;
 		}
 		data->last_updated = jiffies;
 		data->valid = true;
@@ -167,7 +171,7 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
 
 	switch (attr) {
 	case hwmon_temp_input:
-		*val = temp_from_raw(tmp421->temp[channel],
+		*val = temp_from_raw(tmp421->channel[channel].temp,
 				     tmp421->config & TMP421_CONFIG_RANGE);
 		return 0;
 	case hwmon_temp_fault:
@@ -175,7 +179,7 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
 		 * Any of OPEN or /PVLD bits indicate a hardware mulfunction
 		 * and the conversion result may be incorrect
 		 */
-		*val = !!(tmp421->temp[channel] & 0x03);
+		*val = !!(tmp421->channel[channel].temp & 0x03);
 		return 0;
 	default:
 		return -EOPNOTSUPP;
-- 
2.31.1


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

* [PATCH v4 03/10] hwmon: (tmp421) add support for defining labels from DT
  2021-10-12  9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
  2021-10-12  9:14 ` [PATCH v4 01/10] dt-bindings: hwmon: add missing tmp421 binding Krzysztof Adamski
  2021-10-12  9:26 ` [PATCH v4 02/10] hwmon: (tmp421) introduce a channel struct Krzysztof Adamski
@ 2021-10-12  9:27 ` Krzysztof Adamski
  2021-10-12 14:30   ` Guenter Roeck
  2021-10-12  9:27 ` [PATCH v4 04/10] hwmon: (tmp421) support disabling channels " Krzysztof Adamski
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Krzysztof Adamski @ 2021-10-12  9:27 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

tmp42x is a multichannel temperature sensor with several external
channels. Since those channels can be used to connect diodes placed
anywhere in the system, their meaning will vary depending on the
project. For this case, the hwmon framework has an idea of labels which
allows us to assign the meaning to each channel.

The similar concept is already implemented in ina3221 - the label for
each channel can be defined via device tree. See commit a9e9dd9c6de5
("hwmon: (ina3221) Read channel input source info from DT")

This patch adds support for similar feature to tmp421.

Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
 drivers/hwmon/tmp421.c | 61 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 707310d616a4..ab64d9825ca4 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -88,6 +88,7 @@ static const struct of_device_id __maybe_unused tmp421_of_match[] = {
 MODULE_DEVICE_TABLE(of, tmp421_of_match);
 
 struct tmp421_channel {
+	const char *label;
 	s16 temp;
 };
 
@@ -187,6 +188,16 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
 
 }
 
+static int tmp421_read_string(struct device *dev, enum hwmon_sensor_types type,
+			     u32 attr, int channel, const char **str)
+{
+	struct tmp421_data *data = dev_get_drvdata(dev);
+
+	*str = data->channel[channel].label;
+
+	return 0;
+}
+
 static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
 				 u32 attr, int channel)
 {
@@ -194,6 +205,8 @@ static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
 	case hwmon_temp_fault:
 	case hwmon_temp_input:
 		return 0444;
+	case hwmon_temp_label:
+		return 0444;
 	default:
 		return 0;
 	}
@@ -286,9 +299,53 @@ static int tmp421_detect(struct i2c_client *client,
 	return 0;
 }
 
+static int tmp421_probe_child_from_dt(struct i2c_client *client,
+				      struct device_node *child,
+				      struct tmp421_data *data)
+
+{
+	struct device *dev = &client->dev;
+	u32 i;
+	int err;
+
+	err = of_property_read_u32(child, "reg", &i);
+	if (err) {
+		dev_err(dev, "missing reg property of %pOFn\n", child);
+		return err;
+	}
+
+	if (i >= MAX_CHANNELS) {
+		dev_err(dev, "invalid reg %d of %pOFn\n", i, child);
+		return -EINVAL;
+	}
+
+	of_property_read_string(child, "label", &data->channel[i].label);
+	if (data->channel[i].label)
+		data->temp_config[i] |= HWMON_T_LABEL;
+
+	return 0;
+}
+
+static int tmp421_probe_from_dt(struct i2c_client *client, struct tmp421_data *data)
+{
+	struct device *dev = &client->dev;
+	const struct device_node *np = dev->of_node;
+	struct device_node *child;
+	int err;
+
+	for_each_child_of_node(np, child) {
+		err = tmp421_probe_child_from_dt(client, child, data);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 static const struct hwmon_ops tmp421_ops = {
 	.is_visible = tmp421_is_visible,
 	.read = tmp421_read,
+	.read_string = tmp421_read_string,
 };
 
 static int tmp421_probe(struct i2c_client *client)
@@ -317,6 +374,10 @@ static int tmp421_probe(struct i2c_client *client)
 	for (i = 0; i < data->channels; i++)
 		data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
 
+	err = tmp421_probe_from_dt(client, data);
+	if (err)
+		return err;
+
 	data->chip.ops = &tmp421_ops;
 	data->chip.info = data->info;
 
-- 
2.31.1


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

* [PATCH v4 04/10] hwmon: (tmp421) support disabling channels from DT
  2021-10-12  9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
                   ` (2 preceding siblings ...)
  2021-10-12  9:27 ` [PATCH v4 03/10] hwmon: (tmp421) add support for defining labels from DT Krzysztof Adamski
@ 2021-10-12  9:27 ` Krzysztof Adamski
  2021-10-12 14:32   ` Guenter Roeck
  2021-10-12  9:28 ` [PATCH v4 05/10] hwmon: (tmp421) support specifying n-factor via DT Krzysztof Adamski
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Krzysztof Adamski @ 2021-10-12  9:27 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

The previous patch introduced per channel subnodes in DT that let us
specify some channel specific properties. This built a ground for easily
disabling individual channels of the sensor that may not be connected to
any external diode and thus are not returning any meaningful data.

This patch adds support for parsing the "status" property of channels DT
subnodes and makes sure the -ENODATA is returned when disabled channels
value is read.

Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
 drivers/hwmon/tmp421.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index ab64d9825ca4..fffffd671e34 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -89,6 +89,7 @@ MODULE_DEVICE_TABLE(of, tmp421_of_match);
 
 struct tmp421_channel {
 	const char *label;
+	bool enabled;
 	s16 temp;
 };
 
@@ -170,6 +171,9 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
 	if (ret)
 		return ret;
 
+	if (!tmp421->channel[channel].enabled)
+		return -ENODATA;
+
 	switch (attr) {
 	case hwmon_temp_input:
 		*val = temp_from_raw(tmp421->channel[channel].temp,
@@ -323,6 +327,8 @@ static int tmp421_probe_child_from_dt(struct i2c_client *client,
 	if (data->channel[i].label)
 		data->temp_config[i] |= HWMON_T_LABEL;
 
+	data->channel[i].enabled = of_device_is_available(child);
+
 	return 0;
 }
 
@@ -371,8 +377,10 @@ static int tmp421_probe(struct i2c_client *client)
 	if (err)
 		return err;
 
-	for (i = 0; i < data->channels; i++)
+	for (i = 0; i < data->channels; i++) {
 		data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
+		data->channel[i].enabled = true;
+	}
 
 	err = tmp421_probe_from_dt(client, data);
 	if (err)
-- 
2.31.1


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

* [PATCH v4 05/10] hwmon: (tmp421) support specifying n-factor via DT
  2021-10-12  9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
                   ` (3 preceding siblings ...)
  2021-10-12  9:27 ` [PATCH v4 04/10] hwmon: (tmp421) support disabling channels " Krzysztof Adamski
@ 2021-10-12  9:28 ` Krzysztof Adamski
  2021-10-12 14:34   ` Guenter Roeck
  2021-10-12  9:29 ` [PATCH v4 06/10] hwmon: (tmp421) really disable channels Krzysztof Adamski
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Krzysztof Adamski @ 2021-10-12  9:28 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

Previous patches added a way to specify some channel specific parameters
in DT and n-factor is definitely one of them. This calibration mechanism
is board specific as its value depends on the diodes/transistors being
connected to the sensor and thus the DT seems like a right fit for that
information. It is very similar to the value of shunt resistor that some
drivers allows specifying in DT.

This patch adds a possibility to set n-factor for each channel via
"n-factor" DT property in each channel subnode.

Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
 drivers/hwmon/tmp421.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index fffffd671e34..fcd2932a6ddb 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -34,6 +34,7 @@ enum chips { tmp421, tmp422, tmp423, tmp441, tmp442 };
 #define TMP421_STATUS_REG			0x08
 #define TMP421_CONFIG_REG_1			0x09
 #define TMP421_CONVERSION_RATE_REG		0x0B
+#define TMP421_N_FACTOR_REG_1			0x21
 #define TMP421_MANUFACTURER_ID_REG		0xFE
 #define TMP421_DEVICE_ID_REG			0xFF
 
@@ -310,6 +311,7 @@ static int tmp421_probe_child_from_dt(struct i2c_client *client,
 {
 	struct device *dev = &client->dev;
 	u32 i;
+	s32 val;
 	int err;
 
 	err = of_property_read_u32(child, "reg", &i);
@@ -329,6 +331,20 @@ static int tmp421_probe_child_from_dt(struct i2c_client *client,
 
 	data->channel[i].enabled = of_device_is_available(child);
 
+	if (i == 0)
+		return 0; /* input 0 is internal channel */
+
+	err = of_property_read_s32(child, "ti,n-factor", &val);
+	if (!err) {
+		if (val > 127 || val < -128) {
+			dev_err(dev, "n-factor for channel %d invalid (%d)\n",
+				i, val);
+			return -EINVAL;
+		}
+		i2c_smbus_write_byte_data(client, TMP421_N_FACTOR_REG_1 + i - 1,
+						  val);
+	}
+
 	return 0;
 }
 
-- 
2.31.1


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

* [PATCH v4 06/10] hwmon: (tmp421) really disable channels
  2021-10-12  9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
                   ` (4 preceding siblings ...)
  2021-10-12  9:28 ` [PATCH v4 05/10] hwmon: (tmp421) support specifying n-factor via DT Krzysztof Adamski
@ 2021-10-12  9:29 ` Krzysztof Adamski
  2021-10-12 14:37   ` Guenter Roeck
                     ` (2 more replies)
  2021-10-12  9:29 ` [PATCH v4 07/10] hwmon: (tmp421) support HWMON_T_ENABLE Krzysztof Adamski
                   ` (3 subsequent siblings)
  9 siblings, 3 replies; 26+ messages in thread
From: Krzysztof Adamski @ 2021-10-12  9:29 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

Recent patch added possibility to disable selected channels. That would
only make sure that the ENODATA is returned for those channels but would
not configure the actual hardware.

With this patch, the config register is written to make sure the
channels are disabled also at hardware level.

Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
 drivers/hwmon/tmp421.c | 41 +++++++++++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index fcd2932a6ddb..45cb197cd277 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -33,6 +33,9 @@ enum chips { tmp421, tmp422, tmp423, tmp441, tmp442 };
 /* The TMP421 registers */
 #define TMP421_STATUS_REG			0x08
 #define TMP421_CONFIG_REG_1			0x09
+#define TMP421_CONFIG_REG_2			0x0A
+#define TMP421_CONFIG_REG_REN(x)		(BIT(3 + (x)))
+#define TMP421_CONFIG_REG_REN_MASK		GENMASK(6, 3)
 #define TMP421_CONVERSION_RATE_REG		0x0B
 #define TMP421_N_FACTOR_REG_1			0x21
 #define TMP421_MANUFACTURER_ID_REG		0xFE
@@ -162,6 +165,31 @@ static int tmp421_update_device(struct tmp421_data *data)
 	return 0;
 }
 
+static int tmp421_enable_channels(struct tmp421_data *data)
+{
+	int err;
+	struct i2c_client *client = data->client;
+	struct device *dev = &client->dev;
+	int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
+	int i;
+
+	if (cfg < 0) {
+		dev_err(dev, "error reading register, can't disable channels\n");
+		return err;
+	}
+
+	cfg &= ~TMP421_CONFIG_REG_REN_MASK;
+	for (i = 0; i < data->channels; i++)
+		if (data->channel[i].enabled)
+			cfg |= TMP421_CONFIG_REG_REN(i);
+
+	err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg);
+	if (err < 0)
+		dev_err(dev, "error writing register, can't disable channels\n");
+
+	return err;
+}
+
 static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
 		       u32 attr, int channel, long *val)
 {
@@ -217,9 +245,10 @@ static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
 	}
 }
 
-static int tmp421_init_client(struct i2c_client *client)
+static int tmp421_init_client(struct tmp421_data *data)
 {
 	int config, config_orig;
+	struct i2c_client *client = data->client;
 
 	/* Set the conversion rate to 2 Hz */
 	i2c_smbus_write_byte_data(client, TMP421_CONVERSION_RATE_REG, 0x05);
@@ -240,7 +269,7 @@ static int tmp421_init_client(struct i2c_client *client)
 		i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_1, config);
 	}
 
-	return 0;
+	return tmp421_enable_channels(data);
 }
 
 static int tmp421_detect(struct i2c_client *client,
@@ -389,10 +418,6 @@ static int tmp421_probe(struct i2c_client *client)
 		data->channels = i2c_match_id(tmp421_id, client)->driver_data;
 	data->client = client;
 
-	err = tmp421_init_client(client);
-	if (err)
-		return err;
-
 	for (i = 0; i < data->channels; i++) {
 		data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
 		data->channel[i].enabled = true;
@@ -402,6 +427,10 @@ static int tmp421_probe(struct i2c_client *client)
 	if (err)
 		return err;
 
+	err = tmp421_init_client(data);
+	if (err)
+		return err;
+
 	data->chip.ops = &tmp421_ops;
 	data->chip.info = data->info;
 
-- 
2.31.1


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

* [PATCH v4 07/10] hwmon: (tmp421) support HWMON_T_ENABLE
  2021-10-12  9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
                   ` (5 preceding siblings ...)
  2021-10-12  9:29 ` [PATCH v4 06/10] hwmon: (tmp421) really disable channels Krzysztof Adamski
@ 2021-10-12  9:29 ` Krzysztof Adamski
  2021-10-12 14:43   ` Guenter Roeck
  2021-10-12  9:30 ` [PATCH v4 08/10] hwmon: (tmp421) update documentation Krzysztof Adamski
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Krzysztof Adamski @ 2021-10-12  9:29 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

Since the recent patches added possibility of disabling sensor channels
via DT, it only make sense to allow controlling that from userspace via
HWMON_T_ENABLE mechanism. This patches adds support for that.

Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
 drivers/hwmon/tmp421.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 45cb197cd277..133eca1f2650 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -200,21 +200,27 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
 	if (ret)
 		return ret;
 
-	if (!tmp421->channel[channel].enabled)
-		return -ENODATA;
-
 	switch (attr) {
 	case hwmon_temp_input:
+		if (!tmp421->channel[channel].enabled)
+			return -ENODATA;
+
 		*val = temp_from_raw(tmp421->channel[channel].temp,
 				     tmp421->config & TMP421_CONFIG_RANGE);
+
 		return 0;
 	case hwmon_temp_fault:
+		if (!tmp421->channel[channel].enabled)
+			return -ENODATA;
 		/*
 		 * Any of OPEN or /PVLD bits indicate a hardware mulfunction
 		 * and the conversion result may be incorrect
 		 */
 		*val = !!(tmp421->channel[channel].temp & 0x03);
 		return 0;
+	case hwmon_temp_enable:
+		*val = tmp421->channel[channel].enabled;
+		return 0;
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -231,6 +237,24 @@ static int tmp421_read_string(struct device *dev, enum hwmon_sensor_types type,
 	return 0;
 }
 
+static int tmp421_write(struct device *dev, enum hwmon_sensor_types type,
+			u32 attr, int channel, long val)
+{
+	struct tmp421_data *data = dev_get_drvdata(dev);
+	int ret;
+
+	switch (type) {
+	case hwmon_temp:
+		data->channel[channel].enabled = val;
+		ret = tmp421_enable_channels(data);
+		break;
+	default:
+	    ret = -EOPNOTSUPP;
+	}
+
+	return ret;
+}
+
 static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
 				 u32 attr, int channel)
 {
@@ -240,6 +264,8 @@ static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
 		return 0444;
 	case hwmon_temp_label:
 		return 0444;
+	case hwmon_temp_enable:
+		return 0644;
 	default:
 		return 0;
 	}
@@ -397,6 +423,7 @@ static const struct hwmon_ops tmp421_ops = {
 	.is_visible = tmp421_is_visible,
 	.read = tmp421_read,
 	.read_string = tmp421_read_string,
+	.write = tmp421_write,
 };
 
 static int tmp421_probe(struct i2c_client *client)
@@ -419,7 +446,7 @@ static int tmp421_probe(struct i2c_client *client)
 	data->client = client;
 
 	for (i = 0; i < data->channels; i++) {
-		data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
+		data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_ENABLE;
 		data->channel[i].enabled = true;
 	}
 
-- 
2.31.1


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

* [PATCH v4 08/10] hwmon: (tmp421) update documentation
  2021-10-12  9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
                   ` (6 preceding siblings ...)
  2021-10-12  9:29 ` [PATCH v4 07/10] hwmon: (tmp421) support HWMON_T_ENABLE Krzysztof Adamski
@ 2021-10-12  9:30 ` Krzysztof Adamski
  2021-10-12 14:43   ` Guenter Roeck
  2021-10-12  9:30 ` [PATCH v4 09/10] hwmon: (tmp421) ignore non-channel related DT nodes Krzysztof Adamski
  2021-10-12  9:30 ` [PATCH v4 10/10] dt-bindings: hwmon: allow specifying channels for tmp421 Krzysztof Adamski
  9 siblings, 1 reply; 26+ messages in thread
From: Krzysztof Adamski @ 2021-10-12  9:30 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

Sysfs interface of the tmp421 driver was extended in the recent patches
so lets update the documentation to reflect that.

Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
 Documentation/hwmon/tmp421.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/hwmon/tmp421.rst b/Documentation/hwmon/tmp421.rst
index ddcd5159c75d..a3002117bbd7 100644
--- a/Documentation/hwmon/tmp421.rst
+++ b/Documentation/hwmon/tmp421.rst
@@ -64,3 +64,13 @@ the temperature values via the following sysfs files:
 **temp[1-4]_input**
 
 **temp[2-4]_fault**
+
+Each sensor can be individually disabled via Devicetree or from sysfs
+via:
+
+**temp[1-4]_enable**
+
+If labels were specified in Devicetree, additional sysfs files will
+be present:
+
+**temp[1-4]_label**
-- 
2.31.1


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

* [PATCH v4 09/10] hwmon: (tmp421) ignore non-channel related DT nodes
  2021-10-12  9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
                   ` (7 preceding siblings ...)
  2021-10-12  9:30 ` [PATCH v4 08/10] hwmon: (tmp421) update documentation Krzysztof Adamski
@ 2021-10-12  9:30 ` Krzysztof Adamski
  2021-10-12 14:44   ` Guenter Roeck
  2021-10-12  9:30 ` [PATCH v4 10/10] dt-bindings: hwmon: allow specifying channels for tmp421 Krzysztof Adamski
  9 siblings, 1 reply; 26+ messages in thread
From: Krzysztof Adamski @ 2021-10-12  9:30 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

In case the DT contains some nodes not describing the input channels,
ignore them instead of exiting with error.

Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
 drivers/hwmon/tmp421.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 133eca1f2650..d44112fe2a14 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -411,6 +411,9 @@ static int tmp421_probe_from_dt(struct i2c_client *client, struct tmp421_data *d
 	int err;
 
 	for_each_child_of_node(np, child) {
+		if (strcmp(child->name, "channel"))
+			continue;
+
 		err = tmp421_probe_child_from_dt(client, child, data);
 		if (err)
 			return err;
-- 
2.31.1


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

* [PATCH v4 10/10] dt-bindings: hwmon: allow specifying channels for tmp421
  2021-10-12  9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
                   ` (8 preceding siblings ...)
  2021-10-12  9:30 ` [PATCH v4 09/10] hwmon: (tmp421) ignore non-channel related DT nodes Krzysztof Adamski
@ 2021-10-12  9:30 ` Krzysztof Adamski
  9 siblings, 0 replies; 26+ messages in thread
From: Krzysztof Adamski @ 2021-10-12  9:30 UTC (permalink / raw)
  To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

Add binding description for the per temperature channel configuration
like labels and n-factor.

Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
 .../devicetree/bindings/hwmon/ti,tmp421.yaml  | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml b/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml
index 47040ace4f73..36f649938fb7 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml
@@ -24,12 +24,50 @@ properties:
   reg:
     maxItems: 1
 
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
 required:
   - compatible
   - reg
 
 additionalProperties: false
 
+patternProperties:
+  "^channel@([0-3])$":
+    type: object
+    description: |
+      Represents channels of the device and their specific configuration.
+
+    properties:
+      reg:
+        description: |
+          The channel number. 0 is local channel, 1-3 are remote channels
+        items:
+          minimum: 0
+          maximum: 3
+
+      label:
+        description: |
+          A descriptive name for this channel, like "ambient" or "psu".
+
+      ti,n-factor:
+        description: |
+          The value (two's complement) to be programmed in the channel specific N correction register.
+          For remote channels only.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        items:
+          minimum: 0
+          maximum: 255
+
+    required:
+      - reg
+
+    additionalProperties: false
+
 examples:
   - |
     i2c {
@@ -41,3 +79,32 @@ examples:
         reg = <0x4c>;
       };
     };
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      sensor@4c {
+        compatible = "ti,tmp422";
+        reg = <0x4c>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        channel@0 {
+          reg = <0x0>;
+          ti,n-factor = <0x1>;
+          label = "local";
+        };
+
+        channel@1 {
+          reg = <0x1>;
+          ti,n-factor = <0x0>;
+          label = "somelabel";
+        };
+
+        channel@2 {
+          reg = <0x2>;
+          status = "disabled";
+        };
+      };
+    };
-- 
2.31.1


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

* Re: [PATCH v4 02/10] hwmon: (tmp421) introduce a channel struct
  2021-10-12  9:26 ` [PATCH v4 02/10] hwmon: (tmp421) introduce a channel struct Krzysztof Adamski
@ 2021-10-12 14:25   ` Guenter Roeck
  0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:25 UTC (permalink / raw)
  To: Krzysztof Adamski; +Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree

On Tue, Oct 12, 2021 at 11:26:55AM +0200, Krzysztof Adamski wrote:
> This is a preparatory change. Upcoming patches will introduce more
> per-channel parameters so it's worth organizing them into a struct.
> 
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>

Applied to hwmon-next.

Thanks,
Guenter

> ---
>  drivers/hwmon/tmp421.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 356c96c3588b..707310d616a4 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -87,6 +87,10 @@ static const struct of_device_id __maybe_unused tmp421_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, tmp421_of_match);
>  
> +struct tmp421_channel {
> +	s16 temp;
> +};
> +
>  struct tmp421_data {
>  	struct i2c_client *client;
>  	struct mutex update_lock;
> @@ -98,7 +102,7 @@ struct tmp421_data {
>  	unsigned long last_updated;
>  	unsigned long channels;
>  	u8 config;
> -	s16 temp[MAX_CHANNELS];
> +	struct tmp421_channel channel[MAX_CHANNELS];
>  };
>  
>  static int temp_from_raw(u16 reg, bool extended)
> @@ -133,12 +137,12 @@ static int tmp421_update_device(struct tmp421_data *data)
>  			ret = i2c_smbus_read_byte_data(client, TMP421_TEMP_MSB[i]);
>  			if (ret < 0)
>  				goto exit;
> -			data->temp[i] = ret << 8;
> +			data->channel[i].temp = ret << 8;
>  
>  			ret = i2c_smbus_read_byte_data(client, TMP421_TEMP_LSB[i]);
>  			if (ret < 0)
>  				goto exit;
> -			data->temp[i] |= ret;
> +			data->channel[i].temp |= ret;
>  		}
>  		data->last_updated = jiffies;
>  		data->valid = true;
> @@ -167,7 +171,7 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
>  
>  	switch (attr) {
>  	case hwmon_temp_input:
> -		*val = temp_from_raw(tmp421->temp[channel],
> +		*val = temp_from_raw(tmp421->channel[channel].temp,
>  				     tmp421->config & TMP421_CONFIG_RANGE);
>  		return 0;
>  	case hwmon_temp_fault:
> @@ -175,7 +179,7 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
>  		 * Any of OPEN or /PVLD bits indicate a hardware mulfunction
>  		 * and the conversion result may be incorrect
>  		 */
> -		*val = !!(tmp421->temp[channel] & 0x03);
> +		*val = !!(tmp421->channel[channel].temp & 0x03);
>  		return 0;
>  	default:
>  		return -EOPNOTSUPP;

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

* Re: [PATCH v4 03/10] hwmon: (tmp421) add support for defining labels from DT
  2021-10-12  9:27 ` [PATCH v4 03/10] hwmon: (tmp421) add support for defining labels from DT Krzysztof Adamski
@ 2021-10-12 14:30   ` Guenter Roeck
  0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:30 UTC (permalink / raw)
  To: Krzysztof Adamski; +Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree

On Tue, Oct 12, 2021 at 11:27:14AM +0200, Krzysztof Adamski wrote:
> tmp42x is a multichannel temperature sensor with several external
> channels. Since those channels can be used to connect diodes placed
> anywhere in the system, their meaning will vary depending on the
> project. For this case, the hwmon framework has an idea of labels which
> allows us to assign the meaning to each channel.
> 
> The similar concept is already implemented in ina3221 - the label for
> each channel can be defined via device tree. See commit a9e9dd9c6de5
> ("hwmon: (ina3221) Read channel input source info from DT")
> 
> This patch adds support for similar feature to tmp421.
> 
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
> ---
>  drivers/hwmon/tmp421.c | 61 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 707310d616a4..ab64d9825ca4 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -88,6 +88,7 @@ static const struct of_device_id __maybe_unused tmp421_of_match[] = {
>  MODULE_DEVICE_TABLE(of, tmp421_of_match);
>  
>  struct tmp421_channel {
> +	const char *label;
>  	s16 temp;
>  };
>  
> @@ -187,6 +188,16 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
>  
>  }
>  
> +static int tmp421_read_string(struct device *dev, enum hwmon_sensor_types type,
> +			     u32 attr, int channel, const char **str)
> +{
> +	struct tmp421_data *data = dev_get_drvdata(dev);
> +
> +	*str = data->channel[channel].label;
> +
> +	return 0;
> +}
> +
>  static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
>  				 u32 attr, int channel)
>  {
> @@ -194,6 +205,8 @@ static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
>  	case hwmon_temp_fault:
>  	case hwmon_temp_input:
>  		return 0444;
> +	case hwmon_temp_label:
> +		return 0444;
>  	default:
>  		return 0;
>  	}
> @@ -286,9 +299,53 @@ static int tmp421_detect(struct i2c_client *client,
>  	return 0;
>  }
>  
> +static int tmp421_probe_child_from_dt(struct i2c_client *client,
> +				      struct device_node *child,
> +				      struct tmp421_data *data)
> +
> +{
> +	struct device *dev = &client->dev;
> +	u32 i;
> +	int err;
> +
> +	err = of_property_read_u32(child, "reg", &i);
> +	if (err) {
> +		dev_err(dev, "missing reg property of %pOFn\n", child);
> +		return err;
> +	}
> +
> +	if (i >= MAX_CHANNELS) {

I think this needs to compare against data->channels. Otherwise, if
unsupported channels are listed in dt (say, reg==0x2 with tmp421),
we would end up with label attributes for those channels.

> +		dev_err(dev, "invalid reg %d of %pOFn\n", i, child);
> +		return -EINVAL;
> +	}
> +
> +	of_property_read_string(child, "label", &data->channel[i].label);
> +	if (data->channel[i].label)
> +		data->temp_config[i] |= HWMON_T_LABEL;
> +
> +	return 0;
> +}
> +
> +static int tmp421_probe_from_dt(struct i2c_client *client, struct tmp421_data *data)
> +{
> +	struct device *dev = &client->dev;
> +	const struct device_node *np = dev->of_node;
> +	struct device_node *child;
> +	int err;
> +
> +	for_each_child_of_node(np, child) {
> +		err = tmp421_probe_child_from_dt(client, child, data);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct hwmon_ops tmp421_ops = {
>  	.is_visible = tmp421_is_visible,
>  	.read = tmp421_read,
> +	.read_string = tmp421_read_string,
>  };
>  
>  static int tmp421_probe(struct i2c_client *client)
> @@ -317,6 +374,10 @@ static int tmp421_probe(struct i2c_client *client)
>  	for (i = 0; i < data->channels; i++)
>  		data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
>  
> +	err = tmp421_probe_from_dt(client, data);
> +	if (err)
> +		return err;
> +
>  	data->chip.ops = &tmp421_ops;
>  	data->chip.info = data->info;
>  

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

* Re: [PATCH v4 04/10] hwmon: (tmp421) support disabling channels from DT
  2021-10-12  9:27 ` [PATCH v4 04/10] hwmon: (tmp421) support disabling channels " Krzysztof Adamski
@ 2021-10-12 14:32   ` Guenter Roeck
  0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:32 UTC (permalink / raw)
  To: Krzysztof Adamski, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

On 10/12/21 2:27 AM, Krzysztof Adamski wrote:
> The previous patch introduced per channel subnodes in DT that let us
> specify some channel specific properties. This built a ground for easily
> disabling individual channels of the sensor that may not be connected to
> any external diode and thus are not returning any meaningful data.
> 
> This patch adds support for parsing the "status" property of channels DT
> subnodes and makes sure the -ENODATA is returned when disabled channels
> value is read.
> 
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>

For my reference:

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/hwmon/tmp421.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index ab64d9825ca4..fffffd671e34 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -89,6 +89,7 @@ MODULE_DEVICE_TABLE(of, tmp421_of_match);
>   
>   struct tmp421_channel {
>   	const char *label;
> +	bool enabled;
>   	s16 temp;
>   };
>   
> @@ -170,6 +171,9 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
>   	if (ret)
>   		return ret;
>   
> +	if (!tmp421->channel[channel].enabled)
> +		return -ENODATA;
> +
>   	switch (attr) {
>   	case hwmon_temp_input:
>   		*val = temp_from_raw(tmp421->channel[channel].temp,
> @@ -323,6 +327,8 @@ static int tmp421_probe_child_from_dt(struct i2c_client *client,
>   	if (data->channel[i].label)
>   		data->temp_config[i] |= HWMON_T_LABEL;
>   
> +	data->channel[i].enabled = of_device_is_available(child);
> +
>   	return 0;
>   }
>   
> @@ -371,8 +377,10 @@ static int tmp421_probe(struct i2c_client *client)
>   	if (err)
>   		return err;
>   
> -	for (i = 0; i < data->channels; i++)
> +	for (i = 0; i < data->channels; i++) {
>   		data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
> +		data->channel[i].enabled = true;
> +	}
>   
>   	err = tmp421_probe_from_dt(client, data);
>   	if (err)
> 


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

* Re: [PATCH v4 05/10] hwmon: (tmp421) support specifying n-factor via DT
  2021-10-12  9:28 ` [PATCH v4 05/10] hwmon: (tmp421) support specifying n-factor via DT Krzysztof Adamski
@ 2021-10-12 14:34   ` Guenter Roeck
  0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:34 UTC (permalink / raw)
  To: Krzysztof Adamski, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

On 10/12/21 2:28 AM, Krzysztof Adamski wrote:
> Previous patches added a way to specify some channel specific parameters
> in DT and n-factor is definitely one of them. This calibration mechanism
> is board specific as its value depends on the diodes/transistors being
> connected to the sensor and thus the DT seems like a right fit for that
> information. It is very similar to the value of shunt resistor that some
> drivers allows specifying in DT.
> 
> This patch adds a possibility to set n-factor for each channel via
> "n-factor" DT property in each channel subnode.
> 
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
> ---
>   drivers/hwmon/tmp421.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index fffffd671e34..fcd2932a6ddb 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -34,6 +34,7 @@ enum chips { tmp421, tmp422, tmp423, tmp441, tmp442 };
>   #define TMP421_STATUS_REG			0x08
>   #define TMP421_CONFIG_REG_1			0x09
>   #define TMP421_CONVERSION_RATE_REG		0x0B
> +#define TMP421_N_FACTOR_REG_1			0x21
>   #define TMP421_MANUFACTURER_ID_REG		0xFE
>   #define TMP421_DEVICE_ID_REG			0xFF
>   
> @@ -310,6 +311,7 @@ static int tmp421_probe_child_from_dt(struct i2c_client *client,
>   {
>   	struct device *dev = &client->dev;
>   	u32 i;
> +	s32 val;
>   	int err;
>   
>   	err = of_property_read_u32(child, "reg", &i);
> @@ -329,6 +331,20 @@ static int tmp421_probe_child_from_dt(struct i2c_client *client,
>   
>   	data->channel[i].enabled = of_device_is_available(child);
>   
> +	if (i == 0)
> +		return 0; /* input 0 is internal channel */
> +

I think that should generate an error, ie

> +	err = of_property_read_s32(child, "ti,n-factor", &val);
> +	if (!err) {

		if (!i)
			return -EINVAL;

> +		if (val > 127 || val < -128) {
> +			dev_err(dev, "n-factor for channel %d invalid (%d)\n",
> +				i, val);
> +			return -EINVAL;
> +		}
> +		i2c_smbus_write_byte_data(client, TMP421_N_FACTOR_REG_1 + i - 1,
> +						  val);
> +	}
> +
>   	return 0;
>   }
>   
> 


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

* Re: [PATCH v4 06/10] hwmon: (tmp421) really disable channels
  2021-10-12  9:29 ` [PATCH v4 06/10] hwmon: (tmp421) really disable channels Krzysztof Adamski
@ 2021-10-12 14:37   ` Guenter Roeck
  2021-10-12 18:01     ` kernel test robot
  2021-10-13 18:50     ` kernel test robot
  2 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:37 UTC (permalink / raw)
  To: Krzysztof Adamski, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

On 10/12/21 2:29 AM, Krzysztof Adamski wrote:
> Recent patch added possibility to disable selected channels. That would
> only make sure that the ENODATA is returned for those channels but would
> not configure the actual hardware.
> 
> With this patch, the config register is written to make sure the
> channels are disabled also at hardware level.
> 
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
> ---
>   drivers/hwmon/tmp421.c | 41 +++++++++++++++++++++++++++++++++++------
>   1 file changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index fcd2932a6ddb..45cb197cd277 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -33,6 +33,9 @@ enum chips { tmp421, tmp422, tmp423, tmp441, tmp442 };
>   /* The TMP421 registers */
>   #define TMP421_STATUS_REG			0x08
>   #define TMP421_CONFIG_REG_1			0x09
> +#define TMP421_CONFIG_REG_2			0x0A
> +#define TMP421_CONFIG_REG_REN(x)		(BIT(3 + (x)))
> +#define TMP421_CONFIG_REG_REN_MASK		GENMASK(6, 3)
>   #define TMP421_CONVERSION_RATE_REG		0x0B
>   #define TMP421_N_FACTOR_REG_1			0x21
>   #define TMP421_MANUFACTURER_ID_REG		0xFE
> @@ -162,6 +165,31 @@ static int tmp421_update_device(struct tmp421_data *data)
>   	return 0;
>   }
>   
> +static int tmp421_enable_channels(struct tmp421_data *data)
> +{
> +	int err;
> +	struct i2c_client *client = data->client;
> +	struct device *dev = &client->dev;
> +	int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
> +	int i;
> +
> +	if (cfg < 0) {
> +		dev_err(dev, "error reading register, can't disable channels\n");
> +		return err;

		return cfg;

> +	}
> +
> +	cfg &= ~TMP421_CONFIG_REG_REN_MASK;
> +	for (i = 0; i < data->channels; i++)
> +		if (data->channel[i].enabled)
> +			cfg |= TMP421_CONFIG_REG_REN(i);
> +
> +	err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg);

Please make the write optional: It is only necessary if the old register
value differs from the new register value.

> +	if (err < 0)
> +		dev_err(dev, "error writing register, can't disable channels\n");
> +
> +	return err;
> +}
> +
>   static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
>   		       u32 attr, int channel, long *val)
>   {
> @@ -217,9 +245,10 @@ static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
>   	}
>   }
>   
> -static int tmp421_init_client(struct i2c_client *client)
> +static int tmp421_init_client(struct tmp421_data *data)
>   {
>   	int config, config_orig;
> +	struct i2c_client *client = data->client;
>   
>   	/* Set the conversion rate to 2 Hz */
>   	i2c_smbus_write_byte_data(client, TMP421_CONVERSION_RATE_REG, 0x05);
> @@ -240,7 +269,7 @@ static int tmp421_init_client(struct i2c_client *client)
>   		i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_1, config);
>   	}
>   
> -	return 0;
> +	return tmp421_enable_channels(data);
>   }
>   
>   static int tmp421_detect(struct i2c_client *client,
> @@ -389,10 +418,6 @@ static int tmp421_probe(struct i2c_client *client)
>   		data->channels = i2c_match_id(tmp421_id, client)->driver_data;
>   	data->client = client;
>   
> -	err = tmp421_init_client(client);
> -	if (err)
> -		return err;
> -
>   	for (i = 0; i < data->channels; i++) {
>   		data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
>   		data->channel[i].enabled = true;
> @@ -402,6 +427,10 @@ static int tmp421_probe(struct i2c_client *client)
>   	if (err)
>   		return err;
>   
> +	err = tmp421_init_client(data);
> +	if (err)
> +		return err;
> +
>   	data->chip.ops = &tmp421_ops;
>   	data->chip.info = data->info;
>   
> 


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

* Re: [PATCH v4 07/10] hwmon: (tmp421) support HWMON_T_ENABLE
  2021-10-12  9:29 ` [PATCH v4 07/10] hwmon: (tmp421) support HWMON_T_ENABLE Krzysztof Adamski
@ 2021-10-12 14:43   ` Guenter Roeck
  0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:43 UTC (permalink / raw)
  To: Krzysztof Adamski, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

On 10/12/21 2:29 AM, Krzysztof Adamski wrote:
> Since the recent patches added possibility of disabling sensor channels
> via DT, it only make sense to allow controlling that from userspace via
> HWMON_T_ENABLE mechanism. This patches adds support for that.
> 
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
> ---
>   drivers/hwmon/tmp421.c | 35 +++++++++++++++++++++++++++++++----
>   1 file changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 45cb197cd277..133eca1f2650 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -200,21 +200,27 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
>   	if (ret)
>   		return ret;
>   
> -	if (!tmp421->channel[channel].enabled)
> -		return -ENODATA;
> -
>   	switch (attr) {
>   	case hwmon_temp_input:
> +		if (!tmp421->channel[channel].enabled)
> +			return -ENODATA;
> +
>   		*val = temp_from_raw(tmp421->channel[channel].temp,
>   				     tmp421->config & TMP421_CONFIG_RANGE);
> +

Please drop those empty lines for consistency with the code below.

>   		return 0;
>   	case hwmon_temp_fault:
> +		if (!tmp421->channel[channel].enabled)
> +			return -ENODATA;
>   		/*
>   		 * Any of OPEN or /PVLD bits indicate a hardware mulfunction
>   		 * and the conversion result may be incorrect
>   		 */
>   		*val = !!(tmp421->channel[channel].temp & 0x03);
>   		return 0;
> +	case hwmon_temp_enable:
> +		*val = tmp421->channel[channel].enabled;
> +		return 0;
>   	default:
>   		return -EOPNOTSUPP;
>   	}
> @@ -231,6 +237,24 @@ static int tmp421_read_string(struct device *dev, enum hwmon_sensor_types type,
>   	return 0;
>   }
>   
> +static int tmp421_write(struct device *dev, enum hwmon_sensor_types type,
> +			u32 attr, int channel, long val)
> +{
> +	struct tmp421_data *data = dev_get_drvdata(dev);
> +	int ret;
> +
> +	switch (type) {
> +	case hwmon_temp:

Please check for attr here. Checking the type (which is always
hwmon_temp) is misleading.

> +		data->channel[channel].enabled = val;
> +		ret = tmp421_enable_channels(data);
> +		break;
> +	default:
> +	    ret = -EOPNOTSUPP;
> +	}
> +
> +	return ret;
> +}
> +
>   static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
>   				 u32 attr, int channel)
>   {
> @@ -240,6 +264,8 @@ static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
>   		return 0444;
>   	case hwmon_temp_label:
>   		return 0444;
> +	case hwmon_temp_enable:
> +		return 0644;
>   	default:
>   		return 0;
>   	}
> @@ -397,6 +423,7 @@ static const struct hwmon_ops tmp421_ops = {
>   	.is_visible = tmp421_is_visible,
>   	.read = tmp421_read,
>   	.read_string = tmp421_read_string,
> +	.write = tmp421_write,
>   };
>   
>   static int tmp421_probe(struct i2c_client *client)
> @@ -419,7 +446,7 @@ static int tmp421_probe(struct i2c_client *client)
>   	data->client = client;
>   
>   	for (i = 0; i < data->channels; i++) {
> -		data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
> +		data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_ENABLE;
>   		data->channel[i].enabled = true;
>   	}
>   
> 


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

* Re: [PATCH v4 08/10] hwmon: (tmp421) update documentation
  2021-10-12  9:30 ` [PATCH v4 08/10] hwmon: (tmp421) update documentation Krzysztof Adamski
@ 2021-10-12 14:43   ` Guenter Roeck
  0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:43 UTC (permalink / raw)
  To: Krzysztof Adamski, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

On 10/12/21 2:30 AM, Krzysztof Adamski wrote:
> Sysfs interface of the tmp421 driver was extended in the recent patches
> so lets update the documentation to reflect that.
> 
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>

For my reference:

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   Documentation/hwmon/tmp421.rst | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/hwmon/tmp421.rst b/Documentation/hwmon/tmp421.rst
> index ddcd5159c75d..a3002117bbd7 100644
> --- a/Documentation/hwmon/tmp421.rst
> +++ b/Documentation/hwmon/tmp421.rst
> @@ -64,3 +64,13 @@ the temperature values via the following sysfs files:
>   **temp[1-4]_input**
>   
>   **temp[2-4]_fault**
> +
> +Each sensor can be individually disabled via Devicetree or from sysfs
> +via:
> +
> +**temp[1-4]_enable**
> +
> +If labels were specified in Devicetree, additional sysfs files will
> +be present:
> +
> +**temp[1-4]_label**
> 


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

* Re: [PATCH v4 09/10] hwmon: (tmp421) ignore non-channel related DT nodes
  2021-10-12  9:30 ` [PATCH v4 09/10] hwmon: (tmp421) ignore non-channel related DT nodes Krzysztof Adamski
@ 2021-10-12 14:44   ` Guenter Roeck
  0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:44 UTC (permalink / raw)
  To: Krzysztof Adamski, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree

On 10/12/21 2:30 AM, Krzysztof Adamski wrote:
> In case the DT contains some nodes not describing the input channels,
> ignore them instead of exiting with error.
> 
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>

For my reference:

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/hwmon/tmp421.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 133eca1f2650..d44112fe2a14 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -411,6 +411,9 @@ static int tmp421_probe_from_dt(struct i2c_client *client, struct tmp421_data *d
>   	int err;
>   
>   	for_each_child_of_node(np, child) {
> +		if (strcmp(child->name, "channel"))
> +			continue;
> +
>   		err = tmp421_probe_child_from_dt(client, child, data);
>   		if (err)
>   			return err;
> 


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

* Re: [PATCH v4 06/10] hwmon: (tmp421) really disable channels
  2021-10-12  9:29 ` [PATCH v4 06/10] hwmon: (tmp421) really disable channels Krzysztof Adamski
@ 2021-10-12 18:01     ` kernel test robot
  2021-10-12 18:01     ` kernel test robot
  2021-10-13 18:50     ` kernel test robot
  2 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2021-10-12 18:01 UTC (permalink / raw)
  To: Krzysztof Adamski, Guenter Roeck, Jean Delvare
  Cc: llvm, kbuild-all, Rob Herring, linux-hwmon, devicetree

[-- Attachment #1: Type: text/plain, Size: 3028 bytes --]

Hi Krzysztof,

I love your patch! Perhaps something to improve:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on next-20211012]
[cannot apply to robh/for-next v5.15-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: arm64-randconfig-r024-20211012 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c3dcf39554dbea780d6cb7e12239451ba47a2668)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/4a0f8262fe071b0b27c6fba7455627f3c5a5209e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
        git checkout 4a0f8262fe071b0b27c6fba7455627f3c5a5209e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/hwmon/tmp421.c:178:10: warning: variable 'err' is uninitialized when used here [-Wuninitialized]
                   return err;
                          ^~~
   drivers/hwmon/tmp421.c:170:9: note: initialize the variable 'err' to silence this warning
           int err;
                  ^
                   = 0
   1 warning generated.


vim +/err +178 drivers/hwmon/tmp421.c

   167	
   168	static int tmp421_enable_channels(struct tmp421_data *data)
   169	{
   170		int err;
   171		struct i2c_client *client = data->client;
   172		struct device *dev = &client->dev;
   173		int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
   174		int i;
   175	
   176		if (cfg < 0) {
   177			dev_err(dev, "error reading register, can't disable channels\n");
 > 178			return err;
   179		}
   180	
   181		cfg &= ~TMP421_CONFIG_REG_REN_MASK;
   182		for (i = 0; i < data->channels; i++)
   183			if (data->channel[i].enabled)
   184				cfg |= TMP421_CONFIG_REG_REN(i);
   185	
   186		err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg);
   187		if (err < 0)
   188			dev_err(dev, "error writing register, can't disable channels\n");
   189	
   190		return err;
   191	}
   192	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45131 bytes --]

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

* Re: [PATCH v4 06/10] hwmon: (tmp421) really disable channels
@ 2021-10-12 18:01     ` kernel test robot
  0 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2021-10-12 18:01 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3102 bytes --]

Hi Krzysztof,

I love your patch! Perhaps something to improve:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on next-20211012]
[cannot apply to robh/for-next v5.15-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: arm64-randconfig-r024-20211012 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c3dcf39554dbea780d6cb7e12239451ba47a2668)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/4a0f8262fe071b0b27c6fba7455627f3c5a5209e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
        git checkout 4a0f8262fe071b0b27c6fba7455627f3c5a5209e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/hwmon/tmp421.c:178:10: warning: variable 'err' is uninitialized when used here [-Wuninitialized]
                   return err;
                          ^~~
   drivers/hwmon/tmp421.c:170:9: note: initialize the variable 'err' to silence this warning
           int err;
                  ^
                   = 0
   1 warning generated.


vim +/err +178 drivers/hwmon/tmp421.c

   167	
   168	static int tmp421_enable_channels(struct tmp421_data *data)
   169	{
   170		int err;
   171		struct i2c_client *client = data->client;
   172		struct device *dev = &client->dev;
   173		int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
   174		int i;
   175	
   176		if (cfg < 0) {
   177			dev_err(dev, "error reading register, can't disable channels\n");
 > 178			return err;
   179		}
   180	
   181		cfg &= ~TMP421_CONFIG_REG_REN_MASK;
   182		for (i = 0; i < data->channels; i++)
   183			if (data->channel[i].enabled)
   184				cfg |= TMP421_CONFIG_REG_REN(i);
   185	
   186		err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg);
   187		if (err < 0)
   188			dev_err(dev, "error writing register, can't disable channels\n");
   189	
   190		return err;
   191	}
   192	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 45131 bytes --]

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

* Re: [PATCH v4 06/10] hwmon: (tmp421) really disable channels
  2021-10-12  9:29 ` [PATCH v4 06/10] hwmon: (tmp421) really disable channels Krzysztof Adamski
  2021-10-12 14:37   ` Guenter Roeck
@ 2021-10-13  8:36 ` Dan Carpenter
  2021-10-13 18:50     ` kernel test robot
  2 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2021-10-12 22:41 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 3667 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <eaee0947c813b04b77ee8364724d1a406dccc33b.1634029538.git.krzysztof.adamski@nokia.com>
References: <eaee0947c813b04b77ee8364724d1a406dccc33b.1634029538.git.krzysztof.adamski@nokia.com>
TO: Krzysztof Adamski <krzysztof.adamski@nokia.com>
TO: Guenter Roeck <linux@roeck-us.net>
TO: Jean Delvare <jdelvare@suse.com>
CC: Rob Herring <robh+dt@kernel.org>
CC: linux-hwmon(a)vger.kernel.org
CC: devicetree(a)vger.kernel.org

Hi Krzysztof,

I love your patch! Perhaps something to improve:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on next-20211012]
[cannot apply to robh/for-next v5.15-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
:::::: branch date: 13 hours ago
:::::: commit date: 13 hours ago
config: i386-randconfig-m031-20211012 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/hwmon/tmp421.c:178 tmp421_enable_channels() error: uninitialized symbol 'err'.

vim +/err +178 drivers/hwmon/tmp421.c

9410700b881f86 Andre Prendel     2009-09-15  167  
4a0f8262fe071b Krzysztof Adamski 2021-10-12  168  static int tmp421_enable_channels(struct tmp421_data *data)
4a0f8262fe071b Krzysztof Adamski 2021-10-12  169  {
4a0f8262fe071b Krzysztof Adamski 2021-10-12  170  	int err;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  171  	struct i2c_client *client = data->client;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  172  	struct device *dev = &client->dev;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  173  	int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
4a0f8262fe071b Krzysztof Adamski 2021-10-12  174  	int i;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  175  
4a0f8262fe071b Krzysztof Adamski 2021-10-12  176  	if (cfg < 0) {
4a0f8262fe071b Krzysztof Adamski 2021-10-12  177  		dev_err(dev, "error reading register, can't disable channels\n");
4a0f8262fe071b Krzysztof Adamski 2021-10-12 @178  		return err;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  179  	}
4a0f8262fe071b Krzysztof Adamski 2021-10-12  180  
4a0f8262fe071b Krzysztof Adamski 2021-10-12  181  	cfg &= ~TMP421_CONFIG_REG_REN_MASK;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  182  	for (i = 0; i < data->channels; i++)
4a0f8262fe071b Krzysztof Adamski 2021-10-12  183  		if (data->channel[i].enabled)
4a0f8262fe071b Krzysztof Adamski 2021-10-12  184  			cfg |= TMP421_CONFIG_REG_REN(i);
4a0f8262fe071b Krzysztof Adamski 2021-10-12  185  
4a0f8262fe071b Krzysztof Adamski 2021-10-12  186  	err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg);
4a0f8262fe071b Krzysztof Adamski 2021-10-12  187  	if (err < 0)
4a0f8262fe071b Krzysztof Adamski 2021-10-12  188  		dev_err(dev, "error writing register, can't disable channels\n");
4a0f8262fe071b Krzysztof Adamski 2021-10-12  189  
4a0f8262fe071b Krzysztof Adamski 2021-10-12  190  	return err;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  191  }
4a0f8262fe071b Krzysztof Adamski 2021-10-12  192  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 34043 bytes --]

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

* Re: [PATCH v4 06/10] hwmon: (tmp421) really disable channels
@ 2021-10-13  8:36 ` Dan Carpenter
  0 siblings, 0 replies; 26+ messages in thread
From: Dan Carpenter @ 2021-10-13  8:36 UTC (permalink / raw)
  To: kbuild, Krzysztof Adamski, Guenter Roeck, Jean Delvare
  Cc: lkp, kbuild-all, Rob Herring, linux-hwmon, devicetree

Hi Krzysztof,

url:    https://github.com/0day-ci/linux/commits/Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: i386-randconfig-m031-20211012 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/hwmon/tmp421.c:178 tmp421_enable_channels() error: uninitialized symbol 'err'.

vim +/err +178 drivers/hwmon/tmp421.c

4a0f8262fe071b Krzysztof Adamski 2021-10-12  168  static int tmp421_enable_channels(struct tmp421_data *data)
4a0f8262fe071b Krzysztof Adamski 2021-10-12  169  {
4a0f8262fe071b Krzysztof Adamski 2021-10-12  170  	int err;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  171  	struct i2c_client *client = data->client;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  172  	struct device *dev = &client->dev;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  173  	int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
4a0f8262fe071b Krzysztof Adamski 2021-10-12  174  	int i;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  175  
4a0f8262fe071b Krzysztof Adamski 2021-10-12  176  	if (cfg < 0) {
4a0f8262fe071b Krzysztof Adamski 2021-10-12  177  		dev_err(dev, "error reading register, can't disable channels\n");
4a0f8262fe071b Krzysztof Adamski 2021-10-12 @178  		return err;

"return cfg;"

4a0f8262fe071b Krzysztof Adamski 2021-10-12  179  	}
4a0f8262fe071b Krzysztof Adamski 2021-10-12  180  
4a0f8262fe071b Krzysztof Adamski 2021-10-12  181  	cfg &= ~TMP421_CONFIG_REG_REN_MASK;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  182  	for (i = 0; i < data->channels; i++)
4a0f8262fe071b Krzysztof Adamski 2021-10-12  183  		if (data->channel[i].enabled)
4a0f8262fe071b Krzysztof Adamski 2021-10-12  184  			cfg |= TMP421_CONFIG_REG_REN(i);
4a0f8262fe071b Krzysztof Adamski 2021-10-12  185  
4a0f8262fe071b Krzysztof Adamski 2021-10-12  186  	err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg);
4a0f8262fe071b Krzysztof Adamski 2021-10-12  187  	if (err < 0)
4a0f8262fe071b Krzysztof Adamski 2021-10-12  188  		dev_err(dev, "error writing register, can't disable channels\n");
4a0f8262fe071b Krzysztof Adamski 2021-10-12  189  
4a0f8262fe071b Krzysztof Adamski 2021-10-12  190  	return err;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  191  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org


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

* Re: [PATCH v4 06/10] hwmon: (tmp421) really disable channels
@ 2021-10-13  8:36 ` Dan Carpenter
  0 siblings, 0 replies; 26+ messages in thread
From: Dan Carpenter @ 2021-10-13  8:36 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2658 bytes --]

Hi Krzysztof,

url:    https://github.com/0day-ci/linux/commits/Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: i386-randconfig-m031-20211012 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/hwmon/tmp421.c:178 tmp421_enable_channels() error: uninitialized symbol 'err'.

vim +/err +178 drivers/hwmon/tmp421.c

4a0f8262fe071b Krzysztof Adamski 2021-10-12  168  static int tmp421_enable_channels(struct tmp421_data *data)
4a0f8262fe071b Krzysztof Adamski 2021-10-12  169  {
4a0f8262fe071b Krzysztof Adamski 2021-10-12  170  	int err;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  171  	struct i2c_client *client = data->client;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  172  	struct device *dev = &client->dev;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  173  	int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
4a0f8262fe071b Krzysztof Adamski 2021-10-12  174  	int i;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  175  
4a0f8262fe071b Krzysztof Adamski 2021-10-12  176  	if (cfg < 0) {
4a0f8262fe071b Krzysztof Adamski 2021-10-12  177  		dev_err(dev, "error reading register, can't disable channels\n");
4a0f8262fe071b Krzysztof Adamski 2021-10-12 @178  		return err;

"return cfg;"

4a0f8262fe071b Krzysztof Adamski 2021-10-12  179  	}
4a0f8262fe071b Krzysztof Adamski 2021-10-12  180  
4a0f8262fe071b Krzysztof Adamski 2021-10-12  181  	cfg &= ~TMP421_CONFIG_REG_REN_MASK;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  182  	for (i = 0; i < data->channels; i++)
4a0f8262fe071b Krzysztof Adamski 2021-10-12  183  		if (data->channel[i].enabled)
4a0f8262fe071b Krzysztof Adamski 2021-10-12  184  			cfg |= TMP421_CONFIG_REG_REN(i);
4a0f8262fe071b Krzysztof Adamski 2021-10-12  185  
4a0f8262fe071b Krzysztof Adamski 2021-10-12  186  	err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg);
4a0f8262fe071b Krzysztof Adamski 2021-10-12  187  	if (err < 0)
4a0f8262fe071b Krzysztof Adamski 2021-10-12  188  		dev_err(dev, "error writing register, can't disable channels\n");
4a0f8262fe071b Krzysztof Adamski 2021-10-12  189  
4a0f8262fe071b Krzysztof Adamski 2021-10-12  190  	return err;
4a0f8262fe071b Krzysztof Adamski 2021-10-12  191  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

* Re: [PATCH v4 06/10] hwmon: (tmp421) really disable channels
  2021-10-12  9:29 ` [PATCH v4 06/10] hwmon: (tmp421) really disable channels Krzysztof Adamski
@ 2021-10-13 18:50     ` kernel test robot
  2021-10-12 18:01     ` kernel test robot
  2021-10-13 18:50     ` kernel test robot
  2 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2021-10-13 18:50 UTC (permalink / raw)
  To: Krzysztof Adamski, Guenter Roeck, Jean Delvare
  Cc: llvm, kbuild-all, Rob Herring, linux-hwmon, devicetree

[-- Attachment #1: Type: text/plain, Size: 2916 bytes --]

Hi Krzysztof,

I love your patch! Yet something to improve:

[auto build test ERROR on groeck-staging/hwmon-next]
[also build test ERROR on next-20211013]
[cannot apply to robh/for-next v5.15-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: i386-buildonly-randconfig-r004-20211013 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project b6a8c695542b2987eb9a203d5663a0740cb4725f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/4a0f8262fe071b0b27c6fba7455627f3c5a5209e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
        git checkout 4a0f8262fe071b0b27c6fba7455627f3c5a5209e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/hwmon/tmp421.c:178:10: error: variable 'err' is uninitialized when used here [-Werror,-Wuninitialized]
                   return err;
                          ^~~
   drivers/hwmon/tmp421.c:170:9: note: initialize the variable 'err' to silence this warning
           int err;
                  ^
                   = 0
   1 error generated.


vim +/err +178 drivers/hwmon/tmp421.c

   167	
   168	static int tmp421_enable_channels(struct tmp421_data *data)
   169	{
   170		int err;
   171		struct i2c_client *client = data->client;
   172		struct device *dev = &client->dev;
   173		int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
   174		int i;
   175	
   176		if (cfg < 0) {
   177			dev_err(dev, "error reading register, can't disable channels\n");
 > 178			return err;
   179		}
   180	
   181		cfg &= ~TMP421_CONFIG_REG_REN_MASK;
   182		for (i = 0; i < data->channels; i++)
   183			if (data->channel[i].enabled)
   184				cfg |= TMP421_CONFIG_REG_REN(i);
   185	
   186		err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg);
   187		if (err < 0)
   188			dev_err(dev, "error writing register, can't disable channels\n");
   189	
   190		return err;
   191	}
   192	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40883 bytes --]

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

* Re: [PATCH v4 06/10] hwmon: (tmp421) really disable channels
@ 2021-10-13 18:50     ` kernel test robot
  0 siblings, 0 replies; 26+ messages in thread
From: kernel test robot @ 2021-10-13 18:50 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2988 bytes --]

Hi Krzysztof,

I love your patch! Yet something to improve:

[auto build test ERROR on groeck-staging/hwmon-next]
[also build test ERROR on next-20211013]
[cannot apply to robh/for-next v5.15-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: i386-buildonly-randconfig-r004-20211013 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project b6a8c695542b2987eb9a203d5663a0740cb4725f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/4a0f8262fe071b0b27c6fba7455627f3c5a5209e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
        git checkout 4a0f8262fe071b0b27c6fba7455627f3c5a5209e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/hwmon/tmp421.c:178:10: error: variable 'err' is uninitialized when used here [-Werror,-Wuninitialized]
                   return err;
                          ^~~
   drivers/hwmon/tmp421.c:170:9: note: initialize the variable 'err' to silence this warning
           int err;
                  ^
                   = 0
   1 error generated.


vim +/err +178 drivers/hwmon/tmp421.c

   167	
   168	static int tmp421_enable_channels(struct tmp421_data *data)
   169	{
   170		int err;
   171		struct i2c_client *client = data->client;
   172		struct device *dev = &client->dev;
   173		int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
   174		int i;
   175	
   176		if (cfg < 0) {
   177			dev_err(dev, "error reading register, can't disable channels\n");
 > 178			return err;
   179		}
   180	
   181		cfg &= ~TMP421_CONFIG_REG_REN_MASK;
   182		for (i = 0; i < data->channels; i++)
   183			if (data->channel[i].enabled)
   184				cfg |= TMP421_CONFIG_REG_REN(i);
   185	
   186		err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg);
   187		if (err < 0)
   188			dev_err(dev, "error writing register, can't disable channels\n");
   189	
   190		return err;
   191	}
   192	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 40883 bytes --]

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

end of thread, other threads:[~2021-10-13 18:51 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-12  9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
2021-10-12  9:14 ` [PATCH v4 01/10] dt-bindings: hwmon: add missing tmp421 binding Krzysztof Adamski
2021-10-12  9:26 ` [PATCH v4 02/10] hwmon: (tmp421) introduce a channel struct Krzysztof Adamski
2021-10-12 14:25   ` Guenter Roeck
2021-10-12  9:27 ` [PATCH v4 03/10] hwmon: (tmp421) add support for defining labels from DT Krzysztof Adamski
2021-10-12 14:30   ` Guenter Roeck
2021-10-12  9:27 ` [PATCH v4 04/10] hwmon: (tmp421) support disabling channels " Krzysztof Adamski
2021-10-12 14:32   ` Guenter Roeck
2021-10-12  9:28 ` [PATCH v4 05/10] hwmon: (tmp421) support specifying n-factor via DT Krzysztof Adamski
2021-10-12 14:34   ` Guenter Roeck
2021-10-12  9:29 ` [PATCH v4 06/10] hwmon: (tmp421) really disable channels Krzysztof Adamski
2021-10-12 14:37   ` Guenter Roeck
2021-10-12 18:01   ` kernel test robot
2021-10-12 18:01     ` kernel test robot
2021-10-13 18:50   ` kernel test robot
2021-10-13 18:50     ` kernel test robot
2021-10-12  9:29 ` [PATCH v4 07/10] hwmon: (tmp421) support HWMON_T_ENABLE Krzysztof Adamski
2021-10-12 14:43   ` Guenter Roeck
2021-10-12  9:30 ` [PATCH v4 08/10] hwmon: (tmp421) update documentation Krzysztof Adamski
2021-10-12 14:43   ` Guenter Roeck
2021-10-12  9:30 ` [PATCH v4 09/10] hwmon: (tmp421) ignore non-channel related DT nodes Krzysztof Adamski
2021-10-12 14:44   ` Guenter Roeck
2021-10-12  9:30 ` [PATCH v4 10/10] dt-bindings: hwmon: allow specifying channels for tmp421 Krzysztof Adamski
2021-10-12 22:41 [PATCH v4 06/10] hwmon: (tmp421) really disable channels kernel test robot
2021-10-13  8:36 ` Dan Carpenter
2021-10-13  8:36 ` Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.