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

[Guenter Roeck <linux@roeck-us.net>]

On 9/7/26 01:08, hsyemail2@gmail.com wrote:
> From: Sheng-Yuan Huang <syhuang3@nuvoton.com>
> 
> The Nuvoton NCT7715 is compatible with the LM75 temperature and
> limit register layout, but its 16-bit configuration register is
> transmitted most-significant byte first. Add support for the NCT7715,
> including byte-swapped SMBus accesses for its configuration register.
> 
> Handle its conversion-rate field and shutdown bit separately, since
> their bit positions differ from the standard LM75 layout.
> 
I don't think so. See below.

> Signed-off-by: Sheng-Yuan Huang <syhuang3@nuvoton.com>
> ---
>   .../devicetree/bindings/hwmon/lm75.yaml       |  1 +
>   Documentation/hwmon/lm75.rst                  |  6 ++
>   drivers/hwmon/lm75.c                          | 57 ++++++++++++++++++-
>   3 files changed, 62 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..0b55f6b7c68d 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(14, 13)
> +#define NCT7715_CONFIG_SHUTDOWN			BIT(8)
> +#define NCT7715_CONFIG_CONVERSION_RATE_MASK	GENMASK(7, 6)
> +#define NCT7715_CONFIG_EXTENDED_MODE		BIT(4)
> +#define NCT7715_CONFIG_RATE_0_25HZ		0
> +#define NCT7715_CONFIG_RATE_1HZ			BIT(6)
> +#define NCT7715_CONFIG_RATE_4HZ			BIT(7)
> +#define NCT7715_CONFIG_RATE_8HZ			GENMASK(7, 6)
> +
>   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,
> @@ -609,6 +644,8 @@ static int lm75_i2c_reg_read(void *context, unsigned int reg, unsigned int *val)
>   	if (reg == LM75_REG_CONF) {
>   		if (!data->params->config_reg_16bits)
>   			ret = i2c_smbus_read_byte_data(client, LM75_REG_CONF);
> +		else if (data->kind == nct7715)
> +			ret = i2c_smbus_read_word_swapped(client, LM75_REG_CONF);
>   		else
>   			ret = i2c_smbus_read_word_data(client, LM75_REG_CONF);
>   	} else {
> @@ -628,8 +665,11 @@ static int lm75_i2c_reg_write(void *context, unsigned int reg, unsigned int val)
>   	if (reg == PCT2075_REG_IDLE ||
>   	    (reg == LM75_REG_CONF && !data->params->config_reg_16bits))
>   		return i2c_smbus_write_byte_data(client, reg, val);
> -	else if (reg == LM75_REG_CONF)
> +	else if (reg == LM75_REG_CONF) {
> +		if (data->kind == nct7715)
> +			return i2c_smbus_write_word_swapped(client, reg, val);

This code is double swapping. The calling code swaps (bit settings),
regmap calls this function, where the word is swapped again.

Guenter