Re: [PATCH v4 0/3] ECAP support on TI AM62x SoC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 14/08/2022 20:00, William Breathitt Gray wrote:
On Wed, Aug 10, 2022 at 04:07:21PM +0200, Julien Panis wrote:
The Enhanced Capture (ECAP) module can be used to timestamp events
detected on signal input pin. It can be used for time measurements
of pulse train signals.

ECAP module includes 4 timestamp capture registers. For all 4 sequenced
timestamp capture events (1->2->3->4->1->...), edge polarity (falling/rising
edge) can be selected.

This driver leverages counter subsystem to :
- select edge polarity for all 4 capture events (event mode)
- log timestamps for each capture event
Event polarity, and CAP1/2/3/4 timestamps give all the information
about the input pulse train. Further information can easily be computed :
period and/or duty cycle if frequency is constant, elapsed time between
pulses, etc...

Modifications since v3:
	- Migrate driver from IIO to Counter subsystem
	- Minor modification in yaml ($id) to match Counter subsystem
	- Add ABI documentation

Userspace commands :
	### SIGNAL ###
	cd /sys/bus/counter/devices/counter0/signal0

	# Get available polarities for each capture event
	cat polarity1_available
	cat polarity2_available
	cat polarity3_available
	cat polarity4_available

	# Get polarity for each capture event
	cat polarity1
	cat polarity2
	cat polarity3
	cat polarity4

	# Set polarity for each capture event
	echo rising > polarity1
	echo falling > polarity2
	echo rising > polarity3
	echo falling > polarity4

	### COUNT ###
	cd /sys/bus/counter/devices/counter0/count0

	# Run ECAP
	echo 1 > enable

	# Get current timebase counter value
	cat count

	# Get captured timestamps
	cat capture1
	cat capture2
	cat capture3
	cat capture4

	# Note that counter watches can also be used to get
	# data from userspace application
	# -> see tools/counter/counter_example.c

	# Stop ECAP
	echo 0 > enable

Julien Panis (3):
   dt-binding: counter: add ti,am62-ecap-capture.yaml
   Documentation: ABI: add sysfs-bus-counter-ecap
   counter: capture-tiecap: capture driver support for ECAP

  .../ABI/testing/sysfs-bus-counter-ecap        |  64 ++
  .../counter/ti,am62-ecap-capture.yaml         |  61 ++
  drivers/counter/Kconfig                       |  14 +
  drivers/counter/Makefile                      |   1 +
  drivers/counter/capture-tiecap.c              | 634 ++++++++++++++++++
  include/uapi/linux/counter.h                  |   2 +
  6 files changed, 776 insertions(+)
  create mode 100644 Documentation/ABI/testing/sysfs-bus-counter-ecap
  create mode 100644 Documentation/devicetree/bindings/counter/ti,am62-ecap-capture.yaml
  create mode 100644 drivers/counter/capture-tiecap.c

--
2.25.1
Something that has become apparent to me is the code repetition in this
driver in order to support the capture buffer. Buffers are common
components in devices, so it'll be good for us to standardize some of
what we're exploring here into an interface that other drivers can also
use. We have two ABIs to consider: the driver interface and the sysfs
interface.

For the sysfs interface, I think we'll have to expose each element
individually (e.g. capture1, capture2, etc.) because sysfs attributes
are suppose to expose only a single datum for any given attribute.

For the driver side, we might want to introduce a new Counter component
type for buffers and respective macros to streamline some of the code
for driver authors. For example, a new COUNTER_COMP_BUFFER_U64 enum
counter_comp_type constant could be introduced to represent a u64 buffer
element; respective struct counter_comp read callbacks could be
introduced::

     int (*count_buffer_u64_read)(struct counter_device *counter,
                                  struct counter_count *count,
				 size_t index, u64 *val);

So a driver author can use the "index" parameter to locate the buffer
element and pass back its value via the "val" parameter. To define the
buffer, maybe helper macros like this could be introduced::

     COUNTER_COMP_COUNT_BUFFER_U64("capture", ecap_cnt_cap_read, 4)

This would define four u64 buffer elements each named prefixed with
"capture" and with their read callbacks set to ecap_cnt_cap_read().

One problem however is that I'm not sure if the C preprocessor would be
able to unroll the COUNTER_COMP_COUNT_BUFFER_U64 to a dynamic number of
elements based on a macro parameter (maybe there is a GCC extension).

I'm just throwing out ideas, so I'd like to hear some comments and
suggestions from others about how we should add buffer support to the
Counter subsystem.

William Breathitt Gray

Hi William,

I am going to send a new version (PATCH v5) without buffers as described above (I added macros for cap1/2/3/4 similar functions). I tried taking into account the
comments made by Jonathan and you.

Regarding buffers, I don't know how driver upstream process is done when 'significant' subsystem modifications are being considered. Is it a problem for you if we continue driver upstream process until it's merged, and then modify counter subsystem (and ECAP driver with it) to add some buffer functionalities ? Or do you prefer adding such functionalities to counter subsystem while upstreaming ECAP driver, without any
intermediate ECAP driver version ?

Julien Panis




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux