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] [thread-next>] [day] [month] [year] [list]
Message-ID: <IA1PR11MB609829F67A1D13E1FD4890E9F4042@IA1PR11MB6098.namprd11.prod.outlook.com>
Date: Tue, 17 Dec 2024 01:30:30 +0000
From: "Xu, Even" <even.xu@...el.com>
To: Mark Pearson <mpearson-lenovo@...ebb.ca>, Jiri Kosina <jikos@...nel.org>,
	"bentiss@...nel.org" <bentiss@...nel.org>, Jonathan Corbet <corbet@....net>,
	"bagasdotme@...il.com" <bagasdotme@...il.com>, "Aaron, Ma"
	<aaron.ma@...onical.com>, Randy Dunlap <rdunlap@...radead.org>
CC: "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>
Subject: RE: [PATCH v3 00/22] Add Intel Touch Host Controller drivers



> -----Original Message-----
> From: Mark Pearson <mpearson-lenovo@...ebb.ca>
> Sent: Tuesday, December 17, 2024 12:45 AM
> To: Xu, Even <even.xu@...el.com>; Jiri Kosina <jikos@...nel.org>;
> bentiss@...nel.org; Jonathan Corbet <corbet@....net>;
> bagasdotme@...il.com; Aaron, Ma <aaron.ma@...onical.com>; Randy Dunlap
> <rdunlap@...radead.org>
> Cc: linux-input@...r.kernel.org; linux-kernel@...r.kernel.org; linux-
> doc@...r.kernel.org
> Subject: Re: [PATCH v3 00/22] Add Intel Touch Host Controller drivers
> 
> Hi,
> 
> On Sun, Dec 15, 2024, at 8:41 PM, Even Xu wrote:
> > Intel Touch Host Controller (THC) is a new high performance input IP
> > which can benefit HID device's data transaction, such as touch screen,
> > touch pad, stylus.
> >
> > THC IP now evoluates to V4, it can support 3 different modes: IPTS,
> > HIDSPI and HIDI2C. Here are upgrade history:
> > - THC v1, for TGL/LKF, supports intel private IPTS (Intel Precise Touch
> >   and Stylus) protocol ( IPTS mode)
> > - THC v2, for ADL, add industrial standard HID over SPI protocol support
> >   (HIDSPI mode)
> > - THC v3, for MTL, enhance HID over SPI mode
> > - THC v4, for LNL, add inudstrial standard HID over I2C protocol support
> >   (HIDI2C mode)
> >
> > Linux Surface community (https://github.com/linux-surface) already
> > implemented IPTS mode. These patch series provides THC HIDSPI mode and
> > THC HIDI2C mode support on Linux.
> >
> > These patch series includes:
> > 1. Document for THC hardware and software introduction.
> > 2. Intel THC Hardware layer driver which provides control interfaces
> >    for protocol layer.
> > 3. Intel QuickSPI (R) driver working as a HIDSPI device driver which
> >    implements HID over SPI protocol and flow.
> > 4. Intel QuickI2C (R) driver working as a HIDI2C device driver which
> >    implements HID over I2C protocol and flow.
> >
> > Change logs:
> > v3:
> > - Change tables in documents from literal block to table format
> > - Change symbol namespace to string literal according to patch:
> >   cdd30ebb1b9f ("module: Convert symbol namespace to string literal")
> > - Refine Kconfig description
> > - Enhance Quickspi and Quicki2c driver by clearing THC hardware interal
> >   state before doing initialization to avoid BIOS impacts.
> > - A fix in Quicki2c driver when does set_report
> >
> > v2:
> > - Fix document format issues
> > - Add THC device IDs for Intel Panther Lake (PTL) platform
> >
> >
> > Even Xu (13):
> >   HID: THC: Add documentation
> >   HID: intel-thc-hid: intel-thc: Add THC DMA interfaces
> >   HID: intel-thc-hid: intel-thc: Add THC I2C config interfaces
> >   HID: intel-thc-hid: intel-quickspi: Add THC QuickSPI driver hid layer
> >   HID: intel-thc-hid: intel-quickspi: Add THC QuickSPI ACPI interfaces
> >   HID: intel-thc-hid: intel-quickspi: Add HIDSPI protocol implementation
> >   HID: intel-thc-hid: intel-quickspi: Add PM implementation
> >   HID: intel-thc-hid: intel-quicki2c: Add THC QuickI2C driver skeleton
> >   HID: intel-thc-hid: intel-quicki2c: Add THC QuickI2C driver hid layer
> >   HID: intel-thc-hid: intel-quicki2c: Add THC QuickI2C ACPI interfaces
> >   HID: intel-thc-hid: intel-quicki2c: Add HIDI2C protocol implementation
> >   HID: intel-thc-hid: intel-quicki2c: Complete THC QuickI2C driver
> >   HID: intel-thc-hid: intel-quicki2c: Add PM implementation
> >
> > Xinpeng Sun (9):
> >   HID: intel-thc-hid: Add basic THC driver skeleton
> >   HID: intel-thc-hid: intel-thc: Add THC registers definition
> >   HID: intel-thc-hid: intel-thc: Add THC PIO operation APIs
> >   HID: intel-thc-hid: intel-thc: Add APIs for interrupt
> >   HID: intel-thc-hid: intel-thc: Add THC LTR interfaces
> >   HID: intel-thc-hid: intel-thc: Add THC interrupt handler
> >   HID: intel-thc-hid: intel-thc: Add THC SPI config interfaces
> >   HID: intel-thc-hid: intel-quickspi: Add THC QuickSPI driver skeleton
> >   HID: intel-thc-hid: intel-quickspi: Complete THC QuickSPI driver
> >
> >  Documentation/hid/index.rst                   |    1 +
> >  Documentation/hid/intel-thc-hid.rst           |  568 ++++++
> >  MAINTAINERS                                   |    6 +
> >  drivers/hid/Kconfig                           |    2 +
> >  drivers/hid/Makefile                          |    2 +
> >  drivers/hid/intel-thc-hid/Kconfig             |   42 +
> >  drivers/hid/intel-thc-hid/Makefile            |   22 +
> >  .../intel-quicki2c/pci-quicki2c.c             |  966 ++++++++++
> >  .../intel-quicki2c/quicki2c-dev.h             |  186 ++
> >  .../intel-quicki2c/quicki2c-hid.c             |  166 ++
> >  .../intel-quicki2c/quicki2c-hid.h             |   14 +
> >  .../intel-quicki2c/quicki2c-protocol.c        |  224 +++
> >  .../intel-quicki2c/quicki2c-protocol.h        |   20 +
> >  .../intel-quickspi/pci-quickspi.c             |  987 +++++++++++
> >  .../intel-quickspi/quickspi-dev.h             |  172 ++
> >  .../intel-quickspi/quickspi-hid.c             |  165 ++
> >  .../intel-quickspi/quickspi-hid.h             |   14 +
> >  .../intel-quickspi/quickspi-protocol.c        |  409 +++++
> >  .../intel-quickspi/quickspi-protocol.h        |   25 +
> >  .../intel-thc-hid/intel-thc/intel-thc-dev.c   | 1578 +++++++++++++++++
> >  .../intel-thc-hid/intel-thc/intel-thc-dev.h   |  116 ++
> >  .../intel-thc-hid/intel-thc/intel-thc-dma.c   |  969 ++++++++++
> >  .../intel-thc-hid/intel-thc/intel-thc-dma.h   |  146 ++
> >  .../intel-thc-hid/intel-thc/intel-thc-hw.h    |  881 +++++++++
> >  include/linux/hid-over-i2c.h                  |  117 ++
> >  include/linux/hid-over-spi.h                  |  155 ++
> >  26 files changed, 7953 insertions(+)
> >  create mode 100644 Documentation/hid/intel-thc-hid.rst
> >  create mode 100644 drivers/hid/intel-thc-hid/Kconfig  create mode
> > 100644 drivers/hid/intel-thc-hid/Makefile
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-hid.c
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-hid.h
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-protocol.c
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-protocol.h
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-quickspi/quickspi-hid.c
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-quickspi/quickspi-hid.h
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.h
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-thc/intel-thc-dev.c
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-thc/intel-thc-dev.h
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
> >  create mode 100644
> > drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.h
> >  create mode 100644 drivers/hid/intel-thc-hid/intel-thc/intel-thc-hw.h
> >  create mode 100644 include/linux/hid-over-i2c.h  create mode 100644
> > include/linux/hid-over-spi.h
> >
> > --
> > 2.40.1
> 
> For the series:
> 
> Tested on a 'to be announced' Lenovo Lunarlake laptop that uses the THC
> controller and confirmed it worked well. The platform under test is using the
> quicki2c driver.
> Tested touchpad and touchscreen and didn't see any issues.
> 
> Tested-by: Mark Pearson <mpearson-lenovo@...ebb.ca>

Thanks Mark for testing, will add your "Tested-by" in next version.

> 
> Of limited value, as I don't know the hid subsystem well, I did do a code review
> and it looked good to me (only minor notes are typo 'recevie' on patch 6 commit
> description and 'calcualte' on patch 7 commit description).
> 
> Reviewed-by: Mark Pearson <mpearson-lenovo@...ebb.ca>

Thanks for the review, will fix them.

> 
> Thanks!
> Mark

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