Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v2 1/2] Input: snvs_pwrkey: Improve probe error handling
From: phucduc.bui @ 2026-07-20  9:08 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, bui duc phuc

From: bui duc phuc <phucduc.bui@gmail.com>

Use dev_err_probe() for devm_clk_get_optional_enabled() to handle
deferred probe correctly.
Return the original error from platform_get_irq() instead of replacing
it with -EINVAL.
Drop the redundant error message after devm_request_irq(), as the
helper already reports request failures internally.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/input/keyboard/snvs_pwrkey.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
index 954055aaf6e2..fa08ac561bf2 100644
--- a/drivers/input/keyboard/snvs_pwrkey.c
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -142,16 +142,15 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
 	}
 
 	clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
-	if (IS_ERR(clk)) {
-		dev_err(&pdev->dev, "Failed to get snvs clock (%pe)\n", clk);
-		return PTR_ERR(clk);
-	}
+	if (IS_ERR(clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(clk),
+				     "Failed to get snvs clock\n");
 
 	pdata->wakeup = of_property_read_bool(np, "wakeup-source");
 
 	pdata->irq = platform_get_irq(pdev, 0);
 	if (pdata->irq < 0)
-		return -EINVAL;
+		return pdata->irq;
 
 	error = of_property_read_u32(np, "power-off-time-sec", &val);
 	if (!error) {
@@ -209,10 +208,8 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
 	error = devm_request_irq(&pdev->dev, pdata->irq,
 			       imx_snvs_pwrkey_interrupt,
 			       0, pdev->name, pdev);
-	if (error) {
-		dev_err(&pdev->dev, "interrupt not available.\n");
+	if (error)
 		return error;
-	}
 
 	error = input_register_device(input);
 	if (error < 0) {
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH v3 4/4] HID: Intel-thc-hid: Remove redundant dev_err()
From: sashiko-bot @ 2026-07-20  8:59 UTC (permalink / raw)
  To: Pan Chuang; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260720084310.210976-5-panchuang@vivo.com>

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

Pre-existing issues:
- [Critical] Requesting the IRQ before allocating report buffers and initializing DMA causes an infinite interrupt storm that hangs the CPU.
- [High] Bypassing `quicki2c_dma_deinit()` on the error path causes a memory leak of non-managed DMA buffers and mappings.
--

--- Patch [4]: [PATCH v3 4/4] HID: Intel-thc-hid: Remove redundant dev_err() ---
commit 7eb5c91c4d9eb3ab7af0dabaab924561ec278776
Author: Pan Chuang <panchuang@vivo.com>

HID: Intel-thc-hid: Remove redundant dev_err()

This patch removes redundant driver-specific dev_err() calls after
devm_request_threaded_irq(). The core genirq/devres code automatically
logs detailed error messages on failure now.

> diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> index 46d3e9a019995..59f500345acbf 100644
> --- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> @@ -682,11 +682,8 @@ static int quicki2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  					quicki2c_irq_thread_handler,
>  					IRQF_ONESHOT, KBUILD_MODNAME,
>  					qcdev);
> -	if (ret) {
> -		dev_err_once(&pdev->dev,
> -			     "Failed to request threaded IRQ, irq = %d.\n", pdev->irq);
> +	if (ret)
>  		goto dev_deinit;
> -	}

[Severity: Critical]
This is a pre-existing issue, but does requesting the IRQ here before allocating
report buffers and initializing DMA cause an interrupt loop?

If a pending or spurious interrupt occurs right after devm_request_threaded_irq()
is called but before quicki2c_alloc_report_buf() and quicki2c_dma_init(), the
RT-priority threaded handler could execute.

The handler would find input_buf is NULL and fail to read the DMA buffer. Any
attempt to recover would fail because dma_init() hasn't run. The handler would
then unmask the interrupt. Because the hardware interrupt condition was never
cleared, it could immediately fire again, preempting the probe thread and
hanging the CPU.

>  
>  	ret = quicki2c_get_device_descriptor(qcdev);
>  	if (ret) {

[Severity: High]
This isn't a bug introduced by this patch, but could the error handling further
down in this function leak unmanaged DMA memory?

After quicki2c_dma_init() successfully allocates DMA buffers, subsequent
failures in thc_interrupt_quiesce(), quicki2c_set_power(), or
quicki2c_reset() jump to the dev_deinit label:

	ret = quicki2c_set_power(qcdev, HIDI2C_ON);
	if (ret) {
		dev_err(&pdev->dev, "Set Power On command failed, ret= %d\n", ret);
		goto dev_deinit;
	}

Jumping to dev_deinit bypasses the dma_deinit label and quicki2c_dma_deinit(),
which leaves the DMA resources allocated.

[ ... ]

> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> index 4ae2e1718b30c..504ef3c842ab6 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> @@ -636,11 +636,8 @@ static int quickspi_probe(struct pci_dev *pdev,
>  					quickspi_irq_thread_handler,
>  					IRQF_ONESHOT, KBUILD_MODNAME,
>  					qsdev);
> -	if (ret) {
> -		dev_err(&pdev->dev,
> -			"Failed to request threaded IRQ, irq = %d.\n", pdev->irq);
> +	if (ret)
>  		goto dev_deinit;
> -	}

[Severity: Critical]
This is a pre-existing issue, but might requesting the IRQ here before allocating
report buffers and initializing DMA lead to a similar interrupt loop?

Just like in quicki2c_probe(), if an interrupt fires right after
devm_request_threaded_irq() is called but before quickspi_alloc_report_buf()
and quickspi_dma_init(), the handler could fail to recover DMA state. It would
then unmask the interrupt, which immediately fires again, hanging the CPU in
an RT-priority interrupt loop.

>  
>  	ret = reset_tic(qsdev);
>  	if (ret) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720084310.210976-1-panchuang@vivo.com?part=4

^ permalink raw reply

* Re: [PATCH 2/2] Input: st-keyscan: Improve probe error handling
From: sashiko-bot @ 2026-07-20  8:56 UTC (permalink / raw)
  To: phucduc.bui; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260720085041.224896-2-phucduc.bui@gmail.com>

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

Pre-existing issues:
- [High] Deferred probe errors from `platform_get_irq()` are incorrectly masked and converted to `-EINVAL`.
- [High] The interrupt handler is registered and unmasked while the hardware clock is disabled, which could lead to a crash on an unhandled or pending interrupt.
--

--- Patch [2]: [PATCH 2/2] Input: st-keyscan: Improve probe error handling ---
commit c25a1a7eacb3523c173790ed6cd29726473a8fa7
Author: bui duc phuc <phucduc.bui@gmail.com>

Input: st-keyscan: Improve probe error handling

Use dev_err_probe() for devm_clk_get() so deferred probe errors are
handled correctly.
Drop the redundant error message after devm_request_irq(), since the
helper already reports request failures internally.

> diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
> index e53ef4c670e4b..d86f6f16ae1b2 100644
> --- a/drivers/input/keyboard/st-keyscan.c
> +++ b/drivers/input/keyboard/st-keyscan.c

[ ... ]

> @@ -192,10 +191,8 @@ static int keyscan_probe(struct platform_device *pdev)
>  
>  	error = devm_request_irq(&pdev->dev, keypad_data->irq, keyscan_isr, 0,
>  				 pdev->name, keypad_data);

[Severity: High]
This is a pre-existing issue, but since you are improving error handling in 
keyscan_probe(), could we also fix the error return for platform_get_irq() 
just prior to this?

Currently the code does this:

	keypad_data->irq = platform_get_irq(pdev, 0);
	if (keypad_data->irq < 0)
		return -EINVAL;

Does overwriting the real error code from platform_get_irq() with -EINVAL
break deferred probe if the interrupt controller is not yet ready?

[Severity: High]
This isn't a bug introduced by this patch, but I also noticed a potential 
issue here related to the IRQ registration.

Just before calling devm_request_irq(), keyscan_probe() calls 
keyscan_stop() which disables the clock.

If the IRQ line is asserted (e.g. from a pending bootloader state or
spurious event), keyscan_isr() will fire immediately once devm_request_irq()
is called.

The ISR then accesses peripheral registers:

drivers/input/keyboard/st-keyscan.c:keyscan_isr() {
	...
	state = readl(keypad->base + KEYSCAN_MATRIX_STATE_OFF) & 0xffff;
	...
}

Can accessing peripheral registers while the corresponding clock is disabled 
cause a synchronous bus fault or kernel panic on these systems? Should the 
devm_request_irq() call be moved after the clock is re-enabled?

> -	if (error) {
> -		dev_err(&pdev->dev, "failed to request IRQ\n");
> +	if (error)
>  		return error;
> -	}
>  
>  	error = input_register_device(input_dev);
>  	if (error) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720085041.224896-1-phucduc.bui@gmail.com?part=2

^ permalink raw reply

* [PATCH 2/2] Input: st-keyscan: Improve probe error handling
From: phucduc.bui @ 2026-07-20  8:50 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, bui duc phuc
In-Reply-To: <20260720085041.224896-1-phucduc.bui@gmail.com>

From: bui duc phuc <phucduc.bui@gmail.com>

Use dev_err_probe() for devm_clk_get() so deferred probe errors are
handled correctly.
Drop the redundant error message after devm_request_irq(), since the
helper already reports request failures internally.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/input/keyboard/st-keyscan.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
index e53ef4c670e4..d86f6f16ae1b 100644
--- a/drivers/input/keyboard/st-keyscan.c
+++ b/drivers/input/keyboard/st-keyscan.c
@@ -173,10 +173,9 @@ static int keyscan_probe(struct platform_device *pdev)
 		return PTR_ERR(keypad_data->base);
 
 	keypad_data->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(keypad_data->clk)) {
-		dev_err(&pdev->dev, "cannot get clock\n");
-		return PTR_ERR(keypad_data->clk);
-	}
+	if (IS_ERR(keypad_data->clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(keypad_data->clk),
+				     "cannot get clock\n");
 
 	error = clk_enable(keypad_data->clk);
 	if (error) {
@@ -192,10 +191,8 @@ static int keyscan_probe(struct platform_device *pdev)
 
 	error = devm_request_irq(&pdev->dev, keypad_data->irq, keyscan_isr, 0,
 				 pdev->name, keypad_data);
-	if (error) {
-		dev_err(&pdev->dev, "failed to request IRQ\n");
+	if (error)
 		return error;
-	}
 
 	error = input_register_device(input_dev);
 	if (error) {
-- 
2.43.0


^ permalink raw reply related

* [PATCH 1/2] Input: snvs_pwrkey: Improve probe error handling
From: phucduc.bui @ 2026-07-20  8:50 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, bui duc phuc

From: bui duc phuc <phucduc.bui@gmail.com>

Use dev_err_probe() for devm_clk_get_optional_enabled() to handle
deferred probe correctly.
Return the original error from platform_get_irq() instead of replacing
it with -EINVAL.
Drop the redundant error message after devm_request_irq(), as the
helper already reports request failures internally.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/input/keyboard/snvs_pwrkey.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
index 954055aaf6e2..fa08ac561bf2 100644
--- a/drivers/input/keyboard/snvs_pwrkey.c
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -142,16 +142,15 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
 	}
 
 	clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
-	if (IS_ERR(clk)) {
-		dev_err(&pdev->dev, "Failed to get snvs clock (%pe)\n", clk);
-		return PTR_ERR(clk);
-	}
+	if (IS_ERR(clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(clk),
+				     "Failed to get snvs clock\n");
 
 	pdata->wakeup = of_property_read_bool(np, "wakeup-source");
 
 	pdata->irq = platform_get_irq(pdev, 0);
 	if (pdata->irq < 0)
-		return -EINVAL;
+		return pdata->irq;
 
 	error = of_property_read_u32(np, "power-off-time-sec", &val);
 	if (!error) {
@@ -209,10 +208,8 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
 	error = devm_request_irq(&pdev->dev, pdata->irq,
 			       imx_snvs_pwrkey_interrupt,
 			       0, pdev->name, pdev);
-	if (error) {
-		dev_err(&pdev->dev, "interrupt not available.\n");
+	if (error)
 		return error;
-	}
 
 	error = input_register_device(input);
 	if (error < 0) {
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 4/4] HID: Intel-thc-hid: Remove redundant dev_err()
From: Pan Chuang @ 2026-07-20  8:43 UTC (permalink / raw)
  To: Even Xu, Xinpeng Sun, Jiri Kosina, Benjamin Tissoires,
	Sakari Ailus, Pan Chuang, Danny D., Abhishek Tamboli,
	open list:HID CORE LAYER, open list
In-Reply-To: <20260720084310.210976-1-panchuang@vivo.com>

Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()"), devm_request_threaded_irq() automatically logs
detailed error messages on failure. Remove the now-redundant
driver-specific dev_err() calls.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
Reviewed-by: Even Xu <even.xu@intel.com>
---
 drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c | 5 +----
 drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c | 5 +----
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
index 46d3e9a01999..59f500345acb 100644
--- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
+++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
@@ -682,11 +682,8 @@ static int quicki2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 					quicki2c_irq_thread_handler,
 					IRQF_ONESHOT, KBUILD_MODNAME,
 					qcdev);
-	if (ret) {
-		dev_err_once(&pdev->dev,
-			     "Failed to request threaded IRQ, irq = %d.\n", pdev->irq);
+	if (ret)
 		goto dev_deinit;
-	}
 
 	ret = quicki2c_get_device_descriptor(qcdev);
 	if (ret) {
diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
index 4ae2e1718b30..504ef3c842ab 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
@@ -636,11 +636,8 @@ static int quickspi_probe(struct pci_dev *pdev,
 					quickspi_irq_thread_handler,
 					IRQF_ONESHOT, KBUILD_MODNAME,
 					qsdev);
-	if (ret) {
-		dev_err(&pdev->dev,
-			"Failed to request threaded IRQ, irq = %d.\n", pdev->irq);
+	if (ret)
 		goto dev_deinit;
-	}
 
 	ret = reset_tic(qsdev);
 	if (ret) {
-- 
2.34.1


^ permalink raw reply related

* [PATCH v3 3/4] HID: intel-ish-hid: ipc: Remove redundant dev_err()
From: Pan Chuang @ 2026-07-20  8:43 UTC (permalink / raw)
  To: Srinivas Pandruvada, Jiri Kosina, Benjamin Tissoires, Zhang Lixu,
	Andy Shevchenko, Pan Chuang, Vineeth Pillai,
	open list:INTEL INTEGRATED SENSOR HUB DRIVER, open list
In-Reply-To: <20260720084310.210976-1-panchuang@vivo.com>

Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()"), devm_request_irq() automatically logs
detailed error messages on failure. Remove the now-redundant
driver-specific dev_err() calls.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/hid/intel-ish-hid/ipc/pci-ish.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index 8d36ae96a3ee..e7196e429c8b 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -232,10 +232,8 @@ static int ish_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	ret = devm_request_irq(dev, pdev->irq, ish_irq_handler,
 			       irq_flag, KBUILD_MODNAME, ishtp);
-	if (ret) {
-		dev_err(dev, "ISH: request IRQ %d failed\n", pdev->irq);
+	if (ret)
 		return ret;
-	}
 
 	dev_set_drvdata(ishtp->devc, ishtp);
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH v3 2/4] HID: hid-goodix: Remove redundant dev_err()
From: Pan Chuang @ 2026-07-20  8:43 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, open list:HID CORE LAYER,
	open list
  Cc: Pan Chuang
In-Reply-To: <20260720084310.210976-1-panchuang@vivo.com>

Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()"), devm_request_threaded_irq() automatically logs
detailed error messages on failure. Remove the now-redundant
driver-specific dev_err() calls.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/hid/hid-goodix-spi.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/hid/hid-goodix-spi.c b/drivers/hid/hid-goodix-spi.c
index 288cb827e9d6..03d549efbdce 100644
--- a/drivers/hid/hid-goodix-spi.c
+++ b/drivers/hid/hid-goodix-spi.c
@@ -722,11 +722,8 @@ static int goodix_spi_probe(struct spi_device *spi)
 	error = devm_request_threaded_irq(&ts->spi->dev, ts->spi->irq,
 					  NULL, goodix_hid_irq, IRQF_ONESHOT,
 					  "goodix_spi_hid", ts);
-	if (error) {
-		dev_err(ts->dev, "could not register interrupt, irq = %d, %d",
-			ts->spi->irq, error);
+	if (error)
 		goto err_destroy_hid;
-	}
 
 	return 0;
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH v3 1/4] HID: amd_sfh: Remove redundant dev_err()
From: Pan Chuang @ 2026-07-20  8:42 UTC (permalink / raw)
  To: Basavaraj Natikar, Jiri Kosina, Benjamin Tissoires,
	open list:AMD SENSOR FUSION HUB DRIVER, open list
  Cc: Pan Chuang, Basavaraj Natikar
In-Reply-To: <20260720084310.210976-1-panchuang@vivo.com>

Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()"), devm_request_irq() automatically logs
detailed error messages on failure. Remove the now-redundant
driver-specific dev_err() calls.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
Acked-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
index 4b81cebdc335..4d0a95fbc4e4 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
@@ -122,19 +122,10 @@ static irqreturn_t amd_sfh_irq_handler(int irq, void *data)
 
 int amd_sfh_irq_init_v2(struct amd_mp2_dev *privdata)
 {
-	int rc;
-
 	pcim_intx(privdata->pdev, true);
 
-	rc = devm_request_irq(&privdata->pdev->dev, privdata->pdev->irq,
-			      amd_sfh_irq_handler, 0, DRIVER_NAME, privdata);
-	if (rc) {
-		dev_err(&privdata->pdev->dev, "failed to request irq %d err=%d\n",
-			privdata->pdev->irq, rc);
-		return rc;
-	}
-
-	return 0;
+	return devm_request_irq(&privdata->pdev->dev, privdata->pdev->irq,
+				amd_sfh_irq_handler, 0, DRIVER_NAME, privdata);
 }
 
 static int amd_sfh_dis_sts_v2(struct amd_mp2_dev *privdata)
-- 
2.34.1


^ permalink raw reply related

* [PATCH v3 0/4] HID: Remove redundant error messages on IRQ request failure
From: Pan Chuang @ 2026-07-20  8:42 UTC (permalink / raw)
  To: Basavaraj Natikar, Jiri Kosina, Benjamin Tissoires,
	Srinivas Pandruvada, Even Xu, Xinpeng Sun, Zhang Lixu,
	Andy Shevchenko, Steven Rostedt, Pan Chuang, Vineeth Pillai,
	Sakari Ailus, Abhishek Tamboli, Danny D.,
	open list:AMD SENSOR FUSION HUB DRIVER, open list

Commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()") added automatic error logging to
devm_request_threaded_irq() and devm_request_any_context_irq()
via the new devm_request_result() helper, which prints device
name, IRQ number, handler functions, and error code on failure.

Since devm_request_irq() is a static inline wrapper around
devm_request_threaded_irq(), it also benefits from this
automatic logging.

Remove the now-redundant dev_err() and dev_err_probe() calls
in hid drivers that follow these devm_request_*_irq()
functions, as the core now provides more detailed diagnostic
information on failure.

v1 -> v2:
- Added the same change for the intel-quicki2c driver

v2 -> v3:
- amd_sfh: directly return devm_request_irq() result

Pan Chuang (4):
  HID: amd_sfh: Remove redundant dev_err()
  HID: hid-goodix: Remove redundant dev_err()
  HID: intel-ish-hid: ipc: Remove redundant dev_err()
  HID: Intel-thc-hid: Remove redundant dev_err()

 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c              | 13 ++-----------
 drivers/hid/hid-goodix-spi.c                        |  5 +----
 drivers/hid/intel-ish-hid/ipc/pci-ish.c             |  4 +---
 .../hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c |  5 +----
 .../hid/intel-thc-hid/intel-quickspi/pci-quickspi.c |  5 +----
 5 files changed, 6 insertions(+), 26 deletions(-)

-- 
2.34.1


^ permalink raw reply

* Re: [PATCH v2 3/3] dt-bindings: leds: backlight: Convert TPS65217 to DT schema
From: Krzysztof Kozlowski @ 2026-07-20  6:54 UTC (permalink / raw)
  To: Eduard Bostina
  Cc: Aaro Koskinen, Andreas Kemnade, Conor Dooley, Daniel Thompson,
	devicetree, Dmitry Torokhov, dri-devel, Jingoo Han, Kevin Hilman,
	Krzysztof Kozlowski, Lee Jones, linux-input, linux-kernel,
	linux-leds, linux-omap, Pavel Machek, Rob Herring, Roger Quadros,
	Tony Lindgren, daniel.baluta, simona.toaca, goledhruva,
	m-chawdhry
In-Reply-To: <20260718215632.3295049-4-egbostina@gmail.com>

On Sat, Jul 18, 2026 at 09:56:32PM +0000, Eduard Bostina wrote:
 +
> +    patternProperties:
> +      "^regulator@[0-6]$":
> +        type: object
> +        $ref: /schemas/regulator/regulator.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          reg:
> +            maxItems: 1
> +
> +          regulator-compatible:
> +            $ref: /schemas/types.yaml#/definitions/string

You should not need this property, it is already in regulator.yaml.

> +
> +  charger:
> +    $ref: /schemas/power/supply/tps65217-charger.yaml#

unevaluatedProperties: false

> +
> +  pwrbutton:
> +    $ref: /schemas/input/ti,tps65217-pwrbutton.yaml#

unevaluatedProperties: false

> +
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        tps@24 {

pmic@24

Please make the example mostly complete - add rest of properties and nodes.

> +            reg = <0x24>;
> +            compatible = "ti,tps65217";
> +
> +            backlight {
> +                isel = <1>;   /* 1 - ISET1, 2 ISET2 */
> +                fdim = <100>; /* TPS65217_BL_FDIM_100HZ */
> +                default-brightness = <50>;
> +            };
> +        };

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 2/3] dt-bindings: input: Convert TI TPS65217 power button to DT schema
From: Krzysztof Kozlowski @ 2026-07-20  6:51 UTC (permalink / raw)
  To: Eduard Bostina
  Cc: Aaro Koskinen, Andreas Kemnade, Conor Dooley, Daniel Thompson,
	devicetree, Dmitry Torokhov, dri-devel, Jingoo Han, Kevin Hilman,
	Krzysztof Kozlowski, Lee Jones, linux-input, linux-kernel,
	linux-leds, linux-omap, Pavel Machek, Rob Herring, Roger Quadros,
	Tony Lindgren, daniel.baluta, simona.toaca, goledhruva,
	m-chawdhry
In-Reply-To: <20260718215632.3295049-3-egbostina@gmail.com>

On Sat, Jul 18, 2026 at 09:56:31PM +0000, Eduard Bostina wrote:
> Convert the Texas Instruments TPS65217 and TPS65218 Power Button
> bindings to DT schema.
> 
> Signed-off-by: Eduard Bostina <egbostina@gmail.com>
> ---
>  .../bindings/input/ti,tps65217-pwrbutton.yaml | 42 +++++++++++++++++++
>  .../bindings/input/tps65218-pwrbutton.txt     | 30 -------------
>  2 files changed, 42 insertions(+), 30 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/input/ti,tps65217-pwrbutton.yaml
>  delete mode 100644 Documentation/devicetree/bindings/input/tps65218-pwrbutton.txt

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 1/3] ARM: dts: ti: am335x-guardian: Drop unused isink-en property
From: Krzysztof Kozlowski @ 2026-07-20  6:49 UTC (permalink / raw)
  To: Eduard Bostina
  Cc: Aaro Koskinen, Andreas Kemnade, Conor Dooley, Daniel Thompson,
	devicetree, Dmitry Torokhov, dri-devel, Jingoo Han, Kevin Hilman,
	Krzysztof Kozlowski, Lee Jones, linux-input, linux-kernel,
	linux-leds, linux-omap, Pavel Machek, Rob Herring, Roger Quadros,
	Tony Lindgren, daniel.baluta, simona.toaca, goledhruva,
	m-chawdhry
In-Reply-To: <20260718215632.3295049-2-egbostina@gmail.com>

On Sat, Jul 18, 2026 at 09:56:30PM +0000, Eduard Bostina wrote:
> The 'isink-en' property is not read by any driver, so remove it from the

"...is not documented and not read by any driver..."

> TPS65217 backlight node.

With this change:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2] Input: byd - synchronize timer deletion before freeing private data
From: sashiko-bot @ 2026-07-20  6:26 UTC (permalink / raw)
  To: Linmao Li; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260720061259.1601281-1-lilinmao@kylinos.cn>

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

Pre-existing issues:
- [High] Concurrent execution of the protocol handler `byd_process_byte()` during or after `byd_disconnect()` can lead to a Use-After-Free or NULL pointer dereference of the driver's private data.
--

--- Patch [1]: [PATCH v2] Input: byd - synchronize timer deletion before freeing private data ---
commit e08a94a088b6cedd4010692fa9ab4cd3e5287e71
Author: Linmao Li <lilinmao@kylinos.cn>

    Input: byd - synchronize timer deletion before freeing private data
    
    byd_disconnect() uses timer_delete() before freeing the driver's private
    data.  This does not wait for a running byd_clear_touch() callback, which
    dereferences the private data and its psmouse pointer.  A callback racing
    with disconnect can therefore access the private data after it has been
    freed.  The timer can also still be re-armed by byd_process_byte() while
    the disconnect is in progress.
    
    Use timer_shutdown_sync() before freeing the private data: it waits for
    a running callback and turns any later re-arm attempt into a no-op.

> diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c
> index f5770a3af2f12..5fc3c629590ae 100644
> --- a/drivers/input/mouse/byd.c
> +++ b/drivers/input/mouse/byd.c
> @@ -423,7 +423,7 @@ static void byd_disconnect(struct psmouse *psmouse)
>  	struct byd_data *priv = psmouse->private;
>  
>  	if (priv) {
> -		timer_delete(&priv->timer);
> +		timer_shutdown_sync(&priv->timer);
>  		kfree(psmouse->private);
                ^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
This is a pre-existing issue, but could this free lead to a use-after-free
or null pointer dereference if a hardware interrupt occurs concurrently?

During disconnect, the psmouse core sets the software state to
PSMOUSE_CMD_MODE in psmouse_disconnect(), which still allows the interrupt
handler to process incoming bytes.

If a full packet is received, the protocol handler byd_process_byte() is
called and unconditionally dereferences psmouse->private, which may have
just been freed or set to NULL:

drivers/input/mouse/byd.c:byd_process_byte() {
    struct byd_data *priv = psmouse->private;
    ...
    switch (pkt[3]) {
    case BYD_PACKET_ABSOLUTE:
        if (!priv->touch) {
    ...
}

Should the state be set to PSMOUSE_IGNORE or the RX queue otherwise paused
before we free the private data here?

>  		psmouse->private = NULL;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720061259.1601281-1-lilinmao@kylinos.cn?part=1

^ permalink raw reply

* [PATCH v2] Input: byd - synchronize timer deletion before freeing private data
From: Linmao Li @ 2026-07-20  6:12 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Kees Cook, Vivek BalachandharTN, Richard Pospesel, Chris Diamand,
	linux-input, linux-kernel, Linmao Li, stable
In-Reply-To: <20260720033030.1580891-1-lilinmao@kylinos.cn>

byd_disconnect() uses timer_delete() before freeing the driver's private
data.  This does not wait for a running byd_clear_touch() callback, which
dereferences the private data and its psmouse pointer.  A callback racing
with disconnect can therefore access the private data after it has been
freed.  The timer can also still be re-armed by byd_process_byte() while
the disconnect is in progress.

Use timer_shutdown_sync() before freeing the private data: it waits for
a running callback and turns any later re-arm attempt into a no-op.

Fixes: 2d5f5611dd0d ("Input: byd - enable absolute mode")
Cc: stable@vger.kernel.org
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
v2:
- use timer_shutdown_sync() instead of timer_delete_sync() so the
  timer cannot be re-armed by byd_process_byte() while disconnect is
  in progress (Sashiko review, Dmitry)

 drivers/input/mouse/byd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c
index f5770a3af2f1..5fc3c629590a 100644
--- a/drivers/input/mouse/byd.c
+++ b/drivers/input/mouse/byd.c
@@ -423,7 +423,7 @@ static void byd_disconnect(struct psmouse *psmouse)
 	struct byd_data *priv = psmouse->private;
 
 	if (priv) {
-		timer_delete(&priv->timer);
+		timer_shutdown_sync(&priv->timer);
 		kfree(psmouse->private);
 		psmouse->private = NULL;
 	}
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH] Input: byd - synchronize timer deletion before freeing private data
From: Dmitry Torokhov @ 2026-07-20  5:18 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Linmao Li, linux-input
In-Reply-To: <20260720034855.07D8C1F000E9@smtp.kernel.org>

On Mon, Jul 20, 2026 at 03:48:54AM +0000, sashiko-bot@kernel.org wrote:
> 
> Should this use timer_shutdown_sync() instead to permanently prevent the timer
> from being re-armed during teardown?

This is a good suggestion.

> 
> >  		kfree(psmouse->private);
> >  		psmouse->private = NULL;
> 
> [Severity: High]
> This is a pre-existing issue, but is it possible for an interrupt to trigger
> byd_process_byte() concurrently with or immediately after freeing this memory?

This has to be solved in psmouse core, I'll take a look.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v4 3/3] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: Fenglin Wu @ 2026-07-20  4:33 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-arm-msm, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones, Stephen Boyd, Bjorn Andersson, Konrad Dybcio,
	David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa,
	linux-input, devicetree, linux-kernel
In-Reply-To: <alpWEL1hJgQ18I7d@google.com>



On 7/18/2026 12:51 AM, Dmitry Torokhov wrote:
...

Thank you for reviewing the change!

>> +
>> +static void haptics_fifo_irq_enable(struct qcom_haptics *h, bool enable)
>> +{
>> +	if (h->irq_enabled == enable)
>> +		return;
> 
> Should t you know if given code runs with interrupts disabled or
> enabled? I believe this tracking and the wrapper should be removed.
> 
>> +
>> +	if (enable)
>> +		enable_irq(h->fifo_empty_irq);
>> +	else
>> +		disable_irq(h->fifo_empty_irq);
>> +
>> +	h->irq_enabled = enable;
>> +}
>> +
In normal handling, the code should be able to track the IRQ status. The
IRQ is not auto enabled after the registration. It's only enabled when
the upload data doesn't fit in the initial FIFO fill and requires to
refill based on the interrupt. And after play is done, the IRQ can be
disabled.

There are error paths or concurrency cases which might cause the IRQ
being disabled multiple times, for example:
1) IRQ is enabled when playing an effect with a long FIFO data
2) During the FIFO refill, if any SPMI write errors, and if the SPMI bus
issue persists, there would be an IRQ storm as the IRQ is still kept as
enabled and the HW FIFO is still 'empty'. So I need to disable the IRQ
to avoid the IRQ storming before the play is stopped in step 3) below.
3) Stop the play and disable the IRQ, either when the play is done, or
when userspace issues a stop command.

