Re: [PATCH v6 2/3] Input: stmfts - support FTS5

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Missing capability initialization for ABS_MT_TOOL_TYPE causes stylus and palm events to be silently dropped.

Pre-existing issues:
- [High] System resume leaves an active device in hardware sleep mode.
--

commit b210ef23e8e26cf6ac531eb08d54cae1f1a1de1d
Author: David Heidelberg <david@ixit.cz>

Input: stmfts - support FTS5

This patch adds support for the FTS5 touchscreen controller to the stmfts
driver. It introduces a mode-switch GPIO to switch between AP and SLPI modes,
and handles the new FTS5 event format.

> diff --git a/drivers/input/touchscreen/stmfts.c b/drivers/input/touchscreen/stmfts.c
> index 972687797f826..019945f64c48b 100644
> --- a/drivers/input/touchscreen/stmfts.c
> +++ b/drivers/input/touchscreen/stmfts.c
[ ... ]
> @@ -205,6 +254,97 @@ static void stmfts_report_contact_release(struct stmfts_data *sdata,
[ ... ]
> +static void stmfts5_report_contact_event(struct stmfts_data *sdata,
> +					 const u8 event[])
> +{
[ ... ]
> +	input_mt_slot(sdata->input, touch_id);
> +	input_mt_report_slot_state(sdata->input, tool, true);

[Severity: Medium]
Will these tool type events be silently dropped by the input core?
The driver attempts to emit tool type events (like MT_TOOL_PEN or
MT_TOOL_PALM) via input_mt_report_slot_state(), but the ABS_MT_TOOL_TYPE axis
does not seem to be registered.

[ ... ]
> @@ -623,11 +834,25 @@ static int stmfts_power_on(struct stmfts_data *sdata)
>  
>  err_disable_irq:
>  	disable_irq(sdata->client->irq);
> -err_disable_regulators:
>  	regulator_bulk_disable(ARRAY_SIZE(stmfts_supplies), sdata->supplies);
>  	return err;
>  }

[Severity: High]
This is a pre-existing issue, but does system resume leave an active device in
hardware sleep mode?

The PM core invokes the stmfts_resume() sleep callback, which unconditionally
calls stmfts_power_on(). Inside stmfts_power_on(), just above the error
cleanup path shown here, the code issues a STMFTS_SLEEP_IN command.

If the device was active (open) prior to system suspend, it appears it is
forced back into sleep mode without restoring the active scanning state (no
STMFTS_SLEEP_OUT or set_scan(true) is called). Because the PM core already
considers the device active upon return, it doesn't trigger a subsequent
pm_runtime_resume(), leaving the touchscreen unresponsive until userspace
closes and reopens the device.

[ ... ]
> @@ -640,6 +865,48 @@ static void stmfts_power_off(void *data)
[ ... ]
> +static int stmfts5_setup_input(struct stmfts_data *sdata)
> +{
[ ... ]
> +	/* Enable stylus support if requested */
> +	sdata->stylus_enabled = device_property_read_bool(dev, "stylus-enabled");
> +
> +	return input_mt_init_slots(sdata->input, STMFTS_MAX_FINGERS,
> +				   INPUT_MT_DIRECT);
> +}

[Severity: Medium]
Should the ABS_MT_TOOL_TYPE capability be initialized here?
If a stylus is supported, it seems input_set_abs_params() would be needed for
the ABS_MT_TOOL_TYPE axis so the input core doesn't filter out the events.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-stmfts5-v6-0-50fbf6af8718@ixit.cz?part=2