linux-kernel - Re: [PATCH v3] 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]

Message-Id: <NC3QCS.LEAYSBJ8Y8GC1@lyndeno.ca>
Date: Mon, 29 Apr 2024 15:21:59 -0600
From: Lyndon Sanche <lsanche@...deno.ca>
To: Mario Limonciello <mario.limonciello@....com>
Cc: pali@...nel.org, W_Armin@....de, srinivas.pandruvada@...ux.intel.com,
	ilpo.jarvinen@...ux.intel.com, Matthew Garrett <mjg59@...f.ucam.org>,
	Hans de Goede <hdegoede@...hat.com>, platform-driver-x86@...r.kernel.org,
	linux-kernel@...r.kernel.org, Dell.Client.Kernel@...l.com
Subject: Re: [PATCH v3] platform/x86: dell-laptop: Implement platform_profile



On Mon, Apr 29 2024 at 12:45:19 PM -05:00:00, Mario Limonciello 
<mario.limonciello@....com> wrote:
> On 4/29/2024 11:48, Lyndon Sanche wrote:
>>   #include <linux/i8042.h>
>>   #include <linux/debugfs.h>
>>   #include <linux/seq_file.h>
>> +#include <linux/platform_profile.h>
>> +#include <linux/bitfield.h>
> 
> These should be inserted in alphabetical order.

Agree

>> +
>> +	// Clean up but do not fail
> 
> Switch comment style to /* */

Agree

> 
>> +	if (platform_profile_register(thermal_handler))
>> +		kfree(thermal_handler);
> 
> Don't you also want to return an error in this case?  Because this 
> means that the platform supports thermal modes but it failed to setup 
> due to other issues.
> 
> It's different than the case of no supported modes in which case 
> returning 0 makes sense.
> 
> Maybe like this:
> 
> 
> ret = platform_profile_register(thermal_handler);
> if (ret)
> 	kfree(thermal_handler);
> 
> return ret;

Good idea, I will propogate this error. Failure of module will then 
occur on allocation or platform_profile error.

> 
> 
>> 
>>   		goto fail_rfkill;
>>   	}
>>   +	// Do not fail module if thermal modes not supported, just skip
> 
> Switch comment style to /* */

Agree.

Thank you for this feedback.

Lyndon
>