Re: [PATCH v5 0/5] Add support for Counter array components

[Julien Panis <jpanis@baylibre.com>]

Hi William,

On 20/09/2022 19:21, William Breathitt Gray wrote:
> Changes in v5:
>   - Introduce the Count capture component
>   - Add DEFINE_COUNTER_ARRAY_CAPTURE macro and COUNTER_COMP_ARRAY_CAPTURE
>     for Counter arrays of capture elements
>   - Fix Counter array handling in counter-chrdev.c; previously
>     misinterpreted component id for COUNTER_COMPONENT_EXTENSION type when
>     Counter array components were present

This v5 works with TI ECAP capture driver !

Tested-by: Julien Panis <jpanis@baylibre.com>

Julien

>
> The COUNTER_COMP_ARRAY Counter component type is introduced to enable
> support for Counter array components. With Counter array components,
> exposure for buffers on counter devices can be defined via new Counter
> array component macros. This should simplify code for driver authors who
> would otherwise need to define individual Counter components for each
> array element.
>
> Eight Counter array component macros are introduced::
>
>          DEFINE_COUNTER_ARRAY_U64(_name, _length)
>          DEFINE_COUNTER_ARRAY_CAPTURE(_name, _length)
>          DEFINE_COUNTER_ARRAY_POLARITY(_name, _enums, _length)
>          COUNTER_COMP_DEVICE_ARRAY_U64(_name, _read, _write, _array)
>          COUNTER_COMP_COUNT_ARRAY_U64(_name, _read, _write, _array)
>          COUNTER_COMP_SIGNAL_ARRAY_U64(_name, _read, _write, _array)
>          COUNTER_COMP_ARRAY_CAPTURE(_read, _write, _array)
>          COUNTER_COMP_ARRAY_POLARITY(_read, _write, _array)
>
> Eight Counter array callbacks are introduced as well::
>
>          int (*signal_array_u32_read)(struct counter_device *counter,
>                                       struct counter_signal *signal,
>                                       size_t idx, u32 *val);
>          int (*signal_array_u32_write)(struct counter_device *counter,
>                                        struct counter_signal *signal,
>                                        size_t idx, u32 val);
>          int (*device_array_u64_read)(struct counter_device *counter,
>                                       size_t idx, u64 *val);
>          int (*count_array_u64_read)(struct counter_device *counter,
>                                      struct counter_count *count,
>                                      size_t idx, u64 *val);
>          int (*signal_array_u64_read)(struct counter_device *counter,
>                                       struct counter_signal *signal,
>                                       size_t idx, u64 *val);
>          int (*device_array_u64_write)(struct counter_device *counter,
>                                        size_t idx, u64 val);
>          int (*count_array_u64_write)(struct counter_device *counter,
>                                       struct counter_count *count,
>                                       size_t idx, u64 val);
>          int (*signal_array_u64_write)(struct counter_device *counter,
>                                        struct counter_signal *signal,
>                                        size_t idx, u64 val);
>
> Driver authors can handle reads/writes for an array component by
> receiving an element index via the `idx` parameter and processing the
> respective value via the `val` parameter.
>
> For example, suppose a driver wants to expose a Count's read-only
> capture buffer of four elements using a callback
> `foobar_capture_read()`::
>
>          DEFINE_COUNTER_ARRAY_CAPTURE(foobar_capture_array, 4);
>          COUNTER_COMP_ARRAY_CAPTURE(foobar_capture_read, NULL,
>                                     foobar_capture_array)
>
> Respective sysfs attributes for each array element would appear for the
> respective Count:
>
> * /sys/bus/counter/devices/counterX/countY/capture0
> * /sys/bus/counter/devices/counterX/countY/capture1
> * /sys/bus/counter/devices/counterX/countY/capture2
> * /sys/bus/counter/devices/counterX/countY/capture3
>
> If a user tries to read _capture2_ for example, `idx` will be `2` when
> passed to the `foobar_capture_read()` callback, and thus the driver
> knows which array element to handle.
>
> In addition, this patchset introduces the Signal polarity component,
> which represents the active level of a respective Signal. There are two
> possible states: positive (rising edge) and negative (falling edge). The
> 104-quad-8 driver is updated to expose its index_polarity functionality
> via this new polarity component.
>
> Counter arrays for polarity elements can be defined in a similar
> manner as u64 elements::
>
>          const enum counter_signal_polarity foobar_polarity_states[] = {
>                  COUNTER_SIGNAL_POLARITY_POSITIVE,
>                  COUNTER_SIGNAL_POLARITY_NEGATIVE,
>          };
>          DEFINE_COUNTER_ARRAY_POLARITY(foobar_polarity_array,
>                                        foobar_polarity_states, 4);
>          COUNTER_COMP_ARRAY_POLARITY(foobar_polarity_read,
>                                      foobar_polarity_write,
>                                      foobar_polarity_array)
>
> The only component types supported for Counter arrays currently are
> COUNTER_COMP_U64 and COUNTER_COMP_SIGNAL_POLARITY.
>
> William Breathitt Gray (5):
>    counter: Introduce the Signal polarity component
>    counter: 104-quad-8: Add Signal polarity component
>    counter: Introduce the Count capture component
>    counter: Consolidate Counter extension sysfs attribute creation
>    counter: Introduce the COUNTER_COMP_ARRAY component type
>
>   Documentation/ABI/testing/sysfs-bus-counter |  19 ++
>   drivers/counter/104-quad-8.c                |  35 +++
>   drivers/counter/counter-chrdev.c            | 135 +++++++--
>   drivers/counter/counter-sysfs.c             | 304 ++++++++++++++++----
>   include/linux/counter.h                     | 147 ++++++++++
>   include/uapi/linux/counter.h                |   8 +
>   6 files changed, 581 insertions(+), 67 deletions(-)
>
>
> base-commit: f95ec98139dc58db72e4bd0df049a3097990a8e7