[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <3bf744e2-aced-482d-9690-1bb3354aafad@roeck-us.net>
Date: Fri, 26 Sep 2025 06:33:46 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Troy Mitchell <troy.mitchell@...ux.dev>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Jean Delvare <jdelvare@...e.com>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-hwmon@...r.kernel.org
Subject: Re: [PATCH 3/3] hwmon: (ctf2301) Add support for CTF2301
On 9/26/25 06:21, Troy Mitchell wrote:
> On Thu, Sep 25, 2025 at 08:57:13PM -0700, Guenter Roeck wrote:
>> On 9/25/25 18:32, Troy Mitchell wrote:
>>> Hi Guenter, Thanks for your review.
>>> There are many things to improve in this driver.
>>>
>>> On Wed, Sep 24, 2025 at 08:43:35AM -0700, Guenter Roeck wrote:
>>>> On Tue, Sep 16, 2025 at 12:46:46PM +0800, Troy Mitchell wrote:
>>> [...]
>>>>> diff --git a/drivers/hwmon/ctf2301.c b/drivers/hwmon/ctf2301.c
>>> [...]
>>>>> +
>>>>> +#define CTF2301_LOCAL_TEMP_MSB 0x00
>>>> LM90_REG_LOCAL_TEMP
>>>>> +#define CTF2301_RMT_TEMP_MSB 0x01
>>>> LM90_REG_REMOTE_TEMPH
>>>>> +#define CTF2301_ALERT_STATUS 0x02
>>>> LM90_REG_STATUS
>>>>> +#define CTF2301_GLOBAL_CFG 0x03
>>>> LM90_REG_CONFIG1
>>>>> +#define CTF2301_RMT_TEMP_LSB 0x10
>>>> LM90_REG_REMOTE_TEMPL
>>>>> +#define CTF2301_LOCAL_TEMP_LSB 0x15
>>>> TMP451_REG_LOCAL_TEMPL
>>>>> +#define CTF2301_ALERT_MASK 0x16
>>>> TMP461_REG_CHEN
>>>>
>>>> So far this looks like a chip based on LM90 or TMP451/TMP461
>>>> with an added fan controller. I can not immediatey determine
>>>> if it would be better to add the pwm/tach support to the lm90
>>>> driver. Given that the chip (based on registers) does support
>>>> limits, which is not implemented here but essential for a chip
>>>> like this, I would very much prefer adding support for it to the
>>>> lm90 driver if possible.
>>>>
>>>> The public datasheet does not provide register details, making it
>>>> all but impossible to do a real evaluation. Any idea how to get
>>>> a complete datasheet ?
>>> Yeah, more register info at [1].
>>> I've checked the detailed review below,
>>> but I'll hold off on sending v2 until you decide if we really need a new driver.
>>>
>>> Is this chip more like the LM63, by the way?
>>>
>>
>> Good catch. Yes, looks like you are correct. LM63 is an almost perfect match.
>> CTF2301 has a couple of extra registers, mostly local setpoint and temp LSB
>> plus the registers in the 0x3x range. Actually, those registers _are_ defined
>> for LM96163, so that chip is an even closer match.
> Yes, so just to confirm,
> you agree that the development should be done on top of the lm63 driver, right?
>
Yes.
Thanks,
Guenter
Powered by blists - more mailing lists