[PATCH 0/2] accel: Remove redundant error messages on IRQ request failure

[PATCH 0/2] accel: Remove redundant error messages on IRQ request failure

[Pan Chuang]

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 accel drivers that follow these devm_request_*_irq()
functions, as the core now provides more detailed diagnostic
information on failure.

Pan Chuang (2):
  accel: ethosu: Remove redundant dev_err()
  accel: rocket: Remove redundant dev_err()

 drivers/accel/ethosu/ethosu_job.c | 4 +---
 drivers/accel/rocket/rocket_job.c | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH 1/2] accel: ethosu: Remove redundant dev_err()

[Pan Chuang]

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/accel/ethosu/ethosu_job.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
index 74f3493fb11a..cdc0d61ee53d 100644
--- a/drivers/accel/ethosu/ethosu_job.c
+++ b/drivers/accel/ethosu/ethosu_job.c
@@ -338,10 +338,8 @@ int ethosu_job_init(struct ethosu_device *edev)
 					ethosu_job_irq_handler_thread,
 					IRQF_SHARED, KBUILD_MODNAME,
 					edev);
-	if (ret) {
-		dev_err(dev, "failed to request irq\n");
+	if (ret)
 		return ret;
-	}
 
 	edev->fence_context = dma_fence_context_alloc(1);
 
-- 
2.34.1

Re: [PATCH 1/2] accel: ethosu: Remove redundant dev_err()

[sashiko-bot]

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

Pre-existing issues:
- [High] Calling `drm_sched_fini()` after `drm_sched_init()` fails causes a kernel panic due to uninitialized structures.
--

--- Patch [1]: [PATCH 1/2] accel: ethosu: Remove redundant dev_err() ---
commit ab2aa317daa0785d0207a88d5d5b51fcef216f77
Author: Pan Chuang <panchuang@vivo.com>

accel: ethosu: Remove redundant dev_err()

This commit removes redundant driver-specific error logging after
devm_request_threaded_irq(), relying on the core IRQ code to automatically
log failures.

> diff --git a/drivers/accel/ethosu/ethosu_job.c b/drivers/accel/ethosu/ethosu_job.c
> --- a/drivers/accel/ethosu/ethosu_job.c
> +++ b/drivers/accel/ethosu/ethosu_job.c
> @@ -338,10 +338,8 @@ int ethosu_job_init(struct ethosu_device *edev)
[ ... ]
> -	if (ret) {
> -		dev_err(dev, "failed to request irq\n");
> +	if (ret)
>  		return ret;
> -	}
>  
>  	edev->fence_context = dma_fence_context_alloc(1);

[Severity: High]
This is a pre-existing issue, but does the error handling path later in
ethosu_job_init() cause a kernel panic if drm_sched_init() fails?

If drm_sched_init() returns an error (for example, due to an out-of-memory
condition), the code jumps to the error label:

drivers/accel/ethosu/ethosu_job.c:ethosu_job_init() {
    ...
    ret = drm_sched_init(&edev->sched, &args);
    if (ret) {
        dev_err(dev, "Failed to create scheduler: %d\n", ret);
        goto err_sched;
    }
    ...
err_sched:
    drm_sched_fini(&edev->sched);
    return ret;
}

At this point, the job_scheduled waitqueue inside drm_gpu_scheduler remains
completely zero-initialized. When drm_sched_fini() attempts to wake up tasks
by calling wake_up_all(&sched->job_scheduled), the zero-initialized list_head
means its next pointer is NULL. The list_empty() check incorrectly returns
false, and the subsequent list iteration dereferences the NULL pointer.

Could this lead to an immediate panic during device probe failures?

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

[PATCH 2/2] accel: rocket: Remove redundant dev_err()

[Pan Chuang]

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/accel/rocket/rocket_job.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index 2f1861f960cc..8619981782ed 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -457,10 +457,8 @@ int rocket_job_init(struct rocket_core *core)
 					rocket_job_irq_handler_thread,
 					IRQF_SHARED, dev_name(core->dev),
 					core);
-	if (ret) {
-		dev_err(core->dev, "failed to request job irq");
+	if (ret)
 		return ret;
-	}
 
 	core->reset.wq = alloc_ordered_workqueue("rocket-reset-%d", 0, core->index);
 	if (!core->reset.wq)
-- 
2.34.1

Re: [PATCH 2/2] accel: rocket: Remove redundant dev_err()

[sashiko-bot]

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

Pre-existing issues:
- [High] Unsafe register access in shared IRQ handler when device is runtime-suspended or unpowered.
- [High] Calling `drm_sched_fini()` on a partially initialized scheduler during error handling leads to a crash.
--

--- Patch [2]: [PATCH 2/2] accel: rocket: Remove redundant dev_err() ---
commit 8d11ad4c2f7d7988ccd404acec217989feeb7613
Author: Pan Chuang <panchuang@vivo.com>

accel: rocket: Remove redundant dev_err()

This commit removes a redundant dev_err() call after 
devm_request_threaded_irq(), as the core already logs detailed error 
messages automatically on failure.

> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -468,10 +468,8 @@ int rocket_job_init(struct rocket_core *core)
>  	ret = devm_request_threaded_irq(core->dev, core->irq,
>  					rocket_job_irq_handler,
>  					rocket_job_irq_handler_thread,
>  					IRQF_SHARED, dev_name(core->dev),
>  					core);

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to register this
IRQ as shared before the device is powered on, or to keep it active during
runtime suspend?

If another device sharing the line triggers an interrupt,
rocket_job_irq_handler() will unconditionally read hardware registers:

    u32 raw_status = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);

Could this cause a bus fault if the device is unclocked or unpowered at the
time the shared interrupt fires?

> -	if (ret) {
> -		dev_err(core->dev, "failed to request job irq");
> +	if (ret)
>  		return ret;
> -	}
>  
>  	core->reset.wq = alloc_ordered_workqueue("rocket-reset-%d", 0, core->index);

[Severity: High]
This is a pre-existing issue, but can calling drm_sched_fini() in the error
handling path of this function cause a panic if drm_sched_init() fails?

Looking further down in rocket_job_init(), if drm_sched_init() fails (e.g.,
due to memory allocation failure), the code jumps to err_sched:

    err_sched:
        drm_sched_fini(&core->sched);

If drm_sched_init() fails early, core->sched.job_scheduled.head.next might
still be NULL. When drm_sched_fini() calls wake_up_all(&sched->job_scheduled),
will it evaluate waitqueue_active() on a zeroed wait queue and trigger a NULL
pointer dereference?

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