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

[sashiko-bot@kernel.org]

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