Skip to content

Added quirk for Surface Go touchscreen - #100

Merged
qzed merged 1 commit into
linux-surface:v5.12-surface-develfrom
zoltantamasvajda:surface-go-touchscreen-battery-fix
Jun 3, 2021
Merged

qzed merged 1 commit into
linux-surface:v5.12-surface-develfrom
zoltantamasvajda:surface-go-touchscreen-battery-fix

Conversation

@zoltantamasvajda

Copy link
Copy Markdown
Contributor

The Elantech touchscreen/digitizer in the Surface Go mistakenly shows up as having capable of reporting battery (even though it actually cannot). This results in a "low battery" message popping up every time you try to use the pen. This patch adds a quirk allowing the kernel to ignore the the non-existent battery and get rid of the false low battery messages.

@qzed qzed left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@qzed
qzed merged commit b7a5754 into linux-surface:v5.12-surface-devel Jun 3, 2021
@qzed

qzed commented Jun 3, 2021

Copy link
Copy Markdown
Member

Do you plan on taking this upstream?

@zoltantamasvajda
zoltantamasvajda deleted the surface-go-touchscreen-battery-fix branch June 3, 2021 14:04
@zoltantamasvajda

Copy link
Copy Markdown
Contributor Author

I can try and see if they want it. This is more of a workaround than a real solution though. I think the reason why it reports having a battery is that the device does not get identified properly so it uses the default values for most properties which just so happens to be true for having a battery. Adding it as a quirk is just kicking the can down the road until someone makes a custom driver.

@qzed

qzed commented Jun 3, 2021

Copy link
Copy Markdown
Member

Given that there isn't any driver like this in the works yet (AFAIK), it's probably a good idea to have that upstream. It can always be removed later on. Also there seem to be at least two other workarounds of this sort (7c38e76, decfe49), so this might be a more or less common issue.

@zoltantamasvajda

Copy link
Copy Markdown
Contributor Author

Patch submitted upstream. I'll let you know how it goes.

@zoltantamasvajda

Copy link
Copy Markdown
Contributor Author

Patch applied upstream! 🥳

@qzed

qzed commented Jun 14, 2021

Copy link
Copy Markdown
Member

Awesome, thanks!

kitakar5525 pushed a commit to kitakar5525/linux-kernel that referenced this pull request Oct 26, 2021
With PREEMPT_COUNT=y, when a CPU is offlined and then onlined again, we
get:

BUG: scheduling while atomic: swapper/1/0/0x00000000
no locks held by swapper/1/0.
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.15.0-rc2+ linux-surface#100
Call Trace:
 dump_stack_lvl+0xac/0x108
 __schedule_bug+0xac/0xe0
 __schedule+0xcf8/0x10d0
 schedule_idle+0x3c/0x70
 do_idle+0x2d8/0x4a0
 cpu_startup_entry+0x38/0x40
 start_secondary+0x2ec/0x3a0
 start_secondary_prolog+0x10/0x14

