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

[William Breathitt Gray <wbg@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3390 bytes --]

On Thu, Feb 27, 2025 at 03:37:28PM +0100, Alexandre Belloni wrote:
> On 27/02/2025 23:22:36+0900, William Breathitt Gray wrote:
> > On Thu, Feb 27, 2025 at 02:53:30PM +0100, Kamel Bouhara wrote:
> > > On Thu, Feb 27, 2025 at 01:59:57PM +0900, William Breathitt Gray wrote:
> > > > Let me make sure I understand the situation correctly. This SoC has two
> > > > Timer Counter Blocks (TCB) and each TCB has three Timer Counter Channels
> > > > (TCC); each TCC has a Counter Value (CV) and three general registers
> > > > (RA, RB, RC); RA and RB can store Captures, and RC can be used for
> > > > Compare operations.
> > > >
> > > > 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}.
> > 
> > [...]
> > 
> > > > Kamel, what would it take for us to rectify this situation so that the
> > > > TCC are organized together by TCB under the same Counter devices?
> > > 
> > > Hello,
> > > 
> > > Indeed, each TCC operates independently except when quadrature mode is
> > > enabled. I assume this approach was taken to provide more flexibility by
> > > exposing them separately.
> > > 
> > > Currently only one channel is configured this would need to rework the
> > > driver to make the 3 TCCs exposed.
> > > 
> > > Greetings,
> > > Kamel
> > 
> > Skimming through the driver, it looks like what we'll need is for
> > mchp_tc_counts[] to have all three TCCs defined, then have
> > mchp_tc_probe() match on a TCB node and configure each TCC. Once that's
> > setup, then whenever we need to identify which TCC a callback is
> > exposing, we can get it from count->id.
> > 
> > So for example, the TC_CV register offset is calculated as 0x00 +
> > channel * 0x40 + 0x10. In the count_read() callback we can leverage
> > count->id to identify the TCC and thus get the respective TC_CV register
> > at offset + count->id * 0x40 + 0x10.
> > 
> 
> We can't do that because the TCC of a single TCB can have a mix of
> different features. I struggled with the breakage to move away from the
> one TCB, one feature state we had.
> Be fore this, it was not possible to mix features on a single TCB, now,
> we can have e.g. the clocksource on TCC 0 and 1 of TCB0 and a PWM on
> TCC 2. mchp_tc_probe must not match on a TCB node...

Okay I see what you mean, if we match on a TCB mode then we wouldn't be
able to define the cases where one TCC is different from the next in the
same TCB.

The goal however isn't to support all functionality (i.e. PWM-related
settings, etc.) in the counter driver, but just expose the TCB
configuration options that affect the TCCs when configured for counter
mode. For example, the sysfs attributes can be created, but they don't
have to be available until the TCC is in the appropriate mode (e.g.
return -EBUSY until they are in a counter mode).

Is there a way to achieve that? Maybe there's a way we can populate the
sysfs tree on the first encountered TCC, and then somehow indicate when
additional TCCs match. Attributes can become available then dynamically
based on the TCCs that match.

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]