Re: [PATCH v12 2/4] Input: add core support for Goodix Berlin Touchscreen IC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dmitry,

On 10/12/2023 07:53, Dmitry Torokhov wrote:
Hi Neil,

On Sat, Dec 09, 2023 at 08:33:40AM +0100, Neil Armstrong wrote:
Add initial support for the new Goodix "Berlin" touchscreen ICs.

These touchscreen ICs support SPI, I2C and I3C interface, up to
10 finger touch, stylus and gestures events.

This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.

The current implementation only supports BerlinD, aka GT9916.

Support for advanced features like:
- Firmware & config update
- Stylus events
- Gestures events
- Previous revisions support (BerlinA or BerlinB)
is not included in current version.

The current support will work with currently flashed firmware
and config, and bail out if firmware or config aren't flashed yet.

[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers

Reviewed-by: Jeff LaBundy <jeff@xxxxxxxxxxx>
Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>

Thank you for resending the patch. I think there is an issue in how you
read and parse the data in case of more than 2 fingers. It looks like in
that case you are overwriting the checksum form the first 2 and then not
reading the new checksum but use some garbage past the touch data. I
might be mistaken though...

Sure, let me check again to be sure it's not the case


I also believe you are leaking afe_data in case of success. We have the
newfangled __free(kfree) from cleanup.h that should help there.

Indeed, it was added in the meantime, so let's switch to it


Another request - we should not have anything in goodix_berlin.h that is
not used by the I2C and SPI sub-drivers, so the only thing it should
contain is goodix_berlin_probe() declaration and dev_pm_ops. All other
defines and definitions should go to goodix_berlin_core.h.

I made a few more cosmetic changes in the attached patch, please
consider applying it.

Sure, I'll apply it, thanks for the suggestions,

Neil


Thanks.






[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux