Messages in this thread | | | Date | Tue, 9 Aug 2022 08:19:58 +0200 | Subject | Re: [PATCH v3 2/2] iio: time: capture-tiecap: capture driver support for ECAP | From | Julien Panis <> |
| |
On 08/08/2022 18:19, William Breathitt Gray wrote: > On Mon, Aug 08, 2022 at 10:58:22AM +0200, Julien Panis wrote: >> On 08/08/2022 02:30, William Breathitt Gray wrote: >>> Hi Julien, >>> >>> I've taken a cursory look over the TI ECAP reference guide and your >>> descriptions in this thread. I think a device driver for this would fit >>> better in the Counter subsystem than IIO. >>> >>> First I want to correct a minor misunderstanding: the "timestamp" >>> member of struct counter_event is simply a way to identify Counter >>> events on the system as a way of grouping multiple Counter watches. In >>> other words, the "timestamp" member here represents when a Counter event >>> was detected by the system, not when an event was logged on the counter >>> device hardware. Instead, hardware timestamps such as the CAPx registers >>> would be provided by the "value" member of struct counter_event. >>> >>> Now, I have a few ideas for how we could expose the timestamps using a >>> Counter device driver, but first I want to make sure I understand >>> correctly what's happening in this device. If I understand correctly, we >>> have the following device components: >>> >>> * CTR: 32-bit counter timer >>> * Mod4: 2-bit counter >>> * CAP1-CAP4: four 32-bit registers, each indepedently store a timestamp >>> * ECAP: input signal providing event trigger edges >>> >>> Four edge polarities are configured corresponding to each CAPx register, >>> yet the input signal is still the same single ECAP pin. The event that >>> is fired is instead determined by the Mod4 counter: when Mod4 is 0 and >>> the edge of ECAP matches the polarity configured for CAP1 then an event >>> is triggered which saves the current CTR value to CAP1 and increments >>> Mod4 to 1, etc. >>> >>> Is my understanding of how this device behaves correct? >> Hi William. Thank you for your help. >> Yes, your understanding of how this device behaves is correct. >> >>> If so, then one possible way to represent this device in the Counter >>> sysfs tree is something like this: >>> >>> * CTR: /sys/bus/counter/devices/counterX/count0/count >>> * Mod4: /sys/bus/counter/devices/counterX/count1/count >>> * CAP1: /sys/bus/counter/devices/counterX/count1/cap1 >>> * CAP2: /sys/bus/counter/devices/counterX/count1/cap2 >>> * CAP3: /sys/bus/counter/devices/counterX/count1/cap3 >>> * CAP4: /sys/bus/counter/devices/counterX/count1/cap4 >>> * ECAP: /sys/bus/counter/devices/counterX/signal0/signal >>> * polarity1: /sys/bus/counter/devices/counterX/signal0/cap1_polarity >>> * polarity2: /sys/bus/counter/devices/counterX/signal0/cap2_polarity >>> * polarity3: /sys/bus/counter/devices/counterX/signal0/cap3_polarity >>> * polarity4: /sys/bus/counter/devices/counterX/signal0/cap4_polarity >>> >>> This is just a tentative arrangement (you could also include "enable" >>> attributes as well), but it should give you an idea of how it could be >>> organized. >>> >>> In your driver, you could then use counter_push_event() whenever you get >>> an event triggered. In userspace, your application will add Counter >>> watches for the CAPx registers they want. When an event triggers, >>> userspace can then received all four CAP register values at the same >>> time via the respective /dev/counterX character device node. >>> >>> Would this design work for your needs? >> Yes, that would work for my needs. >> The "how" is not fully clear to me yet, since I never used counter >> subsystem. But the >> best way to understand better how it works is probably to start working with >> it. :-) >> So, next patch version will be based on counter subsystem. > 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.
> Finally, you can take a look at tools/counter/counter_example.c as an > example userspace application. The interesting bits for you are > COUNTER_ADD_WATCH_IOCTL/COUNTER_ENABLE_EVENTS_IOCTL ioctl calls and > reading the event data out of the queue. You will need to first define > an array of struct counter_watch indicating that you want to watch the > "capture" attributes of the CTR count; something like this (assuming > event channel 0):: > > /* assuming capture attributes are under the count0 directory */ > #define CAPTURE_WATCH(_id, _channel) \ > { \ > .component.type = COUNTER_COMPONENT_EXTENSION, \ > .component.scope = COUNTER_SCOPE_COUNT, \ > .component.parent = 0, \ > .component.id = _id, \ > .event = COUNTER_EVENT_CAPTURE, \ > .channel = _channel, \ > } > /* get id from respective "captureX_component_id" attributes */ > static struct counter_watch watches[4] = { > CAPTURE_WATCH(42, 0), > CAPTURE_WATCH(43, 0), > CAPTURE_WATCH(44, 0), > CAPTURE_WATCH(45, 0), > }; > > Later you add the watches, enable events, and finally read the event > data as it arrives:: > > struct counter_event event_data[4]; > > ioctl(fd, COUNTER_ADD_WATCH_IOCTL, &watches[0]); > ioctl(fd, COUNTER_ADD_WATCH_IOCTL, &watches[1]); > ioctl(fd, COUNTER_ADD_WATCH_IOCTL, &watches[2]); > ioctl(fd, COUNTER_ADD_WATCH_IOCTL, &watches[3]); > ioctl(fd, COUNTER_ENABLE_EVENTS_IOCTL); > > for (;;) { > read(fd, event_data, sizeof(event_data)); > printf("cap1: %llu", event_data[0].value); > printf("cap2: %llu", event_data[1].value); > printf("cap3: %llu", event_data[2].value); > printf("cap4: %llu", event_data[3].value); > } > > If you want to keep track of channel, you can take a look under the > event_data[i].watch.channel member. > > William Breathitt Gray
Thank you for all these explanations. After a few hours working with counter subsystem I began to understand how it works, but now it's fully clear.
| |