Received-SPF: pass (google.com: domain of linux-kernel+bounces-159103-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 25 Apr 2024 22:12:13 +0200
From: Pali =?utf-8?B?Um9ow6Fy?= <pali@kernel.org>
To: Lyndon Sanche <lsanche@lyndeno.ca>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Ilpo =?utf-8?B?SsOkcnZpbmVu?= <ilpo.jarvinen@linux.intel.com>,
	platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org,
	Dell.Client.Kernel@dell.com
Subject: Re: [PATCH] platform/x86: dell-laptop: Implement platform_profile
Message-ID: <20240425201213.apwdxazbdbjddtpr@pali>
References: <20240425172758.67831-1-lsanche@lyndeno.ca>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240425172758.67831-1-lsanche@lyndeno.ca>
User-Agent: NeoMutt/20180716

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;