linux-kernel - Re: [PATCH] counter: microchip-tcb-capture: Fix undefined counter channel state on probe

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: <Z8gn_4qxtEqbn20O@ishi>
Date: Wed, 5 Mar 2025 19:31:27 +0900
From: William Breathitt Gray <wbg@...nel.org>
To: Csókás Bence <csokas.bence@...lan.hu>,
	Kamel Bouhara <kamel.bouhara@...tlin.com>
Cc: linux-arm-kernel@...ts.infradead.org, linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] counter: microchip-tcb-capture: Fix undefined counter
 channel state on probe

On Wed, Mar 05, 2025 at 07:01:19PM +0900, William Breathitt Gray wrote:
> Hardware initialize of the timer counter channel does not occur on probe
> thus leaving the Count in an undefined state until the first
> function_write() callback is executed. Fix this by performing the proper
> hardware initialization during probe.
> 
> Fixes: 106b104137fd ("counter: Add microchip TCB capture counter")
> Reported-by: Csókás Bence <csokas.bence@...lan.hu>
> Closes: https://lore.kernel.org/all/bfa70e78-3cc3-4295-820b-3925c26135cb@prolan.hu/
> Signed-off-by: William Breathitt Gray <wbg@...nel.org>
> ---
> This should fix the issue where a user needs to set the count function
> before they can use the counter. I don't have this hardware in person,
> so please test this patch and let me know whether it works for you.

While developing this bug fix, I noticed the following code in the
mchp_tc_count_function_write() function:

	if (!priv->tc_cfg->has_gclk)
		cmr |= ATMEL_TC_TIMER_CLOCK2;
	else
		cmr |= ATMEL_TC_TIMER_CLOCK1;
	/* Setup the period capture mode */
	cmr |=  ATMEL_TC_CMR_MASK;
	cmr &= ~(ATMEL_TC_ABETRG | ATMEL_TC_XC0);

It looks like it's trying to choose the TCCLKS value by evaluating
has_gclk. However, a couple lines later the cmr value is masked by
ATMEL_TC_XC0 which will clobber the previous choice by resetting bit 0.

Is this a bug, or am I misunderstanding how the TCCLKS value is set by
these defines?

William Breathitt Gray

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)