Re: [PATCH v4 0/9] Add support for SAMA5D2 touchscreen

[Jonathan Cameron]

On Mon, 30 Apr 2018 13:32:06 +0300
Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx> wrote:

> Hello,
> 
> This patch series is a rework of my previous series named:
> [PATCH 00/14] iio: triggers: add consumer support
> 
> This is the version 4 of the series, and addresses the received feedback
> on the v2 series named:
> [PATCH v2 00/10]  Add support for SAMA5D2 touchscreen
> and the v3 series named
> [PATCH v3 00/11]  Add support for SAMA5D2 touchscreen
> 
> This series applies on top of fixes-togreg branch of iio.git,
> specifically on top of commit:
> "f0c8d1f" : iio: adc: at91-sama5d2_adc:
>  fix channel configuration for differential channels
> 
> Changes in v3 and v4 are presented at the end of the cover letter below.
> Thanks everyone for the feedback. Below is the original v2 cover letter:
> 
> In few words, this is the implementation of splitting the functionality
> of the IP block ADC device in SAMA5D2 SoC from ADC with touchscreen
> support. In order to avoid having a MFD device, two separate
> drivers that would work on same register base and split the IRQ,etc,
> as advised on the mailing list, I created a consumer driver for the
> channels, that will connect to the ADC as described in the device tree.
> 
> I have collected feedback from everyone and here is the result:
> I have added a new generic resistive touchscreen driver, which acts
> as a iio consumer for the given channels and will create an input
> device and report the events. It uses a callback buffer to register
> to the IIO device and waits for data to be pushed.
> Inside the IIO device, I have kept a similar approach with the first version
> of the series, except that now the driver can take multiple buffers, and
> will configure the touchscreen part of the hardware device if the specific
> channels are requested.
> 
> The SAMA5D2 ADC driver registers three new channels: two for the
> position on the X and Y axis, and one for the touch pressure.
> When channels are requested, it will check if the touchscreen channel mask
> includes the requested channels (it is possible that the consumer driver
> will not request pressure for example). If it's the case, it will work
> in touchscreen mode, and will refuse to do usual analog-digital conversion,
> because we have a single trigger and the touchscreen needs it.
> When the scan mask will include only old channels, the driver will function
> in the same way as before. If the scan mask somehow is a mix of the two (the
> masks intersect), the driver will refuse to work whatsoever (cannot have both
> in the same time).
> The driver allows reading raw data for the new channels, if claim direct
> mode works: no touchscreen driver requested anything. The new channels can
> act like the old ones. However, when requesting these channels, the usual
> trigger will not work and will not be enabled. The touchscreen channels
> require special trigger and irq configuration: pen detect, no pen detect
> and a periodic trigger to sample the touchscreen position and pressure.
> If the user attempts to use another trigger while there is a buffer
> that already requested the touchscreen channels (thus the trigger), the
> driver will refuse to comply.
> 
> In order to have defines for the channel numbers, I added a bindings include
> file that goes on a separate commit :
> dt-bindings: iio: adc: at91-sama5d2_adc: add channel specific consumer info
> This should go in the same tree with the following commits :
>   ARM: dts: at91: sama5d2: add channel cells for ADC device
>   ARM: dts: at91: sama5d2: Add resistive touch device
> 
> as build will break because these commits depend on the binding one
> which creates the included header file.
> 
> Changes in v4:
>  - removed patch for inkern module get/set kref
>  - addressed feedback on both the ADC and the touchscreen driver. each
> patch has a history inside the patch file for the specific changes.
>  - patch that fixes the channel fix
> [PATCH v3 01/11] iio: adc: at91-sama5d2_adc:
>  fix channel configuration for differential channels
> was accepted in fixes-togreg branch thus removed from this series.
>  - added Reviewed-by for the bindings by Rob Herring
> 
> Changes in v3:
>  - changed input driver name according to feedback and reworked in commits
> to adapt to binding changes and new name.
>  - moved channel index fix in at91-sama5d2_adc at the beginning of the series
> (PATCH 01/11)
>  - created a new optional binding for the touchscreen as a separate commit
> and added it to the series :
>  [PATCH v3 04/11] dt-bindings: input: touchscreen: add pressure
>  threshold touchscreen property
>  - changed at91-sama5d2_adc driver patch to address the comments. Exact changes
> are in the patch file for the driver source file.
> 
> 
> Eugen Hristev (9):
>   MAINTAINERS: add generic resistive touchscreen adc
>   iio: Add channel for Position Relative
>   dt-bindings: input: touchscreen: add pressure threshold touchscreen
>     property
>   dt-bindings: input: touchscreen: resistive-adc-touch: create bindings
>   iio: adc: at91-sama5d2_adc: add support for position and pressure
>     channels
>   input: touchscreen: resistive-adc-touch: add generic resistive ADC
>     touchscreen
>   dt-bindings: iio: adc: at91-sama5d2_adc: add channel specific consumer
>     info
>   ARM: dts: at91: sama5d2: add channel cells for ADC device
>   ARM: dts: at91: sama5d2: Add resistive touch device
> 
>  Documentation/ABI/testing/sysfs-bus-iio            |  12 +
>  .../bindings/iio/adc/at91-sama5d2_adc.txt          |   9 +
>  .../input/touchscreen/resistive-adc-touch.txt      |  30 +
>  .../bindings/input/touchscreen/touchscreen.txt     |   3 +
>  MAINTAINERS                                        |   6 +
>  arch/arm/boot/dts/sama5d2.dtsi                     |  12 +
>  drivers/iio/adc/at91-sama5d2_adc.c                 | 609 +++++++++++++++++++--
>  drivers/iio/industrialio-core.c                    |   1 +
>  drivers/input/touchscreen/Kconfig                  |  13 +
>  drivers/input/touchscreen/Makefile                 |   1 +
>  drivers/input/touchscreen/resistive-adc-touch.c    | 199 +++++++
>  include/dt-bindings/iio/adc/at91-sama5d2_adc.h     |  16 +
>  include/uapi/linux/iio/types.h                     |   1 +
>  tools/iio/iio_event_monitor.c                      |   2 +
>  14 files changed, 856 insertions(+), 58 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/resistive-adc-touch.txt
>  create mode 100644 drivers/input/touchscreen/resistive-adc-touch.c
>  create mode 100644 include/dt-bindings/iio/adc/at91-sama5d2_adc.h
> 

Hi All,

This looks to be pretty close to needing a path upstream.
I propose that I  take the driver parts (not dts changes obviously) through
iio.  I can do an immutable branch if anyone wants one (just let
me know).

Thanks,

Jonathan