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

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

On 9/9/26 00:10, Sheng-Yuan Huang wrote:
> Thank you for the review. I may have misunderstood which conversion
> you are referring to, so I would like to clarify both parts.
> 
> If this refers to the NCT7715_CONFIG_* masks, they use the logical bit
> positions of the 16-bit configuration register defined by the
> NCT7715 specification. They are not pre-swapped for the SMBus word
> byte order.
> 
> If this refers to the I2C accessors, the NCT7715 transfers its 16-bit
> configuration register MSB first. The lm75 regmap bus uses
> reg_read/reg_write callbacks. In this path, regmap passes the logical
> register value to the callbacks, and i2c_smbus_*_word_swapped()
> performs the conversion between that value and the SMBus word byte
> order.
> 
> I verified this on hardware by changing update_interval. i2cget shows
> 0xe060, 0xa060, 0x6060, and 0x2060 for 125, 250, 1000, and 4000 ms,
> respectively. After accounting for the SMBus word byte order, these
> are logical values 0x60e0, 0x60a0, 0x6060, and 0x6020. In each case,
> only the NCT7715 conversion-rate field, bits [7:6], is changed.
> 

Sure, it works, because your code converts the bytes twice, as I said.
That doesn't make those conversions necessary.

Other chips also use 16-bit config registers. Just use the same approach.

Also, please do not top-post.

Thanks,
Guenter

> Guenter Roeck <linux@roeck-us.net> 於 2026年9月7日週一 下午11:30寫道：
>>
>> 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
>>