Re: [PATCH v2] thermal: Fail object registration if thermal class is not registered

[Zhang, Rui]

On Mon, 2023-01-23 at 21:44 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> 
> If thermal_class is not registered with the driver core, there is no
> way
> to expose the interfaces used by the thermal control framework, so
> prevent thermal zones and cooling devices from being registered in
> that case by returning an error from object registration functions.
> 
> For this purpose, use a thermal_class pointer that will be NULL if
> the
> class is not registered.  To avoid wasting memory in that case,
> allocate
> the thermal class object dynamically and if it fails to register,
> free
> it and clear the thermal_class pointer to NULL.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

Reviewed-by: Zhang Rui <rui.zhang@xxxxxxxxx>

thanks,
rui
> ---
> 
> -> v2: This replaces the series at
>        https://lore.kernel.org/linux-pm/5905717.lOV4Wx5bFT@kreacher/
> 
> ---
>  drivers/thermal/thermal_core.c |   31 +++++++++++++++++++++++-------
> -
>  1 file changed, 23 insertions(+), 8 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -774,10 +774,7 @@ static void thermal_release(struct devic
>  	}
>  }
>  
> -static struct class thermal_class = {
> -	.name = "thermal",
> -	.dev_release = thermal_release,
> -};
> +static struct class *thermal_class;
>  
>  static inline
>  void print_bind_err_msg(struct thermal_zone_device *tz,
> @@ -880,6 +877,9 @@ __thermal_cooling_device_register(struct
>  	    !ops->set_cur_state)
>  		return ERR_PTR(-EINVAL);
>  
> +	if (!thermal_class)
> +		return ERR_PTR(-ENODEV);
> +
>  	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
>  	if (!cdev)
>  		return ERR_PTR(-ENOMEM);
> @@ -901,7 +901,7 @@ __thermal_cooling_device_register(struct
>  	cdev->np = np;
>  	cdev->ops = ops;
>  	cdev->updated = false;
> -	cdev->device.class = &thermal_class;
> +	cdev->device.class = thermal_class;
>  	cdev->devdata = devdata;
>  
>  	ret = cdev->ops->get_max_state(cdev, &cdev->max_state);
> @@ -1349,6 +1349,9 @@ thermal_zone_device_register_with_trips(
>  	if (num_trips > 0 && (!ops->get_trip_type || !ops-
> >get_trip_temp) && !trips)
>  		return ERR_PTR(-EINVAL);
>  
> +	if (!thermal_class)
> +		return ERR_PTR(-ENODEV);
> +
>  	tz = kzalloc(sizeof(*tz), GFP_KERNEL);
>  	if (!tz)
>  		return ERR_PTR(-ENOMEM);
> @@ -1370,7 +1373,7 @@ thermal_zone_device_register_with_trips(
>  
>  	tz->ops = ops;
>  	tz->tzp = tzp;
> -	tz->device.class = &thermal_class;
> +	tz->device.class = thermal_class;
>  	tz->devdata = devdata;
>  	tz->trips = trips;
>  	tz->num_trips = num_trips;
> @@ -1615,9 +1618,21 @@ static int __init thermal_init(void)
>  	if (result)
>  		goto error;
>  
> -	result = class_register(&thermal_class);
> -	if (result)
> +	thermal_class = kzalloc(sizeof(*thermal_class), GFP_KERNEL);
> +	if (!thermal_class) {
> +		result = -ENOMEM;
> +		goto unregister_governors;
> +	}
> +
> +	thermal_class->name = "thermal";
> +	thermal_class->dev_release = thermal_release;
> +
> +	result = class_register(thermal_class);
> +	if (result) {
> +		kfree(thermal_class);
> +		thermal_class = NULL;
>  		goto unregister_governors;
> +	}
>  
>  	result = register_pm_notifier(&thermal_pm_nb);
>  	if (result)
> 
> 
>