Re: [PATCH v6 0/5] Introduce the Counter character device interface

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

 



On 11/22/20 2:29 PM, William Breathitt Gray wrote:

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 implicitly cast these values via the driver
    callback parameters.

    This question is primarily addressed to David Lechner. I'm somewhat
    confused about how this setup would look in device drivers. I've gone
    ahead and refactored the code to support u32 enums, and pushed it to
    a separate branch on my repository called counter_chrdev_v6_u32_enum:
    https://gitlab.com/vilhelmgray/iio/-/tree/counter_chrdev_v6_u32_enum

    Please check it out and let me know what you think. Is this the
    support you had in mind? I'm curious to see an example of how would
    your driver callback functions would look in this case. If everything
    works out fine, then I'll submit this branch as v7 of this patchset.

I haven't had time to look at this in depth, but just superficially looking
at it, it is mostly there. The driver callback would just use the enum type
in place of u32. For example:

static int ti_eqep_function_write(struct counter_device *counter,
				  struct counter_count *count,
				  enum counter_function function)

and the COUNTER_FUNCTION_* constants would be defined as:

enum counter_function {
	COUNTER_FUNCTION_INCREASE,
	...
};

instead of using #define macros.

One advantage I see to using u8, at least in the user API data structures,
is that it increases the number of events that fit in the kfifo buffer by
a significant factor.

And that is not to say that we couldn't do both: have the user API structs
use u8 for enum values and still use u32/strong enum types internally in
the callback functions.




2. How should we handle "raw" timestamps?

    Ahmad Fatoum brought up the possibility of returning "raw" timestamps
    similar to what the network stack offers (see the network stack
    SOF_TIMESTAMPING_{RAW,SYS}_HARDWARE support).

    I'm not very familiar with the networking stack code, but if I
    understand correctly the SOF_TIMESTAMPING_RAW_HARDWARE timestamps are
    values returned from the device. If so, I suspect we would be able to
    support these "raw" timestamps by defining them as Counter Extensions
    and returning them in struct counter_event elements similar to the
    other Extension values.

Is nanosecond resolution good enough? In the TI eQEP driver I considered
returning the raw timer value, but quickly realized that it would not be
very nice to expect the user code to know the clock rate of the timer. It
was very easy to get the clock rate in the kernel and just convert the
timer value to nanoseconds before returning it to userspace.

So if there is some specialized case where it can be solved no other way
besides using raw timestamps, then sure, include it. Otherwise I think we
should stick with nanoseconds for time values when possible.



[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