linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 1/2] dt-bindings: hwmon: Add nct7802 bindings
@ 2021-10-09 18:52 Oskar Senft
  2021-10-09 18:52 ` [PATCH v5 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable Oskar Senft
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Oskar Senft @ 2021-10-09 18:52 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Rob Herring, linux-hwmon,
	linux-kernel, devicetree
  Cc: Oskar Senft

This change documents the device tree bindings for the Nuvoton
NCT7802Y driver.

Signed-off-by: Oskar Senft <osk@google.com>
---
 .../bindings/hwmon/nuvoton,nct7802.yaml       | 142 ++++++++++++++++++
 1 file changed, 142 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct7802.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct7802.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7802.yaml
new file mode 100644
index 000000000000..ff99f40034f2
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7802.yaml
@@ -0,0 +1,142 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/nuvoton,nct7802.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton NCT7802Y Hardware Monitoring IC
+
+maintainers:
+  - Guenter Roeck <linux@roeck-us.net>
+
+description: |
+  The NCT7802Y is a hardware monitor IC which supports one on-die and up to
+  5 remote temperature sensors with SMBus interface.
+
+  Datasheets:
+    https://www.nuvoton.com/export/resource-files/Nuvoton_NCT7802Y_Datasheet_V12.pdf
+
+properties:
+  compatible:
+    enum:
+      - nuvoton,nct7802
+
+  reg:
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  channel@0:
+    type: object
+    description: Local Temperature Sensor ("LTD")
+    properties:
+      reg:
+        const: 0
+    required:
+      - reg
+
+  channel@1:
+    type: object
+    description: Remote Temperature Sensor or Voltage Sensor ("RTD1")
+    properties:
+      reg:
+        const: 1
+      sensor-type:
+        items:
+          - enum:
+              - temperature
+              - voltage
+      temperature-mode:
+        items:
+          - enum:
+              - thermistor
+              - thermal-diode
+    required:
+      - reg
+      - sensor-type
+
+  channel@2:
+    type: object
+    description: Remote Temperature Sensor or Voltage Sensor ("RTD2")
+    properties:
+      reg:
+        const: 2
+      sensor-type:
+        items:
+          - enum:
+              - temperature
+              - voltage
+      temperature-mode:
+        items:
+          - enum:
+              - thermistor
+              - thermal-diode
+    required:
+      - reg
+      - sensor-type
+
+  channel@3:
+    type: object
+    description: Remote Temperature Sensor or Voltage Sensor ("RTD3")
+    properties:
+      reg:
+        const: 3
+      sensor-type:
+        items:
+          - enum:
+              - temperature
+              - voltage
+    required:
+      - reg
+      - sensor-type
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        nct7802@28 {
+            compatible = "nuvoton,nct7802";
+            reg = <0x28>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            channel@0 { /* LTD */
+              reg = <0>;
+              status = "okay";
+            };
+
+            channel@1 { /* RTD1 */
+              reg = <1>;
+              status = "okay";
+              sensor-type = "temperature";
+              temperature-mode = "thermistor";
+            };
+
+            channel@2 { /* RTD2 */
+              reg = <2>;
+              status = "okay";
+              sensor-type = "temperature";
+              temperature-mode = "thermal-diode";
+            };
+
+            channel@3 { /* RTD3 */
+              reg = <3>;
+              status = "okay";
+              sensor-type = "voltage";
+            };
+        };
+    };
-- 
2.33.0.882.g93a45727a2-goog


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

* [PATCH v5 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable
  2021-10-09 18:52 [PATCH v5 1/2] dt-bindings: hwmon: Add nct7802 bindings Oskar Senft
@ 2021-10-09 18:52 ` Oskar Senft
  2021-10-09 18:53   ` Oskar Senft
                     ` (2 more replies)
  2021-10-09 18:53 ` [PATCH v5 1/2] dt-bindings: hwmon: Add nct7802 bindings Oskar Senft
  2021-10-09 23:46 ` Guenter Roeck
  2 siblings, 3 replies; 10+ messages in thread
From: Oskar Senft @ 2021-10-09 18:52 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Rob Herring, linux-hwmon,
	linux-kernel, devicetree
  Cc: Oskar Senft

This change allows LTD and RTD inputs to be configured via
device tree bindings. If the DT bindings are not present or
invalid, the input configuration is not modified and left at
HW defaults.

Signed-off-by: Oskar Senft <osk@google.com>
---
 drivers/hwmon/nct7802.c | 133 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 129 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
