linux-kernel - Re: [PATCH v4] 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: Tue, 30 Apr 2024 19:42:26 -0600
From: "Lyndon Sanche" <lsanche@...deno.ca>
To: Pali Rohár <pali@...nel.org>
Cc: "Mario Limonciello" <mario.limonciello@....com>, W_Armin@....de,
 srinivas.pandruvada@...ux.intel.com,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 lkp@...el.com, "Matthew Garrett" <mjg59@...f.ucam.org>,
 "Hans de Goede" <hdegoede@...hat.com>, platform-driver-x86@...r.kernel.org,
 LKML <linux-kernel@...r.kernel.org>, Dell.Client.Kernel@...l.com
Subject: Re: [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