linux-kernel - Re: [PATCH] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250512133114.GA6440@legfed1>
Date: Mon, 12 May 2025 15:31:14 +0200
From: Dimitri Fedrau <dima.fedrau@...il.com>
To: Guenter Roeck <linux@...ck-us.net>
Cc: dimitri.fedrau@...bherr.com,
	Uwe Kleine-König <ukleinek@...nel.org>,
	Jean Delvare <jdelvare@...e.com>, linux-pwm@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-hwmon@...r.kernel.org
Subject: Re: [PATCH] pwm: mc33xs2410: add support for temperature sensors

Hi Guenter,

Am Mon, May 12, 2025 at 06:04:33AM -0700 schrieb Guenter Roeck:
> On 5/12/25 04:26, Dimitri Fedrau via B4 Relay wrote:
> > From: Dimitri Fedrau <dimitri.fedrau@...bherr.com>
> > 
> > The MC33XS2410 provides temperature sensors for the central die temperature
> > and the four outputs. Additionally a common temperature warning threshold
> > can be configured for the outputs. Add hwmon support for the sensors.
> > 
> > Signed-off-by: Dimitri Fedrau <dimitri.fedrau@...bherr.com>
> > ---
> 
> > +
> > +static int mc33xs2410_hwmon_read_out_status(struct spi_device *spi,
> > +					    int channel, u16 *val)
> > +{
> > +	int ret;
> > +
> > +	ret = mc33xs2410_read_reg_diag(spi, MC33XS2410_OUT_STA(channel), val);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* Bits latches high */
> > +	return mc33xs2410_read_reg_diag(spi, MC33XS2410_OUT_STA(channel), val);
> 
> Is that double read of the same register needed ? If so, you'll probably
> need a lock to prevent it from being executed from multiple threads at the
> same time.
> 
> The comment "Bit latches high" doesn't really mean anything to me and doesn't
> explain why the register needs to be read twice.
> 
>

All bits of the output status registers are latched high. In case there
was overtemperature detected, the bit stays set until read once and cleared
afterwards. So I need a second read to get the "realtime" status.
Otherwise I might end up returning an false positive overtemperature
warning. I don't think a lock is really necessary, since I'm only
interested in the "realtime" status but not if there was a warning in
the past. What do you think ?

Will improve the comment.

Best regards
Dimitri Fedrau