Closed Danct12 closed 2 years ago
1st commit (with touchscreen) should have a description line. Template: "The ${device} is using a ${ts_model} touchscreen, add support for it" <- should be enough IMO
1st commit (with touchscreen) should have a description line. Template: "The ${device} is using a ${ts_model} touchscreen, add support for it" <- should be enough IMO
For now, I've added a FIXME to the commit until we can figure out the exact touchscreen is used in the device.
Which kernel driver (config) is used to try this?
stupid me, it's the one added in the target branch
I see,
enum nt36xxx_chips {
NT36525_IC = 0,
NT36672A_IC,
NT36676F_IC,
NT36772_IC,
NT36870_IC,
NTMAX_IC,
};
...
static const struct of_device_id nt36xxx_of_match[] = {
{ .compatible = "novatek,nt36525" },
{ }
};
So, driver supports several chips, I dunno why, but I always thought that lavender has 36672a (same as panel)
Correct way would be to add debug print to https://github.com/sdm660-mainline/linux/blob/qcom-sdm660-5.10.y/drivers/input/touchscreen/nt36xxx.c#L576 and figure out which one there is :)
So I added this
diff --git a/drivers/input/touchscreen/nt36xxx.c b/drivers/input/touchscreen/nt36xxx.c
index a572d2b87464..9936e15acb1e 100644
--- a/drivers/input/touchscreen/nt36xxx.c
+++ b/drivers/input/touchscreen/nt36xxx.c
@@ -5,6 +5,7 @@
* Copyright (C) 2010 - 2017 Novatek, Inc.
* Copyright (C) 2020 AngeloGioacchino Del Regno <kholk11@gmail.com>
*/
+#define DEBUG
#include <linux/delay.h>
#include <linux/gpio/consumer.h>
@@ -618,6 +619,7 @@ static int nt36xxx_i2c_chip_version_init(struct nt36xxx_i2c *ts)
if (i == NT36XXX_ID_LEN_MAX) {
mapid = trim_id_table[list].mapid;
ts->mmap = &nt36xxx_memory_maps[mapid];
+ printk(KERN_ERR "novatek_ts: found matching map ID: %d\n", mapid);
return 0;
}
And I see
ID=1 means that NT36672A_IC
enum value is matched. You can reword the commit message properly:
"Xiaomi Redmi Note 7 is using a Novatek 36672A touchscreen, add support for it in device tree."
Interacting with wled changes brightness indeed, but results in the following trace in dmesg:
[ 499.906317] ------------[ cut here ]------------
[ 499.906478] Unbalanced enable for IRQ 39
[ 499.910118] WARNING: CPU: 4 PID: 74 at kernel/irq/manage.c:774 __enable_irq+0x4c/0x80
[ 499.914094] Modules linked in:
[ 499.921705] CPU: 4 PID: 74 Comm: kworker/4:2 Not tainted 5.17.0-sdm660-07615-gd33d148796b0-dirty #23
[ 499.924800] Hardware name: Xiaomi Redmi Note 7 (DT)
[ 499.933988] Workqueue: events wled_ovp_work
[ 499.938571] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 499.942803] pc : __enable_irq+0x4c/0x80
[ 499.949687] lr : __enable_irq+0x4c/0x80
[ 499.953506] sp : ffff800008a73d70
[ 499.957311] x29: ffff800008a73d70 x28: 0000000000000000 x27: 0000000000000000
[ 499.960890] x26: ffffad5f9fa4efc0 x25: ffff60283e4baf05 x24: ffff602781f65130
[ 499.968010] x23: ffff60283e4baf00 x22: ffff60283e4b7180 x21: 0000000000000000
[ 499.975128] x20: 0000000000000027 x19: ffff602781136600 x18: ffffffffffffffff
[ 499.982243] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000001
[ 499.989363] x14: 0000000000000000 x13: ffffad5f9f92fea8 x12: 0000000000000357
[ 499.996479] x11: 000000000000011d x10: ffffad5f9f987ea8 x9 : ffffad5f9f92fea8
[ 500.003597] x8 : 00000000ffffefff x7 : ffffad5f9f987ea8 x6 : 0000000000000000
[ 500.010714] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000
[ 500.017832] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff6027820c8000
[ 500.024954] Call trace:
[ 500.047230] __enable_irq+0x4c/0x80
[ 500.062686] enable_irq+0x48/0xa0
[ 500.078190] wled_ovp_work+0x14/0x20
[ 500.093761] process_one_work+0x1d0/0x320
[ 500.105080] worker_thread+0x14c/0x444
[ 500.120700] kthread+0x10c/0x110
[ 500.136347] ret_from_fork+0x10/0x20
[ 500.151981] ---[ end trace 0000000000000000 ]---
ID=1 means that NT36672A_IC enum value is matched.
If that's the case, maybe we should change the touchscreen driver compatible to novatek,nt36xxx
or something similar.
If that's the case, maybe we should change the touchscreen driver compatible to novatek,nt36xxx or something similar.
No, this goes against devicetree rules, compatible string should be very specific (I think?). But driver can support multiple compatible strings. So if you wish you can modify the driver to add
static const struct of_device_id nt36xxx_of_match[] = {
{ .compatible = "novatek,nt36525" },
+ { .compatible = "novatek,nt36672a" },
{ }
};
and use it in lavender dts, but I'll not require it for this MR. Only the commit message change. I don't know what upstream maintainers will say, they might ask for this.
Given that the touchscreen driver isn't in the kernel yet, it's probably best to ask them.
It has also been ~2 years, and nothing has been done upstream about the driver: https://lore.kernel.org/all/20201028221302.66583-1-kholk11@gmail.com/
v5 of that link has:
- Changed compatible string as per Krzysztof K. request
So probably the compatible initially was something like nt36xxx and then changed in v5 to be more specific.
Thanks!
The WLED patch is currently on Patchwork (https://patchwork.kernel.org/project/linux-arm-msm/patch/20220425032824.211975-1-danct12@riseup.net/)
And the touchscreen patch should not be upstreamed until the touchscreen driver is in the kernel.