[v7,00/48] atmel_mxt_ts misc
mbox series

Message ID 20200212084218.32344-1-jiada_wang@mentor.com
Headers show
Series
  • atmel_mxt_ts misc
Related show

Message

Wang, Jiada Feb. 12, 2020, 8:41 a.m. UTC
This patch-set forward ports Nick Dyer's work in ndyer/linux github repository
as long as some other features and fixes

Balasubramani Vivekanandan (2):
  Input: atmel_mxt_ts: Limit the max bytes transferred in an i2c
    transaction
  Input: atmel_mxt_ts: use gpiod_set_value_cansleep for reset pin

Dean Jenkins (1):
  Input: atmel_mxt_ts: return error from
    mxt_process_messages_until_invalid()

Deepak Das (6):
  Input: Atmel: improve error handling in mxt_start()
  Input: Atmel: improve error handling in mxt_initialize()
  Input: Atmel: improve error handling in mxt_update_cfg()
  Input: Atmel: Improve error handling in mxt_initialize_input_device()
  Input: Atmel: handle ReportID "0x00" while processing T5 messages
  Input: Atmel: use T44 object to process T5 messages

George G. Davis (1):
  input: atmel_mxt_ts: export GPIO reset line via sysfs

Jiada Wang (3):
  Input: introduce input_mt_report_slot_inactive
  Input: atmel_mxt_ts - eliminate data->raw_info_block
  Input: atmel_mxt_ts - Fix compilation warning

Karl Tsou (1):
  Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msgs

Kautuk Consul (2):
  Input: atmel_mxt_ts - Change call-points of mxt_free_* functions
  Input: atmel_mxt_ts - rely on calculated_crc rather than file
    config_crc

Naveen Chakka (2):
  input: touchscreen: atmel_mxt_ts: Added sysfs entry for touchscreen
    status
  input: atmel_mxt_ts: added sysfs interface to update atmel T38 data

Nick Dyer (26):
  Input: atmel_mxt_ts - rework sysfs init/remove
  Input: atmel_mxt_ts - only read messages in mxt_acquire_irq() when
    necessary
  Input: atmel_mxt_ts - split large i2c transfers into blocks
  Input: atmel_mxt_ts - output status from T48 Noise Supression
  Input: atmel_mxt_ts - output status from T42 Touch Suppression
  Input: atmel_mxt_ts - implement T9 vector/orientation support
  Input: atmel_mxt_ts - implement T15 Key Array support
  Input: atmel_mxt_ts - handle reports from T47 Stylus object
  Input: atmel_mxt_ts - implement support for T107 active stylus
  Input: atmel_mxt_ts - release touch state during suspend
  Input: atmel_mxt_ts - add regulator control support
  Input: atmel_mxt_ts - report failures in suspend/resume
  Input: atmel_mxt_ts - allow specification of firmware file name
  Input: atmel_mxt_ts - handle cfg filename via pdata/sysfs
  Input: atmel_mxt_ts - allow input name to be specified in platform
    data
  Input: atmel_mxt_ts - refactor firmware flash to extract context into
    struct
  Input: atmel_mxt_ts - refactor code to enter bootloader into separate
    func
  Input: atmel_mxt_ts - combine bootloader version query with probe
  Input: atmel_mxt_ts - improve bootloader state machine handling
  Input: atmel_mxt_ts - rename bl_completion to chg_completion
  Input: atmel_mxt_ts - make bootloader interrupt driven
  Input: atmel_mxt_ts - delay enabling IRQ when not using regulators
  Input: atmel_mxt_ts - implement I2C retries
  Input: atmel_mxt_ts - orientation is not present in hover
  Input: atmel_mxt_ts - implement debug output for messages
  Input: atmel_mxt_ts - implement improved debug message interface

Nikhil Ravindran (1):
  Input: atmel_mxt_ts: Add support for run self-test routine.

Sanjeev Chugh (1):
  Input: atmel_mxt_ts: Implement synchronization during various
    operation

karl tsou (1):
  Input: atmel_mxt_ts - add config checksum attribute to sysfs

keerthikumarp (1):
  input: atmel_mxt_ts: Add Missing Delay for reset handling of Atmel
    touch panel controller in detachable displays.

---
v7:
Fix regression found when updating firmware
Following commits have been updated to fix regression found when
updating firmware
Input: atmel_mxt_ts - improve bootloader state machine handling
Input: atmel_mxt_ts - make bootloader interrupt driven
input: touchscreen: atmel_mxt_ts: Added sysfs entry for touchscreen status
Input: atmel_mxt_ts: Implement synchronization during various operation

v6:
Fix issue in commit ("Input: introduce input_mt_report_slot_inactive")
reported by kernel test robot

v5:
Following commits have been updated to address warnings & errors
reported by kbuild test robot 
Input: atmel_mxt_ts - make bootloader interrupt driven
Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msgs

Following commit has been updated
Input: introduce input_mt_report_slot_inactive

v4:
Following commit in v3 patch-set has been removed
Input: switch to use return value of input_mt_report_slot_state

Following commit has been updated to address checkpatch warning
Input: atmel_mxt_ts: Implement synchronization during various operation

v3:
Following commits have been updated compared to v2 patchset
Input: atmel_mxt_ts - implement debug output for messages
- added inline comment
Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msg
- changed dev_info() to dev_dbg()

v2:
Following commit in v1 patchset has been split into two commits
Input: introduce input_mt_report_slot_inactive

Following commits have been updated compared to v1 patchset
Input: atmel_mxt_ts - split large i2c transfers into blocks
Input: atmel_mxt_ts - output status from T42 Touch Suppression

Following commits in v1 patchset have been squashed
Input: touchscreen: Atmel: Add device tree support for T15 key array
objects
Input: atmel_mxt_ts - check data->input_dev is not null in
mxt_input_sync()
Input: atmel_mxt_ts - check firmware format before entering bootloader
Input: atmel_mxt_ts: update stale use_retrigen_workaround flag
input: atmel_mxt_ts: move bootloader probe from mxt_initialize()
input: Atmel: limit the max bytes transferred while reading T5 messages
Input: atmel_mxt_ts: Use msecs_to_jiffies() instead of HZ
Input: atmel_mxt_ts: Use complete when in_bootloader true
Input: atmel_mxt_ts: Prevent crash due to freeing of input device
input: atmel_mxt_ts: Add NULL check for sysfs attribute debug_msg_attr

Following commits in v1 patchset have been dropped:
Input: atmel_mxt_ts - configure and use gpios as real gpios
Input: touchscreen: Atmel: Enable IRQ_DISABLE_UNLAZY flag for interrupt
Input: atmel_mxt_ts - add memory access interface via sysfs
Input: atmel_mxt_ts: Remove sysfs attributes during driver detach
Input: atmel_mxt_ts: Avoid race condition in freeing of input device