There are potential multiple times of IRQ disabling in such cases, I
created this helper function to track the IRQ status to prevent
disabling the IRQ permanently.

BTW, Sashiko AI flagged a deadlock issue on the 'disable_irq()' usage,
when a stop() command came 1st and acquired the 'fifo_lock' and then the
IRQ thread is scheduled, in 'disable_irq()" the IRQ handler would
compete the 'fifo_lock' and cause a deadlock. I will need to change it
to use the _nosync() version consider how to prevent the races of FIFO
resources being used by the IRQ handler after it is freed in stop().


>> +
>> +/*
>> + * haptics_fifo_empty_irq: Threaded IRQ handler for the FIFO-empty interrupt.
>> + *
>> + * While a FIFO play is in progress the hardware fires this interrupt when
>> + * the number of samples in the FIFO drops below the programmed threshold.
>> + * The handler refills the FIFO from the effect's data buffer.  When all
>> + * samples have been written the threshold is set to zero. The HW would
>> + * stop the play automatically after all of the samples in FIFO memory are
>> + * played out.
>> + */
>> +static irqreturn_t haptics_fifo_empty_irq(int irq, void *dev_id)
>> +{
>> +	struct qcom_haptics *h = dev_id;
>> +	u32 sts, to_write;
>> +	int ret;
>> +
>> +	ret = regmap_read(h->regmap,
>> +			  h->cfg_base + HAP_CFG_INT_RT_STS_REG, &sts);
>> +	if (ret || !(sts & FIFO_EMPTY_BIT))
>> +		return IRQ_HANDLED;
>> +
>> +	guard(mutex)(&h->fifo_lock);
>> +
>> +	if (!h->fifo_data)
>> +		return IRQ_HANDLED;
>> +
>> +	/* Refill: write the next chunk */
>> +	to_write = min_t(u32, h->data_len - h->data_written,
>> +			 h->fifo_len - FIFO_EMPTY_THRESH);
>> +	ret = haptics_write_fifo_chunk(h, &h->fifo_data[h->data_written], to_write);
>> +	if (ret) {
>> +		dev_err(h->dev, "refill FIFO samples failed, ret=%d\n", ret);
>> +		/*
>> +		 * If data refilling is failed,stop the HW play and disable the
>> +		 * IRQ to prevent the FIFO empty IRQ being fired continuously.
>> +		 */
> 
> Is recovery possible after this?

It depends on how serious the problem is. Ideally, the SPMI write access
error above should only indicate a bus transaction fault but the haptics
HW should still work on its FIFO data which has already been programmed.

If the SPMI write fault just happens temporarily, it might only cause
the haptics driving waveform distortion and impact on the vibration
effect. If the SPMI write fault is caused by SPMI bus hung, then the
haptics module won't be functional anymore as all of the commands are
SPMI based. For safety, I stop the play (doesn't work in the latter
case) and disable the IRQ to prevent the IRQ being fired continuously.

Thanks
Fenglin

^ permalink raw reply

* Re: [PATCH] Input: byd - synchronize timer deletion before freeing private data
From: sashiko-bot @ 2026-07-20  3:48 UTC (permalink / raw)
  To: Linmao Li; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260720033030.1580891-1-lilinmao@kylinos.cn>

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

Pre-existing issues:
- [High] Use-After-Free or NULL pointer dereference in byd_process_byte() due to missing RX synchronization during disconnect.
- [High] Improper timer teardown API allows the timer to be re-armed after sync, causing a delayed Use-After-Free.
--

--- Patch [1]: [PATCH] Input: byd - synchronize timer deletion before freeing private data ---
commit eb1fe8255d54e5e900b504d5aed98e3fff1721fb
Author: Linmao Li <lilinmao@kylinos.cn>

Input: byd - synchronize timer deletion before freeing private data
    
byd_disconnect() uses timer_delete() before freeing the driver's private
data.  This does not wait for a running byd_clear_touch() callback, which
dereferences the private data and its psmouse pointer.  A callback racing
with disconnect can therefore access the private data after it has been
freed.
    
Use timer_delete_sync() to wait for any running callback before releasing
the private data.
    
Fixes: 2d5f5611dd0d ("Input: byd - enable absolute mode")
Cc: stable@vger.kernel.org
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>

> diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c
> index f5770a3af2f12..7a69724c1ac6b 100644
> --- a/drivers/input/mouse/byd.c
> +++ b/drivers/input/mouse/byd.c
> @@ -423,7 +423,7 @@ static void byd_disconnect(struct psmouse *psmouse)
>  	struct byd_data *priv = psmouse->private;
>  
>  	if (priv) {
> -		timer_delete(&priv->timer);
> +		timer_delete_sync(&priv->timer);

[Severity: High]
This isn't a bug introduced by this patch, but does timer_delete_sync()
prevent the timer from being re-armed concurrently?

Because device interrupts are not disabled during byd_disconnect(),
byd_process_byte() can execute immediately after timer_delete_sync()
returns but before psmouse->private is freed.

If byd_process_byte() processes a touch event, it can call mod_timer()
and successfully re-arm the timer on the priv structure:

drivers/input/mouse/byd.c:byd_process_byte() {
	...
	mod_timer(&priv->timer, jiffies + BYD_TOUCH_TIMEOUT);
	...
}

When byd_disconnect() subsequently frees priv, won't the timer callback
execute on the freed memory?

Should this use timer_shutdown_sync() instead to permanently prevent the timer
from being re-armed during teardown?

>  		kfree(psmouse->private);
>  		psmouse->private = NULL;

[Severity: High]
This is a pre-existing issue, but is it possible for an interrupt to trigger
byd_process_byte() concurrently with or immediately after freeing this memory?

The core psmouse_disconnect() function invokes the driver's disconnect callback
without first stopping device interrupts (such as by using serio_pause_rx()).

If the hardware sends a byte during device teardown, ps2_interrupt() can
route the data to the active protocol handler byd_process_byte(). This
unconditionally dereferences the freed or NULL psmouse->private pointer:

drivers/input/mouse/byd.c:byd_process_byte() {
	struct byd_data *priv = psmouse->private;
	...
}

Does the receive path need to be paused or synchronized before freeing the
private data to prevent a use-after-free or NULL pointer dereference?

>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720033030.1580891-1-lilinmao@kylinos.cn?part=1

^ permalink raw reply

* Re: [PATCH v13 2/4] HID: hid-msi: Add M-key mapping attributes
From: sashiko-bot @ 2026-07-20  3:32 UTC (permalink / raw)
  To: Derek J. Clark; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260720031549.2272658-3-derekjohn.clark@gmail.com>

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

New issues:
- [High] Race condition in `claw_profile_event` causes stale data to be returned for profile reads.
- [Low] Compilation error due to invalid C syntax for embedded tagged struct.
- [Low] Subsystem guide violation: Mixing `goto`-based cleanup labels with `__free()` automated cleanup.

Pre-existing issues:
- [High] Generic ACK responses improperly complete commands waiting for specific data payloads.
- [High] Gamepad sysfs attributes are permanently hidden and never created.
--

--- Patch [2]: [PATCH v13 2/4] HID: hid-msi: Add M-key mapping attributes ---
commit 4c48f895b57157a8a075f47f1218b8052e864f1b
Author: Derek J. Clark <derekjohn.clark@gmail.com>

HID: hid-msi: Add M-key mapping attributes
    
This patch adds attributes that allow for remapping the M-keys on MSI
devices when in macro mode. It routes mapped buttons to appropriate event
devices for gamepad, keyboard, or mouse events.

Link: https://patch.msgid.link/20260529072111.7565-3-derekjohn.clark@gmail.com

> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 6687e4579faa1..023d63cee6b06 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
> @@ -255,6 +255,15 @@ struct claw_command_report {
>  	u8 data[59];
>  } __packed;
>  
> +struct claw_profile_report {
> +	u8 profile;
> +	__be16 read_addr;
> +} __packed;
> +
> +struct claw_mkey_report {
> +	struct claw_profile_report;
> +	u8 padding_0;

[Severity: Low]
Will this compile successfully, or does this introduce a regression?
Standard C11 only permits anonymous structs if they do not have a tag. Using
a tagged struct (like struct claw_profile_report;) without a field name acts
as a forward declaration.

This makes padding_0 the first member, causing the initialization in
claw_mkey_store() to fail, and attempts to access mkeys->read_addr in
claw_profile_event() will fail with a missing member error.

[ ... ]
> +static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
> +{
> +	enum claw_profile_ack_pending profile;
> +	struct claw_mkey_report *mkeys;
> +	u8 *codes, key;
> +	int i;
> +
> +	scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
> +		profile = drvdata->profile_pending;
> +
> +	switch (profile) {
> +	case CLAW_M1_PENDING:
[ ... ]
> +	default:
> +		dev_dbg(&drvdata->hdev->dev,
> +			"Got profile event without changes pending from command: %x\n",
> +			cmd_rep->cmd);
> +		return -EINVAL;
> +	}
> +	scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
> +		drvdata->profile_pending = CLAW_NO_PENDING;
> +
> +	return 0;
> +}

[Severity: High]
Is there a race condition here that introduces a regression where stale
data is returned?

The lock is dropped after reading profile_pending, opening a race window
where another thread could update it. When this thread resumes and
unconditionally writes CLAW_NO_PENDING, it could destroy pending profile
requests initiated during the race window.

For example, if Thread 1 times out leaving CLAW_M1_PENDING, and Thread 2
sets CLAW_M2_PENDING, a delayed hardware response for M1 could cause this
function to overwrite Thread 2's state with CLAW_NO_PENDING. This would
cause Thread 2 to return unmodified M2 data.

[ ... ]
>  static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
>  			  u8 *data, int size)
>  {
[ ... ]
>  	case CLAW_COMMAND_TYPE_ACK:
>  		if (drvdata->orphan_ack_pending) {
>  			drvdata->orphan_ack_pending = false;
>  			complete(&drvdata->orphan_ack_complete);
>  			break;
>  		}
>  
>  		if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_NONE) {
>  			dev_warn(&drvdata->hdev->dev, "Got unexpected ACK from MCU, ignoring\n");
>  			break;
>  		}
>  
>  		drvdata->cmd_status = 0;
>  		complete(&drvdata->send_cmd_complete);

[Severity: High]
This is a pre-existing issue, but do generic ACKs improperly complete
commands waiting for specific data payloads?

If a previous command (like SYNC_TO_ROM with a timeout of 0) takes longer
than the timeout to send its ACK, the orphan_ack_pending flag might clear
before the generic ACK arrives. 

When the delayed generic ACK finally arrives while waiting_cmd is set for
a new command (like READ_PROFILE), this branch skips the orphan check and
unconditionally triggers complete(&drvdata->send_cmd_complete). This
prematurely completes the new command's wait, potentially returning stale
or uninitialized data.

[ ... ]
> static int __claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
>                                    size_t len, unsigned int timeout)
> {
>         unsigned char *dmabuf __free(kfree) = NULL;
[ ... ]
> err:
>         scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
>                 drvdata->waiting_cmd = CLAW_COMMAND_TYPE_NONE;
>                 drvdata->orphan_ack_pending = false;
>         }
>         return ret;
> }

[Severity: Low]
This is a pre-existing issue, but does this function mix goto-based cleanup
labels with __free() automated cleanup?

The function declares unsigned char *dmabuf __free(kfree) = NULL; but also
utilizes a goto err; label for state cleanup. The cleanup and guard
subsystem guidelines state that functions should either convert all
resources to scope-based cleanup or convert none of them.

[ ... ]
>  static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
>  					    int n)
>  {
>  	struct hid_device *hdev = to_hid_device(kobj_to_dev(kobj));
>  	struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
> 
>  	scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
>  		/* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
>  		if (!smp_load_acquire(&drvdata->gp_registered))
>  			return 0;
>  	}

[Severity: High]
This is a pre-existing issue, but are the gamepad sysfs attributes ever
created and made visible?

In cfg_setup_fn(), device_add_group() is called before gp_registered is
set to true. During sysfs group creation, this callback evaluates
!smp_load_acquire(&drvdata->gp_registered) and returns 0.

Because it returns 0 during creation, the files are never created. Even
though gp_registered is set to true immediately afterward, the attributes
will remain permanently hidden since sysfs_update_group is not called.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720031549.2272658-1-derekjohn.clark@gmail.com?part=2

^ permalink raw reply

* Re: [PATCH v5] Input: gunze: replace deprecated APIs and fix warning style
From: sashiko-bot @ 2026-07-20  3:32 UTC (permalink / raw)
  To: Bivash Kumar Singh; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260720032407.4644-1-bivashraj750@gmail.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Off-by-one buffer overflow when NUL-terminating the receive buffer corrupts the device physical path.
- [High] Early return on coordinate parsing failure drops malformed release packets, permanently sticking the touch state.
--

--- Patch [1]: [PATCH v5] Input: gunze: replace deprecated APIs and fix warning style ---
commit e46d9a0baa3ae281ee5b9c02aad4e7f63a5cf33f
Author: Bivash Kumar Singh <bivashraj750@gmail.com>

Input: gunze: replace deprecated APIs and fix warning style

This commit updates the driver to use dev_warn_ratelimited() instead of
printk() and replaces simple_strtoul() with sscanf() for parsing coordinate
data. It also NUL-terminates the receive buffer to allow safe string parsing.

> diff --git a/drivers/input/touchscreen/gunze.c b/drivers/input/touchscreen/gunze.c
> index 2baeb4f3b941c..7a92052eba805 100644
> --- a/drivers/input/touchscreen/gunze.c
> +++ b/drivers/input/touchscreen/gunze.c
> @@ -41,15 +41,20 @@ struct gunze {
>  static void gunze_process_packet(struct gunze *gunze)
>  {
>  	struct input_dev *dev = gunze->dev;
> +	unsigned int x, y;
>  
> -	if (gunze->idx != GUNZE_MAX_LENGTH || gunze->data[5] != ',' ||
> -		(gunze->data[0] != 'T' && gunze->data[0] != 'R')) {
> -		printk(KERN_WARNING "gunze.c: bad packet: >%.*s<\n", GUNZE_MAX_LENGTH, gunze->data);
> +	if (gunze->data[0] != 'T' && gunze->data[0] != 'R') {
> +		dev_warn_ratelimited(&gunze->serio->dev, "bad packet: >%s<\n", gunze->data);
>  		return;
>  	}
>  
> -	input_report_abs(dev, ABS_X, simple_strtoul(gunze->data + 1, NULL, 10));
> -	input_report_abs(dev, ABS_Y, 1024 - simple_strtoul(gunze->data + 6, NULL, 10));
> +	if (sscanf(gunze->data + 1, "%4u,%4u", &x, &y) != 2) {
> +		dev_warn_ratelimited(&gunze->serio->dev, "bad packet: >%s<\n", gunze->data);
> +		return;
> +	}

[Severity: High]
Does this early return risk leaving the touch state permanently stuck?

If a release packet (starting with 'R') is received but the coordinate data
contains noise causing sscanf() to fail, this will return early and skip the
input_report_key() call below, dropping the release event.

> +
> +	input_report_abs(dev, ABS_X, x);
> +	input_report_abs(dev, ABS_Y, 1024 - y);
>  	input_report_key(dev, BTN_TOUCH, gunze->data[0] == 'T');
>  	input_sync(dev);
>  }
> @@ -60,6 +65,7 @@ static irqreturn_t gunze_interrupt(struct serio *serio,
>  	struct gunze *gunze = serio_get_drvdata(serio);
>  
>  	if (data == '\r') {
> +		gunze->data[gunze->idx] = '\0';

[Severity: High]
Can this write past the end of the data array and corrupt the physical path?

The data array in struct gunze is sized exactly to GUNZE_MAX_LENGTH (10):

struct gunze {
	...
	unsigned char data[GUNZE_MAX_LENGTH];
	char phys[32];
};

When a full-length valid touch packet is received followed by a '\r',
gunze->idx will be 10. Writing '\0' at this index will write out of bounds
and overwrite the adjacent phys array. Could the array size be increased to
accommodate the NUL terminator?

>  		gunze_process_packet(gunze);
>  		gunze->idx = 0;
>  	} else {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720032407.4644-1-bivashraj750@gmail.com?part=1

^ permalink raw reply

* [PATCH] Input: byd - synchronize timer deletion before freeing private data
From: Linmao Li @ 2026-07-20  3:30 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Kees Cook, Vivek BalachandharTN, Richard Pospesel, Chris Diamand,
	linux-input, linux-kernel, Linmao Li, stable

byd_disconnect() uses timer_delete() before freeing the driver's private
data.  This does not wait for a running byd_clear_touch() callback, which
dereferences the private data and its psmouse pointer.  A callback racing
with disconnect can therefore access the private data after it has been
freed.

Use timer_delete_sync() to wait for any running callback before releasing
the private data.

Fixes: 2d5f5611dd0d ("Input: byd - enable absolute mode")
Cc: stable@vger.kernel.org
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 drivers/input/mouse/byd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c
index f5770a3af2f1..7a69724c1ac6 100644
--- a/drivers/input/mouse/byd.c
+++ b/drivers/input/mouse/byd.c
@@ -423,7 +423,7 @@ static void byd_disconnect(struct psmouse *psmouse)
 	struct byd_data *priv = psmouse->private;
 
 	if (priv) {
-		timer_delete(&priv->timer);
+		timer_delete_sync(&priv->timer);
 		kfree(psmouse->private);
 		psmouse->private = NULL;
 	}
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH v13 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: sashiko-bot @ 2026-07-20  3:30 UTC (permalink / raw)
  To: Derek J. Clark; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260720031549.2272658-5-derekjohn.clark@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The function `__claw_hw_output_report` violates the cleanup subsystem rules by mixing `goto`-based error handling with scope-based cleanup (`__free(kfree)`).
--

--- Patch [4]: [PATCH v13 4/4] HID: hid-msi: Add Rumble Intensity Attributes ---
commit 9f513aab0b37d6357132f5ca7a76ad690510bbc7
Author: Derek J. Clark <derekjohn.clark@gmail.com>

HID: hid-msi: Add Rumble Intensity Attributes

This commit adds intensity adjustment attributes for the left and right rumble
motors on the MSI Claw device. It introduces sysfs store and show functions
to read and write rumble profile data to the hardware.

Link: https://patch.msgid.link/20260529072111.7565-5-derekjohn.clark@gmail.com

> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 5cb85fc2f0534..3225d3bf61254 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -991,6 +1023,162 @@ static ssize_t button_mapping_options_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(button_mapping_options);
>  
> +static ssize_t rumble_intensity_left_store(struct device *dev,
> +					   struct device_attribute *attr,
> +					   const char *buf, size_t count)
> +{
[ ... ]
> +	ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
> +				    (u8 *)&report, sizeof(report), 25);
> +	if (ret)
> +		return ret;

[Severity: Low]
Does __claw_hw_output_report() mix goto-based error handling with scope-based
cleanup?

Looking at __claw_hw_output_report(), which is called via
claw_hw_output_report() here:

	unsigned char *dmabuf __free(kfree) = NULL;
...
	ret = hid_hw_output_report(hdev, dmabuf, CLAW_PACKET_SIZE);
	if (ret < 0)
		goto err;
...
err:
	scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
		drvdata->waiting_cmd = CLAW_COMMAND_TYPE_NONE;
		drvdata->orphan_ack_pending = false;
	}
	return ret;

The cleanup subsystem guidelines in include/linux/cleanup.h state that the
expectation is to never mix "goto" and cleanup helpers in the same function.

Is it possible to convert the error path to use scope-based cleanup entirely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720031549.2272658-1-derekjohn.clark@gmail.com?part=4

^ permalink raw reply

* Re: [PATCH v13 3/4] HID: hid-msi: Add RGB control interface
From: sashiko-bot @ 2026-07-20  3:30 UTC (permalink / raw)
  To: Derek J. Clark; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260720031549.2272658-4-derekjohn.clark@gmail.com>

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

New issues:
- [Medium] Data races and locking mismatches when accessing RGB configuration variables and LED core state.

Pre-existing issues:
- [High] Use-after-free on device teardown due to missing cancellation of delayed works.
--

--- Patch [3]: [PATCH v13 3/4] HID: hid-msi: Add RGB control interface ---
commit d67de53a026c42fe9260a66fca23392240a52c26
Author: Derek J. Clark <derekjohn.clark@gmail.com>

HID: hid-msi: Add RGB control interface

This commit adds an RGB control interface for MSI Claw devices. It provides
support for configuring up to 9 LED zones via an MCU memory interface and
sets up the corresponding sysfs attributes and LED class device.

Link: https://patch.msgid.link/20260529072111.7565-4-derekjohn.clark@gmail.com

> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 023d63cee6b06..5cb85fc2f0534 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c

[ ... ]

> @@ -349,6 +410,38 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
>  		for (i = 0; i < CLAW_KEYS_MAX; i++)
>  			codes[i] = (mkeys->codes[i]);
>  		break;
> +	case CLAW_RGB_PENDING:
> +		frame = (struct claw_rgb_report *)cmd_rep->data;

[ ... ]

> +		scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
> +			memcpy(&drvdata->rgb_frames[f_idx], &frame->zone_data,
> +			       sizeof(struct rgb_frame));
> +
> +			/* Only use frame 0 for remaining variable assignment */
> +			if (f_idx != 0)
> +				break;
> +
> +			drvdata->rgb_speed = frame->speed;
> +			drvdata->led_mc.led_cdev.brightness = frame->brightness;
> +			drvdata->led_mc.subled_info[0].intensity = frame->zone_data.zone[0].red;
> +			drvdata->led_mc.subled_info[1].intensity = frame->zone_data.zone[0].green;
> +			drvdata->led_mc.subled_info[2].intensity = frame->zone_data.zone[0].blue;
> +		}
> +
> +		break;

