platform-driver-x86.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Lyndon Sanche" <lsanche@lyndeno.ca>
To: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: "Mario Limonciello" <mario.limonciello@amd.com>,
	"Pali Rohár" <pali@kernel.org>, "Armin Wolf" <W_Armin@gmx.de>,
	srinivas.pandruvada@linux.intel.com,
	"kernel test robot" <lkp@intel.com>,
	"Hans de Goede" <hdegoede@redhat.com>,
	"Yijun Shen" <Yijun.Shen@dell.com>,
	"Matthew Garrett" <mjg59@srcf.ucam.org>,
	"Vegard Nossum" <vegard.nossum@oracle.com>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Heiner Kallweit" <hkallweit1@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	platform-driver-x86@vger.kernel.org, Dell.Client.Kernel@dell.com
Subject: Re: [PATCH v7 3/3] platform/x86: dell-pc: Implement platform_profile
Date: Tue, 28 May 2024 10:16:17 -0600	[thread overview]
Message-ID: <a0322878-eb27-45e4-9545-136f5ade6a0e@app.fastmail.com> (raw)
In-Reply-To: <c971cfae-e7d5-fbde-f77b-1a936c6e1ee7@linux.intel.com>

On Mon, May 27, 2024, at 3:39 AM, Ilpo Järvinen wrote:
> On Fri, 17 May 2024, Lyndon Sanche wrote:
>
>> +#include <linux/platform_profile.h>
>> +#include <linux/slab.h>
>> +#include "dell-smbios.h"
>
> Add empty line between <> and "" includes.

Agreed.

