* Re: [PATCH 0/7] Add samsung-milletwifi
From: Linus Walleij @ 2023-11-05 21:19 UTC (permalink / raw)
To: Bryant Mairs
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov, linux-arm-msm,
devicetree, linux-kernel, linux-input
In-Reply-To: <20231105204759.37107-1-bryant@mai.rs>
On Sun, Nov 5, 2023 at 9:48 PM Bryant Mairs <bryant@mai.rs> wrote:
> This series adds support for samsung-milletwifi, the smaller cousin
> to samsung-matisselte. I've used the manufacturer's naming convention
> for consistency.
The series looks very good to me:
Acked-by: Linus Walleij <linus.walleij@linaro.org>
I also see that DRM+panel is in the works for this interesting device,
which is great!
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] HID: apple: add Jamesdonkey and A3R to non-apple keyboards list
From: Rahul Rameshbabu @ 2023-11-06 3:11 UTC (permalink / raw)
To: Yihong Cao; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <SYYP282MB2110B4E87983EAFEDC8741E49BA2A@SYYP282MB2110.AUSP282.PROD.OUTLOOK.COM>
On Mon, 30 Oct, 2023 01:05:38 +0800 "Yihong Cao" <caoyihong4@outlook.com> wrote:
> Jamesdonkey A3R keyboard is identified as "Jamesdonkey A3R" in wired
> mode, "A3R-U" in wireless mode and "A3R" in bluetooth mode. Adding them
> to non-apple keyboards fixes function key.
>
> Signed-off-by: Yihong Cao <caoyihong4@outlook.com>
> ---
> drivers/hid/hid-apple.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> index 3ca45975c686..d9e9829b2200 100644
> --- a/drivers/hid/hid-apple.c
> +++ b/drivers/hid/hid-apple.c
> @@ -345,6 +345,8 @@ static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
> { "AONE" },
> { "GANSS" },
> { "Hailuck" },
> + { "Jamesdonkey" },
Sorry, maybe I misunderstood the commit message. In wired mode, if the
keyboard is identified as "Jamesdonkey A3R", shouldn't this value be
"Jamesdonkey A3R" instead of "Jamesdonkey"?
> + { "A3R" },
> };
>
> static bool apple_is_non_apple_keyboard(struct hid_device *hdev)
--
Thanks,
Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH] HID: glorious: fix Glorious Model I HID report
From: Rahul Rameshbabu @ 2023-11-06 3:28 UTC (permalink / raw)
To: Brett Raye; +Cc: jikos, benjamin.tissoires, linux-input
In-Reply-To: <20231103011038.27462-1-braye@fastmail.com>
On Thu, 02 Nov, 2023 18:10:38 -0700 "Brett Raye" <braye@fastmail.com> wrote:
> The Glorious Model I mouse has a buggy HID report descriptor for its
> keyboard endpoint (used for programmable buttons). For report ID 2, there
> is a mismatch between Logical Minimum and Usage Minimum in the array that
> reports keycodes.
>
> The offending portion of the descriptor: (from hid-decode)
>
> 0x95, 0x05, // Report Count (5) 30
> 0x75, 0x08, // Report Size (8) 32
> 0x15, 0x00, // Logical Minimum (0) 34
> 0x25, 0x65, // Logical Maximum (101) 36
> 0x05, 0x07, // Usage Page (Keyboard) 38
> 0x19, 0x01, // Usage Minimum (1) 40
> 0x29, 0x65, // Usage Maximum (101) 42
> 0x81, 0x00, // Input (Data,Arr,Abs) 44
>
> This bug shifts all programmed keycodes up by 1. Importantly, this causes
> "empty" array indexes of 0x00 to be interpreted as 0x01, ErrorRollOver.
> The presence of ErrorRollOver causes the system to ignore all keypresses
> from the endpoint and breaks the ability to use the programmable buttons.
>
> Setting byte 41 to 0x00 fixes this, and causes keycodes to be interpreted
> correctly.
>
> Also, USB_VENDOR_ID_GLORIOUS is changed to USB_VENDOR_ID_SINOWEALTH,
> and a new ID for Laview Technology is added. Glorious seems to be
> white-labeling controller boards or mice from these vendors. There isn't a
> single canonical vendor ID for Glorious products.
I agree with what this patch is doing overall. However, I think this
patch should be split into two patches that are part of a patch series.
One patch that does s/USB_VENDOR_ID_GLORIOUS/USB_VENDOR_ID_SINOWEALTH.
The follow-up patch in the series will contain the work on the Model I
keyboard.
>
> Signed-off-by: Brett Raye <braye@fastmail.com>
> ---
--
Thanks,
Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH] HID: wacom_sys: add error code check in wacom_feature_mapping
From: Rahul Rameshbabu @ 2023-11-06 3:57 UTC (permalink / raw)
To: Su Hui
Cc: ping.cheng, jason.gerecke, jikos, benjamin.tissoires, linux-input,
linux-kernel, kernel-janitors
In-Reply-To: <20231020090237.201029-1-suhui@nfschina.com>
On Fri, 20 Oct, 2023 17:02:38 +0800 "Su Hui" <suhui@nfschina.com> wrote:
> hid_report_raw_event() can return error code like '-ENOMEM' if
> failed, so check 'ret' to make sure all things work fine.
I can agree with adding logging for error cases personally.
>
> Signed-off-by: Su Hui <suhui@nfschina.com>
> ---
> drivers/hid/wacom_sys.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 3f704b8072e8..1f898d4ee708 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -320,6 +320,8 @@ static void wacom_feature_mapping(struct hid_device *hdev,
> if (ret == n && features->type == HID_GENERIC) {
> ret = hid_report_raw_event(hdev,
> HID_FEATURE_REPORT, data, n, 0);
> + if (ret)
> + hid_warn(hdev, "failed to report feature\n");
I think we should report the returned error information as well.
https://docs.kernel.org/core-api/printk-formats.html#error-pointers
Typically what I do is use ERR_PTR in tandem with the %pe modifier for
printing errors.
> } else if (ret == 2 && features->type != HID_GENERIC) {
> features->touch_max = data[1];
> } else {
> @@ -381,6 +383,8 @@ static void wacom_feature_mapping(struct hid_device *hdev,
> if (ret == n) {
> ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT,
> data, n, 0);
> + if (ret)
> + hid_warn(hdev, "failed to report feature\n");
> } else {
> hid_warn(hdev, "%s: could not retrieve sensor offsets\n",
> __func__);
--
Thanks for the patch,
Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH v2] HID: fix a crash in hid_debug_events_release
From: Rahul Rameshbabu @ 2023-11-06 4:51 UTC (permalink / raw)
To: Charles Yi; +Cc: jikos, benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <20231031043239.157943-1-be286@163.com>
Lets clean up the subject/commit message heading.
HID: fix HID device resource race between HID core and debugging support
In the commit message body, we can expand on the details a bit more.
On Tue, 31 Oct, 2023 12:32:39 +0800 "Charles Yi" <be286@163.com> wrote:
> hid_debug_events_release() access released memory by
> hid_device_release(). This is fixed by the patch.
>
> When hid_debug_events_release() was being called, in most case,
> hid_device_release() finish already, the memory of list->hdev
> freed by hid_device_release(), if list->hdev memory
> reallocate by others, and it's modified, zeroed, then
> list->hdev->debug_list_lock occasioned crash come out.
Lets clean up these paragraphs a bit.
hid_debug_events_release releases resources bound to the HID device
instance. hid_device_release releases the underlying HID device
instance potentially before hid_debug_events_release has completed
releasing debug resources bound to the same HID device instance.
Reference count to prevent the HID device instance from being torn
down preemptively when HID debugging support is used. When count
reaches zero, release core resources of HID device instance using
hiddev_free.
Feel free to use the above if you think its nice or feel free to polish
up the commit message body you originally had a bit more.
>
> The crash:
>
> [ 120.728477][ T4396] kernel BUG at lib/list_debug.c:53!
> [ 120.728505][ T4396] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> [ 120.739806][ T4396] Modules linked in: bcmdhd dhd_static_buf 8822cu pcie_mhi r8168
> [ 120.747386][ T4396] CPU: 1 PID: 4396 Comm: hidt_bridge Not tainted 5.10.110 #257
> [ 120.754771][ T4396] Hardware name: Rockchip RK3588 EVB4 LP4 V10 Board (DT)
> [ 120.761643][ T4396] pstate: 60400089 (nZCv daIf +PAN -UAO -TCO BTYPE=--)
> [ 120.768338][ T4396] pc : __list_del_entry_valid+0x98/0xac
> [ 120.773730][ T4396] lr : __list_del_entry_valid+0x98/0xac
> [ 120.779120][ T4396] sp : ffffffc01e62bb60
> [ 120.783126][ T4396] x29: ffffffc01e62bb60 x28: ffffff818ce3a200
> [ 120.789126][ T4396] x27: 0000000000000009 x26: 0000000000980000
> [ 120.795126][ T4396] x25: ffffffc012431000 x24: ffffff802c6d4e00
> [ 120.801125][ T4396] x23: ffffff8005c66f00 x22: ffffffc01183b5b8
> [ 120.807125][ T4396] x21: ffffff819df2f100 x20: 0000000000000000
> [ 120.813124][ T4396] x19: ffffff802c3f0700 x18: ffffffc01d2cd058
> [ 120.819124][ T4396] x17: 0000000000000000 x16: 0000000000000000
> [ 120.825124][ T4396] x15: 0000000000000004 x14: 0000000000003fff
> [ 120.831123][ T4396] x13: ffffffc012085588 x12: 0000000000000003
> [ 120.837123][ T4396] x11: 00000000ffffbfff x10: 0000000000000003
> [ 120.843123][ T4396] x9 : 455103d46b329300 x8 : 455103d46b329300
> [ 120.849124][ T4396] x7 : 74707572726f6320 x6 : ffffffc0124b8cb5
> [ 120.855124][ T4396] x5 : ffffffffffffffff x4 : 0000000000000000
> [ 120.861123][ T4396] x3 : ffffffc011cf4f90 x2 : ffffff81fee7b948
> [ 120.867122][ T4396] x1 : ffffffc011cf4f90 x0 : 0000000000000054
> [ 120.873122][ T4396] Call trace:
> [ 120.876259][ T4396] __list_del_entry_valid+0x98/0xac
> [ 120.881304][ T4396] hid_debug_events_release+0x48/0x12c
> [ 120.886617][ T4396] full_proxy_release+0x50/0xbc
> [ 120.891323][ T4396] __fput+0xdc/0x238
> [ 120.895075][ T4396] ____fput+0x14/0x24
> [ 120.898911][ T4396] task_work_run+0x90/0x148
> [ 120.903268][ T4396] do_exit+0x1bc/0x8a4
> [ 120.907193][ T4396] do_group_exit+0x8c/0xa4
> [ 120.911458][ T4396] get_signal+0x468/0x744
> [ 120.915643][ T4396] do_signal+0x84/0x280
> [ 120.919650][ T4396] do_notify_resume+0xd0/0x218
> [ 120.924262][ T4396] work_pending+0xc/0x3f0
>
> Fixes: <cd667ce24796> (HID: use debugfs for events/reports dumping)
The formatting of the Fixes: tag would look like the following.
Fixes: cd667ce24796 ("HID: use debugfs for events/reports dumping")
You can also eliminate the whitespace between your git trailers, so the
end result looks like the following (minus the indentation).
Fixes: cd667ce24796 ("HID: use debugfs for events/reports dumping")
Signed-off-by: Charles Yi <be286@163.com>
>
> Signed-off-by: Charles Yi <be286@163.com>
>
> ---
> Changes in V2:
> - Add "Fixes:" tag and call trace to commit message.
> ---
> drivers/hid/hid-core.c | 12 ++++++++++--
> drivers/hid/hid-debug.c | 3 +++
> include/linux/hid.h | 3 +++
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 8992e3c1e769..e0181218ad85 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -702,15 +702,22 @@ static void hid_close_report(struct hid_device *device)
> * Free a device structure, all reports, and all fields.
> */
>
> -static void hid_device_release(struct device *dev)
> +void hiddev_free(struct kref *ref)
Lets call this hid_hiddev_free. Took a look through hid-core.c, and I
think this would be better than calling it just hiddev_free.
> {
> - struct hid_device *hid = to_hid_device(dev);
> + struct hid_device *hid = container_of(ref, struct hid_device, ref);
>
> hid_close_report(hid);
> kfree(hid->dev_rdesc);
> kfree(hid);
> }
>
> +static void hid_device_release(struct device *dev)
> +{
> + struct hid_device *hid = to_hid_device(dev);
> +
> + kref_put(&hid->ref, hiddev_free);
> +}
> +
> /*
> * Fetch a report description item from the data stream. We support long
> * items, though they are not used yet.
> @@ -2846,6 +2853,7 @@ struct hid_device *hid_allocate_device(void)
> spin_lock_init(&hdev->debug_list_lock);
> sema_init(&hdev->driver_input_lock, 1);
> mutex_init(&hdev->ll_open_lock);
> + kref_init(&hdev->ref);
>
> hid_bpf_device_init(hdev);
>
> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> index e7ef1ea107c9..7dd83ec74f8a 100644
> --- a/drivers/hid/hid-debug.c
> +++ b/drivers/hid/hid-debug.c
> @@ -1135,6 +1135,7 @@ static int hid_debug_events_open(struct inode *inode, struct file *file)
> goto out;
> }
> list->hdev = (struct hid_device *) inode->i_private;
> + kref_get(&list->hdev->ref);
> file->private_data = list;
> mutex_init(&list->read_mutex);
>
> @@ -1227,6 +1228,8 @@ static int hid_debug_events_release(struct inode *inode, struct file *file)
> list_del(&list->node);
> spin_unlock_irqrestore(&list->hdev->debug_list_lock, flags);
> kfifo_free(&list->hid_debug_fifo);
> +
> + kref_put(&list->hdev->ref, hiddev_free);
> kfree(list);
>
> return 0;
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 964ca1f15e3f..3b08a2957229 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -679,6 +679,7 @@ struct hid_device { /* device report descriptor */
> struct list_head debug_list;
> spinlock_t debug_list_lock;
> wait_queue_head_t debug_wait;
> + struct kref ref;
>
> unsigned int id; /* system unique id */
>
> @@ -687,6 +688,8 @@ struct hid_device { /* device report descriptor */
> #endif /* CONFIG_BPF */
> };
>
> +void hiddev_free(struct kref *ref);
> +
> #define to_hid_device(pdev) \
> container_of(pdev, struct hid_device, dev)
--
Thanks for the patch,
Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH] HID: wacom_sys: add error code check in wacom_feature_mapping
From: Su Hui @ 2023-11-06 5:50 UTC (permalink / raw)
To: Rahul Rameshbabu
Cc: ping.cheng, jason.gerecke, jikos, benjamin.tissoires, linux-input,
linux-kernel, kernel-janitors
In-Reply-To: <871qd31qkx.fsf@protonmail.com>
On 2023/11/6 11:57, Rahul Rameshbabu wrote:
> On Fri, 20 Oct, 2023 17:02:38 +0800 "Su Hui" <suhui@nfschina.com> wrote:
>> hid_report_raw_event() can return error code like '-ENOMEM' if
>> failed, so check 'ret' to make sure all things work fine.
> I can agree with adding logging for error cases personally.
>
>> Signed-off-by: Su Hui <suhui@nfschina.com>
>> ---
>> drivers/hid/wacom_sys.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
>> index 3f704b8072e8..1f898d4ee708 100644
>> --- a/drivers/hid/wacom_sys.c
>> +++ b/drivers/hid/wacom_sys.c
>> @@ -320,6 +320,8 @@ static void wacom_feature_mapping(struct hid_device *hdev,
>> if (ret == n && features->type == HID_GENERIC) {
>> ret = hid_report_raw_event(hdev,
>> HID_FEATURE_REPORT, data, n, 0);
>> + if (ret)
>> + hid_warn(hdev, "failed to report feature\n");
> I think we should report the returned error information as well.
Agreed, I will send v2 soon.
Thanks for your suggestions!
Su Hui
>
> https://docs.kernel.org/core-api/printk-formats.html#error-pointers
>
> Typically what I do is use ERR_PTR in tandem with the %pe modifier for
> printing errors.
>
>> } else if (ret == 2 && features->type != HID_GENERIC) {
>> features->touch_max = data[1];
>> } else {
>> @@ -381,6 +383,8 @@ static void wacom_feature_mapping(struct hid_device *hdev,
>> if (ret == n) {
>> ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT,
>> data, n, 0);
>> + if (ret)
>> + hid_warn(hdev, "failed to report feature\n");
>> } else {
>> hid_warn(hdev, "%s: could not retrieve sensor offsets\n",
>> __func__);
> --
> Thanks for the patch,
>
> Rahul Rameshbabu
>
^ permalink raw reply
* [PATCH v2] HID: wacom_sys: add error code check in wacom_feature_mapping
From: Su Hui @ 2023-11-06 6:08 UTC (permalink / raw)
To: sergeantsagara, ping.cheng, jason.gerecke, jikos,
benjamin.tissoires
Cc: Su Hui, linux-input, linux-kernel, kernel-janitors
hid_report_raw_event() can return error code like '-ENOMEM' if
failed, so check 'ret' to make sure all things work fine.
Signed-off-by: Su Hui <suhui@nfschina.com>
---
v2:
- report the returned error (Rahul Rameshbabu) and __func__.
drivers/hid/wacom_sys.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 3f704b8072e8..899579c6db9d 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -320,6 +320,9 @@ static void wacom_feature_mapping(struct hid_device *hdev,
if (ret == n && features->type == HID_GENERIC) {
ret = hid_report_raw_event(hdev,
HID_FEATURE_REPORT, data, n, 0);
+ if (ret)
+ hid_warn(hdev, "%s: failed to report feature: %pe\n",
+ __func__, ERR_PTR(ret));
} else if (ret == 2 && features->type != HID_GENERIC) {
features->touch_max = data[1];
} else {
@@ -381,6 +384,9 @@ static void wacom_feature_mapping(struct hid_device *hdev,
if (ret == n) {
ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT,
data, n, 0);
+ if (ret)
+ hid_warn(hdev, "%s: failed to report feature: %pe\n",
+ __func__, ERR_PTR(ret));
} else {
hid_warn(hdev, "%s: could not retrieve sensor offsets\n",
__func__);
--
2.30.2
^ permalink raw reply related
* Re: [PATCH v2] HID: wacom_sys: add error code check in wacom_feature_mapping
From: Rahul Rameshbabu @ 2023-11-06 6:16 UTC (permalink / raw)
To: Su Hui
Cc: ping.cheng, jason.gerecke, jikos, benjamin.tissoires, linux-input,
linux-kernel, kernel-janitors
In-Reply-To: <20231106060815.104971-1-suhui@nfschina.com>
On Mon, 06 Nov, 2023 14:08:16 +0800 "Su Hui" <suhui@nfschina.com> wrote:
> hid_report_raw_event() can return error code like '-ENOMEM' if
> failed, so check 'ret' to make sure all things work fine.
>
> Signed-off-by: Su Hui <suhui@nfschina.com>
> ---
> v2:
> - report the returned error (Rahul Rameshbabu) and __func__.
Thanks for the patch. Like the addition of using __func__.
Reviewed-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
^ permalink raw reply
* Re: [PATCH 1/7] dt-bindings: input: melfas,mms114: add MMS252 compatible
From: Krzysztof Kozlowski @ 2023-11-06 7:36 UTC (permalink / raw)
To: Bryant Mairs, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Linus Walleij, linux-arm-msm, devicetree, linux-kernel,
linux-input
Cc: Luca Weiss
In-Reply-To: <20231105204759.37107-2-bryant@mai.rs>
On 05/11/2023 21:46, Bryant Mairs wrote:
> From: Luca Weiss <luca@z3ntu.xyz>
>
> Add a compatible for MMS252 touchscreen which appears to work fine with
> the MMS114 driver.
>
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> Signed-off-by: Bryant Mairs <bryant@mai.rs>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/7] dt-bindings: arm: qcom: Document samsung,milletwifi device
From: Krzysztof Kozlowski @ 2023-11-06 7:37 UTC (permalink / raw)
To: Bryant Mairs, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Linus Walleij, linux-arm-msm, devicetree, linux-kernel,
linux-input
In-Reply-To: <20231105204759.37107-3-bryant@mai.rs>
On 05/11/2023 21:46, Bryant Mairs wrote:
> Add binding documentation for Samsung Galaxy Tab 4 8.0 Wi-Fi
> tablet which is based on Snapdragon 400 (apq8026) SoC.
>
> Signed-off-by: Bryant Mairs <bryant@mai.rs>
> ---
> Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> index 88b84035e7b1..c66980b79f59 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -156,6 +156,7 @@ properties:
> - huawei,sturgeon
> - lg,lenok
> - samsung,matisse-wifi
> + - samsung,milletwifi
Not millet-wifi like the other one?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 4/7] ARM: dts: qcom: Disable pm8941 & pm8226 smbb charger by default
From: Krzysztof Kozlowski @ 2023-11-06 7:39 UTC (permalink / raw)
To: Bryant Mairs, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Linus Walleij, linux-arm-msm, devicetree, linux-kernel,
linux-input
In-Reply-To: <20231105204759.37107-5-bryant@mai.rs>
On 05/11/2023 21:46, Bryant Mairs wrote:
> Some platforms don't use the built-in charging hardware (e.g. milletwifi).
> As this is an optional peripheral, default it to off.
So your patch order is not correct. If millet-wifi does not use it, why
do you enable it in previous patch?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 5/7] ARM: dts: qcom: apq8026-samsung-milletwifi: Enable charger
From: Krzysztof Kozlowski @ 2023-11-06 7:40 UTC (permalink / raw)
To: Bryant Mairs, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Linus Walleij, linux-arm-msm, devicetree, linux-kernel,
linux-input
In-Reply-To: <20231105204759.37107-6-bryant@mai.rs>
On 05/11/2023 21:46, Bryant Mairs wrote:
> Enable charging support.
>
> Signed-off-by: Bryant Mairs <bryant@mai.rs>
> ---
> .../qcom/qcom-apq8026-samsung-milletwifi.dts | 28 +++++++++++++++++++
> 1 file changed, 28 insertions(+)
Squash with new board. One logical change is to add new board. Not add
incomplete board and immediately fix it.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 6/7] ARM: dts: qcom: apq8026-samsung-milletwifi: Enable Wi-Fi and Bluetooth
From: Krzysztof Kozlowski @ 2023-11-06 7:40 UTC (permalink / raw)
To: Bryant Mairs, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Linus Walleij, linux-arm-msm, devicetree, linux-kernel,
linux-input
In-Reply-To: <20231105204759.37107-7-bryant@mai.rs>
On 05/11/2023 21:46, Bryant Mairs wrote:
> Enable Wi-Fi and Bluetooth support for milletwifi. This device
> uses the WCN3660A that is already supported, so it only needs to be
> enabled in the DTS.
>
Squash.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 7/7] ARM: dts: qcom: apq8026-samsung-milletwifi: Enable modem
From: Krzysztof Kozlowski @ 2023-11-06 7:41 UTC (permalink / raw)
To: Bryant Mairs, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Linus Walleij, linux-arm-msm, devicetree, linux-kernel,
linux-input
In-Reply-To: <20231105204759.37107-8-bryant@mai.rs>
On 05/11/2023 21:46, Bryant Mairs wrote:
> Enable the modem hardware for milletwifi, which is actually only the GPS
> functionality.
>
Squash.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/7] dt-bindings: arm: qcom: Document samsung,milletwifi device
From: Bryant Mairs @ 2023-11-06 8:07 UTC (permalink / raw)
To: Krzysztof Kozlowski, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Linus Walleij, linux-arm-msm, devicetree, linux-kernel,
linux-input
In-Reply-To: <1ea6c498-09bc-4491-a083-37fa242766c8@linaro.org>
Sending again in plain text, sorry about that!
-----
That's a good question. Note that there is also samsung-matisselte, so I wasn't certain what the policy was on naming, which is why I deferred to the manufacturer naming scheme, which seems to be the most common approach.
On Mon, Nov 6, 2023, at 08:37, Krzysztof Kozlowski wrote:
> On 05/11/2023 21:46, Bryant Mairs wrote:
> > Add binding documentation for Samsung Galaxy Tab 4 8.0 Wi-Fi
> > tablet which is based on Snapdragon 400 (apq8026) SoC.
> >
> > Signed-off-by: Bryant Mairs <bryant@mai.rs>
> > ---
> > Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> > index 88b84035e7b1..c66980b79f59 100644
> > --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> > +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> > @@ -156,6 +156,7 @@ properties:
> > - huawei,sturgeon
> > - lg,lenok
> > - samsung,matisse-wifi
> > + - samsung,milletwifi
>
> Not millet-wifi like the other one?
>
> Best regards,
> Krzysztof
>
>
^ permalink raw reply
* [PATCH v11 0/4] Input: add initial support for Goodix Berlin touchscreen IC
From: Neil Armstrong @ 2023-11-06 8:33 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bastien Nocera,
Hans de Goede, Henrik Rydberg, Jeff LaBundy, linux-arm-msm,
devicetree, linux-kernel, Neil Armstrong, Rob Herring
These touchscreen ICs support SPI, I2C and I3C interface, up to
10 finger touch, stylus and gestures events.
This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.
The current implementation only supports BerlinD, aka GT9916.
Support for advanced features like:
- Firmware & config update
- Stylus events
- Gestures events
- Previous revisions support (BerlinA or BerlinB)
is not included in current version.
The current support will work with currently flashed firmware
and config, and bail out if firmware or config aren't flashed yet.
[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Changes in v11:
- Fixes according to Jeff's review:
- s/dev_info/dev_err/ in goodix_berlin_get_ic_info()
- remove spurious return instead of goto in goodix_berlin_get_ic_info()
- added back dropped msleep() in goodix_berlin_request_handle_reset()
- Link to v10: https://lore.kernel.org/r/20231023-topic-goodix-berlin-upstream-initial-v10-0-88eec2e51c0b@linaro.org
Changes in v10:
- Fix according to Dmitry's review:
- move goodix_berlin_get_ic_info() afe_data to heap
- merge the goodix_berlin_parse_finger() loops and skip invalid fingers instead of returning
- remove unwanted goodix_berlin_touch_handler() "static" for buffer
- only call goodix_berlin_request_handle_reset() if gpio was provided
- use "error = func(); if(error) return error;" instead of "return func()" when function handles multiple error cases
- Link to v9: https://lore.kernel.org/r/20231021-topic-goodix-berlin-upstream-initial-v9-0-13fb4e887156@linaro.org
Changes in v9:
- Rebased on next-20231020
- Link to v8: https://lore.kernel.org/r/20231003-topic-goodix-berlin-upstream-initial-v8-0-171606102ed6@linaro.org
Changes in v8:
- Add missing bitfield.h include in core
- Link to v7: https://lore.kernel.org/r/20231002-topic-goodix-berlin-upstream-initial-v7-0-792fb91f5e88@linaro.org
Changes in v7:
- rebased on v6.6-rc3
- Link to v6: https://lore.kernel.org/r/20230912-topic-goodix-berlin-upstream-initial-v6-0-b4ecfa49fb9d@linaro.org
Changes in v6:
- rebased on v6.6-rc1
- changed commit message prefix to match the other Input commits
- Link to v5: https://lore.kernel.org/r/20230801-topic-goodix-berlin-upstream-initial-v5-0-079252935593@linaro.org
Changes in v5:
- rebased on next-20230801
- Link to v4: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v4-0-0947c489be17@linaro.org
Changes in v4:
- Core updates:
- drop kconfig depends, deps will be handled by _SPI and _I2C
- change power_on() error labels
- print errors on all dev_err() prints
- remove useless default variable initialization
- switch irq touch checksum error to dev_err()
- add Jeff's review tag
- I2C changes
- change REGMAP_I2C Kconfig from depends to select
- add Jeff's review tag
- SPI changes
- add select REGMAP to Kconfig
- added GOODIX_BERLIN_ prefix to defines
- switched from ret to error
- add Jeff's review tag
- Link to v3: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v3-0-f0577cead709@linaro.org
Changes in v3:
- Another guge cleanups after Jeff's review:
- appended goodix_berlin_ before all defines
- removed some unused defines
- removed retries on most of read functions, can be added back later
- added __le to ic_info structures
- reworked and simplified irq handling, dropped enum and ts_event structs
- added struct for touch data
- simplified and cleaned goodix_berlin_check_checksum & goodix_berlin_is_dummy_data
- moved touch_data_addr to the end of the main code_data
- reworked probe to get_irq last and right before setip input device
- cleaned probe by removing the "cd->dev"
- added short paragraph to justify new driver for berlin devices
- defined all offsets & masks
- Added bindings review tag
- Link to v2: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v2-0-26bc8fe1e90e@linaro.org
Changes in v2:
- Huge cleanups after Jeff's review:
- switch to error instead of ret
- drop dummy vendor/product ids
- drop unused defined/enums
- drop unused ic_info and only keep needes values
- cleanup namings and use goodix_berlin_ everywhere
- fix regulator setup
- fix default variables value when assigned afterwars
- removed indirections
- dropped debugfs
- cleaned input_dev setup
- dropped _remove()
- sync'ed i2c and spi drivers
- fixed yaml bindings
- Link to v1: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v1-0-4a0741b8aefd@linaro.org
---
Neil Armstrong (4):
dt-bindings: input: document Goodix Berlin Touchscreen IC
Input: add core support for Goodix Berlin Touchscreen IC
Input: goodix-berlin - add I2C support for Goodix Berlin Touchscreen IC
Input: goodix-berlin - add SPI support for Goodix Berlin Touchscreen IC
.../bindings/input/touchscreen/goodix,gt9916.yaml | 95 ++++
drivers/input/touchscreen/Kconfig | 31 ++
drivers/input/touchscreen/Makefile | 3 +
drivers/input/touchscreen/goodix_berlin.h | 159 ++++++
drivers/input/touchscreen/goodix_berlin_core.c | 595 +++++++++++++++++++++
drivers/input/touchscreen/goodix_berlin_i2c.c | 74 +++
drivers/input/touchscreen/goodix_berlin_spi.c | 177 ++++++
7 files changed, 1134 insertions(+)
---
base-commit: 2030579113a1b1b5bfd7ff24c0852847836d8fd1
change-id: 20230606-topic-goodix-berlin-upstream-initial-ba97e8ec8f4c
Best regards,
--
Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply
* [PATCH v11 1/4] dt-bindings: input: document Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-11-06 8:33 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bastien Nocera,
Hans de Goede, Henrik Rydberg, Jeff LaBundy, linux-arm-msm,
devicetree, linux-kernel, Neil Armstrong, Rob Herring
In-Reply-To: <20231106-topic-goodix-berlin-upstream-initial-v11-0-5c47e9707c03@linaro.org>
Document the Goodix GT9916 wich is part of the "Berlin" serie
of Touchscreen controllers IC from Goodix.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
.../bindings/input/touchscreen/goodix,gt9916.yaml | 95 ++++++++++++++++++++++
1 file changed, 95 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9916.yaml b/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9916.yaml
new file mode 100644
index 000000000000..d90f045ac06c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9916.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/goodix,gt9916.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Goodix Berlin series touchscreen controller
+
+description: The Goodix Berlin series of touchscreen controllers
+ be connected to either I2C or SPI buses.
+
+maintainers:
+ - Neil Armstrong <neil.armstrong@linaro.org>
+
+allOf:
+ - $ref: touchscreen.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+ enum:
+ - goodix,gt9916
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ avdd-supply:
+ description: Analog power supply regulator on AVDD pin
+
+ vddio-supply:
+ description: power supply regulator on VDDIO pin
+
+ spi-max-frequency: true
+ touchscreen-inverted-x: true
+ touchscreen-inverted-y: true
+ touchscreen-size-x: true
+ touchscreen-size-y: true
+ touchscreen-swapped-x-y: true
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - avdd-supply
+ - touchscreen-size-x
+ - touchscreen-size-y
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ touchscreen@5d {
+ compatible = "goodix,gt9916";
+ reg = <0x5d>;
+ interrupt-parent = <&gpio>;
+ interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
+ reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
+ avdd-supply = <&ts_avdd>;
+ touchscreen-size-x = <1024>;
+ touchscreen-size-y = <768>;
+ };
+ };
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ num-cs = <1>;
+ cs-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+ touchscreen@0 {
+ compatible = "goodix,gt9916";
+ reg = <0>;
+ interrupt-parent = <&gpio>;
+ interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
+ reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
+ avdd-supply = <&ts_avdd>;
+ spi-max-frequency = <1000000>;
+ touchscreen-size-x = <1024>;
+ touchscreen-size-y = <768>;
+ };
+ };
+
+...
--
2.34.1
^ permalink raw reply related
* [PATCH v11 3/4] Input: goodix-berlin - add I2C support for Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-11-06 8:33 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bastien Nocera,
Hans de Goede, Henrik Rydberg, Jeff LaBundy, linux-arm-msm,
devicetree, linux-kernel, Neil Armstrong
In-Reply-To: <20231106-topic-goodix-berlin-upstream-initial-v11-0-5c47e9707c03@linaro.org>
Add initial support for the new Goodix "Berlin" touchscreen ICs
over the I2C interface.
This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.
The current implementation only supports BerlinD, aka GT9916.
[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
Reviewed-by: Jeff LaBundy <jeff@labundy.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/input/touchscreen/Kconfig | 14 +++++
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_berlin_i2c.c | 74 +++++++++++++++++++++++++++
3 files changed, 89 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 950da599ae1a..cc7b88118158 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -419,6 +419,20 @@ config TOUCHSCREEN_GOODIX
config TOUCHSCREEN_GOODIX_BERLIN_CORE
tristate
+config TOUCHSCREEN_GOODIX_BERLIN_I2C
+ tristate "Goodix Berlin I2C touchscreen"
+ depends on I2C
+ select REGMAP_I2C
+ select TOUCHSCREEN_GOODIX_BERLIN_CORE
+ help
+ Say Y here if you have a Goodix Berlin IC connected to
+ your system via I2C.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called goodix_berlin_i2c.
+
config TOUCHSCREEN_HIDEEP
tristate "HiDeep Touch IC"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 2e2f3e70cd2c..7ef677cf7a10 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_TOUCHSCREEN_EXC3000) += exc3000.o
obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C) += goodix_berlin_i2c.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
diff --git a/drivers/input/touchscreen/goodix_berlin_i2c.c b/drivers/input/touchscreen/goodix_berlin_i2c.c
new file mode 100644
index 000000000000..4d5bcc101569
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin_i2c.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Goodix Berlin Touchscreen Driver
+ *
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_ts_berlin driver.
+ */
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "goodix_berlin.h"
+
+#define I2C_MAX_TRANSFER_SIZE 256
+
+static const struct regmap_config goodix_berlin_i2c_regmap_conf = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .max_raw_read = I2C_MAX_TRANSFER_SIZE,
+ .max_raw_write = I2C_MAX_TRANSFER_SIZE,
+};
+
+/* vendor & product left unassigned here, should probably be updated from fw info */
+static const struct input_id goodix_berlin_i2c_input_id = {
+ .bustype = BUS_I2C,
+};
+
+static int goodix_berlin_i2c_probe(struct i2c_client *client)
+{
+ struct regmap *regmap;
+ int error;
+
+ regmap = devm_regmap_init_i2c(client, &goodix_berlin_i2c_regmap_conf);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ error = goodix_berlin_probe(&client->dev, client->irq,
+ &goodix_berlin_i2c_input_id, regmap);
+ if (error)
+ return error;
+
+ return 0;
+}
+
+static const struct i2c_device_id goodix_berlin_i2c_id[] = {
+ { "gt9916", 0 },
+ { }
+};
+
+MODULE_DEVICE_TABLE(i2c, goodix_berlin_i2c_id);
+
+static const struct of_device_id goodix_berlin_i2c_of_match[] = {
+ { .compatible = "goodix,gt9916", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, goodix_berlin_i2c_of_match);
+
+static struct i2c_driver goodix_berlin_i2c_driver = {
+ .driver = {
+ .name = "goodix-berlin-i2c",
+ .of_match_table = goodix_berlin_i2c_of_match,
+ .pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
+ },
+ .probe = goodix_berlin_i2c_probe,
+ .id_table = goodix_berlin_i2c_id,
+};
+module_i2c_driver(goodix_berlin_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix Berlin I2C Touchscreen driver");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
--
2.34.1
^ permalink raw reply related
* [PATCH v11 2/4] Input: add core support for Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-11-06 8:33 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bastien Nocera,
Hans de Goede, Henrik Rydberg, Jeff LaBundy, linux-arm-msm,
devicetree, linux-kernel, Neil Armstrong
In-Reply-To: <20231106-topic-goodix-berlin-upstream-initial-v11-0-5c47e9707c03@linaro.org>
Add initial support for the new Goodix "Berlin" touchscreen ICs.
These touchscreen ICs support SPI, I2C and I3C interface, up to
10 finger touch, stylus and gestures events.
This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.
The current implementation only supports BerlinD, aka GT9916.
Support for advanced features like:
- Firmware & config update
- Stylus events
- Gestures events
- Previous revisions support (BerlinA or BerlinB)
is not included in current version.
The current support will work with currently flashed firmware
and config, and bail out if firmware or config aren't flashed yet.
[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
Reviewed-by: Jeff LaBundy <jeff@labundy.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/input/touchscreen/Kconfig | 3 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_berlin.h | 159 +++++++
drivers/input/touchscreen/goodix_berlin_core.c | 595 +++++++++++++++++++++++++
4 files changed, 758 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index e3e2324547b9..950da599ae1a 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -416,6 +416,9 @@ config TOUCHSCREEN_GOODIX
To compile this driver as a module, choose M here: the
module will be called goodix.
+config TOUCHSCREEN_GOODIX_BERLIN_CORE
+ tristate
+
config TOUCHSCREEN_HIDEEP
tristate "HiDeep Touch IC"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 62bd24f3ac8e..2e2f3e70cd2c 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_TOUCHSCREEN_EGALAX_SERIAL) += egalax_ts_serial.o
obj-$(CONFIG_TOUCHSCREEN_EXC3000) += exc3000.o
obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
new file mode 100644
index 000000000000..235f44947a28
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin.h
@@ -0,0 +1,159 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Goodix Touchscreen Driver
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_berlin_berlin driver.
+ */
+
+#ifndef __GOODIX_BERLIN_H_
+#define __GOODIX_BERLIN_H_
+
+#include <linux/gpio/consumer.h>
+#include <linux/input.h>
+#include <linux/input/touchscreen.h>
+#include <linux/regulator/consumer.h>
+#include <linux/sizes.h>
+
+#define GOODIX_BERLIN_MAX_TOUCH 10
+
+#define GOODIX_BERLIN_NORMAL_RESET_DELAY_MS 100
+
+#define GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN 8
+#define GOODIX_BERLIN_STATUS_OFFSET 0
+#define GOODIX_BERLIN_REQUEST_TYPE_OFFSET 2
+
+#define GOODIX_BERLIN_BYTES_PER_POINT 8
+#define GOODIX_BERLIN_COOR_DATA_CHECKSUM_SIZE 2
+#define GOODIX_BERLIN_COOR_DATA_CHECKSUM_MASK GENMASK(15, 0)
+
+/* Read n finger events */
+#define GOODIX_BERLIN_IRQ_READ_LEN(n) (GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN + \
+ (GOODIX_BERLIN_BYTES_PER_POINT * (n)) + \
+ GOODIX_BERLIN_COOR_DATA_CHECKSUM_SIZE)
+
+#define GOODIX_BERLIN_TOUCH_EVENT BIT(7)
+#define GOODIX_BERLIN_REQUEST_EVENT BIT(6)
+#define GOODIX_BERLIN_TOUCH_COUNT_MASK GENMASK(3, 0)
+
+#define GOODIX_BERLIN_REQUEST_CODE_RESET 3
+
+#define GOODIX_BERLIN_POINT_TYPE_MASK GENMASK(3, 0)
+#define GOODIX_BERLIN_POINT_TYPE_STYLUS_HOVER 1
+#define GOODIX_BERLIN_POINT_TYPE_STYLUS 3
+
+#define GOODIX_BERLIN_TOUCH_ID_MASK GENMASK(7, 4)
+
+#define GOODIX_BERLIN_DEV_CONFIRM_VAL 0xAA
+#define GOODIX_BERLIN_BOOTOPTION_ADDR 0x10000
+#define GOODIX_BERLIN_FW_VERSION_INFO_ADDR 0x10014
+
+#define GOODIX_BERLIN_IC_INFO_MAX_LEN SZ_1K
+#define GOODIX_BERLIN_IC_INFO_ADDR 0x10070
+
+struct goodix_berlin_fw_version {
+ u8 rom_pid[6];
+ u8 rom_vid[3];
+ u8 rom_vid_reserved;
+ u8 patch_pid[8];
+ u8 patch_vid[4];
+ u8 patch_vid_reserved;
+ u8 sensor_id;
+ u8 reserved[2];
+ __le16 checksum;
+} __packed;
+
+struct goodix_berlin_ic_info_version {
+ u8 info_customer_id;
+ u8 info_version_id;
+ u8 ic_die_id;
+ u8 ic_version_id;
+ __le32 config_id;
+ u8 config_version;
+ u8 frame_data_customer_id;
+ u8 frame_data_version_id;
+ u8 touch_data_customer_id;
+ u8 touch_data_version_id;
+ u8 reserved[3];
+} __packed;
+
+struct goodix_berlin_ic_info_feature {
+ __le16 freqhop_feature;
+ __le16 calibration_feature;
+ __le16 gesture_feature;
+ __le16 side_touch_feature;
+ __le16 stylus_feature;
+} __packed;
+
+struct goodix_berlin_ic_info_misc {
+ __le32 cmd_addr;
+ __le16 cmd_max_len;
+ __le32 cmd_reply_addr;
+ __le16 cmd_reply_len;
+ __le32 fw_state_addr;
+ __le16 fw_state_len;
+ __le32 fw_buffer_addr;
+ __le16 fw_buffer_max_len;
+ __le32 frame_data_addr;
+ __le16 frame_data_head_len;
+ __le16 fw_attr_len;
+ __le16 fw_log_len;
+ u8 pack_max_num;
+ u8 pack_compress_version;
+ __le16 stylus_struct_len;
+ __le16 mutual_struct_len;
+ __le16 self_struct_len;
+ __le16 noise_struct_len;
+ __le32 touch_data_addr;
+ __le16 touch_data_head_len;
+ __le16 point_struct_len;
+ __le16 reserved1;
+ __le16 reserved2;
+ __le32 mutual_rawdata_addr;
+ __le32 mutual_diffdata_addr;
+ __le32 mutual_refdata_addr;
+ __le32 self_rawdata_addr;
+ __le32 self_diffdata_addr;
+ __le32 self_refdata_addr;
+ __le32 iq_rawdata_addr;
+ __le32 iq_refdata_addr;
+ __le32 im_rawdata_addr;
+ __le16 im_readata_len;
+ __le32 noise_rawdata_addr;
+ __le16 noise_rawdata_len;
+ __le32 stylus_rawdata_addr;
+ __le16 stylus_rawdata_len;
+ __le32 noise_data_addr;
+ __le32 esd_addr;
+} __packed;
+
+struct goodix_berlin_touch_data {
+ u8 id;
+ u8 unused;
+ __le16 x;
+ __le16 y;
+ __le16 w;
+} __packed;
+
+struct goodix_berlin_core {
+ struct device *dev;
+ struct regmap *regmap;
+ struct regulator *avdd;
+ struct regulator *iovdd;
+ struct gpio_desc *reset_gpio;
+ struct touchscreen_properties props;
+ struct goodix_berlin_fw_version fw_version;
+ struct input_dev *input_dev;
+ int irq;
+
+ /* Runtime parameters extracted from IC_INFO buffer */
+ u32 touch_data_addr;
+};
+
+int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
+ struct regmap *regmap);
+
+extern const struct dev_pm_ops goodix_berlin_pm_ops;
+
+#endif
diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
new file mode 100644
index 000000000000..c66e2f0c6529
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin_core.c
@@ -0,0 +1,595 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Goodix Touchscreen Driver
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_ts_berlin driver.
+ */
+#include <asm/unaligned.h>
+#include <linux/bitfield.h>
+#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
+#include <linux/regmap.h>
+
+#include "goodix_berlin.h"
+
+/*
+ * Goodix "Berlin" Touchscreen IC driver
+ *
+ * This driver is distinct from goodix.c since hardware interface
+ * is different enough to require a new driver.
+ * None of the register address or data structure are close enough
+ * to the previous generations.
+ *
+ * Currently only handles Multitouch events with already
+ * programmed firmware and "config" for "Revision D" Berlin IC.
+ *
+ * Support is missing for:
+ * - ESD Management
+ * - Firmware update/flashing
+ * - "Config" update/flashing
+ * - Stylus Events
+ * - Gesture Events
+ * - Support for older revisions (A & B)
+ */
+
+static bool goodix_berlin_checksum_valid(const u8 *data, int size)
+{
+ u32 cal_checksum = 0;
+ u16 r_checksum;
+ u32 i;
+
+ if (size < GOODIX_BERLIN_COOR_DATA_CHECKSUM_SIZE)
+ return false;
+
+ for (i = 0; i < size - GOODIX_BERLIN_COOR_DATA_CHECKSUM_SIZE; i++)
+ cal_checksum += data[i];
+
+ r_checksum = get_unaligned_le16(&data[i]);
+
+ return FIELD_GET(GOODIX_BERLIN_COOR_DATA_CHECKSUM_MASK, cal_checksum) == r_checksum;
+}
+
+static bool goodix_berlin_is_dummy_data(struct goodix_berlin_core *cd,
+ const u8 *data, int size)
+{
+ int i;
+
+ /*
+ * If the device is missing or doesn't respond the buffer
+ * could be filled with bus default line state, 0x00 or 0xff,
+ * so declare success the first time we encounter neither.
+ */
+ for (i = 0; i < size; i++)
+ if (data[i] > 0 && data[i] < 0xff)
+ return false;
+
+ return true;
+}
+
+static int goodix_berlin_dev_confirm(struct goodix_berlin_core *cd)
+{
+ u8 tx_buf[8], rx_buf[8];
+ int retry = 3;
+ int error;
+
+ memset(tx_buf, GOODIX_BERLIN_DEV_CONFIRM_VAL, sizeof(tx_buf));
+ while (retry--) {
+ error = regmap_raw_write(cd->regmap, GOODIX_BERLIN_BOOTOPTION_ADDR, tx_buf,
+ sizeof(tx_buf));
+ if (error)
+ return error;
+
+ error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_BOOTOPTION_ADDR, rx_buf,
+ sizeof(rx_buf));
+ if (error)
+ return error;
+
+ if (!memcmp(tx_buf, rx_buf, sizeof(tx_buf)))
+ return 0;
+
+ usleep_range(5000, 5100);
+ }
+
+ dev_err(cd->dev, "device confirm failed, rx_buf: %*ph\n", 8, rx_buf);
+
+ return -EINVAL;
+}
+
+static int goodix_berlin_power_on(struct goodix_berlin_core *cd, bool on)
+{
+ int error = 0;
+
+ if (on) {
+ error = regulator_enable(cd->iovdd);
+ if (error) {
+ dev_err(cd->dev, "Failed to enable iovdd: %d\n", error);
+ return error;
+ }
+
+ /* Vendor waits 3ms for IOVDD to settle */
+ usleep_range(3000, 3100);
+
+ error = regulator_enable(cd->avdd);
+ if (error) {
+ dev_err(cd->dev, "Failed to enable avdd: %d\n", error);
+ goto err_iovdd_disable;
+ }
+
+ /* Vendor waits 15ms for IOVDD to settle */
+ usleep_range(15000, 15100);
+
+ gpiod_set_value(cd->reset_gpio, 0);
+
+ /* Vendor waits 4ms for Firmware to initialize */
+ usleep_range(4000, 4100);
+
+ error = goodix_berlin_dev_confirm(cd);
+ if (error)
+ goto err_dev_reset;
+
+ /* Vendor waits 100ms for Firmware to fully boot */
+ msleep(GOODIX_BERLIN_NORMAL_RESET_DELAY_MS);
+
+ return 0;
+ }
+
+err_dev_reset:
+ gpiod_set_value(cd->reset_gpio, 1);
+
+ regulator_disable(cd->avdd);
+
+err_iovdd_disable:
+ regulator_disable(cd->iovdd);
+
+ return error;
+}
+
+static int goodix_berlin_read_version(struct goodix_berlin_core *cd)
+{
+ u8 buf[sizeof(struct goodix_berlin_fw_version)];
+ int error;
+
+ error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_FW_VERSION_INFO_ADDR, buf, sizeof(buf));
+ if (error) {
+ dev_err(cd->dev, "error reading fw version, %d\n", error);
+ return error;
+ }
+
+ if (!goodix_berlin_checksum_valid(buf, sizeof(buf))) {
+ dev_err(cd->dev, "invalid fw version: checksum error\n");
+ return -EINVAL;
+ }
+
+ memcpy(&cd->fw_version, buf, sizeof(cd->fw_version));
+
+ return 0;
+}
+
+/* Only extract necessary data for runtime */
+static int goodix_berlin_convert_ic_info(struct goodix_berlin_core *cd,
+ const u8 *data, u16 length)
+{
+ struct goodix_berlin_ic_info_misc misc;
+ unsigned int offset = 0;
+ u8 param_num;
+
+ offset += sizeof(__le16); /* length */
+ offset += sizeof(struct goodix_berlin_ic_info_version);
+ offset += sizeof(struct goodix_berlin_ic_info_feature);
+
+ /* IC_INFO Parameters, variable width structure */
+ offset += 4 * sizeof(u8); /* drv_num, sen_num, button_num, force_num */
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* active_scan_rate_num */
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* mutual_freq_num*/
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* self_tx_freq_num */
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* self_rx_freq_num */
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* stylus_freq_num */
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset + sizeof(misc) > length)
+ goto invalid_offset;
+
+ /* goodix_berlin_ic_info_misc */
+ memcpy(&misc, &data[offset], sizeof(misc));
+
+ cd->touch_data_addr = le32_to_cpu(misc.touch_data_addr);
+
+ return 0;
+
+invalid_offset:
+ dev_err(cd->dev, "ic_info length is invalid (offset %d length %d)\n",
+ offset, length);
+ return -EINVAL;
+}
+
+static int goodix_berlin_get_ic_info(struct goodix_berlin_core *cd)
+{
+ __le16 length_raw;
+ u8 *afe_data;
+ u16 length;
+ int error;
+
+ afe_data = kzalloc(GOODIX_BERLIN_IC_INFO_MAX_LEN, GFP_KERNEL);
+ if (!afe_data)
+ return -ENOMEM;
+
+ error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_IC_INFO_ADDR,
+ &length_raw, sizeof(length_raw));
+ if (error) {
+ dev_err(cd->dev, "failed get ic info length, %d\n", error);
+ goto free_afe_data;
+ }
+
+ length = le16_to_cpu(length_raw);
+ if (length >= GOODIX_BERLIN_IC_INFO_MAX_LEN) {
+ dev_err(cd->dev, "invalid ic info length %d\n", length);
+ error = -EINVAL;
+ goto free_afe_data;
+ }
+
+ error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_IC_INFO_ADDR,
+ afe_data, length);
+ if (error) {
+ dev_err(cd->dev, "failed get ic info data, %d\n", error);
+ goto free_afe_data;
+ }
+
+ /* check whether the data is valid (ex. bus default values) */
+ if (goodix_berlin_is_dummy_data(cd, afe_data, length)) {
+ dev_err(cd->dev, "fw info data invalid\n");
+ error = -EINVAL;
+ goto free_afe_data;
+ }
+
+ if (!goodix_berlin_checksum_valid(afe_data, length)) {
+ dev_err(cd->dev, "fw info checksum error\n");
+ error = -EINVAL;
+ goto free_afe_data;
+ }
+
+ error = goodix_berlin_convert_ic_info(cd, afe_data, length);
+ if (error)
+ goto free_afe_data;
+
+ /* check some key info */
+ if (!cd->touch_data_addr) {
+ dev_err(cd->dev, "touch_data_addr is null\n");
+ error = -EINVAL;
+ goto free_afe_data;
+ }
+
+ return 0;
+
+free_afe_data:
+ kfree(afe_data);
+
+ return error;
+}
+
+static void goodix_berlin_parse_finger(struct goodix_berlin_core *cd,
+ const void *buf, int touch_num)
+{
+ const struct goodix_berlin_touch_data *touch_data = buf;
+ int i;
+
+ /* Report finger touches */
+ for (i = 0; i < touch_num; i++) {
+ unsigned int id;
+
+ id = FIELD_GET(GOODIX_BERLIN_TOUCH_ID_MASK, touch_data[i].id);
+
+ if (id >= GOODIX_BERLIN_MAX_TOUCH) {
+ dev_warn_ratelimited(cd->dev, "invalid finger id %d\n", id);
+ continue;
+ }
+
+ input_mt_slot(cd->input_dev, id);
+ input_mt_report_slot_state(cd->input_dev, MT_TOOL_FINGER, true);
+
+ touchscreen_report_pos(cd->input_dev, &cd->props,
+ __le16_to_cpu(touch_data[i].x),
+ __le16_to_cpu(touch_data[i].y),
+ true);
+ input_report_abs(cd->input_dev, ABS_MT_TOUCH_MAJOR,
+ __le16_to_cpu(touch_data[i].w));
+ }
+
+ input_mt_sync_frame(cd->input_dev);
+ input_sync(cd->input_dev);
+}
+
+static void goodix_berlin_touch_handler(struct goodix_berlin_core *cd,
+ const void *pre_buf, u32 pre_buf_len)
+{
+ u8 buffer[GOODIX_BERLIN_IRQ_READ_LEN(GOODIX_BERLIN_MAX_TOUCH)];
+ u8 point_type, touch_num;
+ int error;
+
+ /* copy pre-data to buffer */
+ memcpy(buffer, pre_buf, pre_buf_len);
+
+ touch_num = FIELD_GET(GOODIX_BERLIN_TOUCH_COUNT_MASK,
+ buffer[GOODIX_BERLIN_REQUEST_TYPE_OFFSET]);
+
+ if (touch_num > GOODIX_BERLIN_MAX_TOUCH) {
+ dev_warn(cd->dev, "invalid touch num %d\n", touch_num);
+ return;
+ }
+
+ if (touch_num) {
+ /* read more data if more than 2 touch events */
+ if (unlikely(touch_num > 2)) {
+ error = regmap_raw_read(cd->regmap,
+ cd->touch_data_addr + pre_buf_len,
+ &buffer[pre_buf_len],
+ (touch_num - 2) * GOODIX_BERLIN_BYTES_PER_POINT);
+ if (error) {
+ dev_err_ratelimited(cd->dev, "failed to get touch data, %d\n",
+ error);
+ return;
+ }
+ }
+
+ point_type = FIELD_GET(GOODIX_BERLIN_POINT_TYPE_MASK,
+ buffer[GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN]);
+
+ if (point_type == GOODIX_BERLIN_POINT_TYPE_STYLUS ||
+ point_type == GOODIX_BERLIN_POINT_TYPE_STYLUS_HOVER) {
+ dev_warn_once(cd->dev, "Stylus event type not handled\n");
+ return;
+ }
+
+ if (!goodix_berlin_checksum_valid(&buffer[GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN],
+ touch_num * GOODIX_BERLIN_BYTES_PER_POINT + 2)) {
+ dev_err(cd->dev, "touch data checksum error, data: %*ph\n",
+ touch_num * GOODIX_BERLIN_BYTES_PER_POINT + 2,
+ &buffer[GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN]);
+ return;
+ }
+ }
+
+ goodix_berlin_parse_finger(cd, &buffer[GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN],
+ touch_num);
+}
+
+static int goodix_berlin_request_handle_reset(struct goodix_berlin_core *cd)
+{
+ gpiod_set_value(cd->reset_gpio, 1);
+ usleep_range(2000, 2100);
+ gpiod_set_value(cd->reset_gpio, 0);
+
+ msleep(GOODIX_BERLIN_NORMAL_RESET_DELAY_MS);
+
+ return 0;
+}
+
+static irqreturn_t goodix_berlin_threadirq_func(int irq, void *data)
+{
+ struct goodix_berlin_core *cd = data;
+ u8 buf[GOODIX_BERLIN_IRQ_READ_LEN(2)];
+ u8 event_status;
+ int error;
+
+ /* First, read buffer with space for 2 touch events */
+ error = regmap_raw_read(cd->regmap, cd->touch_data_addr, buf,
+ GOODIX_BERLIN_IRQ_READ_LEN(2));
+ if (error) {
+ dev_err_ratelimited(cd->dev, "failed get event head data, %d\n", error);
+ return IRQ_HANDLED;
+ }
+
+ if (buf[GOODIX_BERLIN_STATUS_OFFSET] == 0)
+ return IRQ_HANDLED;
+
+ if (!goodix_berlin_checksum_valid(buf, GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN)) {
+ dev_warn_ratelimited(cd->dev, "touch head checksum err : %*ph\n",
+ GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN, buf);
+ return IRQ_HANDLED;
+ }
+
+ event_status = buf[GOODIX_BERLIN_STATUS_OFFSET];
+
+ if (event_status & GOODIX_BERLIN_TOUCH_EVENT)
+ goodix_berlin_touch_handler(cd, buf, GOODIX_BERLIN_IRQ_READ_LEN(2));
+
+ if (event_status & GOODIX_BERLIN_REQUEST_EVENT) {
+ switch (buf[GOODIX_BERLIN_REQUEST_TYPE_OFFSET]) {
+ case GOODIX_BERLIN_REQUEST_CODE_RESET:
+ if (cd->reset_gpio)
+ goodix_berlin_request_handle_reset(cd);
+ break;
+
+ default:
+ dev_warn(cd->dev, "unsupported request code 0x%x\n",
+ buf[GOODIX_BERLIN_REQUEST_TYPE_OFFSET]);
+ }
+ }
+
+ /* Clear up status field */
+ regmap_write(cd->regmap, cd->touch_data_addr, 0);
+
+ return IRQ_HANDLED;
+}
+
+static int goodix_berlin_input_dev_config(struct goodix_berlin_core *cd,
+ const struct input_id *id)
+{
+ struct input_dev *input_dev;
+ int error;
+
+ input_dev = devm_input_allocate_device(cd->dev);
+ if (!input_dev)
+ return -ENOMEM;
+
+ cd->input_dev = input_dev;
+ input_set_drvdata(input_dev, cd);
+
+ input_dev->name = "Goodix Berlin Capacitive TouchScreen";
+ input_dev->phys = "input/ts";
+
+ input_dev->id = *id;
+
+ input_set_abs_params(cd->input_dev, ABS_MT_POSITION_X, 0, SZ_64K - 1, 0, 0);
+ input_set_abs_params(cd->input_dev, ABS_MT_POSITION_Y, 0, SZ_64K - 1, 0, 0);
+ input_set_abs_params(cd->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+
+ touchscreen_parse_properties(cd->input_dev, true, &cd->props);
+
+ error = input_mt_init_slots(cd->input_dev, GOODIX_BERLIN_MAX_TOUCH,
+ INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
+ if (error)
+ return error;
+
+ error = input_register_device(cd->input_dev);
+ if (error)
+ return error;
+
+ return 0;
+}
+
+static int goodix_berlin_pm_suspend(struct device *dev)
+{
+ struct goodix_berlin_core *cd = dev_get_drvdata(dev);
+
+ disable_irq(cd->irq);
+
+ return goodix_berlin_power_on(cd, false);
+}
+
+static int goodix_berlin_pm_resume(struct device *dev)
+{
+ struct goodix_berlin_core *cd = dev_get_drvdata(dev);
+ int error;
+
+ error = goodix_berlin_power_on(cd, true);
+ if (error)
+ return error;
+
+ enable_irq(cd->irq);
+
+ return 0;
+}
+
+EXPORT_GPL_SIMPLE_DEV_PM_OPS(goodix_berlin_pm_ops,
+ goodix_berlin_pm_suspend,
+ goodix_berlin_pm_resume);
+
+static void goodix_berlin_power_off(void *data)
+{
+ struct goodix_berlin_core *cd = data;
+
+ goodix_berlin_power_on(cd, false);
+}
+
+int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
+ struct regmap *regmap)
+{
+ struct goodix_berlin_core *cd;
+ int error;
+
+ if (irq <= 0) {
+ dev_err(dev, "Missing interrupt number\n");
+ return -EINVAL;
+ }
+
+ cd = devm_kzalloc(dev, sizeof(*cd), GFP_KERNEL);
+ if (!cd)
+ return -ENOMEM;
+
+ cd->dev = dev;
+ cd->regmap = regmap;
+ cd->irq = irq;
+
+ cd->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(cd->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(cd->reset_gpio),
+ "Failed to request reset gpio\n");
+
+ cd->avdd = devm_regulator_get(dev, "avdd");
+ if (IS_ERR(cd->avdd))
+ return dev_err_probe(dev, PTR_ERR(cd->avdd),
+ "Failed to request avdd regulator\n");
+
+ cd->iovdd = devm_regulator_get(dev, "iovdd");
+ if (IS_ERR(cd->iovdd))
+ return dev_err_probe(dev, PTR_ERR(cd->iovdd),
+ "Failed to request iovdd regulator\n");
+
+ error = goodix_berlin_power_on(cd, true);
+ if (error) {
+ dev_err(dev, "failed power on");
+ return error;
+ }
+
+ error = devm_add_action_or_reset(dev, goodix_berlin_power_off, cd);
+ if (error)
+ return error;
+
+ error = goodix_berlin_read_version(cd);
+ if (error) {
+ dev_err(dev, "failed to get version info");
+ return error;
+ }
+
+ error = goodix_berlin_get_ic_info(cd);
+ if (error) {
+ dev_err(dev, "invalid ic info, abort");
+ return error;
+ }
+
+ error = goodix_berlin_input_dev_config(cd, id);
+ if (error) {
+ dev_err(dev, "failed set input device");
+ return error;
+ }
+
+ error = devm_request_threaded_irq(dev, irq, NULL,
+ goodix_berlin_threadirq_func,
+ IRQF_ONESHOT, "goodix-berlin", cd);
+ if (error) {
+ dev_err(dev, "request threaded irq failed: %d\n", error);
+ return error;
+ }
+
+ dev_set_drvdata(dev, cd);
+
+ dev_dbg(dev, "Goodix Berlin %s Touchscreen Controller", cd->fw_version.patch_pid);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(goodix_berlin_probe);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix Berlin Core Touchscreen driver");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
--
2.34.1
^ permalink raw reply related
* [PATCH v11 4/4] Input: goodix-berlin - add SPI support for Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-11-06 8:33 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bastien Nocera,
Hans de Goede, Henrik Rydberg, Jeff LaBundy, linux-arm-msm,
devicetree, linux-kernel, Neil Armstrong
In-Reply-To: <20231106-topic-goodix-berlin-upstream-initial-v11-0-5c47e9707c03@linaro.org>
Add initial support for the new Goodix "Berlin" touchscreen ICs
over the SPI interface.
The driver doesn't use the regmap_spi code since the SPI messages
needs to be prefixed, thus this custom regmap code.
This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.
The current implementation only supports BerlinD, aka GT9916.
[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
Reviewed-by: Jeff LaBundy <jeff@labundy.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/input/touchscreen/Kconfig | 14 ++
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_berlin_spi.c | 177 ++++++++++++++++++++++++++
3 files changed, 192 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index cc7b88118158..c821fe3ee794 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -433,6 +433,20 @@ config TOUCHSCREEN_GOODIX_BERLIN_I2C
To compile this driver as a module, choose M here: the
module will be called goodix_berlin_i2c.
+config TOUCHSCREEN_GOODIX_BERLIN_SPI
+ tristate "Goodix Berlin SPI touchscreen"
+ depends on SPI_MASTER
+ select REGMAP
+ select TOUCHSCREEN_GOODIX_BERLIN_CORE
+ help
+ Say Y here if you have a Goodix Berlin IC connected to
+ your system via SPI.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called goodix_berlin_spi.
+
config TOUCHSCREEN_HIDEEP
tristate "HiDeep Touch IC"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 7ef677cf7a10..a81cb5aa21a5 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C) += goodix_berlin_i2c.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_SPI) += goodix_berlin_spi.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
diff --git a/drivers/input/touchscreen/goodix_berlin_spi.c b/drivers/input/touchscreen/goodix_berlin_spi.c
new file mode 100644
index 000000000000..502b143e9e05
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin_spi.c
@@ -0,0 +1,177 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Goodix Berlin Touchscreen Driver
+ *
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_ts_berlin driver.
+ */
+#include <asm/unaligned.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+#include "goodix_berlin.h"
+
+#define GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN 1
+#define GOODIX_BERLIN_REGISTER_WIDTH 4
+#define GOODIX_BERLIN_SPI_READ_DUMMY_LEN 3
+#define GOODIX_BERLIN_SPI_READ_PREFIX_LEN (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
+ GOODIX_BERLIN_REGISTER_WIDTH + \
+ GOODIX_BERLIN_SPI_READ_DUMMY_LEN)
+#define GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
+ GOODIX_BERLIN_REGISTER_WIDTH)
+
+#define GOODIX_BERLIN_SPI_WRITE_FLAG 0xF0
+#define GOODIX_BERLIN_SPI_READ_FLAG 0xF1
+
+static int goodix_berlin_spi_read(void *context, const void *reg_buf,
+ size_t reg_size, void *val_buf,
+ size_t val_size)
+{
+ struct spi_device *spi = context;
+ struct spi_transfer xfers;
+ struct spi_message spi_msg;
+ const u32 *reg = reg_buf; /* reg is stored as native u32 at start of buffer */
+ u8 *buf;
+ int error;
+
+ if (reg_size != GOODIX_BERLIN_REGISTER_WIDTH)
+ return -EINVAL;
+
+ buf = kzalloc(GOODIX_BERLIN_SPI_READ_PREFIX_LEN + val_size, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ spi_message_init(&spi_msg);
+ memset(&xfers, 0, sizeof(xfers));
+
+ /* buffer format: 0xF1 + addr(4bytes) + dummy(3bytes) + data */
+ buf[0] = GOODIX_BERLIN_SPI_READ_FLAG;
+ put_unaligned_be32(*reg, buf + GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN);
+ memset(buf + GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + GOODIX_BERLIN_REGISTER_WIDTH,
+ 0xff, GOODIX_BERLIN_SPI_READ_DUMMY_LEN);
+
+ xfers.tx_buf = buf;
+ xfers.rx_buf = buf;
+ xfers.len = GOODIX_BERLIN_SPI_READ_PREFIX_LEN + val_size;
+ xfers.cs_change = 0;
+ spi_message_add_tail(&xfers, &spi_msg);
+
+ error = spi_sync(spi, &spi_msg);
+ if (error < 0)
+ dev_err(&spi->dev, "spi transfer error, %d", error);
+ else
+ memcpy(val_buf, buf + GOODIX_BERLIN_SPI_READ_PREFIX_LEN, val_size);
+
+ kfree(buf);
+ return error;
+}
+
+static int goodix_berlin_spi_write(void *context, const void *data,
+ size_t count)
+{
+ unsigned int len = count - GOODIX_BERLIN_REGISTER_WIDTH;
+ struct spi_device *spi = context;
+ struct spi_transfer xfers;
+ struct spi_message spi_msg;
+ const u32 *reg = data; /* reg is stored as native u32 at start of buffer */
+ u8 *buf;
+ int error;
+
+ buf = kzalloc(GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN + len, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ spi_message_init(&spi_msg);
+ memset(&xfers, 0, sizeof(xfers));
+
+ buf[0] = GOODIX_BERLIN_SPI_WRITE_FLAG;
+ put_unaligned_be32(*reg, buf + GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN);
+ memcpy(buf + GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN,
+ data + GOODIX_BERLIN_REGISTER_WIDTH, len);
+
+ xfers.tx_buf = buf;
+ xfers.len = GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN + len;
+ xfers.cs_change = 0;
+ spi_message_add_tail(&xfers, &spi_msg);
+
+ error = spi_sync(spi, &spi_msg);
+ if (error < 0)
+ dev_err(&spi->dev, "spi transfer error, %d", error);
+
+ kfree(buf);
+ return error;
+}
+
+static const struct regmap_config goodix_berlin_spi_regmap_conf = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .read = goodix_berlin_spi_read,
+ .write = goodix_berlin_spi_write,
+};
+
+/* vendor & product left unassigned here, should probably be updated from fw info */
+static const struct input_id goodix_berlin_spi_input_id = {
+ .bustype = BUS_SPI,
+};
+
+static int goodix_berlin_spi_probe(struct spi_device *spi)
+{
+ struct regmap_config regmap_config;
+ struct regmap *regmap;
+ size_t max_size;
+ int error = 0;
+
+ spi->mode = SPI_MODE_0;
+ spi->bits_per_word = 8;
+ error = spi_setup(spi);
+ if (error)
+ return error;
+
+ max_size = spi_max_transfer_size(spi);
+
+ regmap_config = goodix_berlin_spi_regmap_conf;
+ regmap_config.max_raw_read = max_size - GOODIX_BERLIN_SPI_READ_PREFIX_LEN;
+ regmap_config.max_raw_write = max_size - GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN;
+
+ regmap = devm_regmap_init(&spi->dev, NULL, spi, ®map_config);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ error = goodix_berlin_probe(&spi->dev, spi->irq,
+ &goodix_berlin_spi_input_id, regmap);
+ if (error)
+ return error;
+
+ return 0;
+}
+
+static const struct spi_device_id goodix_berlin_spi_ids[] = {
+ { "gt9916" },
+ { },
+};
+MODULE_DEVICE_TABLE(spi, goodix_berlin_spi_ids);
+
+static const struct of_device_id goodix_berlin_spi_of_match[] = {
+ { .compatible = "goodix,gt9916", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, goodix_berlin_spi_of_match);
+
+static struct spi_driver goodix_berlin_spi_driver = {
+ .driver = {
+ .name = "goodix-berlin-spi",
+ .of_match_table = goodix_berlin_spi_of_match,
+ .pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
+ },
+ .probe = goodix_berlin_spi_probe,
+ .id_table = goodix_berlin_spi_ids,
+};
+module_spi_driver(goodix_berlin_spi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix Berlin SPI Touchscreen driver");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: Marco Felsch @ 2023-11-06 12:29 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Kamel Bouhara, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, linux-input, linux-kernel, devicetree,
mark.satterthwaite, bartp, hannah.rossiter, Thomas Petazzoni,
Gregory Clement, bsp-development.geo
In-Reply-To: <ZT9uJMExvf7B0gtR@google.com>
Hi Dimitry,
On 23-10-30, Dmitry Torokhov wrote:
> On Sun, Oct 22, 2023 at 09:35:29PM +0200, Kamel Bouhara wrote:
> > On Fri, Oct 20, 2023 at 02:03:10PM +0200, Marco Felsch wrote:
> > > > +
> > > > +static int
> > > > +axiom_i2c_write(struct i2c_client *client, u8 usage, u8 page, u8 *buf, u16 len)
> > > > +{
> > > > + struct axiom_data *ts = i2c_get_clientdata(client);
> > > > + struct axiom_cmd_header cmd_header;
> > > > + struct i2c_msg msg[2];
> > > > + int ret;
> > > > +
> > > > + cmd_header.target_address = cpu_to_le16(usage_to_target_address(ts, usage, page, 0));
> > > > + cmd_header.length = cpu_to_le16(len);
> > > > + cmd_header.read = 0;
> > > > +
> > > > + msg[0].addr = client->addr;
> > > > + msg[0].flags = 0;
> > > > + msg[0].len = sizeof(cmd_header);
> > > > + msg[0].buf = (u8 *)&cmd_header;
> > > > +
> > > > + msg[1].addr = client->addr;
> > > > + msg[1].flags = 0;
> > > > + msg[1].len = len;
> > > > + msg[1].buf = (char *)buf;
> > >
> > > Please check the "comms protocol app note", for write it is not allowed
> > > to send a stop, so the whole data must be send in one i2c_msg.
> > >
> >
> > Well I only have the "Programmer's Guide", I'll have to check that as it
> > really seems to works as it yet.
>
> As far as I know we only send "stop" on the last message in a sequence
> of messages in i2c_transfer() unless it is explicitly requested with
> I2C_M_STOP flag.
You're right, I re-checked this again but this approach is still wrong
since the protocol does not allow sending the payload as separate
message. The payload must be send as one message starting with the
i2c-address of the touchconroller followed by the target register
address and how many bytes should be written followed by the payload.
> ...
> > > > +
> > > > +static void axiom_i2c_remove(struct i2c_client *client)
> > > > +{
> > > > + struct axiom_data *ts = i2c_get_clientdata(client);
> > > > +
> > > > + input_unregister_device(ts->input_dev);
> > >
> > > This can be part of devm_add_action_or_reset() and we could remove the
> > > .remove() callback for this driver.
> > >
> >
> > Sure, thanks for the tips :)!
>
> Actually input devices allocated with devm do not need to be explicitly
> inregistered, so you do not need to bother with
> devm_add_action_or_reset() and simply delete axiom_i2c_remove().
Ah I see.. so all devm_alloced...() device don't need that explicite
unregister. Thanks.
Regards,
Marco
>
> Thanks.
>
> --
> Dmitry
>
^ permalink raw reply
* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
From: David Revoy @ 2023-11-06 13:17 UTC (permalink / raw)
To: Illia Ostapyshyn
Cc: jkosina, benjamin.tissoires, jason.gerecke, jose.exposito89,
linux-input, linux-kernel, nils, peter.hutterer, ping.cheng,
bagasdotme
In-Reply-To: <20231103200524.53930-1-ostapyshyn@sra.uni-hannover.de>
Hi Illia, Jiri, Bagas,
Thanks to Jiri for forwarding my request for help to this mailing list.
I apologise in advance to Bagas and you all for not being able to properly configure my email client to follow the correct etiquette (Protonmail, unsuitable for kernel development, but we've made some progress with them on Mastodon on the encryption issue [1]).
Thanks to Illia for the detailed explanation. Thanks also for fixing it, and for explaining how the fix broke my workflow, and for your kind words about the situation. I appreciate it.
However, after reading your reply, I still have my doubts about the preference to put an eraser on the top button of the pen by default. But after a few days and re-reading your first sentence "The XP-Pen hardware reports the Eraser usage for the upper stylus button.", I *think* I understood it: this is an internal signal that is sent by the device itself, and is therefore a specification that has to be followed, right? In this case, it makes sense for a generic driver to follow such a signal if it is sent by the hardware.
Having said that, behaving like this still makes no sense in one case: I'm thinking of my other display tablet, the XPPEN 16 Artist Pro (Gen2). In this case, the stylus has the top button as an eraser, and an eraser tip as well, as you can see in this photo [2]. Was it also a decision by XPPEN to include this signal in the hardware, knowing that they already had an eraser tip? In this case, please let me know, as it sounds like a hardware problem at the design stage and I have probably a way of passing on the information to their technical team.
> You can still remap the eraser button to a right click using xsetwacom:
> xsetwacom set "UGTABLET 24 inch PenDisplay eraser" "Button" "1" "3"
Unfortunately, it doesn't work.
Firstly, such tablets are often connected to a computer that already has a display. So each device (pen/eraser) needs to be mapped to the correct display and set to the correct 'Area' for calibration. A better syntax in my case to test your workaround is written like this:
tableteraser="UGTABLET 24 inch PenDisplay eraser"
xsetwacom set "$tableteraser" MapToOutput "DisplayPort-2"
xsetwacom set "$tableteraser" Area 100 120 32794 32797
xsetwacom set "$tableteraser" "Button" "1" "3"
Secondly, forwarding the eraser button 1 to button 3 (a right click) in xsetwacom doesn't work.
Pressing the button switches the device to an eraser and no right click happens. You'll need to hold down the button and click with the tip of the pen to create a right-click event.
In a digital painting session in Krita, the software will switch your device to an eraser tool preset while you hold down the button, and the feedback on the canvas will be the eraser cursor. You'll then have to click "with the eraser" to get the right-click that triggers the pop-up palette [3].
I'd also like to mention that if you haven't correctly mapped and calibrated your eraser device with xsetwacom, as I did in the code snippet above, the cursor will by default jump to a different location when you hold down the eraser button. It can be on a different display.
Finally, in the case of the XPPEN 16 Artist Pro (Gen2) with a real eraser device (Photo, [2]), the setting 'xsetwacom set "$tableteraser" "Button" "1" "3"' will affect both erasers. Flipping the stylus on the eraser side and entering in 'hover' mode will return a correct eraser, but as soon as you start erasing with the tip, a right click will also be entered.
> You can also do this with "xinput set-button-map", which works for libinput as well.
$ xinput list
⎡ Virtual core pointer
⎜ ↳ UGTABLET 24 inch PenDisplay Mouse id=8 [slave pointer (2)]
⎜ ↳ UGTABLET 24 inch PenDisplay stylus id=10 [slave pointer (2)]
⎜ ↳ UGTABLET 24 inch PenDisplay eraser id=17 [slave pointer (2)]
[...]
$ xinput get-button-map 17
1 2 3 4 5 6 7 8
$ xinput set-button-map 17 3 3 3 3 3 3 3 3
$ xinput get-button-map 17
3 3 3 3 3 3 3 3
Even after that, it doesn't work. My original article [4] mentions in the last paragraph that I have tested almost all methods, and this was one of them. Even a udev rule doesn't fix it. For reference, xinput produces the same behaviour as xsetwacom: you have to hold the button (it triggers an eraser device switch) then click with the tip to get a right-click...
> We have tested this with several XP-Pen devices, including Artist 24.
Sorry if my tests show something different. Maybe there is something wrong with my GNU/Linux operating system (Fedora 38 KDE spin). Let me know if you have any idea why I get these results and guide me with what distro I should switch to get a similar obsevation than you do (and also to report the issue to the Fedora team).
---
All in all (and in the case that my observations and tests are correct), I still stand by the points that I made in my blog post:
- I don't have any way to customise the hardcoded eraser buttons in userspace and **it is a problem**: for devices without a touchscreen or mouse, not having access to a right-click by default on the device is a handicap. Many actions on the D.E. and applications use the right click. The preference to replace it with an 'eraser switch' action by default is IMHO highly debatable here.
- In the case of a pen that already has an eraser tip on the other side of the device [2], it makes no sense to exchange the right-click top button for another way to erase. Also in userspace, there seems to be no way to distinguish between the two buttons (the eraser tip and the eraser button). All actions from xsetwacom, xinput/libinput target the two eraser devices, and they target them unsuccessfully too.
[1]: https://mastodon.social/@protonmail/111346195283677411
[2]: https://www.davidrevoy.com/data/images/blog/2023/2023-11-01_linux-kernel-broke-my-stylus_xp-pen-artist-pro-16-gen2_net.jpg
[3]: https://docs.krita.org/en/reference_manual/popup-palette.html
[4]: https://www.davidrevoy.com/article995/how-a-kernel-update-broke-my-stylus-need-help
------- Original Message -------
On Friday, November 3rd, 2023 at 21:05, Illia Ostapyshyn <ostapyshyn@sra.uni-hannover.de> wrote:
> Hello David, Hello Jiri,
>
> The XP-Pen hardware reports the Eraser usage for the upper stylus button.
> Generally, styli report Invert usages when erasing, as described in [1].
> XP-Pen digitizers, however, tend to omit them.
>
> The generic driver maps the Eraser usage to BTN_TOUCH and the Invert
> usage to BTN_TOOL_RUBBER. Pens conforming to [1] send the Invert usage
> first (switching the tool to BTN_TOOL_RUBBER) followed by Eraser, which
> appears in userspace as a BTN_TOUCH event with the rubber tool set.
>
> Due to an oversight, devices not reporting Invert had the BTN_TOOL_RUBBER
> event masked. This has caused the kernel to send only BTN_TOUCH events
> without the tool switch when erasing.
>
> The situation got worse with refactoring done in 87562fcd1342. An eraser
> without Invert caused the hidinput_hid_event state machine to get stuck
> with BTN_TOOL_RUBBER internally (due to it being masked). For the
> userspace, this looked as if the pen was never hovering again, rendering
> it unusable until the next reset. 276e14e6c3 fixes this by adding
> support for digitizers that do not report Invert usages when erasing.
>
> ---
>
> David, we are sorry that our patch broke your workflow. However,
> forwarding hardware events as-is to the userspace has always been the
> intended behavior, with a kernel bug preventing it so far. You can still
> remap the eraser button to a right click using xsetwacom:
>
> xsetwacom set "UGTABLET 24 inch PenDisplay eraser" "Button" "1" "3"
>
> Replace the device name with the corresponding eraser device from
> "xsetwacom list devices". You can also do this with "xinput set-button-map",
> which works for libinput as well. We have tested this with several
> XP-Pen devices, including Artist 24.
>
> [1] https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
^ permalink raw reply
* PSA: migrating linux-input to new vger infrastructure
From: Konstantin Ryabitsev @ 2023-11-06 13:26 UTC (permalink / raw)
To: linux-input
Good day!
I plan to migrate the linux-input@vger.kernel.org list to the new
infrastructure this week. We're still doing it list-by-list to make sure that
we don't run into scaling issues with the new infra.
The migration will be performed live and should not require any downtime.
There will be no changes to how anyone interacts with the list after
migration is completed, so no action is required on anyone's part.
Please let me know if you have any concerns.
Best wishes,
-K
^ permalink raw reply
* [PATCH] Input: atkbd - Skip ATKBD_CMD_GETID in translated mode
From: Hans de Goede @ 2023-11-06 15:54 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Hans de Goede, linux-input, Shang Ye, gurevitch, Egor Ignatov,
Anton Zhilyaev
There have been multiple reports of keyboard issues on recent laptop models
which can be worked around by setting i8042.dumbkbd, with the downside
being this breaks the capslock LED.
It seems that these issues are caused by recent laptops getting confused by
ATKBD_CMD_GETID. Rather then adding and endless growing list of quirks for
this, lets just skip ATKBD_CMD_GETID alltogether when in translated mode.
The main goal of sending ATKBD_CMD_GETID is to skip binding to ps/2
mice/touchpads and those are never used in translated mode.
Examples of laptop models which benefit from skipping ATKBD_CMD_GETID:
* "HP Laptop 15s-fq2xxx", "HP laptop 15s-fq4xxx" and "HP Laptop 15-dy2xxx"
models the kbd stops working for the first 2 - 5 minutes after boot
(waiting for EC watchdog reset?)
* On "HP Spectre x360 13-aw2xxx" atkbd fails to probe the keyboard
* At least 9 different Lenovo models have issues with ATKBD_CMD_GETID, see:
https://github.com/yaescallop/atkbd-nogetid
Note this also removes the "NCD terminal keyboards are only supported on
non-translating controllers." warning since that code is now unreachable.
This has been tested on:
1. A MSI B550M PRO-VDH WIFI desktop, where the i8042 controller is not
in translated mode when no keyboard is plugged in and with a ps/2 kbd
a "AT Translated Set 2 keyboard" /dev/input/event# node shows up
2. A Dell Latitude 9420 (always has a "AT Translated Set 2 keyboard")
3. A Lenovo ThinkPad X1 Yoga gen 8 (idem)
Reported-by: Shang Ye <yesh25@mail2.sysu.edu.cn>
Closes: https://lore.kernel.org/linux-input/886D6167733841AE+20231017135318.11142-1-yesh25@mail2.sysu.edu.cn/
Closes: https://github.com/yaescallop/atkbd-nogetid
Reported-by: gurevitch <mail@gurevit.ch>
Closes: https://lore.kernel.org/linux-input/2iAJTwqZV6lQs26cTb38RNYqxvsink6SRmrZ5h0cBUSuf9NT0tZTsf9fEAbbto2maavHJEOP8GA1evlKa6xjKOsaskDhtJWxjcnrgPigzVo=@gurevit.ch/
Reported-by: Egor Ignatov <egori@altlinux.org>
Closes: https://lore.kernel.org/all/20210609073333.8425-1-egori@altlinux.org/
Reported-by: Anton Zhilyaev <anton@cpp.in>
Closes: https://lore.kernel.org/linux-input/20210201160336.16008-1-anton@cpp.in/
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2086156
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Note this supersedes my previous atkbd series:
https://lore.kernel.org/linux-input/20231005201544.26983-1-hdegoede@redhat.com/
---
drivers/input/keyboard/atkbd.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index c92e544c792d..c5ffa79548d5 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -786,6 +786,20 @@ static int atkbd_probe(struct atkbd *atkbd)
"keyboard reset failed on %s\n",
ps2dev->serio->phys);
+/*
+ * On many modern laptops ATKBD_CMD_GETID may cause problems, on these laptops
+ * the controller is always in translated mode. In this mode mice/touchpads will
+ * not work. So in this case simply assume a keyboard is connected to avoid
+ * confusing some laptop keyboards.
+ *
+ * Using a fake keyboard id is ok in translated mode. Only atkbd_select_set()
+ * checks atkbd->id and in translated mode that is a no-op.
+ */
+ if (atkbd->translated) {
+ atkbd->id = 0xabba;
+ goto deactivate;
+ }
+
/*
* Then we check the keyboard ID. We should get 0xab83 under normal conditions.
* Some keyboards report different values, but the first byte is always 0xab or
@@ -813,13 +827,7 @@ static int atkbd_probe(struct atkbd *atkbd)
atkbd->id = (param[0] << 8) | param[1];
- if (atkbd->id == 0xaca1 && atkbd->translated) {
- dev_err(&ps2dev->serio->dev,
- "NCD terminal keyboards are only supported on non-translating controllers. "
- "Use i8042.direct=1 to disable translation.\n");
- return -1;
- }
-
+deactivate:
/*
* Make sure nothing is coming from the keyboard and disturbs our
* internal state.
--
2.41.0
^ permalink raw reply related
* Re: [PATCH] HID:i2c-hid:goodix:Modify post_power_delay_ms to avoid touchscreen not working
From: Doug Anderson @ 2023-11-06 16:40 UTC (permalink / raw)
To: xiazhengqiao
Cc: jikos, benjamin.tissoires, mka, fshao, linux-input, linux-kernel,
xuxinxiong
In-Reply-To: <20231105063602.10737-1-xiazhengqiao@huaqin.corp-partner.google.com>
Hi,
On Sat, Nov 4, 2023 at 11:36 PM xiazhengqiao
<xiazhengqiao@huaqin.corp-partner.google.com> wrote:
>
> For "goodix,gt7375p" touchscreen, When the device restarts,
> the reset pin of the touchscreen will be pulled down by 10ms,
> but this time will make the touchscreen have a probability of
> not working properly. Increase post_power_delay_ms to 20ms,
> so that the reset pin is pulled down 20ms, and toucsreen works fine.
s/toucsreen/touchscreen
> Signed-off-by: xiazhengqiao <xiazhengqiao@huaqin.corp-partner.google.com>
> ---
> drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
> index f1597ad67e7c..caabf7a62cde 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
> @@ -111,7 +111,7 @@ static int i2c_hid_of_goodix_probe(struct i2c_client *client)
> }
>
> static const struct goodix_i2c_hid_timing_data goodix_gt7375p_timing_data = {
> - .post_power_delay_ms = 10,
> + .post_power_delay_ms = 20,
Do you actually have a Goodix "GT7375P" touchscreen, or do you have
some other touchscreen that is using the Goodix GT7375P compatible
string? As far as I know the 10 ms here came from the Goodix GT7375P
datasheet and has been working fine with devices that have Goodix
GT7373P. Has this always been wrong for the Goodix GT7373P, or (as I
suspect) do you actually have a different touchscreen and for that
different touchscreen you need a longer delay?
-Doug
^ 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