lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Sun, 29 Oct 2023 03:26:06 +0000
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     Wei-Shih Lin <frank101417@...il.com>
Cc:     robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        conor+dt@...nel.org, linux-input@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] Input: Add driver for Novatek NT519XX series
 touchscreen devices

Hi Wei-Shih,

On Wed, Oct 25, 2023 at 04:20:54PM +0800, Wei-Shih Lin wrote:
> This patch adds support for Novatek NT519XX series touchscreen devices.
> Existing Novatek touchscreen driver code developed for Acer Iconia One 7
> B1-750 tablet with Novatek IC NT11205 is novatek-nvt-ts.c in the path
> drivers/input/touchscreen/. However, this patch supports touch features
> for automotive display with Novatek TDDI NT519XX.

How different the protocol of this part from NT11205? Can the existing
driver be modified to support both variants?

You already got feedback from Krzysztof, on top of his, if we want to
continue with a separate driver:

- it should use standard device properties
- it should use gpiod API
- it looks like it will benefit of regmap's paging support
- helpers like nvt_irq_enable() should not be used - your code should
  know whether itq is enabled or disabled at all times
- all caps are reserved for macros (CTP_I2C_WRITE and others)
- I am sure we have crc8 helpers in the kernel
- please use u8, u16, etc in the kernel code instead of uint8_t,
  uint16_t
- your driver will likely benefit from devm APIs
- no compile-time conditionals like "#if TOUCH_MAX_FINGER_NUM > 1"

Thanks.

-- 
Dmitry

Powered by blists - more mailing lists