linux-kernel - Re: [PATCH 3/3] hwmon: (ctf2301) Add support for CTF2301

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8c6f609e-c086-4b6c-abb5-8d33ec85df47@roeck-us.net>
Date: Thu, 25 Sep 2025 20:57:13 -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/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.

What happens if you just instantiate the lm63 driver, possibly after updating
the detect function ?

Guenter