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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aNaTPE494MMExSBz@troy-wujie14pro-arch>
Date: Fri, 26 Sep 2025 21:21:00 +0800
From: Troy Mitchell <troy.mitchell@...ux.dev>
To: Guenter Roeck <linux@...ck-us.net>,
	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 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?

> 
> What happens if you just instantiate the lm63 driver, possibly after updating
> the detect function ?
I will run the tests the day after tomorrow and provide a log.

                - Troy

> 
> Guenter
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