>> +enum thermal_mode_bits {
>> +	DELL_BALANCED = BIT(0),
>> +	DELL_COOL_BOTTOM = BIT(1),
>> +	DELL_QUIET = BIT(2),
>> +	DELL_PERFORMANCE = BIT(3),
>
> A few nits still to address.
>
> Can you please align these so that the values align (IIRC, I asked this 
> earlier but perhaps my request was too unclear):
>
> 	DELL_XX			= BIT(X),
> 	DELL_YYYYYYYYY		= BIT(Y),
>

I must have missed this, I will get this corrected.

>> +static int thermal_get_supported_modes(int *supported_bits)
>> +{
>> +	struct calling_interface_buffer buffer;
>> +	int ret;
>> +
>> +	dell_fill_request(&buffer, 0x0, 0, 0, 0);
>> +	ret = dell_send_request(&buffer, CLASS_INFO, SELECT_THERMAL_MANAGEMENT);
>> +	if (ret) {
>> +		/* Thermal function not supported */
>> +		if (ret == -ENXIO) {
>> +			*supported_bits = 0;
>> +			return 0;
>> +		} else {
>
> Drop else because the previous block ends into return.
>

Agreed.

>> +			return ret;
>> +		}
>> +	}
>
> Remove the outer if (ret) block and put the inner ones directly on the 
> main level as two if () conditions.
>

Agreed.

>> +	dell_fill_request(&buffer, 0x1, FIELD_PREP(DELL_ACC_SET_FIELD, acc_mode) | state, 0, 0);
>> +	ret = dell_send_request(&buffer, CLASS_INFO, SELECT_THERMAL_MANAGEMENT);
>> +	return ret;
>
> Return directly on the previous line.
>

Agreed.

>> +	int ret;
>> +	int supported_modes;
>> +
>> +	/* If thermal commands not supported, exit without error */
>
> Fix grammar, you're perhaps missing "are".
>

Agreed.

>> +	if (!dell_smbios_class_is_supported(CLASS_INFO))
>> +		return 0;
>> +
>> +	/* If thermal modes not supported, exit without error */
>
> Ditto.
>
> -- 
>  i.

Thank you for your feedback. I will include this in the next patch series.

Thanks,

Lyndon

  reply	other threads:[~2024-05-28 16:16 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-25 17:27 [PATCH] platform/x86: dell-laptop: Implement platform_profile Lyndon Sanche
2024-04-25 20:07 ` Mario Limonciello
2024-04-25 20:24   ` Lyndon Sanche
2024-04-25 20:28     ` Mario Limonciello
2024-04-25 21:51       ` Srinivas Pandruvada
2024-04-26  0:38         ` Lyndon Sanche
2024-04-26 16:14     ` srinivas pandruvada
2024-04-26 18:23       ` Lyndon Sanche
2024-04-26 18:24         ` srinivas pandruvada
2024-04-25 20:12 ` Pali Rohár
2024-04-25 20:27   ` Lyndon Sanche
2024-04-25 20:31     ` Pali Rohár
2024-04-25 21:07 ` Armin Wolf
2024-04-26  0:54   ` Lyndon Sanche
2024-04-26  2:04 ` [PATCH v2] " Lyndon Sanche
2024-04-26  9:23   ` Ilpo Järvinen
2024-04-26 18:05     ` Lyndon Sanche
2024-05-13 20:09   ` kernel test robot
2024-04-26  6:57 ` [PATCH] " Ilpo Järvinen
2024-04-29 16:48 ` [PATCH v3] " Lyndon Sanche
2024-04-29 17:45   ` Mario Limonciello
2024-04-29 17:51     ` Mario Limonciello
2024-04-29 21:25       ` Lyndon Sanche
2024-04-29 21:21     ` Lyndon Sanche
2024-04-30 10:31   ` Ilpo Järvinen
2024-04-30 18:38     ` Lyndon Sanche
2024-04-30 15:36   ` kernel test robot
2024-05-01  8:16   ` kernel test robot
2024-05-01 16:35   ` kernel test robot
2024-05-01 17:07   ` kernel test robot
2024-05-01  1:14 ` [PATCH v4] " Lyndon Sanche
2024-05-01  1:36   ` Pali Rohár
2024-05-01  1:42     ` Lyndon Sanche
2024-05-01 21:58 ` [PATCH v5] " Lyndon Sanche
2024-05-03 10:19   ` kernel test robot
2024-05-04  1:03     ` Lyndon Sanche
2024-05-06 10:18       ` Hans de Goede
2024-05-07 16:00         ` Lyndon Sanche
2024-05-03 21:19   ` Armin Wolf
2024-05-04  0:59     ` Lyndon Sanche
2024-05-08 14:24   ` Shen, Yijun
2024-05-08 15:53     ` Mario Limonciello
2024-05-11 15:05       ` Shen, Yijun
2024-05-11 15:12         ` Limonciello, Mario
2024-05-11 15:56           ` Shen, Yijun
2024-05-12 17:53             ` Armin Wolf
2024-05-12 17:58               ` Limonciello, Mario
2024-05-12 18:47                 ` Armin Wolf
2024-05-12 22:14                   ` Limonciello, Mario
2024-05-11 16:02         ` Lyndon Sanche
2024-05-09 15:10     ` Lyndon Sanche
2024-05-11  1:49       ` Lyndon Sanche
2024-05-11 15:22         ` Shen, Yijun
2024-05-11 15:54           ` Lyndon Sanche
2024-05-11 16:12             ` Shen, Yijun
2024-05-11  2:36 ` [PATCH v6 0/2] " Lyndon Sanche
2024-05-11  2:36 ` [PATCH v6 1/2] platform/x86: dell-smbios: Add helper for checking supported commands Lyndon Sanche
2024-05-11 15:13   ` Limonciello, Mario
2024-05-12 18:00   ` Armin Wolf
2024-05-11  2:36 ` [PATCH v6 2/2] platform/x86: dell-laptop: Implement platform_profile Lyndon Sanche
2024-05-11 15:16   ` Limonciello, Mario
2024-05-11 15:59     ` Lyndon Sanche
     [not found]       ` <48JCDS.E4RT1F9DTKFU1@lyndeno.ca>
2024-05-12  1:43         ` Limonciello, Mario
2024-05-12 15:25           ` Hans de Goede
2024-05-12  0:14     ` Lyndon Sanche
2024-05-12 18:05   ` Armin Wolf
2024-05-15 17:06     ` Lyndon Sanche
2024-05-17 22:42 ` [PATCH v7 0/3] platform/x86: dell: " Lyndon Sanche
2024-05-17 22:42   ` [PATCH v7 1/3] platform/x86: dell-smbios: Add helper for checking supported class Lyndon Sanche
2024-05-17 22:42   ` [PATCH v7 2/3] platform/x86: dell-smbios: Move request functions for reuse Lyndon Sanche
2024-05-17 22:42   ` [PATCH v7 3/3] platform/x86: dell-pc: Implement platform_profile Lyndon Sanche
2024-05-27  9:39     ` Ilpo Järvinen
2024-05-28 16:16       ` Lyndon Sanche [this message]
2024-05-21 20:50   ` [PATCH v7 0/3] platform/x86: dell: " Mario Limonciello
2024-05-29 17:47 ` [PATCH v8 " Lyndon Sanche
2024-05-29 17:47   ` [PATCH v8 1/3] platform/x86: dell-smbios: Add helper for checking supported class Lyndon Sanche
2024-05-29 17:47   ` [PATCH v8 2/3] platform/x86: dell-smbios: Move request functions for reuse Lyndon Sanche
2024-05-29 17:47   ` [PATCH v8 3/3] platform/x86: dell-pc: Implement platform_profile Lyndon Sanche
2024-05-30  8:39     ` Ilpo Järvinen
2024-05-31  1:19       ` Lyndon Sanche

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a0322878-eb27-45e4-9545-136f5ade6a0e@app.fastmail.com \
    --to=lsanche@lyndeno.ca \
    --cc=Dell.Client.Kernel@dell.com \
    --cc=W_Armin@gmx.de \
    --cc=Yijun.Shen@dell.com \
    --cc=corbet@lwn.net \
    --cc=hdegoede@redhat.com \
    --cc=hkallweit1@gmail.com \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=mario.limonciello@amd.com \
    --cc=mjg59@srcf.ucam.org \
    --cc=pali@kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=srinivas.pandruvada@linux.intel.com \
    --cc=vegard.nossum@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).