v1: initial version
---
 .../bindings/input/atmel,maxtouch.txt         |   14 +
 MAINTAINERS                                   |    1 +
 drivers/hid/hid-alps.c                        |    3 +-
 drivers/hid/hid-multitouch.c                  |    6 +-
 drivers/input/misc/xen-kbdfront.c             |    2 +-
 drivers/input/mouse/elan_i2c_core.c           |    2 +-
 drivers/input/touchscreen/atmel_mxt_ts.c      | 2270 ++++++++++++++---
 drivers/input/touchscreen/cyttsp4_core.c      |    5 +-
 drivers/input/touchscreen/cyttsp_core.c       |    2 +-
 drivers/input/touchscreen/melfas_mip4.c       |    4 +-
 drivers/input/touchscreen/mms114.c            |    2 +-
 drivers/input/touchscreen/raspberrypi-ts.c    |    2 +-
 drivers/input/touchscreen/stmfts.c            |    2 +-
 include/dt-bindings/input/atmel_mxt_ts.h      |   22 +
 include/linux/input/mt.h                      |    5 +
 15 files changed, 1985 insertions(+), 357 deletions(-)
 create mode 100644 include/dt-bindings/input/atmel_mxt_ts.h

Comments

Wang, Jiada March 11, 2020, 1:09 a.m. UTC | #1
Hello Dmitry and all

Kind reminder on this v7 patch-set

Thanks,
Jiada

