linux-kernel - Re: [PATCH v4 3/3] counter: Add STM based counter

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: <f5a96574-4d81-480f-b5fe-45c437a49288@linaro.org>
Date: Thu, 8 Jan 2026 18:11:28 +0100
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Frank Li <Frank.li@....com>
Cc: wbg@...nel.org, robh@...nel.org, conor+dt@...nel.org, krzk+dt@...nel.org,
 s32@....com, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-iio@...r.kernel.org
Subject: Re: [PATCH v4 3/3] counter: Add STM based counter

On 1/8/26 17:41, Frank Li wrote:
> On Wed, Jan 07, 2026 at 02:39:52PM +0100, Daniel Lezcano wrote:
>> The NXP S32G2 automotive platform integrates four Cortex-A53 cores and
>> three Cortex-M7 cores, along with a large number of timers and
>> counters. These hardware blocks can be used as clocksources or
>> clockevents, or as timestamp counters shared across the various
>> subsystems running alongside the Linux kernel, such as firmware
>> components. Their actual usage depends on the overall platform
>> software design.
>>
>> In a Linux-based system, the kernel controls the counter, which is a
>> read-only shared resource for the other subsystems. One of its primary
>> purposes is to act as a common timestamp source for messages or
>> traces, allowing correlation of events occurring in different
>> operating system contexts.
>>
>> These changes introduce a basic counter driver that can start, stop,
>> and reset the counter. It also handles overflow accounting and
>> configures the prescaler value.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org>
>> ---

[ ... ]

>> +
>> +static struct counter_comp stm_cnt_count_ext[] = {
>> +	COUNTER_COMP_COUNT_U8("prescaler", nxp_stm_cnt_prescaler_read, nxp_stm_cnt_prescaler_write),
>> +	COUNTER_COMP_ENABLE(nxp_stm_cnt_count_enable_read, nxp_stm_cnt_count_enable_write),
>> +};
>> +
>> +static int nxp_stm_cnt_action_read(struct counter_device *counter,
>> +				   struct counter_count *count,
>> +				   struct counter_synapse *synapse,
>> +				   enum counter_synapse_action *action)
>> +{
>> +	*action = COUNTER_SYNAPSE_ACTION_RISING_EDGE;
>> +
>> +	return 0;
>> +}
>> +
>> +static int nxp_stm_cnt_count_read(struct counter_device *dev,
>> +				  struct counter_count *count, u64 *val)
>> +{
>> +	struct nxp_stm_cnt *stm_cnt = counter_priv(dev);
>> +	unsigned long irqflags;
>> +
>> +	spin_lock_irqsave(&stm_cnt->lock, irqflags);
>> +	*val = stm_cnt->counter + nxp_stm_cnt_get_counter(stm_cnt);
>> +	spin_unlock_irqrestore(&stm_cnt->lock, irqflags);
> 
> It think there are still rise condition here.
> 
> cpu 0                                            cpu 1
> 1:  nxp_stm_cnt_get_counter(stm_cnt); (0)
> 2: 						 irq_handle()
> 					         counter += INT_MAX;
> 3:
> 
> when wrap happen, nxp_stm_cnt_get_counter() return 0, but, irq still not
> handle yet.
> 
> so nxp_stm_cnt_count_read() return 0 at 1.
> 
> it return INT_MAX at 3 suddently.

Thanks for the review. Yes, it seems like there is a race for the reader 
result.

Except disabling / enabling the counter when reading the value, I don't 
see how to handle that case.



bool is_started = nxp_stm_cnt_is_started(stm_cnt);

if (is_started)
	nxp_stm_cnt_disable(stm_cnt);

/* If the counter wrapped, the interrupt fired already */

spin_lock_irqsave(&stm_cnt->lock, irqflags);

/* Counter can be zero (or more), we don't care, UINT_MAX was added */
*val = stm_cnt->counter + nxp_stm_cnt_get_counter(stm_cnt);

spin_unlock_irqrestore(&stm_cnt->lock, irqflags);

if (is_started)
	nxp_stm_cnt_enable(stm_cnt);





-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog