Re: [PATCH v6 1/3] drm/xe/pm: s/xe_device_sysfs_init/xe_pm_sysfs_init

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Sat, May 03, 2025 at 12:07:37AM +0530, Raag Jadav wrote:
> Attributes being exposed in xe_pm_init() are PM specific and should
> follow its naming.
> 
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> Reviewed-by: Riana Tauro <riana.tauro@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device_sysfs.c | 6 +++---
>  drivers/gpu/drm/xe/xe_device_sysfs.h | 2 +-
>  drivers/gpu/drm/xe/xe_pm.c           | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
> index 7efbd4c52791..2d25e5b5d4bf 100644
> --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
> @@ -63,14 +63,14 @@ vram_d3cold_threshold_store(struct device *dev, struct device_attribute *attr,
>  
>  static DEVICE_ATTR_RW(vram_d3cold_threshold);
>  
> -static void xe_device_sysfs_fini(void *arg)
> +static void xe_pm_sysfs_fini(void *arg)
>  {
>  	struct xe_device *xe = arg;
>  
>  	sysfs_remove_file(&xe->drm.dev->kobj, &dev_attr_vram_d3cold_threshold.attr);
>  }
>  
> -int xe_device_sysfs_init(struct xe_device *xe)
> +int xe_pm_sysfs_init(struct xe_device *xe)
>  {
>  	struct device *dev = xe->drm.dev;
>  	int ret;
> @@ -79,5 +79,5 @@ int xe_device_sysfs_init(struct xe_device *xe)
>  	if (ret)
>  		return ret;
>  
> -	return devm_add_action_or_reset(dev, xe_device_sysfs_fini, xe);
> +	return devm_add_action_or_reset(dev, xe_pm_sysfs_fini, xe);
>  }
> diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.h b/drivers/gpu/drm/xe/xe_device_sysfs.h
> index f9e83d8bd2c7..2c1fabdba9e4 100644
> --- a/drivers/gpu/drm/xe/xe_device_sysfs.h
> +++ b/drivers/gpu/drm/xe/xe_device_sysfs.h
> @@ -8,6 +8,6 @@
>  
>  struct xe_device;
>  
> -int xe_device_sysfs_init(struct xe_device *xe);
> +int xe_pm_sysfs_init(struct xe_device *xe);
>  
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index 38514cef817e..183813e43b29 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -347,7 +347,7 @@ int xe_pm_init(struct xe_device *xe)
>  	xe->d3cold.capable = xe_pm_pci_d3cold_capable(xe);
>  
>  	if (xe->d3cold.capable) {
> -		err = xe_device_sysfs_init(xe);
> +		err = xe_pm_sysfs_init(xe);

Ah, this patch is the one that is wrong. We should not change the name of this function.
Perpahs we could move the xe_device_sysfs_init to a better place and then have the d3cold capable
check there and ensuring that it is set before...  But this rename here is wrong and we should
not merge it.

Thanks,
Rodrigo.

>  		if (err)
>  			goto err_unregister;
>  
> -- 
> 2.34.1
>