All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/9] Add per channel properies support in tmp421
@ 2021-10-14 13:00 Krzysztof Adamski
  2021-10-14 13:00 ` [PATCH v5 1/9] dt-bindings: hwmon: add missing tmp421 binding Krzysztof Adamski
                   ` (8 more replies)
  0 siblings, 9 replies; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-14 13:00 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 v5. Changes compared to v4:

- Use data->channels instead of MAX_CHANNELS to protect from specifying configuration for non-existing channels
- Return an error (instead of silently ignoring that) if n-factor is specified for internal channel
- Dropped some unneeded empty lines in tmp421_read
- Check attr, not type, in tmp421_write
- Fixed variable used for early return from tmp421_enable_channels
- Only update TMP421_CONFIG_REG_2 if it was changed

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 (9):
   dt-bindings: hwmon: add missing tmp421 binding
   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                        | 157 +++++++++++++++++-
  3 files changed, 272 insertions(+), 5 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml

-- 
2.31.1


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

* [PATCH v5 1/9] dt-bindings: hwmon: add missing tmp421 binding
  2021-10-14 13:00 [PATCH v5 0/9] Add per channel properies support in tmp421 Krzysztof Adamski
@ 2021-10-14 13:00 ` Krzysztof Adamski
  2021-10-14 16:31   ` Rob Herring
  2021-10-15 22:29   ` Guenter Roeck
  2021-10-14 13:00 ` [PATCH v5 2/9] hwmon: (tmp421) add support for defining labels from DT Krzysztof Adamski
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-14 13:00 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] 22+ messages in thread

* [PATCH v5 2/9] hwmon: (tmp421) add support for defining labels from DT
  2021-10-14 13:00 [PATCH v5 0/9] Add per channel properies support in tmp421 Krzysztof Adamski
  2021-10-14 13:00 ` [PATCH v5 1/9] dt-bindings: hwmon: add missing tmp421 binding Krzysztof Adamski
@ 2021-10-14 13:00 ` Krzysztof Adamski
  2021-10-15 22:45   ` Guenter Roeck
  2021-10-14 13:01 ` [PATCH v5 3/9] hwmon: (tmp421) support disabling channels " Krzysztof Adamski
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-14 13:00 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..89346ca6c9a9 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 >= data->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] 22+ messages in thread

* [PATCH v5 3/9] hwmon: (tmp421) support disabling channels from DT
  2021-10-14 13:00 [PATCH v5 0/9] Add per channel properies support in tmp421 Krzysztof Adamski
  2021-10-14 13:00 ` [PATCH v5 1/9] dt-bindings: hwmon: add missing tmp421 binding Krzysztof Adamski
  2021-10-14 13:00 ` [PATCH v5 2/9] hwmon: (tmp421) add support for defining labels from DT Krzysztof Adamski
@ 2021-10-14 13:01 ` Krzysztof Adamski
  2021-10-15 22:47   ` Guenter Roeck
  2021-10-14 13:01 ` [PATCH v5 4/9] hwmon: (tmp421) support specifying n-factor via DT Krzysztof Adamski
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-14 13:01 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 89346ca6c9a9..af08bc985a13 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] 22+ messages in thread

* [PATCH v5 4/9] hwmon: (tmp421) support specifying n-factor via DT
  2021-10-14 13:00 [PATCH v5 0/9] Add per channel properies support in tmp421 Krzysztof Adamski
                   ` (2 preceding siblings ...)
  2021-10-14 13:01 ` [PATCH v5 3/9] hwmon: (tmp421) support disabling channels " Krzysztof Adamski
@ 2021-10-14 13:01 ` Krzysztof Adamski
  2021-10-15 22:49   ` Guenter Roeck
  2021-10-14 13:09 ` [PATCH v5 5/9] hwmon: (tmp421) really disable channels Krzysztof Adamski
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-14 13:01 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 | 18 ++++++++++++++++++
  1 file changed, 18 insertions(+)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index af08bc985a13..606914f20910 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,22 @@ static int tmp421_probe_child_from_dt(struct i2c_client *client,
  
  	data->channel[i].enabled = of_device_is_available(child);
  
+	err = of_property_read_s32(child, "ti,n-factor", &val);
+	if (!err) {
+		if (i == 0) {
+			dev_err(dev, "n-factor can't be set for internal channel\n");
+			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;
  }
  
-- 
2.31.1


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

* [PATCH v5 5/9] hwmon: (tmp421) really disable channels
  2021-10-14 13:00 [PATCH v5 0/9] Add per channel properies support in tmp421 Krzysztof Adamski
                   ` (3 preceding siblings ...)
  2021-10-14 13:01 ` [PATCH v5 4/9] hwmon: (tmp421) support specifying n-factor via DT Krzysztof Adamski
@ 2021-10-14 13:09 ` Krzysztof Adamski
  2021-10-15 22:53   ` Guenter Roeck
  2021-10-14 13:09 ` [PATCH v5 6/9] hwmon: (tmp421) support HWMON_T_ENABLE Krzysztof Adamski
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-14 13:09 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 | 44 ++++++++++++++++++++++++++++++++++++------
  1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 606914f20910..b402e81ae5d5 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,34 @@ 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 old = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
+	int new, i;
+
+	if (old < 0) {
+		dev_err(dev, "error reading register, can't disable channels\n");
+		return old;
+	}
+
+	new = old & ~TMP421_CONFIG_REG_REN_MASK;
+	for (i = 0; i < data->channels; i++)
+		if (data->channel[i].enabled)
+			new |= TMP421_CONFIG_REG_REN(i);
+
+	if (new == old)
+		return 0;
+
+	err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, new);
+	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 +248,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 +272,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,
@@ -391,10 +423,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;
@@ -404,6 +432,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] 22+ messages in thread

* [PATCH v5 6/9] hwmon: (tmp421) support HWMON_T_ENABLE
  2021-10-14 13:00 [PATCH v5 0/9] Add per channel properies support in tmp421 Krzysztof Adamski
                   ` (4 preceding siblings ...)
  2021-10-14 13:09 ` [PATCH v5 5/9] hwmon: (tmp421) really disable channels Krzysztof Adamski
@ 2021-10-14 13:09 ` Krzysztof Adamski
  2021-10-15 22:53   ` Guenter Roeck
  2021-10-14 13:10 ` [PATCH v5 7/9] hwmon: (tmp421) update documentation Krzysztof Adamski
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-14 13:09 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 | 33 +++++++++++++++++++++++++++++----
  1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index b402e81ae5d5..2e20c558fcb0 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -203,21 +203,25 @@ 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;
  	}
@@ -234,6 +238,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 (attr) {
+	case hwmon_temp_enable:
+		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)
  {
@@ -243,6 +265,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;
  	}
@@ -402,6 +426,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)
@@ -424,7 +449,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] 22+ messages in thread

* [PATCH v5 7/9] hwmon: (tmp421) update documentation
  2021-10-14 13:00 [PATCH v5 0/9] Add per channel properies support in tmp421 Krzysztof Adamski
                   ` (5 preceding siblings ...)
  2021-10-14 13:09 ` [PATCH v5 6/9] hwmon: (tmp421) support HWMON_T_ENABLE Krzysztof Adamski
@ 2021-10-14 13:10 ` Krzysztof Adamski
  2021-10-15 22:54   ` Guenter Roeck
  2021-10-14 13:11 ` [PATCH v5 8/9] hwmon: (tmp421) ignore non-channel related DT nodes Krzysztof Adamski
  2021-10-14 13:11 ` [PATCH v5 9/9] dt-bindings: hwmon: allow specifying channels for tmp421 Krzysztof Adamski
  8 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-14 13:10 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] 22+ messages in thread

* [PATCH v5 8/9] hwmon: (tmp421) ignore non-channel related DT nodes
  2021-10-14 13:00 [PATCH v5 0/9] Add per channel properies support in tmp421 Krzysztof Adamski
                   ` (6 preceding siblings ...)
  2021-10-14 13:10 ` [PATCH v5 7/9] hwmon: (tmp421) update documentation Krzysztof Adamski
