This change documents the device tree bindings for the Nuvoton
NCT7802Y driver.
Signed-off-by: Oskar Senft <[email protected]>
---
.../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 <[email protected]>
+
+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
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 <[email protected]> wrote:
>
> This change documents the device tree bindings for the Nuvoton
> NCT7802Y driver.
>
> Signed-off-by: Oskar Senft <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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
>
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 <[email protected]>
---
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", ®)) {
+ 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
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 <[email protected]> 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 <[email protected]>
> ---
> 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", ®)) {
> + 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
>
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 <[email protected]>
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/[email protected]
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 <[email protected]>
> ---
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", ®)) {
> + 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;
>
>
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 <[email protected]>
> ---
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 <[email protected]>
> +
> +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";
> + };
> + };
> + };
>
Hi Guenter
Thanks again for your review!
> > Signed-off-by: Oskar Senft <[email protected]>
> > ---
>
> 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";
> > + };
> > + };
> > + };
> >
>
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.
On 10/9/21 8:06 PM, Oskar Senft wrote:
> Hi Guenter
>
> Thanks again for your review!
>
>>> Signed-off-by: Oskar Senft <[email protected]>
>>> ---
>>
>> 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";
>>> + };
>>> + };
>>> + };
>>>
>>