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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 25 Apr 2024 22:12:13 +0200
From: Pali Rohár <pali@...nel.org>
To: Lyndon Sanche <lsanche@...deno.ca>
Cc: Matthew Garrett <mjg59@...f.ucam.org>,
	Hans de Goede <hdegoede@...hat.com>,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
	Dell.Client.Kernel@...l.com
Subject: Re: [PATCH] platform/x86: dell-laptop: Implement platform_profile

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;