Re: [PATCH] Add driver for GOODiX GTx5 series touchsereen

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Wang,

On Wed, Jun 07, 2017 at 12:05:51PM +0800, Wang Yafei wrote:
> This driver is for GOODiX GTx5 series touchscreen controllers
> such as GT8589, GT7589. This driver designed with hierarchial structure,
> for that can be modified to support subsequent controllers easily.
> Some zones of the touchscreen can be set to buttons(according to the
> hardware). That is why it handles button and multitouch events.
> 
> A brief description of driver structure
> - Core Layer: This layer responsible for basic input events report,
>   GPIO pinctrl, Interrupt, Power resources manager and submodules
>   manager.
> - Hardware Layer: This layer responsible for controllers initialization,
>   irq handle as well as bus read/write.
> - External Module Layer: This layer used for support more features
>   such as firmware update, debug tools and gesture wakeup.


What is the difference between this version and the version you posted a
couple of days ago?

A few high level comments/questions:

- is this for a different chip that what is handled by the Goodix driver
  that we already have in the kernel?

- you should be using standard touchscreen bindings described in
  Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

- please use gpiod API without gpio fallbacks: you are submitting the
  driver for inclusion into mainline that does have gpiod API and for
  mainline it does not make sense to use fallbacks.

- similarly drop other compat stuff for older kernels

- please drop Android stiff (EARLY_SUSPEND, ets).

- use generic device properties (device_property_read_*()) instead of
  OF-specific variants.

- use get_unaligned_*() API to convert data on wire to CPU format
  instead of doing the same by hand.

- use dev_err(), dev_dbg(), etc for logging.

Thanks.

-- 
Dmitry