[PATCH v2] hwmon: (lm75) Add support for Nuvoton NCT7715

[PATCH v2] hwmon: (lm75) Add support for Nuvoton NCT7715

[hsyemail2]

From: Sheng-Yuan Huang <syhuang3@nuvoton.com>

The Nuvoton NCT7715 is compatible with the LM75 temperature and
limit register layout. Add support for the NCT7715, including its
16-bit configuration register.

Handle its conversion-rate field and shutdown bit separately, since
their bit positions differ from the standard LM75 layout.

Signed-off-by: Sheng-Yuan Huang <syhuang3@nuvoton.com>
---
v2:
- Use the standard SMBus word representation for the NCT7715
  configuration register, removing the NCT7715-specific byte swapping
  and updating its configuration masks accordingly.

 .../devicetree/bindings/hwmon/lm75.yaml       |  1 +
 Documentation/hwmon/lm75.rst                  |  6 +++
 drivers/hwmon/lm75.c                          | 52 ++++++++++++++++++-
 3 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
index b48bf3fd721f..832a8c5d83e9 100644
--- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
+++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml
@@ -31,6 +31,7 @@ properties:
       - nxp,p3t1750
       - nxp,p3t1755
       - nxp,pct2075
+      - nuvoton,nct7715
       - st,stds75
       - st,stlm75
       - microchip,tcn75
diff --git a/Documentation/hwmon/lm75.rst b/Documentation/hwmon/lm75.rst
index ca46754e028b..fac0b8f29ddc 100644
--- a/Documentation/hwmon/lm75.rst
+++ b/Documentation/hwmon/lm75.rst
@@ -150,6 +150,12 @@ Supported chips:
 
                https://ams.com/documents/20143/36005/AS6200_DS000449_4-00.pdf
 