On 2020/02/12 17:41, Jiada Wang wrote:
> This patch-set forward ports Nick Dyer's work in ndyer/linux github repository
> as long as some other features and fixes
> 
> Balasubramani Vivekanandan (2):
>    Input: atmel_mxt_ts: Limit the max bytes transferred in an i2c
>      transaction
>    Input: atmel_mxt_ts: use gpiod_set_value_cansleep for reset pin
> 
> Dean Jenkins (1):
>    Input: atmel_mxt_ts: return error from
>      mxt_process_messages_until_invalid()
> 
> Deepak Das (6):
>    Input: Atmel: improve error handling in mxt_start()
>    Input: Atmel: improve error handling in mxt_initialize()
>    Input: Atmel: improve error handling in mxt_update_cfg()
>    Input: Atmel: Improve error handling in mxt_initialize_input_device()
>    Input: Atmel: handle ReportID "0x00" while processing T5 messages
>    Input: Atmel: use T44 object to process T5 messages
> 
> George G. Davis (1):
>    input: atmel_mxt_ts: export GPIO reset line via sysfs
> 
> Jiada Wang (3):
>    Input: introduce input_mt_report_slot_inactive
>    Input: atmel_mxt_ts - eliminate data->raw_info_block
>    Input: atmel_mxt_ts - Fix compilation warning
> 
> Karl Tsou (1):
>    Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msgs
> 
> Kautuk Consul (2):
>    Input: atmel_mxt_ts - Change call-points of mxt_free_* functions
>    Input: atmel_mxt_ts - rely on calculated_crc rather than file
>      config_crc
> 
> Naveen Chakka (2):
>    input: touchscreen: atmel_mxt_ts: Added sysfs entry for touchscreen
>      status
>    input: atmel_mxt_ts: added sysfs interface to update atmel T38 data
> 
> Nick Dyer (26):
>    Input: atmel_mxt_ts - rework sysfs init/remove
>    Input: atmel_mxt_ts - only read messages in mxt_acquire_irq() when
>      necessary
>    Input: atmel_mxt_ts - split large i2c transfers into blocks
>    Input: atmel_mxt_ts - output status from T48 Noise Supression
>    Input: atmel_mxt_ts - output status from T42 Touch Suppression
>    Input: atmel_mxt_ts - implement T9 vector/orientation support
>    Input: atmel_mxt_ts - implement T15 Key Array support
>    Input: atmel_mxt_ts - handle reports from T47 Stylus object
>    Input: atmel_mxt_ts - implement support for T107 active stylus
>    Input: atmel_mxt_ts - release touch state during suspend
>    Input: atmel_mxt_ts - add regulator control support
>    Input: atmel_mxt_ts - report failures in suspend/resume
>    Input: atmel_mxt_ts - allow specification of firmware file name
>    Input: atmel_mxt_ts - handle cfg filename via pdata/sysfs
>    Input: atmel_mxt_ts - allow input name to be specified in platform
>      data
>    Input: atmel_mxt_ts - refactor firmware flash to extract context into
>      struct
>    Input: atmel_mxt_ts - refactor code to enter bootloader into separate
>      func
>    Input: atmel_mxt_ts - combine bootloader version query with probe
>    Input: atmel_mxt_ts - improve bootloader state machine handling
>    Input: atmel_mxt_ts - rename bl_completion to chg_completion
>    Input: atmel_mxt_ts - make bootloader interrupt driven
>    Input: atmel_mxt_ts - delay enabling IRQ when not using regulators
>    Input: atmel_mxt_ts - implement I2C retries
>    Input: atmel_mxt_ts - orientation is not present in hover
>    Input: atmel_mxt_ts - implement debug output for messages
>    Input: atmel_mxt_ts - implement improved debug message interface
> 
> Nikhil Ravindran (1):
>    Input: atmel_mxt_ts: Add support for run self-test routine.
> 
> Sanjeev Chugh (1):
>    Input: atmel_mxt_ts: Implement synchronization during various
>      operation
> 
> karl tsou (1):
>    Input: atmel_mxt_ts - add config checksum attribute to sysfs
> 
> keerthikumarp (1):
>    input: atmel_mxt_ts: Add Missing Delay for reset handling of Atmel
>      touch panel controller in detachable displays.
> 
> ---
> v7:
> Fix regression found when updating firmware
> Following commits have been updated to fix regression found when
> updating firmware
> Input: atmel_mxt_ts - improve bootloader state machine handling
> Input: atmel_mxt_ts - make bootloader interrupt driven
> input: touchscreen: atmel_mxt_ts: Added sysfs entry for touchscreen status
> Input: atmel_mxt_ts: Implement synchronization during various operation
> 
> v6:
> Fix issue in commit ("Input: introduce input_mt_report_slot_inactive")
> reported by kernel test robot
> 
> v5:
> Following commits have been updated to address warnings & errors
> reported by kbuild test robot
> Input: atmel_mxt_ts - make bootloader interrupt driven
> Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msgs
> 
> Following commit has been updated
> Input: introduce input_mt_report_slot_inactive
> 
> v4:
> Following commit in v3 patch-set has been removed
> Input: switch to use return value of input_mt_report_slot_state
> 
> Following commit has been updated to address checkpatch warning
> Input: atmel_mxt_ts: Implement synchronization during various operation
> 
> v3:
> Following commits have been updated compared to v2 patchset
> Input: atmel_mxt_ts - implement debug output for messages
> - added inline comment
> Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msg
> - changed dev_info() to dev_dbg()
> 
> v2:
> Following commit in v1 patchset has been split into two commits
> Input: introduce input_mt_report_slot_inactive
> 
> Following commits have been updated compared to v1 patchset
> Input: atmel_mxt_ts - split large i2c transfers into blocks
> Input: atmel_mxt_ts - output status from T42 Touch Suppression
> 
> Following commits in v1 patchset have been squashed
> Input: touchscreen: Atmel: Add device tree support for T15 key array
> objects
> Input: atmel_mxt_ts - check data->input_dev is not null in
> mxt_input_sync()
> Input: atmel_mxt_ts - check firmware format before entering bootloader
> Input: atmel_mxt_ts: update stale use_retrigen_workaround flag
> input: atmel_mxt_ts: move bootloader probe from mxt_initialize()
> input: Atmel: limit the max bytes transferred while reading T5 messages
> Input: atmel_mxt_ts: Use msecs_to_jiffies() instead of HZ
> Input: atmel_mxt_ts: Use complete when in_bootloader true
> Input: atmel_mxt_ts: Prevent crash due to freeing of input device
> input: atmel_mxt_ts: Add NULL check for sysfs attribute debug_msg_attr
> 
> Following commits in v1 patchset have been dropped:
> Input: atmel_mxt_ts - configure and use gpios as real gpios
> Input: touchscreen: Atmel: Enable IRQ_DISABLE_UNLAZY flag for interrupt
> Input: atmel_mxt_ts - add memory access interface via sysfs
> Input: atmel_mxt_ts: Remove sysfs attributes during driver detach
> Input: atmel_mxt_ts: Avoid race condition in freeing of input device
> 
> 
> v1: initial version
> ---
>   .../bindings/input/atmel,maxtouch.txt         |   14 +
>   MAINTAINERS                                   |    1 +
>   drivers/hid/hid-alps.c                        |    3 +-
>   drivers/hid/hid-multitouch.c                  |    6 +-
>   drivers/input/misc/xen-kbdfront.c             |    2 +-
>   drivers/input/mouse/elan_i2c_core.c           |    2 +-
>   drivers/input/touchscreen/atmel_mxt_ts.c      | 2270 ++++++++++++++---
>   drivers/input/touchscreen/cyttsp4_core.c      |    5 +-
>   drivers/input/touchscreen/cyttsp_core.c       |    2 +-
>   drivers/input/touchscreen/melfas_mip4.c       |    4 +-
>   drivers/input/touchscreen/mms114.c            |    2 +-
>   drivers/input/touchscreen/raspberrypi-ts.c    |    2 +-
>   drivers/input/touchscreen/stmfts.c            |    2 +-
>   include/dt-bindings/input/atmel_mxt_ts.h      |   22 +
>   include/linux/input/mt.h                      |    5 +
>   15 files changed, 1985 insertions(+), 357 deletions(-)
>   create mode 100644 include/dt-bindings/input/atmel_mxt_ts.h
>
Dmitry Osipenko March 12, 2020, 3:21 p.m. UTC | #2
12.02.2020 11:41, Jiada Wang пишет:
> This patch-set forward ports Nick Dyer's work in ndyer/linux github repository
> as long as some other features and fixes
> 
> Balasubramani Vivekanandan (2):
>   Input: atmel_mxt_ts: Limit the max bytes transferred in an i2c
>     transaction
>   Input: atmel_mxt_ts: use gpiod_set_value_cansleep for reset pin
> 
> Dean Jenkins (1):
>   Input: atmel_mxt_ts: return error from
>     mxt_process_messages_until_invalid()
> 
> Deepak Das (6):
>   Input: Atmel: improve error handling in mxt_start()
>   Input: Atmel: improve error handling in mxt_initialize()
>   Input: Atmel: improve error handling in mxt_update_cfg()
>   Input: Atmel: Improve error handling in mxt_initialize_input_device()
>   Input: Atmel: handle ReportID "0x00" while processing T5 messages
>   Input: Atmel: use T44 object to process T5 messages
> 
> George G. Davis (1):
>   input: atmel_mxt_ts: export GPIO reset line via sysfs
> 
> Jiada Wang (3):
>   Input: introduce input_mt_report_slot_inactive
>   Input: atmel_mxt_ts - eliminate data->raw_info_block
>   Input: atmel_mxt_ts - Fix compilation warning
> 
> Karl Tsou (1):
>   Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msgs
> 
> Kautuk Consul (2):
>   Input: atmel_mxt_ts - Change call-points of mxt_free_* functions
>   Input: atmel_mxt_ts - rely on calculated_crc rather than file
>     config_crc
> 
> Naveen Chakka (2):
>   input: touchscreen: atmel_mxt_ts: Added sysfs entry for touchscreen
>     status
>   input: atmel_mxt_ts: added sysfs interface to update atmel T38 data
> 
> Nick Dyer (26):
>   Input: atmel_mxt_ts - rework sysfs init/remove
>   Input: atmel_mxt_ts - only read messages in mxt_acquire_irq() when
>     necessary
>   Input: atmel_mxt_ts - split large i2c transfers into blocks
>   Input: atmel_mxt_ts - output status from T48 Noise Supression
>   Input: atmel_mxt_ts - output status from T42 Touch Suppression
>   Input: atmel_mxt_ts - implement T9 vector/orientation support
>   Input: atmel_mxt_ts - implement T15 Key Array support
>   Input: atmel_mxt_ts - handle reports from T47 Stylus object
>   Input: atmel_mxt_ts - implement support for T107 active stylus
>   Input: atmel_mxt_ts - release touch state during suspend
>   Input: atmel_mxt_ts - add regulator control support
>   Input: atmel_mxt_ts - report failures in suspend/resume
>   Input: atmel_mxt_ts - allow specification of firmware file name
>   Input: atmel_mxt_ts - handle cfg filename via pdata/sysfs
>   Input: atmel_mxt_ts - allow input name to be specified in platform
>     data
>   Input: atmel_mxt_ts - refactor firmware flash to extract context into
>     struct
>   Input: atmel_mxt_ts - refactor code to enter bootloader into separate
>     func
>   Input: atmel_mxt_ts - combine bootloader version query with probe
>   Input: atmel_mxt_ts - improve bootloader state machine handling
>   Input: atmel_mxt_ts - rename bl_completion to chg_completion
>   Input: atmel_mxt_ts - make bootloader interrupt driven
>   Input: atmel_mxt_ts - delay enabling IRQ when not using regulators
>   Input: atmel_mxt_ts - implement I2C retries
>   Input: atmel_mxt_ts - orientation is not present in hover
>   Input: atmel_mxt_ts - implement debug output for messages
>   Input: atmel_mxt_ts - implement improved debug message interface
> 
> Nikhil Ravindran (1):
>   Input: atmel_mxt_ts: Add support for run self-test routine.
> 
> Sanjeev Chugh (1):
>   Input: atmel_mxt_ts: Implement synchronization during various
>     operation
> 
> karl tsou (1):
>   Input: atmel_mxt_ts - add config checksum attribute to sysfs
> 
> keerthikumarp (1):
>   input: atmel_mxt_ts: Add Missing Delay for reset handling of Atmel
>     touch panel controller in detachable displays.
> 
> ---
> v7:
> Fix regression found when updating firmware
> Following commits have been updated to fix regression found when
> updating firmware
> Input: atmel_mxt_ts - improve bootloader state machine handling
> Input: atmel_mxt_ts - make bootloader interrupt driven
> input: touchscreen: atmel_mxt_ts: Added sysfs entry for touchscreen status
> Input: atmel_mxt_ts: Implement synchronization during various operation
> 
> v6:
> Fix issue in commit ("Input: introduce input_mt_report_slot_inactive")
> reported by kernel test robot
> 
> v5:
> Following commits have been updated to address warnings & errors
> reported by kbuild test robot 
> Input: atmel_mxt_ts - make bootloader interrupt driven
> Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msgs
> 
> Following commit has been updated
> Input: introduce input_mt_report_slot_inactive
> 
> v4:
> Following commit in v3 patch-set has been removed
> Input: switch to use return value of input_mt_report_slot_state
> 
> Following commit has been updated to address checkpatch warning
> Input: atmel_mxt_ts: Implement synchronization during various operation
> 
> v3:
> Following commits have been updated compared to v2 patchset
> Input: atmel_mxt_ts - implement debug output for messages
> - added inline comment
> Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msg
> - changed dev_info() to dev_dbg()
> 
> v2:
> Following commit in v1 patchset has been split into two commits
> Input: introduce input_mt_report_slot_inactive
> 
> Following commits have been updated compared to v1 patchset
> Input: atmel_mxt_ts - split large i2c transfers into blocks
> Input: atmel_mxt_ts - output status from T42 Touch Suppression
> 
> Following commits in v1 patchset have been squashed
> Input: touchscreen: Atmel: Add device tree support for T15 key array
> objects
> Input: atmel_mxt_ts - check data->input_dev is not null in
> mxt_input_sync()
> Input: atmel_mxt_ts - check firmware format before entering bootloader
> Input: atmel_mxt_ts: update stale use_retrigen_workaround flag
> input: atmel_mxt_ts: move bootloader probe from mxt_initialize()
> input: Atmel: limit the max bytes transferred while reading T5 messages
> Input: atmel_mxt_ts: Use msecs_to_jiffies() instead of HZ
> Input: atmel_mxt_ts: Use complete when in_bootloader true
> Input: atmel_mxt_ts: Prevent crash due to freeing of input device
> input: atmel_mxt_ts: Add NULL check for sysfs attribute debug_msg_attr
> 
> Following commits in v1 patchset have been dropped:
> Input: atmel_mxt_ts - configure and use gpios as real gpios
> Input: touchscreen: Atmel: Enable IRQ_DISABLE_UNLAZY flag for interrupt
> Input: atmel_mxt_ts - add memory access interface via sysfs
> Input: atmel_mxt_ts: Remove sysfs attributes during driver detach
> Input: atmel_mxt_ts: Avoid race condition in freeing of input device
> 
> 
> v1: initial version
> ---
>  .../bindings/input/atmel,maxtouch.txt         |   14 +
>  MAINTAINERS                                   |    1 +
>  drivers/hid/hid-alps.c                        |    3 +-
>  drivers/hid/hid-multitouch.c                  |    6 +-
>  drivers/input/misc/xen-kbdfront.c             |    2 +-
>  drivers/input/mouse/elan_i2c_core.c           |    2 +-
>  drivers/input/touchscreen/atmel_mxt_ts.c      | 2270 ++++++++++++++---
>  drivers/input/touchscreen/cyttsp4_core.c      |    5 +-
>  drivers/input/touchscreen/cyttsp_core.c       |    2 +-
>  drivers/input/touchscreen/melfas_mip4.c       |    4 +-
>  drivers/input/touchscreen/mms114.c            |    2 +-
>  drivers/input/touchscreen/raspberrypi-ts.c    |    2 +-
>  drivers/input/touchscreen/stmfts.c            |    2 +-
>  include/dt-bindings/input/atmel_mxt_ts.h      |   22 +
>  include/linux/input/mt.h                      |    5 +
>  15 files changed, 1985 insertions(+), 357 deletions(-)
>  create mode 100644 include/dt-bindings/input/atmel_mxt_ts.h
> 