[Severity: Medium]
Is there a locking mismatch for rgb_speed? Here in claw_profile_event(),
rgb_speed is updated while holding frame_lock. However, in speed_store(),
it is updated while holding profile_lock.

Also, does this direct modification of the LED class device internal state
need synchronization? Updating brightness and subled_info[x].intensity here
is done without holding the standard led_access lock used by the LED core,
which could cause data races with concurrent sysfs accesses.

[ ... ]

> +/* Send RGB configuration to device */
> +static int claw_write_rgb_state(struct claw_drvdata *drvdata)
> +{
> +	struct claw_rgb_report report = { {0x01, 0}, CLAW_RGB_FRAME_OFFSET, 0x00,
> +			drvdata->rgb_frame_count, 0x09, drvdata->rgb_speed,
> +			drvdata->led_mc.led_cdev.brightness };

[Severity: Medium]
Does claw_write_rgb_state() need to hold a lock when reading rgb_speed?
It appears to be read locklessly here, but is updated with a lock elsewhere.
This also applies to other variables like rgb_effect and rgb_enabled read
locklessly in claw_apply_rgb_state() and sysfs show functions.

[ ... ]

> +static ssize_t speed_store(struct device *dev, struct device_attribute *attr,
> +			   const char *buf, size_t count)
> +{

[ ... ]

> +	/* 0 is fastest, invert value for intuitive userspace speed */
> +	speed = 20 - val;
> +
> +	scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
> +		drvdata->rgb_speed = speed;
> +
> +	mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
> +
> +	return count;
> +}

[Severity: Medium]
This is where rgb_speed is written using profile_lock instead of the
frame_lock used in claw_profile_event().

[ ... ]

> @@ -1088,6 +1727,7 @@ static void claw_remove(struct hid_device *hdev)
>  {
>  	struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
>  	bool gp_registered;
> +	bool rgb_registered;
>  
>  	if (!drvdata)
>  		return;

[ ... ]

>  	if (gp_registered)
>  		device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
>  
> +	if (rgb_registered) {
> +		device_remove_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
> +		led_classdev_multicolor_unregister(&drvdata->led_mc);
> +	}
> +	cancel_delayed_work_sync(&drvdata->rgb_queue);
> +
>  	hid_hw_close(hdev);
>  }

[Severity: High]
This is a pre-existing issue, but does this teardown path leak delayed works?
The driver initializes the cfg_setup and cfg_resume delayed works during
probe, but they don't appear to be cancelled here in claw_remove().

If the device is removed while either of those works is pending, could they
execute after the devres-allocated drvdata is freed, causing a
use-after-free crash?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720031549.2272658-1-derekjohn.clark@gmail.com?part=3

^ permalink raw reply

* Re: [PATCH v13 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: sashiko-bot @ 2026-07-20  3:27 UTC (permalink / raw)
  To: Derek J. Clark; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260720031549.2272658-2-derekjohn.clark@gmail.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Delayed generic ACKs from a previous timed-out command can falsely complete a subsequent command expecting a specific ACK, leading to silent state corruption.
- [Medium] Mixing `goto`-based error handling with scope-based `__free()` cleanup in the same function.
- [Medium] Sysfs attributes can spuriously return `-ENODEV` immediately after creation because they are exposed before `gp_registered` is set to true.
--

--- Patch [1]: [PATCH v13 1/4] HID: hid-msi: Add MSI Claw configuration driver ---
commit 12a4a941e97290e1a15080092de490d5782eef1c
Author: Derek J. Clark <derekjohn.clark@gmail.com>

HID: hid-msi: Add MSI Claw configuration driver

Adds configuration HID driver for the MSI Claw series of handheld PC's.
In this initial patch add the initial driver outline and attributes for
changing the gamepad mode, M-key behavior, and add a WO reset function.

> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> +static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
> +			  u8 *data, int size)
> +{
[ ... ]
> +	switch (cmd_rep->cmd) {
> +	case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
> +		ret = claw_gamepad_mode_event(drvdata, cmd_rep);
> +		if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE) {
> +			drvdata->cmd_status = ret;
> +			complete(&drvdata->send_cmd_complete);
> +		}
> +
> +		break;
> +	case CLAW_COMMAND_TYPE_ACK:
> +		if (drvdata->orphan_ack_pending) {
> +			drvdata->orphan_ack_pending = false;
> +			complete(&drvdata->orphan_ack_complete);
> +			break;
> +		}
> +
> +		if (drvdata->waiting_cmd == CLAW_COMMAND_TYPE_NONE) {
> +			dev_warn(&drvdata->hdev->dev, "Got unexpected ACK from MCU, ignoring\n");
> +			break;
> +		}
> +
> +		drvdata->cmd_status = 0;
> +		complete(&drvdata->send_cmd_complete);

[Severity: Medium]
When claw_raw_event() receives a generic ACK, it only verifies that
waiting_cmd is not CLAW_COMMAND_TYPE_NONE before calling
complete(&drvdata->send_cmd_complete).

If a previous command with timeout == 0 timed out, __claw_hw_output_report()
clears orphan_ack_pending.

If the generic ACK for that command arrives later while a new command is
waiting (e.g., waiting for GAMEPAD_MODE_ACK), this logic will accept the
generic ACK and falsely complete the new command.

Could this lead to silent state corruption where commands finish early without
receiving their actual payloads?

[ ... ]
> +static int __claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
> +				   size_t len, unsigned int timeout)
> +{
> +	unsigned char *dmabuf __free(kfree) = NULL;
[ ... ]
> +	ret = hid_hw_output_report(hdev, dmabuf, CLAW_PACKET_SIZE);
> +	if (ret < 0)
> +		goto err;
> +
> +	ret = ret == CLAW_PACKET_SIZE ? 0 : -EIO;
> +	if (ret)
> +		goto err;

[Severity: Medium]
This function uses the scope-based __free(kfree) cleanup for dmabuf, but also
uses manual goto err labels for error handling.

The cleanup subsystem guidelines specify that using goto and cleanup helpers
like __free() in the same function creates confusing ownership semantics and is
an anti-pattern.

Could we use a scope-based guard for the state machine cleanup, or stick
entirely to manual goto cleanup?

[ ... ]
> +static void cfg_setup_fn(struct work_struct *work)
> +{
[ ... ]
> +	/* Add sysfs attributes after we get the device state */
> +	ret = device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
> +	if (ret) {
> +		dev_err(&drvdata->hdev->dev,
> +			"Failed to setup device, can't create gamepad attrs: %d\n", ret);
> +		return;
> +	}
> +	scoped_guard(spinlock_irqsave, &drvdata->registration_lock)
> +		/* Pairs with smp_load_acquire in attribute show/store functions */
> +		smp_store_release(&drvdata->gp_registered, true);

[Severity: Medium]
The sysfs attributes are exposed to userspace by device_add_group() before
gp_registered is set to true.

If an automated userspace tool reacts to the sysfs creation event and accesses
the attributes in this small timing window, the attribute show/store functions
will observe gp_registered == false and return -ENODEV.

Would it be better to set gp_registered to true before calling
device_add_group(), and revert it to false if the addition fails?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720031549.2272658-1-derekjohn.clark@gmail.com?part=1

^ permalink raw reply

* [PATCH v5] Input: gunze: replace deprecated APIs and fix warning style
From: Bivash Kumar Singh @ 2026-07-20  3:24 UTC (permalink / raw)
  To: linux-input; +Cc: dmitry.torokhov, Bivash Kumar Singh
In-Reply-To: <20260718190040.10613-1-bivashraj750@gmail.com>

Replace printk(KERN_WARNING) with dev_warn_ratelimited() using the
serio device, which is the correct logging style for driver code and
prevents log spam on noisy serial lines.

Replace simple_strtoul() with sscanf() to parse the comma-separated
coordinate data. NUL-terminate the receive buffer in gunze_interrupt()
before calling gunze_process_packet() so sscanf() and the warning
message can safely treat the data as a string.

Signed-off-by: Bivash Kumar Singh <bivashraj750@gmail.com>
---
Changes in v5:
  - Switch from kstrtoul() to sscanf() as suggested by Dmitry Torokhov.
    sscanf() naturally handles comma-separated coordinate data without
    needing a local buffer copy or manual NUL termination tricks.
  - NUL-terminate receive buffer in gunze_interrupt() before processing.
  - Use dev_warn_ratelimited() to prevent log spam on noisy serial lines.

Changes in v4:
  - Remove early return on kstrtoul() failure to prevent touch state
    getting permanently stuck if a release packet contains noisy data.
    Initialize x and y to 0 as safe fallback values instead.
    (reported by Sashiko AI review)

Changes in v3:
  - Fix comment style: add space after /* and use NUL instead of NULL
  - Add missing Changes section that was absent in v2

Changes in v2:
  - Copy packet data to a local NUL-terminated buffer before calling
    kstrtoul(), so the comma separator does not cause parsing to fail
    on every valid touch event. (reported by Sashiko AI review)
---
 drivers/input/touchscreen/gunze.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/gunze.c b/drivers/input/touchscreen/gunze.c
index 2baeb4f3b941..7a92052eba80 100644
--- a/drivers/input/touchscreen/gunze.c
+++ b/drivers/input/touchscreen/gunze.c
@@ -41,15 +41,20 @@ struct gunze {
 static void gunze_process_packet(struct gunze *gunze)
 {
 	struct input_dev *dev = gunze->dev;
+	unsigned int x, y;
 
-	if (gunze->idx != GUNZE_MAX_LENGTH || gunze->data[5] != ',' ||
-		(gunze->data[0] != 'T' && gunze->data[0] != 'R')) {
-		printk(KERN_WARNING "gunze.c: bad packet: >%.*s<\n", GUNZE_MAX_LENGTH, gunze->data);
+	if (gunze->data[0] != 'T' && gunze->data[0] != 'R') {
+		dev_warn_ratelimited(&gunze->serio->dev, "bad packet: >%s<\n", gunze->data);
 		return;
 	}
 
-	input_report_abs(dev, ABS_X, simple_strtoul(gunze->data + 1, NULL, 10));
-	input_report_abs(dev, ABS_Y, 1024 - simple_strtoul(gunze->data + 6, NULL, 10));
+	if (sscanf(gunze->data + 1, "%4u,%4u", &x, &y) != 2) {
+		dev_warn_ratelimited(&gunze->serio->dev, "bad packet: >%s<\n", gunze->data);
+		return;
+	}
+
+	input_report_abs(dev, ABS_X, x);
+	input_report_abs(dev, ABS_Y, 1024 - y);
 	input_report_key(dev, BTN_TOUCH, gunze->data[0] == 'T');
 	input_sync(dev);
 }
@@ -60,6 +65,7 @@ static irqreturn_t gunze_interrupt(struct serio *serio,
 	struct gunze *gunze = serio_get_drvdata(serio);
 
 	if (data == '\r') {
+		gunze->data[gunze->idx] = '\0';
 		gunze_process_packet(gunze);
 		gunze->idx = 0;
 	} else {
-- 
2.53.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox