lkml.org 
[lkml]   [2024]   [Apr]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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@amd.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
>



\
 
 \ /
  Last update: 2024-04-29 23:22    [W:0.290 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site