2022-09-20 20:37:45

by William Breathitt Gray

[permalink] [raw]
Subject: [PATCH v5 0/5] Add support for Counter array components

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

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
--
2.37.3


2022-09-20 20:41:43

by William Breathitt Gray

[permalink] [raw]
Subject: [PATCH v5 2/5] counter: 104-quad-8: Add Signal polarity component

The 104-quad-8 driver provides support for Index signal polarity modes
via the "index_polarity" Signal component. This patch exposes the same
functionality through the more standard "polarity" Signal component.

Signed-off-by: William Breathitt Gray <[email protected]>
---
drivers/counter/104-quad-8.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)

diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c
index 1323edfbe40c..2a9d8259ed4b 100644
--- a/drivers/counter/104-quad-8.c
+++ b/drivers/counter/104-quad-8.c
@@ -549,6 +549,32 @@ static int quad8_index_polarity_set(struct counter_device *counter,
return 0;
}

+static int quad8_polarity_read(struct counter_device *counter,
+ struct counter_signal *signal,
+ enum counter_signal_polarity *polarity)
+{
+ int err;
+ u32 index_polarity;
+
+ err = quad8_index_polarity_get(counter, signal, &index_polarity);
+ if (err)
+ return err;
+
+ *polarity = (index_polarity) ? COUNTER_SIGNAL_POLARITY_POSITIVE :
+ COUNTER_SIGNAL_POLARITY_NEGATIVE;
+
+ return 0;
+}
+
+static int quad8_polarity_write(struct counter_device *counter,
+ struct counter_signal *signal,
+ enum counter_signal_polarity polarity)
+{
+ const u32 pol = (polarity == COUNTER_SIGNAL_POLARITY_POSITIVE) ? 1 : 0;
+
+ return quad8_index_polarity_set(counter, signal, pol);
+}
+
static const char *const quad8_synchronous_modes[] = {
"non-synchronous",
"synchronous"
@@ -977,6 +1003,13 @@ static struct counter_comp quad8_signal_ext[] = {
quad8_signal_fck_prescaler_write)
};

+static const enum counter_signal_polarity quad8_polarities[] = {
+ COUNTER_SIGNAL_POLARITY_POSITIVE,
+ COUNTER_SIGNAL_POLARITY_NEGATIVE,
+};
+
+static DEFINE_COUNTER_AVAILABLE(quad8_polarity_available, quad8_polarities);
+
static DEFINE_COUNTER_ENUM(quad8_index_pol_enum, quad8_index_polarity_modes);
static DEFINE_COUNTER_ENUM(quad8_synch_mode_enum, quad8_synchronous_modes);

@@ -984,6 +1017,8 @@ static struct counter_comp quad8_index_ext[] = {
COUNTER_COMP_SIGNAL_ENUM("index_polarity", quad8_index_polarity_get,
quad8_index_polarity_set,
quad8_index_pol_enum),
+ COUNTER_COMP_POLARITY(quad8_polarity_read, quad8_polarity_write,
+ quad8_polarity_available),
COUNTER_COMP_SIGNAL_ENUM("synchronous_mode", quad8_synchronous_mode_get,
quad8_synchronous_mode_set,
quad8_synch_mode_enum),
--
2.37.3

2022-09-20 20:43:15

by William Breathitt Gray

[permalink] [raw]
Subject: [PATCH v5 3/5] counter: Introduce the Count capture component

Some devices provide a latch function to save historic Count values.
This patch standardizes exposure of such functionality as Count capture
components. A COUNTER_COMP_CAPTURE macro is provided for driver authors
to define a capture component. A new event COUNTER_EVENT_CAPTURE is
introduced to represent Count value capture events.

Cc: Julien Panis <[email protected]>
Signed-off-by: William Breathitt Gray <[email protected]>
---
Documentation/ABI/testing/sysfs-bus-counter | 6 ++++++
include/linux/counter.h | 3 +++
include/uapi/linux/counter.h | 2 ++
3 files changed, 11 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-counter b/Documentation/ABI/testing/sysfs-bus-counter
index a234022f9add..30b6e1faa6f6 100644
--- a/Documentation/ABI/testing/sysfs-bus-counter
+++ b/Documentation/ABI/testing/sysfs-bus-counter
@@ -4,6 +4,12 @@ Contact: [email protected]
Description:
Count data of Count Y represented as a string.

+What: /sys/bus/counter/devices/counterX/countY/capture
+KernelVersion: 6.1
+Contact: [email protected]
+Description:
+ Historical capture of the Count Y count data.
+
What: /sys/bus/counter/devices/counterX/countY/ceiling
KernelVersion: 5.2
Contact: [email protected]
diff --git a/include/linux/counter.h b/include/linux/counter.h
index 60428d06915d..2c6594c240d4 100644
--- a/include/linux/counter.h
+++ b/include/linux/counter.h
@@ -453,6 +453,9 @@ struct counter_available {
.priv = &(_available), \
}

+#define COUNTER_COMP_CAPTURE(_read, _write) \
+ COUNTER_COMP_COUNT_U64("capture", _read, _write)
+
#define COUNTER_COMP_CEILING(_read, _write) \
COUNTER_COMP_COUNT_U64("ceiling", _read, _write)

diff --git a/include/uapi/linux/counter.h b/include/uapi/linux/counter.h
index e9610e1944dc..8ab12d731e3b 100644
--- a/include/uapi/linux/counter.h
+++ b/include/uapi/linux/counter.h
@@ -63,6 +63,8 @@ enum counter_event_type {
COUNTER_EVENT_INDEX,
/* State of counter is changed */
COUNTER_EVENT_CHANGE_OF_STATE,
+ /* Count value captured */
+ COUNTER_EVENT_CAPTURE,
};

/**
--
2.37.3

2022-09-20 20:44:51

by William Breathitt Gray

[permalink] [raw]
Subject: [PATCH v5 4/5] counter: Consolidate Counter extension sysfs attribute creation

Counter extensions are handled for the Device, Counts, and Signals. The
code loops through each Counter extension and creates the expected sysfs
attributes. This patch consolidates that code into functions to reduce
redundancy and make the intention of the code clearer.

Signed-off-by: William Breathitt Gray <[email protected]>
---
drivers/counter/counter-sysfs.c | 98 ++++++++++++++++-----------------
1 file changed, 49 insertions(+), 49 deletions(-)

diff --git a/drivers/counter/counter-sysfs.c b/drivers/counter/counter-sysfs.c
index e5dd36e1a45f..b393da402e0b 100644
--- a/drivers/counter/counter-sysfs.c
+++ b/drivers/counter/counter-sysfs.c
@@ -592,6 +592,46 @@ static int counter_comp_id_attr_create(struct device *const dev,
return 0;
}

+static int counter_ext_attrs_create(struct device *const dev,
+ struct counter_attribute_group *const group,
+ const struct counter_comp *const ext,
+ const enum counter_scope scope,
+ void *const parent, const size_t id)
+{
+ int err;
+
+ /* Create main extension attribute */
+ err = counter_attr_create(dev, group, ext, scope, parent);
+ if (err < 0)
+ return err;
+
+ /* Create extension id attribute */
+ return counter_comp_id_attr_create(dev, group, ext->name, id);
+}
+
+static int counter_sysfs_exts_add(struct device *const dev,
+ struct counter_attribute_group *const group,
+ const struct counter_comp *const exts,
+ const size_t num_ext,
+ const enum counter_scope scope,
+ void *const parent)
+{
+ size_t i;
+ const struct counter_comp *ext;
+ int err;
+
+ /* Create attributes for each extension */
+ for (i = 0; i < num_ext; i++) {
+ ext = &exts[i];
+ err = counter_ext_attrs_create(dev, group, ext, scope, parent,
+ i);
+ if (err < 0)
+ return err;
+ }
+
+ return 0;
+}
+
static struct counter_comp counter_signal_comp = {
.type = COUNTER_COMP_SIGNAL_LEVEL,
.name = "signal",
@@ -605,8 +645,6 @@ static int counter_signal_attrs_create(struct counter_device *const counter,
struct device *const dev = &counter->dev;
int err;
struct counter_comp comp;
- size_t i;
- struct counter_comp *ext;

/* Create main Signal attribute */
comp = counter_signal_comp;
@@ -620,21 +658,9 @@ static int counter_signal_attrs_create(struct counter_device *const counter,
if (err < 0)
return err;

- /* Create an attribute for each extension */
- for (i = 0; i < signal->num_ext; i++) {
- ext = &signal->ext[i];
-
- err = counter_attr_create(dev, cattr_group, ext, scope, signal);
- if (err < 0)
- return err;
-
- err = counter_comp_id_attr_create(dev, cattr_group, ext->name,
- i);
- if (err < 0)
- return err;
- }
-
- return 0;
+ /* Add Signal extensions */
+ return counter_sysfs_exts_add(dev, cattr_group, signal->ext,
+ signal->num_ext, scope, signal);
}

static int counter_sysfs_signals_add(struct counter_device *const counter,
@@ -719,8 +745,6 @@ static int counter_count_attrs_create(struct counter_device *const counter,
struct device *const dev = &counter->dev;
int err;
struct counter_comp comp;
- size_t i;
- struct counter_comp *ext;

/* Create main Count attribute */
comp = counter_count_comp;
@@ -743,21 +767,9 @@ static int counter_count_attrs_create(struct counter_device *const counter,
if (err < 0)
return err;

- /* Create an attribute for each extension */
- for (i = 0; i < count->num_ext; i++) {
- ext = &count->ext[i];
-
- err = counter_attr_create(dev, cattr_group, ext, scope, count);
- if (err < 0)
- return err;
-
- err = counter_comp_id_attr_create(dev, cattr_group, ext->name,
- i);
- if (err < 0)
- return err;
- }
-
- return 0;
+ /* Add Count extensions */
+ return counter_sysfs_exts_add(dev, cattr_group, count->ext,
+ count->num_ext, scope, count);
}

static int counter_sysfs_counts_add(struct counter_device *const counter,
@@ -850,8 +862,6 @@ static int counter_sysfs_attr_add(struct counter_device *const counter,
const enum counter_scope scope = COUNTER_SCOPE_DEVICE;
struct device *const dev = &counter->dev;
int err;
- size_t i;
- struct counter_comp *ext;

/* Add Signals sysfs attributes */
err = counter_sysfs_signals_add(counter, cattr_group);
@@ -888,19 +898,9 @@ static int counter_sysfs_attr_add(struct counter_device *const counter,
if (err < 0)
return err;

- /* Create an attribute for each extension */
- for (i = 0; i < counter->num_ext; i++) {
- ext = &counter->ext[i];
-
- err = counter_attr_create(dev, cattr_group, ext, scope, NULL);
- if (err < 0)
- return err;
-
- err = counter_comp_id_attr_create(dev, cattr_group, ext->name,
- i);
- if (err < 0)
- return err;
- }
+ /* Add device extensions */
+ return counter_sysfs_exts_add(dev, cattr_group, counter->ext,
+ counter->num_ext, scope, NULL);

return 0;
}
--
2.37.3

2022-09-21 10:07:13

by Julien Panis

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

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 <[email protected]>

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