lkml.org 
[lkml]   [2024]   [Apr]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4] platform/x86: dell-laptop: Implement platform_profile
On Tue, Apr 30, 2024, at 7:36 PM, Pali Rohár wrote:
> On Tuesday 30 April 2024 19:14:52 Lyndon Sanche wrote:
>> + * Bit 2 AAC (Quiet)
>> + * Bit 3 AAC (Performance)
>> + * cbRes3, byte 3 Current Fan Failure Mode
>> + * Bit 0 Minimal Fan Failure (at least one fan has failed, one fan working)
>> + * Bit 1 Catastrophic Fan Failure (all fans have failed)
>> + * cbArg1 0x1 (Set Thermal Information), both desired thermal mode and
>
> Broken indentation. cbArg1 should have only one space after "*" and be
> at the same level as the previous cbArg1 description.
>
> And I would suggest to add a newline before cbArg1 as it start
> description of the next command.
>

I will fix this.

>> + * desired AAC mode shall be applied
>> + * cbArg2, byte 0 Desired Thermal Mode to set
>> + * (only one bit may be set for this parameter)
>> + * Bit 0 Balanced
>> + * Bit 1 Cool Bottom
>> + * Bit 2 Quiet
>> + * Bit 3 Performance
>> + * cbArg2, byte 1 Desired Active Acoustic Controller (AAC) Mode to set
>> + * If AAC Configuration Type is Global,
>> + * 0 AAC mode disabled
>> + * 1 AAC mode enabled
>> + *
>
> And here I would suggest to remove empty line as the comment below
> belongs to the AAC description above the empty line.
>

Agreed.

>>
>> /* dell-rbtn.c driver export functions which will not work correctly (and could
>> diff --git a/drivers/platform/x86/dell/dell-smbios-base.c b/drivers/platform/x86/dell/dell-smbios-base.c
>> index e61bfaf8b5c4..40aa4469b38b 100644
>> --- a/drivers/platform/x86/dell/dell-smbios-base.c
>> +++ b/drivers/platform/x86/dell/dell-smbios-base.c
>> @@ -9,6 +9,7 @@
>> * Based on documentation in the libsmbios package:
>> * Copyright (C) 2005-2014 Dell Inc.
>> */
>> +#include "linux/wmi.h"
>
> Is this include file really used? Because only SELECT_THERMAL_MANAGEMENT
> was added and it is in the dell-smbios.h. And others constants like
> SELECT_KBD_BACKLIGHT did not needed it.
>

No it is not. It seems my IDE automatically inserted this. I glossed over it when committing it seems.

Thank you for your quick feedback.

Lyndon

\
 
 \ /
  Last update: 2024-05-01 03:43    [W:0.099 / U:0.592 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site