Re: [PATCH] Input: synaptics - add quirk for Daffodil DC253D touchpad

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The newly added DMI quirk is bypassed if the touchpad initializes in SMBus (InterTouch) mode.
- [Medium] Using exact string matching with dmi_match() instead of a dmi_system_id table.
- [Low] Spaces are used instead of tabs for indentation.
--

commit 49d6473335088b14bbb30544a1f0fe866d10ddb5
Author: Ashwin Gundarapu <linuxuser509@zohomail.in>
Input: synaptics - add quirk for Daffodil DC253D touchpad

This commit adds a DMI quirk to clear the CLICKPAD capability bit for the
Daffodil DC253D touchpad. It aims to fix an issue where the device is
incorrectly detected as a clickpad, causing the physical right button to be
missing.

Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2153983

> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 26071128f43a7..4ca85fb4ab70d 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -534,9 +534,16 @@ static int synaptics_invert_y(int y)
>   * Apply quirk(s) if the hardware matches
>   */
>  static void synaptics_apply_quirks(struct psmouse *psmouse,

[Severity: High]
Does this logic get bypassed if the touchpad initializes in SMBus
(InterTouch) mode?

It looks like synaptics_apply_quirks() is only called from the PS/2 fallback
initialization path (synaptics_init_ps2()). During device initialization in
synaptics_init(), if InterTouch (SMBus) is supported, the setup is routed to
synaptics_setup_intertouch().

This SMBus path directly evaluates SYN_CAP_CLICKPAD(info->ext_cap_0c) to
configure the .buttonpad property for the RMI4 driver without ever running
synaptics_apply_quirks(). This would cause the intended fix to fail on modern
systems using InterTouch.

> -				   struct synaptics_device_info *info)
> +                                   struct synaptics_device_info *info)
>  {
> -	int i;
> +        int i;

[Severity: Low]
This isn't a bug, but it looks like spaces were used for indentation here and
in the modified function arguments instead of tabs. The Linux kernel coding
style mandates the use of 8-character tabs for indentation.

> +
> +        /* Daffodil DC253D has a physical right button */
> +        if (dmi_match(DMI_SYS_VENDOR, "Daffodil Computers Ltd.") &&
> +            dmi_match(DMI_PRODUCT_NAME, "DC253D")) {

[Severity: Medium]
Will this exact string matching fail if the DMI strings contain trailing
whitespace or padding?

The dmi_match() function uses exact string matching. DMI strings from OEM BIOS
vendors frequently include trailing whitespace or padding, which would cause
this match to fail and the quirk to be silently ignored.

The standard and more robust approach used throughout the kernel and within
synaptics.c is to use dmi_check_system() with a struct dmi_system_id table.
This utilizes DMI_MATCH() to perform substring matching, safely ignoring any
trailing spaces.

> +                info->ext_cap_0c &= ~BIT(20);  /* Clear CLICKPAD bit */
> +                psmouse_info(psmouse, "Force right button on Daffodil DC253D\n");
> +        }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/19e5047ead1.ce62bdb39017.6022546617731203250@zohomail.in?part=1