From: William Breathitt Gray <vilhelm.gray@gmail.com>
To: jic23@kernel.org
Cc: kamel.bouhara@bootlin.com, gwendal@chromium.org,
alexandre.belloni@bootlin.com, david@lechnology.com,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, syednwaris@gmail.com,
patrick.havelange@essensium.com, fabrice.gasnier@st.com,
mcoquelin.stm32@gmail.com, alexandre.torgue@st.com,
William Breathitt Gray <vilhelm.gray@gmail.com>
Subject: [PATCH v5 0/5] Introduce the Counter character device interface
Date: Sat, 26 Sep 2020 22:18:13 -0400 [thread overview]
Message-ID: <cover.1601170670.git.vilhelm.gray@gmail.com> (raw)
Changes in v5:
- Fixed typographical errors in documentation and comments
- Updated flow charts in documentation for clarity
- Moved uapi header to be part of the character device intro patch
- Fix git squash mistake in 104-quad-8.c; remove redundant changes
- Fix git merge mistake in 104-quad-8.c; fix locking race condition
- Minor code cleanup for clarity; adjust whitespace/flow
- Use put_device if device_add fails
- Document sysfs structures
- Rename "owner" symbols to "scope"; more apt name
- Use resource-managed devm_* allocation functions
- Rename *_free functions to *_remove; following common convention
- Rename COUNTER_DATA* to COUNTER_COMP*; more obvious meaning
- Rename various symbol and define names for clarity
- Bring back static ops function; more secure to have static const
- Rename counter_available union members to "enums" and "strs"
- Implement COUNTER_EVENT* constants; event types are now standard
- Implement atomic Counter watches swap; no more racy event config
Over the past couple years we have noticed some shortcomings with the
Counter sysfs interface. Although useful in the majority of situations,
there are certain use-cases where interacting through sysfs attributes
can become cumbersome and inefficient. A desire to support more advanced
functionality such as timestamps, multi-axes positioning tables, and
other such latency-sensitive applications, has motivated a reevaluation
of the Counter subsystem. I believe a character device interface will be
helpful for this more niche area of counter device use.
To quell any concerns from the offset: this patchset makes no changes to
the existing Counter sysfs userspace interface -- existing userspace
applications will continue to work with no modifications necessary. I
request that driver maintainers please test their applications to verify
that this is true, and report any discrepancies if they arise.
However, this patchset does contain a major reimplementation of the
Counter subsystem core and driver API. A reimplementation was necessary
in order to separate the sysfs code from the counter device drivers and
internalize it as a dedicated component of the core Counter subsystem
module. A minor benefit from all of this is that the sysfs interface is
now ensured a certain amount of consistency because the translation is
performed outside of individual counter device drivers.
Essentially, the reimplementation has enabled counter device drivers to
pass and handle data as native C datatypes now rather than the sysfs
strings from before.
A high-level view of how a count value is passed down from a counter
driver is exemplified by the following. The driver callbacks are first
registered to the Counter core component for use by the Counter
userspace interface components:
+----------------------------+
| Counter device driver |
+----------------------------+
| Processes data from device |
+----------------------------+
|
-------------------
/ driver callbacks /
-------------------
|
V
+----------------------+
| Counter core |
+----------------------+
| Routes device driver |
| callbacks to the |
| userspace interfaces |
+----------------------+
|
-------------------
/ driver callbacks /
-------------------
|
+---------------+---------------+
| |
V V
+--------------------+ +---------------------+
| Counter sysfs | | Counter chrdev |
+--------------------+ +---------------------+
| Translates to the | | Translates to the |
| standard Counter | | standard Counter |
| sysfs output | | character device |
+--------------------+ +---------------------+
Thereafter, data can be transferred directly between the Counter device
driver and Counter userspace interface:
----------------------
/ Counter device \
+----------------------+
| Count register: 0x28 |
+----------------------+
|
-----------------
/ raw count data /
-----------------
|
V
+----------------------------+
| Counter device driver |
+----------------------------+
| Processes data from device |
|----------------------------|
| Type: u64 |
| Value: 42 |
+----------------------------+
|
----------
/ u64 /
----------
|
+---------------+---------------+
| |
V V
+--------------------+ +---------------------+
| Counter sysfs | | Counter chrdev |
+--------------------+ +---------------------+
| Translates to the | | Translates to the |
| standard Counter | | standard Counter |
| sysfs output | | character device |
|--------------------| |---------------------|
| Type: const char * | | Type: u64 |
| Value: "42" | | Value: 42 |
+--------------------+ +---------------------+
| |
--------------- -----------------------
/ const char * / / struct counter_event /
--------------- -----------------------
| |
| V
| +-----------+
| | read |
| +-----------+
| \ Count: 42 /
| -----------
|
V
+--------------------------------------------------+
| `/sys/bus/counter/devices/counterX/countY/count` |
+--------------------------------------------------+
\ Count: "42" /
--------------------------------------------------
Counter device data is exposed through standard character device read
operations. Device data is gathered when a Counter event is pushed by
the respective Counter device driver. Configuration is handled via ioctl
operations on the respective Counter character device node.
The following are some questions I have about this patchset:
1. Should standard Counter component data types be defined as u8 or u32?
Many standard Counter component types such COUNTER_COMP_SIGNAL_LEVEL
have standard values defined (e.g. COUNTER_SIGNAL_LEVEL_LOW and
COUNTER_SIGNAL_LEVEL_HIGH). These values are currently handled by the
Counter subsystem code as u8 data types.
If u32 is used for these values instead, C enum structures could be
used by driver authors to implicit cast these values via the driver
callback parameters; userspace would still use u32 with no issue.
In theory this can work because GCC will treat enums are having a
32-bit size; but I worry about the possibility of build targets that
have -fshort-enums enabled, resulting in enums having a size less
than 32 bits. Would this be a problem?
2. Should I have reserved members in the userspace structures?
The structures in include/uapi/linux/counter.h are available to
userspace applications. Should I reserve space in these structures
for future additions and usage? Will endianess and packing be a
concern here?
William Breathitt Gray (5):
counter: Internalize sysfs interface code
docs: counter: Update to reflect sysfs internalization
counter: Add character device interface
docs: counter: Document character device interface
counter: 104-quad-8: Add IRQ support for the ACCES 104-QUAD-8
Documentation/ABI/testing/sysfs-bus-counter | 18 +
.../ABI/testing/sysfs-bus-counter-104-quad-8 | 32 +
Documentation/driver-api/generic-counter.rst | 408 ++++-
.../userspace-api/ioctl/ioctl-number.rst | 1 +
MAINTAINERS | 2 +-
drivers/counter/104-quad-8.c | 775 +++++----
drivers/counter/Kconfig | 6 +-
drivers/counter/Makefile | 1 +
drivers/counter/counter-chrdev.c | 451 +++++
drivers/counter/counter-chrdev.h | 16 +
drivers/counter/counter-core.c | 190 +++
drivers/counter/counter-sysfs.c | 862 ++++++++++
drivers/counter/counter-sysfs.h | 13 +
drivers/counter/counter.c | 1496 -----------------
drivers/counter/ftm-quaddec.c | 60 +-
drivers/counter/microchip-tcb-capture.c | 100 +-
drivers/counter/stm32-lptimer-cnt.c | 175 +-
drivers/counter/stm32-timer-cnt.c | 145 +-
drivers/counter/ti-eqep.c | 226 +--
include/linux/counter.h | 618 +++----
include/linux/counter_enum.h | 45 -
include/uapi/linux/counter.h | 99 ++
22 files changed, 3053 insertions(+), 2686 deletions(-)
create mode 100644 drivers/counter/counter-chrdev.c
create mode 100644 drivers/counter/counter-chrdev.h
create mode 100644 drivers/counter/counter-core.c
create mode 100644 drivers/counter/counter-sysfs.c
create mode 100644 drivers/counter/counter-sysfs.h
delete mode 100644 drivers/counter/counter.c
delete mode 100644 include/linux/counter_enum.h
create mode 100644 include/uapi/linux/counter.h
--
2.28.0
WARNING: multiple messages have this Message-ID (diff)
From: William Breathitt Gray <vilhelm.gray@gmail.com>
To: jic23@kernel.org
Cc: kamel.bouhara@bootlin.com, gwendal@chromium.org,
david@lechnology.com, linux-iio@vger.kernel.org,
patrick.havelange@essensium.com, alexandre.belloni@bootlin.com,
linux-kernel@vger.kernel.org, mcoquelin.stm32@gmail.com,
William Breathitt Gray <vilhelm.gray@gmail.com>,
fabrice.gasnier@st.com, syednwaris@gmail.com,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, alexandre.torgue@st.com
Subject: [PATCH v5 0/5] Introduce the Counter character device interface
Date: Sat, 26 Sep 2020 22:18:13 -0400 [thread overview]
Message-ID: <cover.1601170670.git.vilhelm.gray@gmail.com> (raw)
Changes in v5:
- Fixed typographical errors in documentation and comments
- Updated flow charts in documentation for clarity
- Moved uapi header to be part of the character device intro patch
- Fix git squash mistake in 104-quad-8.c; remove redundant changes
- Fix git merge mistake in 104-quad-8.c; fix locking race condition
- Minor code cleanup for clarity; adjust whitespace/flow
- Use put_device if device_add fails
- Document sysfs structures
- Rename "owner" symbols to "scope"; more apt name
- Use resource-managed devm_* allocation functions
- Rename *_free functions to *_remove; following common convention
- Rename COUNTER_DATA* to COUNTER_COMP*; more obvious meaning
- Rename various symbol and define names for clarity
- Bring back static ops function; more secure to have static const
- Rename counter_available union members to "enums" and "strs"
- Implement COUNTER_EVENT* constants; event types are now standard
- Implement atomic Counter watches swap; no more racy event config
Over the past couple years we have noticed some shortcomings with the
Counter sysfs interface. Although useful in the majority of situations,
there are certain use-cases where interacting through sysfs attributes
can become cumbersome and inefficient. A desire to support more advanced
functionality such as timestamps, multi-axes positioning tables, and
other such latency-sensitive applications, has motivated a reevaluation
of the Counter subsystem. I believe a character device interface will be
helpful for this more niche area of counter device use.
To quell any concerns from the offset: this patchset makes no changes to
the existing Counter sysfs userspace interface -- existing userspace
applications will continue to work with no modifications necessary. I
request that driver maintainers please test their applications to verify
that this is true, and report any discrepancies if they arise.
However, this patchset does contain a major reimplementation of the
Counter subsystem core and driver API. A reimplementation was necessary
in order to separate the sysfs code from the counter device drivers and
internalize it as a dedicated component of the core Counter subsystem
module. A minor benefit from all of this is that the sysfs interface is
now ensured a certain amount of consistency because the translation is
performed outside of individual counter device drivers.
Essentially, the reimplementation has enabled counter device drivers to
pass and handle data as native C datatypes now rather than the sysfs
strings from before.
A high-level view of how a count value is passed down from a counter
driver is exemplified by the following. The driver callbacks are first
registered to the Counter core component for use by the Counter
userspace interface components:
+----------------------------+
| Counter device driver |
+----------------------------+
| Processes data from device |
+----------------------------+
|
-------------------
/ driver callbacks /
-------------------
|
V
+----------------------+
| Counter core |
+----------------------+
| Routes device driver |
| callbacks to the |
| userspace interfaces |
+----------------------+
|
-------------------
/ driver callbacks /
-------------------
|
+---------------+---------------+
| |
V V
+--------------------+ +---------------------+
| Counter sysfs | | Counter chrdev |
+--------------------+ +---------------------+
| Translates to the | | Translates to the |
| standard Counter | | standard Counter |
| sysfs output | | character device |
+--------------------+ +---------------------+
Thereafter, data can be transferred directly between the Counter device
driver and Counter userspace interface:
----------------------
/ Counter device \
+----------------------+
| Count register: 0x28 |
+----------------------+
|
-----------------
/ raw count data /
-----------------
|
V
+----------------------------+
| Counter device driver |
+----------------------------+
| Processes data from device |
|----------------------------|
| Type: u64 |
| Value: 42 |
+----------------------------+
|
----------
/ u64 /
----------
|
+---------------+---------------+
| |
V V
+--------------------+ +---------------------+
| Counter sysfs | | Counter chrdev |
+--------------------+ +---------------------+
| Translates to the | | Translates to the |
| standard Counter | | standard Counter |
| sysfs output | | character device |
|--------------------| |---------------------|
| Type: const char * | | Type: u64 |
| Value: "42" | | Value: 42 |
+--------------------+ +---------------------+
| |
--------------- -----------------------
/ const char * / / struct counter_event /
--------------- -----------------------
| |
| V
| +-----------+
| | read |
| +-----------+
| \ Count: 42 /
| -----------
|
V
+--------------------------------------------------+
| `/sys/bus/counter/devices/counterX/countY/count` |
+--------------------------------------------------+
\ Count: "42" /
--------------------------------------------------
Counter device data is exposed through standard character device read
operations. Device data is gathered when a Counter event is pushed by
the respective Counter device driver. Configuration is handled via ioctl
operations on the respective Counter character device node.
The following are some questions I have about this patchset:
1. Should standard Counter component data types be defined as u8 or u32?
Many standard Counter component types such COUNTER_COMP_SIGNAL_LEVEL
have standard values defined (e.g. COUNTER_SIGNAL_LEVEL_LOW and
COUNTER_SIGNAL_LEVEL_HIGH). These values are currently handled by the
Counter subsystem code as u8 data types.
If u32 is used for these values instead, C enum structures could be
used by driver authors to implicit cast these values via the driver
callback parameters; userspace would still use u32 with no issue.
In theory this can work because GCC will treat enums are having a
32-bit size; but I worry about the possibility of build targets that
have -fshort-enums enabled, resulting in enums having a size less
than 32 bits. Would this be a problem?
2. Should I have reserved members in the userspace structures?
The structures in include/uapi/linux/counter.h are available to
userspace applications. Should I reserve space in these structures
for future additions and usage? Will endianess and packing be a
concern here?
William Breathitt Gray (5):
counter: Internalize sysfs interface code
docs: counter: Update to reflect sysfs internalization
counter: Add character device interface
docs: counter: Document character device interface
counter: 104-quad-8: Add IRQ support for the ACCES 104-QUAD-8
Documentation/ABI/testing/sysfs-bus-counter | 18 +
.../ABI/testing/sysfs-bus-counter-104-quad-8 | 32 +
Documentation/driver-api/generic-counter.rst | 408 ++++-
.../userspace-api/ioctl/ioctl-number.rst | 1 +
MAINTAINERS | 2 +-
drivers/counter/104-quad-8.c | 775 +++++----
drivers/counter/Kconfig | 6 +-
drivers/counter/Makefile | 1 +
drivers/counter/counter-chrdev.c | 451 +++++
drivers/counter/counter-chrdev.h | 16 +
drivers/counter/counter-core.c | 190 +++
drivers/counter/counter-sysfs.c | 862 ++++++++++
drivers/counter/counter-sysfs.h | 13 +
drivers/counter/counter.c | 1496 -----------------
drivers/counter/ftm-quaddec.c | 60 +-
drivers/counter/microchip-tcb-capture.c | 100 +-
drivers/counter/stm32-lptimer-cnt.c | 175 +-
drivers/counter/stm32-timer-cnt.c | 145 +-
drivers/counter/ti-eqep.c | 226 +--
include/linux/counter.h | 618 +++----
include/linux/counter_enum.h | 45 -
include/uapi/linux/counter.h | 99 ++
22 files changed, 3053 insertions(+), 2686 deletions(-)
create mode 100644 drivers/counter/counter-chrdev.c
create mode 100644 drivers/counter/counter-chrdev.h
create mode 100644 drivers/counter/counter-core.c
create mode 100644 drivers/counter/counter-sysfs.c
create mode 100644 drivers/counter/counter-sysfs.h
delete mode 100644 drivers/counter/counter.c
delete mode 100644 include/linux/counter_enum.h
create mode 100644 include/uapi/linux/counter.h
--
2.28.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next reply other threads:[~2020-09-27 2:18 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-27 2:18 William Breathitt Gray [this message]
2020-09-27 2:18 ` [PATCH v5 0/5] Introduce the Counter character device interface William Breathitt Gray
2020-09-27 2:18 ` [PATCH v5 1/5] counter: Internalize sysfs interface code William Breathitt Gray
2020-10-13 2:15 ` David Lechner
2020-10-13 2:15 ` David Lechner
2020-10-18 14:49 ` William Breathitt Gray
2020-10-18 14:49 ` William Breathitt Gray
2020-10-20 15:38 ` David Lechner
2020-10-20 15:38 ` David Lechner
2020-10-23 13:12 ` William Breathitt Gray
2020-10-23 13:12 ` William Breathitt Gray
2020-10-15 1:38 ` David Lechner
2020-10-15 1:38 ` David Lechner
2020-10-18 17:00 ` William Breathitt Gray
2020-10-18 17:00 ` William Breathitt Gray
2020-09-27 2:18 ` [PATCH v5 2/5] docs: counter: Update to reflect sysfs internalization William Breathitt Gray
2020-09-27 2:18 ` William Breathitt Gray
2020-09-27 2:18 ` [PATCH v5 3/5] counter: Add character device interface William Breathitt Gray
2020-09-27 2:18 ` William Breathitt Gray
2020-10-14 1:40 ` David Lechner
2020-10-14 1:40 ` David Lechner
2020-10-18 16:49 ` William Breathitt Gray
2020-10-18 16:49 ` William Breathitt Gray
2020-10-20 15:53 ` David Lechner
2020-10-20 15:53 ` David Lechner
2020-10-25 12:55 ` William Breathitt Gray
2020-10-25 12:55 ` William Breathitt Gray
2020-10-25 16:36 ` David Lechner
2020-10-25 16:36 ` David Lechner
2020-10-14 17:43 ` David Lechner
2020-10-14 17:43 ` David Lechner
2020-10-14 19:05 ` William Breathitt Gray
2020-10-14 19:05 ` William Breathitt Gray
2020-10-14 22:32 ` David Lechner
2020-10-14 22:32 ` David Lechner
2020-10-14 22:40 ` David Lechner
2020-10-14 22:40 ` David Lechner
2020-10-18 16:58 ` William Breathitt Gray
2020-10-18 16:58 ` William Breathitt Gray
2020-10-20 16:06 ` David Lechner
2020-10-20 16:06 ` David Lechner
2020-10-25 13:18 ` William Breathitt Gray
2020-10-25 13:18 ` William Breathitt Gray
2020-10-25 16:34 ` David Lechner
2020-10-25 16:34 ` David Lechner
2020-10-25 17:53 ` William Breathitt Gray
2020-10-25 17:53 ` William Breathitt Gray
2020-09-27 2:18 ` [PATCH v5 4/5] docs: counter: Document " William Breathitt Gray
2020-09-27 2:18 ` William Breathitt Gray
2020-10-08 8:09 ` Pavel Machek
2020-10-08 8:09 ` Pavel Machek
2020-10-08 12:28 ` William Breathitt Gray
2020-10-08 12:28 ` William Breathitt Gray
2020-10-12 17:04 ` David Lechner
2020-10-12 17:04 ` David Lechner
2020-10-13 18:58 ` William Breathitt Gray
2020-10-13 18:58 ` William Breathitt Gray
2020-10-13 19:08 ` David Lechner
2020-10-13 19:08 ` David Lechner
2020-10-13 19:27 ` William Breathitt Gray
2020-10-13 19:27 ` William Breathitt Gray
2020-09-27 2:18 ` [PATCH v5 5/5] counter: 104-quad-8: Add IRQ support for the ACCES 104-QUAD-8 William Breathitt Gray
2020-09-27 2:18 ` William Breathitt Gray
2020-10-14 0:13 ` David Lechner
2020-10-14 0:13 ` David Lechner
2020-10-18 14:50 ` William Breathitt Gray
2020-10-18 14:50 ` William Breathitt Gray
2020-10-13 0:35 ` [PATCH v5 0/5] Introduce the Counter character device interface David Lechner
2020-10-13 0:35 ` David Lechner
2020-10-18 14:14 ` William Breathitt Gray
2020-10-18 14:14 ` William Breathitt Gray
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1601170670.git.vilhelm.gray@gmail.com \
--to=vilhelm.gray@gmail.com \
--cc=alexandre.belloni@bootlin.com \
--cc=alexandre.torgue@st.com \
--cc=david@lechnology.com \
--cc=fabrice.gasnier@st.com \
--cc=gwendal@chromium.org \
--cc=jic23@kernel.org \
--cc=kamel.bouhara@bootlin.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=mcoquelin.stm32@gmail.com \
--cc=patrick.havelange@essensium.com \
--cc=syednwaris@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.