* Re: [PATCH v2 3/5] input/touchscreen: imagis: Add support for Imagis IST3038B
From: kernel test robot @ 2023-10-04 2:50 UTC (permalink / raw)
To: karelb, Markuss Broks, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg, linux-input,
devicetree, linux-kernel, ~postmarketos/upstreaming
Cc: oe-kbuild-all, Duje Mihanović, Karel Balej
In-Reply-To: <20231003133440.4696-4-karelb@gimli.ms.mff.cuni.cz>
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on dtor-input/next]
[also build test WARNING on dtor-input/for-linus robh/for-next linus/master v6.6-rc4 next-20231003]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/karelb-gimli-ms-mff-cuni-cz/input-touchscreen-imagis-Correct-the-maximum-touch-area-value/20231003-213739
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20231003133440.4696-4-karelb%40gimli.ms.mff.cuni.cz
patch subject: [PATCH v2 3/5] input/touchscreen: imagis: Add support for Imagis IST3038B
config: x86_64-randconfig-004-20231004 (https://download.01.org/0day-ci/archive/20231004/202310041036.tddy1jGm-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231004/202310041036.tddy1jGm-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310041036.tddy1jGm-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/input/touchscreen/imagis.c:374:39: warning: 'imagis_3038c_data' defined but not used [-Wunused-const-variable=]
374 | static const struct imagis_properties imagis_3038c_data = {
| ^~~~~~~~~~~~~~~~~
>> drivers/input/touchscreen/imagis.c:366:39: warning: 'imagis_3038b_data' defined but not used [-Wunused-const-variable=]
366 | static const struct imagis_properties imagis_3038b_data = {
| ^~~~~~~~~~~~~~~~~
vim +/imagis_3038c_data +374 drivers/input/touchscreen/imagis.c
365
> 366 static const struct imagis_properties imagis_3038b_data = {
367 .interrupt_msg_cmd = IST3038B_REG_STATUS,
368 .touch_coord_cmd = IST3038B_REG_STATUS,
369 .whoami_cmd = IST3038B_REG_CHIPID,
370 .whoami_val = IST3038B_WHOAMI,
371 .protocol_b = true,
372 };
373
> 374 static const struct imagis_properties imagis_3038c_data = {
375 .interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE,
376 .touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
377 .whoami_cmd = IST3038C_REG_CHIPID,
378 .whoami_val = IST3038C_WHOAMI,
379 };
380
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH] Input: i8042 - add Fujitsu Lifebook E5411 to i8042 quirk table
From: Szilard Fabian @ 2023-10-04 1:17 UTC (permalink / raw)
To: linux-kernel, linux-input, dmitry.torokhov; +Cc: Szilard Fabian
In the initial boot stage the integrated keyboard of Fujitsu Lifebook E5411
refuses to work and it's not possible to type for example a dm-crypt
passphrase without the help of an external keyboard.
i8042.nomux kernel parameter resolves this issue but using that a PS/2
mouse is detected. This input device is unused even when the i2c-hid-acpi
kernel module is blacklisted making the integrated ELAN touchpad
(04F3:308A) not working at all.
Since the integrated touchpad is managed by the i2c_designware input
driver in the Linux kernel and you can't find a PS/2 mouse port on the
computer I think it's safe to not use the PS/2 mouse port at all.
Signed-off-by: Szilard Fabian <szfabian@bluemarch.art>
---
I think the same patch could be applied to E4411, E4511 and E5511 too
but sadly I can't test this theory. But these computers use the same
UEFI updates so I think there isn't much difference in input devices.
---
drivers/input/serio/i8042-acpipnpio.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
index 028e45bd050b..3f73fa69b8ce 100644
--- a/drivers/input/serio/i8042-acpipnpio.h
+++ b/drivers/input/serio/i8042-acpipnpio.h
@@ -618,6 +618,14 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
},
.driver_data = (void *)(SERIO_QUIRK_NOMUX)
},
+ {
+ /* Fujitsu Lifebook E5411 */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU CLIENT COMPUTING LIMITED"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "LIFEBOOK E5411"),
+ },
+ .driver_data = (void *)(SERIO_QUIRK_NOAUX)
+ },
{
/* Gigabyte M912 */
.matches = {
--
2.42.0
^ permalink raw reply related
* Re: [regression] Resume broken on T14s Gen1 (AMD) due to "Input: psmouse - add delay when deactivating for SMBus mode"
From: Jeffery Miller @ 2023-10-04 1:08 UTC (permalink / raw)
To: Thorsten Leemhuis
Cc: Andrew Duggan, Dmitry Torokhov, Benjamin Tissoires, linux-input,
linux-kernel, Linux kernel regressions list
In-Reply-To: <906cfb11-ee93-4251-a6ff-1c4d9656b577@leemhuis.info>
On Tue, Oct 3, 2023 at 2:30 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>
> This didn't print anything on resume, so `psmouse->private` apparently
> is set.
>
Thank you for reporting this and providing the information!
need_deactivate is never being set on the smbdev struct since it's elantouch.
On this machine SMBus is not used so it falls back to PS/2 mode.
When this occurs the psmouse->private pointer is being replaced but
psmouse_smbus_reconnect is still being called on resume expecting smbdev.
That explains why when it is setup needs_deactivate is false, but on resume it
has somehow changed to true.
I've submitted a fix for this at
https://lore.kernel.org/all/20231004005729.3943515-1-jefferymiller@google.com/
and it should resolve this issue for you.
Thanks,
Jeff
^ permalink raw reply
* [PATCH] Input: elantech - fix fast_reconnect callback in ps2 mode
From: Jeffery Miller @ 2023-10-04 0:57 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: regressions, benjamin.tissoires, linux, Andrew Duggan,
Andrew Duggan, loic.poulain, Jeffery Miller, Greg Kroah-Hartman,
linux-input, linux-kernel
Make `elantech_setup_ps2` set a compatible fast_reconnect pointer
when its ps2 mode is used.
When an SMBus connection is attempted and fails `psmouse_smbus_init`
sets fast_reconnect to `psmouse_smbus_reconnect`.
`psmouse_smbus_reconnect` expects `psmouse->private` to be
`struct psmouse_smbus_dev` but `elantech_setup_ps2` replaces
it with its private data. This was causing an issue on resume
since psmouse_smbus_reconnect was being called while in ps2, not SMBus
mode.
This was uncovered by commit 92e24e0e57f7 ("Input: psmouse - add delay when
deactivating for SMBus mode")
Closes:
Link:https://lore.kernel.org/all/ca0109fa-c64b-43c1-a651-75b294d750a1@leemhuis.info/
Reported-by: Thorsten Leemhuis <linux@leemhuis.info>
Signed-off-by: Jeffery Miller <jefferymiller@google.com>
---
The other callbacks set in psmouse_smbus_init are already replaced.
Should fast_reconnect be set to `elantech_reconnect` instead?
drivers/input/mouse/elantech.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 2118b2075f43..4e38229404b4 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -2114,6 +2114,7 @@ static int elantech_setup_ps2(struct psmouse *psmouse,
psmouse->protocol_handler = elantech_process_byte;
psmouse->disconnect = elantech_disconnect;
psmouse->reconnect = elantech_reconnect;
+ psmouse->fast_reconnect = NULL;
psmouse->pktsize = info->hw_version > 1 ? 6 : 4;
return 0;
--
2.42.0.582.g8ccd20d70d-goog
^ permalink raw reply related
* Re: [PATCH v2] HID: uhid: replace deprecated strncpy with strscpy
From: Kees Cook @ 2023-10-03 23:41 UTC (permalink / raw)
To: Justin Stitt
Cc: David Rheinsberg, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel, linux-hardening
In-Reply-To: <20231003-strncpy-drivers-hid-uhid-c-v2-1-6a501402581e@google.com>
On Tue, Oct 03, 2023 at 09:01:58PM +0000, Justin Stitt wrote:
> `strncpy` is deprecated for use on NUL-terminated destination strings
> [1] and as such we should prefer more robust and less ambiguous string
> interfaces.
>
> A suitable replacement is `strscpy` [2] due to the fact that it
> guarantees NUL-termination on the destination buffer without
> unnecessarily NUL-padding.
>
> Furthermore, let's make sure `hid->xyz` and `ev->u.create2.xyz` are the
> same size at compile time to prevent silent truncation.
>
> With these changes, it is abundantly clear what the intent and behavior
> of the code is -- We are getting a string to string copy with
> NUL-termination and no truncation.
>
> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-hardening@vger.kernel.org
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Justin Stitt <justinstitt@google.com>
Great! This looks much simpler to me. Thanks for adjusting it.
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v2 0/3] selftests/hid: fix building for older kernels
From: Justin Stitt @ 2023-10-03 22:34 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Eduard Zingerman,
linux-input, linux-kselftest, linux-kernel, bpf
In-Reply-To: <lhb7u2lg7fv2wx3kzrboftqcdtmbjvbzz7zssfn5mho72hcrvj@i53fzzis7b4q>
On Mon, Oct 2, 2023 at 7:48 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> On Sep 26 2023, Justin Stitt wrote:
> > Hey all,
> >
> > Gentle ping on this patch. Looking to get this patch and [1] slated
> > for 6.7 wherein we can start getting cleaner kselftests builds.
> >
> > I do not think I am able to successfully run the hid/bpf selftests due
> > to my kernel version being too low (and an inability to upgrade it as
> > I'm on a corp rolling release). I'd appreciate some insight on how to
> > get the tests running or if someone could actually build+run the tests
> > with this patch applied.
>
> I wanted to apply this series today, but it failed my own CI now with
> the enums being already defined:
> https://gitlab.freedesktop.org/bentiss/hid/-/jobs/49754306
>
> I'll probably squash the following patch in 1/3, would you mind giving
> it a test?
Works for me with this incantation:
$ make LLVM=1 -j128 ARCH=x86_64 mrproper headers && make LLVM=1 -j128
ARCH=x86_64 -C tools/testing/selftests TARGETS=hid
...
---> BINARY hid_bpf
Although, the tests expectedly fail.
Looks good to me.
>
> ---
> From 37feca6c0e84705ad65e621643206c287b63bb0a Mon Sep 17 00:00:00 2001
> From: Benjamin Tissoires <bentiss@kernel.org>
> Date: Mon, 2 Oct 2023 15:37:18 +0200
> Subject: [PATCH] fix selftests/hid: ensure we can compile the tests on kernels
> pre-6.3
>
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> .../selftests/hid/progs/hid_bpf_helpers.h | 30 ++++++++++++++++++-
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> index ab3b18ba48c4..feed5a991e05 100644
> --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> @@ -5,16 +5,44 @@
> #ifndef __HID_BPF_HELPERS_H
> #define __HID_BPF_HELPERS_H
>
> -/* "undefine" structs in vmlinux.h, because we "override" them below */
> +/* "undefine" structs and enums in vmlinux.h, because we "override" them below */
> #define hid_bpf_ctx hid_bpf_ctx___not_used
> #define hid_report_type hid_report_type___not_used
> #define hid_class_request hid_class_request___not_used
> #define hid_bpf_attach_flags hid_bpf_attach_flags___not_used
> +#define HID_INPUT_REPORT HID_INPUT_REPORT___not_used
> +#define HID_OUTPUT_REPORT HID_OUTPUT_REPORT___not_used
> +#define HID_FEATURE_REPORT HID_FEATURE_REPORT___not_used
> +#define HID_REPORT_TYPES HID_REPORT_TYPES___not_used
> +#define HID_REQ_GET_REPORT HID_REQ_GET_REPORT___not_used
> +#define HID_REQ_GET_IDLE HID_REQ_GET_IDLE___not_used
> +#define HID_REQ_GET_PROTOCOL HID_REQ_GET_PROTOCOL___not_used
> +#define HID_REQ_SET_REPORT HID_REQ_SET_REPORT___not_used
> +#define HID_REQ_SET_IDLE HID_REQ_SET_IDLE___not_used
> +#define HID_REQ_SET_PROTOCOL HID_REQ_SET_PROTOCOL___not_used
> +#define HID_BPF_FLAG_NONE HID_BPF_FLAG_NONE___not_used
> +#define HID_BPF_FLAG_INSERT_HEAD HID_BPF_FLAG_INSERT_HEAD·___not_used
> +#define HID_BPF_FLAG_MAX HID_BPF_FLAG_MAX___not_used
> +
> #include "vmlinux.h"
> +
> #undef hid_bpf_ctx
> #undef hid_report_type
> #undef hid_class_request
> #undef hid_bpf_attach_flags
> +#undef HID_INPUT_REPORT
> +#undef HID_OUTPUT_REPORT
> +#undef HID_FEATURE_REPORT
> +#undef HID_REPORT_TYPES
> +#undef HID_REQ_GET_REPORT
> +#undef HID_REQ_GET_IDLE
> +#undef HID_REQ_GET_PROTOCOL
> +#undef HID_REQ_SET_REPORT
> +#undef HID_REQ_SET_IDLE
> +#undef HID_REQ_SET_PROTOCOL
> +#undef HID_BPF_FLAG_NONE
> +#undef HID_BPF_FLAG_INSERT_HEAD
> +#undef HID_BPF_FLAG_MAX
>
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_tracing.h>
> --
> 2.41.0
> ---
>
> Cheers,
> Benjamin
>
> >
> > On Sat, Sep 9, 2023 at 7:22 AM Justin Stitt <justinstitt@google.com> wrote:
> > >
> > > Hi, I am sending this series on behalf of myself and Benjamin Tissoires. There
> > > existed an initial n=3 patch series which was later expanded to n=4 and
> > > is now back to n=3 with some fixes added in and rebased against
> > > mainline.
> > >
> > > This patch series aims to ensure that the hid/bpf selftests can be built
> > > without errors.
> > >
> > > Here's Benjamin's initial cover letter for context:
> > > | These fixes have been triggered by [0]:
> > > | basically, if you do not recompile the kernel first, and are
> > > | running on an old kernel, vmlinux.h doesn't have the required
> > > | symbols and the compilation fails.
> > > |
> > > | The tests will fail if you run them on that very same machine,
> > > | of course, but the binary should compile.
> > > |
> > > | And while I was sorting out why it was failing, I realized I
> > > | could do a couple of improvements on the Makefile.
> > > |
> > > | [0] https://lore.kernel.org/linux-input/56ba8125-2c6f-a9c9-d498-0ca1c153dcb2@redhat.com/T/#t
> > >
> > > Changes from v1 -> v2:
> > > - roll Justin's fix into patch 1/3
> > > - add __attribute__((preserve_access_index)) (thanks Eduard)
> > > - rebased onto mainline (2dde18cd1d8fac735875f2e4987f11817cc0bc2c)
> > > - Link to v1: https://lore.kernel.org/all/20230825-wip-selftests-v1-0-c862769020a8@kernel.org/
> > >
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/1698
> > > Link: https://github.com/ClangBuiltLinux/continuous-integration2/issues/61
> > > ---
> > > Benjamin Tissoires (3):
> > > selftests/hid: ensure we can compile the tests on kernels pre-6.3
> > > selftests/hid: do not manually call headers_install
> > > selftests/hid: force using our compiled libbpf headers
> > >
> > > tools/testing/selftests/hid/Makefile | 10 ++---
> > > tools/testing/selftests/hid/progs/hid.c | 3 --
> > > .../testing/selftests/hid/progs/hid_bpf_helpers.h | 49 ++++++++++++++++++++++
> > > 3 files changed, 53 insertions(+), 9 deletions(-)
> > > ---
> > > base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
> > > change-id: 20230908-kselftest-09-08-56d7f4a8d5c4
> > >
> > > Best regards,
> > > --
> > > Justin Stitt <justinstitt@google.com>
> > >
> >
> > [1]: https://lore.kernel.org/all/20230912-kselftest-param_test-c-v1-1-80a6cffc7374@google.com/
> >
> > Thanks
> > Justin
Thanks
Justin
^ permalink raw reply
* [PATCH] Input: goodix - Ensure int GPIO is in input for gpio_count == 1 && gpio_int_idx == 0 case
From: Hans de Goede @ 2023-10-03 21:51 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Hans de Goede, Bastien Nocera, linux-input, Michael Smith
Add a special case for gpio_count == 1 && gpio_int_idx == 0 to
goodix_add_acpi_gpio_mappings().
It seems that on newer x86/ACPI devices the reset and irq GPIOs are no
longer listed as GPIO resources instead there is only 1 GpioInt resource
and _PS0 does the whole reset sequence for us.
This means that we must call acpi_device_fix_up_power() on these devices
to ensure that the chip is reset before we try to use it.
This part was already fixed in commit 3de93e6ed2df ("Input: goodix - call
acpi_device_fix_up_power() in some cases") by adding a call to
acpi_device_fix_up_power() to the generic "Unexpected ACPI resources"
catch all.
But it turns out that this case on some hw needs some more special
handling. Specifically the firmware may bootup with the IRQ pin in
output mode. The reset sequence from ACPI _PS0 (executed by
acpi_device_fix_up_power()) should put the pin in input mode,
but the GPIO subsystem has cached the direction at bootup, causing
request_irq() to fail due to gpiochip_lock_as_irq() failure:
[ 9.119864] Goodix-TS i2c-GDIX1002:00: Unexpected ACPI resources: gpio_count 1, gpio_int_idx 0
[ 9.317443] Goodix-TS i2c-GDIX1002:00: ID 911, version: 1060
[ 9.321902] input: Goodix Capacitive TouchScreen as /devices/pci0000:00/0000:00:17.0/i2c_designware.4/i2c-5/i2c-GDIX1002:00/input/input8
[ 9.327840] gpio gpiochip0: (INT3453:00): gpiochip_lock_as_irq: tried to flag a GPIO set as output for IRQ
[ 9.327856] gpio gpiochip0: (INT3453:00): unable to lock HW IRQ 26 for IRQ
[ 9.327861] genirq: Failed to request resources for GDIX1002:00 (irq 131) on irqchip intel-gpio
[ 9.327912] Goodix-TS i2c-GDIX1002:00: request IRQ failed: -5
Fix this by adding a special case for gpio_count == 1 && gpio_int_idx == 0
which adds an ACPI GPIO lookup table for the int GPIO even though we cannot
use it for reset purposes (as there is no reset GPIO).
Adding the lookup will make the gpiod_int = gpiod_get(..., GPIOD_IN) call
succeed, which will explicitly set the direction to input fixing the issue.
Note this re-uses the acpi_goodix_int_first_gpios[] lookup table, since
there is only 1 GPIO in the ACPI resources the reset entry in that
lookup table will amount to a no-op.
Reported-and-tested-by: Michael Smith <1973.mjsmith@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
- No Closes tag as this was reported by private email
---
drivers/input/touchscreen/goodix.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index da9954d6df44..af32fbe57b63 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -900,6 +900,25 @@ static int goodix_add_acpi_gpio_mappings(struct goodix_ts_data *ts)
dev_info(dev, "No ACPI GpioInt resource, assuming that the GPIO order is reset, int\n");
ts->irq_pin_access_method = IRQ_PIN_ACCESS_ACPI_GPIO;
gpio_mapping = acpi_goodix_int_last_gpios;
+ } else if (ts->gpio_count == 1 && ts->gpio_int_idx == 0) {
+ /*
+ * On newer devices there is only 1 GpioInt resource and _PS0
+ * does the whole reset sequence for us.
+ */
+ acpi_device_fix_up_power(ACPI_COMPANION(dev));
+
+ /*
+ * Before the _PS0 call the int GPIO may have been in output
+ * mode and the call should have put the int GPIO in input mode,
+ * but the GPIO subsys cached state may still think it is
+ * in output mode, causing gpiochip_lock_as_irq() failure.
+ *
+ * Add a mapping for the int GPIO to make the
+ * gpiod_int = gpiod_get(..., GPIOD_IN) call succeed,
+ * which will explicitly set the direction to input.
+ */
+ ts->irq_pin_access_method = IRQ_PIN_ACCESS_NONE;
+ gpio_mapping = acpi_goodix_int_first_gpios;
} else {
dev_warn(dev, "Unexpected ACPI resources: gpio_count %d, gpio_int_idx %d\n",
ts->gpio_count, ts->gpio_int_idx);
--
2.41.0
^ permalink raw reply related
* [PATCH v2] HID: uhid: replace deprecated strncpy with strscpy
From: Justin Stitt @ 2023-10-03 21:01 UTC (permalink / raw)
To: David Rheinsberg, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, Kees Cook, linux-kernel, David Rheinsberg,
linux-hardening, Justin Stitt
`strncpy` is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.
A suitable replacement is `strscpy` [2] due to the fact that it
guarantees NUL-termination on the destination buffer without
unnecessarily NUL-padding.
Furthermore, let's make sure `hid->xyz` and `ev->u.create2.xyz` are the
same size at compile time to prevent silent truncation.
With these changes, it is abundantly clear what the intent and behavior
of the code is -- We are getting a string to string copy with
NUL-termination and no truncation.
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Changes in v2:
- update subject + commit message (thanks Kees)
- use BUILD_BUG_ON size mismatch (thanks Kees and David)
- Link to v1: https://lore.kernel.org/r/20230914-strncpy-drivers-hid-uhid-c-v1-1-18a190060d8d@google.com
---
drivers/hid/uhid.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 4588d2cd4ea4..a54c7995b9be 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -490,7 +490,7 @@ static int uhid_dev_create2(struct uhid_device *uhid,
const struct uhid_event *ev)
{
struct hid_device *hid;
- size_t rd_size, len;
+ size_t rd_size;
void *rd_data;
int ret;
@@ -514,13 +514,12 @@ static int uhid_dev_create2(struct uhid_device *uhid,
goto err_free;
}
- /* @hid is zero-initialized, strncpy() is correct, strlcpy() not */
- len = min(sizeof(hid->name), sizeof(ev->u.create2.name)) - 1;
- strncpy(hid->name, ev->u.create2.name, len);
- len = min(sizeof(hid->phys), sizeof(ev->u.create2.phys)) - 1;
- strncpy(hid->phys, ev->u.create2.phys, len);
- len = min(sizeof(hid->uniq), sizeof(ev->u.create2.uniq)) - 1;
- strncpy(hid->uniq, ev->u.create2.uniq, len);
+ BUILD_BUG_ON(sizeof(hid->name) != sizeof(ev->u.create2.name));
+ strscpy(hid->name, ev->u.create2.name, sizeof(hid->name));
+ BUILD_BUG_ON(sizeof(hid->phys) != sizeof(ev->u.create2.phys));
+ strscpy(hid->phys, ev->u.create2.phys, sizeof(hid->phys));
+ BUILD_BUG_ON(sizeof(hid->uniq) != sizeof(ev->u.create2.uniq));
+ strscpy(hid->uniq, ev->u.create2.uniq, sizeof(hid->uniq));
hid->ll_driver = &uhid_hid_driver;
hid->bus = ev->u.create2.bus;
---
base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
change-id: 20230914-strncpy-drivers-hid-uhid-c-a465f3e784dd
Best regards,
--
Justin Stitt <justinstitt@google.com>
^ permalink raw reply related
* [PATCH] HID: Add quirk to ignore the touchscreen battery on HP ENVY 15-eu0556ng
From: Fabian Vogt @ 2023-10-03 19:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, linux-kernel
Like various other devices using similar hardware, this model reports a
perpetually empty battery (0-1%).
Join the others and apply HID_BATTERY_QUIRK_IGNORE.
Signed-off-by: Fabian Vogt <fabian@ritter-vogt.de>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-input.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 7e499992a793..e4d2dfd5d253 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -425,6 +425,7 @@
#define I2C_DEVICE_ID_HP_SPECTRE_X360_13T_AW100 0x29F5
#define I2C_DEVICE_ID_HP_SPECTRE_X360_14T_EA100_V1 0x2BED
#define I2C_DEVICE_ID_HP_SPECTRE_X360_14T_EA100_V2 0x2BEE
+#define I2C_DEVICE_ID_HP_ENVY_X360_15_EU0556NG 0x2D02
#define USB_VENDOR_ID_ELECOM 0x056e
#define USB_DEVICE_ID_ELECOM_BM084 0x0061
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 0235cc1690a1..c8b20d44b147 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -409,6 +409,8 @@ static const struct hid_device_id hid_battery_quirks[] = {
HID_BATTERY_QUIRK_IGNORE },
{ HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_HP_SPECTRE_X360_14T_EA100_V2),
HID_BATTERY_QUIRK_IGNORE },
+ { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_HP_ENVY_X360_15_EU0556NG),
+ HID_BATTERY_QUIRK_IGNORE },
{}
};
--
2.42.0
^ permalink raw reply related
* Re: [PATCH RESEND 1/1] Add support for touch screens using the General Touch ST6001S controller.
From: Gareth Randall @ 2023-10-03 18:32 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
In-Reply-To: <YYi8FvXkV5i9baoN@google.com>
Hi Dmitry,
Thanks very much for the feedback. Apologies for the long delay in
getting back to you on this. Updated patch is below, and apologies for
the email layout. I've also added comments to your points in the quoted
message.
Add support for touch screens using the General Touch ST6001S
controller, as found in the GPEG model AOD22WZ-ST monitor.
This controller can output the ELO 10-byte protocol,
but requires different initialisation.
Signed-off-by: Gareth Randall <gareth@garethrandall.com>
diff --git a/drivers/input/touchscreen/elo.c
b/drivers/input/touchscreen/elo.c
index 96173232e53f..53ba056173df 100644
--- a/drivers/input/touchscreen/elo.c
+++ b/drivers/input/touchscreen/elo.c
@@ -26,6 +26,27 @@ MODULE_AUTHOR("Vojtech Pavlik <vojtech@ucw.cz>");
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE("GPL");
+static uint gt_abs_x_min = 0;
+module_param(gt_abs_x_min, uint, S_IRUSR | S_IRGRP | S_IROTH);
+MODULE_PARM_DESC(gt_abs_x_min, "abs_x min value in General Touch mode
(default: 0)");
+
+static uint gt_abs_x_max = 4095;
+module_param(gt_abs_x_max, uint, S_IRUSR | S_IRGRP | S_IROTH);
+MODULE_PARM_DESC(gt_abs_x_max, "abs_x max value in General Touch mode
(default: 4095)");
+
+static uint gt_abs_y_min = 0;
+module_param(gt_abs_y_min, uint, S_IRUSR | S_IRGRP | S_IROTH);
+MODULE_PARM_DESC(gt_abs_y_min, "abs_y min value in General Touch mode
(default: 0)");
+
+static uint gt_abs_y_max = 4095;
+module_param(gt_abs_y_max, uint, S_IRUSR | S_IRGRP | S_IROTH);
+MODULE_PARM_DESC(gt_abs_y_max, "abs_y max value in General Touch mode
(default: 4095)");
+
+static bool gt_mode_override = false;
+module_param(gt_mode_override, bool, S_IRUSR | S_IRGRP | S_IROTH);
+MODULE_PARM_DESC(gt_mode_override, "force the use of General Touch mode
(default: false)");
+
+
/*
* Definitions & global arrays.
*/
@@ -44,6 +65,8 @@ MODULE_LICENSE("GPL");
#define ELO10_ACK_PACKET 'A'
#define ELI10_ID_PACKET 'I'
+#define ELO_GT_INIT_PACKET "\001XfE\r"
+
/*
* Per-touchscreen data.
*/
@@ -201,6 +224,7 @@ static irqreturn_t elo_interrupt(struct serio *serio,
switch (elo->id) {
case 0:
+ case 4:
elo_process_data_10(elo, data);
break;
@@ -255,6 +279,50 @@ static int elo_command_10(struct elo *elo, unsigned
char *packet)
return rc;
}
+/*
+ * Initialise the General Touch ST6001S controller.
+ */
+static int elo_command_10_gt(struct elo *elo)
+{
+ int rc = -1;
+ int i;
+ unsigned char *packet = ELO_GT_INIT_PACKET;
+
+ mutex_lock(&elo->cmd_mutex);
+
+ serio_pause_rx(elo->serio);
+ init_completion(&elo->cmd_done);
+ serio_continue_rx(elo->serio);
+
+ for (i = 0; i < (int)strlen(packet); i++) {
+ if (serio_write(elo->serio, packet[i]))
+ goto out;
+ }
+
+ wait_for_completion_timeout(&elo->cmd_done, HZ);
+ rc = 0;
+
+ out:
+ mutex_unlock(&elo->cmd_mutex);
+ return rc;
+}
+
+static int elo_setup_10_gt(struct elo *elo)
+{
+ struct input_dev *dev = elo->dev;
+
+ if (elo_command_10_gt(elo))
+ return -EIO;
+
+ input_set_abs_params(dev, ABS_X, gt_abs_x_min, gt_abs_x_max, 0, 0);
+ input_set_abs_params(dev, ABS_Y, gt_abs_y_min, gt_abs_y_max, 0, 0);
+
+ dev_info(&elo->serio->dev,
+ "GeneralTouch ST6001S touchscreen");
+
+ return 0;
+}
+
static int elo_setup_10(struct elo *elo)
{
static const char *elo_types[] = { "Accu", "Dura", "Intelli",
"Carroll" };
@@ -332,12 +400,16 @@ static int elo_connect(struct serio *serio, struct
serio_driver *drv)
input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+ __set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
serio_set_drvdata(serio, elo);
err = serio_open(serio, drv);
if (err)
goto fail2;
+ if (gt_mode_override)
+ elo->id = 4;
+
switch (elo->id) {
case 0: /* 10-byte protocol */
@@ -361,6 +433,13 @@ static int elo_connect(struct serio *serio, struct
serio_driver *drv)
input_set_abs_params(input_dev, ABS_X, 0, 255, 0, 0);
input_set_abs_params(input_dev, ABS_Y, 0, 255, 0, 0);
break;
+
+ case 4: /* 10-byte protocol with General Touch initialisation */
+ if (elo_setup_10_gt(elo)) {
+ err = -EIO;
+ goto fail3;
+ }
+ break;
}
err = input_register_device(elo->dev);
On 08/11/2021 05:56, Dmitry Torokhov wrote:
> Hi Gareth,
>
> On Sun, Oct 03, 2021 at 10:54:21PM +0100, Gareth Randall wrote:
>> Add support for touch screens using the General Touch ST6001S
>> controller, as found in the GPEG model AOD22WZ-ST monitor.
>> This controller can output the ELO 10-byte protocol,
>> but requires different initialisation.
>>
>> Signed-off-by: Gareth Randall <gareth@garethrandall.com>
>> ---
>> drivers/input/touchscreen/elo.c | 58 +++++++++++++++++++++++++++++++++
>> 1 file changed, 58 insertions(+)
>>
>> diff --git a/drivers/input/touchscreen/elo.c
>> b/drivers/input/touchscreen/elo.c
>> index 96173232e53f..8c15e0eea6b4 100644
>> --- a/drivers/input/touchscreen/elo.c
>> +++ b/drivers/input/touchscreen/elo.c
>> @@ -44,6 +44,8 @@ MODULE_LICENSE("GPL");
>> #define ELO10_ACK_PACKET 'A'
>> #define ELI10_ID_PACKET 'I'
>>
>> +#define ELO_GT_INIT_PACKET "\001XfE\r"
>> +
>> /*
>> * Per-touchscreen data.
>> */
>> @@ -201,6 +203,7 @@ static irqreturn_t elo_interrupt(struct serio *serio,
>>
>> switch (elo->id) {
>> case 0:
>> + case 4:
>> elo_process_data_10(elo, data);
>> break;
>>
>> @@ -255,6 +258,54 @@ static int elo_command_10(struct elo *elo, unsigned
>> char *packet)
>> return rc;
>> }
>>
>> +/*
>> + * Initialise the General Touch ST6001S controller.
>> + */
>> +static int elo_command_10_gt(struct elo *elo)
>> +{
>> + int rc = -1;
>> + int i;
>> + unsigned char *packet = ELO_GT_INIT_PACKET;
>> +
>> + mutex_lock(&elo->cmd_mutex);
>> +
>> + serio_pause_rx(elo->serio);
>> + init_completion(&elo->cmd_done);
>> + serio_continue_rx(elo->serio);
>> +
>> + for (i = 0; i < (int)strlen(packet); i++) {
>> + if (serio_write(elo->serio, packet[i]))
>> + goto out;
>> + }
>> +
>> + wait_for_completion_timeout(&elo->cmd_done, HZ);
>> + rc = 0;
>> +
>> + out:
>> + mutex_unlock(&elo->cmd_mutex);
>> + return rc;
>> +}
>> +
>> +static int elo_setup_10_gt(struct elo *elo)
>> +{
>> + struct input_dev *dev = elo->dev;
>> +
>> + if (elo_command_10_gt(elo))
>> + return -1;
>
> return -EIO;
>
>> +
>> + __set_bit(INPUT_PROP_DIRECT, dev->propbit);
>
> Please make this a separate patch that would set this property for all
> variants of ELO touchscreens (i.e. move it into elo_connect()).
>
>> +
>> + // Values taken from a GPEG model AOD22WZ-ST monitor
>> + input_set_abs_params(dev, ABS_X, 1365, 5828, 0, 0);
>
> I believe the datasheet says that the touch resolution is 4096 x 4096:
> http://www.boardcon.com/download/LCD_datasheet/15inch_SAW_LCD/Serial%20Controller%20ST6001S%20SPEC.pdf
I never felt comfortable with the hard-coded values that I'd put in my
patch, so I've changed them to your suggestion (0 and 4095) but with the
option to override using kernel parameters. I hope this is okay.
(My understanding of input_set_abs_params is that the input device has
to be able to reach the values, even if it can go above the max and
below the min. When moving my finger on the screen concerned, 1365 was
the lowest it would comfortably go and 5828 was the highest, meaning
that it would never be able to reach 0.)
>
>
>> + // max and min inverted because screen axis is inverted
>> + input_set_abs_params(dev, ABS_Y, 5013, 2260, 0, 0);
>
> I dont think this changes anything for reported coordinates by the
> driver.
When experimenting with the hardware the pointer would move correctly
with "inverted" max and min, but allowing these to be overridden through
kernel parameters will be fine.
>> +
>> + dev_info(&elo->serio->dev,
>> + "GeneralTouch ST6001S touchscreen");
>> +
>> + return 0;
>> +}
>> +
>> static int elo_setup_10(struct elo *elo)
>> {
>> static const char *elo_types[] = { "Accu", "Dura", "Intelli", "Carroll" };
>> @@ -361,6 +412,13 @@ static int elo_connect(struct serio *serio, struct
>> serio_driver *drv)
>> input_set_abs_params(input_dev, ABS_X, 0, 255, 0, 0);
>> input_set_abs_params(input_dev, ABS_Y, 0, 255, 0, 0);
>> break;
>> +
>> + case 4: /* 10-byte protocol with General Touch initialisation */
>> + if (elo_setup_10_gt(elo)) {
>> + err = -EIO;
>> + goto fail3;
>> + }
>> + break;
>> }
>>
>> err = input_register_device(elo->dev);
>> --
>> 2.27.0
>
> Thanks.
>
Thanks very much for your feedback.
Yours,
Gareth
^ permalink raw reply related
* [PATCH] HID: intel-ish-hid: ipc: Disable and reenable ACPI GPE bit
From: Srinivas Pandruvada @ 2023-10-03 15:53 UTC (permalink / raw)
To: jikos, benjamin.tissoires
Cc: linux-input, linux-kernel, Srinivas Pandruvada, Kai-Heng Feng
The EHL (Elkhart Lake) based platforms provide a OOB (Out of band)
service, which allows to wakup device when the system is in S5 (Soft-Off
state). This OOB service can be enabled/disabled from BIOS settings. When
enabled, the ISH device gets PME wake capability. To enable PME wakeup,
driver also needs to enable ACPI GPE bit.
On resume, BIOS will clear the wakeup bit. So driver need to re-enable it
in resume function to keep the next wakeup capability. But this BIOS
clearing of wakeup bit doesn't decrement internal OS GPE reference count,
so this reenabling on every resume will cause reference count to overflow.
So first disable and reenable ACPI GPE bit using acpi_disable_gpe().
Fixes: 2e23a70edabe ("HID: intel-ish-hid: ipc: finish power flow for EHL OOB")
Reported-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Closes: https://lore.kernel.org/lkml/CAAd53p4=oLYiH2YbVSmrPNj1zpMcfp=Wxbasb5vhMXOWCArLCg@mail.gmail.com/T/
Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ipc/pci-ish.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index 55cb25038e63..710fda5f19e1 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -133,6 +133,14 @@ static int enable_gpe(struct device *dev)
}
wakeup = &adev->wakeup;
+ /*
+ * Call acpi_disable_gpe(), so that reference count
+ * gpe_event_info->runtime_count doesn't overflow.
+ * When gpe_event_info->runtime_count = 0, the call
+ * to acpi_disable_gpe() simply return.
+ */
+ acpi_disable_gpe(wakeup->gpe_device, wakeup->gpe_number);
+
acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup->gpe_number);
if (ACPI_FAILURE(acpi_sts)) {
dev_err(dev, "enable ose_gpe failed\n");
--
2.41.0
^ permalink raw reply related
* Re: [PATCH v2 0/5] input/touchscreen: imagis: add support for IST3032C
From: Karel Balej @ 2023-10-03 13:45 UTC (permalink / raw)
To: Markuss Broks, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Henrik Rydberg, linux-input, devicetree,
linux-kernel, ~postmarketos/upstreaming
Cc: Duje Mihanović
In-Reply-To: <20231003133440.4696-1-karelb@gimli.ms.mff.cuni.cz>
Hello,
> From: Karel Balej <karelb@gimli.ms.mff.cuni.cz>
I am very sorry, I wanted to use a different email address for sending
than for commiting so that the message would also reach people whose
email providers have more strict requirements on sender authentication
(such as Google), but it seems that I have made a mistake and confused
git altogether.
I will fix it in a possible v3 after I receive some feedback or I will
resend it before it gets applied and I will make sure to properly test
the setup then.
My apologies,
K. B.
^ permalink raw reply
* [PATCH v2 5/5] input/touchscreen: imagis: add support for IST3032C
From: karelb @ 2023-10-03 13:34 UTC (permalink / raw)
To: Markuss Broks, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Henrik Rydberg, linux-input, devicetree,
linux-kernel, ~postmarketos/upstreaming
Cc: Duje Mihanović, Karel Balej, Karel Balej
In-Reply-To: <20231003133440.4696-1-karelb@gimli.ms.mff.cuni.cz>
From: Karel Balej <karelb@gimli.ms.mff.cuni.cz>
From: Karel Balej <balejk@matfyz.cz>
IST3032C is a touchscreen chip used for instance in the
samsung,coreprimevelte smartphone, with which this was tested. Add the
chip specific information to the driver.
Signed-off-by: Karel Balej <balejk@matfyz.cz>
---
drivers/input/touchscreen/imagis.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index 84a02672ac47..41f28e6e9cb1 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -35,6 +35,8 @@
#define IST3038B_REG_CHIPID 0x30
#define IST3038B_WHOAMI 0x30380b
+#define IST3032C_WHOAMI 0x32c
+
struct imagis_properties {
unsigned int interrupt_msg_cmd;
unsigned int touch_coord_cmd;
@@ -363,6 +365,13 @@ static int imagis_resume(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(imagis_pm_ops, imagis_suspend, imagis_resume);
+static const struct imagis_properties imagis_3032c_data = {
+ .interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE,
+ .touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
+ .whoami_cmd = IST3038C_REG_CHIPID,
+ .whoami_val = IST3032C_WHOAMI,
+};
+
static const struct imagis_properties imagis_3038b_data = {
.interrupt_msg_cmd = IST3038B_REG_STATUS,
.touch_coord_cmd = IST3038B_REG_STATUS,
@@ -380,6 +389,7 @@ static const struct imagis_properties imagis_3038c_data = {
#ifdef CONFIG_OF
static const struct of_device_id imagis_of_match[] = {
+ { .compatible = "imagis,ist3032c", .data = &imagis_3032c_data },
{ .compatible = "imagis,ist3038b", .data = &imagis_3038b_data },
{ .compatible = "imagis,ist3038c", .data = &imagis_3038c_data },
{ },
--
2.42.0
^ permalink raw reply related
* [PATCH v2 4/5] dt-bindings: input/touchscreen: imagis: add compatible for IST3032C
From: karelb @ 2023-10-03 13:34 UTC (permalink / raw)
To: Markuss Broks, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Henrik Rydberg, linux-input, devicetree,
linux-kernel, ~postmarketos/upstreaming
Cc: Duje Mihanović, Karel Balej, Karel Balej
In-Reply-To: <20231003133440.4696-1-karelb@gimli.ms.mff.cuni.cz>
From: Karel Balej <karelb@gimli.ms.mff.cuni.cz>
From: Karel Balej <balejk@matfyz.cz>
Document possible usage of the Imagis driver with the IST3032C
touchscreen.
Signed-off-by: Karel Balej <balejk@matfyz.cz>
---
.../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
index b5372c4eae56..2af71cbcc97d 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
@@ -18,6 +18,7 @@ properties:
compatible:
enum:
+ - imagis,ist3032c
- imagis,ist3038b
- imagis,ist3038c
--
2.42.0
^ permalink raw reply related
* [PATCH v2 3/5] input/touchscreen: imagis: Add support for Imagis IST3038B
From: karelb @ 2023-10-03 13:34 UTC (permalink / raw)
To: Markuss Broks, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Henrik Rydberg, linux-input, devicetree,
linux-kernel, ~postmarketos/upstreaming
Cc: Duje Mihanović, Karel Balej, Karel Balej
In-Reply-To: <20231003133440.4696-1-karelb@gimli.ms.mff.cuni.cz>
From: Karel Balej <karelb@gimli.ms.mff.cuni.cz>
From: Markuss Broks <markuss.broks@gmail.com>
Imagis IST3038B is another variant of Imagis IST3038 IC, which has
a different register interface from IST3038C (possibly firmware defined).
This should also work for IST3044B (though untested), however other
variants using this interface/protocol(IST3026, IST3032, IST3026B,
IST3032B) have a different format for coordinates, and they'd need
additional effort to be supported by this driver.
Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
Signed-off-by: Karel Balej <balejk@matfyz.cz>
---
drivers/input/touchscreen/imagis.c | 58 ++++++++++++++++++++++++------
1 file changed, 47 insertions(+), 11 deletions(-)
diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index e67fd3011027..84a02672ac47 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -13,7 +13,7 @@
#define IST3038C_HIB_ACCESS (0x800B << 16)
#define IST3038C_DIRECT_ACCESS BIT(31)
-#define IST3038C_REG_CHIPID 0x40001000
+#define IST3038C_REG_CHIPID (0x40001000 | IST3038C_DIRECT_ACCESS)
#define IST3038C_REG_HIB_BASE 0x30000100
#define IST3038C_REG_TOUCH_STATUS (IST3038C_REG_HIB_BASE | IST3038C_HIB_ACCESS)
#define IST3038C_REG_TOUCH_COORD (IST3038C_REG_HIB_BASE | IST3038C_HIB_ACCESS | 0x8)
@@ -31,8 +31,21 @@
#define IST3038C_FINGER_COUNT_SHIFT 12
#define IST3038C_FINGER_STATUS_MASK GENMASK(9, 0)
+#define IST3038B_REG_STATUS 0x20
+#define IST3038B_REG_CHIPID 0x30
+#define IST3038B_WHOAMI 0x30380b
+
+struct imagis_properties {
+ unsigned int interrupt_msg_cmd;
+ unsigned int touch_coord_cmd;
+ unsigned int whoami_cmd;
+ unsigned int whoami_val;
+ bool protocol_b;
+};
+
struct imagis_ts {
struct i2c_client *client;
+ const struct imagis_properties *tdata;
struct input_dev *input_dev;
struct touchscreen_properties prop;
struct regulator_bulk_data supplies[2];
@@ -84,8 +97,7 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
int i;
int error;
- error = imagis_i2c_read_reg(ts, IST3038C_REG_INTR_MESSAGE,
- &intr_message);
+ error = imagis_i2c_read_reg(ts, ts->tdata->interrupt_msg_cmd, &intr_message);
if (error) {
dev_err(&ts->client->dev,
"failed to read the interrupt message: %d\n", error);
@@ -104,9 +116,13 @@ static irqreturn_t imagis_interrupt(int irq, void *dev_id)
finger_pressed = intr_message & IST3038C_FINGER_STATUS_MASK;
for (i = 0; i < finger_count; i++) {
- error = imagis_i2c_read_reg(ts,
- IST3038C_REG_TOUCH_COORD + (i * 4),
- &finger_status);
+ if (ts->tdata->protocol_b)
+ error = imagis_i2c_read_reg(ts,
+ ts->tdata->touch_coord_cmd, &finger_status);
+ else
+ error = imagis_i2c_read_reg(ts,
+ ts->tdata->touch_coord_cmd + (i * 4),
+ &finger_status);
if (error) {
dev_err(&ts->client->dev,
"failed to read coordinates for finger %d: %d\n",
@@ -261,6 +277,12 @@ static int imagis_probe(struct i2c_client *i2c)
ts->client = i2c;
+ ts->tdata = device_get_match_data(dev);
+ if (!ts->tdata) {
+ dev_err(dev, "missing chip data\n");
+ return -EINVAL;
+ }
+
error = imagis_init_regulators(ts);
if (error) {
dev_err(dev, "regulator init error: %d\n", error);
@@ -279,15 +301,13 @@ static int imagis_probe(struct i2c_client *i2c)
return error;
}
- error = imagis_i2c_read_reg(ts,
- IST3038C_REG_CHIPID | IST3038C_DIRECT_ACCESS,
- &chip_id);
+ error = imagis_i2c_read_reg(ts, ts->tdata->whoami_cmd, &chip_id);
if (error) {
dev_err(dev, "chip ID read failure: %d\n", error);
return error;
}
- if (chip_id != IST3038C_WHOAMI) {
+ if (chip_id != ts->tdata->whoami_val) {
dev_err(dev, "unknown chip ID: 0x%x\n", chip_id);
return -EINVAL;
}
@@ -343,9 +363,25 @@ static int imagis_resume(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(imagis_pm_ops, imagis_suspend, imagis_resume);
+static const struct imagis_properties imagis_3038b_data = {
+ .interrupt_msg_cmd = IST3038B_REG_STATUS,
+ .touch_coord_cmd = IST3038B_REG_STATUS,
+ .whoami_cmd = IST3038B_REG_CHIPID,
+ .whoami_val = IST3038B_WHOAMI,
+ .protocol_b = true,
+};
+
+static const struct imagis_properties imagis_3038c_data = {
+ .interrupt_msg_cmd = IST3038C_REG_INTR_MESSAGE,
+ .touch_coord_cmd = IST3038C_REG_TOUCH_COORD,
+ .whoami_cmd = IST3038C_REG_CHIPID,
+ .whoami_val = IST3038C_WHOAMI,
+};
+
#ifdef CONFIG_OF
static const struct of_device_id imagis_of_match[] = {
- { .compatible = "imagis,ist3038c", },
+ { .compatible = "imagis,ist3038b", .data = &imagis_3038b_data },
+ { .compatible = "imagis,ist3038c", .data = &imagis_3038c_data },
{ },
};
MODULE_DEVICE_TABLE(of, imagis_of_match);
--
2.42.0
^ permalink raw reply related
* [PATCH v2 2/5] dt-bindings: input/touchscreen: Add compatible for IST3038B
From: karelb @ 2023-10-03 13:34 UTC (permalink / raw)
To: Markuss Broks, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Henrik Rydberg, linux-input, devicetree,
linux-kernel, ~postmarketos/upstreaming
Cc: Duje Mihanović, Karel Balej, Karel Balej
In-Reply-To: <20231003133440.4696-1-karelb@gimli.ms.mff.cuni.cz>
From: Karel Balej <karelb@gimli.ms.mff.cuni.cz>
From: Markuss Broks <markuss.broks@gmail.com>
Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC,
add the compatible for it to the IST3038C bindings.
Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
Signed-off-by: Karel Balej <balejk@matfyz.cz>
---
.../devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
index 0d6b033fd5fb..b5372c4eae56 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/imagis,ist3038c.yaml
@@ -18,6 +18,7 @@ properties:
compatible:
enum:
+ - imagis,ist3038b
- imagis,ist3038c
reg:
--
2.42.0
^ permalink raw reply related
* [PATCH v2 1/5] input/touchscreen: imagis: Correct the maximum touch area value
From: karelb @ 2023-10-03 13:34 UTC (permalink / raw)
To: Markuss Broks, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Henrik Rydberg, linux-input, devicetree,
linux-kernel, ~postmarketos/upstreaming
Cc: Duje Mihanović, Karel Balej, Karel Balej
In-Reply-To: <20231003133440.4696-1-karelb@gimli.ms.mff.cuni.cz>
From: Karel Balej <karelb@gimli.ms.mff.cuni.cz>
From: Markuss Broks <markuss.broks@gmail.com>
As specified in downstream IST3038B driver and proved by testing,
the correct maximum reported value of touch area is 16.
Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
Signed-off-by: Karel Balej <balejk@matfyz.cz>
---
drivers/input/touchscreen/imagis.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/imagis.c b/drivers/input/touchscreen/imagis.c
index 07111ca24455..e67fd3011027 100644
--- a/drivers/input/touchscreen/imagis.c
+++ b/drivers/input/touchscreen/imagis.c
@@ -210,7 +210,7 @@ static int imagis_init_input_dev(struct imagis_ts *ts)
input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X);
input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y);
- input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, 16, 0, 0);
touchscreen_parse_properties(input_dev, true, &ts->prop);
if (!ts->prop.max_x || !ts->prop.max_y) {
--
2.42.0
^ permalink raw reply related
* [PATCH v2 0/5] input/touchscreen: imagis: add support for IST3032C
From: karelb @ 2023-10-03 13:34 UTC (permalink / raw)
To: Markuss Broks, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Henrik Rydberg, linux-input, devicetree,
linux-kernel, ~postmarketos/upstreaming
Cc: Duje Mihanović, Karel Balej
From: Karel Balej <karelb@gimli.ms.mff.cuni.cz>
This patch series generalizes the Imagis touchscreen driver to support
other Imagis chips, namely IST3038B, which use a slightly different
protocol.
It also adds necessary information to the driver so that the IST3032C
touchscreen can be used with it. The motivation for this is the
samsung,coreprimevelte smartphone with which this series has been
tested. However, the support for this device is not yet in-tree, the
effort is happening at [1]. In particular, the driver for the regulator
needed by the touchscreen on this device has not been rewritten for
mainline yet.
[1] https://lore.kernel.org/all/20230812-pxa1908-lkml-v5-0-a5d51937ee34@skole.hr/
---
Changes in v2:
- Do not rename the driver.
- Do not hardcode voltage required by the IST3032C.
- Use Markuss' series which generalizes the driver.
- Separate bindings into separate patch.
- v1: https://lore.kernel.org/all/20230926173531.18715-1-balejk@matfyz.cz/
---
Karel Balej (2):
dt-bindings: input/touchscreen: imagis: add compatible for IST3032C
input/touchscreen: imagis: add support for IST3032C
Markuss Broks (3):
input/touchscreen: imagis: Correct the maximum touch area value
dt-bindings: input/touchscreen: Add compatible for IST3038B
input/touchscreen: imagis: Add support for Imagis IST3038B
.../input/touchscreen/imagis,ist3038c.yaml | 2 +
drivers/input/touchscreen/imagis.c | 70 +++++++++++++++----
2 files changed, 60 insertions(+), 12 deletions(-)
--
2.42.0
^ permalink raw reply
* Re: [PATCH 2/3] Input: cap11xx - Convert to use maple tree register cache
From: Mark Brown @ 2023-10-03 12:16 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <ZRux0yLPxZGLNF5A@google.com>
[-- Attachment #1: Type: text/plain, Size: 770 bytes --]
On Mon, Oct 02, 2023 at 11:16:51PM -0700, Dmitry Torokhov wrote:
> On Sun, Oct 01, 2023 at 01:43:39AM +0200, Mark Brown wrote:
> > - .cache_type = REGCACHE_RBTREE,
> > + .cache_type = REGCACHE_MAPLE,
> I do not think these driver care much about the cache type. Optimal one
> might even depend on the architecture. I wonder if we could have
> something like REGCACHE_DEFAULT to signal that whatever is the "best
> default" implementation it should be used?
I do sometimes wonder about that but there's also been enough issues
with the earlier stage of the transition and shaking out bugs in the
new code that it makes me a bit nervous about using one. It has also
been a useful exercise to go through and actually look at the users, but
that could be done any time.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH v8 4/4] Input: goodix-berlin - add SPI support for Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-10-03 8:38 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bastien Nocera,
Hans de Goede, Henrik Rydberg, Jeff LaBundy, linux-arm-msm,
devicetree, linux-kernel, Neil Armstrong
In-Reply-To: <20231003-topic-goodix-berlin-upstream-initial-v8-0-171606102ed6@linaro.org>
Add initial support for the new Goodix "Berlin" touchscreen ICs
over the SPI interface.
The driver doesn't use the regmap_spi code since the SPI messages
needs to be prefixed, thus this custom regmap code.
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.
[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@labundy.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/input/touchscreen/Kconfig | 14 +++
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_berlin_spi.c | 173 ++++++++++++++++++++++++++
3 files changed, 188 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index cc7b88118158..c821fe3ee794 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -433,6 +433,20 @@ config TOUCHSCREEN_GOODIX_BERLIN_I2C
To compile this driver as a module, choose M here: the
module will be called goodix_berlin_i2c.
+config TOUCHSCREEN_GOODIX_BERLIN_SPI
+ tristate "Goodix Berlin SPI touchscreen"
+ depends on SPI_MASTER
+ select REGMAP
+ select TOUCHSCREEN_GOODIX_BERLIN_CORE
+ help
+ Say Y here if you have a Goodix Berlin IC connected to
+ your system via SPI.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called goodix_berlin_spi.
+
config TOUCHSCREEN_HIDEEP
tristate "HiDeep Touch IC"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 7ef677cf7a10..a81cb5aa21a5 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C) += goodix_berlin_i2c.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_SPI) += goodix_berlin_spi.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
diff --git a/drivers/input/touchscreen/goodix_berlin_spi.c b/drivers/input/touchscreen/goodix_berlin_spi.c
new file mode 100644
index 000000000000..1f57c3592918
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin_spi.c
@@ -0,0 +1,173 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Goodix Berlin Touchscreen Driver
+ *
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_ts_berlin driver.
+ */
+#include <asm/unaligned.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+#include "goodix_berlin.h"
+
+#define GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN 1
+#define GOODIX_BERLIN_REGISTER_WIDTH 4
+#define GOODIX_BERLIN_SPI_READ_DUMMY_LEN 3
+#define GOODIX_BERLIN_SPI_READ_PREFIX_LEN (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
+ GOODIX_BERLIN_REGISTER_WIDTH + \
+ GOODIX_BERLIN_SPI_READ_DUMMY_LEN)
+#define GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
+ GOODIX_BERLIN_REGISTER_WIDTH)
+
+#define GOODIX_BERLIN_SPI_WRITE_FLAG 0xF0
+#define GOODIX_BERLIN_SPI_READ_FLAG 0xF1
+
+static int goodix_berlin_spi_read(void *context, const void *reg_buf,
+ size_t reg_size, void *val_buf,
+ size_t val_size)
+{
+ struct spi_device *spi = context;
+ struct spi_transfer xfers;
+ struct spi_message spi_msg;
+ const u32 *reg = reg_buf; /* reg is stored as native u32 at start of buffer */
+ u8 *buf;
+ int error;
+
+ if (reg_size != GOODIX_BERLIN_REGISTER_WIDTH)
+ return -EINVAL;
+
+ buf = kzalloc(GOODIX_BERLIN_SPI_READ_PREFIX_LEN + val_size, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ spi_message_init(&spi_msg);
+ memset(&xfers, 0, sizeof(xfers));
+
+ /* buffer format: 0xF1 + addr(4bytes) + dummy(3bytes) + data */
+ buf[0] = GOODIX_BERLIN_SPI_READ_FLAG;
+ put_unaligned_be32(*reg, buf + GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN);
+ memset(buf + GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + GOODIX_BERLIN_REGISTER_WIDTH,
+ 0xff, GOODIX_BERLIN_SPI_READ_DUMMY_LEN);
+
+ xfers.tx_buf = buf;
+ xfers.rx_buf = buf;
+ xfers.len = GOODIX_BERLIN_SPI_READ_PREFIX_LEN + val_size;
+ xfers.cs_change = 0;
+ spi_message_add_tail(&xfers, &spi_msg);
+
+ error = spi_sync(spi, &spi_msg);
+ if (error < 0)
+ dev_err(&spi->dev, "spi transfer error, %d", error);
+ else
+ memcpy(val_buf, buf + GOODIX_BERLIN_SPI_READ_PREFIX_LEN, val_size);
+
+ kfree(buf);
+ return error;
+}
+
+static int goodix_berlin_spi_write(void *context, const void *data,
+ size_t count)
+{
+ unsigned int len = count - GOODIX_BERLIN_REGISTER_WIDTH;
+ struct spi_device *spi = context;
+ struct spi_transfer xfers;
+ struct spi_message spi_msg;
+ const u32 *reg = data; /* reg is stored as native u32 at start of buffer */
+ u8 *buf;
+ int error;
+
+ buf = kzalloc(GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN + len, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ spi_message_init(&spi_msg);
+ memset(&xfers, 0, sizeof(xfers));
+
+ buf[0] = GOODIX_BERLIN_SPI_WRITE_FLAG;
+ put_unaligned_be32(*reg, buf + GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN);
+ memcpy(buf + GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN,
+ data + GOODIX_BERLIN_REGISTER_WIDTH, len);
+
+ xfers.tx_buf = buf;
+ xfers.len = GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN + len;
+ xfers.cs_change = 0;
+ spi_message_add_tail(&xfers, &spi_msg);
+
+ error = spi_sync(spi, &spi_msg);
+ if (error < 0)
+ dev_err(&spi->dev, "spi transfer error, %d", error);
+
+ kfree(buf);
+ return error;
+}
+
+static const struct regmap_config goodix_berlin_spi_regmap_conf = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .read = goodix_berlin_spi_read,
+ .write = goodix_berlin_spi_write,
+};
+
+/* vendor & product left unassigned here, should probably be updated from fw info */
+static const struct input_id goodix_berlin_spi_input_id = {
+ .bustype = BUS_SPI,
+};
+
+static int goodix_berlin_spi_probe(struct spi_device *spi)
+{
+ struct regmap_config regmap_config;
+ struct regmap *regmap;
+ size_t max_size;
+ int error = 0;
+
+ spi->mode = SPI_MODE_0;
+ spi->bits_per_word = 8;
+ error = spi_setup(spi);
+ if (error)
+ return error;
+
+ max_size = spi_max_transfer_size(spi);
+
+ regmap_config = goodix_berlin_spi_regmap_conf;
+ regmap_config.max_raw_read = max_size - GOODIX_BERLIN_SPI_READ_PREFIX_LEN;
+ regmap_config.max_raw_write = max_size - GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN;
+
+ regmap = devm_regmap_init(&spi->dev, NULL, spi, ®map_config);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ return goodix_berlin_probe(&spi->dev, spi->irq,
+ &goodix_berlin_spi_input_id, regmap);
+}
+
+static const struct spi_device_id goodix_berlin_spi_ids[] = {
+ { "gt9916" },
+ { },
+};
+MODULE_DEVICE_TABLE(spi, goodix_berlin_spi_ids);
+
+static const struct of_device_id goodix_berlin_spi_of_match[] = {
+ { .compatible = "goodix,gt9916", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, goodix_berlin_spi_of_match);
+
+static struct spi_driver goodix_berlin_spi_driver = {
+ .driver = {
+ .name = "goodix-berlin-spi",
+ .of_match_table = goodix_berlin_spi_of_match,
+ .pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
+ },
+ .probe = goodix_berlin_spi_probe,
+ .id_table = goodix_berlin_spi_ids,
+};
+module_spi_driver(goodix_berlin_spi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix Berlin SPI Touchscreen driver");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
--
2.34.1
^ permalink raw reply related
* [PATCH v8 2/4] Input: add core support for Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-10-03 8:38 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bastien Nocera,
Hans de Goede, Henrik Rydberg, Jeff LaBundy, linux-arm-msm,
devicetree, linux-kernel, Neil Armstrong
In-Reply-To: <20231003-topic-goodix-berlin-upstream-initial-v8-0-171606102ed6@linaro.org>
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@labundy.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/input/touchscreen/Kconfig | 3 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_berlin.h | 159 +++++++
drivers/input/touchscreen/goodix_berlin_core.c | 582 +++++++++++++++++++++++++
4 files changed, 745 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index e3e2324547b9..950da599ae1a 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -416,6 +416,9 @@ config TOUCHSCREEN_GOODIX
To compile this driver as a module, choose M here: the
module will be called goodix.
+config TOUCHSCREEN_GOODIX_BERLIN_CORE
+ tristate
+
config TOUCHSCREEN_HIDEEP
tristate "HiDeep Touch IC"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 62bd24f3ac8e..2e2f3e70cd2c 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_TOUCHSCREEN_EGALAX_SERIAL) += egalax_ts_serial.o
obj-$(CONFIG_TOUCHSCREEN_EXC3000) += exc3000.o
obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
new file mode 100644
index 000000000000..235f44947a28
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin.h
@@ -0,0 +1,159 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Goodix Touchscreen Driver
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_berlin_berlin driver.
+ */
+
+#ifndef __GOODIX_BERLIN_H_
+#define __GOODIX_BERLIN_H_
+
+#include <linux/gpio/consumer.h>
+#include <linux/input.h>
+#include <linux/input/touchscreen.h>
+#include <linux/regulator/consumer.h>
+#include <linux/sizes.h>
+
+#define GOODIX_BERLIN_MAX_TOUCH 10
+
+#define GOODIX_BERLIN_NORMAL_RESET_DELAY_MS 100
+
+#define GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN 8
+#define GOODIX_BERLIN_STATUS_OFFSET 0
+#define GOODIX_BERLIN_REQUEST_TYPE_OFFSET 2
+
+#define GOODIX_BERLIN_BYTES_PER_POINT 8
+#define GOODIX_BERLIN_COOR_DATA_CHECKSUM_SIZE 2
+#define GOODIX_BERLIN_COOR_DATA_CHECKSUM_MASK GENMASK(15, 0)
+
+/* Read n finger events */
+#define GOODIX_BERLIN_IRQ_READ_LEN(n) (GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN + \
+ (GOODIX_BERLIN_BYTES_PER_POINT * (n)) + \
+ GOODIX_BERLIN_COOR_DATA_CHECKSUM_SIZE)
+
+#define GOODIX_BERLIN_TOUCH_EVENT BIT(7)
+#define GOODIX_BERLIN_REQUEST_EVENT BIT(6)
+#define GOODIX_BERLIN_TOUCH_COUNT_MASK GENMASK(3, 0)
+
+#define GOODIX_BERLIN_REQUEST_CODE_RESET 3
+
+#define GOODIX_BERLIN_POINT_TYPE_MASK GENMASK(3, 0)
+#define GOODIX_BERLIN_POINT_TYPE_STYLUS_HOVER 1
+#define GOODIX_BERLIN_POINT_TYPE_STYLUS 3
+
+#define GOODIX_BERLIN_TOUCH_ID_MASK GENMASK(7, 4)
+
+#define GOODIX_BERLIN_DEV_CONFIRM_VAL 0xAA
+#define GOODIX_BERLIN_BOOTOPTION_ADDR 0x10000
+#define GOODIX_BERLIN_FW_VERSION_INFO_ADDR 0x10014
+
+#define GOODIX_BERLIN_IC_INFO_MAX_LEN SZ_1K
+#define GOODIX_BERLIN_IC_INFO_ADDR 0x10070
+
+struct goodix_berlin_fw_version {
+ u8 rom_pid[6];
+ u8 rom_vid[3];
+ u8 rom_vid_reserved;
+ u8 patch_pid[8];
+ u8 patch_vid[4];
+ u8 patch_vid_reserved;
+ u8 sensor_id;
+ u8 reserved[2];
+ __le16 checksum;
+} __packed;
+
+struct goodix_berlin_ic_info_version {
+ u8 info_customer_id;
+ u8 info_version_id;
+ u8 ic_die_id;
+ u8 ic_version_id;
+ __le32 config_id;
+ u8 config_version;
+ u8 frame_data_customer_id;
+ u8 frame_data_version_id;
+ u8 touch_data_customer_id;
+ u8 touch_data_version_id;
+ u8 reserved[3];
+} __packed;
+
+struct goodix_berlin_ic_info_feature {
+ __le16 freqhop_feature;
+ __le16 calibration_feature;
+ __le16 gesture_feature;
+ __le16 side_touch_feature;
+ __le16 stylus_feature;
+} __packed;
+
+struct goodix_berlin_ic_info_misc {
+ __le32 cmd_addr;
+ __le16 cmd_max_len;
+ __le32 cmd_reply_addr;
+ __le16 cmd_reply_len;
+ __le32 fw_state_addr;
+ __le16 fw_state_len;
+ __le32 fw_buffer_addr;
+ __le16 fw_buffer_max_len;
+ __le32 frame_data_addr;
+ __le16 frame_data_head_len;
+ __le16 fw_attr_len;
+ __le16 fw_log_len;
+ u8 pack_max_num;
+ u8 pack_compress_version;
+ __le16 stylus_struct_len;
+ __le16 mutual_struct_len;
+ __le16 self_struct_len;
+ __le16 noise_struct_len;
+ __le32 touch_data_addr;
+ __le16 touch_data_head_len;
+ __le16 point_struct_len;
+ __le16 reserved1;
+ __le16 reserved2;
+ __le32 mutual_rawdata_addr;
+ __le32 mutual_diffdata_addr;
+ __le32 mutual_refdata_addr;
+ __le32 self_rawdata_addr;
+ __le32 self_diffdata_addr;
+ __le32 self_refdata_addr;
+ __le32 iq_rawdata_addr;
+ __le32 iq_refdata_addr;
+ __le32 im_rawdata_addr;
+ __le16 im_readata_len;
+ __le32 noise_rawdata_addr;
+ __le16 noise_rawdata_len;
+ __le32 stylus_rawdata_addr;
+ __le16 stylus_rawdata_len;
+ __le32 noise_data_addr;
+ __le32 esd_addr;
+} __packed;
+
+struct goodix_berlin_touch_data {
+ u8 id;
+ u8 unused;
+ __le16 x;
+ __le16 y;
+ __le16 w;
+} __packed;
+
+struct goodix_berlin_core {
+ struct device *dev;
+ struct regmap *regmap;
+ struct regulator *avdd;
+ struct regulator *iovdd;
+ struct gpio_desc *reset_gpio;
+ struct touchscreen_properties props;
+ struct goodix_berlin_fw_version fw_version;
+ struct input_dev *input_dev;
+ int irq;
+
+ /* Runtime parameters extracted from IC_INFO buffer */
+ u32 touch_data_addr;
+};
+
+int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
+ struct regmap *regmap);
+
+extern const struct dev_pm_ops goodix_berlin_pm_ops;
+
+#endif
diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
new file mode 100644
index 000000000000..caf364b8c223
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin_core.c
@@ -0,0 +1,582 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Goodix Touchscreen Driver
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_ts_berlin driver.
+ */
+#include <asm/unaligned.h>
+#include <linux/bitfield.h>
+#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
+#include <linux/regmap.h>
+
+#include "goodix_berlin.h"
+
+/*
+ * Goodix "Berlin" Touchscreen ID driver
+ *
+ * This driver is distinct from goodix.c since hardware interface
+ * is different enough to require a new driver.
+ * None of the register address or data structure are close enough
+ * to the previous generations.
+ *
+ * Currently only handles Multitouch events with already
+ * programmed firmware and "config" for "Revision D" Berlin IC.
+ *
+ * Support is missing for:
+ * - ESD Management
+ * - Firmware update/flashing
+ * - "Config" update/flashing
+ * - Stylus Events
+ * - Gesture Events
+ * - Support for older revisions (A & B)
+ */
+
+static bool goodix_berlin_checksum_valid(const u8 *data, int size)
+{
+ u32 cal_checksum = 0;
+ u16 r_checksum;
+ u32 i;
+
+ if (size < GOODIX_BERLIN_COOR_DATA_CHECKSUM_SIZE)
+ return false;
+
+ for (i = 0; i < size - GOODIX_BERLIN_COOR_DATA_CHECKSUM_SIZE; i++)
+ cal_checksum += data[i];
+
+ r_checksum = get_unaligned_le16(&data[i]);
+
+ return FIELD_GET(GOODIX_BERLIN_COOR_DATA_CHECKSUM_MASK, cal_checksum) == r_checksum;
+}
+
+static bool goodix_berlin_is_dummy_data(struct goodix_berlin_core *cd,
+ const u8 *data, int size)
+{
+ int i;
+
+ /*
+ * If the device is missing or doesn't respond the buffer
+ * could be filled with bus default line state, 0x00 or 0xff,
+ * so declare success the first time we encounter neither.
+ */
+ for (i = 0; i < size; i++)
+ if (data[i] > 0 && data[i] < 0xff)
+ return false;
+
+ return true;
+}
+
+static int goodix_berlin_dev_confirm(struct goodix_berlin_core *cd)
+{
+ u8 tx_buf[8], rx_buf[8];
+ int retry = 3;
+ int error;
+
+ memset(tx_buf, GOODIX_BERLIN_DEV_CONFIRM_VAL, sizeof(tx_buf));
+ while (retry--) {
+ error = regmap_raw_write(cd->regmap, GOODIX_BERLIN_BOOTOPTION_ADDR, tx_buf,
+ sizeof(tx_buf));
+ if (error)
+ return error;
+
+ error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_BOOTOPTION_ADDR, rx_buf,
+ sizeof(rx_buf));
+ if (error)
+ return error;
+
+ if (!memcmp(tx_buf, rx_buf, sizeof(tx_buf)))
+ return 0;
+
+ usleep_range(5000, 5100);
+ }
+
+ dev_err(cd->dev, "device confirm failed, rx_buf: %*ph\n", 8, rx_buf);
+
+ return -EINVAL;
+}
+
+static int goodix_berlin_power_on(struct goodix_berlin_core *cd, bool on)
+{
+ int error = 0;
+
+ if (on) {
+ error = regulator_enable(cd->iovdd);
+ if (error) {
+ dev_err(cd->dev, "Failed to enable iovdd: %d\n", error);
+ return error;
+ }
+
+ /* Vendor waits 3ms for IOVDD to settle */
+ usleep_range(3000, 3100);
+
+ error = regulator_enable(cd->avdd);
+ if (error) {
+ dev_err(cd->dev, "Failed to enable avdd: %d\n", error);
+ goto err_iovdd_disable;
+ }
+
+ /* Vendor waits 15ms for IOVDD to settle */
+ usleep_range(15000, 15100);
+
+ gpiod_set_value(cd->reset_gpio, 0);
+
+ /* Vendor waits 4ms for Firmware to initialize */
+ usleep_range(4000, 4100);
+
+ error = goodix_berlin_dev_confirm(cd);
+ if (error)
+ goto err_dev_reset;
+
+ /* Vendor waits 100ms for Firmware to fully boot */
+ msleep(GOODIX_BERLIN_NORMAL_RESET_DELAY_MS);
+
+ return 0;
+ }
+
+err_dev_reset:
+ gpiod_set_value(cd->reset_gpio, 1);
+
+ regulator_disable(cd->avdd);
+
+err_iovdd_disable:
+ regulator_disable(cd->iovdd);
+
+ return error;
+}
+
+static int goodix_berlin_read_version(struct goodix_berlin_core *cd)
+{
+ u8 buf[sizeof(struct goodix_berlin_fw_version)];
+ int error;
+
+ error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_FW_VERSION_INFO_ADDR, buf, sizeof(buf));
+ if (error) {
+ dev_err(cd->dev, "error reading fw version, %d\n", error);
+ return error;
+ }
+
+ if (!goodix_berlin_checksum_valid(buf, sizeof(buf))) {
+ dev_err(cd->dev, "invalid fw version: checksum error\n");
+ return -EINVAL;
+ }
+
+ memcpy(&cd->fw_version, buf, sizeof(cd->fw_version));
+
+ return 0;
+}
+
+/* Only extract necessary data for runtime */
+static int goodix_berlin_convert_ic_info(struct goodix_berlin_core *cd,
+ const u8 *data, u16 length)
+{
+ struct goodix_berlin_ic_info_misc misc;
+ unsigned int offset = 0;
+ u8 param_num;
+
+ offset += sizeof(__le16); /* length */
+ offset += sizeof(struct goodix_berlin_ic_info_version);
+ offset += sizeof(struct goodix_berlin_ic_info_feature);
+
+ /* IC_INFO Parameters, variable width structure */
+ offset += 4 * sizeof(u8); /* drv_num, sen_num, button_num, force_num */
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* active_scan_rate_num */
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* mutual_freq_num*/
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* self_tx_freq_num */
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* self_rx_freq_num */
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* stylus_freq_num */
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset + sizeof(misc) > length)
+ goto invalid_offset;
+
+ /* goodix_berlin_ic_info_misc */
+ memcpy(&misc, &data[offset], sizeof(misc));
+
+ cd->touch_data_addr = le32_to_cpu(misc.touch_data_addr);
+
+ return 0;
+
+invalid_offset:
+ dev_err(cd->dev, "ic_info length is invalid (offset %d length %d)\n",
+ offset, length);
+ return -EINVAL;
+}
+
+static int goodix_berlin_get_ic_info(struct goodix_berlin_core *cd)
+{
+ u8 afe_data[GOODIX_BERLIN_IC_INFO_MAX_LEN];
+ __le16 length_raw;
+ u16 length;
+ int error;
+
+ error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_IC_INFO_ADDR,
+ &length_raw, sizeof(length_raw));
+ if (error) {
+ dev_info(cd->dev, "failed get ic info length, %d\n", error);
+ return error;
+ }
+
+ length = le16_to_cpu(length_raw);
+ if (length >= GOODIX_BERLIN_IC_INFO_MAX_LEN) {
+ dev_info(cd->dev, "invalid ic info length %d\n", length);
+ return -EINVAL;
+ }
+
+ error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_IC_INFO_ADDR,
+ afe_data, length);
+ if (error) {
+ dev_info(cd->dev, "failed get ic info data, %d\n", error);
+ return error;
+ }
+
+ /* check whether the data is valid (ex. bus default values) */
+ if (goodix_berlin_is_dummy_data(cd, (const uint8_t *)afe_data, length)) {
+ dev_err(cd->dev, "fw info data invalid\n");
+ return -EINVAL;
+ }
+
+ if (!goodix_berlin_checksum_valid((const uint8_t *)afe_data, length)) {
+ dev_info(cd->dev, "fw info checksum error\n");
+ return -EINVAL;
+ }
+
+ error = goodix_berlin_convert_ic_info(cd, afe_data, length);
+ if (error)
+ return error;
+
+ /* check some key info */
+ if (!cd->touch_data_addr) {
+ dev_err(cd->dev, "touch_data_addr is null\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static void goodix_berlin_parse_finger(struct goodix_berlin_core *cd,
+ const void *buf, int touch_num)
+{
+ const struct goodix_berlin_touch_data *touch_data = buf;
+ int i;
+
+ /* Check for data validity */
+ for (i = 0; i < touch_num; i++) {
+ unsigned int id;
+
+ id = FIELD_GET(GOODIX_BERLIN_TOUCH_ID_MASK, touch_data[i].id);
+
+ if (id >= GOODIX_BERLIN_MAX_TOUCH) {
+ dev_warn(cd->dev, "invalid finger id %d\n", id);
+ return;
+ }
+ }
+
+ /* Report finger touches */
+ for (i = 0; i < touch_num; i++) {
+ input_mt_slot(cd->input_dev,
+ FIELD_GET(GOODIX_BERLIN_TOUCH_ID_MASK,
+ touch_data[i].id));
+ input_mt_report_slot_state(cd->input_dev, MT_TOOL_FINGER, true);
+
+ touchscreen_report_pos(cd->input_dev, &cd->props,
+ __le16_to_cpu(touch_data[i].x),
+ __le16_to_cpu(touch_data[i].y),
+ true);
+ input_report_abs(cd->input_dev, ABS_MT_TOUCH_MAJOR,
+ __le16_to_cpu(touch_data[i].w));
+ }
+
+ input_mt_sync_frame(cd->input_dev);
+ input_sync(cd->input_dev);
+}
+
+static void goodix_berlin_touch_handler(struct goodix_berlin_core *cd,
+ const void *pre_buf, u32 pre_buf_len)
+{
+ static u8 buffer[GOODIX_BERLIN_IRQ_READ_LEN(GOODIX_BERLIN_MAX_TOUCH)];
+ u8 point_type, touch_num;
+ int error;
+
+ /* copy pre-data to buffer */
+ memcpy(buffer, pre_buf, pre_buf_len);
+
+ touch_num = FIELD_GET(GOODIX_BERLIN_TOUCH_COUNT_MASK,
+ buffer[GOODIX_BERLIN_REQUEST_TYPE_OFFSET]);
+
+ if (touch_num > GOODIX_BERLIN_MAX_TOUCH) {
+ dev_warn(cd->dev, "invalid touch num %d\n", touch_num);
+ return;
+ }
+
+ if (touch_num) {
+ /* read more data if more than 2 touch events */
+ if (unlikely(touch_num > 2)) {
+ error = regmap_raw_read(cd->regmap,
+ cd->touch_data_addr + pre_buf_len,
+ &buffer[pre_buf_len],
+ (touch_num - 2) * GOODIX_BERLIN_BYTES_PER_POINT);
+ if (error) {
+ dev_err_ratelimited(cd->dev, "failed to get touch data, %d\n",
+ error);
+ return;
+ }
+ }
+
+ point_type = FIELD_GET(GOODIX_BERLIN_POINT_TYPE_MASK,
+ buffer[GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN]);
+
+ if (point_type == GOODIX_BERLIN_POINT_TYPE_STYLUS ||
+ point_type == GOODIX_BERLIN_POINT_TYPE_STYLUS_HOVER) {
+ dev_warn_once(cd->dev, "Stylus event type not handled\n");
+ return;
+ }
+
+ if (!goodix_berlin_checksum_valid(&buffer[GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN],
+ touch_num * GOODIX_BERLIN_BYTES_PER_POINT + 2)) {
+ dev_err(cd->dev, "touch data checksum error, data: %*ph\n",
+ touch_num * GOODIX_BERLIN_BYTES_PER_POINT + 2,
+ &buffer[GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN]);
+ return;
+ }
+ }
+
+ goodix_berlin_parse_finger(cd, &buffer[GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN],
+ touch_num);
+}
+
+static int goodix_berlin_request_handle_reset(struct goodix_berlin_core *cd)
+{
+ gpiod_set_value(cd->reset_gpio, 1);
+ usleep_range(2000, 2100);
+ gpiod_set_value(cd->reset_gpio, 0);
+
+ msleep(GOODIX_BERLIN_NORMAL_RESET_DELAY_MS);
+
+ return 0;
+}
+
+static irqreturn_t goodix_berlin_threadirq_func(int irq, void *data)
+{
+ struct goodix_berlin_core *cd = data;
+ u8 buf[GOODIX_BERLIN_IRQ_READ_LEN(2)];
+ u8 event_status;
+ int error;
+
+ /* First, read buffer with space for 2 touch events */
+ error = regmap_raw_read(cd->regmap, cd->touch_data_addr, buf,
+ GOODIX_BERLIN_IRQ_READ_LEN(2));
+ if (error) {
+ dev_err_ratelimited(cd->dev, "failed get event head data, %d\n", error);
+ return IRQ_HANDLED;
+ }
+
+ if (buf[GOODIX_BERLIN_STATUS_OFFSET] == 0)
+ return IRQ_HANDLED;
+
+ if (!goodix_berlin_checksum_valid(buf, GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN)) {
+ dev_warn_ratelimited(cd->dev, "touch head checksum err : %*ph\n",
+ GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN, buf);
+ return IRQ_HANDLED;
+ }
+
+ event_status = buf[GOODIX_BERLIN_STATUS_OFFSET];
+
+ if (event_status & GOODIX_BERLIN_TOUCH_EVENT)
+ goodix_berlin_touch_handler(cd, buf, GOODIX_BERLIN_IRQ_READ_LEN(2));
+
+ if (event_status & GOODIX_BERLIN_REQUEST_EVENT) {
+ switch (buf[GOODIX_BERLIN_REQUEST_TYPE_OFFSET]) {
+ case GOODIX_BERLIN_REQUEST_CODE_RESET:
+ goodix_berlin_request_handle_reset(cd);
+ break;
+
+ default:
+ dev_warn(cd->dev, "unsupported request code 0x%x\n",
+ buf[GOODIX_BERLIN_REQUEST_TYPE_OFFSET]);
+ }
+ }
+
+ /* Clear up status field */
+ regmap_write(cd->regmap, cd->touch_data_addr, 0);
+
+ return IRQ_HANDLED;
+}
+
+static int goodix_berlin_input_dev_config(struct goodix_berlin_core *cd,
+ const struct input_id *id)
+{
+ struct input_dev *input_dev;
+ int error;
+
+ input_dev = devm_input_allocate_device(cd->dev);
+ if (!input_dev)
+ return -ENOMEM;
+
+ cd->input_dev = input_dev;
+ input_set_drvdata(input_dev, cd);
+
+ input_dev->name = "Goodix Berlin Capacitive TouchScreen";
+ input_dev->phys = "input/ts";
+
+ input_dev->id = *id;
+
+ input_set_abs_params(cd->input_dev, ABS_MT_POSITION_X, 0, SZ_64K - 1, 0, 0);
+ input_set_abs_params(cd->input_dev, ABS_MT_POSITION_Y, 0, SZ_64K - 1, 0, 0);
+ input_set_abs_params(cd->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+
+ touchscreen_parse_properties(cd->input_dev, true, &cd->props);
+
+ error = input_mt_init_slots(cd->input_dev, GOODIX_BERLIN_MAX_TOUCH,
+ INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
+ if (error)
+ return error;
+
+ return input_register_device(cd->input_dev);
+}
+
+static int goodix_berlin_pm_suspend(struct device *dev)
+{
+ struct goodix_berlin_core *cd = dev_get_drvdata(dev);
+
+ disable_irq(cd->irq);
+
+ return goodix_berlin_power_on(cd, false);
+}
+
+static int goodix_berlin_pm_resume(struct device *dev)
+{
+ struct goodix_berlin_core *cd = dev_get_drvdata(dev);
+ int error;
+
+ error = goodix_berlin_power_on(cd, true);
+ if (error)
+ return error;
+
+ enable_irq(cd->irq);
+
+ return 0;
+}
+
+EXPORT_GPL_SIMPLE_DEV_PM_OPS(goodix_berlin_pm_ops,
+ goodix_berlin_pm_suspend,
+ goodix_berlin_pm_resume);
+
+static void goodix_berlin_power_off(void *data)
+{
+ struct goodix_berlin_core *cd = data;
+
+ goodix_berlin_power_on(cd, false);
+}
+
+int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
+ struct regmap *regmap)
+{
+ struct goodix_berlin_core *cd;
+ int error;
+
+ if (irq <= 0) {
+ dev_err(dev, "Missing interrupt number\n");
+ return -EINVAL;
+ }
+
+ cd = devm_kzalloc(dev, sizeof(*cd), GFP_KERNEL);
+ if (!cd)
+ return -ENOMEM;
+
+ cd->dev = dev;
+ cd->regmap = regmap;
+ cd->irq = irq;
+
+ cd->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(cd->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(cd->reset_gpio),
+ "Failed to request reset gpio\n");
+
+ cd->avdd = devm_regulator_get(dev, "avdd");
+ if (IS_ERR(cd->avdd))
+ return dev_err_probe(dev, PTR_ERR(cd->avdd),
+ "Failed to request avdd regulator\n");
+
+ cd->iovdd = devm_regulator_get(dev, "iovdd");
+ if (IS_ERR(cd->iovdd))
+ return dev_err_probe(dev, PTR_ERR(cd->iovdd),
+ "Failed to request iovdd regulator\n");
+
+ error = goodix_berlin_power_on(cd, true);
+ if (error) {
+ dev_err(dev, "failed power on");
+ return error;
+ }
+
+ error = devm_add_action_or_reset(dev, goodix_berlin_power_off, cd);
+ if (error)
+ return error;
+
+ error = goodix_berlin_read_version(cd);
+ if (error) {
+ dev_err(dev, "failed to get version info");
+ return error;
+ }
+
+ error = goodix_berlin_get_ic_info(cd);
+ if (error) {
+ dev_err(dev, "invalid ic info, abort");
+ return error;
+ }
+
+ error = goodix_berlin_input_dev_config(cd, id);
+ if (error) {
+ dev_err(dev, "failed set input device");
+ return error;
+ }
+
+ error = devm_request_threaded_irq(dev, irq, NULL,
+ goodix_berlin_threadirq_func,
+ IRQF_ONESHOT, "goodix-berlin", cd);
+ if (error) {
+ dev_err(dev, "request threaded irq failed: %d\n", error);
+ return error;
+ }
+
+ dev_set_drvdata(dev, cd);
+
+ dev_dbg(dev, "Goodix Berlin %s Touchscreen Controller", cd->fw_version.patch_pid);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(goodix_berlin_probe);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix Berlin Core Touchscreen driver");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
--
2.34.1
^ permalink raw reply related
* [PATCH v8 3/4] Input: goodix-berlin - add I2C support for Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-10-03 8:38 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bastien Nocera,
Hans de Goede, Henrik Rydberg, Jeff LaBundy, linux-arm-msm,
devicetree, linux-kernel, Neil Armstrong
In-Reply-To: <20231003-topic-goodix-berlin-upstream-initial-v8-0-171606102ed6@linaro.org>
Add initial support for the new Goodix "Berlin" touchscreen ICs
over the I2C interface.
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.
[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@labundy.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/input/touchscreen/Kconfig | 14 ++++++
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_berlin_i2c.c | 69 +++++++++++++++++++++++++++
3 files changed, 84 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 950da599ae1a..cc7b88118158 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -419,6 +419,20 @@ config TOUCHSCREEN_GOODIX
config TOUCHSCREEN_GOODIX_BERLIN_CORE
tristate
+config TOUCHSCREEN_GOODIX_BERLIN_I2C
+ tristate "Goodix Berlin I2C touchscreen"
+ depends on I2C
+ select REGMAP_I2C
+ select TOUCHSCREEN_GOODIX_BERLIN_CORE
+ help
+ Say Y here if you have a Goodix Berlin IC connected to
+ your system via I2C.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called goodix_berlin_i2c.
+
config TOUCHSCREEN_HIDEEP
tristate "HiDeep Touch IC"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 2e2f3e70cd2c..7ef677cf7a10 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_TOUCHSCREEN_EXC3000) += exc3000.o
obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C) += goodix_berlin_i2c.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
diff --git a/drivers/input/touchscreen/goodix_berlin_i2c.c b/drivers/input/touchscreen/goodix_berlin_i2c.c
new file mode 100644
index 000000000000..6407b2258eb1
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin_i2c.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Goodix Berlin Touchscreen Driver
+ *
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_ts_berlin driver.
+ */
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "goodix_berlin.h"
+
+#define I2C_MAX_TRANSFER_SIZE 256
+
+static const struct regmap_config goodix_berlin_i2c_regmap_conf = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .max_raw_read = I2C_MAX_TRANSFER_SIZE,
+ .max_raw_write = I2C_MAX_TRANSFER_SIZE,
+};
+
+/* vendor & product left unassigned here, should probably be updated from fw info */
+static const struct input_id goodix_berlin_i2c_input_id = {
+ .bustype = BUS_I2C,
+};
+
+static int goodix_berlin_i2c_probe(struct i2c_client *client)
+{
+ struct regmap *regmap;
+
+ regmap = devm_regmap_init_i2c(client, &goodix_berlin_i2c_regmap_conf);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ return goodix_berlin_probe(&client->dev, client->irq,
+ &goodix_berlin_i2c_input_id, regmap);
+}
+
+static const struct i2c_device_id goodix_berlin_i2c_id[] = {
+ { "gt9916", 0 },
+ { }
+};
+
+MODULE_DEVICE_TABLE(i2c, goodix_berlin_i2c_id);
+
+static const struct of_device_id goodix_berlin_i2c_of_match[] = {
+ { .compatible = "goodix,gt9916", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, goodix_berlin_i2c_of_match);
+
+static struct i2c_driver goodix_berlin_i2c_driver = {
+ .driver = {
+ .name = "goodix-berlin-i2c",
+ .of_match_table = goodix_berlin_i2c_of_match,
+ .pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
+ },
+ .probe = goodix_berlin_i2c_probe,
+ .id_table = goodix_berlin_i2c_id,
+};
+module_i2c_driver(goodix_berlin_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix Berlin I2C Touchscreen driver");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
--
2.34.1
^ permalink raw reply related
* [PATCH v8 1/4] dt-bindings: input: document Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-10-03 8:38 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bastien Nocera,
Hans de Goede, Henrik Rydberg, Jeff LaBundy, linux-arm-msm,
devicetree, linux-kernel, Neil Armstrong, Rob Herring
In-Reply-To: <20231003-topic-goodix-berlin-upstream-initial-v8-0-171606102ed6@linaro.org>
Document the Goodix GT9916 wich is part of the "Berlin" serie
of Touchscreen controllers IC from Goodix.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
.../bindings/input/touchscreen/goodix,gt9916.yaml | 95 ++++++++++++++++++++++
1 file changed, 95 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9916.yaml b/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9916.yaml
new file mode 100644
index 000000000000..d90f045ac06c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9916.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/goodix,gt9916.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Goodix Berlin series touchscreen controller
+
+description: The Goodix Berlin series of touchscreen controllers
+ be connected to either I2C or SPI buses.
+
+maintainers:
+ - Neil Armstrong <neil.armstrong@linaro.org>
+
+allOf:
+ - $ref: touchscreen.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+ enum:
+ - goodix,gt9916
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ avdd-supply:
+ description: Analog power supply regulator on AVDD pin
+
+ vddio-supply:
+ description: power supply regulator on VDDIO pin
+
+ spi-max-frequency: true
+ touchscreen-inverted-x: true
+ touchscreen-inverted-y: true
+ touchscreen-size-x: true
+ touchscreen-size-y: true
+ touchscreen-swapped-x-y: true
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - avdd-supply
+ - touchscreen-size-x
+ - touchscreen-size-y
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ touchscreen@5d {
+ compatible = "goodix,gt9916";
+ reg = <0x5d>;
+ interrupt-parent = <&gpio>;
+ interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
+ reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
+ avdd-supply = <&ts_avdd>;
+ touchscreen-size-x = <1024>;
+ touchscreen-size-y = <768>;
+ };
+ };
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ num-cs = <1>;
+ cs-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+ touchscreen@0 {
+ compatible = "goodix,gt9916";
+ reg = <0>;
+ interrupt-parent = <&gpio>;
+ interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
+ reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
+ avdd-supply = <&ts_avdd>;
+ spi-max-frequency = <1000000>;
+ touchscreen-size-x = <1024>;
+ touchscreen-size-y = <768>;
+ };
+ };
+
+...
--
2.34.1
^ permalink raw reply related
* [PATCH v8 0/4] Input: add initial support for Goodix Berlin touchscreen IC
From: Neil Armstrong @ 2023-10-03 8:38 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bastien Nocera,
Hans de Goede, Henrik Rydberg, Jeff LaBundy, linux-arm-msm,
devicetree, linux-kernel, Neil Armstrong, Rob Herring
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
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Changes in v8:
- Add missing bitfield.h include in core
- Link to v7: https://lore.kernel.org/r/20231002-topic-goodix-berlin-upstream-initial-v7-0-792fb91f5e88@linaro.org
Changes in v7:
- rebased on v6.6-rc3
- Link to v6: https://lore.kernel.org/r/20230912-topic-goodix-berlin-upstream-initial-v6-0-b4ecfa49fb9d@linaro.org
Changes in v6:
- rebased on v6.6-rc1
- changed commit message prefix to match the other Input commits
- Link to v5: https://lore.kernel.org/r/20230801-topic-goodix-berlin-upstream-initial-v5-0-079252935593@linaro.org
Changes in v5:
- rebased on next-20230801
- Link to v4: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v4-0-0947c489be17@linaro.org
Changes in v4:
- Core updates:
- drop kconfig depends, deps will be handled by _SPI and _I2C
- change power_on() error labels
- print errors on all dev_err() prints
- remove useless default variable initialization
- switch irq touch checksum error to dev_err()
- add Jeff's review tag
- I2C changes
- change REGMAP_I2C Kconfig from depends to select
- add Jeff's review tag
- SPI changes
- add select REGMAP to Kconfig
- added GOODIX_BERLIN_ prefix to defines
- switched from ret to error
- add Jeff's review tag
- Link to v3: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v3-0-f0577cead709@linaro.org
Changes in v3:
- Another guge cleanups after Jeff's review:
- appended goodix_berlin_ before all defines
- removed some unused defines
- removed retries on most of read functions, can be added back later
- added __le to ic_info structures
- reworked and simplified irq handling, dropped enum and ts_event structs
- added struct for touch data
- simplified and cleaned goodix_berlin_check_checksum & goodix_berlin_is_dummy_data
- moved touch_data_addr to the end of the main code_data
- reworked probe to get_irq last and right before setip input device
- cleaned probe by removing the "cd->dev"
- added short paragraph to justify new driver for berlin devices
- defined all offsets & masks
- Added bindings review tag
- Link to v2: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v2-0-26bc8fe1e90e@linaro.org
Changes in v2:
- Huge cleanups after Jeff's review:
- switch to error instead of ret
- drop dummy vendor/product ids
- drop unused defined/enums
- drop unused ic_info and only keep needes values
- cleanup namings and use goodix_berlin_ everywhere
- fix regulator setup
- fix default variables value when assigned afterwars
- removed indirections
- dropped debugfs
- cleaned input_dev setup
- dropped _remove()
- sync'ed i2c and spi drivers
- fixed yaml bindings
- Link to v1: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v1-0-4a0741b8aefd@linaro.org
---
Neil Armstrong (4):
dt-bindings: input: document Goodix Berlin Touchscreen IC
Input: add core support for Goodix Berlin Touchscreen IC
Input: goodix-berlin - add I2C support for Goodix Berlin Touchscreen IC
Input: goodix-berlin - add SPI support for Goodix Berlin Touchscreen IC
.../bindings/input/touchscreen/goodix,gt9916.yaml | 95 ++++
drivers/input/touchscreen/Kconfig | 31 ++
drivers/input/touchscreen/Makefile | 3 +
drivers/input/touchscreen/goodix_berlin.h | 159 ++++++
drivers/input/touchscreen/goodix_berlin_core.c | 582 +++++++++++++++++++++
drivers/input/touchscreen/goodix_berlin_i2c.c | 69 +++
drivers/input/touchscreen/goodix_berlin_spi.c | 173 ++++++
7 files changed, 1112 insertions(+)
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230606-topic-goodix-berlin-upstream-initial-ba97e8ec8f4c
Best regards,
--
Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply
* Re: [regression] Resume broken on T14s Gen1 (AMD) due to "Input: psmouse - add delay when deactivating for SMBus mode"
From: Thorsten Leemhuis @ 2023-10-03 7:30 UTC (permalink / raw)
To: Jeffery Miller
Cc: Andrew Duggan, Dmitry Torokhov, Benjamin Tissoires, linux-input,
linux-kernel, Linux kernel regressions list
In-Reply-To: <CAAzPG9MD+UQb_RdiMkPkpQGYe-arD1nMKWngMj4P5s3_zJvphQ@mail.gmail.com>
On 02.10.23 18:52, Jeffery Miller wrote:
> On Sat, Sep 30, 2023 at 4:04 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>> """
>>> diff --git a/drivers/input/mouse/psmouse-smbus.c b/drivers/input/mouse/psmouse-smbus.c
>>> index 7b13de979908..fe12385bb856 100644
>>> --- a/drivers/input/mouse/psmouse-smbus.c
>>> +++ b/drivers/input/mouse/psmouse-smbus.c
>>> @@ -121,11 +121,11 @@ static psmouse_ret_t psmouse_smbus_process_byte(struct psmouse *psmouse)
>>>
>>> static void psmouse_activate_smbus_mode(struct psmouse_smbus_dev *smbdev)
>>> {
>>> - if (smbdev->need_deactivate) {
>>> - psmouse_deactivate(smbdev->psmouse);
>>> - /* Give the device time to switch into SMBus mode */
>>> - msleep(30);
>>> - }
>>> + if (smbdev->psmouse == NULL) {
>>> + printk("XXX: smbdev->psmouse is null\n");
>>> + } else {
>>> + printk("XXX: smbdev->psmouse is set\n");
>>> + }
>>> }
>>>
>>> static int psmouse_smbus_reconnect(struct psmouse *psmouse)
>> """
>>
>> During boot this prints "XXX: smbdev->psmouse is set". But it helped, as
>> the machine now resumes from s2idle again -- while printing "XXX:
>> smbdev->psmouse is null". And that should not be the case I assume. Or
>> did my brute force test go sideways due to my limited skills?
>
> This was a good test. You've identified where it is crashing.
>
> Maybe we could confirm that `psmouse->private` is not-NULL?:
> ```
> diff --git a/drivers/input/mouse/psmouse-smbus.c
> b/drivers/input/mouse/psmouse-smbus.c
> index 7b13de979908..432615df9ae8 100644
> --- a/drivers/input/mouse/psmouse-smbus.c
> +++ b/drivers/input/mouse/psmouse-smbus.c
> @@ -130,7 +130,10 @@ static void psmouse_activate_smbus_mode(struct
> psmouse_smbus_dev *smbdev)
>
> static int psmouse_smbus_reconnect(struct psmouse *psmouse)
> {
> - psmouse_activate_smbus_mode(psmouse->private);
> + if (psmouse->private == NULL) {
> + printk("XXX smbdev is null");
> + }
> + //psmouse_activate_smbus_mode(psmouse->private);
> return 0;
> }
> ```
This didn't print anything on resume, so `psmouse->private` apparently
is set.
Tried brute force again afterwards to find what might unset
smbdev->psmouse by adding printk statements to
psmouse_smbus_disconnect() and psmouse_smbus_cleanup() but those didn't
fire, so it must be something else I didn't spot.
Ciao, Thorsten
> On Thu, Sep 28, 2023 at 4:08 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>>
>> On 27.09.23 19:23, Thorsten Leemhuis wrote:
>>> On 27.09.23 17:55, Jeffery Miller wrote:
>>>> On Wed, Sep 27, 2023 at 10:43 AM Jeffery Miller
>>>> <jefferymiller@google.com> wrote:
>>>>> On Wed, Sep 27, 2023 at 3:54 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>>>>>>
>>>>>> My dmesg from a kernel with the revert:
>>>>>> https://www.leemhuis.info/files/misc/dmesg
>>>
>>> Thx for looking into this!
>>>
>>>>> In this dmesg output it shows that this is an elantech smbus device:
>>>>> ```
>>>>> [ 4.260415] psmouse serio1: elantech: assuming hardware version 4 (with firmware version 0x7f3001)
>>>>> [ 4.279297] psmouse serio1: elantech: Synaptics capabilities query result 0x90, 0x18, 0x0f.
>>>>> [ 4.292788] psmouse serio1: elantech: Elan sample query result 00, 80, c9
>>>>> [ 4.319184] psmouse serio1: elantech: Elan ic body: 0x10, current fw version: 0x3
>>>>> ...
>>>>> [ 4.346951] psmouse serio1: elantech: Trying to set up SMBus access
>>>>> [ 4.346986] psmouse serio1: elantech: SMbus companion is not ready yet
>>>>> [ 4.369993] input: ETPS/2 Elantech TrackPoint as /devices/platform/i8042/serio1/input/input7
>>>>> [ 4.376200] systemd[1]: bpf-lsm: LSM BPF program attached
>>>>> [ 4.385192] input: ETPS/2 Elantech Touchpad as /devices/platform/i8042/serio1/input/input5
>>>>> ```
>>>>> The change in 92e24e0e57f72e shouldn't affect the elantouch device as elantech_setup_smbus
>>>>> initializes `psmouse_smbus_init` with need_deactivate = false.
>>>
>>> Hmmm. Wondering if I should warm up the compiler again to recheck my
>>> result one more time[1].
>>
>> Just did that. Ran "make clean" and compiled mainline as of now
>> (633b47cb009d) and the machine does never resume from s2idle; then I
>> reverted 92e24e0e57f7 and compiled again (for completeness: without
>> running "make clean" beforehand) and with that kernel s2idle resume
>> works perfectly fine.
>>
>> Wondering if I or the compiler is doing something stupid here -- or if
>> we missed some small but important detail somewhere.
>>
>> Ciao, Thorsten
>>
>>>>> Did you store dmesg logs from boot without the applied patch?
>>>> I intended to ask if you have logs from a boot without 92e24e0e57f72e reverted.
>>>
>>> https://www.leemhuis.info/files/misc/dmesg-6.6-rc3-vanilla
>>>
>>>>> If the delay was being applied the timestamps should show the 30ms delay between
>>>>> `psmouse serio1: elantech: Trying to set up SMBus access`
>>>>> and
>>>>> `psmouse serio1: elantech: SMbus companion is not ready yet`
>>>
>>> Unless I missed something there is not difference. :-/
>>>
>>> Ciao, Thorsten
>>>
>>> [1] FWIW, this is my bisect log
>>>
>>> """
>>>> git bisect start
>>>> # status: waiting for both good and bad commits
>>>> # bad: [6465e260f48790807eef06b583b38ca9789b6072] Linux 6.6-rc3
>>>> git bisect bad 6465e260f48790807eef06b583b38ca9789b6072
>>>> # status: waiting for good commit(s), bad commit known
>>>> # good: [2dde18cd1d8fac735875f2e4987f11817cc0bc2c] Linux 6.5
>>>> git bisect good 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
>>>> # good: [4fb0dacb78c6a041bbd38ddd998df806af5c2c69] Merge tag 'sound-6.6-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
>>>> git bisect good 4fb0dacb78c6a041bbd38ddd998df806af5c2c69
>>>> # good: [307d59039fb26212a84a9aa6a134a7d2bdea34ca] Merge tag 'media/v6.6-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
>>>> git bisect good 307d59039fb26212a84a9aa6a134a7d2bdea34ca
>>>> # bad: [4a0fc73da97efd23a383ca839e6fe86410268f6b] Merge tag 's390-6.6-2' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
>>>> git bisect bad 4a0fc73da97efd23a383ca839e6fe86410268f6b
>>>> # good: [e4f1b8202fb59c56a3de7642d50326923670513f] Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost
>>>> git bisect good e4f1b8202fb59c56a3de7642d50326923670513f
>>>> # good: [5eea5820c7340d39e56e169e1b87199391105f6b] Merge tag 'mm-stable-2023-09-04-14-00' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>> git bisect good 5eea5820c7340d39e56e169e1b87199391105f6b
>>>> # good: [65d6e954e37872fd9afb5ef3fc0481bb3c2f20f4] Merge tag 'gfs2-v6.5-rc5-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2
>>>> git bisect good 65d6e954e37872fd9afb5ef3fc0481bb3c2f20f4
>>>> # bad: [744a759492b5c57ff24a6e8aabe47b17ad8ee964] Merge tag 'input-for-v6.6-rc0' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input
>>>> git bisect bad 744a759492b5c57ff24a6e8aabe47b17ad8ee964
>>>> # good: [dbce1a7d5dce7318d8465b1e0d052ef1d2202237] Input: Explicitly include correct DT includes
>>>> git bisect good dbce1a7d5dce7318d8465b1e0d052ef1d2202237
>>>> # good: [29057cc5bddc785ea0a11534d7ad2546fa0872d3] Merge tag 'linux-watchdog-6.6-rc1' of git://www.linux-watchdog.org/linux-watchdog
>>>> git bisect good 29057cc5bddc785ea0a11534d7ad2546fa0872d3
>>>> # bad: [3e4bb047b23375a34dbf5885709ac3729d9cfb22] Input: qt2160 - convert to use devm_* api
>>>> git bisect bad 3e4bb047b23375a34dbf5885709ac3729d9cfb22
>>>> # good: [e175eae16c1bf92062f1f431a95f476a61a77c48] Input: mcs-touchkey - convert to use devm_* api
>>>> git bisect good e175eae16c1bf92062f1f431a95f476a61a77c48
>>>> # bad: [92e24e0e57f72e06c2df87116557331fd2d4dda2] Input: psmouse - add delay when deactivating for SMBus mode
>>>> git bisect bad 92e24e0e57f72e06c2df87116557331fd2d4dda2
>>>> # good: [8362bf82fb5441613aac7c6c9dbb6b83def6ad3b] Input: mcs-touchkey - fix uninitialized use of error in mcs_touchkey_probe()
>>>> git bisect good 8362bf82fb5441613aac7c6c9dbb6b83def6ad3b
>>>> # first bad commit: [92e24e0e57f72e06c2df87116557331fd2d4dda2] Input: psmouse - add delay when deactivating for SMBus mode
>>> """
>>>
>>>
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox