Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Input: edt-ft5x06: edt_ft5x06_ts_init_hw() registers device before IRQ --
commit 5aab9a2f7e5d9f696c6cdfb8f9baf87581af8689 Author: Luka Panio <[email protected]> Input: edt-ft5x06 - move the controller bring-up into a helper This commit moves the controller reset release, hardware identification, input device registration, and debugfs setup out of edt_ft5x06_probe() into a new edt_ft5x06_ts_init_hw() helper function. > diff --git a/drivers/input/touchscreen/edt-ft5x06.c > b/drivers/input/touchscreen/edt-ft5x06.c > index 543cc62088c7a..536d633d97a78 100644 > --- a/drivers/input/touchscreen/edt-ft5x06.c > +++ b/drivers/input/touchscreen/edt-ft5x06.c > @@ -1116,16 +1116,96 @@ static void edt_ft5x06_disable_regulators(void *arg) > regulator_disable(data->iovcc); > } > > +/* > + * Take the controller out of reset, identify it and register the input > + * device. > + */ > +static int edt_ft5x06_ts_init_hw(struct edt_ft5x06_ts_data *tsdata) > +{ [ ... ] > + error = input_register_device(input); > + if (error) > + return error; > + > + edt_ft5x06_ts_prepare_debugfs(tsdata); > + > + return 0; > +} > + > int edt_ft5x06_probe(struct device *dev, struct regmap *regmap, int irq, > const struct edt_ft5x06_chip_data *chip_data, u16 bustype, > struct dentry *debugfs) > { [ ... ] > @@ -1225,78 +1305,22 @@ int edt_ft5x06_probe(struct device *dev, struct > regmap *regmap, int irq, [ ... ] > + error = edt_ft5x06_ts_init_hw(tsdata); > + if (error) > return error; > - } > > irq_flags = irq_get_trigger_type(irq); > if (irq_flags == IRQF_TRIGGER_NONE) > @@ -1310,12 +1334,6 @@ int edt_ft5x06_probe(struct device *dev, struct regmap > *regmap, int irq, > return error; > } > > - error = input_register_device(input); > - if (error) > - return error; > - > - edt_ft5x06_ts_prepare_debugfs(tsdata); > - > dev_dbg(dev, > "EDT FT5x06 initialized: IRQ %d, WAKE pin %d, Reset pin %d.\n", > irq, [Severity: Medium] Previously, edt_ft5x06_probe() called devm_request_threaded_irq() before calling input_register_device() and edt_ft5x06_ts_prepare_debugfs(). With edt_ft5x06_ts_init_hw() now running before devm_request_threaded_irq(), the input device and debugfs files are published to userspace before the touchscreen IRQ is requested. If devm_request_threaded_irq() fails, the input device is announced to userspace only to be immediately torn down during devres release. Additionally, because edt_ft5x06_ts_prepare_debugfs() exposes the debugfs mode file early, writing 1 to it before or during IRQ setup triggers: edt_ft5x06_debugfs_mode_set() -> edt_ft5x06_factory_mode() disable_irq(tsdata->irq); if (!tsdata->raw_buffer) { tsdata->raw_bufsize = tsdata->num_x * tsdata->num_y * sizeof(u16); tsdata->raw_buffer = kzalloc(tsdata->raw_bufsize, GFP_KERNEL); ... } Can this cause disable_irq() (or enable_irq() on error or when switching back in edt_ft5x06_work_mode()) to be called before the IRQ is requested, or cause irq_startup() inside devm_request_threaded_irq() to re-enable the interrupt while tsdata->factory_mode is true? Also, if devm_request_threaded_irq() fails after tsdata->raw_buffer is allocated, edt_ft5x06_remove() -> edt_ft5x06_ts_teardown_debugfs() is not run on probe failure. Does this leak tsdata->raw_buffer? Should devm_request_threaded_irq() be called before edt_ft5x06_ts_init_hw() (for example, with IRQF_NO_AUTOEN if needed until the hardware is ready), or should input_register_device() and edt_ft5x06_ts_prepare_debugfs() happen after the IRQ is requested? oggler -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
