linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 1/2] dt-bindings: hwmon: Add nct7802 bindings
@ 2021-10-11  1:22 Oskar Senft
  2021-10-11  1:22 ` [PATCH v7 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable Oskar Senft
  2021-10-18 16:50 ` [PATCH v7 1/2] dt-bindings: hwmon: Add nct7802 bindings Rob Herring
  0 siblings, 2 replies; 10+ messages in thread
From: Oskar Senft @ 2021-10-11  1:22 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>
---
Changes from PATCH v6:
- Fixed formatting error reported by yamllint

Changes from PATCH v5:
- Refactored to use patternProperties.
- Added validation for sensor-type and temperature-mode.
---
 .../bindings/hwmon/nuvoton,nct7802.yaml       | 144 ++++++++++++++++++
 1 file changed, 144 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..a0a245938528
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7802.yaml
@@ -0,0 +1,144 @@
+# 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
+
+patternProperties:
+  "^channel@[0-3]$":
+    type: object
+    properties:
+      reg:
+        items:
+          - enum:
+              - 0    # Local Temperature Sensor ("LTD")
+              - 1    # Remote Temperature Sensor or Voltage Sensor 1 ("RTD1")
+              - 2    # Remote Temperature Sensor or Voltage Sensor 2 ("RTD2")
+              - 3    # Remote Temperature Sensor or Voltage Sensor 3 ("RTD3")
+      sensor-type:
+        items:
+          - enum:
+              - temperature
+              - voltage
+      temperature-mode:
+        items:
+          - enum:
+              - thermistor
+              - thermal-diode
+    required:
+      - reg
+    allOf:
+      # For channels RTD1, RTD2 and RTD3, require sensor-type to be set.
+      # Otherwise (for all other channels), do not allow temperature-mode to be
+      # set.
+      - if:
+          properties:
+            reg:
+              items:
+                - enum:
+                    - 1
+                    - 2
+                    - 3
+        then:
+          required:
+            - sensor-type
+        else:
+          not:
+            required:
+              - sensor-type
+
+      # For channels RTD1 and RTD2 and if sensor-type is "temperature", require
+      # temperature-mode to be set. Otherwise (for all other channels or
+      # sensor-type settings), do not allow temperature-mode to be set
+      - if:
+          properties:
+            reg:
+              items:
+                - enum:
+                    - 1
+                    - 2
+            sensor-type:
+              items:
+                - enum:
+                    - temperature
+        then:
+          required:
+            - temperature-mode
+        else:
+          not:
+            required:
+              - temperature-mode
+
+    additionalProperties: false
+
+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 = "voltage";
+            };
+
+            channel@2 { /* RTD2 */
+              reg = <2>;
+              status = "okay";
+              sensor-type = "temperature";
+              temperature-mode = "thermal-diode";
+            };
+
+            channel@3 { /* RTD3 */
+              reg = <3>;
+              status = "okay";
+              sensor-type = "temperature";
+            };
+        };
+    };
-- 
2.33.0.882.g93a45727a2-goog


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

