Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing `synchronize_irq()` in the runtime suspend callback for a device using shared IRQs. -- commit badac0ffd08d84278aad7a9068c91e38322fa44f Author: ZhaoJinming <[email protected]> accel/rocket: Fix error path handling in rocket_job_run() This commit fixes bugs in the error paths of rocket_job_run() by replacing pm_runtime_get_sync() with pm_runtime_resume_and_get() to prevent leaking the runtime PM reference. It also properly releases fence references on error and returns ERR_PTR instead of an unsignaled fence. > diff --git a/drivers/accel/rocket/rocket_job.c > b/drivers/accel/rocket/rocket_job.c > index 2f1861f960ccb..cf9f61682dd8b 100644 > --- a/drivers/accel/rocket/rocket_job.c > +++ b/drivers/accel/rocket/rocket_job.c [ ... ] > @@ -310,13 +310,22 @@ static struct dma_fence *rocket_job_run(struct > drm_sched_job *sched_job) > dma_fence_put(job->done_fence); > job->done_fence = dma_fence_get(fence); > > - ret = pm_runtime_get_sync(core->dev); > - if (ret < 0) > - return fence; > + ret = pm_runtime_resume_and_get(core->dev); [Severity: High] This isn't a bug introduced by this patch, but since we are modifying the runtime PM handling here, I noticed a regression in the runtime suspend path: drivers/accel/rocket/rocket_drv.c:rocket_device_runtime_suspend() { ... if (!rocket_job_is_idle(&rdev->cores[core])) return -EBUSY; clk_bulk_disable_unprepare(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks); return 0; } Since the device's IRQ is registered with the IRQF_SHARED flag, should synchronize_irq() be called before gating the device clocks? Without synchronization, could a concurrently running IRQ handler on another CPU be halfway through executing when the clocks are abruptly disabled, potentially causing an external abort when the handler attempts its next MMIO access? I saw that the IRQ handler was updated in a later commit ("accel/rocket: Fix iommu_group leak and unsafe IRQ register access") to use pm_runtime_get_if_active(), but doesn't the PM subsystem still mandate this synchronization pattern for all drivers using IRQF_SHARED to guarantee no handler is executing mid-flight during power down? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
