linux-kernel - Re: [PATCH v3] pwm: mc33xs2410: add support for temperature sensors

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: <d8cad439-0106-4079-9164-64b11d1681c6@roeck-us.net>
Date: Mon, 23 Jun 2025 06:52:40 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Uwe Kleine-König <ukleinek@...nel.org>
Cc: dimitri.fedrau@...bherr.com, Jean Delvare <jdelvare@...e.com>,
	linux-pwm@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-hwmon@...r.kernel.org, Dimitri Fedrau <dima.fedrau@...il.com>
Subject: Re: [PATCH v3] pwm: mc33xs2410: add support for temperature sensors

On Mon, Jun 23, 2025 at 11:07:24AM +0200, Uwe Kleine-König wrote:
> hello Dimitri,
> 
> On Thu, Jun 19, 2025 at 07:32:42PM +0200, Dimitri Fedrau via B4 Relay wrote:
> > @@ -29,6 +30,8 @@
> >  
> >  #include <linux/spi/spi.h>
> >  
> > +/* ctrl registers */
> > +
> >  #define MC33XS2410_GLB_CTRL			0x00
> >  #define MC33XS2410_GLB_CTRL_MODE		GENMASK(7, 6)
> >  #define MC33XS2410_GLB_CTRL_MODE_NORMAL		FIELD_PREP(MC33XS2410_GLB_CTRL_MODE, 1)
> > @@ -51,6 +54,21 @@
> >  
> >  #define MC33XS2410_WDT				0x14
> >  
> > +#define MC33XS2410_TEMP_WT			0x29
> > +#define MC33XS2410_TEMP_WT_MASK			GENMASK(7, 0)
> > +
> > +/* diag registers */
> > +
> > +/* chan in { 1 ... 4 } */
> > +#define MC33XS2410_OUT_STA(chan)		(0x02 + (chan) - 1)
> > +#define MC33XS2410_OUT_STA_OTW			BIT(8)
> > +
> > +#define MC33XS2410_TS_TEMP_DIE			0x26
> > +#define MC33XS2410_TS_TEMP_MASK			GENMASK(9, 0)
> 
> Keep the registers in address order please
> 
> > +/* chan in { 1 ... 4 } */
> > +#define MC33XS2410_TS_TEMP(chan)		(0x2f + (chan) - 1)
> 
> I wonder if it would be cleaner if this was abstracted using mfd. Then
> the hwmon driver could live in drivers/hwmon
> 

For some reason the recent tendency is to move code out of hwmon, not into
it. Maybe my review feedback is too strong or something. Either case,
consider using auxiliary devices if you really want to move the code. That
seems to be a better fit, and it would be more lightweight since it doesn't
require an interconnect driver.

Guenter