* [v2 1/2] dt-bindings: HID: i2c-hid: elan: Introduce bindings for Ilitek ili2901
From: xiazhengqiao @ 2023-12-26 2:37 UTC (permalink / raw)
To: linux-input, devicetree, linux-kernel
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, dianders, xiazhengqiao
In-Reply-To: <20231226023737.25618-1-xiazhengqiao@huaqin.corp-partner.google.com>
Because ilitek, ili2901 needs to use reset to pull down the time for 10ms,
so we need to control the reset, use this drive control.
Signed-off-by: xiazhengqiao <xiazhengqiao@huaqin.corp-partner.google.com>
---
Documentation/devicetree/bindings/input/elan,ekth6915.yaml | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
index 3e2d216c6432..dc4ac41f2441 100644
--- a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
+++ b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
@@ -18,8 +18,9 @@ allOf:
properties:
compatible:
- items:
- - const: elan,ekth6915
+ enum:
+ - elan,ekth6915
+ - ilitek,ili2901
reg:
const: 0x10
--
2.17.1
^ permalink raw reply related
* [v2 2/2] HID: i2c-hid: elan: Add ili2901 timing
From: xiazhengqiao @ 2023-12-26 2:37 UTC (permalink / raw)
To: linux-input, devicetree, linux-kernel
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, dianders, xiazhengqiao
In-Reply-To: <20231226023737.25618-1-xiazhengqiao@huaqin.corp-partner.google.com>
ILI2901 requires reset to pull down time greater than 10ms,
so the configuration post_power_delay_ms is 10, and the chipset
initial time is required to be greater than 100ms,
so the post_gpio_reset_on_delay_ms is set to 100.
Signed-off-by: xiazhengqiao <xiazhengqiao@huaqin.corp-partner.google.com>
---
drivers/hid/i2c-hid/i2c-hid-of-elan.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
index 31abab57ad44..5b91fb106cfc 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
@@ -130,9 +130,17 @@ static const struct elan_i2c_hid_chip_data ilitek_ili9882t_chip_data = {
.main_supply_name = NULL,
};
+static const struct elan_i2c_hid_chip_data ilitek_ili2901_chip_data = {
+ .post_power_delay_ms = 10,
+ .post_gpio_reset_on_delay_ms = 100,
+ .hid_descriptor_address = 0x0001,
+ .main_supply_name = "vcc33",
+};
+
static const struct of_device_id elan_i2c_hid_of_match[] = {
{ .compatible = "elan,ekth6915", .data = &elan_ekth6915_chip_data },
{ .compatible = "ilitek,ili9882t", .data = &ilitek_ili9882t_chip_data },
+ { .compatible = "ilitek,ili2901", .data = &ilitek_ili2901_chip_data },
{ }
};
MODULE_DEVICE_TABLE(of, elan_i2c_hid_of_match);
--
2.17.1
^ permalink raw reply related
* Re: [v2 1/2] dt-bindings: HID: i2c-hid: elan: Introduce bindings for Ilitek ili2901
From: Krzysztof Kozlowski @ 2023-12-26 9:09 UTC (permalink / raw)
To: xiazhengqiao, linux-input, devicetree, linux-kernel
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, dianders
In-Reply-To: <20231226023737.25618-2-xiazhengqiao@huaqin.corp-partner.google.com>
On 26/12/2023 03:37, xiazhengqiao wrote:
> Because ilitek, ili2901 needs to use reset to pull down the time for 10ms,
> so we need to control the reset, use this drive control.
I don't see relation between commit msg and the patch itself. Perhaps
you wanted to say you document new device which is different than elan one?
Please use standard email subjects, so with the PATCH keyword in the
title. `git format-patch` helps here to create proper versioned patches.
Another useful tool is b4. Skipping the PATCH keyword makes filtering of
emails more difficult thus making the review process less convenient.
A nit, subject: drop second/last, redundant "bindings for". The
"dt-bindings" prefix is already stating that these are bindings.
>
> Signed-off-by: xiazhengqiao <xiazhengqiao@huaqin.corp-partner.google.com>
Your name still does not look like in other reply.
> ---
> Documentation/devicetree/bindings/input/elan,ekth6915.yaml | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
What supplies does the device have? Not the driver, the device as
written in datasheet?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] HID: lenovo: Restrict detection of patched firmware only to USB cptkbd
From: Yauhen Kharuzhy @ 2023-12-26 9:36 UTC (permalink / raw)
To: Uli v. d. Ohe
Cc: Mikhail Khvoinitsky, jkosina, benjamin.tissoires, iam,
linux-input, linux-kernel
In-Reply-To: <d13694c9-4409-4566-919b-7a577afd583c@mailbox.org>
вс, 24 дек. 2023 г. в 18:51, Uli v. d. Ohe <u-v@mailbox.org>:
>
> > So this means that the only reliable way is to add a sysfs parameter.
> > I'll send a patch.
>
> Thank you for the quick action!
>
> Perhaps it would be possible to modify the firmware further in order to
> facilitate reliable detection of this modified firmware? But for now the
> solution with a sysfs parameter (and defaulting to the workaround) seems
> good.
Unfortunately no, the firmware is closed-source and was patched in
binary form AFAIR (by replacing 1-2 instructions). Moreover, it cannot
be updated in the wireless version of the keyboard (ThinkPad Compact
Keyboard II).
>
> Best regards,
> Uli
--
Yauhen Kharuzhy
^ permalink raw reply
* Re: [v2 1/2] dt-bindings: HID: i2c-hid: elan: Introduce bindings for Ilitek ili2901
From: Zhengqiao Xia @ 2023-12-26 11:44 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-input, devicetree, linux-kernel, dmitry.torokhov, robh+dt,
krzysztof.kozlowski+dt, conor+dt, jikos, benjamin.tissoires,
dianders
In-Reply-To: <b415e6d7-d69f-4fc8-8b4f-13e942859ead@linaro.org>
Hi Krzysztof，
Thanks for your patient reply.
On Tue, Dec 26, 2023 at 5:09 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 26/12/2023 03:37, xiazhengqiao wrote:
> > Because ilitek, ili2901 needs to use reset to pull down the time for 10ms,
> > so we need to control the reset, use this drive control.
>
> I don't see relation between commit msg and the patch itself. Perhaps
> you wanted to say you document new device which is different than elan one?
>
Yes, I added a new touch, its timing is a little different from "elan,
ekth6915", and other control logic is similar.
I will re-commit my message.
>
> Please use standard email subjects, so with the PATCH keyword in the
> title. `git format-patch` helps here to create proper versioned patches.
> Another useful tool is b4. Skipping the PATCH keyword makes filtering of
> emails more difficult thus making the review process less convenient.
>
> A nit, subject: drop second/last, redundant "bindings for". The
> "dt-bindings" prefix is already stating that these are bindings.
>
Thanks for your guidance, I will modify it.
> >
> > Signed-off-by: xiazhengqiao <xiazhengqiao@huaqin.corp-partner.google.com>
>
> Your name still does not look like in other reply.
Do you mean there is something wrong with my name? How about changing
it to the following:
Signed-off-by: Zhengqiao Xia <xiazhengqiao@huaqin.corp-partner.google.com>
>
> > ---
> > Documentation/devicetree/bindings/input/elan,ekth6915.yaml | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
>
>
> What supplies does the device have? Not the driver, the device as
> written in datasheet?
This device only has a 3.3V power supply, I call it "vcc33".
>
> Best regards,
> Krzysztof
>
Best regards,
Zhengqiao
^ permalink raw reply
* [PATCH] HID: bpf: One function call less in call_hid_bpf_rdesc_fixup() after error detection
From: Markus Elfring @ 2023-12-26 18:24 UTC (permalink / raw)
To: bpf, linux-input, kernel-janitors, Alexei Starovoitov,
Benjamin Tissoires, David Vernet, Jiri Kosina
Cc: LKML, cocci
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 26 Dec 2023 19:13:25 +0100
The kfree() function was called in one case by the
call_hid_bpf_rdesc_fixup() function during error handling
even if the passed data structure member contained a null pointer.
This issue was detected by using the Coccinelle software.
Thus adjust jump targets.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/hid/bpf/hid_bpf_dispatch.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index d9ef45fcaeab..c84fe55be5ed 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -118,17 +118,17 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s
ctx_kern.data = kzalloc(ctx_kern.ctx.allocated_size, GFP_KERNEL);
if (!ctx_kern.data)
- goto ignore_bpf;
+ goto dup_mem;
memcpy(ctx_kern.data, rdesc, min_t(unsigned int, *size, HID_MAX_DESCRIPTOR_SIZE));
ret = hid_bpf_prog_run(hdev, HID_BPF_PROG_TYPE_RDESC_FIXUP, &ctx_kern);
if (ret < 0)
- goto ignore_bpf;
+ goto free_data;
if (ret) {
if (ret > ctx_kern.ctx.allocated_size)
- goto ignore_bpf;
+ goto free_data;
*size = ret;
}
@@ -137,8 +137,9 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s
return rdesc;
- ignore_bpf:
+free_data:
kfree(ctx_kern.data);
+dup_mem:
return kmemdup(rdesc, *size, GFP_KERNEL);
}
EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
--
2.43.0
^ permalink raw reply related
* [PATCH] Input: MT - Return directly after a failed kzalloc() in input_mt_init_slots()
From: Markus Elfring @ 2023-12-26 19:43 UTC (permalink / raw)
To: linux-input, kernel-janitors, Dmitry Torokhov, Henrik Rydberg; +Cc: LKML, cocci
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 26 Dec 2023 20:36:09 +0100
The kfree() function was called in one case by
the input_mt_init_slots() function during error handling
even if the passed variable contained a null pointer.
This issue was detected by using the Coccinelle software.
Thus return directly after a call of the function “kzalloc” failed
at the beginning.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/input/input-mt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index 14b53dac1253..24064447d600 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -49,7 +49,7 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
mt = kzalloc(struct_size(mt, slots, num_slots), GFP_KERNEL);
if (!mt)
- goto err_mem;
+ return -ENOMEM;
mt->num_slots = num_slots;
mt->flags = flags;
--
2.43.0
^ permalink raw reply related
* [PATCH] Input: usbtouchscreen - Return directly after a failed kmalloc() in nexio_init()
From: Markus Elfring @ 2023-12-26 20:08 UTC (permalink / raw)
To: linux-input, kernel-janitors, Dmitry Torokhov, Oliver Graute,
Uwe Kleine-König, ye xingchen
Cc: LKML, cocci
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 26 Dec 2023 21:00:25 +0100
The kfree() function was called in one case by
the nexio_init() function during error handling
even if the passed variable contained a null pointer.
This issue was detected by using the Coccinelle software.
Thus return directly after a call of the function “kmalloc” failed
at the beginning.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/input/touchscreen/usbtouchscreen.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
index 60354ebc7242..1873c7918a78 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -977,7 +977,7 @@ static int nexio_init(struct usbtouch_usb *usbtouch)
buf = kmalloc(NEXIO_BUFSIZE, GFP_NOIO);
if (!buf)
- goto out_buf;
+ return ret;
/* two empty reads */
for (i = 0; i < 2; i++) {
--
2.43.0
^ permalink raw reply related
* Re: [v1 2/2] HID: i2c-hid: elan: Add ili2901 timing
From: Linus Walleij @ 2023-12-26 21:22 UTC (permalink / raw)
To: xiazhengqiao
Cc: linux-input, devicetree, linux-kernel, dmitry.torokhov, robh+dt,
jikos, benjamin.tissoires, dianders
In-Reply-To: <20231225092843.5993-3-xiazhengqiao@huaqin.corp-partner.google.com>
On Mon, Dec 25, 2023 at 10:29 AM xiazhengqiao
<xiazhengqiao@huaqin.corp-partner.google.com> wrote:
> ILI2901 requires reset to pull down time greater than 10ms,
> so the configuration post_power_delay_ms is 10, and the chipset
> initial time is required to be greater than 100ms,
> so the post_gpio_reset_on_delay_ms is set to 100.
>
> Signed-off-by: xiazhengqiao <xiazhengqiao@huaqin.corp-partner.google.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] HID: bpf: One function call less in call_hid_bpf_rdesc_fixup() after error detection
From: Hou Tao @ 2023-12-27 1:13 UTC (permalink / raw)
To: Markus Elfring, bpf, linux-input, kernel-janitors,
Alexei Starovoitov, Benjamin Tissoires, David Vernet, Jiri Kosina
Cc: LKML, cocci
In-Reply-To: <3203eb44-6e69-4bda-b585-426408cb75ee@web.de>
Hi,
On 12/27/2023 2:24 AM, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 26 Dec 2023 19:13:25 +0100
>
> The kfree() function was called in one case by the
> call_hid_bpf_rdesc_fixup() function during error handling
> even if the passed data structure member contained a null pointer.
> This issue was detected by using the Coccinelle software.
It is totally OK to free a null pointer through kfree() and the ENOMEM
case is an unlikely case, so I don't think the patch is necessary.
>
> Thus adjust jump targets.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/hid/bpf/hid_bpf_dispatch.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> index d9ef45fcaeab..c84fe55be5ed 100644
> --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> @@ -118,17 +118,17 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s
>
> ctx_kern.data = kzalloc(ctx_kern.ctx.allocated_size, GFP_KERNEL);
> if (!ctx_kern.data)
> - goto ignore_bpf;
> + goto dup_mem;
>
> memcpy(ctx_kern.data, rdesc, min_t(unsigned int, *size, HID_MAX_DESCRIPTOR_SIZE));
>
> ret = hid_bpf_prog_run(hdev, HID_BPF_PROG_TYPE_RDESC_FIXUP, &ctx_kern);
> if (ret < 0)
> - goto ignore_bpf;
> + goto free_data;
>
> if (ret) {
> if (ret > ctx_kern.ctx.allocated_size)
> - goto ignore_bpf;
> + goto free_data;
>
> *size = ret;
> }
> @@ -137,8 +137,9 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s
>
> return rdesc;
>
> - ignore_bpf:
> +free_data:
> kfree(ctx_kern.data);
> +dup_mem:
> return kmemdup(rdesc, *size, GFP_KERNEL);
> }
> EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
> --
> 2.43.0
>
>
> .
^ permalink raw reply
* [PATCH v3 0/2] Add HX83102j driver for HIMAX HID touchscreen
From: Allen_Lin @ 2023-12-27 5:35 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, linux-input, devicetree, linux-kernel
Cc: Allen_Lin
Hi,
This driver implements for Himax HID touchscreen HX83102j.
Using SPI interface to receive/send HID packets.
Patchs notes as below
1. Add the Maintainer and devicetree bindings document for driver
2. Add the driver code and modify Kconfig/Makefile to support the driver
change in v2 :
- Fix kernel test robot build warnings.
change in v3 :
- Modify code according to review suggesions.
Thanks.
Allen_Lin (2):
dt-bindings: input: Add Himax HX83102J touchscreen
Input: Add Himax HX83102J touchscreen driver
.../bindings/input/himax,hx83102j.yaml | 65 +
MAINTAINERS | 7 +
drivers/hid/Kconfig | 8 +
drivers/hid/Makefile | 2 +
drivers/hid/hid-himax-83102j.c | 1096 +++++++++++++++++
drivers/hid/hid-himax-83102j.h | 202 +++
6 files changed, 1380 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/himax,hx83102j.yaml
create mode 100644 drivers/hid/hid-himax-83102j.c
create mode 100644 drivers/hid/hid-himax-83102j.h
--
2.34.1
^ permalink raw reply
* [PATCH v3 1/2] dt-bindings: input: Add Himax HX83102J touchscreen
From: Allen_Lin @ 2023-12-27 5:35 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, linux-input, devicetree, linux-kernel
Cc: Allen_Lin
In-Reply-To: <20231227053509.894642-1-allencl_lin@hotmail.com>
Add the HX83102j touchscreen device tree bindings documents.
Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
---
.../bindings/input/himax,hx83102j.yaml | 65 +++++++++++++++++++
MAINTAINERS | 6 ++
2 files changed, 71 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/himax,hx83102j.yaml
diff --git a/Documentation/devicetree/bindings/input/himax,hx83102j.yaml b/Documentation/devicetree/bindings/input/himax,hx83102j.yaml
new file mode 100644
index 000000000000..872b478c5753
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/himax,hx83102j.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/himax,hx83102j.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Himax hx83102j touchscreen
+
+maintainers:
+ - Allen Lin <allencl_lin@hotmail.com>
+
+description:
+ This Himax hx83102j touchscreen uses the spi-hid protocol.
+
+allOf:
+ - $ref: /schemas/input/touchscreen/touchscreen.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+ enum:
+ - himax,hx83102j
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ spi-cpha: true
+
+ spi-cpol: true
+
+ spi-max-frequency: true
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - reset-gpios
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ hid-himax-spi@0 {
+ compatible = "himax,hx83102j";
+ reg = <0>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&touch_int0 &touch_reset>;
+ reset-gpios = <&gpio1 8 GPIO_ACTIVE_LOW>;
+ spi-cpha;
+ spi-cpol;
+ interrupt-parent = <&gpio1>;
+ interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 012df8ccf34e..6a92e40d126d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9502,6 +9502,12 @@ L: linux-kernel@vger.kernel.org
S: Maintained
F: drivers/misc/hisi_hikey_usb.c
+HIMAX HID HX83102J TOUCHSCREEN
+M: Allen Lin <allencl_lin@hotmail.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/input/himax,hx83102j.yaml
+
HIMAX HX83112B TOUCHSCREEN SUPPORT
M: Job Noorman <job@noorman.info>
L: linux-input@vger.kernel.org
--
2.34.1
^ permalink raw reply related
* [PATCH v3 2/2] Input: Add Himax HX83102J touchscreen driver
From: Allen_Lin @ 2023-12-27 5:35 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, linux-input, devicetree, linux-kernel
Cc: Allen_Lin
In-Reply-To: <20231227053509.894642-1-allencl_lin@hotmail.com>
Add a new driver for Himax touchscreen series touchscreen controllers.
This driver supports Himax IC using the SPI interface to
acquire HID packets.
Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
---
MAINTAINERS | 1 +
drivers/hid/Kconfig | 8 +
drivers/hid/Makefile | 2 +
drivers/hid/hid-himax-83102j.c | 1096 ++++++++++++++++++++++++++++++++
drivers/hid/hid-himax-83102j.h | 202 ++++++
5 files changed, 1309 insertions(+)
create mode 100644 drivers/hid/hid-himax-83102j.c
create mode 100644 drivers/hid/hid-himax-83102j.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 6a92e40d126d..e9553edf0bf0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9507,6 +9507,7 @@ M: Allen Lin <allencl_lin@hotmail.com>
L: linux-input@vger.kernel.org
S: Maintained
F: Documentation/devicetree/bindings/input/himax,hx83102j.yaml
+F: drivers/hid/hid-himax-83102j.*
HIMAX HX83112B TOUCHSCREEN SUPPORT
M: Job Noorman <job@noorman.info>
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4ce74af79657..e9bb95c288ec 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1325,6 +1325,14 @@ config HID_KUNIT_TEST
If in doubt, say "N".
+config HID_HIMAX_HX83102J
+ tristate "Himax hx83102j touchpanel CHIPSET"
+ depends on HID
+ help
+ Say Y here if you have a Himax CHIPSET touchscreen.
+ HIMAX controllers are multi touch controllers which can
+ report 10 touches at a time.
+ If unsure, say N.
endmenu
source "drivers/hid/bpf/Kconfig"
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 8a06d0f840bc..3474a9d8c43d 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -169,3 +169,5 @@ obj-$(INTEL_ISH_FIRMWARE_DOWNLOADER) += intel-ish-hid/
obj-$(CONFIG_AMD_SFH_HID) += amd-sfh-hid/
obj-$(CONFIG_SURFACE_HID_CORE) += surface-hid/
+
+obj-$(CONFIG_HID_HIMAX_HX83102J) += hid-himax-83102j.o
diff --git a/drivers/hid/hid-himax-83102j.c b/drivers/hid/hid-himax-83102j.c
new file mode 100644
index 000000000000..414c5fb99885
--- /dev/null
+++ b/drivers/hid/hid-himax-83102j.c
@@ -0,0 +1,1096 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Himax hx83102j Driver Code for Common IC to simulate HID
+ *
+ * Copyright (C) 2023 Himax Corporation.
+ */
+
+#include "hid-himax-83102j.h"
+
+static void hx83102j_pin_reset(struct himax_ts_data *ts);
+static void himax_ts_work(struct himax_ts_data *ts);
+static int himax_resume(struct device *dev);
+static int himax_suspend(struct device *dev);
+static int himax_chip_init(struct himax_ts_data *ts);
+static bool hx83102j_sense_off(struct himax_ts_data *ts, bool check_en);
+
+static int himax_spi_read(struct himax_ts_data *ts, u8 *cmd,
+ u8 cmd_len, u8 *buf, u32 len)
+{
+ struct spi_message m;
+ int result = 0;
+ int retry;
+ int error;
+ struct spi_transfer t = {
+ .len = cmd_len + len,
+ };
+
+ t.tx_buf = ts->xfer_data;
+ t.rx_buf = ts->xfer_data;
+
+ spi_message_init(&m);
+ spi_message_add_tail(&t, &m);
+
+ for (retry = 0; retry < HIMAX_BUS_RETRY_TIMES; retry++) {
+ error = spi_sync(ts->spi, &m);
+ if (!unlikely(error))
+ break;
+ }
+
+ if (retry == HIMAX_BUS_RETRY_TIMES) {
+ dev_err(ts->dev, "SPI read error retry over %d", HIMAX_BUS_RETRY_TIMES);
+ result = -EIO;
+ goto err_retry_over;
+ } else {
+ memcpy(buf, ts->xfer_data + cmd_len, len);
+ }
+
+err_retry_over:
+ return result;
+}
+
+static int himax_spi_write(struct himax_ts_data *ts, u8 *buf,
+ u32 length)
+{
+ int status;
+ struct spi_message m;
+ struct spi_transfer t = {
+ .tx_buf = buf,
+ .len = length,
+ };
+
+ spi_message_init(&m);
+ spi_message_add_tail(&t, &m);
+
+ status = spi_sync(ts->spi, &m);
+
+ if (status == 0) {
+ status = m.status;
+ if (status == 0)
+ status = m.actual_length;
+ }
+
+ return status;
+}
+
+static int himax_bus_read(struct himax_ts_data *ts, u8 cmd,
+ u8 *buf, u32 len)
+{
+ int result = -1;
+ u8 hw_addr = 0x00;
+
+ if (len > HIMAX_BUS_R_DLEN) {
+ dev_err(ts->dev, "len[%d] is over %d", len, HIMAX_BUS_R_DLEN);
+ return result;
+ }
+
+ mutex_lock(&ts->rw_lock);
+
+ hw_addr = 0xF3;
+
+ memset(ts->xfer_data, 0, HIMAX_BUS_R_HLEN + len);
+ ts->xfer_data[0] = hw_addr;
+ ts->xfer_data[1] = cmd;
+ ts->xfer_data[2] = 0x00;
+ result = himax_spi_read(ts, ts->xfer_data, HIMAX_BUS_R_HLEN, buf, len);
+
+ mutex_unlock(&ts->rw_lock);
+
+ return result;
+}
+
+static int himax_bus_write(struct himax_ts_data *ts, u8 cmd,
+ u8 *addr, u8 *data, u32 len)
+{
+ int result = -1;
+ u8 offset = 0;
+ u32 tmp_len = len;
+ u8 hw_addr = 0x00;
+
+ if (len > HIMAX_BUS_W_DLEN) {
+ dev_err(ts->dev, "len[%d] is over %d", len, HIMAX_BUS_W_DLEN);
+ return -EFAULT;
+ }
+
+ mutex_lock(&ts->rw_lock);
+
+ hw_addr = 0xF2;
+
+ ts->xfer_data[0] = hw_addr;
+ ts->xfer_data[1] = cmd;
+ offset = HIMAX_BUS_W_HLEN;
+
+ if (addr) {
+ memcpy(ts->xfer_data + offset, addr, 4);
+ offset += 4;
+ tmp_len -= 4;
+ }
+
+ if (data)
+ memcpy(ts->xfer_data + offset, data, tmp_len);
+
+ result = himax_spi_write(ts, ts->xfer_data, len + HIMAX_BUS_W_HLEN);
+
+ mutex_unlock(&ts->rw_lock);
+
+ return (result == len + HIMAX_BUS_W_HLEN) ? 0 : -EIO;
+}
+
+static void himax_int_enable(struct himax_ts_data *ts, int enable)
+{
+ unsigned long irqflags = 0;
+ int irqnum = ts->himax_irq;
+
+ spin_lock_irqsave(&ts->irq_lock, irqflags);
+ if (enable == 1 && atomic_read(&ts->irq_state) == 0) {
+ atomic_set(&ts->irq_state, 1);
+ enable_irq(irqnum);
+ ts->irq_enabled = 1;
+ } else if (enable == 0 && atomic_read(&ts->irq_state) == 1) {
+ atomic_set(&ts->irq_state, 0);
+ disable_irq_nosync(irqnum);
+ ts->irq_enabled = 0;
+ }
+
+ spin_unlock_irqrestore(&ts->irq_lock, irqflags);
+}
+
+static void himax_ts_isr_func(struct himax_ts_data *ts)
+{
+ himax_ts_work(ts);
+}
+
+static irqreturn_t himax_ts_thread(int irq, void *ptr)
+{
+ himax_ts_isr_func((struct himax_ts_data *)ptr);
+
+ return IRQ_HANDLED;
+}
+
+static int himax_int_register_trigger(struct himax_ts_data *ts)
+{
+ int ret = 0;
+
+ if (ts->ic_data->HX_INT_IS_EDGE) {
+ ret = devm_request_threaded_irq(ts->dev, ts->himax_irq, NULL,
+ himax_ts_thread, IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ ts->dev->driver->name, ts);
+ } else {
+ ret = devm_request_threaded_irq(ts->dev, ts->himax_irq, NULL,
+ himax_ts_thread, IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+ ts->dev->driver->name, ts);
+ }
+
+ return ret;
+}
+
+static int himax_ts_register_interrupt(struct himax_ts_data *ts)
+{
+ int ret = 0;
+
+ ts->irq_enabled = 0;
+
+ if (ts->himax_irq) {
+
+ ret = himax_int_register_trigger(ts);
+
+ if (ret == 0) {
+ ts->irq_enabled = 1;
+ atomic_set(&ts->irq_state, 1);
+ } else {
+
+ dev_err(ts->dev, "request_irq failed");
+ }
+ }
+
+ return ret;
+}
+
+static void himax_mcu_burst_enable(struct himax_ts_data *ts,
+ u8 auto_add_4_byte)
+{
+ u8 tmp_data[HIMAX_REG_DATA_LEN];
+ int ret;
+
+ tmp_data[0] = HIMAX_IC_CMD_CONTI;
+
+ ret = himax_bus_write(ts, HIMAX_IC_ADR_CONTI, NULL, tmp_data, 1);
+ if (ret < 0) {
+ dev_err(ts->dev, "bus access fail!");
+ return;
+ }
+
+ tmp_data[0] = (HIMAX_IC_CMD_INCR4 | auto_add_4_byte);
+
+ ret = himax_bus_write(ts, HIMAX_IC_ADR_INCR4, NULL, tmp_data, 1);
+ if (ret < 0) {
+ dev_err(ts->dev, "bus access fail!");
+ return;
+ }
+}
+static int himax_mcu_register_read(struct himax_ts_data *ts, u32 addr,
+ u8 *buf, u32 len)
+{
+ int ret = 0;
+ union {
+ u8 byte[4];
+ u32 word;
+ } target_addr = { .word = cpu_to_le32(addr) };
+ u8 direction_switch = HIMAX_IC_CMD_AHB_ACCESS_DIRECTION_READ;
+
+ mutex_lock(&ts->reg_lock);
+
+ if (addr == HIMAX_FLASH_ADDR_SPI200_DATA)
+ himax_mcu_burst_enable(ts, 0);
+ else if (len > HIMAX_REG_DATA_LEN)
+ himax_mcu_burst_enable(ts, 1);
+ else
+ himax_mcu_burst_enable(ts, 0);
+
+ ret = himax_bus_write(ts, HIMAX_IC_ADR_AHB_ADDR_BYTE_0, target_addr.byte, NULL, 4);
+ if (ret < 0) {
+ dev_err(ts->dev, "bus access fail!");
+ goto read_end;
+ }
+
+ ret = himax_bus_write(ts, HIMAX_IC_ADR_AHB_ACCESS_DIRECTION, NULL,
+ &direction_switch, 1);
+ if (ret < 0) {
+ dev_err(ts->dev, "bus access fail!");
+ goto read_end;
+ }
+
+ ret = himax_bus_read(ts, HIMAX_IC_ADR_AHB_RDATA_BYTE_0, buf, len);
+ if (ret < 0) {
+ dev_err(ts->dev, "bus access fail!");
+ goto read_end;
+ }
+
+read_end:
+ mutex_unlock(&ts->reg_lock);
+
+ return ret;
+}
+
+static int himax_mcu_register_write(struct himax_ts_data *ts, u32 addr,
+ u8 *val, u32 len)
+{
+ int ret = 0;
+ const u32 max_trans_sz = 4 * 1024;
+ int i = 0;
+ union {
+ u8 byte[4];
+ u16 half[2];
+ u32 word;
+ } target_addr;
+ u32 temp_len = 0;
+
+ mutex_lock(&ts->reg_lock);
+ if (addr == HIMAX_FLASH_ADDR_SPI200_DATA)
+ himax_mcu_burst_enable(ts, 0);
+ else if (len > HIMAX_REG_DATA_LEN)
+ himax_mcu_burst_enable(ts, 1);
+ else
+ himax_mcu_burst_enable(ts, 0);
+
+ if (len > max_trans_sz) {
+ for (i = 0; i < len; i += max_trans_sz) {
+ if ((len - i) > max_trans_sz)
+ temp_len = max_trans_sz;
+ else
+ temp_len = len % max_trans_sz;
+
+ target_addr.word = cpu_to_le32(addr + i);
+ ret = himax_bus_write(ts, HIMAX_IC_ADR_AHB_ADDR_BYTE_0,
+ target_addr.byte, val + i, temp_len + HIMAX_REG_ADDR_LEN);
+ if (ret < 0) {
+ dev_err(ts->dev, "xfer fail!");
+ goto write_end;
+ }
+ }
+ } else {
+ target_addr.word = cpu_to_le32(addr);
+ ret = himax_bus_write(ts, HIMAX_IC_ADR_AHB_ADDR_BYTE_0, target_addr.byte, val,
+ len + HIMAX_REG_ADDR_LEN);
+ if (ret < 0) {
+ dev_err(ts->dev, "xfer fail!");
+ goto write_end;
+ }
+ }
+write_end:
+ mutex_unlock(&ts->reg_lock);
+
+ return ret;
+}
+
+
+static void himax_ap_notify_fw_sus(struct himax_ts_data *ts, int suspend)
+{
+ int retry = 0;
+ int read_sts = 0;
+ union {
+ u8 byte[4];
+ u16 half[2];
+ u32 word;
+ } rdata, data;
+
+ if (suspend)
+ data.word = HIMAX_FW_DATA_AP_NOTIFY_FW_SUS_EN;
+ else
+ data.word = HIMAX_FW_DATA_AP_NOTIFY_FW_SUS_DIS;
+
+ data.word = cpu_to_le32(data.word);
+ do {
+ himax_mcu_register_write(ts, HIMAX_FW_ADDR_AP_NOTIFY_FW_SUS, data.byte,
+ 4);
+ usleep_range(1000, 1001);
+ read_sts = himax_mcu_register_read(ts, HIMAX_FW_ADDR_AP_NOTIFY_FW_SUS, rdata.byte,
+ 4);
+ } while ((retry++ < 10) && (read_sts != 0) &&
+ (rdata.word != data.word));
+}
+
+static void himax_resume_proc(struct himax_ts_data *ts, bool suspended)
+{
+ himax_ap_notify_fw_sus(ts, 0);
+}
+
+static void himax_mcu_ic_reset(struct himax_ts_data *ts, u8 loadconfig,
+ u8 int_off)
+{
+
+ if (ts->gpiod_rst) {
+ if (int_off)
+ himax_int_enable(ts, 0);
+
+ hx83102j_pin_reset(ts);
+
+ if (int_off)
+ himax_int_enable(ts, 1);
+ }
+}
+static void himax_mcu_touch_information(struct himax_ts_data *ts)
+{
+ if (ts->ic_data->HX_RX_NUM == 0xFFFFFFFF)
+ ts->ic_data->HX_RX_NUM = 48;
+
+ if (ts->ic_data->HX_TX_NUM == 0xFFFFFFFF)
+ ts->ic_data->HX_TX_NUM = 32;
+
+ if (ts->ic_data->HX_BT_NUM == 0xFFFFFFFF)
+ ts->ic_data->HX_BT_NUM = 0;
+
+ if (ts->ic_data->HX_MAX_PT == 0xFFFFFFFF)
+ ts->ic_data->HX_MAX_PT = 10;
+
+ if (ts->ic_data->HX_INT_IS_EDGE == 0xFF)
+ ts->ic_data->HX_INT_IS_EDGE = false;
+
+ if (ts->ic_data->HX_STYLUS_FUNC == 0xFF)
+ ts->ic_data->HX_STYLUS_FUNC = 1;
+
+ if (ts->ic_data->HX_STYLUS_ID_V2 == 0xFF)
+ ts->ic_data->HX_STYLUS_ID_V2 = 0;
+
+ if (ts->ic_data->HX_STYLUS_RATIO == 0xFF)
+ ts->ic_data->HX_STYLUS_RATIO = 1;
+
+}
+static bool hx83102j_chip_detect(struct himax_ts_data *ts)
+{
+ union {
+ u8 byte[4];
+ u16 half[2];
+ u32 word;
+ } data;
+ bool ret_data = false;
+ int ret = 0;
+ int i = 0;
+ bool check_flash;
+
+ hx83102j_pin_reset(ts);
+ ret = himax_bus_read(ts, 0x13, data.byte, 1);
+ if (ret < 0) {
+ dev_err(ts->dev, "bus access fail!");
+ return false;
+ }
+
+ check_flash = false;
+
+ if (hx83102j_sense_off(ts, check_flash) == false) {
+ ret_data = false;
+ dev_err(ts->dev, "hx83102_sense_off Fail!");
+ return ret_data;
+ }
+
+ for (i = 0; i < 5; i++) {
+ ret = himax_mcu_register_read(ts, HIMAX_HX83102J_ICID_ADDR, data.byte, 4);
+ if (ret != 0) {
+ ret_data = false;
+ dev_err(ts->dev, "read ic id Fail");
+ return ret_data;
+ }
+
+ if ((data.word & 0xFFFFFF00) == HIMAX_HX83102J_ICID_DATA) {
+ strscpy(ts->chip_name,
+ HIMAX_HX83102J_ID, 30);
+ ts->ic_data->icid = data.word;
+ ret_data = true;
+ break;
+ }
+ dev_err(ts->dev, "Read driver IC ID = %X,%X,%X",
+ data.byte[3], data.byte[2], data.byte[1]);
+ ret_data = false;
+ dev_err(ts->dev, "Read driver ID register Fail!");
+ dev_err(ts->dev, "Could NOT find Himax Chipset");
+ dev_err(ts->dev, "Please check:\n1.VCCD,VCCA,VSP,VSN");
+ dev_err(ts->dev, "2. LCM_RST,TP_RST");
+ dev_err(ts->dev, "3. Power On Sequence");
+ }
+
+ return ret_data;
+}
+static bool hx83102j_sense_off(struct himax_ts_data *ts, bool check_en)
+{
+ u32 cnt = 0;
+ union {
+ u8 byte[4];
+ u16 half[2];
+ u32 word;
+ } data;
+ int ret = 0;
+
+ do {
+ data.word = cpu_to_le32(HIMAX_FW_DATA_FW_STOP);
+ if (cnt == 0 ||
+ (data.byte[0] != 0xA5 &&
+ data.byte[0] != 0x00 &&
+ data.byte[0] != 0x87))
+ himax_mcu_register_write(ts, HIMAX_FW_ADDR_CTRL_FW,
+ data.byte, 4);
+ usleep_range(10000, 10001);
+
+ himax_mcu_register_read(ts, HIMAX_IC_ADR_CS_CENTRAL_STATE,
+ data.byte, 4);
+
+ if (data.byte[0] != 0x05)
+ break;
+
+ himax_mcu_register_read(ts, HIMAX_FW_ADDR_CTRL_FW,
+ data.byte, 4);
+ } while (data.byte[0] != 0x87 && ++cnt < 35 && check_en);
+
+ cnt = 0;
+
+ do {
+ /**
+ * set Enter safe mode : 0x31 ==> 0x9527
+ */
+ data.half[0] = cpu_to_le16(HIMAX_HX83102J_SAFE_MODE_PASSWORD);
+ ret = himax_bus_write(ts, 0x31, NULL, data.byte, 2);
+ if (ret < 0) {
+ dev_err(ts->dev, "bus access fail!");
+ return false;
+ }
+
+ /**
+ *Check enter_save_mode
+ */
+ himax_mcu_register_read(ts, HIMAX_IC_ADR_CS_CENTRAL_STATE, data.byte, 4);
+
+ if (data.byte[0] == 0x0C) {
+ /**
+ *Reset TCON
+ */
+ data.word = 0;
+ himax_mcu_register_write(ts, HIMAX_HX83102J_IC_ADR_TCON_RST, data.byte, 4);
+ usleep_range(1000, 1001);
+ return true;
+ }
+ usleep_range(5000, 5001);
+ hx83102j_pin_reset(ts);
+ } while (cnt++ < 5);
+
+ return false;
+}
+
+static bool hx83102j_read_event_stack(struct himax_ts_data *ts,
+ u8 *buf, u32 length)
+{
+ int ret = 0;
+
+ ret = himax_bus_read(ts, HIMAX_FW_ADDR_EVENT_ADDR, buf, length);
+
+ return (ret == 0) ? true : false;
+}
+
+static void hx83102j_pin_reset(struct himax_ts_data *ts)
+{
+ if (ts->gpiod_rst) {
+ gpiod_set_value(ts->gpiod_rst, 1);
+ usleep_range(100 * 100, 101 * 100);
+ gpiod_set_value(ts->gpiod_rst, 0);
+ usleep_range(200 * 100, 201 * 100);
+ }
+}
+
+static int himax_touch_get(struct himax_ts_data *ts, u8 *buf, int ts_path)
+{
+ u32 read_size = 0;
+ int ts_status = 0;
+
+ switch (ts_path) {
+ case HIMAX_REPORT_COORD:
+ read_size = ts->touch_all_size;
+ break;
+ default:
+ break;
+ }
+
+ if (read_size == 0) {
+ dev_err(ts->dev, "Read size fault!");
+ ts_status = HIMAX_TS_GET_DATA_FAIL;
+ } else {
+ if (!hx83102j_read_event_stack(ts, buf, read_size)) {
+ dev_err(ts->dev, "can't read data from chip!");
+ ts_status = HIMAX_TS_GET_DATA_FAIL;
+ }
+ }
+
+ return ts_status;
+}
+
+static int himax_hid_parse(struct hid_device *hid)
+{
+ struct himax_ts_data *ts = NULL;
+ int ret;
+
+ if (!hid) {
+ dev_err(ts->dev, "hid is NULL");
+ return -EINVAL;
+ }
+
+ ts = hid->driver_data;
+ if (!ts) {
+ dev_err(ts->dev, "hid->driver_data is NULL");
+ return -EINVAL;
+ }
+
+ ret = hid_parse_report(hid, ts->hid_rd_data.rd_data,
+ ts->hid_rd_data.rd_length);
+ if (ret) {
+ dev_err(ts->dev, "failed parse report");
+ return ret;
+ }
+ return 0;
+}
+
+static int himax_hid_start(struct hid_device *hid)
+{
+ return 0;
+}
+
+static void himax_hid_stop(struct hid_device *hid)
+{
+}
+
+static int himax_hid_open(struct hid_device *hid)
+{
+ return 0;
+}
+
+static void himax_hid_close(struct hid_device *hid)
+{
+}
+
+static int himax_hid_get_raw_report(const struct hid_device *hid, unsigned char reportnum,
+ __u8 *buf, size_t len, unsigned char report_type)
+{
+ struct himax_ts_data *ts = NULL;
+ int ret = 0;
+
+ ts = hid->driver_data;
+ if (!ts) {
+ dev_err(ts->dev, "hid->driver_data is NULL");
+ return -EINVAL;
+ }
+
+
+ switch (reportnum) {
+ case ID_CONTACT_COUNT:
+ if (!ts->ic_data) {
+ dev_err(ts->dev, "ts->ic_data is NULL");
+ return -EINVAL;
+ }
+ buf[1] = ts->ic_data->HX_MAX_PT;
+ ret = len;
+ break;
+ default:
+ ret = -EINVAL;
+ };
+ return ret;
+}
+
+static int himax_raw_request(struct hid_device *hid, unsigned char reportnum,
+ __u8 *buf, size_t len, unsigned char rtype, int reqtype)
+{
+ switch (reqtype) {
+ case HID_REQ_GET_REPORT:
+ return himax_hid_get_raw_report(hid, reportnum, buf, len, rtype);
+ default:
+ return -EIO;
+ }
+
+ return -EINVAL;
+}
+
+static struct hid_ll_driver himax_hid_ll_driver = {
+ .parse = himax_hid_parse,
+ .start = himax_hid_start,
+ .stop = himax_hid_stop,
+ .open = himax_hid_open,
+ .close = himax_hid_close,
+ .raw_request = himax_raw_request
+};
+
+static int himax_hid_report(const struct himax_ts_data *ts, u8 *data, s32 len)
+{
+ int ret = 0;
+
+ if (ts->hid)
+ ret = hid_input_report(ts->hid, HID_INPUT_REPORT, data, len, 1);
+
+ return ret;
+}
+static int himax_hid_probe(struct himax_ts_data *ts)
+{
+ int ret;
+ struct hid_device *hid = NULL;
+
+ if (!ts) {
+ dev_err(ts->dev, "ts is NULL");
+ return -EINVAL;
+ }
+ hid = ts->hid;
+ if (hid) {
+ hid_destroy_device(hid);
+ hid = NULL;
+ }
+
+ hid = hid_allocate_device();
+ if (IS_ERR(hid)) {
+ ret = PTR_ERR(hid);
+ return ret;
+ }
+
+ hid->driver_data = ts;
+ hid->ll_driver = &himax_hid_ll_driver;
+ hid->bus = BUS_SPI;
+ hid->dev.parent = &ts->spi->dev;
+
+ hid->version = ts->hid_desc.bcd_version;
+ hid->vendor = ts->hid_desc.vendor_id;
+ hid->product = ts->hid_desc.product_id;
+ snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X", "hid-hxtp",
+ hid->vendor, hid->product);
+
+ ret = hid_add_device(hid);
+ if (ret) {
+ dev_err(ts->dev, "failed add hid device");
+ goto err_hid_data;
+ }
+ ts->hid = hid;
+ mutex_unlock(&ts->hid_ioctl_lock);
+ return 0;
+
+err_hid_data:
+ hid_destroy_device(hid);
+ return ret;
+}
+
+static void himax_hid_remove(struct himax_ts_data *ts)
+{
+ mutex_lock(&ts->hid_ioctl_lock);
+ if (ts && ts->hid)
+ hid_destroy_device(ts->hid);
+ else
+ goto out;
+ ts->hid = NULL;
+out:
+ mutex_unlock(&ts->hid_ioctl_lock);
+}
+
+
+static int himax_ts_operation(struct himax_ts_data *ts,
+ int ts_path)
+{
+ int ts_status = HIMAX_TS_NORMAL_END;
+ int ret = 0;
+ u32 offset = 0;
+
+ memset(ts->xfer_buff,
+ 0x00,
+ ts->touch_all_size * sizeof(u8));
+ ts_status = himax_touch_get(ts, ts->xfer_buff, ts_path);
+ if (ts_status == HIMAX_TS_GET_DATA_FAIL)
+ goto end_function;
+ if (ts->hid_probe) {
+ offset += ts->hid_desc.max_input_length;
+ if (ts->ic_data->HX_STYLUS_FUNC) {
+ ret += himax_hid_report(ts,
+ ts->xfer_buff + offset + HIMAX_HID_REPORT_HDR_SZ,
+ ts->hid_desc.max_input_length - HIMAX_HID_REPORT_HDR_SZ);
+ offset += ts->hid_desc.max_input_length;
+ }
+ }
+
+ if (ret != 0)
+ ts_status = HIMAX_TS_GET_DATA_FAIL;
+
+end_function:
+ return ts_status;
+}
+static void himax_ts_work(struct himax_ts_data *ts)
+{
+ int ts_status = HIMAX_TS_NORMAL_END;
+ int ts_path = 0;
+
+
+ ts_path = HIMAX_REPORT_COORD;
+ ts_status = himax_ts_operation(ts, ts_path);
+ if (ts_status == HIMAX_TS_GET_DATA_FAIL)
+ himax_mcu_ic_reset(ts, false, true);
+
+}
+
+static int himax_hid_rd_init(struct himax_ts_data *ts)
+{
+ int ret = 0;
+ u32 rd_sz = 0;
+
+ rd_sz = ts->hid_desc.report_desc_length;
+ if (ts->flash_ver_info.addr_hid_rd_desc != 0) {
+ if (ts->hid_rd_data.rd_data &&
+ rd_sz != ts->hid_rd_data.rd_length) {
+ kfree(ts->hid_rd_data.rd_data);
+ ts->hid_rd_data.rd_data = NULL;
+ }
+
+ if (!ts->hid_rd_data.rd_data)
+ ts->hid_rd_data.rd_data = kzalloc(rd_sz, GFP_KERNEL);
+
+ if (ts->hid_rd_data.rd_data) {
+ } else {
+ dev_err(ts->dev, "hid rd data alloc fail");
+ ret = -ENOMEM;
+ }
+ }
+
+ return ret;
+}
+
+static void himax_hid_register(struct himax_ts_data *ts)
+{
+ if (ts->hid_probe) {
+ hid_destroy_device(ts->hid);
+ ts->hid = NULL;
+ ts->hid_probe = false;
+ }
+
+ if (himax_hid_probe(ts) != 0) {
+ dev_err(ts->dev, "hid probe fail");
+ ts->hid_probe = false;
+ } else {
+ ts->hid_probe = true;
+ }
+}
+
+static int himax_hid_report_data_init(struct himax_ts_data *ts)
+{
+ int ret = 0;
+
+ ts->touch_info_size = ts->hid_desc.max_input_length;
+ if (ts->ic_data->HX_STYLUS_FUNC)
+ ts->touch_info_size += ts->hid_desc.max_input_length;
+
+ ts->touch_all_size = ts->touch_info_size;
+ return ret;
+}
+
+static void himax_hid_update(struct work_struct *work)
+{
+ struct himax_ts_data *ts = container_of(work, struct himax_ts_data,
+ work_hid_update.work);
+
+ himax_int_enable(ts, false);
+ if (himax_hid_rd_init(ts) == 0) {
+ himax_hid_register(ts);
+ if (ts->hid_probe)
+ himax_hid_report_data_init(ts);
+ }
+ himax_int_enable(ts, true);
+}
+
+static int himax_chip_suspend(struct himax_ts_data *ts)
+{
+ int ret = 0;
+
+ ts->suspended = true;
+ himax_int_enable(ts, false);
+ if (ts->gpiod_rst)
+ gpiod_set_value(ts->gpiod_rst, 1);
+ himax_hid_remove(ts);
+ return ret;
+}
+
+static int himax_chip_resume(struct himax_ts_data *ts)
+{
+ int ret = 0;
+
+ ts->suspended = false;
+ if (ts->gpiod_rst)
+ gpiod_set_value(ts->gpiod_rst, 0);
+ himax_resume_proc(ts, ts->suspended);
+ himax_hid_probe(ts);
+ himax_int_enable(ts, true);
+ return ret;
+}
+
+static int himax_suspend(struct device *dev)
+{
+ struct himax_ts_data *ts = dev_get_drvdata(dev);
+
+ if (!ts->initialized) {
+ dev_err(ts->dev, "init not ready, skip!");
+ return -ECANCELED;
+ }
+ himax_chip_suspend(ts);
+ return 0;
+}
+
+static void himax_shutdown(struct spi_device *spi)
+{
+ struct himax_ts_data *ts = spi_get_drvdata(spi);
+
+ if (!ts->initialized) {
+ dev_err(ts->dev, "init not ready, skip!");
+ return;
+ }
+
+ himax_int_enable(ts, false);
+ himax_hid_remove(ts);
+}
+
+static int himax_resume(struct device *dev)
+{
+ int ret = 0;
+ struct himax_ts_data *ts = dev_get_drvdata(dev);
+
+ if (!ts->initialized) {
+ if (himax_chip_init(ts))
+ return -ECANCELED;
+ }
+ ret = himax_chip_resume(ts);
+ if (ret < 0)
+ dev_err(ts->dev, "resume failed!");
+ return ret;
+}
+static const struct dev_pm_ops himax_hid_pm = {
+ .suspend = himax_suspend,
+ .resume = himax_resume,
+ .restore = himax_resume,
+};
+
+#if defined(CONFIG_OF)
+static const struct of_device_id himax_table[] = {
+ { .compatible = "himax,hx83102j" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, himax_table);
+#endif
+
+static int himax_chip_init(struct himax_ts_data *ts)
+{
+ himax_mcu_touch_information(ts);
+ spin_lock_init(&ts->irq_lock);
+ if (himax_ts_register_interrupt(ts)) {
+ dev_err(ts->dev, "register interrupt failed");
+ return -EIO;
+ }
+ himax_int_enable(ts, false);
+ INIT_DELAYED_WORK(&ts->work_hid_update, himax_hid_update);
+ ts->suspended = false;
+ ts->initialized = true;
+ return 0;
+
+}
+static bool himax_platform_init(struct himax_ts_data *ts,
+ struct himax_platform_data *local_pdata)
+{
+ struct himax_platform_data *pdata;
+
+ ts->xfer_buff = devm_kzalloc(ts->dev, HIMAX_FULL_STACK_SIZE, GFP_KERNEL);
+ if (!ts->xfer_buff)
+ return false;
+
+ pdata = devm_kzalloc(ts->dev, sizeof(struct himax_platform_data), GFP_KERNEL);
+ if (!pdata)
+ return false;
+
+
+ ts->ic_data = devm_kzalloc(ts->dev, sizeof(struct himax_ic_data), GFP_KERNEL);
+ if (!ts->ic_data)
+ return false;
+
+ memset(ts->ic_data, 0xFF, sizeof(struct himax_ic_data));
+ memcpy(pdata, local_pdata, sizeof(struct himax_platform_data));
+ ts->pdata = pdata;
+ pdata->ts = ts;
+ ts->gpiod_rst = pdata->gpiod_rst;
+ if (pdata->gpiod_rst)
+ gpiod_set_value(pdata->gpiod_rst, 1);
+ if (pdata->gpiod_rst)
+ gpiod_set_value(pdata->gpiod_rst, 0);
+
+ return true;
+}
+
+static int himax_spi_drv_probe(struct spi_device *spi)
+{
+ struct himax_ts_data *ts = NULL;
+ int ret = 0;
+ bool bret = false;
+ static struct himax_platform_data pdata = {0};
+
+ ts = devm_kzalloc(&spi->dev, sizeof(struct himax_ts_data), GFP_KERNEL);
+ if (!ts)
+ return -ENOMEM;
+
+ ts->dev = &spi->dev;
+ if (spi->master->flags & SPI_MASTER_HALF_DUPLEX) {
+ dev_err(ts->dev, "Full duplex not supported by host");
+ return -EIO;
+ }
+ pdata.ts = ts;
+ ts->dev = &spi->dev;
+ if (!spi->irq) {
+ dev_dbg(ts->dev, "no IRQ?\n");
+ return -EINVAL;
+ }
+ ts->himax_irq = spi->irq;
+ pdata.gpiod_rst = devm_gpiod_get(ts->dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(pdata.gpiod_rst)) {
+ dev_err(ts->dev, "gpio-rst value is not valid");
+ return -EIO;
+ }
+
+
+ ts->xfer_data = devm_kzalloc(ts->dev, HIMAX_BUS_RW_MAX_LEN, GFP_KERNEL);
+ if (!ts->xfer_data)
+ return -ENOMEM;
+
+ spi->bits_per_word = 8;
+ spi->mode = SPI_MODE_3;
+ spi->chip_select = 0;
+
+ ts->spi = spi;
+ mutex_init(&ts->rw_lock);
+ mutex_init(&ts->reg_lock);
+ mutex_init(&ts->hid_ioctl_lock);
+ dev_set_drvdata(&spi->dev, ts);
+ spi_set_drvdata(spi, ts);
+
+ ts->probe_finish = false;
+ ts->initialized = false;
+ ts->ic_boot_done = false;
+ bret = himax_platform_init(ts, &pdata);
+ if (!bret) {
+ dev_err(ts->dev, "platform init failed");
+ return -ENODEV;
+ }
+
+ bret = hx83102j_chip_detect(ts);
+ if (!bret) {
+ dev_err(ts->dev, "IC detect failed");
+ return -ENODEV;
+ }
+
+ ret = himax_chip_init(ts);
+ if (ret < 0)
+ return ret;
+ ts->probe_finish = true;
+ return ret;
+
+}
+
+
+static void himax_spi_drv_remove(struct spi_device *spi)
+{
+ struct himax_ts_data *ts = spi_get_drvdata(spi);
+
+ if (ts->probe_finish) {
+ if (ts->ic_boot_done) {
+ himax_int_enable(ts, false);
+
+ if (ts->hid_probe) {
+ himax_hid_remove(ts);
+ ts->hid_probe = false;
+ }
+
+ kfree(ts->hid_rd_data.rd_data);
+ ts->hid_rd_data.rd_data = NULL;
+
+ ts->ic_boot_done = false;
+ }
+ }
+ spi_set_drvdata(spi, NULL);
+
+}
+static struct spi_driver himax_hid_over_spi_driver = {
+ .driver = {
+ .name = "hx83102j",
+ .owner = THIS_MODULE,
+ .pm = &himax_hid_pm,
+#if defined(CONFIG_OF)
+ .of_match_table = of_match_ptr(himax_table),
+#endif
+ },
+ .probe = himax_spi_drv_probe,
+ .remove = himax_spi_drv_remove,
+ .shutdown = himax_shutdown,
+};
+static void himax_spi_drv_exit(void)
+{
+ spi_unregister_driver(&himax_hid_over_spi_driver);
+}
+
+static int himax_spi_drv_init(void)
+{
+ int ret;
+
+ ret = spi_register_driver(&himax_hid_over_spi_driver);
+ return ret;
+}
+
+static int __init himax_ic_init(void)
+{
+ int ret = 0;
+
+ ret = himax_spi_drv_init();
+ return ret;
+}
+
+static void __exit himax_ic_exit(void)
+{
+ himax_spi_drv_exit();
+}
+
+#if !defined(CONFIG_HID_HIMAX)
+module_init(himax_ic_init);
+#else
+late_initcall(himax_ic_init);
+#endif
+module_exit(himax_ic_exit);
+
+MODULE_DESCRIPTION("Himax SPI driver for HID simulator for " HIMAX_HX83102J_ID);
+MODULE_AUTHOR("Himax, Inc.");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-himax-83102j.h b/drivers/hid/hid-himax-83102j.h
new file mode 100644
index 000000000000..61e9d006f9be
--- /dev/null
+++ b/drivers/hid/hid-himax-83102j.h
@@ -0,0 +1,202 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __HX_IC_83102J_H__
+#define __HX_IC_83102J_H__
+
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/delay.h>
+#include <linux/regulator/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/acpi.h>
+#include <linux/spi/spi.h>
+#include <linux/hid.h>
+#include <linux/sizes.h>
+#include <linux/fs.h>
+#include <linux/gpio.h>
+#include <linux/types.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/proc_fs.h>
+#include <linux/version.h>
+#include <linux/firmware.h>
+#include <linux/stddef.h>
+#include <linux/power_supply.h>
+
+#define HIMAX_BUS_RETRY_TIMES 3
+// SPI bus read/write max length
+#define HIMAX_BUS_RW_MAX_LEN 0x20006
+// SPI bus read header length
+#define HIMAX_BUS_R_HLEN 3
+// SPI bus read data length, must be multiple of 4 and smaller than BUS_RW_MAX_LEN - BUS_R_HLEN
+#define HIMAX_BUS_R_DLEN ((HIMAX_BUS_RW_MAX_LEN - HIMAX_BUS_R_HLEN) - ((HIMAX_BUS_RW_MAX_LEN - HIMAX_BUS_R_HLEN) % 4))
+// SPI bus write header length
+#define HIMAX_BUS_W_HLEN 2
+// SPI bus write data length, must be multiple of 4 and smaller than BUS_RW_MAX_LEN - BUS_W_HLEN
+#define HIMAX_BUS_W_DLEN ((HIMAX_BUS_RW_MAX_LEN - HIMAX_BUS_W_HLEN) - ((HIMAX_BUS_RW_MAX_LEN - HIMAX_BUS_W_HLEN) % 4))
+
+enum HID_ID_FUNCT {
+ ID_CONTACT_COUNT = 0x03,
+};
+
+enum HID_FW_UPDATE_STATUS_CODE {
+ FWUP_ERROR_NO_ERROR = 0x77,
+ FWUP_ERROR_NO_MAIN = 0xC2,
+ FWUP_ERROR_BL_COMPLETE = 0xB1,
+ FWUP_ERROR_BL = 0xB2,
+ FWUP_ERROR_FLASH_PROGRAMMING = 0xB5,
+};
+
+
+// Register setting
+#define HIMAX_REG_DATA_LEN 4
+#define HIMAX_REG_ADDR_LEN 4
+#define HIMAX_MAX_TRANS_SZ 128
+#define HIMAX_MAX_RETRY_TIMES 5
+
+#define HIMAX_HX83102J_STACK_SIZE 128
+#define HIMAX_HX83102J_IC_ADR_TCON_RST 0x80020004
+#define HIMAX_HX83102J_SAFE_MODE_PASSWORD 0x9527
+#define HIMAX_HX83102J_ICID_ADDR 0x900000D0
+#define HIMAX_HX83102J_ICID_DATA 0x83102900
+#define HIMAX_HX83102J_MAX_RX_NUM 48
+#define HIMAX_HX83102J_MAX_TX_NUM 32
+
+#define HIMAX_IC_ADR_AHB_ADDR_BYTE_0 0x00
+#define HIMAX_IC_ADR_AHB_RDATA_BYTE_0 0x08
+#define HIMAX_IC_ADR_AHB_ACCESS_DIRECTION 0x0c
+#define HIMAX_IC_ADR_CONTI 0x13
+#define HIMAX_IC_ADR_INCR4 0x0D
+#define HIMAX_IC_CMD_AHB_ACCESS_DIRECTION_READ 0x00
+#define HIMAX_IC_CMD_CONTI 0x31
+#define HIMAX_IC_CMD_INCR4 0x10
+#define HIMAX_IC_ADR_CS_CENTRAL_STATE 0x900000A8
+
+#define HIMAX_FW_ADDR_CTRL_FW 0x9000005c
+#define HIMAX_FW_USB_DETECT_ADDR 0x10007F38
+#define HIMAX_FW_DATA_SAFE_MODE_RELEASE_PW_RESET 0x00000000
+#define HIMAX_FW_DATA_FW_STOP 0x000000A5
+#define HIMAX_FW_ADDR_AP_NOTIFY_FW_SUS 0x10007FD0
+#define HIMAX_FW_DATA_AP_NOTIFY_FW_SUS_EN 0xA55AA55A
+#define HIMAX_FW_DATA_AP_NOTIFY_FW_SUS_DIS 0x00000000
+#define HIMAX_FW_ADDR_EVENT_ADDR 0x30
+#define HIMAX_FW_FUNC_HANDSHAKING_PWD 0xA55AA55A
+
+#define HIMAX_FLASH_ADDR_CTRL_BASE 0x80000000
+#define HIMAX_FLASH_ADDR_SPI200_DATA (HIMAX_FLASH_ADDR_CTRL_BASE + 0x2c)
+
+#define HIMAX_HID_REPORT_HDR_SZ (2)
+#define HIMAX_HX83102J_ID "HX83102J"
+
+
+struct flash_version_info {
+ u32 addr_hid_rd_desc;
+};
+
+struct himax_hid_rd_data_t {
+ u8 *rd_data;
+ u32 rd_length;
+};
+union himax_dword_data_t {
+ u32 dword;
+ u8 byte[4];
+};
+
+enum hid_reg_action {
+ REG_READ = 0,
+ REG_WRITE = 1
+};
+
+enum hid_reg_types {
+ REG_TYPE_EXT_AHB,
+ REG_TYPE_EXT_SRAM,
+ REG_TYPE_EXT_TYPE = 0xFFFFFFFF
+};
+struct himax_hid_req_cfg_t {
+ u32 data_type;
+ u32 input_RD_de;
+};
+
+#define HIMAX_FULL_STACK_SIZE \
+ (HIMAX_HX83102J_STACK_SIZE +\
+ (2 + HIMAX_HX83102J_MAX_RX_NUM * HIMAX_HX83102J_MAX_TX_NUM + HIMAX_HX83102J_MAX_TX_NUM + HIMAX_HX83102J_MAX_RX_NUM)\
+ * 2)
+
+struct himax_ic_data {
+ u32 HX_RX_NUM;
+ u32 HX_TX_NUM;
+ u32 HX_BT_NUM;
+ u32 HX_MAX_PT;
+ u8 HX_INT_IS_EDGE;
+ u8 HX_STYLUS_FUNC;
+ u8 HX_STYLUS_ID_V2;
+ u8 HX_STYLUS_RATIO;
+ u32 icid;
+};
+
+enum HX_TS_PATH {
+ HIMAX_REPORT_COORD = 1,
+};
+
+enum HX_TS_STATUS {
+ HIMAX_TS_GET_DATA_FAIL = -4,
+ HIMAX_TS_NORMAL_END = 0,
+};
+
+struct himax_hid_desc_t {
+ u16 desc_length;
+ u16 bcd_version;
+ u16 report_desc_length;
+ u16 max_input_length;
+ u16 max_output_length;
+ u16 max_fragment_length;
+ u16 vendor_id;
+ u16 product_id;
+ u16 version_id;
+ u16 flags;
+ u32 reserved;
+} __packed;
+
+struct himax_ts_data {
+ bool initialized;
+ bool probe_finish;
+ bool suspended;
+ char chip_name[30];
+ bool ic_boot_done;
+ u8 *xfer_data;
+ struct himax_ic_data *ic_data;
+ int touch_all_size;
+ int touch_info_size;
+ struct flash_version_info flash_ver_info;
+ u8 irq_enabled;
+ struct gpio_desc *gpiod_rst;
+ s32 (*power)(s32 on);
+ struct device *dev;
+ struct himax_platform_data *pdata;
+ /* mutex lock for reg access */
+ struct mutex reg_lock;
+ /* mutex lock for read/write action */
+ struct mutex rw_lock;
+ /* mutex lock for hid ioctl action */
+ struct mutex hid_ioctl_lock;
+ atomic_t irq_state;
+ /* spin lock for irq */
+ spinlock_t irq_lock;
+ struct spi_device *spi;
+ s32 himax_irq;
+ u8 *xfer_buff;
+ struct hid_device *hid;
+ struct himax_hid_desc_t hid_desc;
+ struct himax_hid_rd_data_t hid_rd_data;
+ bool hid_probe;
+ struct delayed_work work_hid_update;
+};
+
+struct himax_platform_data {
+ struct himax_ts_data *ts;
+ struct gpio_desc *gpiod_rst;
+};
+
+#endif
--
2.34.1
^ permalink raw reply related
* Re: HID: bpf: One function call less in call_hid_bpf_rdesc_fixup() after error detection
From: Markus Elfring @ 2023-12-27 8:19 UTC (permalink / raw)
To: Hou Tao, bpf, linux-input, kernel-janitors, Alexei Starovoitov,
Benjamin Tissoires, David Vernet, Jiri Kosina
Cc: LKML, cocci
In-Reply-To: <618f886f-b2ff-4d50-cf74-e8478a7e8547@huaweicloud.com>
>> The kfree() function was called in one case by the
>> call_hid_bpf_rdesc_fixup() function during error handling
>> even if the passed data structure member contained a null pointer.
>> This issue was detected by using the Coccinelle software.
>
> It is totally OK to free a null pointer through kfree() and the ENOMEM
> case is an unlikely case, so I don't think the patch is necessary.
Would you ever like to avoid redundant data processing a bit more?
Regards,
Markus
^ permalink raw reply
* [PATCH v3 0/2] HID: i2c-hid: elan: Add ili2901 timing
From: Zhengqiao Xia @ 2023-12-27 8:50 UTC (permalink / raw)
To: linux-input, devicetree, linux-kernel
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, linus.walleij, dianders, xiazhengqiao
ILI2901 requires reset to pull down time greater than 10ms,
so the configuration post_power_delay_ms is 10, and the chipset
initial time is required to be greater than 100ms,
so the post_gpio_reset_on_delay_ms is set to 100.
Change in v3:
- PATCH 1/2: Modify title and commit
- PATCH 2/2: No change
- Link to v2: https://lore.kernel.org/all/20231226023737.25618-2-xiazhengqiao@huaqin.corp-partner.google.com/
Change in v2:
- PATCH 1/2: Modify compatible properties values
- PATCH 2/2: No change
- Link to v1: https://lore.kernel.org/all/20231225092843.5993-3-xiazhengqiao@huaqin.corp-partner.google.com/
xiazhengqiao (2):
dt-bindings: HID: i2c-hid: elan: Introduce Ilitek ili2901
HID: i2c-hid: elan: Add ili2901 timing
.../devicetree/bindings/input/elan,ekth6915.yaml | 5 +++--
drivers/hid/i2c-hid/i2c-hid-of-elan.c | 8 ++++++++
2 files changed, 11 insertions(+), 2 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v3 1/2] dt-bindings: HID: i2c-hid: elan: Introduce Ilitek ili2901
From: Zhengqiao Xia @ 2023-12-27 8:50 UTC (permalink / raw)
To: linux-input, devicetree, linux-kernel
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, linus.walleij, dianders, xiazhengqiao
In-Reply-To: <20231227085013.1317-1-xiazhengqiao@huaqin.corp-partner.google.com>
The Ilitek ili2901 touch screen chip same as Elan eKTH6915 controller
has a reset gpio. The difference is that they have different
post_power_delay_ms and post_gpio_reset_on_delay_ms.
Ilitek ili2901 also uses 3.3V power supply.
Signed-off-by: Zhengqiao Xia <xiazhengqiao@huaqin.corp-partner.google.com>
---
Documentation/devicetree/bindings/input/elan,ekth6915.yaml | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
index 3e2d216c6432..dc4ac41f2441 100644
--- a/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
+++ b/Documentation/devicetree/bindings/input/elan,ekth6915.yaml
@@ -18,8 +18,9 @@ allOf:
properties:
compatible:
- items:
- - const: elan,ekth6915
+ enum:
+ - elan,ekth6915
+ - ilitek,ili2901
reg:
const: 0x10
--
2.17.1
^ permalink raw reply related
* [PATCH v3 2/2] HID: i2c-hid: elan: Add ili2901 timing
From: Zhengqiao Xia @ 2023-12-27 8:50 UTC (permalink / raw)
To: linux-input, devicetree, linux-kernel
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, linus.walleij, dianders, xiazhengqiao
In-Reply-To: <20231227085013.1317-1-xiazhengqiao@huaqin.corp-partner.google.com>
ILI2901 requires reset to pull down time greater than 10ms,
so the configuration post_power_delay_ms is 10, and the chipset
initial time is required to be greater than 100ms,
so the post_gpio_reset_on_delay_ms is set to 100.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Zhengqiao Xia <xiazhengqiao@huaqin.corp-partner.google.com>
---
drivers/hid/i2c-hid/i2c-hid-of-elan.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
index 31abab57ad44..5b91fb106cfc 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
@@ -130,9 +130,17 @@ static const struct elan_i2c_hid_chip_data ilitek_ili9882t_chip_data = {
.main_supply_name = NULL,
};
+static const struct elan_i2c_hid_chip_data ilitek_ili2901_chip_data = {
+ .post_power_delay_ms = 10,
+ .post_gpio_reset_on_delay_ms = 100,
+ .hid_descriptor_address = 0x0001,
+ .main_supply_name = "vcc33",
+};
+
static const struct of_device_id elan_i2c_hid_of_match[] = {
{ .compatible = "elan,ekth6915", .data = &elan_ekth6915_chip_data },
{ .compatible = "ilitek,ili9882t", .data = &ilitek_ili9882t_chip_data },
+ { .compatible = "ilitek,ili2901", .data = &ilitek_ili2901_chip_data },
{ }
};
MODULE_DEVICE_TABLE(of, elan_i2c_hid_of_match);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v3 1/2] dt-bindings: HID: i2c-hid: elan: Introduce Ilitek ili2901
From: Krzysztof Kozlowski @ 2023-12-27 11:41 UTC (permalink / raw)
To: Zhengqiao Xia, linux-input, devicetree, linux-kernel
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, linus.walleij, dianders
In-Reply-To: <20231227085013.1317-2-xiazhengqiao@huaqin.corp-partner.google.com>
On 27/12/2023 09:50, Zhengqiao Xia wrote:
> The Ilitek ili2901 touch screen chip same as Elan eKTH6915 controller
> has a reset gpio. The difference is that they have different
> post_power_delay_ms and post_gpio_reset_on_delay_ms.
> Ilitek ili2901 also uses 3.3V power supply.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 2/5] dt-bindings: input/touchscreen: Add compatible for IST3038B
From: Karel Balej @ 2023-12-27 11:55 UTC (permalink / raw)
To: Markuss Broks
Cc: Conor Dooley, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Henrik Rydberg, linux-input, devicetree,
linux-kernel, Duje Mihanović, ~postmarketos/upstreaming,
phone-devel, Karel Balej, Conor Dooley
In-Reply-To: <20231209-casing-music-bded1c7b5475@spud>
Markuss,
On Sat Dec 9, 2023 at 11:58 AM CET, Conor Dooley wrote:
> On Sat, Dec 09, 2023 at 10:05:27AM +0100, Karel Balej wrote:
> > On Mon Dec 4, 2023 at 1:52 PM CET, Conor Dooley wrote:
> > > On Mon, Dec 04, 2023 at 02:40:44PM +0200, Markuss Broks wrote:
> > > > On 12/3/23 13:20, Conor Dooley wrote:
> > > > > On Sat, Dec 02, 2023 at 01:48:33PM +0100, Karel Balej wrote:
> > > > > > 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.
> > > > > This one is better, but would be well served by mentioning what
> > > > > specifically is different (register addresses or firmware commands?)
> > > >
> > > > I don't think anyone knows this other than Imagis itself. I would guess it's
> > > > different hardware, since register addresses are indeed different, but on
> > > > the other hand, there is a possibility that firmware on the MCU could be
> > > > responding to those commands. I suppose "... IST3038B is a hardware variant
> > > > of ... IST3038" would be more correct.
> > >
> > > Only Imagis might know the specifics, but you (plural) have made driver
> > > changes so you know what is different in terms of the programming model.
> > > I'm just asking for you to mention how the programming model varies in
> > > the commit message. Otherwise I can't know whether you should have added
> > > a fallback compatible, without going and reading your driver change. The
> > > commit message for the bindings should stand on its own merit in that
> > > regard.
> > > "Variant" alone does not suffice, as many variants of devices have a
> > > compatible programming model, be that for a subset of features or
> > > complete compatibility.
> > >
> > > > The reason why I think it could be firmware-defined is because we have a lot
> > > > of variants (30xxA, 30xxB, 30xxC, plain 30xx), and the numbers usually mean
> > > > feature level/completeness, e.g. some don't support the touch pressure or
> > > > touchkeys, and we don't know what A/B/C/none means.
> > >
> > > Ultimately whether it is due to firmware or the hardware isn't
> > > particular important, just mention what is incompatibly different.
> >
> > I propose to update the commit description as such:
> >
> > Imagis IST3038B is a variant (firmware?) of Imagis IST3038 IC
> > differing from IST3038C in its register interface. Add the
> > compatible for it to the IST3038C bindings.
is this change OK with you?
>
>
> SGTM. You can add
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> with that commit message update.
>
> Thanks,
> Conor.
Kind regards,
K. B.
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: HID: i2c-hid: elan: Introduce Ilitek ili2901
From: Jiri Kosina @ 2023-12-27 14:36 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Zhengqiao Xia, linux-input, devicetree, linux-kernel,
dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
benjamin.tissoires, linus.walleij, dianders
In-Reply-To: <e304aec7-5835-4f4f-89cb-bc3e1dfb78d4@linaro.org>
On Wed, 27 Dec 2023, Krzysztof Kozlowski wrote:
> > The Ilitek ili2901 touch screen chip same as Elan eKTH6915 controller
> > has a reset gpio. The difference is that they have different
> > post_power_delay_ms and post_gpio_reset_on_delay_ms.
> > Ilitek ili2901 also uses 3.3V power supply.
>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Thanks. I will take it together with the HID patch through my tree, if
that's fine with you.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: HID: bpf: One function call less in call_hid_bpf_rdesc_fixup() after error detection
From: Greg KH @ 2023-12-27 15:46 UTC (permalink / raw)
To: Markus Elfring
Cc: Hou Tao, bpf, linux-input, kernel-janitors, Alexei Starovoitov,
Benjamin Tissoires, David Vernet, Jiri Kosina, LKML, cocci
In-Reply-To: <b75d66cc-a507-432a-af60-655950671b8a@web.de>
On Wed, Dec 27, 2023 at 09:19:27AM +0100, Markus Elfring wrote:
> >> The kfree() function was called in one case by the
> >> call_hid_bpf_rdesc_fixup() function during error handling
> >> even if the passed data structure member contained a null pointer.
> >> This issue was detected by using the Coccinelle software.
> >
> > It is totally OK to free a null pointer through kfree() and the ENOMEM
> > case is an unlikely case, so I don't think the patch is necessary.
>
> Would you ever like to avoid redundant data processing a bit more?
Hi,
This is the semi-friendly patch-bot of Greg Kroah-Hartman.
Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list. I strongly suggest that you not do this anymore. Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.
Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all. The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback. Please feel free to also ignore emails
from them.
thanks,
greg k-h's patch email bot
^ permalink raw reply
* Re: [PATCH RESEND v11 2/2] input: joystick: driver for Adafruit Seesaw Gamepad
From: Jeff LaBundy @ 2023-12-27 19:43 UTC (permalink / raw)
To: Anshul Dalal
Cc: linux-input, devicetree, Conor Dooley, Dmitry Torokhov,
Thomas Weißschuh, linux-kernel, Krzysztof Kozlowski,
Conor Dooley, Rob Herring, Krzysztof Kozlowski,
linux-kernel-mentees
In-Reply-To: <20231215031125.284939-2-anshulusr@gmail.com>
Hi Anshul,
Looking great so far, just a few more comments.
On Fri, Dec 15, 2023 at 08:41:23AM +0530, Anshul Dalal wrote:
> Adds a driver for a mini gamepad that communicates over i2c, the gamepad
> has bidirectional thumb stick input and six buttons.
>
> The gamepad chip utilizes the open framework from Adafruit called 'Seesaw'
> to transmit the ADC data for the joystick and digital pin state for the
> buttons. I have only implemented the functionality required to receive the
> thumb stick and button state.
>
> Steps in reading the gamepad state over i2c:
> 1. Reset the registers
> 2. Set the pin mode of the pins specified by the `BUTTON_MASK` to input
> `BUTTON_MASK`: A bit-map for the six digital pins internally
> connected to the joystick buttons.
> 3. Enable internal pullup resistors for the `BUTTON_MASK`
> 4. Bulk set the pin state HIGH for `BUTTON_MASK`
> 5. Poll the device for button and joystick state done by:
> `seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)`
>
> Product page:
> https://www.adafruit.com/product/5743
> Arduino driver:
> https://github.com/adafruit/Adafruit_Seesaw
>
> Driver tested on RPi Zero 2W
>
> Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
>
> ---
>
> Changes for v11:
> - Removed `of_match_ptr` to prevent warning:
> 'seesaw_of_table' defined but not used
> on non OF platforms
>
> Changes for v10:
> - no updates
>
> Changes for v9:
> - Added of_device_id
>
> Changes for v8:
> - Updated invalid references to `adafruit_seesaw` to `adafruit-seesaw`
>
> Changes for v7:
> adafruit-seesaw.c
> - Fixed formatting for macro definitions
> - Made SEESAW_BUTTON_MASK static const
> - Removed __be16 type for x and y fields of seesaw_data
> - Used sparse_keymap implementation instead of custom keymap
> - Used i2c_msg instead of i2c_master_send and recv in
> seesaw_register_read
> - Use temporary variable `adc_data` to store data received from ADC
> - Changed read_buf from u8[4] to __be32
> - Use usleep_range instead of msleep
> - Removed 'Reviewed-by: Thomas Weißschuh' due to large number of changes
> since last review
> Kconfig:
> - Added `select INPUT_SPARSEKMAP`
>
> Changes for v6:
> - Added TODO
> - Removed `clang-format` directives
> - Namespaced device buttons
> - Removed `char physical_path[32]` field from `struct seesaw_gamepad`
> - Added `packed` attribute to `struct seesaw_data`
> - Moved from having booleans per button to single `u32 button_state`
> - Added `seesaw_button_description` array to directly associate device
> buttons with respective keycodes
> - Added wrapper functions `seesaw_register_` around `i2c_master_` API
> - Ratelimited input error reporting with `dev_err_ratelimited`
> - Removed `of_device_id`
>
> Changes for v5:
> - Added link to the datasheet
> - Added debug log message when `seesaw_read_data` fails
>
> Changes for v4:
> - Changed `1UL << BUTTON_` to BIT(BUTTON_)
> - Removed `hardware_id` field from `struct seesaw_gamepad`
> - Removed redundant checks for the number of bytes written and received by
> `i2c_master_send` and `i2c_master_recv`
> - Used `get_unaligned_be32` to instantiate `u32 result` from `read_buf`
> - Changed `result & (1UL << BUTTON_)` to
> `test_bit(BUTTON_, (long *)&result)`
> - Changed `KBUILD_MODNAME` in id-tables to `SEESAW_DEVICE_NAME`
> - Fixed formatting issues
> - Changed button reporting:
> Since the gamepad had the action buttons in a non-standard layout:
> (X)
> (Y) (A)
> (B)
> Therefore moved to using generic directional action button event codes
> instead of BTN_[ABXY].
>
> Changes for v3:
> - no updates
>
> Changes for v2:
> adafruit-seesaw.c:
> - Renamed file from 'adafruit_seesaw.c'
> - Changed device name from 'seesaw_gamepad' to 'seesaw-gamepad'
> - Changed count parameter for receiving joystick x on line 118:
> `2` to `sizeof(write_buf)`
> - Fixed invalid buffer size on line 123 and 126:
> `data->y` to `sizeof(data->y)`
> - Added comment for the `mdelay(10)` on line 169
> - Changed inconsistent indentation on line 271
> Kconfig:
> - Fixed indentation for the help text
> - Updated module name
> Makefile:
> - Updated module object file name
> MAINTAINERS:
> - Updated file name for the driver and bindings
>
> Previous versions:
> v11: https://lore.kernel.org/lkml/20231127161158.1651716-2-anshulusr@gmail.com/
> v10: https://lore.kernel.org/lkml/20231121123409.2231115-2-anshulusr@gmail.com/
> v9: https://lore.kernel.org/lkml/20231121101751.2189965-2-anshulusr@gmail.com/
> v8: https://lore.kernel.org/lkml/20231108005337.45069-2-anshulusr@gmail.com/
> v7: https://lore.kernel.org/lkml/20231106164134.114668-2-anshulusr@gmail.com/
> v6: https://lore.kernel.org/lkml/20231027051819.81333-2-anshulusr@gmail.com/
> v5: https://lore.kernel.org/lkml/20231017034356.1436677-2-anshulusr@gmail.com/
> v4: https://lore.kernel.org/lkml/20231010184827.1213507-2-anshulusr@gmail.com/
> v3: https://lore.kernel.org/linux-input/20231008185709.2448423-2-anshulusr@gmail.com/
> v2: https://lore.kernel.org/linux-input/20231008172435.2391009-2-anshulusr@gmail.com/
> v1: https://lore.kernel.org/linux-input/20231007144052.1535417-2-anshulusr@gmail.com/
> ---
> MAINTAINERS | 7 +
> drivers/input/joystick/Kconfig | 10 +
> drivers/input/joystick/Makefile | 1 +
> drivers/input/joystick/adafruit-seesaw.c | 318 +++++++++++++++++++++++
> 4 files changed, 336 insertions(+)
> create mode 100644 drivers/input/joystick/adafruit-seesaw.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 81d5fc0bba68..b3f101edc24b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -441,6 +441,13 @@ W: http://wiki.analog.com/AD7879
> W: https://ez.analog.com/linux-software-drivers
> F: drivers/input/touchscreen/ad7879.c
>
> +ADAFRUIT MINI I2C GAMEPAD
> +M: Anshul Dalal <anshulusr@gmail.com>
> +L: linux-input@vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
> +F: drivers/input/joystick/adafruit-seesaw.c
> +
> ADDRESS SPACE LAYOUT RANDOMIZATION (ASLR)
> M: Jiri Kosina <jikos@kernel.org>
> S: Maintained
> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
> index ac6925ce8366..7755e5b454d2 100644
> --- a/drivers/input/joystick/Kconfig
> +++ b/drivers/input/joystick/Kconfig
> @@ -412,4 +412,14 @@ config JOYSTICK_SENSEHAT
> To compile this driver as a module, choose M here: the
> module will be called sensehat_joystick.
>
> +config JOYSTICK_SEESAW
> + tristate "Adafruit Mini I2C Gamepad with Seesaw"
> + depends on I2C
> + select INPUT_SPARSEKMAP
> + help
> + Say Y here if you want to use the Adafruit Mini I2C Gamepad.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called adafruit-seesaw.
> +
> endif
> diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
> index 3937535f0098..9976f596a920 100644
> --- a/drivers/input/joystick/Makefile
> +++ b/drivers/input/joystick/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_JOYSTICK_N64) += n64joy.o
> obj-$(CONFIG_JOYSTICK_PSXPAD_SPI) += psxpad-spi.o
> obj-$(CONFIG_JOYSTICK_PXRC) += pxrc.o
> obj-$(CONFIG_JOYSTICK_QWIIC) += qwiic-joystick.o
> +obj-$(CONFIG_JOYSTICK_SEESAW) += adafruit-seesaw.o
> obj-$(CONFIG_JOYSTICK_SENSEHAT) += sensehat-joystick.o
> obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
> obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
> diff --git a/drivers/input/joystick/adafruit-seesaw.c b/drivers/input/joystick/adafruit-seesaw.c
> new file mode 100644
> index 000000000000..0f4a54c9519e
> --- /dev/null
> +++ b/drivers/input/joystick/adafruit-seesaw.c
> @@ -0,0 +1,318 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2023 Anshul Dalal <anshulusr@gmail.com>
> + *
> + * Driver for Adafruit Mini I2C Gamepad
> + *
> + * Based on the work of:
> + * Oleh Kravchenko (Sparkfun Qwiic Joystick driver)
> + *
> + * Datasheet: https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf
> + * Product page: https://www.adafruit.com/product/5743
> + * Firmware and hardware sources: https://github.com/adafruit/Adafruit_Seesaw
> + *
> + * TODO:
> + * - Add interrupt support
> + */
> +
> +#include <asm/unaligned.h>
> +#include <linux/bits.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/input/sparse-keymap.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +
> +#define SEESAW_DEVICE_NAME "seesaw-gamepad"
> +
> +#define SEESAW_STATUS_BASE 0x00
> +#define SEESAW_GPIO_BASE 0x01
> +#define SEESAW_ADC_BASE 0x09
> +
> +#define SEESAW_GPIO_DIRCLR_BULK 0x03
> +#define SEESAW_GPIO_BULK 0x04
> +#define SEESAW_GPIO_BULK_SET 0x05
> +#define SEESAW_GPIO_PULLENSET 0x0b
> +
> +#define SEESAW_STATUS_HW_ID 0x01
> +#define SEESAW_STATUS_SWRST 0x7f
> +
> +#define SEESAW_ADC_OFFSET 0x07
> +
> +#define SEESAW_BUTTON_A 0x05
> +#define SEESAW_BUTTON_B 0x01
> +#define SEESAW_BUTTON_X 0x06
> +#define SEESAW_BUTTON_Y 0x02
> +#define SEESAW_BUTTON_START 0x10
> +#define SEESAW_BUTTON_SELECT 0x00
> +
> +#define SEESAW_ANALOG_X 0x0e
> +#define SEESAW_ANALOG_Y 0x0f
> +
> +#define SEESAW_JOYSTICK_MAX_AXIS 1023
> +#define SEESAW_JOYSTICK_FUZZ 2
> +#define SEESAW_JOYSTICK_FLAT 4
> +
> +#define SEESAW_GAMEPAD_POLL_INTERVAL 16
> +#define SEESAW_GAMEPAD_POLL_MIN 8
> +#define SEESAW_GAMEPAD_POLL_MAX 32
Normally we want to include units in #defines that represent actual units,
e.g. SEESAW_GAMEPAD_POLL_INTERVAL_MS.
> +
> +#define MSEC_PER_USEC 1000
I think you meant USEC_PER_MSEC here, which is already defined. But I think
you should just drop it entirely; more on that below.
> +
> +static const u32 SEESAW_BUTTON_MASK =
> + BIT(SEESAW_BUTTON_A) | BIT(SEESAW_BUTTON_B) | BIT(SEESAW_BUTTON_X) |
> + BIT(SEESAW_BUTTON_Y) | BIT(SEESAW_BUTTON_START) |
> + BIT(SEESAW_BUTTON_SELECT);
> +
> +struct seesaw_gamepad {
> + struct input_dev *input_dev;
> + struct i2c_client *i2c_client;
> +};
> +
> +struct seesaw_data {
> + u16 x;
> + u16 y;
> + u32 button_state;
> +};
> +
> +static const struct key_entry seesaw_buttons_new[] = {
> + { KE_KEY, SEESAW_BUTTON_A, .keycode = BTN_SOUTH },
> + { KE_KEY, SEESAW_BUTTON_B, .keycode = BTN_EAST },
> + { KE_KEY, SEESAW_BUTTON_X, .keycode = BTN_NORTH },
> + { KE_KEY, SEESAW_BUTTON_Y, .keycode = BTN_WEST },
> + { KE_KEY, SEESAW_BUTTON_START, .keycode = BTN_START },
> + { KE_KEY, SEESAW_BUTTON_SELECT, .keycode = BTN_SELECT },
> + { KE_END, 0 }
> +};
> +
> +static int seesaw_register_read(struct i2c_client *client, u8 register_high,
> + u8 register_low, char *buf, int count)
> +{
> + int ret;
> + u8 register_buf[2] = { register_high, register_low };
> +
This method of passing "high" and "low" 8-bit registers is unnatural. It
seems this device effectively has a 16-bit register map, so this function
should simply accept a u16 for the register address, and then declare the
following:
__be16 register_buf = cpu_to_be16(register);
Finally, just #define all the register addresses to fit this model instead
of splitting them into a base and offset as the Arduino code appears to do.
For example, the GPIO registers would look like the following:
#define SEESAW_GPIO_BULK 0x0104
#define SEESAW_GPIO_BULK_SET 0x0105
#define SEESAW_GPIO_PULLENSET 0x010b
It seems the ADC and STATUS groups can follow suit as well.
> + struct i2c_msg message_buf[2] = {
> + {
> + .addr = client->addr,
> + .flags = client->flags,
> + .len = sizeof(register_buf),
> + .buf = register_buf,
> + },
> + {
> + .addr = client->addr,
> + .flags = client->flags | I2C_M_RD,
> + .len = count,
> + .buf = buf,
You shouldn't burden all callers of seesaw_register_read() with having to cast
their data values to (char *) outside of this function; it defeats the purpose
of having a helper. Please redefine buf as a *void, then do the following here:
.buf = (u8 *)&buf,
> + },
> + };
Please add a newline between declarations and code.
> + ret = i2c_transfer(client->adapter, message_buf,
> + ARRAY_SIZE(message_buf));
> +
Nit: extraneous newline.
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int seesaw_register_write_u8(struct i2c_client *client, u8 register_high,
> + u8 register_low, u8 value)
> +{
> + int ret;
> + u8 write_buf[3] = { register_high, register_low, value };
Same idea here. Simply accept a u16 for the register address, and then do
the following:
put_unaligned_be16(register, write_buf);
write_buf[sizeof(register)] = value;
> +
> + ret = i2c_master_send(client, write_buf, sizeof(write_buf));
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int seesaw_register_write_u32(struct i2c_client *client,
> + u8 register_high, u8 register_low,
> + u32 value)
> +{
> + int ret;
> + u8 write_buf[6] = { register_high, register_low };
> +
> + put_unaligned_be32(value, write_buf + 2);
And here:
put_unaligned_be16(register, write_buf);
put_unaligned_be32(value, sizeof(register));
> + ret = i2c_master_send(client, write_buf, sizeof(write_buf));
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)
> +{
> + int ret;
> + __be16 adc_data;
> + __be32 read_buf;
> +
> + ret = seesaw_register_read(client, SEESAW_GPIO_BASE, SEESAW_GPIO_BULK,
> + (char *)&read_buf, sizeof(read_buf));
> + if (ret)
> + return ret;
Normally in the input subsystem, we use 'error' for return values that can only
be zero (success) or a negative error code (failure). Here, we have a mix of 'ret'
and 'err'. It's fine to use 'ret' for the above calls to i2c_* API, but please
use 'error' for the rest.
> +
> + data->button_state = ~be32_to_cpu(read_buf);
> +
> + ret = seesaw_register_read(client, SEESAW_ADC_BASE,
> + SEESAW_ADC_OFFSET + SEESAW_ANALOG_X,
> + (char *)&adc_data, sizeof(adc_data));
> + if (ret)
> + return ret;
> + /*
> + * ADC reads left as max and right as 0, must be reversed since kernel
> + * expects reports in opposite order.
> + */
> + data->x = SEESAW_JOYSTICK_MAX_AXIS - be16_to_cpu(adc_data);
> +
> + ret = seesaw_register_read(client, SEESAW_ADC_BASE,
> + SEESAW_ADC_OFFSET + SEESAW_ANALOG_Y,
> + (char *)&adc_data, sizeof(adc_data));
> + if (ret)
> + return ret;
> + data->y = be16_to_cpu(adc_data);
> +
> + return 0;
> +}
> +
> +static void seesaw_poll(struct input_dev *input)
> +{
> + int err, i;
> + struct seesaw_gamepad *private = input_get_drvdata(input);
> + struct seesaw_data data;
> +
> + err = seesaw_read_data(private->i2c_client, &data);
> + if (err) {
> + dev_err_ratelimited(&input->dev,
> + "failed to read joystick state: %d\n", err);
> + return;
> + }
> +
> + input_report_abs(input, ABS_X, data.x);
> + input_report_abs(input, ABS_Y, data.y);
> +
> + for_each_set_bit(i, (long *)&SEESAW_BUTTON_MASK,
> + BITS_PER_TYPE(SEESAW_BUTTON_MASK)) {
> + if (!sparse_keymap_report_event(
> + input, i, data.button_state & BIT(i), false)) {
This line break is difficult to read; please include at least one variable
on top, like the following:
if (!sparse_keymap_report_event(input, i,
data.button_state & BIT(I),
false))
dev_err_ratelimited(...);
Note that curly braces are not required here.
> + dev_err_ratelimited(&input->dev,
> + "failed to report keymap event");
> + };
> + }
> +
> + input_sync(input);
> +}
> +
> +static int seesaw_probe(struct i2c_client *client)
> +{
> + int ret;
> + u8 hardware_id;
> + struct seesaw_gamepad *seesaw;
> +
> + ret = seesaw_register_write_u8(client, SEESAW_STATUS_BASE,
> + SEESAW_STATUS_SWRST, 0xFF);
> + if (ret)
> + return ret;
> +
> + /* Wait for the registers to reset before proceeding */
> + usleep_range(10 * MSEC_PER_USEC, 15 * MSEC_PER_USEC);
It's perfectly fine, and quite common, to simply write these out as 10000
and 15000.
> +
> + seesaw = devm_kzalloc(&client->dev, sizeof(*seesaw), GFP_KERNEL);
> + if (!seesaw)
> + return -ENOMEM;
> +
> + ret = seesaw_register_read(client, SEESAW_STATUS_BASE,
> + SEESAW_STATUS_HW_ID, &hardware_id,
> + sizeof(hardware_id));
> + if (ret)
> + return ret;
> +
> + dev_dbg(&client->dev, "Adafruit Seesaw Gamepad, Hardware ID: %02x\n",
> + hardware_id);
> +
> + /* Set Pin Mode to input and enable pull-up resistors */
> + ret = seesaw_register_write_u32(client, SEESAW_GPIO_BASE,
> + SEESAW_GPIO_DIRCLR_BULK,
> + SEESAW_BUTTON_MASK);
> + if (ret)
> + return ret;
> + ret = seesaw_register_write_u32(client, SEESAW_GPIO_BASE,
> + SEESAW_GPIO_PULLENSET,
> + SEESAW_BUTTON_MASK);
> + if (ret)
> + return ret;
> + ret = seesaw_register_write_u32(client, SEESAW_GPIO_BASE,
> + SEESAW_GPIO_BULK_SET,
> + SEESAW_BUTTON_MASK);
> + if (ret)
> + return ret;
> +
> + seesaw->i2c_client = client;
> + seesaw->input_dev = devm_input_allocate_device(&client->dev);
> + if (!seesaw->input_dev)
> + return -ENOMEM;
> +
> + seesaw->input_dev->id.bustype = BUS_I2C;
> + seesaw->input_dev->name = "Adafruit Seesaw Gamepad";
> + seesaw->input_dev->phys = "i2c/" SEESAW_DEVICE_NAME;
> + input_set_drvdata(seesaw->input_dev, seesaw);
> + input_set_abs_params(seesaw->input_dev, ABS_X, 0,
> + SEESAW_JOYSTICK_MAX_AXIS, SEESAW_JOYSTICK_FUZZ,
> + SEESAW_JOYSTICK_FLAT);
> + input_set_abs_params(seesaw->input_dev, ABS_Y, 0,
> + SEESAW_JOYSTICK_MAX_AXIS, SEESAW_JOYSTICK_FUZZ,
> + SEESAW_JOYSTICK_FLAT);
> +
> + ret = sparse_keymap_setup(seesaw->input_dev, seesaw_buttons_new, NULL);
> + if (ret) {
> + dev_err(&client->dev,
> + "failed to set up input device keymap: %d\n", ret);
> + return ret;
> + }
> +
> + ret = input_setup_polling(seesaw->input_dev, seesaw_poll);
> + if (ret) {
> + dev_err(&client->dev, "failed to set up polling: %d\n", ret);
> + return ret;
> + }
> +
> + input_set_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_INTERVAL);
> + input_set_max_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_MAX);
> + input_set_min_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_MIN);
> +
> + ret = input_register_device(seesaw->input_dev);
> + if (ret) {
> + dev_err(&client->dev, "failed to register joystick: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id seesaw_id_table[] = {
> + { SEESAW_DEVICE_NAME },
> + { /* Sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(i2c, seesaw_id_table);
> +
> +static const struct of_device_id seesaw_of_table[] = {
> + { .compatible = "adafruit,seesaw-gamepad"},
> + { /* Sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, seesaw_of_table);
> +
> +static struct i2c_driver seesaw_driver = {
> + .driver = {
> + .name = SEESAW_DEVICE_NAME,
> + .of_match_table = seesaw_of_table,
> + },
> + .id_table = seesaw_id_table,
> + .probe = seesaw_probe,
> +};
> +module_i2c_driver(seesaw_driver);
> +
> +MODULE_AUTHOR("Anshul Dalal <anshulusr@gmail.com>");
> +MODULE_DESCRIPTION("Adafruit Mini I2C Gamepad driver");
> +MODULE_LICENSE("GPL");
> --
> 2.43.0
>
Kind regards,
Jeff LaBundy
^ permalink raw reply
* Re: [PATCH v5 2/4] dt-bindings: touchscreen: add overlay-touchscreen and overlay-buttons properties
From: Jeff LaBundy @ 2023-12-27 20:53 UTC (permalink / raw)
To: Javier Carrasco
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch, linux-kernel,
linux-input, devicetree
In-Reply-To: <487555fa-72ad-4d1a-ac68-51826f56f1cd@wolfvision.net>
Hi Javier,
I am so sorry for the delayed response.
On Thu, Nov 23, 2023 at 08:48:56PM +0100, Javier Carrasco wrote:
> Hi Jeff,
>
> On 26.10.23 16:46, Jeff LaBundy wrote:
> > Hi Javier,
> >
> > Thank you for continuing to drive this high-quality work.
> >
> > On Tue, Oct 17, 2023 at 01:00:08PM +0200, Javier Carrasco wrote:
> >> The overlay-touchscreen object defines an area within the touchscreen
> >> where touch events are reported and their coordinates get converted to
> >> the overlay origin. This object avoids getting events from areas that
> >> are physically hidden by overlay frames.
> >>
> >> For touchscreens where overlay buttons on the touchscreen surface are
> >> provided, the overlay-buttons object contains a node for every button
> >> and the key event that should be reported when pressed.
> >>
> >> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
> >> ---
> >> .../bindings/input/touchscreen/touchscreen.yaml | 143 +++++++++++++++++++++
> >> 1 file changed, 143 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> >> index 431c13335c40..5c58eb79ee9a 100644
> >> --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> >> +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> >> @@ -87,6 +87,129 @@ properties:
> >> touchscreen-y-plate-ohms:
> >> description: Resistance of the Y-plate in Ohms
> >>
> >> + overlay-touchscreen:
> >> + description: Clipped touchscreen area
> >> +
> >> + This object can be used to describe a frame that restricts the area
> >> + within touch events are reported, ignoring the events that occur outside
> >> + this area. This is of special interest if the touchscreen is shipped
> >> + with a physical overlay on top of it with a frame that hides some part
> >> + of the original touchscreen area.
> >> +
> >> + The x-origin and y-origin properties of this object define the offset of
> >> + a new origin from where the touchscreen events are referenced.
> >> + This offset is applied to the events accordingly. The x-size and y-size
> >> + properties define the size of the overlay-touchscreen (effective area).
> >> +
> >> + The following example shows the new touchscreen area and the new origin
> >> + (0',0') for the touch events generated by the device.
> >> +
> >> + Touchscreen (full area)
> >> + ┌────────────────────────────────────────┐
> >> + │ ┌───────────────────────────────┐ │
> >> + │ │ │ │
> >> + │ ├ y-size │ │
> >> + │ │ │ │
> >> + │ │ overlay-touchscreen │ │
> >> + │ │ │ │
> >> + │ │ │ │
> >> + │ │ x-size │ │
> >> + │ ┌└──────────────┴────────────────┘ │
> >> + │(0',0') │
> >> + ┌└────────────────────────────────────────┘
> >> + (0,0)
> >> +
> >> + where (0',0') = (0+x-origin,0+y-origin)
> >> +
> >> + type: object
> >> + $ref: '#/$defs/overlay-node'
> >> + unevaluatedProperties: false
> >> +
> >> + required:
> >> + - x-origin
> >> + - y-origin
> >> + - x-size
> >> + - y-size
> >> +
> >> + overlay-buttons:
> >> + description: list of nodes defining the buttons on the touchscreen
> >> +
> >> + This object can be used to describe buttons on the touchscreen area,
> >> + reporting the touch events on their surface as key events instead of
> >> + the original touch events.
> >> +
> >> + This is of special interest if the touchscreen is shipped with a
> >> + physical overlay on top of it where a number of buttons with some
> >> + predefined functionality are printed. In that case a specific behavior
> >> + is expected from those buttons instead of raw touch events.
> >> +
> >> + The overlay-buttons properties define a per-button area as well as an
> >> + origin relative to the real touchscreen origin. Touch events within the
> >> + button area are reported as the key event defined in the linux,code
> >> + property. Given that the key events do not provide coordinates, the
> >> + button origin is only used to place the button area on the touchscreen
> >> + surface. Any event outside the overlay-buttons object is reported as a
> >> + touch event with no coordinate transformation.
> >> +
> >> + The following example shows a touchscreen with a single button on it
> >> +
> >> + Touchscreen (full area)
> >> + ┌───────────────────────────────────┐
> >> + │ │
> >> + │ │
> >> + │ ┌─────────┐ │
> >> + │ │button 0 │ │
> >> + │ │KEY_POWER│ │
> >> + │ └─────────┘ │
> >> + │ │
> >> + │ │
> >> + ┌└───────────────────────────────────┘
> >> + (0,0)
> >> +
> >> + The overlay-buttons object can be combined with the overlay-touchscreen
> >> + object as shown in the following example. In that case only the events
> >> + within the overlay-touchscreen object are reported as touch events.
> >> +
> >> + Touchscreen (full area)
> >> + ┌─────────┬──────────────────────────────┐
> >> + │ │ │
> >> + │ │ ┌───────────────────────┐ │
> >> + │ button 0│ │ │ │
> >> + │KEY_POWER│ │ │ │
> >> + │ │ │ │ │
> >> + ├─────────┤ │ overlay-touchscreen │ │
> >> + │ │ │ │ │
> >> + │ │ │ │ │
> >> + │ button 1│ │ │ │
> >> + │ KEY_INFO│ ┌└───────────────────────┘ │
> >> + │ │(0',0') │
> >> + ┌└─────────┴──────────────────────────────┘
> >> + (0,0)
> >> +
> >> + type: object
> >
> > I am still confused why the buttons need to live under an 'overlay-buttons'
> > parent node, which seems like an imaginary boundary. In my view, the touch
> > surface comprises the following types of rectangular areas:
> >
> > 1. A touchscreen, wherein granular coordinates and pressure are reported.
> > 2. A momentary button, wherein pressure is quantized into a binary value
> > (press or release), and coordinates are ignored.
> >
> > Any contact that falls outside of (1) and (2) is presumed to be part of a
> > border or matting, and is hence ignored.
> >
> > Areas (1) and (2) exist in the same "plane", so why can they not reside
> > under the same parent node? The following seems much more representative
> > of the actual hardware we intend to describe in the device tree:
> >
> > touchscreen {
> > compatible = "...";
> > reg = <...>;
> >
> > /* raw coordinates reported here */
> > touch-area-1 {
> > x-origin = <...>;
> > y-origin = <...>;
> > x-size = <...>;
> > y-size = <...>;
> > };
> >
> > /* a button */
> > touch-area-2a {
> > x-origin = <...>;
> > y-origin = <...>;
> > x-size = <...>;
> > y-size = <...>;
> > linux,code = <KEY_POWER>;
> > };
> >
> > /* another button */
> > touch-area-2b {
> > x-origin = <...>;
> > y-origin = <...>;
> > x-size = <...>;
> > y-size = <...>;
> > linux,code = <KEY_INFO>;
> > };
> > };
> >
> Now that I am working on the approach you suggested, I see that some
> things can get slightly more complicated. I still think that it is worth
> a try, but I would like to discuss a couple of points.
>
> The node parsing is not that simple anymore because the touch-area nodes
> are only surrounded by the touchscreen node. Theoretically they could be
> even be defined with other properties in between. The current approach
> only needs to find the overlay-buttons parent and iterate over all the
> inner nodes(simply by calling device_get_named_child_node() and
> fwnode_for_each_child_node() the parsing is achieved in two lines +
> error checking). So maybe even if we opt for the single-object approach,
> an overlay node to group all the touch-areas could simplify the parsing.
> Or did you have a different approach in mind? Your example would turn
> into this one:
>
> touchscreen {
> compatible = "...";
> reg = <...>;
>
> touch-overlay {
> /* raw coordinates reported here */
> touch-area-1 {
> x-origin = <...>;
> y-origin = <...>;
> x-size = <...>;
> y-size = <...>;
> };
>
> /* a button */
> touch-area-2a {
> x-origin = <...>;
> y-origin = <...>;
> x-size = <...>;
> y-size = <...>;
> linux,code = <KEY_POWER>;
> };
>
> /* another button */
> touch-area-2b {
> x-origin = <...>;
> y-origin = <...>;
> x-size = <...>;
> y-size = <...>;
> linux,code = <KEY_INFO>;
> };
> };
> };
> In my opinion it looks cleaner as well because you are defining a
> physical object: the overlay.
I like this idea. My original example assumes one would include any node
that contains some magic substring (e.g. "touch") in the node name, but
thinking about this more, that may be a bit presumptive. It seems safer
to wrap all of the children in one newly introduced node as you have done
here.
>
> > With this method, the driver merely stores a list head. The parsing code
> > then walks the client device node; for each touch* child encountered, it
> > allocates memory for a structure of five members, and adds it to the list.
> >
> The button objects do not only store the keycode, but also the slot and
> if they are pressed or not. I could allocate memory for these members as
> well, but maybe an additional struct with the button-specific members
> set to NULL for the touch areas with keycode = KEY_RESERVED would make
> sense. I don't know if that's adding too much overhead for two members
> though.
It's still not clear to me why your code is responsible for storing button
state; that's the job of the input subsystem. Your code is only responsible
for reporting instantaneous state after you are told something interesting
happened (e.g. interrupt). The input core is responsible for determining
whether the most recently reported state is different than the last.
>
> > The event handling code then simply iterates through the list and checks
> > if the coordinates reported by the hardware fall within each rectangle. If
> > so, and the keycode in the list element is equal to KEY_RESERVED (zero),
> > we assume the rectangle is of type (1); the coordinates are passed to
> > touchscreen_report_pos() and the pressure is reported as well.
>
> There is another case to consider that might make the iteration less
> optimal, but I don't think it will be critical.
>
> A button could be defined inside an overlay-touchscreen (no keycode)
> area. Given that the other way round (a touchscreen inside a button)
> does not make much sense, the buttons have a higher priority.
>
> Let's take your example: imagine that your third area
> is a button inside the first one. We have to iterate through the whole
> list until we are sure we checked if there are buttons in the given
> position, but keeping in mind that the first object already has the
> right coordinates to handle the touch event. Your approach even allows
> for multiple no-key areas and we do not know if there are buttons when
> we iterate (there could be none).
> Therefore some iterations could be unnecessary, but this is probably an
> edge case that would cost at most a couple of extra iterations compared
> to a two-list approach.
I think we need to model the overlay as having only two dimensions, with
nothing "on top of" or "inside" anything else. For this case of a button
inside a touch surface, with the latter making a square doughnut shape of
sorts, the 'touch-overlay' node would have five children: two tall
rectangles (left and right), two shorter rectanges (above and below the
button), and then finally a button in the center.
Stated another way, the 'touch-overlay' node shall support an infinite
number of infinitesimally small rectangles which comprise the entire touch
surface. It shall be possible for a contact to be in zero rectangles, but
impossible for any contact to be in more than one rectangle. I appreciate
that the devil is in the details, but here we are defining the interface,
independent of the implementation.
>
> I will keep on working on the next version with a single list while we
> clarify these points, so maybe we can save an iteration.
I see there is a v6 now; I'll take a look at that next. Thanks again for
the productive discussion!
> > Kind regards,
> > Jeff LaBundy
>
> Best regards,
> Javier Carrasco
Kind regards,
Jeff LaBundy
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: input: Add Himax HX83102J touchscreen
From: kernel test robot @ 2023-12-28 8:30 UTC (permalink / raw)
To: Allen_Lin, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, jikos, benjamin.tissoires, linux-input, devicetree,
linux-kernel
Cc: oe-kbuild-all, Allen_Lin
In-Reply-To: <SEZPR06MB56080820EE51CBAE9C6B6B3E9E9FA@SEZPR06MB5608.apcprd06.prod.outlook.com>
Hi Allen_Lin,
kernel test robot noticed the following build warnings:
[auto build test WARNING on hid/for-next]
[also build test WARNING on dtor-input/next dtor-input/for-linus robh/for-next linus/master v6.7-rc7 next-20231222]
[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/Allen_Lin/Input-Add-Himax-HX83102J-touchscreen-driver/20231227-133817
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/SEZPR06MB56080820EE51CBAE9C6B6B3E9E9FA%40SEZPR06MB5608.apcprd06.prod.outlook.com
patch subject: [PATCH v3 1/2] dt-bindings: input: Add Himax HX83102J touchscreen
:::::: branch date: 19 hours ago
:::::: commit date: 19 hours ago
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231228/202312280837.b2PFmd9W-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/r/202312280837.b2PFmd9W-lkp@intel.com/
dtcheck warnings: (new ones prefixed by >>)
>> Documentation/devicetree/bindings/input/himax,hx83102j.yaml:1:58: [error] wrong new line character: expected \n (new-lines)
vim +1 Documentation/devicetree/bindings/input/himax,hx83102j.yaml
b6f7a8833439cc Allen_Lin 2023-12-27 @1 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: input: Add Himax HX83102J touchscreen
From: Krzysztof Kozlowski @ 2023-12-28 10:36 UTC (permalink / raw)
To: Allen_Lin, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, jikos, benjamin.tissoires, linux-input, devicetree,
linux-kernel
In-Reply-To: <SEZPR06MB56080820EE51CBAE9C6B6B3E9E9FA@SEZPR06MB5608.apcprd06.prod.outlook.com>
On 27/12/2023 06:35, Allen_Lin wrote:
> Add the HX83102j touchscreen device tree bindings documents.
>
> Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> ---
Where is the changelog? There is no cover letter attached, so changelog
is supposed to be here. There were several comments, so does it mean you
ignored them?
> .../bindings/input/himax,hx83102j.yaml | 65 +++++++++++++++++++
> MAINTAINERS | 6 ++
> 2 files changed, 71 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/himax,hx83102j.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/himax,hx83102j.yaml b/Documentation/devicetree/bindings/input/himax,hx83102j.yaml
> new file mode 100644
> index 000000000000..872b478c5753
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/himax,hx83102j.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/himax,hx83102j.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Himax hx83102j touchscreen
> +
...
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + hid-himax-spi@0 {
Still not the name I asked - it should be generic, like touchscreen.
Best regards,
Krzysztof
^ 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