@ 2021-10-14 13:11 ` Krzysztof Adamski
  2021-10-15 22:56   ` Guenter Roeck
  2021-10-14 13:11 ` [PATCH v5 9/9] dt-bindings: hwmon: allow specifying channels for tmp421 Krzysztof Adamski
  8 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-14 13:11 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 2e20c558fcb0..493a24cdfca2 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -414,6 +414,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] 22+ messages in thread

* [PATCH v5 9/9] dt-bindings: hwmon: allow specifying channels for tmp421
  2021-10-14 13:00 [PATCH v5 0/9] Add per channel properies support in tmp421 Krzysztof Adamski
                   ` (7 preceding siblings ...)
  2021-10-14 13:11 ` [PATCH v5 8/9] hwmon: (tmp421) ignore non-channel related DT nodes Krzysztof Adamski
@ 2021-10-14 13:11 ` Krzysztof Adamski
  2021-10-14 16:31   ` Rob Herring
  2021-10-15 22:57   ` Guenter Roeck
  8 siblings, 2 replies; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-14 13:11 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] 22+ messages in thread

* Re: [PATCH v5 9/9] dt-bindings: hwmon: allow specifying channels for tmp421
  2021-10-14 13:11 ` [PATCH v5 9/9] dt-bindings: hwmon: allow specifying channels for tmp421 Krzysztof Adamski
@ 2021-10-14 16:31   ` Rob Herring
  2021-10-15 22:57   ` Guenter Roeck
  1 sibling, 0 replies; 22+ messages in thread
From: Rob Herring @ 2021-10-14 16:31 UTC (permalink / raw)
  To: Krzysztof Adamski; +Cc: Guenter Roeck, Jean Delvare, linux-hwmon, devicetree

On Thu, Oct 14, 2021 at 03:11:49PM +0200, Krzysztof Adamski wrote:
> 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(+)

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH v5 1/9] dt-bindings: hwmon: add missing tmp421 binding
  2021-10-14 13:00 ` [PATCH v5 1/9] dt-bindings: hwmon: add missing tmp421 binding Krzysztof Adamski
@ 2021-10-14 16:31   ` Rob Herring
  2021-10-15 22:29   ` Guenter Roeck
  1 sibling, 0 replies; 22+ messages in thread
From: Rob Herring @ 2021-10-14 16:31 UTC (permalink / raw)
  To: Krzysztof Adamski
  Cc: Jean Delvare, linux-hwmon, Guenter Roeck, Rob Herring, devicetree

On Thu, 14 Oct 2021 15:00:25 +0200, Krzysztof Adamski wrote:
> 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
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH v5 1/9] dt-bindings: hwmon: add missing tmp421 binding
  2021-10-14 13:00 ` [PATCH v5 1/9] dt-bindings: hwmon: add missing tmp421 binding Krzysztof Adamski
  2021-10-14 16:31   ` Rob Herring
@ 2021-10-15 22:29   ` Guenter Roeck
  1 sibling, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-15 22:29 UTC (permalink / raw)
  To: Krzysztof Adamski; +Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree

On Thu, Oct 14, 2021 at 03:00:25PM +0200, Krzysztof Adamski wrote:
> Add basic description of the tmp421 driver DT bindings.
> 
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Applied.

Thanks,
Guenter

> ---
>   .../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>;
> +      };
> +    };

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

* Re: [PATCH v5 2/9] hwmon: (tmp421) add support for defining labels from DT
  2021-10-14 13:00 ` [PATCH v5 2/9] hwmon: (tmp421) add support for defining labels from DT Krzysztof Adamski
@ 2021-10-15 22:45   ` Guenter Roeck
  0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-15 22:45 UTC (permalink / raw)
  To: Krzysztof Adamski; +Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree

On Thu, Oct 14, 2021 at 03:00:49PM +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..89346ca6c9a9 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;
>   };

git reports this patch as corrupted and claims it was hand-edited.
Looking into it, there is an extra space in unchanged lines of code.
Indeed, it applies cleanly after replacing douple spaces at the beginning
of each line with a single space.

No idea how that happened, but please be careful whend sending patches.

Applied.

Guenter

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

* Re: [PATCH v5 3/9] hwmon: (tmp421) support disabling channels from DT
  2021-10-14 13:01 ` [PATCH v5 3/9] hwmon: (tmp421) support disabling channels " Krzysztof Adamski
@ 2021-10-15 22:47   ` Guenter Roeck
  2021-10-17  0:00     ` Krzysztof Adamski
  0 siblings, 1 reply; 22+ messages in thread
From: Guenter Roeck @ 2021-10-15 22:47 UTC (permalink / raw)
  To: Krzysztof Adamski; +Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree

On Thu, Oct 14, 2021 at 03:01:09PM +0200, 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>

Same problem as with patch 2/9. Any idea what happened ?

Anyway, applied after fixing it up.