Hello Jiada,

Please run all the patches through `scripts/checkpatch.pl --strict` and
fix all the reported problems.

Otherwise this is a very good series, it makes MXT1386 to work because
I2C retying is indeed required for that controller.
Wang, Jiada March 16, 2020, 6:50 a.m. UTC | #3
Hello Dmitry

On 2020/03/13 0:21, Dmitry Osipenko wrote:
> 12.02.2020 11:41, Jiada Wang пишет:
>> This patch-set forward ports Nick Dyer's work in ndyer/linux github repository
>> as long as some other features and fixes
>>
>> Balasubramani Vivekanandan (2):
>>    Input: atmel_mxt_ts: Limit the max bytes transferred in an i2c
>>      transaction
>>    Input: atmel_mxt_ts: use gpiod_set_value_cansleep for reset pin
>>
>> Dean Jenkins (1):
>>    Input: atmel_mxt_ts: return error from
>>      mxt_process_messages_until_invalid()
>>
>> Deepak Das (6):
>>    Input: Atmel: improve error handling in mxt_start()
>>    Input: Atmel: improve error handling in mxt_initialize()
>>    Input: Atmel: improve error handling in mxt_update_cfg()
>>    Input: Atmel: Improve error handling in mxt_initialize_input_device()
>>    Input: Atmel: handle ReportID "0x00" while processing T5 messages
>>    Input: Atmel: use T44 object to process T5 messages
>>
>> George G. Davis (1):
>>    input: atmel_mxt_ts: export GPIO reset line via sysfs
>>
>> Jiada Wang (3):
>>    Input: introduce input_mt_report_slot_inactive
>>    Input: atmel_mxt_ts - eliminate data->raw_info_block
>>    Input: atmel_mxt_ts - Fix compilation warning
>>
>> Karl Tsou (1):
>>    Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msgs
>>
>> Kautuk Consul (2):
>>    Input: atmel_mxt_ts - Change call-points of mxt_free_* functions
>>    Input: atmel_mxt_ts - rely on calculated_crc rather than file
>>      config_crc
>>
>> Naveen Chakka (2):
>>    input: touchscreen: atmel_mxt_ts: Added sysfs entry for touchscreen
>>      status
>>    input: atmel_mxt_ts: added sysfs interface to update atmel T38 data
>>
>> Nick Dyer (26):
>>    Input: atmel_mxt_ts - rework sysfs init/remove
>>    Input: atmel_mxt_ts - only read messages in mxt_acquire_irq() when
>>      necessary
>>    Input: atmel_mxt_ts - split large i2c transfers into blocks
>>    Input: atmel_mxt_ts - output status from T48 Noise Supression
>>    Input: atmel_mxt_ts - output status from T42 Touch Suppression
>>    Input: atmel_mxt_ts - implement T9 vector/orientation support
>>    Input: atmel_mxt_ts - implement T15 Key Array support
>>    Input: atmel_mxt_ts - handle reports from T47 Stylus object
>>    Input: atmel_mxt_ts - implement support for T107 active stylus
>>    Input: atmel_mxt_ts - release touch state during suspend
>>    Input: atmel_mxt_ts - add regulator control support
>>    Input: atmel_mxt_ts - report failures in suspend/resume
>>    Input: atmel_mxt_ts - allow specification of firmware file name
>>    Input: atmel_mxt_ts - handle cfg filename via pdata/sysfs
>>    Input: atmel_mxt_ts - allow input name to be specified in platform
>>      data
>>    Input: atmel_mxt_ts - refactor firmware flash to extract context into
>>      struct
>>    Input: atmel_mxt_ts - refactor code to enter bootloader into separate
>>      func
>>    Input: atmel_mxt_ts - combine bootloader version query with probe
>>    Input: atmel_mxt_ts - improve bootloader state machine handling
>>    Input: atmel_mxt_ts - rename bl_completion to chg_completion
>>    Input: atmel_mxt_ts - make bootloader interrupt driven
>>    Input: atmel_mxt_ts - delay enabling IRQ when not using regulators
>>    Input: atmel_mxt_ts - implement I2C retries
>>    Input: atmel_mxt_ts - orientation is not present in hover
>>    Input: atmel_mxt_ts - implement debug output for messages
>>    Input: atmel_mxt_ts - implement improved debug message interface
>>
>> Nikhil Ravindran (1):
>>    Input: atmel_mxt_ts: Add support for run self-test routine.
>>
>> Sanjeev Chugh (1):
>>    Input: atmel_mxt_ts: Implement synchronization during various
>>      operation
>>
>> karl tsou (1):
>>    Input: atmel_mxt_ts - add config checksum attribute to sysfs
>>
>> keerthikumarp (1):
>>    input: atmel_mxt_ts: Add Missing Delay for reset handling of Atmel
>>      touch panel controller in detachable displays.
>>
>> ---
>> v7:
>> Fix regression found when updating firmware
>> Following commits have been updated to fix regression found when
>> updating firmware
>> Input: atmel_mxt_ts - improve bootloader state machine handling
>> Input: atmel_mxt_ts - make bootloader interrupt driven
>> input: touchscreen: atmel_mxt_ts: Added sysfs entry for touchscreen status
>> Input: atmel_mxt_ts: Implement synchronization during various operation
>>
>> v6:
>> Fix issue in commit ("Input: introduce input_mt_report_slot_inactive")
>> reported by kernel test robot
>>
>> v5:
>> Following commits have been updated to address warnings & errors
>> reported by kbuild test robot
>> Input: atmel_mxt_ts - make bootloader interrupt driven
>> Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msgs
>>
>> Following commit has been updated
>> Input: introduce input_mt_report_slot_inactive
>>
>> v4:
>> Following commit in v3 patch-set has been removed
>> Input: switch to use return value of input_mt_report_slot_state
>>
>> Following commit has been updated to address checkpatch warning
>> Input: atmel_mxt_ts: Implement synchronization during various operation
>>
>> v3:
>> Following commits have been updated compared to v2 patchset
>> Input: atmel_mxt_ts - implement debug output for messages
>> - added inline comment
>> Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msg
>> - changed dev_info() to dev_dbg()
>>
>> v2:
>> Following commit in v1 patchset has been split into two commits
>> Input: introduce input_mt_report_slot_inactive
>>
>> Following commits have been updated compared to v1 patchset
>> Input: atmel_mxt_ts - split large i2c transfers into blocks
>> Input: atmel_mxt_ts - output status from T42 Touch Suppression
>>
>> Following commits in v1 patchset have been squashed
>> Input: touchscreen: Atmel: Add device tree support for T15 key array
>> objects
>> Input: atmel_mxt_ts - check data->input_dev is not null in
>> mxt_input_sync()
>> Input: atmel_mxt_ts - check firmware format before entering bootloader
>> Input: atmel_mxt_ts: update stale use_retrigen_workaround flag
>> input: atmel_mxt_ts: move bootloader probe from mxt_initialize()
>> input: Atmel: limit the max bytes transferred while reading T5 messages
>> Input: atmel_mxt_ts: Use msecs_to_jiffies() instead of HZ
>> Input: atmel_mxt_ts: Use complete when in_bootloader true
>> Input: atmel_mxt_ts: Prevent crash due to freeing of input device
>> input: atmel_mxt_ts: Add NULL check for sysfs attribute debug_msg_attr
>>
>> Following commits in v1 patchset have been dropped:
>> Input: atmel_mxt_ts - configure and use gpios as real gpios
>> Input: touchscreen: Atmel: Enable IRQ_DISABLE_UNLAZY flag for interrupt
>> Input: atmel_mxt_ts - add memory access interface via sysfs
>> Input: atmel_mxt_ts: Remove sysfs attributes during driver detach
>> Input: atmel_mxt_ts: Avoid race condition in freeing of input device
>>
>>
>> v1: initial version
>> ---
>>   .../bindings/input/atmel,maxtouch.txt         |   14 +
>>   MAINTAINERS                                   |    1 +
>>   drivers/hid/hid-alps.c                        |    3 +-
>>   drivers/hid/hid-multitouch.c                  |    6 +-
>>   drivers/input/misc/xen-kbdfront.c             |    2 +-
>>   drivers/input/mouse/elan_i2c_core.c           |    2 +-
>>   drivers/input/touchscreen/atmel_mxt_ts.c      | 2270 ++++++++++++++---
>>   drivers/input/touchscreen/cyttsp4_core.c      |    5 +-
>>   drivers/input/touchscreen/cyttsp_core.c       |    2 +-
>>   drivers/input/touchscreen/melfas_mip4.c       |    4 +-
>>   drivers/input/touchscreen/mms114.c            |    2 +-
>>   drivers/input/touchscreen/raspberrypi-ts.c    |    2 +-
>>   drivers/input/touchscreen/stmfts.c            |    2 +-
>>   include/dt-bindings/input/atmel_mxt_ts.h      |   22 +
>>   include/linux/input/mt.h                      |    5 +
>>   15 files changed, 1985 insertions(+), 357 deletions(-)
>>   create mode 100644 include/dt-bindings/input/atmel_mxt_ts.h
>>
> 
> Hello Jiada,
> 
> Please run all the patches through `scripts/checkpatch.pl --strict` and
> fix all the reported problems.
> 
Thanks, I will try to address checkpatch issues in v8 patch-set