* [PATCH v7 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable
  2021-10-11  1:22 [PATCH v7 1/2] dt-bindings: hwmon: Add nct7802 bindings Oskar Senft
@ 2021-10-11  1:22 ` Oskar Senft
  2021-10-11  1:49   ` Guenter Roeck
  2021-10-11 14:05   ` Guenter Roeck
  2021-10-18 16:50 ` [PATCH v7 1/2] dt-bindings: hwmon: Add nct7802 bindings Rob Herring
  1 sibling, 2 replies; 10+ messages in thread
From: Oskar Senft @ 2021-10-11  1:22 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>
---
Changes from PATCH v6:
- None (resubmitted due to changes in nuvoton,nct7802.yaml).

Changes from PATCH v5:
- Removed unused "found_channel_config" variable.
- Initialize mode_mask and mode_val to defaults.
---
 drivers/hwmon/nct7802.c | 129 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 125 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
index 604af2f6103a..d56f78327619 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,112 @@ 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)
+{
+	/* Enable local temperature sensor by default */
+	u8 mode_mask = MODE_LTD_EN, mode_val = MODE_LTD_EN;
+	struct device_node *node;
+	int err;
+
+	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 +1169,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 +1195,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 v7 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable
  2021-10-11  1:22 ` [PATCH v7 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable Oskar Senft
@ 2021-10-11  1:49   ` Guenter Roeck
  2021-10-11  1:53     ` Oskar Senft
  2021-10-11 14:05   ` Guenter Roeck
  1 sibling, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2021-10-11  1:49 UTC (permalink / raw)
  To: Oskar Senft, Jean Delvare, Rob Herring, linux-hwmon,
	linux-kernel, devicetree

On 10/10/21 6:22 PM, 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>

I sent a Reviewed-by: for v6 of this patch.

> ---
> Changes from PATCH v6:
> - None (resubmitted due to changes in nuvoton,nct7802.yaml).
> 

Why did you drop it if there was no change ?

Guenter

> Changes from PATCH v5:
> - Removed unused "found_channel_config" variable.
> - Initialize mode_mask and mode_val to defaults.
> ---
>   drivers/hwmon/nct7802.c | 129 ++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 125 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
> index 604af2f6103a..d56f78327619 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,112 @@ 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)
> +{
> +	/* Enable local temperature sensor by default */
> +	u8 mode_mask = MODE_LTD_EN, mode_val = MODE_LTD_EN;
> +	struct device_node *node;
> +	int err;
> +
> +	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 +1169,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 +1195,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 v7 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable
  2021-10-11  1:49   ` Guenter Roeck
@ 2021-10-11  1:53     ` Oskar Senft
  2021-10-11  4:08       ` Guenter Roeck
  0 siblings, 1 reply; 10+ messages in thread
From: Oskar Senft @ 2021-10-11  1:53 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Jean Delvare, Rob Herring, linux-hwmon, linux-kernel, devicetree

I wasn't sure if submitting JUST a v7 of nuvoton,nct7802.yaml (i.e.
the 1/2 in this series) would be the right thing to do and thought it
would be easier to follow if I dropped both. I couldn't find
documentation on what's the right thing to do in that case. Would it
have been better to only submit nuvoton,nct7802.yaml as "PATCH v7" ?

Oskar.

On Sun, Oct 10, 2021 at 9:49 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 10/10/21 6:22 PM, 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>
>
> I sent a Reviewed-by: for v6 of this patch.
>
> > ---
> > Changes from PATCH v6:
> > - None (resubmitted due to changes in nuvoton,nct7802.yaml).
> >
>
> Why did you drop it if there was no change ?
>
> Guenter
>
> > Changes from PATCH v5:
> > - Removed unused "found_channel_config" variable.
> > - Initialize mode_mask and mode_val to defaults.
> > ---
> >   drivers/hwmon/nct7802.c | 129 ++++++++++++++++++++++++++++++++++++++--
> >   1 file changed, 125 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
> > index 604af2f6103a..d56f78327619 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,112 @@ 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)
> > +{
> > +     /* Enable local temperature sensor by default */
> > +     u8 mode_mask = MODE_LTD_EN, mode_val = MODE_LTD_EN;
> > +     struct device_node *node;
> > +     int err;
> > +
> > +     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 +1169,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 +1195,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 v7 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable
  2021-10-11  1:53     ` Oskar Senft
@ 2021-10-11  4:08       ` Guenter Roeck
  2021-10-11 12:57         ` Oskar Senft
  0 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2021-10-11  4:08 UTC (permalink / raw)
  To: Oskar Senft
  Cc: Jean Delvare, Rob Herring, linux-hwmon, linux-kernel, devicetree

On 10/10/21 6:53 PM, Oskar Senft wrote:
> I wasn't sure if submitting JUST a v7 of nuvoton,nct7802.yaml (i.e.
> the 1/2 in this series) would be the right thing to do and thought it
> would be easier to follow if I dropped both. I couldn't find
> documentation on what's the right thing to do in that case. Would it
> have been better to only submit nuvoton,nct7802.yaml as "PATCH v7" ?
> 

First, please don't top-post.

Second, no, submitting both patches was the right thing to do, but
you should not drop a Reviewed-by: tag if there was no change to
the patch.

Guenter

