On 09/08/2022 08:19, Julien Panis wrote:
On 08/08/2022 18:19, William Breathitt Gray wrote:
The Counter subsystem is relatively nascent so the number of existing
Counter device drivers to study is unfortunately sparse. If you
encounter any trouble along the way as you work on this, please don't
hestitate to reach out and I'll be happy to answer any questions you may
have. That said, here are some more hints that can help guide you. :-)
Although we've been using CAPx to refer to these registers, in the
context of sysfs it'll be better to call the attributes "capture1",
"capture2", etc.; that will make their use as capture buffers more
obvious. Furthermore, despite my previous example, I think it's better
to have these exist underneath the CTR hierarchy rather than Mod4
because they are captures of the CTR value:
* CTR: /sys/bus/counter/devices/counterX/count0/count
* CAP1: /sys/bus/counter/devices/counterX/count0/capture1
* CAP2: /sys/bus/counter/devices/counterX/count0/capture2
* CAP3: /sys/bus/counter/devices/counterX/count0/capture3
* CAP4: /sys/bus/counter/devices/counterX/count0/capture4
In your device driver, you would define a struct counter_count to
represent CTR. In this struct counter_count there is an "ext" member
where you provide an array of struct count_comp. Each CAPx will have a
corresponding struct count_comp; it'll look something like this::
static struct counter_comp ctr_count_ext[] = {
COUNTER_COMP_COUNT_U64("capture1", cap1_read, NULL),
COUNTER_COMP_COUNT_U64("capture2", cap2_read, NULL),
COUNTER_COMP_COUNT_U64("capture3", cap3_read, NULL),
COUNTER_COMP_COUNT_U64("capture4", cap4_read, NULL),
};
As you already know, counter_push_event() lets you push Counter events
in your interrupt handler. I recommend introducing a new event type
under enum counter_event_type in the include/uapi/linux/counter.h header
file; something like COUNTER_EVENT_CAPTURE should be descriptive enough.
The "channel" member of struct counter_watch refers to Counter event
channels; The purpose here is to allow us to support concurrent events
of the same type (e.g. two COUNTER_EVENT_OVERFLOW but for different
Counts). If I understand the TI ECAP device correctly, we'll be getting
a COUNTER_EVENT_CAPTURE event whenever a CAPx register is updated with a
new capture. It's up to you if you want to group them under the same
channel or separate channels for each CAPx; you would just pass the
channel in counter_push_event() to indicate which COUNTER_EVENT_CAPTURE
event is being handled.
2 options, indeed :
1) By grouping them under the same channel, the only thing I'm not a
great fan of is that
I will need to use separate functions to read capture registers
(cap1_read/cap2_read/
cap3_read/cap4_read), and also to set/get polarity.
2) By using separate channels, code will look more simple but it is
likely to be a little bit
confusing for the user.
I will probably use option 2.
[ERRATUM] : Sorry, I meant that I will probably use option 1 (not 2), to
avoid confusion for the user.
