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
| ||
|
Date: Tue, 9 Aug 2022 08:23:53 +0200 From: Julien Panis <jpanis@...libre.com> To: William Breathitt Gray <william.gray@...aro.org> Cc: Jonathan Cameron <jic23@...nel.org>, robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org, lars@...afoo.de, linux-iio@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, mranostay@...com Subject: Re: [PATCH v3 2/2] iio: time: capture-tiecap: capture driver support for ECAP 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.
Powered by blists - more mailing lists