linux-kernel - Re: [PATCH v4 0/2] microchip-tcb-capture: Add Capture, Compare, Overflow etc. events

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: <Z8B-BW1JwIrI18E4@ishi>
Date: Fri, 28 Feb 2025 00:00:21 +0900
From: William Breathitt Gray <wbg@...nel.org>
To: Csókás Bence <csokas.bence@...lan.hu>
Cc: Kamel Bouhara <kamel.bouhara@...tlin.com>,
	linux-arm-kernel@...ts.infradead.org, linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org, Dharma.B@...rochip.com,
	Ludovic Desroches <ludovic.desroches@...rochip.com>,
	Nicolas Ferre <nicolas.ferre@...rochip.com>,
	Jonathan Cameron <jic23@...nel.org>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v4 0/2] microchip-tcb-capture: Add Capture, Compare,
 Overflow etc. events

On Thu, Feb 27, 2025 at 03:17:55PM +0100, Csókás Bence wrote:
> On 2025. 02. 27. 5:59, William Breathitt Gray wrote:
> > If that is true, then the correct way for this hardware to be exposed is
> > to have each TCB be a Counter device where each TCC is exposed as a
> > Count. So for this SoC: two Counter devices as counter0 and counter1;
> > count0, count1, and count2 as the three TCC; i.e. counter0/count{0,1,2}
> > and counter1/count{0,1,2}.
> 
> And how would the extensions fit into this? `capture{0..6}`/`compare{0..3}?
> For me, the status quo fits better.

For your patchset here, treat it as if nothing is changing. So leave it
as capture0 and capture1 exposing RA and RB respectively.

> > With that setup, configurations that affect the entire TCB (e.g. Block
> > Mode Register) can be exposed as Counter device components. Furthermore,
> > this would allow users to set Counter watches to collect component
> > values for the other two Counts while triggering off of the events of
> > any particular one, which wouldn't be possible if each TCC is isolated
> > to its own Counter device as is the case right now.
> 
> TCCs are pretty self-contained though. BMR only seems to be used

In the Generic Counter Interface, hardware functionality exposure is
scoped by its influence over particular components of the Generic
Counter paradigm. That means, configurations that affect a particular
Count are grouped under that Count (i.e. "count0/enable" enables Count0)
while configurations that affect a particular Signal are grouped under
that Signal (i.e. "signal2/polarity" sets Signal2's polarity).

In the same way, configurations that affect the Counter device as a
whole are exposed as device components. That's what the functionality
BMR provides does, so it's appropriate to expose it as several Counter
device components.

Right now the microchip-tcb-capture doesn't do much with BMR, but
looking at the datasheet I see that it does have the capable to
configure several settings that affect the entire TCB. For example, you
can swap PHA and PHB (via SWAP bit), you can enable autocorrection for
missing pulses (via AUTOC bit), you can define a maximum filter threshold
(via MAXFILT bitfield), etc. These are controls users would expect to
find as Counter device components rather than Count components for
example.

> > Regardless, the three TCC of each TCB should be grouped together
> > logically as they can represent related values. For example,  when using
> > the quadrature decoder TTC0 CV can represent Speed/Position while TTC1
> > CV represents rotation, thus giving a high level of precision on motion
> > system position as the datasheet points out.
> 
> From what I gathered from looking at the code, the quadrature mode uses a
> hardware decoder that gives us processed values already. Though I don't use
> it, so I don't know any specifics.
> 
> One more thing, as Kamel pointed it out, the current implementation allows
> channels of a TCB to perform different functions, e.g. one used for PWM, one
> for clocksource and a third for counter capture. Whether that works in
> practice, I cannot tell either, we only use TCB0 channel 0 for clocksource
> and TCB1 channel 1 for the counter.

In theory this shouldn't be a problem because the counter driver
shouldn't try to perform non-counter functions, just expose the
counter-specific ones when available. We would need to check which mode
the channel is in and return -EBUSY if that respective channel's
components are accessed during a non-supported mode. This is how we
handle similar situations in other counter driver such as the
stm32-lptimer-cnt and intel-qep modules.

William Breathitt Gray

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