Re: [PATCH v2] Input: qt1050 - wait for calibration to complete

[sashiko-bot@kernel.org]

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

New issues:
- [High] Race condition in calibration polling loop causes immediate premature exit.
- [Medium] The 200ms timeout is dangerously tight to wait for the entire calibration process to finish.

Pre-existing issues:
- [High] The interrupt handler reports static firmware keycodes instead of the dynamic `keycodes` array, breaking userspace key remapping (`EVIOCSKEYCODE`).
--

--- Patch [1]: [PATCH v2] Input: qt1050 - wait for calibration to complete ---
commit 4b6f3ed53ac1e4647765d5813d7b91b2a1e6100d
Author: Miles Krause <mileskrause5200@gmail.com>

Input: qt1050 - wait for calibration to complete

The commit fixes a polling condition where regmap_read_poll_timeout() stopped
when calibration started rather than when it finished. It names the
calibration status bit and inverts the polling condition.

Closes: https://lore.kernel.org/r/20260711011903.0605C1F000E9@smtp.kernel.org

> diff --git a/drivers/input/keyboard/qt1050.c b/drivers/input/keyboard/qt1050.c
> index f9f480c910325..457acae0986aa 100644
> --- a/drivers/input/keyboard/qt1050.c
> +++ b/drivers/input/keyboard/qt1050.c
[ ... ]
> @@ -498,7 +499,8 @@ static int qt1050_probe(struct i2c_client *client)
>  		return err;
>  	}
>  	err = regmap_read_poll_timeout(ts->regmap, QT1050_DET_STATUS, status,
> -				 status >> 7 == 1, 10000, 200000);
> +				 !(status & QT1050_DET_STATUS_CALIBRATE),

[Severity: High]
Will this polling condition evaluate to true on the very first read?

Because I2C transactions and hardware state changes take time, the first read
in qt1050_probe() will likely return a status where the calibration bit is
still 0. This would instantly satisfy the wait condition and bypass the loop
before calibration even begins, leaving the device in an uncalibrated state
when the subsequent soft reset occurs.

> +				 10000, 200000);

[Severity: Medium]
Is this 200ms maximum timeout dangerously tight for an operation that takes
approximately 200ms to complete?

The original 200ms timeout was used to wait for calibration to start, but
using it to wait for the entire calibration process leaves virtually no margin
for hardware or timing variances, potentially causing the probe to
sporadically fail with -ETIMEDOUT.

>  	if (err) {
>  		dev_err(dev, "Calibration failed: %d\n", err);
>  		return err;

[Severity: High]
This is a pre-existing issue, but does the interrupt handler report the static
firmware keycode instead of the dynamically mapped one?

During probe, the driver properly sets input->keycode = ts->keycodes to
support dynamic remapping, but in qt1050_irq_threaded() it hardcodes the
static array:

drivers/input/keyboard/qt1050.c:qt1050_irq_threaded() {
    ...
    input_report_key(input, ts->keys[i].keycode,
                     test_bit(i, &new_keys));
    ...
}

When userspace remaps a key via EVIOCSKEYCODE, the input core updates
ts->keycodes and clears the old keycode's bit. Because the ISR still reports
the old, unmapped keycode, the input core safely filters it out and drops the
event, breaking userspace key remapping entirely.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-qt1050-calibration-status-bit-v2-1-a58da64bdf7e@gmail.com?part=1