index 604af2f6103a..e28f8eaf9f0f 100644
--- a/drivers/hwmon/nct7802.c
+++ b/drivers/hwmon/nct7802.c
@@ -51,6 +51,23 @@ static const u8 REG_VOLTAGE_LIMIT_MSB_SHIFT[2][5] = {
 #define REG_CHIP_ID		0xfe
 #define REG_VERSION_ID		0xff
 
+/*
+ * Resistance temperature detector (RTD) modes according to 7.2.32 Mode
+ * Selection Register
+ */
+#define RTD_MODE_CURRENT	0x1
+#define RTD_MODE_THERMISTOR	0x2
+#define RTD_MODE_VOLTAGE	0x3
+
+#define MODE_RTD_MASK		0x3
+#define MODE_LTD_EN		0x40
+
+/*
+ * Bit offset for sensors modes in REG_MODE.
+ * Valid for index 0..2, indicating RTD1..3.
+ */
+#define MODE_BIT_OFFSET_RTD(index) ((index) * 2)
+
 /*
  * Data structures and manipulation thereof
  */
@@ -1038,7 +1055,116 @@ static const struct regmap_config nct7802_regmap_config = {
 	.volatile_reg = nct7802_regmap_is_volatile,
 };
 
-static int nct7802_init_chip(struct nct7802_data *data)
+static int nct7802_get_channel_config(struct device *dev,
+				      struct device_node *node, u8 *mode_mask,
+				      u8 *mode_val)
+{
+	u32 reg;
+	const char *type_str, *md_str;
+	u8 md;
+
+	if (!node->name || of_node_cmp(node->name, "channel"))
+		return 0;
+
+	if (of_property_read_u32(node, "reg", &reg)) {
+		dev_err(dev, "Could not read reg value for '%s'\n",
+			node->full_name);
+		return -EINVAL;
+	}
+
+	if (reg > 3) {
+		dev_err(dev, "Invalid reg (%u) in '%s'\n", reg,
+			node->full_name);
+		return -EINVAL;
+	}
+
+	if (reg == 0) {
+		if (!of_device_is_available(node))
+			*mode_val &= ~MODE_LTD_EN;
+		else
+			*mode_val |= MODE_LTD_EN;
+		*mode_mask |= MODE_LTD_EN;
+		return 0;
+	}
+
+	/* At this point we have reg >= 1 && reg <= 3 */
+
+	if (!of_device_is_available(node)) {
+		*mode_val &= ~(MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1));
+		*mode_mask |= MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1);
+		return 0;
+	}
+
+	if (of_property_read_string(node, "sensor-type", &type_str)) {
+		dev_err(dev, "No type for '%s'\n", node->full_name);
+		return -EINVAL;
+	}
+
+	if (!strcmp(type_str, "voltage")) {
+		*mode_val |= (RTD_MODE_VOLTAGE & MODE_RTD_MASK)
+			     << MODE_BIT_OFFSET_RTD(reg - 1);
+		*mode_mask |= MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1);
+		return 0;
+	}
+
+	if (strcmp(type_str, "temperature")) {
+		dev_err(dev, "Invalid type '%s' for '%s'\n", type_str,
+			node->full_name);
+		return -EINVAL;
+	}
+
+	if (reg == 3) {
+		/* RTD3 only supports thermistor mode */
+		md = RTD_MODE_THERMISTOR;
+	} else {
+		if (of_property_read_string(node, "temperature-mode",
+					    &md_str)) {
+			dev_err(dev, "No mode for '%s'\n", node->full_name);
+			return -EINVAL;
+		}
+
+		if (!strcmp(md_str, "thermal-diode"))
+			md = RTD_MODE_CURRENT;
+		else if (!strcmp(md_str, "thermistor"))
+			md = RTD_MODE_THERMISTOR;
+		else {
+			dev_err(dev, "Invalid mode '%s' for '%s'\n", md_str,
+				node->full_name);
+			return -EINVAL;
+		}
+	}
+
+	*mode_val |= (md & MODE_RTD_MASK) << MODE_BIT_OFFSET_RTD(reg - 1);
+	*mode_mask |= MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1);
+
+	return 0;
+}
+
+static int nct7802_configure_channels(struct device *dev,
+				      struct nct7802_data *data)
+{
+	bool found_channel_config = false;
+	u8 mode_mask = 0, mode_val = 0;
+	struct device_node *node;
+	int err;
+
+	/* Enable local temperature sensor by default */
+	mode_val |= MODE_LTD_EN;
+	mode_mask |= MODE_LTD_EN;
+
+	if (dev->of_node) {
+		for_each_child_of_node(dev->of_node, node) {
+			err = nct7802_get_channel_config(dev, node, &mode_mask,
+							 &mode_val);
+			if (err)
+				return err;
+		}
+	}
+
+	return regmap_update_bits(data->regmap, REG_MODE, mode_mask, mode_val);
+}
+
+static int nct7802_init_chip(struct device *dev, struct nct7802_data *data)
 {
 	int err;
 
@@ -1047,8 +1173,7 @@ static int nct7802_init_chip(struct nct7802_data *data)
 	if (err)
 		return err;
 
-	/* Enable local temperature sensor */
-	err = regmap_update_bits(data->regmap, REG_MODE, 0x40, 0x40);
+	err = nct7802_configure_channels(dev, data);
 	if (err)
 		return err;
 
@@ -1074,7 +1199,7 @@ static int nct7802_probe(struct i2c_client *client)
 	mutex_init(&data->access_lock);
 	mutex_init(&data->in_alarm_lock);
 
-	ret = nct7802_init_chip(data);
+	ret = nct7802_init_chip(dev, data);
 	if (ret < 0)
 		return ret;
 
-- 
2.33.0.882.g93a45727a2-goog


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

* Re: [PATCH v5 1/2] dt-bindings: hwmon: Add nct7802 bindings
  2021-10-09 18:52 [PATCH v5 1/2] dt-bindings: hwmon: Add nct7802 bindings Oskar Senft
  2021-10-09 18:52 ` [PATCH v5 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable Oskar Senft
@ 2021-10-09 18:53 ` Oskar Senft
  2021-10-09 23:46 ` Guenter Roeck
  2 siblings, 0 replies; 10+ messages in thread
From: Oskar Senft @ 2021-10-09 18:53 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Rob Herring, linux-hwmon,
	linux-kernel, devicetree

Changes since "PATCH v4 1/2" as requested in the review:
- Renamed "signal" to "channel"

On Sat, Oct 9, 2021 at 2:53 PM Oskar Senft <osk@google.com> wrote:
>
> This change documents the device tree bindings for the Nuvoton
> NCT7802Y driver.
>
> Signed-off-by: Oskar Senft <osk@google.com>
> ---
>  .../bindings/hwmon/nuvoton,nct7802.yaml       | 142 ++++++++++++++++++
>  1 file changed, 142 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct7802.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct7802.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7802.yaml
> new file mode 100644
> index 000000000000..ff99f40034f2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7802.yaml
> @@ -0,0 +1,142 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/hwmon/nuvoton,nct7802.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nuvoton NCT7802Y Hardware Monitoring IC
> +
> +maintainers:
> +  - Guenter Roeck <linux@roeck-us.net>
> +
> +description: |
> +  The NCT7802Y is a hardware monitor IC which supports one on-die and up to
> +  5 remote temperature sensors with SMBus interface.
> +
> +  Datasheets:
> +    https://www.nuvoton.com/export/resource-files/Nuvoton_NCT7802Y_Datasheet_V12.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - nuvoton,nct7802
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  channel@0:
> +    type: object
> +    description: Local Temperature Sensor ("LTD")
> +    properties:
> +      reg:
> +        const: 0
> +    required:
> +      - reg
> +
> +  channel@1:
> +    type: object
> +    description: Remote Temperature Sensor or Voltage Sensor ("RTD1")
> +    properties:
> +      reg:
> +        const: 1
> +      sensor-type:
> +        items:
> +          - enum:
> +              - temperature
> +              - voltage
> +      temperature-mode:
> +        items:
> +          - enum:
> +              - thermistor
> +              - thermal-diode
> +    required:
> +      - reg
> +      - sensor-type
> +
> +  channel@2:
> +    type: object
> +    description: Remote Temperature Sensor or Voltage Sensor ("RTD2")
> +    properties:
> +      reg:
> +        const: 2
> +      sensor-type:
> +        items:
> +          - enum:
> +              - temperature
> +              - voltage
> +      temperature-mode:
> +        items:
> +          - enum:
> +              - thermistor
> +              - thermal-diode
> +    required:
> +      - reg
> +      - sensor-type
> +
> +  channel@3:
> +    type: object
> +    description: Remote Temperature Sensor or Voltage Sensor ("RTD3")
> +    properties:
> +      reg:
> +        const: 3
> +      sensor-type:
> +        items:
> +          - enum:
> +              - temperature
> +              - voltage
> +    required:
> +      - reg
> +      - sensor-type
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        nct7802@28 {
> +            compatible = "nuvoton,nct7802";
> +            reg = <0x28>;
> +
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            channel@0 { /* LTD */
> +              reg = <0>;
> +              status = "okay";
> +            };
> +
> +            channel@1 { /* RTD1 */
> +              reg = <1>;
> +              status = "okay";
> +              sensor-type = "temperature";
> +              temperature-mode = "thermistor";
> +            };
> +
> +            channel@2 { /* RTD2 */
> +              reg = <2>;
> +              status = "okay";
> +              sensor-type = "temperature";
> +              temperature-mode = "thermal-diode";
> +            };
> +
> +            channel@3 { /* RTD3 */
> +              reg = <3>;
> +              status = "okay";
> +              sensor-type = "voltage";
> +            };
> +        };
> +    };
> --
> 2.33.0.882.g93a45727a2-goog
>

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

* Re: [PATCH v5 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable
  2021-10-09 18:52 ` [PATCH v5 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable Oskar Senft
@ 2021-10-09 18:53   ` Oskar Senft
  2021-10-09 22:40   ` kernel test robot
  2021-10-09 23:40   ` Guenter Roeck
  2 siblings, 0 replies; 10+ messages in thread
From: Oskar Senft @ 2021-10-09 18:53 UTC (permalink / raw)
  To: Jean Delvare, Guenter Roeck, Rob Herring, linux-hwmon,
	linux-kernel, devicetree

Changes since "PATCH v4 2/2" as requested in the review:
- Changed device tree node name from "signal" to "channel".
- Removed unrelated changes to replace literal values with #defines.
- Fixed formatting issues by running clang-format.
- Removed unneccessary check for reg >= 1 && reg <= 3.
- nct7802_get_channel_config is now returning an error value.
- nct7802_configure_channels now stops processing the device tree on
the first error.
- Simplified logic for LTD default configuration in nct7802_configure_channels.

On Sat, Oct 9, 2021 at 2:53 PM Oskar Senft <osk@google.com> wrote:
>
> This change allows LTD and RTD inputs to be configured via
> device tree bindings. If the DT bindings are not present or
> invalid, the input configuration is not modified and left at
> HW defaults.
>
> Signed-off-by: Oskar Senft <osk@google.com>
> ---
>  drivers/hwmon/nct7802.c | 133 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 129 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
> index 604af2f6103a..e28f8eaf9f0f 100644
> --- a/drivers/hwmon/nct7802.c
> +++ b/drivers/hwmon/nct7802.c
> @@ -51,6 +51,23 @@ static const u8 REG_VOLTAGE_LIMIT_MSB_SHIFT[2][5] = {
>  #define REG_CHIP_ID            0xfe
>  #define REG_VERSION_ID         0xff
>
> +/*
> + * Resistance temperature detector (RTD) modes according to 7.2.32 Mode
> + * Selection Register
> + */
> +#define RTD_MODE_CURRENT       0x1
> +#define RTD_MODE_THERMISTOR    0x2
> +#define RTD_MODE_VOLTAGE       0x3
> +
> +#define MODE_RTD_MASK          0x3
> +#define MODE_LTD_EN            0x40
> +
> +/*
> + * Bit offset for sensors modes in REG_MODE.
> + * Valid for index 0..2, indicating RTD1..3.
> + */
> +#define MODE_BIT_OFFSET_RTD(index) ((index) * 2)
> +
>  /*
>   * Data structures and manipulation thereof
>   */
> @@ -1038,7 +1055,116 @@ static const struct regmap_config nct7802_regmap_config = {
>         .volatile_reg = nct7802_regmap_is_volatile,
>  };
>
> -static int nct7802_init_chip(struct nct7802_data *data)
> +static int nct7802_get_channel_config(struct device *dev,
> +                                     struct device_node *node, u8 *mode_mask,
> +                                     u8 *mode_val)
> +{
> +       u32 reg;
> +       const char *type_str, *md_str;
> +       u8 md;
> +
> +       if (!node->name || of_node_cmp(node->name, "channel"))
> +               return 0;
> +
> +       if (of_property_read_u32(node, "reg", &reg)) {
> +               dev_err(dev, "Could not read reg value for '%s'\n",
> +                       node->full_name);
> +               return -EINVAL;
> +       }
> +
> +       if (reg > 3) {
> +               dev_err(dev, "Invalid reg (%u) in '%s'\n", reg,
> +                       node->full_name);
> +               return -EINVAL;
> +       }
> +
> +       if (reg == 0) {
> +               if (!of_device_is_available(node))
> +                       *mode_val &= ~MODE_LTD_EN;
> +               else
> +                       *mode_val |= MODE_LTD_EN;
> +               *mode_mask |= MODE_LTD_EN;
> +               return 0;
> +       }
> +
> +       /* At this point we have reg >= 1 && reg <= 3 */
> +
> +       if (!of_device_is_available(node)) {
> +               *mode_val &= ~(MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1));
> +               *mode_mask |= MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1);
> +               return 0;
> +       }
> +
> +       if (of_property_read_string(node, "sensor-type", &type_str)) {
> +               dev_err(dev, "No type for '%s'\n", node->full_name);
> +               return -EINVAL;
> +       }
> +
> +       if (!strcmp(type_str, "voltage")) {
> +               *mode_val |= (RTD_MODE_VOLTAGE & MODE_RTD_MASK)
> +                            << MODE_BIT_OFFSET_RTD(reg - 1);
> +               *mode_mask |= MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1);
> +               return 0;
> +       }
> +
> +       if (strcmp(type_str, "temperature")) {
> +               dev_err(dev, "Invalid type '%s' for '%s'\n", type_str,
> +                       node->full_name);
> +               return -EINVAL;
> +       }
> +
> +       if (reg == 3) {
> +               /* RTD3 only supports thermistor mode */
> +               md = RTD_MODE_THERMISTOR;
> +       } else {
> +               if (of_property_read_string(node, "temperature-mode",
> +                                           &md_str)) {
> +                       dev_err(dev, "No mode for '%s'\n", node->full_name);
> +                       return -EINVAL;
> +               }
> +
> +               if (!strcmp(md_str, "thermal-diode"))
> +                       md = RTD_MODE_CURRENT;
> +               else if (!strcmp(md_str, "thermistor"))
> +                       md = RTD_MODE_THERMISTOR;
> +               else {
> +                       dev_err(dev, "Invalid mode '%s' for '%s'\n", md_str,
> +                               node->full_name);
> +                       return -EINVAL;
> +               }
> +       }
> +
> +       *mode_val |= (md & MODE_RTD_MASK) << MODE_BIT_OFFSET_RTD(reg - 1);
> +       *mode_mask |= MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1);
> +
> +       return 0;
> +}
> +
> +static int nct7802_configure_channels(struct device *dev,
> +                                     struct nct7802_data *data)
> +{
> +       bool found_channel_config = false;
> +       u8 mode_mask = 0, mode_val = 0;
> +       struct device_node *node;
> +       int err;
> +
> +       /* Enable local temperature sensor by default */
> +       mode_val |= MODE_LTD_EN;
> +       mode_mask |= MODE_LTD_EN;
> +
> +       if (dev->of_node) {
> +               for_each_child_of_node(dev->of_node, node) {
> +                       err = nct7802_get_channel_config(dev, node, &mode_mask,
> +                                                        &mode_val);
> +                       if (err)
> +                               return err;
> +               }
> +       }
> +
> +       return regmap_update_bits(data->regmap, REG_MODE, mode_mask, mode_val);
> +}
> +
> +static int nct7802_init_chip(struct device *dev, struct nct7802_data *data)
>  {
>         int err;
>
> @@ -1047,8 +1173,7 @@ static int nct7802_init_chip(struct nct7802_data *data)
>         if (err)
>                 return err;
>
> -       /* Enable local temperature sensor */
> -       err = regmap_update_bits(data->regmap, REG_MODE, 0x40, 0x40);
> +       err = nct7802_configure_channels(dev, data);
>         if (err)
>                 return err;
>
> @@ -1074,7 +1199,7 @@ static int nct7802_probe(struct i2c_client *client)
>         mutex_init(&data->access_lock);
>         mutex_init(&data->in_alarm_lock);
>
> -       ret = nct7802_init_chip(data);
> +       ret = nct7802_init_chip(dev, data);
>         if (ret < 0)
>                 return ret;
>
> --
> 2.33.0.882.g93a45727a2-goog
>

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

* Re: [PATCH v5 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable
  2021-10-09 18:52 ` [PATCH v5 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable Oskar Senft
  2021-10-09 18:53   ` Oskar Senft
@ 2021-10-09 22:40   ` kernel test robot
  2021-10-09 23:40   ` Guenter Roeck
  2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-10-09 22:40 UTC (permalink / raw)
  To: Oskar Senft, Jean Delvare, Guenter Roeck, Rob Herring,
	linux-hwmon, linux-kernel, devicetree
  Cc: kbuild-all, Oskar Senft

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

Hi Oskar,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on groeck-staging/hwmon-next]
[also build test ERROR on robh/for-next v5.15-rc4 next-20211008]
[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/Oskar-Senft/dt-bindings-hwmon-Add-nct7802-bindings/20211010-025416
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 11.2.0
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/d4f58d8c83ce0406595cce4816db4bad37c19920
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Oskar-Senft/dt-bindings-hwmon-Add-nct7802-bindings/20211010-025416
        git checkout d4f58d8c83ce0406595cce4816db4bad37c19920
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=powerpc 

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/nct7802.c: In function 'nct7802_configure_channels':
>> drivers/hwmon/nct7802.c:1146:14: error: unused variable 'found_channel_config' [-Werror=unused-variable]
    1146 |         bool found_channel_config = false;
         |              ^~~~~~~~~~~~~~~~~~~~
   cc1: all warnings being treated as errors


vim +/found_channel_config +1146 drivers/hwmon/nct7802.c

  1142	
  1143	static int nct7802_configure_channels(struct device *dev,
  1144					      struct nct7802_data *data)
  1145	{
> 1146		bool found_channel_config = false;
  1147		u8 mode_mask = 0, mode_val = 0;
  1148		struct device_node *node;
  1149		int err;
  1150	
  1151		/* Enable local temperature sensor by default */
  1152		mode_val |= MODE_LTD_EN;
  1153		mode_mask |= MODE_LTD_EN;
  1154	
  1155		if (dev->of_node) {
  1156			for_each_child_of_node(dev->of_node, node) {
  1157				err = nct7802_get_channel_config(dev, node, &mode_mask,
  1158								 &mode_val);
  1159				if (err)
  1160					return err;
  1161			}
  1162		}
  1163	
  1164		return regmap_update_bits(data->regmap, REG_MODE, mode_mask, mode_val);
  1165	}
  1166	

---
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: 74080 bytes --]

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

* Re: [PATCH v5 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable
  2021-10-09 18:52 ` [PATCH v5 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable Oskar Senft
  2021-10-09 18:53   ` Oskar Senft
  2021-10-09 22:40   ` kernel test robot
@ 2021-10-09 23:40   ` Guenter Roeck
  2021-10-10  3:26     ` Oskar Senft
  2 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2021-10-09 23:40 UTC (permalink / raw)
  To: Oskar Senft, Jean Delvare, Rob Herring, linux-hwmon,
	linux-kernel, devicetree

Hi Oskar,

On 10/9/21 11:52 AM, Oskar Senft wrote:
> This change allows LTD and RTD inputs to be configured via
> device tree bindings. If the DT bindings are not present or
> invalid, the input configuration is not modified and left at
> HW defaults.
> 
> Signed-off-by: Oskar Senft <osk@google.com>
> ---

change log goes here (after ---), not in a separate e-mail.

>   drivers/hwmon/nct7802.c | 133 ++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 129 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
> index 604af2f6103a..e28f8eaf9f0f 100644
> --- a/drivers/hwmon/nct7802.c
> +++ b/drivers/hwmon/nct7802.c
> @@ -51,6 +51,23 @@ static const u8 REG_VOLTAGE_LIMIT_MSB_SHIFT[2][5] = {
>   #define REG_CHIP_ID		0xfe
>   #define REG_VERSION_ID		0xff
>   
> +/*
> + * Resistance temperature detector (RTD) modes according to 7.2.32 Mode
> + * Selection Register
> + */
> +#define RTD_MODE_CURRENT	0x1
> +#define RTD_MODE_THERMISTOR	0x2
> +#define RTD_MODE_VOLTAGE	0x3
> +
> +#define MODE_RTD_MASK		0x3
> +#define MODE_LTD_EN		0x40
> +
> +/*
> + * Bit offset for sensors modes in REG_MODE.
> + * Valid for index 0..2, indicating RTD1..3.
> + */
> +#define MODE_BIT_OFFSET_RTD(index) ((index) * 2)
> +
>   /*
>    * Data structures and manipulation thereof
>    */
> @@ -1038,7 +1055,116 @@ static const struct regmap_config nct7802_regmap_config = {
>   	.volatile_reg = nct7802_regmap_is_volatile,
>   };
>   
> -static int nct7802_init_chip(struct nct7802_data *data)
> +static int nct7802_get_channel_config(struct device *dev,
> +				      struct device_node *node, u8 *mode_mask,
> +				      u8 *mode_val)
> +{
> +	u32 reg;
> +	const char *type_str, *md_str;
> +	u8 md;
> +
> +	if (!node->name || of_node_cmp(node->name, "channel"))
> +		return 0;
> +
> +	if (of_property_read_u32(node, "reg", &reg)) {
> +		dev_err(dev, "Could not read reg value for '%s'\n",
> +			node->full_name);
> +		return -EINVAL;
> +	}
> +
> +	if (reg > 3) {
> +		dev_err(dev, "Invalid reg (%u) in '%s'\n", reg,
> +			node->full_name);
> +		return -EINVAL;
> +	}
> +
> +	if (reg == 0) {
> +		if (!of_device_is_available(node))
> +			*mode_val &= ~MODE_LTD_EN;
> +		else
> +			*mode_val |= MODE_LTD_EN;
> +		*mode_mask |= MODE_LTD_EN;
> +		return 0;
> +	}
> +
> +	/* At this point we have reg >= 1 && reg <= 3 */
> +
> +	if (!of_device_is_available(node)) {
> +		*mode_val &= ~(MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1));
> +		*mode_mask |= MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1);
> +		return 0;
> +	}
> +
> +	if (of_property_read_string(node, "sensor-type", &type_str)) {
> +		dev_err(dev, "No type for '%s'\n", node->full_name);
> +		return -EINVAL;
> +	}
> +
> +	if (!strcmp(type_str, "voltage")) {
> +		*mode_val |= (RTD_MODE_VOLTAGE & MODE_RTD_MASK)
> +			     << MODE_BIT_OFFSET_RTD(reg - 1);
> +		*mode_mask |= MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1);
> +		return 0;
> +	}
> +
> +	if (strcmp(type_str, "temperature")) {
> +		dev_err(dev, "Invalid type '%s' for '%s'\n", type_str,
> +			node->full_name);
> +		return -EINVAL;
> +	}
> +
> +	if (reg == 3) {
> +		/* RTD3 only supports thermistor mode */
> +		md = RTD_MODE_THERMISTOR;
> +	} else {
> +		if (of_property_read_string(node, "temperature-mode",
> +					    &md_str)) {
> +			dev_err(dev, "No mode for '%s'\n", node->full_name);
> +			return -EINVAL;
> +		}
> +
> +		if (!strcmp(md_str, "thermal-diode"))
> +			md = RTD_MODE_CURRENT;
> +		else if (!strcmp(md_str, "thermistor"))
> +			md = RTD_MODE_THERMISTOR;
> +		else {
> +			dev_err(dev, "Invalid mode '%s' for '%s'\n", md_str,
> +				node->full_name);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	*mode_val |= (md & MODE_RTD_MASK) << MODE_BIT_OFFSET_RTD(reg - 1);
> +	*mode_mask |= MODE_RTD_MASK << MODE_BIT_OFFSET_RTD(reg - 1);
> +
> +	return 0;
> +}
> +
> +static int nct7802_configure_channels(struct device *dev,
> +				      struct nct7802_data *data)
> +{
> +	bool found_channel_config = false;

now unused, as 0-day points out.

> +	u8 mode_mask = 0, mode_val = 0;
> +	struct device_node *node;
> +	int err;
> +
> +	/* Enable local temperature sensor by default */
> +	mode_val |= MODE_LTD_EN;
> +	mode_mask |= MODE_LTD_EN;
> +

Either make it = and drop the initialization above, or better
initialize the variables with MODE_LTD_EN right away.

> +	if (dev->of_node) {
> +		for_each_child_of_node(dev->of_node, node) {
> +			err = nct7802_get_channel_config(dev, node, &mode_mask,
> +							 &mode_val);
> +			if (err)
> +				return err;
> +		}
> +	}
> +
> +	return regmap_update_bits(data->regmap, REG_MODE, mode_mask, mode_val);
> +}
> +
> +static int nct7802_init_chip(struct device *dev, struct nct7802_data *data)
>   {
>   	int err;
>   
> @@ -1047,8 +1173,7 @@ static int nct7802_init_chip(struct nct7802_data *data)
>   	if (err)
>   		return err;
>   
> -	/* Enable local temperature sensor */
> -	err = regmap_update_bits(data->regmap, REG_MODE, 0x40, 0x40);
> +	err = nct7802_configure_channels(dev, data);
>   	if (err)
>   		return err;
>   
> @@ -1074,7 +1199,7 @@ static int nct7802_probe(struct i2c_client *client)
>   	mutex_init(&data->access_lock);
>   	mutex_init(&data->in_alarm_lock);
>   
> -	ret = nct7802_init_chip(data);
> +	ret = nct7802_init_chip(dev, data);
>   	if (ret < 0)
>   		return ret;
>   
> 


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

* Re: [PATCH v5 1/2] dt-bindings: hwmon: Add nct7802 bindings
  2021-10-09 18:52 [PATCH v5 1/2] dt-bindings: hwmon: Add nct7802 bindings Oskar Senft
  2021-10-09 18:52 ` [PATCH v5 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable Oskar Senft
  2021-10-09 18:53 ` [PATCH v5 1/2] dt-bindings: hwmon: Add nct7802 bindings Oskar Senft
@ 2021-10-09 23:46 ` Guenter Roeck
  2021-10-10  3:06   ` Oskar Senft
  2 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2021-10-09 23:46 UTC (permalink / raw)
  To: Oskar Senft, Jean Delvare, Rob Herring, linux-hwmon,
	linux-kernel, devicetree

On 10/9/21 11:52 AM, Oskar Senft wrote:
> This change documents the device tree bindings for the Nuvoton
> NCT7802Y driver.
> 
> Signed-off-by: Oskar Senft <osk@google.com>
> ---

change log goes here.

>   .../bindings/hwmon/nuvoton,nct7802.yaml       | 142 ++++++++++++++++++
>   1 file changed, 142 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/hwmon/nuvoton,nct7802.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/nuvoton,nct7802.yaml b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7802.yaml
> new file mode 100644
> index 000000000000..ff99f40034f2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7802.yaml
> @@ -0,0 +1,142 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/hwmon/nuvoton,nct7802.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nuvoton NCT7802Y Hardware Monitoring IC
> +
> +maintainers:
> +  - Guenter Roeck <linux@roeck-us.net>
> +
> +description: |
> +  The NCT7802Y is a hardware monitor IC which supports one on-die and up to
> +  5 remote temperature sensors with SMBus interface.
> +

Just noticed: 5 remote temperature sensors ? Shouldn't that be 3 ?

> +  Datasheets:
> +    https://www.nuvoton.com/export/resource-files/Nuvoton_NCT7802Y_Datasheet_V12.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - nuvoton,nct7802
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  channel@0:
> +    type: object
> +    description: Local Temperature Sensor ("LTD")
> +    properties:
> +      reg:
> +        const: 0
> +    required:
> +      - reg
> +
> +  channel@1:
> +    type: object
> +    description: Remote Temperature Sensor or Voltage Sensor ("RTD1")
> +    properties:
> +      reg:
> +        const: 1
> +      sensor-type:
> +        items:
> +          - enum:
> +              - temperature
> +              - voltage
> +      temperature-mode:
> +        items:
> +          - enum:
> +              - thermistor
> +              - thermal-diode
> +    required:
> +      - reg
> +      - sensor-type

If I understand correctly, "temperature-mode" is implemented as mandatory
for channels 1 and 2 if sensor-type is "temperature" (which makes sense).
No idea though if it is possible to express that in yaml.
If not, can it be mentioned as comment ?

> +
> +  channel@2:
> +    type: object
> +    description: Remote Temperature Sensor or Voltage Sensor ("RTD2")
> +    properties:
> +      reg:
> +        const: 2
> +      sensor-type:
> +        items:
> +          - enum:
> +              - temperature
> +              - voltage
> +      temperature-mode:
> +        items:
> +          - enum:
> +              - thermistor
> +              - thermal-diode
> +    required:
> +      - reg
> +      - sensor-type
> +
> +  channel@3:
> +    type: object
> +    description: Remote Temperature Sensor or Voltage Sensor ("RTD3")
> +    properties:
> +      reg:
> +        const: 3
> +      sensor-type:
> +        items:
> +          - enum:
> +              - temperature
> +              - voltage
> +    required:
> +      - reg
> +      - sensor-type
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        nct7802@28 {
> +            compatible = "nuvoton,nct7802";
> +            reg = <0x28>;
> +
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            channel@0 { /* LTD */
> +              reg = <0>;
> +              status = "okay";
> +            };
> +
> +            channel@1 { /* RTD1 */
> +              reg = <1>;
> +              status = "okay";
> +              sensor-type = "temperature";
> +              temperature-mode = "thermistor";
> +            };
> +
> +            channel@2 { /* RTD2 */
> +              reg = <2>;
> +              status = "okay";
> +              sensor-type = "temperature";
> +              temperature-mode = "thermal-diode";
> +            };
> +
> +            channel@3 { /* RTD3 */
> +              reg = <3>;
> +              status = "okay";
> +              sensor-type = "voltage";
> +            };
> +        };
> +    };
> 


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

* Re: [PATCH v5 1/2] dt-bindings: hwmon: Add nct7802 bindings
  2021-10-09 23:46 ` Guenter Roeck
@ 2021-10-10  3:06   ` Oskar Senft
  2021-10-10  4:10     ` Guenter Roeck
  0 siblings, 1 reply; 10+ messages in thread
From: Oskar Senft @ 2021-10-10  3:06 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Jean Delvare, Rob Herring, linux-hwmon, linux-kernel, devicetree

Hi Guenter

Thanks again for your review!

> > Signed-off-by: Oskar Senft <osk@google.com>
> > ---
>
> change log goes here.
This might be a silly question: I'm using "git send-email", but I
don't think there's a way to edit the e-mail before it goes out. Do I
just add "---\n[Change log]" manually in the commit description?

> > +description: |
> > +  The NCT7802Y is a hardware monitor IC which supports one on-die and up to
> > +  5 remote temperature sensors with SMBus interface.
> > +
>
> Just noticed: 5 remote temperature sensors ? Shouldn't that be 3 ?
This includes 2 temperature sensors that are queried via PECI (i.e.
SMBus). I generated the description from the "general description"
section in the datasheet. I think the driver doesn't implement the 2
PECI sensors at this time, but the statement about the HW is still
true.

> > +      sensor-type:
> > +        items:
> > +          - enum:
> > +              - temperature
> > +              - voltage
> > +      temperature-mode:
> > +        items:
> > +          - enum:
> > +              - thermistor
> > +              - thermal-diode
> > +    required:
> > +      - reg
> > +      - sensor-type
>
> If I understand correctly, "temperature-mode" is implemented as mandatory
> for channels 1 and 2 if sensor-type is "temperature" (which makes sense).
> No idea though if it is possible to express that in yaml.
> If not, can it be mentioned as comment ?

After doing a bit more searching, I found the amazing "if: then:
else:" construct that allows to express this properly and eliminates
the code duplication. I'll follow up in PATCH v6.

Thanks
Oskar.



>
> > +
> > +  channel@2:
> > +    type: object
> > +    description: Remote Temperature Sensor or Voltage Sensor ("RTD2")
> > +    properties:
> > +      reg:
> > +        const: 2
> > +      sensor-type:
> > +        items:
> > +          - enum:
> > +              - temperature
> > +              - voltage
> > +      temperature-mode:
> > +        items:
> > +          - enum:
> > +              - thermistor
> > +              - thermal-diode
> > +    required:
> > +      - reg
> > +      - sensor-type
> > +
> > +  channel@3:
> > +    type: object
> > +    description: Remote Temperature Sensor or Voltage Sensor ("RTD3")
> > +    properties:
> > +      reg:
> > +        const: 3
> > +      sensor-type:
> > +        items:
> > +          - enum:
> > +              - temperature
> > +              - voltage
> > +    required:
> > +      - reg
> > +      - sensor-type
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        nct7802@28 {
> > +            compatible = "nuvoton,nct7802";
> > +            reg = <0x28>;
> > +
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            channel@0 { /* LTD */
> > +              reg = <0>;
> > +              status = "okay";
> > +            };
> > +
> > +            channel@1 { /* RTD1 */
> > +              reg = <1>;
> > +              status = "okay";
> > +              sensor-type = "temperature";
> > +              temperature-mode = "thermistor";
> > +            };
> > +
> > +            channel@2 { /* RTD2 */
> > +              reg = <2>;
> > +              status = "okay";
> > +              sensor-type = "temperature";
> > +              temperature-mode = "thermal-diode";
> > +            };
> > +
> > +            channel@3 { /* RTD3 */
> > +              reg = <3>;
> > +              status = "okay";
> > +              sensor-type = "voltage";
> > +            };
> > +        };
> > +    };
> >
>

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

* Re: [PATCH v5 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable
  2021-10-09 23:40   ` Guenter Roeck
@ 2021-10-10  3:26     ` Oskar Senft
  0 siblings, 0 replies; 10+ messages in thread
From: Oskar Senft @ 2021-10-10  3:26 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Jean Delvare, Rob Herring, linux-hwmon, linux-kernel, devicetree

Hi Guenter

Thanks again for the review!

> > +     bool found_channel_config = false;
>
> now unused, as 0-day points out.
Argh, I'm so sorry. I don't understand why building this for OpenBMC
didn't flag that up. I thought building with warnings as errors was
now the default? But obviously not :-/

> > +     /* Enable local temperature sensor by default */
> > +     mode_val |= MODE_LTD_EN;
> > +     mode_mask |= MODE_LTD_EN;
> > +
>
> Either make it = and drop the initialization above, or better
> initialize the variables with MODE_LTD_EN right away.
Oh yeah, makes sense. Will do in V6.

Thanks
Oskar.

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

* Re: [PATCH v5 1/2] dt-bindings: hwmon: Add nct7802 bindings
  2021-10-10  3:06   ` Oskar Senft
@ 2021-10-10  4:10     ` Guenter Roeck
  0 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2021-10-10  4:10 UTC (permalink / raw)
  To: Oskar Senft
  Cc: Jean Delvare, Rob Herring, linux-hwmon, linux-kernel, devicetree

On 10/9/21 8:06 PM, Oskar Senft wrote:
> Hi Guenter
> 
> Thanks again for your review!
> 
>>> Signed-off-by: Oskar Senft <osk@google.com>
>>> ---
>>
>> change log goes here.
> This might be a silly question: I'm using "git send-email", but I
> don't think there's a way to edit the e-mail before it goes out. Do I
> just add "---\n[Change log]" manually in the commit description?
> 

When you use git send-email, you usually have a patch file to send.
I use git format-patch to create that patch file, add the change
log using an editor, and then send it with git send-email.

>>> +description: |
>>> +  The NCT7802Y is a hardware monitor IC which supports one on-die and up to
>>> +  5 remote temperature sensors with SMBus interface.
>>> +
>>
>> Just noticed: 5 remote temperature sensors ? Shouldn't that be 3 ?
> This includes 2 temperature sensors that are queried via PECI (i.e.
> SMBus). I generated the description from the "general description"
> section in the datasheet. I think the driver doesn't implement the 2
> PECI sensors at this time, but the statement about the HW is still
> true.
> 

Ok, make sense.

Thanks,
Guenter

>>> +      sensor-type:
>>> +        items:
>>> +          - enum:
>>> +              - temperature
>>> +              - voltage
>>> +      temperature-mode:
>>> +        items:
>>> +          - enum:
>>> +              - thermistor
>>> +              - thermal-diode
>>> +    required:
>>> +      - reg
>>> +      - sensor-type
>>
>> If I understand correctly, "temperature-mode" is implemented as mandatory
>> for channels 1 and 2 if sensor-type is "temperature" (which makes sense).
>> No idea though if it is possible to express that in yaml.
>> If not, can it be mentioned as comment ?
> 
> After doing a bit more searching, I found the amazing "if: then:
> else:" construct that allows to express this properly and eliminates
> the code duplication. I'll follow up in PATCH v6.
> 
> Thanks
> Oskar.
> 
> 
> 
>>
>>> +
>>> +  channel@2:
>>> +    type: object
>>> +    description: Remote Temperature Sensor or Voltage Sensor ("RTD2")
>>> +    properties:
>>> +      reg:
>>> +        const: 2
>>> +      sensor-type:
>>> +        items:
>>> +          - enum:
>>> +              - temperature
>>> +              - voltage
>>> +      temperature-mode:
>>> +        items:
>>> +          - enum:
>>> +              - thermistor
>>> +              - thermal-diode
>>> +    required:
>>> +      - reg
>>> +      - sensor-type
>>> +
>>> +  channel@3:
>>> +    type: object
>>> +    description: Remote Temperature Sensor or Voltage Sensor ("RTD3")
>>> +    properties:
>>> +      reg:
>>> +        const: 3
>>> +      sensor-type:
>>> +        items:
>>> +          - enum:
>>> +              - temperature
>>> +              - voltage
>>> +    required:
>>> +      - reg
>>> +      - sensor-type
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    i2c {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +
>>> +        nct7802@28 {
>>> +            compatible = "nuvoton,nct7802";
>>> +            reg = <0x28>;
>>> +
>>> +            #address-cells = <1>;
>>> +            #size-cells = <0>;
>>> +
>>> +            channel@0 { /* LTD */
>>> +              reg = <0>;
>>> +              status = "okay";
>>> +            };
>>> +
>>> +            channel@1 { /* RTD1 */
>>> +              reg = <1>;
>>> +              status = "okay";
>>> +              sensor-type = "temperature";
>>> +              temperature-mode = "thermistor";
>>> +            };
>>> +
>>> +            channel@2 { /* RTD2 */
>>> +              reg = <2>;
>>> +              status = "okay";
>>> +              sensor-type = "temperature";
>>> +              temperature-mode = "thermal-diode";
>>> +            };
>>> +
>>> +            channel@3 { /* RTD3 */
>>> +              reg = <3>;
>>> +              status = "okay";
>>> +              sensor-type = "voltage";
>>> +            };
>>> +        };
>>> +    };
>>>
>>


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

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

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-09 18:52 [PATCH v5 1/2] dt-bindings: hwmon: Add nct7802 bindings Oskar Senft
2021-10-09 18:52 ` [PATCH v5 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable Oskar Senft
2021-10-09 18:53   ` Oskar Senft
2021-10-09 22:40   ` kernel test robot
2021-10-09 23:40   ` Guenter Roeck
2021-10-10  3:26     ` Oskar Senft
2021-10-09 18:53 ` [PATCH v5 1/2] dt-bindings: hwmon: Add nct7802 bindings Oskar Senft
2021-10-09 23:46 ` Guenter Roeck
2021-10-10  3:06   ` Oskar Senft
2021-10-10  4:10     ` Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).