Re: [PATCH] platform/x86: dell-laptop: Implement platform_profile

[Pali Rohár]

On Thursday 25 April 2024 11:27:57 Lyndon Sanche wrote:
> +int thermal_init(void)
> +{
> +	int ret;
> +	int supported_modes;
> +
> +	ret = thermal_get_supported_modes(&supported_modes);
> +
> +	if (ret != 0 || supported_modes == 0)
> +		return -ENXIO;
> +
> +	thermal_handler = kzalloc(sizeof(*thermal_handler), GFP_KERNEL);
> +	if (!thermal_handler)
> +		return -ENOMEM;
> +	thermal_handler->profile_get = thermal_platform_profile_get;
> +	thermal_handler->profile_set = thermal_platform_profile_set;
> +
> +	if ((supported_modes >> DELL_QUIET) & 1)
> +		set_bit(PLATFORM_PROFILE_QUIET, thermal_handler->choices);
> +	if ((supported_modes >> DELL_COOL_BOTTOM) & 1)
> +		set_bit(PLATFORM_PROFILE_COOL, thermal_handler->choices);
> +	if ((supported_modes >> DELL_BALANCED) & 1)
> +		set_bit(PLATFORM_PROFILE_BALANCED, thermal_handler->choices);
> +	if ((supported_modes >> DELL_PERFORMANCE) & 1)
> +		set_bit(PLATFORM_PROFILE_PERFORMANCE, thermal_handler->choices);
> +
> +	platform_profile_register(thermal_handler);
> +
> +	return 0;
> +}
> +
> +void thermal_cleanup(void)
> +{
> +	platform_profile_remove();
> +	kfree(thermal_handler);
> +}
> +
>  static struct led_classdev mute_led_cdev = {
>  	.name = "platform::mute",
>  	.max_brightness = 1,
> @@ -2266,6 +2480,11 @@ static int __init dell_init(void)
>  		mute_led_registered = true;
>  	}
>  
> +	// Do not fail module if thermal modes not supported,
> +	// just skip
> +	if (thermal_init() != 0)
> +		pr_warn("Unable to setup platform_profile, skipping");

I think that -ENOMEM error should be failure of the loading the driver.
It does not make sense to continue of memory allocation failed.

On the other hand when the thermal modes are not support (e.g. old
Latitude models) then there should not be a warning message. It is
expected that on systems without thermal modes the setup fails.

> +
>  	if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
>  		return 0;