+  * Nuvoton NCT7715
+
+    Prefix: 'nct7715'
+
+    Addresses scanned: none
+
 Author: Frodo Looijaard <frodol@dds.nl>
 
 Description
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 2d2d752aeac9..ad7128629e56 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -39,6 +39,7 @@ enum lm75_type {		/* keep sorted in alphabetical order */
 	max6626,
 	max31725,
 	mcp980x,
+	nct7715,
 	p3t1750,
 	p3t1755,
 	pct2075,
@@ -107,6 +108,15 @@ static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b, 0x4c,
 #define LM75_REG_MAX		0x03
 #define PCT2075_REG_IDLE	0x04
 
+#define NCT7715_CONFIG_RESOLUTION_12_BIT	GENMASK(6, 5)
+#define NCT7715_CONFIG_SHUTDOWN			BIT(0)
+#define NCT7715_CONFIG_CONVERSION_RATE_MASK	GENMASK(15, 14)
+#define NCT7715_CONFIG_EXTENDED_MODE		BIT(12)
+#define NCT7715_CONFIG_RATE_0_25HZ		0
+#define NCT7715_CONFIG_RATE_1HZ			BIT(14)
+#define NCT7715_CONFIG_RATE_4HZ			BIT(15)
+#define NCT7715_CONFIG_RATE_8HZ			GENMASK(15, 14)
+
 struct lm75_data {
 	const char *label;
 	struct regmap			*regmap;
@@ -122,6 +132,10 @@ struct lm75_data {
 /*-----------------------------------------------------------------------*/
 
 static const u8 lm75_sample_set_masks[] = { 0 << 5, 1 << 5, 2 << 5, 3 << 5 };
+static const u16 nct7715_sample_set_masks[] = {
+	NCT7715_CONFIG_RATE_8HZ, NCT7715_CONFIG_RATE_4HZ,
+	NCT7715_CONFIG_RATE_1HZ, NCT7715_CONFIG_RATE_0_25HZ
+};
 
 #define LM75_ALERT_POLARITY_HIGH_8_BIT	(BIT(2))
 #define LM75_ALERT_POLARITY_HIGH_16_BIT	(BIT(2) << 8)
@@ -259,6 +273,15 @@ static const struct lm75_params device_params[] = {
 		.sample_times = (unsigned int []){ 30, 60, 120, 240 },
 		.resolutions = (u8 []) {9, 10, 11, 12 },
 	},
+	[nct7715] = {
+		.config_reg_16bits = true,
+		.set_mask = NCT7715_CONFIG_RESOLUTION_12_BIT,
+		.clr_mask = NCT7715_CONFIG_EXTENDED_MODE,
+		.default_resolution = 12,
+		.default_sample_time = 250,
+		.num_sample_times = 4,
+		.sample_times = (unsigned int []){ 125, 250, 1000, 4000 },
+	},
 	[tmp100] = {
 		.set_mask = 3 << 5,	/* 12-bit mode */
 		.clr_mask = 1 << 7,	/* not one-shot mode */
@@ -354,6 +377,11 @@ static inline long lm75_reg_to_mc(s16 temp, u8 resolution)
 static inline int lm75_write_config(struct lm75_data *data, u16 set_mask,
 				    u16 clr_mask)
 {
+	if (data->kind == nct7715)
+		return regmap_update_bits(data->regmap, LM75_REG_CONF,
+					  clr_mask | set_mask | NCT7715_CONFIG_SHUTDOWN,
+					  set_mask);
+
 	return regmap_update_bits(data->regmap, LM75_REG_CONF,
 				  clr_mask | set_mask | LM75_SHUTDOWN, set_mask);
 }
@@ -486,6 +514,13 @@ static int lm75_update_interval(struct device *dev, long val)
 		if (data->params->resolutions)
 			data->resolution = data->params->resolutions[index];
 		break;
+	case nct7715:
+		err = lm75_write_config(data, nct7715_sample_set_masks[index],
+					NCT7715_CONFIG_CONVERSION_RATE_MASK);
+		if (err)
+			return err;
+		data->sample_time = data->params->sample_times[index];
+		break;
 	case tmp112:
 	case as6200:
 		err = regmap_update_bits(data->regmap, LM75_REG_CONF,
@@ -770,7 +805,7 @@ static int lm75_generic_probe(struct device *dev, const char *name,
 	data->orig_conf = status;
 
 	/* Enforce polarity active-low (default) or active-high (devicetree) */
-	if (!data->params->config_reg_16bits)
+	if (!data->params->config_reg_16bits || data->kind == nct7715)
 		pol_mask = LM75_ALERT_POLARITY_HIGH_8_BIT;
 	else
 		pol_mask = LM75_ALERT_POLARITY_HIGH_16_BIT;
@@ -848,6 +883,7 @@ static const struct i2c_device_id lm75_i2c_ids[] = {
 	{ .name = "max31725", .driver_data = max31725 },
 	{ .name = "max31726", .driver_data = max31725 },
 	{ .name = "mcp980x", .driver_data = mcp980x },
+	{ .name = "nct7715", .driver_data = nct7715 },
 	{ .name = "p3t1750", .driver_data = p3t1750 },
 	{ .name = "p3t1755", .driver_data = p3t1755 },
 	{ .name = "pct2075", .driver_data = pct2075 },
@@ -972,6 +1008,10 @@ static const struct of_device_id lm75_of_match[] = {
 		.compatible = "nxp,pct2075",
 		.data = (void *)pct2075
 	},
+	{
+		.compatible = "nuvoton,nct7715",
+		.data = (void *)nct7715
+	},
 	{
 		.compatible = "st,stds75",
 		.data = (void *)stds75
@@ -1129,13 +1169,21 @@ static int lm75_suspend(struct device *dev)
 {
 	struct lm75_data *data = dev_get_drvdata(dev);
 
-	return regmap_update_bits(data->regmap, LM75_REG_CONF, LM75_SHUTDOWN, LM75_SHUTDOWN);
+	if (data->kind == nct7715)
+		return regmap_update_bits(data->regmap, LM75_REG_CONF,
+					  NCT7715_CONFIG_SHUTDOWN,
+					  NCT7715_CONFIG_SHUTDOWN);
+	return regmap_update_bits(data->regmap, LM75_REG_CONF,
+				  LM75_SHUTDOWN, LM75_SHUTDOWN);
 }
 
 static int lm75_resume(struct device *dev)
 {
 	struct lm75_data *data = dev_get_drvdata(dev);
 
+	if (data->kind == nct7715)
+		return regmap_update_bits(data->regmap, LM75_REG_CONF,
+					  NCT7715_CONFIG_SHUTDOWN, 0);
 	return regmap_update_bits(data->regmap, LM75_REG_CONF, LM75_SHUTDOWN, 0);
 }
 
-- 
2.43.0

Re: [PATCH v2] hwmon: (lm75) Add support for Nuvoton NCT7715

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect polarity mask used for 16-bit configuration registers, modifying a reserved bit instead of the actual polarity (POL) bit.
--

commit 7db614efb67592440d087e96063e9c50c4ebec00
Author: Sheng-Yuan Huang <syhuang3@nuvoton.com>

hwmon: (lm75) Add support for Nuvoton NCT7715

This commit adds support for the Nuvoton NCT7715 sensor, which is largely
compatible with the LM75 layout. It introduces a 16-bit configuration
register handling for this specific chip, including unique conversion-rate and
shutdown bit mappings.

> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index 2d2d752aeac9d..ad7128629e56c 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
[ ... ]
> @@ -770,7 +805,7 @@ static int lm75_generic_probe(struct device *dev, const char *name,
>  	data->orig_conf = status;
>  
>  	/* Enforce polarity active-low (default) or active-high (devicetree) */
> -	if (!data->params->config_reg_16bits)
> +	if (!data->params->config_reg_16bits || data->kind == nct7715)
>  		pol_mask = LM75_ALERT_POLARITY_HIGH_8_BIT;
>  	else
>  		pol_mask = LM75_ALERT_POLARITY_HIGH_16_BIT;

[Severity: High]
This is a pre-existing issue, but does this logic in lm75_generic_probe()
apply an incorrect polarity mask for existing 16-bit sensors? 

By adding the bypass for nct7715, it avoids the issue for the new chip, but
it appears the mask used for other 16-bit chips like TMP112 or AS6200 
(LM75_ALERT_POLARITY_HIGH_16_BIT, defined as BIT(10)) targets Byte 2, bit 2. 

Since the SMBus word read natively places Byte 1 (containing the POL bit at 
bit 2) into the lower 8 bits, using BIT(10) might modify a reserved bit 
instead of the actual polarity bit. 

This could cause the thermal alert polarity configuration to silently fail
when a device tree configuration sets ti,alert-polarity-active-high for an
existing 16-bit LM75-compatible chip, breaking thermal alerts.

Could the mask for 16-bit registers be corrected for all devices instead of 
just bypassing it for the nct7715?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911072654.15722-1-syhuang3@nuvoton.com?part=1

Re: [PATCH v2] hwmon: (lm75) Add support for Nuvoton NCT7715

[Guenter Roeck]

On 9/11/26 00:26, hsyemail2@gmail.com wrote:
> From: Sheng-Yuan Huang <syhuang3@nuvoton.com>
> 
> The Nuvoton NCT7715 is compatible with the LM75 temperature and
> limit register layout. Add support for the NCT7715, including its
> 16-bit configuration register.
> 
> Handle its conversion-rate field and shutdown bit separately, since
> their bit positions differ from the standard LM75 layout.
> 
> Signed-off-by: Sheng-Yuan Huang <syhuang3@nuvoton.com>
> ---
> v2:
> - Use the standard SMBus word representation for the NCT7715
>    configuration register, removing the NCT7715-specific byte swapping
>    and updating its configuration masks accordingly.
> 
>   .../devicetree/bindings/hwmon/lm75.yaml       |  1 +
>   Documentation/hwmon/lm75.rst                  |  6 +++
>   drivers/hwmon/lm75.c                          | 52 ++++++++++++++++++-
>   3 files changed, 57 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/lm75.yaml b/Documentation/devicetree/bindings/hwmon/lm75.yaml
> index b48bf3fd721f..832a8c5d83e9 100644
> --- a/Documentation/devicetree/bindings/hwmon/lm75.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/lm75.yaml
> @@ -31,6 +31,7 @@ properties:
>         - nxp,p3t1750
>         - nxp,p3t1755
>         - nxp,pct2075
> +      - nuvoton,nct7715
>         - st,stds75
>         - st,stlm75
>         - microchip,tcn75
> diff --git a/Documentation/hwmon/lm75.rst b/Documentation/hwmon/lm75.rst
> index ca46754e028b..fac0b8f29ddc 100644
> --- a/Documentation/hwmon/lm75.rst
> +++ b/Documentation/hwmon/lm75.rst
> @@ -150,6 +150,12 @@ Supported chips:
>   
>                  https://ams.com/documents/20143/36005/AS6200_DS000449_4-00.pdf
>   
> +  * Nuvoton NCT7715
> +
> +    Prefix: 'nct7715'
> +
> +    Addresses scanned: none
> +
>   Author: Frodo Looijaard <frodol@dds.nl>
>   
>   Description
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index 2d2d752aeac9..ad7128629e56 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -39,6 +39,7 @@ enum lm75_type {		/* keep sorted in alphabetical order */
>   	max6626,
>   	max31725,
>   	mcp980x,
> +	nct7715,
>   	p3t1750,
>   	p3t1755,
>   	pct2075,
> @@ -107,6 +108,15 @@ static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b, 0x4c,
>   #define LM75_REG_MAX		0x03
>   #define PCT2075_REG_IDLE	0x04
>   
> +#define NCT7715_CONFIG_RESOLUTION_12_BIT	GENMASK(6, 5)
> +#define NCT7715_CONFIG_SHUTDOWN			BIT(0)

Same as LM75_SHUTDOWN, and thus unnecessary.

> +#define NCT7715_CONFIG_CONVERSION_RATE_MASK	GENMASK(15, 14)
> +#define NCT7715_CONFIG_EXTENDED_MODE		BIT(12)
> +#define NCT7715_CONFIG_RATE_0_25HZ		0
> +#define NCT7715_CONFIG_RATE_1HZ			BIT(14)
> +#define NCT7715_CONFIG_RATE_4HZ			BIT(15)
> +#define NCT7715_CONFIG_RATE_8HZ			GENMASK(15, 14)
> +

This matches the bit settings of TMP112, meaning the code working for TMP112
works here just fine.

>   struct lm75_data {
>   	const char *label;
>   	struct regmap			*regmap;
> @@ -122,6 +132,10 @@ struct lm75_data {
>   /*-----------------------------------------------------------------------*/
>   
>   static const u8 lm75_sample_set_masks[] = { 0 << 5, 1 << 5, 2 << 5, 3 << 5 };
> +static const u16 nct7715_sample_set_masks[] = {
> +	NCT7715_CONFIG_RATE_8HZ, NCT7715_CONFIG_RATE_4HZ,
> +	NCT7715_CONFIG_RATE_1HZ, NCT7715_CONFIG_RATE_0_25HZ
> +};
>   
>   #define LM75_ALERT_POLARITY_HIGH_8_BIT	(BIT(2))
>   #define LM75_ALERT_POLARITY_HIGH_16_BIT	(BIT(2) << 8)
> @@ -259,6 +273,15 @@ static const struct lm75_params device_params[] = {
>   		.sample_times = (unsigned int []){ 30, 60, 120, 240 },
>   		.resolutions = (u8 []) {9, 10, 11, 12 },
>   	},
> +	[nct7715] = {
> +		.config_reg_16bits = true,
> +		.set_mask = NCT7715_CONFIG_RESOLUTION_12_BIT,
> +		.clr_mask = NCT7715_CONFIG_EXTENDED_MODE,

We don't use chip specific defines for any of the other chips,
so don't do it here either.

> +		.default_resolution = 12,
> +		.default_sample_time = 250,
> +		.num_sample_times = 4,
> +		.sample_times = (unsigned int []){ 125, 250, 1000, 4000 },
> +	},
>   	[tmp100] = {
>   		.set_mask = 3 << 5,	/* 12-bit mode */
>   		.clr_mask = 1 << 7,	/* not one-shot mode */
> @@ -354,6 +377,11 @@ static inline long lm75_reg_to_mc(s16 temp, u8 resolution)
>   static inline int lm75_write_config(struct lm75_data *data, u16 set_mask,
>   				    u16 clr_mask)
>   {
> +	if (data->kind == nct7715)
> +		return regmap_update_bits(data->regmap, LM75_REG_CONF,
> +					  clr_mask | set_mask | NCT7715_CONFIG_SHUTDOWN,

NCT7715_CONFIG_SHUTDOWN == LM75_SHUTDOWN, so this conditional
is unnecessary. There are several similar instances of unnecessary duplication
in the code. Please drop all that. The chip appears to be a clone of tmp112,
and there is no reason to handle this chip differently. If you want to use
a different sample time default, fine, but that does not warrant the extra code.

I'll stop the review here. Please drop all the extra code.

Regarding Sashiko's feedback: It doesn't understand that the byte order
is swapped in the regmap write function. THis is a false positive.

Thanks,
Guenter

> +					  set_mask);
> +
>   	return regmap_update_bits(data->regmap, LM75_REG_CONF,
>   				  clr_mask | set_mask | LM75_SHUTDOWN, set_mask);
>   }
> @@ -486,6 +514,13 @@ static int lm75_update_interval(struct device *dev, long val)
>   		if (data->params->resolutions)
>   			data->resolution = data->params->resolutions[index];
>   		break;
> +	case nct7715:
> +		err = lm75_write_config(data, nct7715_sample_set_masks[index],
> +					NCT7715_CONFIG_CONVERSION_RATE_MASK);
> +		if (err)
> +			return err;
> +		data->sample_time = data->params->sample_times[index];
> +		break;
>   	case tmp112:
>   	case as6200:
>   		err = regmap_update_bits(data->regmap, LM75_REG_CONF,
> @@ -770,7 +805,7 @@ static int lm75_generic_probe(struct device *dev, const char *name,
>   	data->orig_conf = status;
>   
>   	/* Enforce polarity active-low (default) or active-high (devicetree) */
> -	if (!data->params->config_reg_16bits)
> +	if (!data->params->config_reg_16bits || data->kind == nct7715)
>   		pol_mask = LM75_ALERT_POLARITY_HIGH_8_BIT;
>   	else
>   		pol_mask = LM75_ALERT_POLARITY_HIGH_16_BIT;
> @@ -848,6 +883,7 @@ static const struct i2c_device_id lm75_i2c_ids[] = {
>   	{ .name = "max31725", .driver_data = max31725 },
>   	{ .name = "max31726", .driver_data = max31725 },
>   	{ .name = "mcp980x", .driver_data = mcp980x },
> +	{ .name = "nct7715", .driver_data = nct7715 },
>   	{ .name = "p3t1750", .driver_data = p3t1750 },
>   	{ .name = "p3t1755", .driver_data = p3t1755 },
>   	{ .name = "pct2075", .driver_data = pct2075 },
> @@ -972,6 +1008,10 @@ static const struct of_device_id lm75_of_match[] = {
>   		.compatible = "nxp,pct2075",
>   		.data = (void *)pct2075
>   	},
> +	{
> +		.compatible = "nuvoton,nct7715",
> +		.data = (void *)nct7715
> +	},
>   	{
>   		.compatible = "st,stds75",
>   		.data = (void *)stds75
> @@ -1129,13 +1169,21 @@ static int lm75_suspend(struct device *dev)
>   {
>   	struct lm75_data *data = dev_get_drvdata(dev);
>   
> -	return regmap_update_bits(data->regmap, LM75_REG_CONF, LM75_SHUTDOWN, LM75_SHUTDOWN);
> +	if (data->kind == nct7715)
> +		return regmap_update_bits(data->regmap, LM75_REG_CONF,
> +					  NCT7715_CONFIG_SHUTDOWN,
> +					  NCT7715_CONFIG_SHUTDOWN);
> +	return regmap_update_bits(data->regmap, LM75_REG_CONF,
> +				  LM75_SHUTDOWN, LM75_SHUTDOWN);
>   }
>   
>   static int lm75_resume(struct device *dev)
>   {
>   	struct lm75_data *data = dev_get_drvdata(dev);
>   
> +	if (data->kind == nct7715)
> +		return regmap_update_bits(data->regmap, LM75_REG_CONF,
> +					  NCT7715_CONFIG_SHUTDOWN, 0);
>   	return regmap_update_bits(data->regmap, LM75_REG_CONF, LM75_SHUTDOWN, 0);
>   }
>