This is because powerpc's arch_cpu_idle_dead() decrements the idle task's
preempt count, for reasons explained in commit a7c2bb8 ("powerpc:
Re-enable preemption before cpu_die()"), specifically "start_secondary()
expects a preempt_count() of 0."

However, since commit 2c669ef ("powerpc/preempt: Don't touch the idle
task's preempt_count during hotplug") and commit f1a0a37 ("sched/core:
Initialize the idle task with preemption disabled"), that justification no
longer holds.

The idle task isn't supposed to re-enable preemption, so remove the
vestigial preempt_enable() from the CPU offline path.

Tested with pseries and powernv in qemu, and pseries on PowerVM.

Fixes: 2c669ef ("powerpc/preempt: Don't touch the idle task's preempt_count during hotplug")
Signed-off-by: Nathan Lynch <[email protected]>
Reviewed-by: Valentin Schneider <[email protected]>
Reviewed-by: Srikar Dronamraju <[email protected]>
Signed-off-by: Michael Ellerman <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
qzed pushed a commit that referenced this pull request Oct 31, 2021
[ Upstream commit 787252a ]

With PREEMPT_COUNT=y, when a CPU is offlined and then onlined again, we
get:

BUG: scheduling while atomic: swapper/1/0/0x00000000
no locks held by swapper/1/0.
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.15.0-rc2+ #100
Call Trace:
 dump_stack_lvl+0xac/0x108
 __schedule_bug+0xac/0xe0
 __schedule+0xcf8/0x10d0
 schedule_idle+0x3c/0x70
 do_idle+0x2d8/0x4a0
 cpu_startup_entry+0x38/0x40
 start_secondary+0x2ec/0x3a0
 start_secondary_prolog+0x10/0x14

This is because powerpc's arch_cpu_idle_dead() decrements the idle task's
preempt count, for reasons explained in commit a7c2bb8 ("powerpc:
Re-enable preemption before cpu_die()"), specifically "start_secondary()
expects a preempt_count() of 0."

However, since commit 2c669ef ("powerpc/preempt: Don't touch the idle
task's preempt_count during hotplug") and commit f1a0a37 ("sched/core:
Initialize the idle task with preemption disabled"), that justification no
longer holds.

The idle task isn't supposed to re-enable preemption, so remove the
vestigial preempt_enable() from the CPU offline path.

Tested with pseries and powernv in qemu, and pseries on PowerVM.

Fixes: 2c669ef ("powerpc/preempt: Don't touch the idle task's preempt_count during hotplug")
Signed-off-by: Nathan Lynch <[email protected]>
Reviewed-by: Valentin Schneider <[email protected]>
Reviewed-by: Srikar Dronamraju <[email protected]>
Signed-off-by: Michael Ellerman <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Sasha Levin <[email protected]>
davidssurfacebot pushed a commit to davidbitterlich/kernel that referenced this pull request Oct 9, 2026
[BUG]
Recently, we encountered a KASAN warning as follows:

BUG: KASAN: slab-out-of-bounds in fb_pad_aligned_buffer+0x11f/0x140
Read of size 1 at addr ff1100015fd9f6a4 by task tty_fbcon_oob/1239
CPU: 7 UID: 0 PID: 1239 Comm: tty_fbcon_oob Not tainted 7.3.0-rc1 linux-surface#100 PREEMPT(full)
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-4.fc41 04/01/2014
Call Trace:
 <TASK>
 ...
 kasan_report+0xf0/0x120
 fb_pad_aligned_buffer+0x11f/0x140
 ccw_putcs+0x86c/0xa80
 fbcon_putcs+0x338/0x410
 do_update_region+0x21d/0x450
 do_con_write+0x1e0e/0x4880
 con_write+0x13/0x80
 n_tty_write+0x374/0x1010
 file_tty_write.isra.0+0x404/0x7a0
 ...

reproduce:
1) open /dev/tty0, set a KDFONTOP ioctl with op.op = KD_FONT_OP_SET,
op.width = 8 and op.height = 16 (visible VC1)
2) open /dev/tty1, set a KDFONTOP ioctl with op.op = KD_FONT_OP_SET,
op.width = 28 and op.height = 24 (invisible VC2)
3) echo 3 > /sys/devices/virtual/graphics/fbcon/rotate_all
4) write EShash8(esc hash8) to tty1

[CAUSE]
All VCs render to the framebuffer. setfont only modifies the target VC's
font without resizing the framebuffer backing buffer (par->rotated.buf);
the buffer is only resized for the visible VC
(fbcon_do_set_font -> ... -> vc_do_resize -> update_screen). This relies on
the con_should_update() check performed before every update_region().

However, the ESC # 8 path (do_con_trol -> do_update_region) omits the
con_should_update() check. After changing the font size of an invisible VC,
do_update_region() fills using the new font size against a buffer that was
never resized, causing an out-of-bounds access.

[FIX]
Only push the update when the console is visible and not blanked, add
the con_should_update() check in the do_con_trol() like every other call
site of do_update_region() (update_region(), invert_screen(), ...).

Fixes: 1da177e ("Linux-2.6.12-rc2")
Cc: stable <[email protected]>
Signed-off-by: Zizhi Wo <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants