All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Alex Yeo <alexyeo362@gmail.com>, Hans de Goede <hansg@kernel.org>
Cc: platform-driver-x86@vger.kernel.org,
	"Kenneth Chan" <kenneth.t.chan@gmail.com>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] platform/x86: panasonic-laptop: add fan speed mode for newer models
Date: Mon, 8 Jun 2026 16:25:58 +0300 (EEST)	[thread overview]
Message-ID: <bc272424-9ec6-8f3b-19c1-89fded10e230@linux.intel.com> (raw)
In-Reply-To: <20260603011934.43467-1-alexyeo362@gmail.com>

On Wed, 3 Jun 2026, Alex Yeo wrote:

> I have a CF-SR4 and Linux works out of the box. Compared to
> previous models, this one seems to have the fans running at high speed
> by default when the computer starts.
> 
> When Windows 11 is booted up, the high fan speeds persist just until the
> login screen. Once the login screen shows up, the fan spins down.
> 
> I had suspected that this might be the laptop ramping down the fans when
> the OS declares that it is Windows but this does not seem to be the
> case.
> 
> After analyzing the DSDT and SSDT of the computer, I have found the
> following:
> 
> File ssdt10.dsl under \_SB.PC00.LPCB.EC0:
> 
> Name (CEFM, Zero)
> Method (SEFM, 1, Serialized)
> {
>     CEFM = Arg0
>     REFM ()
> }
> 
> Method (REFM, 0, Serialized)
> {
>     If ((\S0IX == 0x03))
>     {
>         Local0 = 0x05
>     }
>     ElseIf ((CEFM == Zero))
>     {
>         Local0 = Zero
>     }
>     Else
>     {
>         Local0 = 0x02
>     }
> 
>     \_SB.PC00.LPCB.EC0.EC88 (0xB5, 0x79, Local0, Zero)
> }
> 
> \_SB.PC00.LPCB.EC0.CEFM would seem be the current value
> for the fan profile. On startup, this is set to 0.
> 
> Based on the code SEFM seems the be the method to set the fan
> profile and REFM is executed right after.
> 
> I don't have access to information as to what the argument officially
> means but based on testing, any number above zero makes the fans spin
> down and behave like the older models where it stops or runs at low
> speed when its cool and ramps up when the processor gets hot.
> 
> The only relevant values for CEFM seem to be just 0 and any number above
> that just gets treated the same. I personally use just 0 and 1.
> 0 seems to be the high fan speed mode and 1 makes it behave like
> Windows.
> 
> Giving 0 as an argument reverts the fan back to the way it was during
> startup where the lowest fan speed is quite high and when load is
> applied, it seems to ramp up to an even higher speed which I think would
> be its 100%.
> 
> A value of 1 seems to have its max speed capped lower than 0.
> 
> For both modes, fan management is still automatic.
> 
> fan_mode only shows up in sysfs only if \_SB.PC00.LPCB.EC0.CEFM and
> \_SB.PC00.LPCB.EC0.SEFM are both present which should mean it should not
> show up on unsupported models. I have tried not hiding it and it just
> outputs a generic error when the value is read.
> 
> I also saw that variables such as eco_mode are kept in memory, however
> for fan_mode I rely on getting and setting the value via ACPI.
> 
> Signed-off-by: Alex Yeo <alexyeo362@gmail.com>
> ---
>  drivers/platform/x86/panasonic-laptop.c | 98 +++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
> 
> diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
> index b83113c26f88..3a8bbd633fd1 100644
> --- a/drivers/platform/x86/panasonic-laptop.c
> +++ b/drivers/platform/x86/panasonic-laptop.c
> @@ -507,6 +507,76 @@ static int set_optd_power_state(int new_state)
>  	return result;
>  }
>  
> +/* on newer models (ex: CF-SR4), fan mode can be set */
> +/* check if this is available */
> +
> +static acpi_status check_fan_mode_present(void)
> +{
> +	acpi_status status = AE_OK;

Unnecessary initialization.

> +	acpi_handle handle;
> +
> +	/* check if read and set are available */
> +	/* read fan speed profile */

The second comment line and the one below seems mostly duplicate 
information so please drop them. If you want, you can to the first line: 

	/* Check if read (CEFM) and set (SEFM) are available */

> +	status = acpi_get_handle(NULL, "\\_SB.PC00.LPCB.EC0.CEFM", &handle);
> +	if (ACPI_FAILURE(status))
> +		goto out;

You should return directly from here.

And please return something else than acpi_status from this function.

> +	/* set fan speed profile */
> +	status = acpi_get_handle(NULL, "\\_SB.PC00.LPCB.EC0.SEFM", &handle);
> +	if (ACPI_FAILURE(status))
> +		goto out;
> +
> +out:
> +	return status;
> +}
> +
> +/* get fan mode state */
> +

Remove blank line.

> +static int get_fan_mode_state(void)
> +{
> +	acpi_status status;
> +	unsigned long long state;

Please use reverse xmas-tree order.

> +
> +	/* bios default is zero which seems to be some sort of performance mode */

BIOS

> +	status = acpi_evaluate_integer(NULL, "\\_SB.PC00.LPCB.EC0.CEFM", NULL, &state);
> +	if (ACPI_FAILURE(status)) {
> +		pr_err("evaluation error _SB.PC00.LPCB.EC0.CEFM\n");
> +		state = -EIO;

Just return directly.

> +	}
> +	int result = (int)state;

This intermediate variable doesn't seem useful and is declared in wrong 
place anyway.

> +	return result;
> +}
> +
> +/* set fan mode */
> +
> +static int set_fan_mode_state(int new_state)
> +{
> +	int result;
> +	acpi_status status;

xmas-tree order.

> +
> +	result = get_fan_mode_state();
> +	if (result < 0)
> +		goto out;
> +	if (new_state == result)
> +		goto out;
> +
> +	union acpi_object param[1];
> +	struct acpi_object_list input;
> +
> +	param[0].type = ACPI_TYPE_INTEGER;
> +	param[0].integer.value = new_state;
> +	input.count = 1; /* takes one arg */
> +	input.pointer = param;
> +
> +	status = acpi_evaluate_object(NULL, "\\_SB.PC00.LPCB.EC0.SEFM",
> +			&input, NULL);

Please align to (

> +	if (ACPI_FAILURE(status)) {
> +		pr_err("_SB.PC00.LPCB.EC0.SEFM evaluation failed\n");

Please write the error messages so that they are meaningful for 
non-technical users.

Include for pr_err() is missing it seems.

> +		return -EINVAL;
> +	}
> +out:
> +	return result;
> +}
> +
>  
>  /* sysfs user interface functions */
>  
> @@ -778,6 +848,29 @@ static ssize_t cdpower_store(struct device *dev, struct device_attribute *attr,
>  	return count;
>  }
>  
> +static ssize_t fan_mode_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	int state = get_fan_mode_state();
> +
> +	if (state < 0)

As per kernel's coding style, call and its error handling should not have 
an empty line in between them (and a blank line is required after local 
var declarations) so you need to declarate variable first without 
assigning to it on the same line.

> +		return state;
> +
> +	return sysfs_emit(buf, "%d\n", state);

This file seems to be lacking sysfs.h, please add the include now.

> +}
> +
> +static ssize_t fan_mode_store(struct device *dev, struct device_attribute *attr,
> +			     const char *buf, size_t count)
> +{
> +	int err, val;
> +
> +	err = kstrtoint(buf, 10, &val);

kstrtouint()?

But I'm unsure whether sysfs is right place for this interface. acer-wmi 
driver seems to have fan mode auto/turbo selection using hwmon interface 
but this driver doesn't have hwmon as is...

> +	if (err)
> +		return err;
> +	set_fan_mode_state(val);
> +	return count;
> +}
> +
>  static DEVICE_ATTR_RO(numbatt);
>  static DEVICE_ATTR_RO(lcdtype);
>  static DEVICE_ATTR_RW(mute);
> @@ -787,6 +880,7 @@ static DEVICE_ATTR_RW(ac_brightness);
>  static DEVICE_ATTR_RW(dc_brightness);
>  static DEVICE_ATTR_RW(current_brightness);
>  static DEVICE_ATTR_RW(cdpower);
> +static DEVICE_ATTR_RW(fan_mode);
>  
>  static umode_t pcc_sysfs_is_visible(struct kobject *kobj, struct attribute *attr, int idx)
>  {
> @@ -803,6 +897,9 @@ static umode_t pcc_sysfs_is_visible(struct kobject *kobj, struct attribute *attr
>  	if (attr == &dev_attr_current_brightness.attr)
>  		return (pcc->num_sifr > SINF_CUR_BRIGHT) ? attr->mode : 0;
>  
> +	if (attr == &dev_attr_fan_mode.attr) /* mostly present on newer models */
> +		return (ACPI_SUCCESS(check_fan_mode_present())) ? attr->mode : 0;
> +
>  	return attr->mode;
>  }
>  
> @@ -816,6 +913,7 @@ static struct attribute *pcc_sysfs_entries[] = {
>  	&dev_attr_dc_brightness.attr,
>  	&dev_attr_current_brightness.attr,
>  	&dev_attr_cdpower.attr,
> +	&dev_attr_fan_mode.attr,
>  	NULL,
>  };
>  
> 

-- 
 i.


  reply	other threads:[~2026-06-08 13:26 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-03  1:19 [PATCH] platform/x86: panasonic-laptop: add fan speed mode for newer models Alex Yeo
2026-06-08 13:25 ` Ilpo Järvinen [this message]
2026-06-10  3:17   ` [RFC PATCH v2] " Alex Yeo
2026-06-10  9:12     ` Ilpo Järvinen
2026-06-25 11:50       ` [RFC PATCH v3] " Alex Yeo
2026-06-25 12:04         ` sashiko-bot
2026-06-26  3:25           ` [RFC PATCH v4] " Alex Yeo
2026-06-26  3:34             ` sashiko-bot
2026-06-26  5:32               ` [RFC PATCH v5] " Alex Yeo
2026-06-26  5:47                 ` sashiko-bot
2026-06-28  5:33                   ` [RFC PATCH v6] " Alex Yeo
2026-07-03 12:11                     ` Ilpo Järvinen
2026-07-18 18:57                       ` [RFC PATCH v7] " Alex Yeo
2026-07-20 13:46                         ` Ilpo Järvinen
2026-07-20 14:41                           ` Guenter Roeck
2026-07-21 23:22                             ` Alex Yeo
2026-07-22  0:00                               ` Guenter Roeck
2026-07-21 22:10                           ` Alex Yeo
2026-06-10 10:13     ` [RFC PATCH v2] " kernel test robot
2026-06-10 10:23     ` kernel test robot
2026-06-11 21:23     ` kernel test robot
2026-06-11 23:52     ` kernel test robot

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=bc272424-9ec6-8f3b-19c1-89fded10e230@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=alexyeo362@gmail.com \
    --cc=hansg@kernel.org \
    --cc=kenneth.t.chan@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.