Guenter

> ---
>  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 89346ca6c9a9..af08bc985a13 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] 22+ messages in thread

* Re: [PATCH v5 4/9] hwmon: (tmp421) support specifying n-factor via DT
  2021-10-14 13:01 ` [PATCH v5 4/9] hwmon: (tmp421) support specifying n-factor via DT Krzysztof Adamski
@ 2021-10-15 22:49   ` Guenter Roeck
  0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-15 22:49 UTC (permalink / raw)
  To: Krzysztof Adamski; +Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree

On Thu, Oct 14, 2021 at 03:01:28PM +0200, 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>

Applied, after "s/^  / /"

Guenter

> ---
>  drivers/hwmon/tmp421.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index af08bc985a13..606914f20910 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,22 @@ static int tmp421_probe_child_from_dt(struct i2c_client *client,
>  
>  	data->channel[i].enabled = of_device_is_available(child);
>  
> +	err = of_property_read_s32(child, "ti,n-factor", &val);
> +	if (!err) {
> +		if (i == 0) {
> +			dev_err(dev, "n-factor can't be set for internal channel\n");
> +			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] 22+ messages in thread

* Re: [PATCH v5 5/9] hwmon: (tmp421) really disable channels
  2021-10-14 13:09 ` [PATCH v5 5/9] hwmon: (tmp421) really disable channels Krzysztof Adamski
@ 2021-10-15 22:53   ` Guenter Roeck
  0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-15 22:53 UTC (permalink / raw)
  To: Krzysztof Adamski; +Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree

On Thu, Oct 14, 2021 at 03:09:08PM +0200, 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>

Applied, again after "s/^  / /"

Guenter

> ---
>  drivers/hwmon/tmp421.c | 44 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 606914f20910..b402e81ae5d5 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,34 @@ 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 old = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
> +	int new, i;
> +
> +	if (old < 0) {
> +		dev_err(dev, "error reading register, can't disable channels\n");
> +		return old;
> +	}
> +
> +	new = old & ~TMP421_CONFIG_REG_REN_MASK;
> +	for (i = 0; i < data->channels; i++)
> +		if (data->channel[i].enabled)
> +			new |= TMP421_CONFIG_REG_REN(i);
> +
> +	if (new == old)
> +		return 0;
> +
> +	err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, new);
> +	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 +248,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 +272,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,
> @@ -391,10 +423,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;
> @@ -404,6 +432,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] 22+ messages in thread

* Re: [PATCH v5 6/9] hwmon: (tmp421) support HWMON_T_ENABLE
  2021-10-14 13:09 ` [PATCH v5 6/9] hwmon: (tmp421) support HWMON_T_ENABLE Krzysztof Adamski
@ 2021-10-15 22:53   ` Guenter Roeck
  0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-15 22:53 UTC (permalink / raw)
  To: Krzysztof Adamski; +Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree

On Thu, Oct 14, 2021 at 03:09:58PM +0200, 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>

Applied, with the usual fixup.

Guenter

> ---
>  drivers/hwmon/tmp421.c | 33 +++++++++++++++++++++++++++++----
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index b402e81ae5d5..2e20c558fcb0 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -203,21 +203,25 @@ 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;
>  	}
> @@ -234,6 +238,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 (attr) {
> +	case hwmon_temp_enable:
> +		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)
>  {
> @@ -243,6 +265,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;
>  	}
> @@ -402,6 +426,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)
> @@ -424,7 +449,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] 22+ messages in thread

* Re: [PATCH v5 7/9] hwmon: (tmp421) update documentation
  2021-10-14 13:10 ` [PATCH v5 7/9] hwmon: (tmp421) update documentation Krzysztof Adamski
@ 2021-10-15 22:54   ` Guenter Roeck
  0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-15 22:54 UTC (permalink / raw)
  To: Krzysztof Adamski; +Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree

On Thu, Oct 14, 2021 at 03:10:44PM +0200, 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>

Applied, again after the fixup.

Guenter

> ---
>  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] 22+ messages in thread

* Re: [PATCH v5 8/9] hwmon: (tmp421) ignore non-channel related DT nodes
  2021-10-14 13:11 ` [PATCH v5 8/9] hwmon: (tmp421) ignore non-channel related DT nodes Krzysztof Adamski
@ 2021-10-15 22:56   ` Guenter Roeck
  0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-15 22:56 UTC (permalink / raw)
  To: Krzysztof Adamski; +Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree

On Thu, Oct 14, 2021 at 03:11:02PM +0200, 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>

Applied, after the usual fixup.

Guenter

> ---
>  drivers/hwmon/tmp421.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 2e20c558fcb0..493a24cdfca2 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -414,6 +414,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] 22+ messages in thread

* Re: [PATCH v5 9/9] dt-bindings: hwmon: allow specifying channels for tmp421
  2021-10-14 13:11 ` [PATCH v5 9/9] dt-bindings: hwmon: allow specifying channels for tmp421 Krzysztof Adamski
  2021-10-14 16:31   ` Rob Herring
@ 2021-10-15 22:57   ` Guenter Roeck
  1 sibling, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-15 22:57 UTC (permalink / raw)
  To: Krzysztof Adamski; +Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree

On Thu, Oct 14, 2021 at 03:11:49PM +0200, Krzysztof Adamski wrote:
> Add binding description for the per temperature channel configuration
> like labels and n-factor.
> 
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Applied, again after 's/^  / /"

Guenter

> ---
>  .../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";
> +        };
> +      };
> +    };

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

* Re: [PATCH v5 3/9] hwmon: (tmp421) support disabling channels from DT
  2021-10-15 22:47   ` Guenter Roeck
@ 2021-10-17  0:00     ` Krzysztof Adamski
  0 siblings, 0 replies; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-17  0:00 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree

Dnia Fri, Oct 15, 2021 at 03:47:41PM -0700, Guenter Roeck napisał(a):
>On Thu, Oct 14, 2021 at 03:01:09PM +0200, 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>
>
>Same problem as with patch 2/9. Any idea what happened ?
>
>Anyway, applied after fixing it up.

Thank you and sorry for the trouble. It isn't trivial to send patches
via our enterprise mail platform and this time I had to contrive more
than usually. I did use mutt, as I usually do, but had to run it from
different machine. Even though I did use the same config, the mutt
version was different and EDITOR might have been configured differently,
so maybe that created a problem? Or it was the Exchange server which
mangled my patches? I don't know now but I will watch out for that,
thank you for the heads up on that.

Best regards,
Krzysztof Adamski

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

end of thread, other threads:[~2021-10-17  0:00 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-14 13:00 [PATCH v5 0/9] Add per channel properies support in tmp421 Krzysztof Adamski
2021-10-14 13:00 ` [PATCH v5 1/9] dt-bindings: hwmon: add missing tmp421 binding Krzysztof Adamski
2021-10-14 16:31   ` Rob Herring
2021-10-15 22:29   ` Guenter Roeck
2021-10-14 13:00 ` [PATCH v5 2/9] hwmon: (tmp421) add support for defining labels from DT Krzysztof Adamski
2021-10-15 22:45   ` Guenter Roeck
2021-10-14 13:01 ` [PATCH v5 3/9] hwmon: (tmp421) support disabling channels " Krzysztof Adamski
2021-10-15 22:47   ` Guenter Roeck
2021-10-17  0:00     ` Krzysztof Adamski
2021-10-14 13:01 ` [PATCH v5 4/9] hwmon: (tmp421) support specifying n-factor via DT Krzysztof Adamski
2021-10-15 22:49   ` Guenter Roeck
2021-10-14 13:09 ` [PATCH v5 5/9] hwmon: (tmp421) really disable channels Krzysztof Adamski
2021-10-15 22:53   ` Guenter Roeck
2021-10-14 13:09 ` [PATCH v5 6/9] hwmon: (tmp421) support HWMON_T_ENABLE Krzysztof Adamski
2021-10-15 22:53   ` Guenter Roeck
2021-10-14 13:10 ` [PATCH v5 7/9] hwmon: (tmp421) update documentation Krzysztof Adamski
2021-10-15 22:54   ` Guenter Roeck
2021-10-14 13:11 ` [PATCH v5 8/9] hwmon: (tmp421) ignore non-channel related DT nodes Krzysztof Adamski
2021-10-15 22:56   ` Guenter Roeck
2021-10-14 13:11 ` [PATCH v5 9/9] dt-bindings: hwmon: allow specifying channels for tmp421 Krzysztof Adamski
2021-10-14 16:31   ` Rob Herring
2021-10-15 22:57   ` Guenter Roeck

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.