Re: [PATCH v10 7/9] drm/xe/amc: Guard xe_amc_work() with runtime PM

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] cleanup: Mixing goto-based error handling with guard() cleanup
- [Low] build: undeclared identifier 'xe' in xe_amc_work()
--

commit 151af4bf3e553f2ab553af055075b9c228711e6e
Author: Raag Jadav <raag.jadav@intel.com>

drm/xe/amc: Guard xe_amc_work() with runtime PM

This commit adds a runtime PM guard to xe_amc_work() to ensure a runtime
PM reference is held for xe_device_declare_wedged() callers.

> diff --git a/drivers/gpu/drm/xe/xe_amc.c b/drivers/gpu/drm/xe/xe_amc.c
> index 9459c63bc3ca2..662aec6c86a72 100644
> --- a/drivers/gpu/drm/xe/xe_amc.c
> +++ b/drivers/gpu/drm/xe/xe_amc.c
> @@ -113,6 +113,8 @@ static void xe_amc_work(struct work_struct *work)
>  	struct i2c_client *client;
>  	int ret;
>  
> +	guard(xe_pm_runtime)(xe);

[Severity: Low]
Is the variable xe declared in this scope? It looks like this introduces
a compilation failure because xe is passed to the guard macro without being
defined anywhere within xe_amc_work().

[Severity: Medium]
According to the cleanup subsystem guidelines, is it acceptable to mix
goto-based error handling and scope-based cleanup in the same function?

The newly added guard is placed in a function that still uses traditional
goto-based error handling on failure paths, as seen directly below.
Mixing these paradigms creates confusing ownership semantics that can lead
to resource leaks or double-free bugs during future refactoring.

> +
>  	client = amc->i2c->client[XE_I2C_CLIENT_AMC];
>  	if (IS_ERR_OR_NULL(client))
>  		goto out_reassert_interrupt;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016072623.457392-1-raag.jadav@intel.com?part=7