Re: [PATCH v3] counter: Add support for Intel Quadrature Encoder Peripheral

[William Breathitt Gray <vilhelm.gray@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3334 bytes --]

On Thu, May 27, 2021 at 04:58:38PM +0300, Jarkko Nikula wrote:
> Add support for Intel Quadrature Encoder Peripheral found on Intel
> Elkhart Lake platform.
> 
> Initial implementation was done by Felipe Balbi while he was working at
> Intel with later changes from Raymond Tan and me.
> 
> Co-developed-by: Felipe Balbi (Intel) <balbi@kernel.org>
> Signed-off-by: Felipe Balbi (Intel) <balbi@kernel.org>
> Co-developed-by: Raymond Tan <raymond.tan@intel.com>
> Signed-off-by: Raymond Tan <raymond.tan@intel.com>
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> ---
> v3:
> - Support for Quadrature x4 with swapped inputs and inverted inputs
>   removed. It turned out in review discussion both are board specific
>   features and questionable should they be even exposed to userspace.
>   Both features are postponed to future contribution if such need arises.
>   Patch 1/2 removed becaused of this.
> - Error out if trying to set 1 clock period long spike filter. Previous
>   version silently disabled the filter and also sysfs behavior in that
>   case was inconsistent: write 10 but read returns 0.
> - Line-continuation characters in INTEL_QEP_COUNTER_EXT_RW() aligned the
>   same way than others.
> 
> v2: https://marc.info/?l=linux-iio&m=162204156231555&w=2
> - counter_to_qep() macro -> counter->priv
> - Use sysfs_emit for user space returned values
> - Use kstrbool for boolean values from userspace
> - enable_write() reworked to be more readable
> - Reworked synapse action control and new sysfs attribute "invert"
>   * Action control before was wrong - what HW does is signal inversion.
>     Implemented "invert" sysfs attribute for it and read-only action
>     mode sysfs returning constant "both edges"
> - Renamed sysfs attribe "noise" as "spike_filter_ns" and define
>   programmable spike filter in terms of nanoseconds instead of raw
>   register value
> - Above and "ceiling" sysfs attribe changed as count extensions instead
>   of device extensions
> - Signal IDs rearranged to be zero based in order to prepare for counter
>   character device interface patches in order to ensure same userspace
>   sysfs paths
> - Initializer macros for counter_signal and counter_synapse
>   initialization
> - Grouping intel_qep_counter_ops, intel_qep_signal_ext and enums near to
>   their callback functions and use
> - "invert" and "spike_filter_ns" sysfs attributes documented
> - Other minor changes like local variable and empty line removal, etc
> 
> v1: https://www.spinics.net/lists/linux-iio/msg59652.html
> ---
>  Documentation/ABI/testing/sysfs-bus-counter |   9 +
>  drivers/counter/Kconfig                     |  10 +
>  drivers/counter/Makefile                    |   1 +
>  drivers/counter/intel-qep.c                 | 546 ++++++++++++++++++++
>  4 files changed, 566 insertions(+)
>  create mode 100644 drivers/counter/intel-qep.c

Hi Jarkko,

I noticed the intel-qep.c file is missing an entry in the MAINTAINERS
file. Would you be able to resubmit this patch with a proper entry added
to MAINTAINERS so that users have the relevant contact info to reach the
maintainers of this driver? You can keep my Signed-off tag on as well so
that we know the code has already been reviewed.

Thanks,

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]