> Oskar.
> 
> On Sun, Oct 10, 2021 at 9:49 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 10/10/21 6:22 PM, 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>
>>
>> I sent a Reviewed-by: for v6 of this patch.
>>
>>> ---
>>> Changes from PATCH v6:
>>> - None (resubmitted due to changes in nuvoton,nct7802.yaml).
>>>
>>
>> Why did you drop it if there was no change ?
>>
>> Guenter
>>
>>> Changes from PATCH v5:
>>> - Removed unused "found_channel_config" variable.
>>> - Initialize mode_mask and mode_val to defaults.
>>> ---
>>>    drivers/hwmon/nct7802.c | 129 ++++++++++++++++++++++++++++++++++++++--
>>>    1 file changed, 125 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
>>> index 604af2f6103a..d56f78327619 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,112 @@ 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)
>>> +{
>>> +     /* Enable local temperature sensor by default */
>>> +     u8 mode_mask = MODE_LTD_EN, mode_val = MODE_LTD_EN;
>>> +     struct device_node *node;
>>> +     int err;
>>> +
>>> +     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 +1169,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 +1195,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 v7 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable
  2021-10-11  4:08       ` Guenter Roeck
@ 2021-10-11 12:57         ` Oskar Senft
  2021-10-11 14:03           ` Guenter Roeck
  0 siblings, 1 reply; 10+ messages in thread
From: Oskar Senft @ 2021-10-11 12:57 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Jean Delvare, Rob Herring, linux-hwmon, linux-kernel, devicetree

Hi Guenter

I appreciate your help with this!

> First, please don't top-post.
Argh, I'm sorry - old habits! The Gmail web UI makes it too easy to do that :-/

> Second, no, submitting both patches was the right thing to do, but
> you should not drop a Reviewed-by: tag if there was no change to
> the patch.
Got it, makes sense. How do I untangle this mess? Resubmit a v8, this
time with the "Reviewed-by:" added back in? Or are you ok tagging the
v7 again. I'm sorry!

Thanks
Oskar.

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

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

On 10/11/21 5:57 AM, Oskar Senft wrote:
> Hi Guenter
> 
> I appreciate your help with this!
> 
>> First, please don't top-post.
> Argh, I'm sorry - old habits! The Gmail web UI makes it too easy to do that :-/
> 
>> Second, no, submitting both patches was the right thing to do, but
>> you should not drop a Reviewed-by: tag if there was no change to
>> the patch.
> Got it, makes sense. How do I untangle this mess? Resubmit a v8, this
> time with the "Reviewed-by:" added back in? Or are you ok tagging the
> v7 again. I'm sorry!
> 

I'll tag again. No worries, just keep it in mind for next time.

Thanks,
Guenter


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

* Re: [PATCH v7 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable
  2021-10-11  1:22 ` [PATCH v7 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable Oskar Senft
  2021-10-11  1:49   ` Guenter Roeck
@ 2021-10-11 14:05   ` Guenter Roeck
  1 sibling, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2021-10-11 14:05 UTC (permalink / raw)
  To: Oskar Senft, Jean Delvare, Rob Herring, linux-hwmon,
	linux-kernel, devicetree

On 10/10/21 6:22 PM, 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>

Again, for my reference:

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

> ---
> Changes from PATCH v6:
> - None (resubmitted due to changes in nuvoton,nct7802.yaml).
> 
> Changes from PATCH v5:
> - Removed unused "found_channel_config" variable.
> - Initialize mode_mask and mode_val to defaults.
> ---
>   drivers/hwmon/nct7802.c | 129 ++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 125 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
> index 604af2f6103a..d56f78327619 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,112 @@ 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)
> +{
> +	/* Enable local temperature sensor by default */
> +	u8 mode_mask = MODE_LTD_EN, mode_val = MODE_LTD_EN;
> +	struct device_node *node;
> +	int err;
> +
> +	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 +1169,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 +1195,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 v7 1/2] dt-bindings: hwmon: Add nct7802 bindings
  2021-10-11  1:22 [PATCH v7 1/2] dt-bindings: hwmon: Add nct7802 bindings Oskar Senft
  2021-10-11  1:22 ` [PATCH v7 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable Oskar Senft
@ 2021-10-18 16:50 ` Rob Herring
  2021-10-20 16:49   ` Oskar Senft
  1 sibling, 1 reply; 10+ messages in thread
From: Rob Herring @ 2021-10-18 16:50 UTC (permalink / raw)
  To: Oskar Senft
  Cc: Jean Delvare, Guenter Roeck, linux-hwmon, linux-kernel, devicetree

On Sun, Oct 10, 2021 at 09:22:11PM -0400, Oskar Senft wrote:
> This change documents the device tree bindings for the Nuvoton
> NCT7802Y driver.
> 
> Signed-off-by: Oskar Senft <osk@google.com>
> ---
> Changes from PATCH v6:
> - Fixed formatting error reported by yamllint
> 
> Changes from PATCH v5:
> - Refactored to use patternProperties.
> - Added validation for sensor-type and temperature-mode.
> ---
>  .../bindings/hwmon/nuvoton,nct7802.yaml       | 144 ++++++++++++++++++
>  1 file changed, 144 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..a0a245938528
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/nuvoton,nct7802.yaml
> @@ -0,0 +1,144 @@
> +# 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
> +
> +patternProperties:
> +  "^channel@[0-3]$":
> +    type: object

I would move the 'additionalProperties' here. I think that's a bit 
easier to read.

> +    properties:
> +      reg:
> +        items:
> +          - enum:
> +              - 0    # Local Temperature Sensor ("LTD")
> +              - 1    # Remote Temperature Sensor or Voltage Sensor 1 ("RTD1")
> +              - 2    # Remote Temperature Sensor or Voltage Sensor 2 ("RTD2")
> +              - 3    # Remote Temperature Sensor or Voltage Sensor 3 ("RTD3")

blank line

> +      sensor-type:
> +        items:
> +          - enum:
> +              - temperature
> +              - voltage

blank line

> +      temperature-mode:
> +        items:
> +          - enum:
> +              - thermistor
> +              - thermal-diode

blank line

> +    required:
> +      - reg

blank line

> +    allOf:
> +      # For channels RTD1, RTD2 and RTD3, require sensor-type to be set.
> +      # Otherwise (for all other channels), do not allow temperature-mode to be
> +      # set.
> +      - if:
> +          properties:
> +            reg:
> +              items:
> +                - enum:
> +                    - 1
> +                    - 2
> +                    - 3
> +        then:
> +          required:
> +            - sensor-type
> +        else:
> +          not:
> +            required:
> +              - sensor-type
> +
> +      # For channels RTD1 and RTD2 and if sensor-type is "temperature", require
> +      # temperature-mode to be set. Otherwise (for all other channels or
> +      # sensor-type settings), do not allow temperature-mode to be set
> +      - if:
> +          properties:
> +            reg:
> +              items:
> +                - enum:
> +                    - 1
> +                    - 2
> +            sensor-type:
> +              items:
> +                - enum:
> +                    - temperature
> +        then:
> +          required:
> +            - temperature-mode
> +        else:
> +          not:
> +            required:
> +              - temperature-mode
> +
> +    additionalProperties: false
> +
> +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";

Don't show status in examples.

> +            };
> +
> +            channel@1 { /* RTD1 */
> +              reg = <1>;
> +              status = "okay";
> +              sensor-type = "voltage";
> +            };
> +
> +            channel@2 { /* RTD2 */
> +              reg = <2>;
> +              status = "okay";
> +              sensor-type = "temperature";
> +              temperature-mode = "thermal-diode";
> +            };
> +
> +            channel@3 { /* RTD3 */
> +              reg = <3>;
> +              status = "okay";
> +              sensor-type = "temperature";
> +            };
> +        };
> +    };
> -- 
> 2.33.0.882.g93a45727a2-goog
> 
> 

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

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

Hi Rob

Thanks for the review comments!

> > +patternProperties:
> > +  "^channel@[0-3]$":
> > +    type: object
>
> I would move the 'additionalProperties' here. I think that's a bit
> easier to read.

Following the same thought, I also moved the 'additionalProperties' on
the top level up to above its 'properties'.

> blank line

Thanks, I added the missing blank lines. I'm not sure what I was thinking.

> > +            channel@0 { /* LTD */
> > +              reg = <0>;
> > +              status = "okay";
>
> Don't show status in examples.

Oh yes, you mentioned that before, I'm sorry. They're removed now.

I sent a PATCH v8 with the changes.

Oskar.

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

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

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-11  1:22 [PATCH v7 1/2] dt-bindings: hwmon: Add nct7802 bindings Oskar Senft
2021-10-11  1:22 ` [PATCH v7 2/2] hwmon: (nct7802) Make temperature/voltage sensors configurable Oskar Senft
2021-10-11  1:49   ` Guenter Roeck
2021-10-11  1:53     ` Oskar Senft
2021-10-11  4:08       ` Guenter Roeck
2021-10-11 12:57         ` Oskar Senft
2021-10-11 14:03           ` Guenter Roeck
2021-10-11 14:05   ` Guenter Roeck
2021-10-18 16:50 ` [PATCH v7 1/2] dt-bindings: hwmon: Add nct7802 bindings Rob Herring
2021-10-20 16:49   ` Oskar Senft

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).