Thanks,
Jiada
> Otherwise this is a very good series, it makes MXT1386 to work because
> I2C retying is indeed required for that controller.
>
Wang, Jiada March 20, 2020, 3:37 a.m. UTC | #4
Hello Dmitry

I have submitted v8 patch-set to address your comments towards v7 patch-set,
most of checkpatch warnings and errors have been addressed,

But I didn't update for following two types of warnings
since I want to keep consistency with legacy code

WARNING: DEVICE_ATTR unusual permissions '0600' used
#290: FILE: drivers/input/touchscreen/atmel_mxt_ts.c:3761:
+static DEVICE_ATTR(debug_v2_enable, 0600, NULL,

WARNING: Consider renaming function(s) 'mxt_debug_notify_show' to 
'debug_notify_show'
#292: FILE: drivers/input/touchscreen/atmel_mxt_ts.c:3763:
+static DEVICE_ATTR(debug_notify, 0444, mxt_debug_notify_show, NULL);

please let me know if you have different view on this

Thanks,
Jiada

On 2020/03/13 0:21, Dmitry Osipenko wrote:
> 12.02.2020 11:41, Jiada Wang пишет:
>> This patch-set forward ports Nick Dyer's work in ndyer/linux github repository
>> as long as some other features and fixes
>>
>> Balasubramani Vivekanandan (2):
>>    Input: atmel_mxt_ts: Limit the max bytes transferred in an i2c
>>      transaction
>>    Input: atmel_mxt_ts: use gpiod_set_value_cansleep for reset pin
>>
>> Dean Jenkins (1):
>>    Input: atmel_mxt_ts: return error from
>>      mxt_process_messages_until_invalid()
>>
>> Deepak Das (6):
>>    Input: Atmel: improve error handling in mxt_start()
>>    Input: Atmel: improve error handling in mxt_initialize()
>>    Input: Atmel: improve error handling in mxt_update_cfg()
>>    Input: Atmel: Improve error handling in mxt_initialize_input_device()
>>    Input: Atmel: handle ReportID "0x00" while processing T5 messages
>>    Input: Atmel: use T44 object to process T5 messages
>>
>> George G. Davis (1):
>>    input: atmel_mxt_ts: export GPIO reset line via sysfs
>>
>> Jiada Wang (3):
>>    Input: introduce input_mt_report_slot_inactive
>>    Input: atmel_mxt_ts - eliminate data->raw_info_block
>>    Input: atmel_mxt_ts - Fix compilation warning
>>
>> Karl Tsou (1):
>>    Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msgs
>>
>> Kautuk Consul (2):
>>    Input: atmel_mxt_ts - Change call-points of mxt_free_* functions
>>    Input: atmel_mxt_ts - rely on calculated_crc rather than file
>>      config_crc
>>
>> Naveen Chakka (2):
>>    input: touchscreen: atmel_mxt_ts: Added sysfs entry for touchscreen
>>      status
>>    input: atmel_mxt_ts: added sysfs interface to update atmel T38 data
>>
>> Nick Dyer (26):
>>    Input: atmel_mxt_ts - rework sysfs init/remove
>>    Input: atmel_mxt_ts - only read messages in mxt_acquire_irq() when
>>      necessary
>>    Input: atmel_mxt_ts - split large i2c transfers into blocks
>>    Input: atmel_mxt_ts - output status from T48 Noise Supression
>>    Input: atmel_mxt_ts - output status from T42 Touch Suppression
>>    Input: atmel_mxt_ts - implement T9 vector/orientation support
>>    Input: atmel_mxt_ts - implement T15 Key Array support
>>    Input: atmel_mxt_ts - handle reports from T47 Stylus object
>>    Input: atmel_mxt_ts - implement support for T107 active stylus
>>    Input: atmel_mxt_ts - release touch state during suspend
>>    Input: atmel_mxt_ts - add regulator control support
>>    Input: atmel_mxt_ts - report failures in suspend/resume
>>    Input: atmel_mxt_ts - allow specification of firmware file name
>>    Input: atmel_mxt_ts - handle cfg filename via pdata/sysfs
>>    Input: atmel_mxt_ts - allow input name to be specified in platform
>>      data
>>    Input: atmel_mxt_ts - refactor firmware flash to extract context into
>>      struct
>>    Input: atmel_mxt_ts - refactor code to enter bootloader into separate
>>      func
>>    Input: atmel_mxt_ts - combine bootloader version query with probe
>>    Input: atmel_mxt_ts - improve bootloader state machine handling
>>    Input: atmel_mxt_ts - rename bl_completion to chg_completion
>>    Input: atmel_mxt_ts - make bootloader interrupt driven
>>    Input: atmel_mxt_ts - delay enabling IRQ when not using regulators
>>    Input: atmel_mxt_ts - implement I2C retries
>>    Input: atmel_mxt_ts - orientation is not present in hover
>>    Input: atmel_mxt_ts - implement debug output for messages
>>    Input: atmel_mxt_ts - implement improved debug message interface
>>
>> Nikhil Ravindran (1):
>>    Input: atmel_mxt_ts: Add support for run self-test routine.
>>
>> Sanjeev Chugh (1):
>>    Input: atmel_mxt_ts: Implement synchronization during various
>>      operation
>>
>> karl tsou (1):
>>    Input: atmel_mxt_ts - add config checksum attribute to sysfs
>>
>> keerthikumarp (1):
>>    input: atmel_mxt_ts: Add Missing Delay for reset handling of Atmel
>>      touch panel controller in detachable displays.
>>
>> ---
>> v7:
>> Fix regression found when updating firmware
>> Following commits have been updated to fix regression found when
>> updating firmware
>> Input: atmel_mxt_ts - improve bootloader state machine handling
>> Input: atmel_mxt_ts - make bootloader interrupt driven
>> input: touchscreen: atmel_mxt_ts: Added sysfs entry for touchscreen status
>> Input: atmel_mxt_ts: Implement synchronization during various operation
>>
>> v6:
>> Fix issue in commit ("Input: introduce input_mt_report_slot_inactive")
>> reported by kernel test robot
>>
>> v5:
>> Following commits have been updated to address warnings & errors
>> reported by kbuild test robot
>> Input: atmel_mxt_ts - make bootloader interrupt driven
>> Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msgs
>>
>> Following commit has been updated
>> Input: introduce input_mt_report_slot_inactive
>>
>> v4:
>> Following commit in v3 patch-set has been removed
>> Input: switch to use return value of input_mt_report_slot_state
>>
>> Following commit has been updated to address checkpatch warning
>> Input: atmel_mxt_ts: Implement synchronization during various operation
>>
>> v3:
>> Following commits have been updated compared to v2 patchset
>> Input: atmel_mxt_ts - implement debug output for messages
>> - added inline comment
>> Input: atmel_mxt_ts - add debug for T92 gesture and T93 touch seq msg
>> - changed dev_info() to dev_dbg()
>>
>> v2:
>> Following commit in v1 patchset has been split into two commits
>> Input: introduce input_mt_report_slot_inactive
>>
>> Following commits have been updated compared to v1 patchset
>> Input: atmel_mxt_ts - split large i2c transfers into blocks
>> Input: atmel_mxt_ts - output status from T42 Touch Suppression
>>
>> Following commits in v1 patchset have been squashed
>> Input: touchscreen: Atmel: Add device tree support for T15 key array
>> objects
>> Input: atmel_mxt_ts - check data->input_dev is not null in
>> mxt_input_sync()
>> Input: atmel_mxt_ts - check firmware format before entering bootloader
>> Input: atmel_mxt_ts: update stale use_retrigen_workaround flag
>> input: atmel_mxt_ts: move bootloader probe from mxt_initialize()
>> input: Atmel: limit the max bytes transferred while reading T5 messages
>> Input: atmel_mxt_ts: Use msecs_to_jiffies() instead of HZ
>> Input: atmel_mxt_ts: Use complete when in_bootloader true
>> Input: atmel_mxt_ts: Prevent crash due to freeing of input device
>> input: atmel_mxt_ts: Add NULL check for sysfs attribute debug_msg_attr
>>
>> Following commits in v1 patchset have been dropped:
>> Input: atmel_mxt_ts - configure and use gpios as real gpios
>> Input: touchscreen: Atmel: Enable IRQ_DISABLE_UNLAZY flag for interrupt
>> Input: atmel_mxt_ts - add memory access interface via sysfs
>> Input: atmel_mxt_ts: Remove sysfs attributes during driver detach
>> Input: atmel_mxt_ts: Avoid race condition in freeing of input device
>>
>>
>> v1: initial version
>> ---
>>   .../bindings/input/atmel,maxtouch.txt         |   14 +
>>   MAINTAINERS                                   |    1 +
>>   drivers/hid/hid-alps.c                        |    3 +-
>>   drivers/hid/hid-multitouch.c                  |    6 +-
>>   drivers/input/misc/xen-kbdfront.c             |    2 +-
>>   drivers/input/mouse/elan_i2c_core.c           |    2 +-
>>   drivers/input/touchscreen/atmel_mxt_ts.c      | 2270 ++++++++++++++---
>>   drivers/input/touchscreen/cyttsp4_core.c      |    5 +-
>>   drivers/input/touchscreen/cyttsp_core.c       |    2 +-
>>   drivers/input/touchscreen/melfas_mip4.c       |    4 +-
>>   drivers/input/touchscreen/mms114.c            |    2 +-
>>   drivers/input/touchscreen/raspberrypi-ts.c    |    2 +-
>>   drivers/input/touchscreen/stmfts.c            |    2 +-
>>   include/dt-bindings/input/atmel_mxt_ts.h      |   22 +
>>   include/linux/input/mt.h                      |    5 +
>>   15 files changed, 1985 insertions(+), 357 deletions(-)
>>   create mode 100644 include/dt-bindings/input/atmel_mxt_ts.h
>>
> 
> Hello Jiada,
> 
> Please run all the patches through `scripts/checkpatch.pl --strict` and
> fix all the reported problems.
> 
> Otherwise this is a very good series, it makes MXT1386 to work because
> I2C retying is indeed required for that controller.
>
Dmitry Osipenko March 20, 2020, 3:53 p.m. UTC | #5
Hello Jiada,

20.03.2020 06:37, Wang, Jiada пишет:
> Hello Dmitry
> 
> I have submitted v8 patch-set to address your comments towards v7
> patch-set,
> most of checkpatch warnings and errors have been addressed,
> 
> But I didn't update for following two types of warnings
> since I want to keep consistency with legacy code
> 
> WARNING: DEVICE_ATTR unusual permissions '0600' used
> #290: FILE: drivers/input/touchscreen/atmel_mxt_ts.c:3761:
> +static DEVICE_ATTR(debug_v2_enable, 0600, NULL,

What will happen if you'll use 0644? Will an empty line be returned or
driver will crash?

> WARNING: Consider renaming function(s) 'mxt_debug_notify_show' to
> 'debug_notify_show'
> #292: FILE: drivers/input/touchscreen/atmel_mxt_ts.c:3763:
> +static DEVICE_ATTR(debug_notify, 0444, mxt_debug_notify_show, NULL);

Perhaps this should be fine to ignore, although the prefix is indeed a
bit superfluous.

> please let me know if you have different view on this

Thank you very much, I'll test v8 during the weekend.
Wang, Jiada March 23, 2020, 2:25 a.m. UTC | #6
Hello Dmitry

On 2020/03/21 0:53, Dmitry Osipenko wrote:
> Hello Jiada,
> 
> 20.03.2020 06:37, Wang, Jiada пишет:
>> Hello Dmitry
>>
>> I have submitted v8 patch-set to address your comments towards v7
>> patch-set,
>> most of checkpatch warnings and errors have been addressed,
>>
>> But I didn't update for following two types of warnings
>> since I want to keep consistency with legacy code
>>
>> WARNING: DEVICE_ATTR unusual permissions '0600' used
>> #290: FILE: drivers/input/touchscreen/atmel_mxt_ts.c:3761:
>> +static DEVICE_ATTR(debug_v2_enable, 0600, NULL,
> 
> What will happen if you'll use 0644? Will an empty line be returned or
> driver will crash?
> 
debug_v2_enable doesn't have .show callback implemented, so after change 
permission to 644, read of it results in an I/O error,

for other 0600 permission interfaces (t38_data, t25 and debug_enable)
added in this series,
change to 644 can return expected information when read.

Do you think it's better to change debug_v2_enable to 0200,
and others to 0644?

Thanks,
Jiada

>> WARNING: Consider renaming function(s) 'mxt_debug_notify_show' to
>> 'debug_notify_show'
>> #292: FILE: drivers/input/touchscreen/atmel_mxt_ts.c:3763:
>> +static DEVICE_ATTR(debug_notify, 0444, mxt_debug_notify_show, NULL);
> 
> Perhaps this should be fine to ignore, although the prefix is indeed a
> bit superfluous.
> 
>> please let me know if you have different view on this
> 
> Thank you very much, I'll test v8 during the weekend.
>
Dmitry Osipenko March 24, 2020, 2:54 p.m. UTC | #7
23.03.2020 05:25, Wang, Jiada пишет:
> Hello Dmitry
> 
> On 2020/03/21 0:53, Dmitry Osipenko wrote:
>> Hello Jiada,
>>
>> 20.03.2020 06:37, Wang, Jiada пишет:
>>> Hello Dmitry
>>>
>>> I have submitted v8 patch-set to address your comments towards v7
>>> patch-set,
>>> most of checkpatch warnings and errors have been addressed,
>>>
>>> But I didn't update for following two types of warnings
>>> since I want to keep consistency with legacy code
>>>
>>> WARNING: DEVICE_ATTR unusual permissions '0600' used
>>> #290: FILE: drivers/input/touchscreen/atmel_mxt_ts.c:3761:
>>> +static DEVICE_ATTR(debug_v2_enable, 0600, NULL,
>>
>> What will happen if you'll use 0644? Will an empty line be returned or
>> driver will crash?
>>
> debug_v2_enable doesn't have .show callback implemented, so after change
> permission to 644, read of it results in an I/O error,
> 
> for other 0600 permission interfaces (t38_data, t25 and debug_enable)
> added in this series,
> change to 644 can return expected information when read.
> 
> Do you think it's better to change debug_v2_enable to 0200,
> and others to 0644?

Since the debug_enable has mxt_debug_enable_show(), the same should be
done for debug_v2_enable, for consistency.

The permissions should be 0644 for everything that is read/write.

The 0200 should be used for everything that is root-only and write-only.
Dmitry Osipenko March 24, 2020, 3:03 p.m. UTC | #8
24.03.2020 17:54, Dmitry Osipenko пишет:
> 23.03.2020 05:25, Wang, Jiada пишет:
>> Hello Dmitry
>>
>> On 2020/03/21 0:53, Dmitry Osipenko wrote:
>>> Hello Jiada,
>>>
>>> 20.03.2020 06:37, Wang, Jiada пишет:
>>>> Hello Dmitry
>>>>
>>>> I have submitted v8 patch-set to address your comments towards v7
>>>> patch-set,
>>>> most of checkpatch warnings and errors have been addressed,
>>>>
>>>> But I didn't update for following two types of warnings
>>>> since I want to keep consistency with legacy code
>>>>
>>>> WARNING: DEVICE_ATTR unusual permissions '0600' used
>>>> #290: FILE: drivers/input/touchscreen/atmel_mxt_ts.c:3761:
>>>> +static DEVICE_ATTR(debug_v2_enable, 0600, NULL,
>>>
>>> What will happen if you'll use 0644? Will an empty line be returned or
>>> driver will crash?
>>>
>> debug_v2_enable doesn't have .show callback implemented, so after change
>> permission to 644, read of it results in an I/O error,
>>
>> for other 0600 permission interfaces (t38_data, t25 and debug_enable)
>> added in this series,
>> change to 644 can return expected information when read.
>>
>> Do you think it's better to change debug_v2_enable to 0200,
>> and others to 0644?
> 
> Since the debug_enable has mxt_debug_enable_show(), the same should be
> done for debug_v2_enable, for consistency.
> 
> The permissions should be 0644 for everything that is read/write.
> 
> The 0200 should be used for everything that is root-only and write-only.
> 

Also, please take a look at [1], see DEVICE_ATTR_WO() and
DEVICE_BOOL_ATTR() macros, which should be handy.

[1]
https://elixir.bootlin.com/linux/v5.6-rc7/source/include/linux/device.h#L125
Wang, Jiada March 25, 2020, 12:13 p.m. UTC | #9
Hi Dmitry

On 2020/03/24 23:54, Dmitry Osipenko wrote:
> 23.03.2020 05:25, Wang, Jiada пишет:
>> Hello Dmitry
>>
>> On 2020/03/21 0:53, Dmitry Osipenko wrote:
>>> Hello Jiada,
>>>
>>> 20.03.2020 06:37, Wang, Jiada пишет:
>>>> Hello Dmitry
>>>>
>>>> I have submitted v8 patch-set to address your comments towards v7
>>>> patch-set,
>>>> most of checkpatch warnings and errors have been addressed,
>>>>
>>>> But I didn't update for following two types of warnings
>>>> since I want to keep consistency with legacy code
>>>>
>>>> WARNING: DEVICE_ATTR unusual permissions '0600' used
>>>> #290: FILE: drivers/input/touchscreen/atmel_mxt_ts.c:3761:
>>>> +static DEVICE_ATTR(debug_v2_enable, 0600, NULL,
>>>
>>> What will happen if you'll use 0644? Will an empty line be returned or
>>> driver will crash?
>>>
>> debug_v2_enable doesn't have .show callback implemented, so after change
>> permission to 644, read of it results in an I/O error,
>>
>> for other 0600 permission interfaces (t38_data, t25 and debug_enable)
>> added in this series,
>> change to 644 can return expected information when read.
>>
>> Do you think it's better to change debug_v2_enable to 0200,
>> and others to 0644?
> 
> Since the debug_enable has mxt_debug_enable_show(), the same should be
> done for debug_v2_enable, for consistency.
> 
> The permissions should be 0644 for everything that is read/write.
> 
> The 0200 should be used for everything that is root-only and write-only.

Thanks for your comments and suggestion in another email,
I will submit v9 patch-set to address these warnings

Thanks,
Jiada
>