[PATCH v2 0/1] platform/surface: platform_profile: add fan profile

[PATCH v2 0/1] platform/surface: platform_profile: add fan profile

[Ivor Wanders]

Second version of a patch that switches the fan profile together with the
platform profile on Microsoft Surface Pro 9 devices, improving the cooling.
Originally submitted in [1] which describes the changes in more detail.

Changes in v2:
  - Use u8 instead of char for the argument of __sam_fan_profile_set.
  - Made profile and profile_le variable const.
  - Added link entry pointing to the Github PR to commit message.
  - Rebased the commit on Torvalds' main branch.

[1]: https://lore.kernel.org/all/20240302170147.13026-1-ivor@iwanders.net/

Ivor Wanders (1):
  platform/surface: platform_profile: add fan profile switching

 .../surface/surface_aggregator_registry.c     | 36 +++++---
 .../surface/surface_platform_profile.c        | 88 ++++++++++++++++---
 2 files changed, 100 insertions(+), 24 deletions(-)

-- 
2.17.1

[PATCH v2 1/1] platform/surface: platform_profile: add fan profile switching

[Ivor Wanders]

Change naming from tmp to platform profile to clarify the module may
interact with both the TMP and FAN subystems. Add functionality that
switches the fan profile when the platform profile is changed when
a fan is present.

Signed-off-by: Ivor Wanders <ivor@iwanders.net>
Link: https://github.com/linux-surface/kernel/pull/145
Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>
---
Changes in v2:
  - Added link entry to commit message.
  - Use u8 instead of char for the argument of __sam_fan_profile_set.
  - Made profile and profile_le variable const.
---
---
 .../surface/surface_aggregator_registry.c     | 36 +++++---
 .../surface/surface_platform_profile.c        | 88 ++++++++++++++++---
 2 files changed, 100 insertions(+), 24 deletions(-)

diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
index 035d6b4105cd..79e52eddabd0 100644
--- a/drivers/platform/surface/surface_aggregator_registry.c
+++ b/drivers/platform/surface/surface_aggregator_registry.c
@@ -68,12 +68,26 @@ static const struct software_node ssam_node_bat_sb3base = {
 	.parent = &ssam_node_hub_base,
 };
 
-/* Platform profile / performance-mode device. */
-static const struct software_node ssam_node_tmp_pprof = {
+/* Platform profile / performance-mode device without a fan. */
+static const struct software_node ssam_node_tmp_perf_profile = {
 	.name = "ssam:01:03:01:00:01",
 	.parent = &ssam_node_root,
 };
 
+/* Platform profile / performance-mode device with a fan, such that
+ * the fan controller profile can also be switched.
+ */
+static const struct property_entry ssam_node_tmp_perf_profile_has_fan[] = {
+	PROPERTY_ENTRY_BOOL("has_fan"),
+	{ }
+};
+
+static const struct software_node ssam_node_tmp_perf_profile_with_fan = {
+	.name = "ssam:01:03:01:00:01",
+	.parent = &ssam_node_root,
+	.properties = ssam_node_tmp_perf_profile_has_fan,
+};
+
 /* Fan speed function. */
 static const struct software_node ssam_node_fan_speed = {
 	.name = "ssam:01:05:01:01:01",
@@ -208,7 +222,7 @@ static const struct software_node ssam_node_pos_tablet_switch = {
  */
 static const struct software_node *ssam_node_group_gen5[] = {
 	&ssam_node_root,
-	&ssam_node_tmp_pprof,
+	&ssam_node_tmp_perf_profile,
 	NULL,
 };
 
@@ -219,7 +233,7 @@ static const struct software_node *ssam_node_group_sb3[] = {
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
 	&ssam_node_bat_sb3base,
-	&ssam_node_tmp_pprof,
+	&ssam_node_tmp_perf_profile,
 	&ssam_node_bas_dtx,
 	&ssam_node_hid_base_keyboard,
 	&ssam_node_hid_base_touchpad,
@@ -233,7 +247,7 @@ static const struct software_node *ssam_node_group_sl3[] = {
 	&ssam_node_root,
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
-	&ssam_node_tmp_pprof,
+	&ssam_node_tmp_perf_profile,
 	&ssam_node_hid_main_keyboard,
 	&ssam_node_hid_main_touchpad,
 	&ssam_node_hid_main_iid5,
@@ -245,7 +259,7 @@ static const struct software_node *ssam_node_group_sl5[] = {
 	&ssam_node_root,
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
-	&ssam_node_tmp_pprof,
+	&ssam_node_tmp_perf_profile,
 	&ssam_node_hid_main_keyboard,
 	&ssam_node_hid_main_touchpad,
 	&ssam_node_hid_main_iid5,
@@ -258,7 +272,7 @@ static const struct software_node *ssam_node_group_sls[] = {
 	&ssam_node_root,
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
-	&ssam_node_tmp_pprof,
+	&ssam_node_tmp_perf_profile,
 	&ssam_node_pos_tablet_switch,
 	&ssam_node_hid_sam_keyboard,
 	&ssam_node_hid_sam_penstash,
@@ -274,7 +288,7 @@ static const struct software_node *ssam_node_group_slg1[] = {
 	&ssam_node_root,
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
-	&ssam_node_tmp_pprof,
+	&ssam_node_tmp_perf_profile,
 	NULL,
 };
 
@@ -283,7 +297,7 @@ static const struct software_node *ssam_node_group_sp7[] = {
 	&ssam_node_root,
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
-	&ssam_node_tmp_pprof,
+	&ssam_node_tmp_perf_profile,
 	NULL,
 };
 
@@ -293,7 +307,7 @@ static const struct software_node *ssam_node_group_sp8[] = {
 	&ssam_node_hub_kip,
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
-	&ssam_node_tmp_pprof,
+	&ssam_node_tmp_perf_profile,
 	&ssam_node_kip_tablet_switch,
 	&ssam_node_hid_kip_keyboard,
 	&ssam_node_hid_kip_penstash,
@@ -310,7 +324,7 @@ static const struct software_node *ssam_node_group_sp9[] = {
 	&ssam_node_hub_kip,
 	&ssam_node_bat_ac,
 	&ssam_node_bat_main,
-	&ssam_node_tmp_pprof,
+	&ssam_node_tmp_perf_profile_with_fan,
 	&ssam_node_fan_speed,
 	&ssam_node_pos_tablet_switch,
 	&ssam_node_hid_kip_keyboard,
diff --git a/drivers/platform/surface/surface_platform_profile.c b/drivers/platform/surface/surface_platform_profile.c
index a5a3941b3f43..3de864bc6610 100644
--- a/drivers/platform/surface/surface_platform_profile.c
+++ b/drivers/platform/surface/surface_platform_profile.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
  * Surface Platform Profile / Performance Mode driver for Surface System
- * Aggregator Module (thermal subsystem).
+ * Aggregator Module (thermal and fan subsystem).
  *
  * Copyright (C) 2021-2022 Maximilian Luz <luzmaximilian@gmail.com>
  */
@@ -14,6 +14,7 @@
 
 #include <linux/surface_aggregator/device.h>
 
+// Enum for the platform performance profile sent to the TMP module.
 enum ssam_tmp_profile {
 	SSAM_TMP_PROFILE_NORMAL             = 1,
 	SSAM_TMP_PROFILE_BATTERY_SAVER      = 2,
@@ -21,15 +22,26 @@ enum ssam_tmp_profile {
 	SSAM_TMP_PROFILE_BEST_PERFORMANCE   = 4,
 };
 
+// Enum for the fan profile sent to the FAN module. This fan profile is
+// only sent to the EC if the 'has_fan' property is set. The integers are
+// not a typo, they differ from the performance profile indices.
+enum ssam_fan_profile {
+	SSAM_FAN_PROFILE_NORMAL             = 2,
+	SSAM_FAN_PROFILE_BATTERY_SAVER      = 1,
+	SSAM_FAN_PROFILE_BETTER_PERFORMANCE = 3,
+	SSAM_FAN_PROFILE_BEST_PERFORMANCE   = 4,
+};
+
 struct ssam_tmp_profile_info {
 	__le32 profile;
 	__le16 unknown1;
 	__le16 unknown2;
 } __packed;
 
-struct ssam_tmp_profile_device {
+struct ssam_platform_profile_device {
 	struct ssam_device *sdev;
 	struct platform_profile_handler handler;
+	bool has_fan;
 };
 
 SSAM_DEFINE_SYNC_REQUEST_CL_R(__ssam_tmp_profile_get, struct ssam_tmp_profile_info, {
@@ -42,6 +54,13 @@ SSAM_DEFINE_SYNC_REQUEST_CL_W(__ssam_tmp_profile_set, __le32, {
 	.command_id      = 0x03,
 });
 
+SSAM_DEFINE_SYNC_REQUEST_W(__ssam_fan_profile_set, u8, {
+	.target_category = SSAM_SSH_TC_FAN,
+	.target_id = SSAM_SSH_TID_SAM,
+	.command_id = 0x0e,
+	.instance_id = 0x01,
+});
+
 static int ssam_tmp_profile_get(struct ssam_device *sdev, enum ssam_tmp_profile *p)
 {
 	struct ssam_tmp_profile_info info;
@@ -57,12 +76,19 @@ static int ssam_tmp_profile_get(struct ssam_device *sdev, enum ssam_tmp_profile
 
 static int ssam_tmp_profile_set(struct ssam_device *sdev, enum ssam_tmp_profile p)
 {
-	__le32 profile_le = cpu_to_le32(p);
+	const __le32 profile_le = cpu_to_le32(p);
 
 	return ssam_retry(__ssam_tmp_profile_set, sdev, &profile_le);
 }
 
-static int convert_ssam_to_profile(struct ssam_device *sdev, enum ssam_tmp_profile p)
+static int ssam_fan_profile_set(struct ssam_device *sdev, enum ssam_fan_profile p)
+{
+	const u8 profile = p;
+
+	return ssam_retry(__ssam_fan_profile_set, sdev->ctrl, &profile);
+}
+
+static int convert_ssam_tmp_to_profile(struct ssam_device *sdev, enum ssam_tmp_profile p)
 {
 	switch (p) {
 	case SSAM_TMP_PROFILE_NORMAL:
@@ -83,7 +109,8 @@ static int convert_ssam_to_profile(struct ssam_device *sdev, enum ssam_tmp_profi
 	}
 }
 
-static int convert_profile_to_ssam(struct ssam_device *sdev, enum platform_profile_option p)
+
+static int convert_profile_to_ssam_tmp(struct ssam_device *sdev, enum platform_profile_option p)
 {
 	switch (p) {
 	case PLATFORM_PROFILE_LOW_POWER:
@@ -105,20 +132,42 @@ static int convert_profile_to_ssam(struct ssam_device *sdev, enum platform_profi
 	}
 }
 
+static int convert_profile_to_ssam_fan(struct ssam_device *sdev, enum platform_profile_option p)
+{
+	switch (p) {
+	case PLATFORM_PROFILE_LOW_POWER:
+		return SSAM_FAN_PROFILE_BATTERY_SAVER;
+
+	case PLATFORM_PROFILE_BALANCED:
+		return SSAM_FAN_PROFILE_NORMAL;
+
+	case PLATFORM_PROFILE_BALANCED_PERFORMANCE:
+		return SSAM_FAN_PROFILE_BETTER_PERFORMANCE;
+
+	case PLATFORM_PROFILE_PERFORMANCE:
+		return SSAM_FAN_PROFILE_BEST_PERFORMANCE;
+
+	default:
+		/* This should have already been caught by platform_profile_store(). */
+		WARN(true, "unsupported platform profile");
+		return -EOPNOTSUPP;
+	}
+}
+
 static int ssam_platform_profile_get(struct platform_profile_handler *pprof,
 				     enum platform_profile_option *profile)
 {
-	struct ssam_tmp_profile_device *tpd;
+	struct ssam_platform_profile_device *tpd;
 	enum ssam_tmp_profile tp;
 	int status;
 
-	tpd = container_of(pprof, struct ssam_tmp_profile_device, handler);
+	tpd = container_of(pprof, struct ssam_platform_profile_device, handler);
 
 	status = ssam_tmp_profile_get(tpd->sdev, &tp);
 	if (status)
 		return status;
 
-	status = convert_ssam_to_profile(tpd->sdev, tp);
+	status = convert_ssam_tmp_to_profile(tpd->sdev, tp);
 	if (status < 0)
 		return status;
 
@@ -129,21 +178,32 @@ static int ssam_platform_profile_get(struct platform_profile_handler *pprof,
 static int ssam_platform_profile_set(struct platform_profile_handler *pprof,
 				     enum platform_profile_option profile)
 {
-	struct ssam_tmp_profile_device *tpd;
+	struct ssam_platform_profile_device *tpd;
 	int tp;
 
-	tpd = container_of(pprof, struct ssam_tmp_profile_device, handler);
+	tpd = container_of(pprof, struct ssam_platform_profile_device, handler);
+
+	tp = convert_profile_to_ssam_tmp(tpd->sdev, profile);
+	if (tp < 0)
+		return tp;
 
-	tp = convert_profile_to_ssam(tpd->sdev, profile);
+	tp = ssam_tmp_profile_set(tpd->sdev, tp);
 	if (tp < 0)
 		return tp;
 
-	return ssam_tmp_profile_set(tpd->sdev, tp);
+	if (tpd->has_fan) {
+		tp = convert_profile_to_ssam_fan(tpd->sdev, profile);
+		if (tp < 0)
+			return tp;
+		tp = ssam_fan_profile_set(tpd->sdev, tp);
+	}
+
+	return tp;
 }
 
 static int surface_platform_profile_probe(struct ssam_device *sdev)
 {
-	struct ssam_tmp_profile_device *tpd;
+	struct ssam_platform_profile_device *tpd;
 
 	tpd = devm_kzalloc(&sdev->dev, sizeof(*tpd), GFP_KERNEL);
 	if (!tpd)
@@ -154,6 +214,8 @@ static int surface_platform_profile_probe(struct ssam_device *sdev)
 	tpd->handler.profile_get = ssam_platform_profile_get;
 	tpd->handler.profile_set = ssam_platform_profile_set;
 
+	tpd->has_fan = device_property_read_bool(&sdev->dev, "has_fan");
+
 	set_bit(PLATFORM_PROFILE_LOW_POWER, tpd->handler.choices);
 	set_bit(PLATFORM_PROFILE_BALANCED, tpd->handler.choices);
 	set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE, tpd->handler.choices);
-- 
2.17.1

Re: [PATCH v2 1/1] platform/surface: platform_profile: add fan profile switching

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

On Thu, 14 Mar 2024, Ivor Wanders wrote:

> Change naming from tmp to platform profile to clarify the module may
> interact with both the TMP and FAN subystems. Add functionality that
> switches the fan profile when the platform profile is changed when
> a fan is present.
> 
> Signed-off-by: Ivor Wanders <ivor@iwanders.net>
> Link: https://github.com/linux-surface/kernel/pull/145
> Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>
> ---
> Changes in v2:
>   - Added link entry to commit message.
>   - Use u8 instead of char for the argument of __sam_fan_profile_set.
>   - Made profile and profile_le variable const.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH v2 1/1] platform/surface: platform_profile: add fan profile switching

[Ivor Wanders]

> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

What is the process to get this merged if no further review is coming in?
Maximilian Luz approved v1 already. I just confirmed this patch still
applies cleanly on the latest upstream master. Do I need to send v3 with
this reviewed-by tag added to the commit or can this version be merged?

Thanks,

~Ivor

Re: [PATCH v2 1/1] platform/surface: platform_profile: add fan profile switching

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]

On Thu, 21 Mar 2024, Ivor Wanders wrote:

> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 
> What is the process to get this merged if no further review is coming in?
> Maximilian Luz approved v1 already. I just confirmed this patch still
> applies cleanly on the latest upstream master.

Hi Ivor,

We're currently in the two weeks merge window (the time between kernel 
release and the next -rc1). Usually, nothing happens during it for new 
patches that are not fixes (there might be exceptions with some 
subsystems but that's a good general rule and even with fixes severity 
matters a lot if they are considered until -rc1 is out).
 
Maintainer will consider your patch after the merge window (in this case, 
it will be Hans' turn for 3.10 cycle to handle for-next while I handle 
fixes branch).

> Do I need to send v3 with
> this reviewed-by tag added to the commit or can this version be merged?

Our tools take care of the tags such as Reviewed-by when they're made 
against the latest version, there is no need to resubmit just because of 
tags people give to your patch.

(That being said, whenever submitting a new version for other reasons, 
remember to add the tags to the new version because tools only capture 
them for the latest version.)

-- 
 i.

Re: [PATCH v2 1/1] platform/surface: platform_profile: add fan profile switching

[Ivor Wanders]

Hey Ilpo,

Thanks for the detailed explanation!

> We're currently in the two weeks merge window (the time between kernel 
> release and the next -rc1). Usually, nothing happens during it for new 
> patches that are not fixes (there might be exceptions with some 
> subsystems but that's a good general rule and even with fixes severity 
> matters a lot if they are considered until -rc1 is out).

I see, time flies, I thought 6.7 was just wrapped up, but that's already
over two months ago.

> Our tools take care of the tags such as Reviewed-by when they're made 
> against the latest version, there is no need to resubmit just because of 
> tags people give to your patch.

Perfect, good to know, I'll just sit tight.

~Ivor

Re: [PATCH v2 1/1] platform/surface: platform_profile: add fan profile switching

[Hans de Goede]

Hi,

On 3/14/24 11:37 PM, Ivor Wanders wrote:
> Change naming from tmp to platform profile to clarify the module may
> interact with both the TMP and FAN subystems. Add functionality that
> switches the fan profile when the platform profile is changed when
> a fan is present.
> 
> Signed-off-by: Ivor Wanders <ivor@iwanders.net>
> Link: https://github.com/linux-surface/kernel/pull/145
> Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans




> ---
> Changes in v2:
>   - Added link entry to commit message.
>   - Use u8 instead of char for the argument of __sam_fan_profile_set.
>   - Made profile and profile_le variable const.
> ---
> ---
>  .../surface/surface_aggregator_registry.c     | 36 +++++---
>  .../surface/surface_platform_profile.c        | 88 ++++++++++++++++---
>  2 files changed, 100 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/platform/surface/surface_aggregator_registry.c b/drivers/platform/surface/surface_aggregator_registry.c
> index 035d6b4105cd..79e52eddabd0 100644
> --- a/drivers/platform/surface/surface_aggregator_registry.c
> +++ b/drivers/platform/surface/surface_aggregator_registry.c
> @@ -68,12 +68,26 @@ static const struct software_node ssam_node_bat_sb3base = {
>  	.parent = &ssam_node_hub_base,
>  };
>  
> -/* Platform profile / performance-mode device. */
> -static const struct software_node ssam_node_tmp_pprof = {
> +/* Platform profile / performance-mode device without a fan. */
> +static const struct software_node ssam_node_tmp_perf_profile = {
>  	.name = "ssam:01:03:01:00:01",
>  	.parent = &ssam_node_root,
>  };
>  
> +/* Platform profile / performance-mode device with a fan, such that
> + * the fan controller profile can also be switched.
> + */
> +static const struct property_entry ssam_node_tmp_perf_profile_has_fan[] = {
> +	PROPERTY_ENTRY_BOOL("has_fan"),
> +	{ }
> +};
> +
> +static const struct software_node ssam_node_tmp_perf_profile_with_fan = {
> +	.name = "ssam:01:03:01:00:01",
> +	.parent = &ssam_node_root,
> +	.properties = ssam_node_tmp_perf_profile_has_fan,
> +};
> +
>  /* Fan speed function. */
>  static const struct software_node ssam_node_fan_speed = {
>  	.name = "ssam:01:05:01:01:01",
> @@ -208,7 +222,7 @@ static const struct software_node ssam_node_pos_tablet_switch = {
>   */
>  static const struct software_node *ssam_node_group_gen5[] = {
>  	&ssam_node_root,
> -	&ssam_node_tmp_pprof,
> +	&ssam_node_tmp_perf_profile,
>  	NULL,
>  };
>  
> @@ -219,7 +233,7 @@ static const struct software_node *ssam_node_group_sb3[] = {
>  	&ssam_node_bat_ac,
>  	&ssam_node_bat_main,
>  	&ssam_node_bat_sb3base,
> -	&ssam_node_tmp_pprof,
> +	&ssam_node_tmp_perf_profile,
>  	&ssam_node_bas_dtx,
>  	&ssam_node_hid_base_keyboard,
>  	&ssam_node_hid_base_touchpad,
> @@ -233,7 +247,7 @@ static const struct software_node *ssam_node_group_sl3[] = {
>  	&ssam_node_root,
>  	&ssam_node_bat_ac,
>  	&ssam_node_bat_main,
> -	&ssam_node_tmp_pprof,
> +	&ssam_node_tmp_perf_profile,
>  	&ssam_node_hid_main_keyboard,
>  	&ssam_node_hid_main_touchpad,
>  	&ssam_node_hid_main_iid5,
> @@ -245,7 +259,7 @@ static const struct software_node *ssam_node_group_sl5[] = {
>  	&ssam_node_root,
>  	&ssam_node_bat_ac,
>  	&ssam_node_bat_main,
> -	&ssam_node_tmp_pprof,
> +	&ssam_node_tmp_perf_profile,
>  	&ssam_node_hid_main_keyboard,
>  	&ssam_node_hid_main_touchpad,
>  	&ssam_node_hid_main_iid5,
> @@ -258,7 +272,7 @@ static const struct software_node *ssam_node_group_sls[] = {
>  	&ssam_node_root,
>  	&ssam_node_bat_ac,
>  	&ssam_node_bat_main,
> -	&ssam_node_tmp_pprof,
> +	&ssam_node_tmp_perf_profile,
>  	&ssam_node_pos_tablet_switch,
>  	&ssam_node_hid_sam_keyboard,
>  	&ssam_node_hid_sam_penstash,
> @@ -274,7 +288,7 @@ static const struct software_node *ssam_node_group_slg1[] = {
>  	&ssam_node_root,
>  	&ssam_node_bat_ac,
>  	&ssam_node_bat_main,
> -	&ssam_node_tmp_pprof,
> +	&ssam_node_tmp_perf_profile,
>  	NULL,
>  };
>  
> @@ -283,7 +297,7 @@ static const struct software_node *ssam_node_group_sp7[] = {
>  	&ssam_node_root,
>  	&ssam_node_bat_ac,
>  	&ssam_node_bat_main,
> -	&ssam_node_tmp_pprof,
> +	&ssam_node_tmp_perf_profile,
>  	NULL,
>  };
>  
> @@ -293,7 +307,7 @@ static const struct software_node *ssam_node_group_sp8[] = {
>  	&ssam_node_hub_kip,
>  	&ssam_node_bat_ac,
>  	&ssam_node_bat_main,
> -	&ssam_node_tmp_pprof,
> +	&ssam_node_tmp_perf_profile,
>  	&ssam_node_kip_tablet_switch,
>  	&ssam_node_hid_kip_keyboard,
>  	&ssam_node_hid_kip_penstash,
> @@ -310,7 +324,7 @@ static const struct software_node *ssam_node_group_sp9[] = {
>  	&ssam_node_hub_kip,
>  	&ssam_node_bat_ac,
>  	&ssam_node_bat_main,
> -	&ssam_node_tmp_pprof,
> +	&ssam_node_tmp_perf_profile_with_fan,
>  	&ssam_node_fan_speed,
>  	&ssam_node_pos_tablet_switch,
>  	&ssam_node_hid_kip_keyboard,
> diff --git a/drivers/platform/surface/surface_platform_profile.c b/drivers/platform/surface/surface_platform_profile.c
> index a5a3941b3f43..3de864bc6610 100644
> --- a/drivers/platform/surface/surface_platform_profile.c
> +++ b/drivers/platform/surface/surface_platform_profile.c
> @@ -1,7 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
>   * Surface Platform Profile / Performance Mode driver for Surface System
> - * Aggregator Module (thermal subsystem).
> + * Aggregator Module (thermal and fan subsystem).
>   *
>   * Copyright (C) 2021-2022 Maximilian Luz <luzmaximilian@gmail.com>
>   */
> @@ -14,6 +14,7 @@
>  
>  #include <linux/surface_aggregator/device.h>
>  
> +// Enum for the platform performance profile sent to the TMP module.
>  enum ssam_tmp_profile {
>  	SSAM_TMP_PROFILE_NORMAL             = 1,
>  	SSAM_TMP_PROFILE_BATTERY_SAVER      = 2,
> @@ -21,15 +22,26 @@ enum ssam_tmp_profile {
>  	SSAM_TMP_PROFILE_BEST_PERFORMANCE   = 4,
>  };
>  
> +// Enum for the fan profile sent to the FAN module. This fan profile is
> +// only sent to the EC if the 'has_fan' property is set. The integers are
> +// not a typo, they differ from the performance profile indices.
> +enum ssam_fan_profile {
> +	SSAM_FAN_PROFILE_NORMAL             = 2,
> +	SSAM_FAN_PROFILE_BATTERY_SAVER      = 1,
> +	SSAM_FAN_PROFILE_BETTER_PERFORMANCE = 3,
> +	SSAM_FAN_PROFILE_BEST_PERFORMANCE   = 4,
> +};
> +
>  struct ssam_tmp_profile_info {
>  	__le32 profile;
>  	__le16 unknown1;
>  	__le16 unknown2;
>  } __packed;
>  
> -struct ssam_tmp_profile_device {
> +struct ssam_platform_profile_device {
>  	struct ssam_device *sdev;
>  	struct platform_profile_handler handler;
> +	bool has_fan;
>  };
>  
>  SSAM_DEFINE_SYNC_REQUEST_CL_R(__ssam_tmp_profile_get, struct ssam_tmp_profile_info, {
> @@ -42,6 +54,13 @@ SSAM_DEFINE_SYNC_REQUEST_CL_W(__ssam_tmp_profile_set, __le32, {
>  	.command_id      = 0x03,
>  });
>  
> +SSAM_DEFINE_SYNC_REQUEST_W(__ssam_fan_profile_set, u8, {
> +	.target_category = SSAM_SSH_TC_FAN,
> +	.target_id = SSAM_SSH_TID_SAM,
> +	.command_id = 0x0e,
> +	.instance_id = 0x01,
> +});
> +
>  static int ssam_tmp_profile_get(struct ssam_device *sdev, enum ssam_tmp_profile *p)
>  {
>  	struct ssam_tmp_profile_info info;
> @@ -57,12 +76,19 @@ static int ssam_tmp_profile_get(struct ssam_device *sdev, enum ssam_tmp_profile
>  
>  static int ssam_tmp_profile_set(struct ssam_device *sdev, enum ssam_tmp_profile p)
>  {
> -	__le32 profile_le = cpu_to_le32(p);
> +	const __le32 profile_le = cpu_to_le32(p);
>  
>  	return ssam_retry(__ssam_tmp_profile_set, sdev, &profile_le);
>  }
>  
> -static int convert_ssam_to_profile(struct ssam_device *sdev, enum ssam_tmp_profile p)
> +static int ssam_fan_profile_set(struct ssam_device *sdev, enum ssam_fan_profile p)
> +{
> +	const u8 profile = p;
> +
> +	return ssam_retry(__ssam_fan_profile_set, sdev->ctrl, &profile);
> +}
> +
> +static int convert_ssam_tmp_to_profile(struct ssam_device *sdev, enum ssam_tmp_profile p)
>  {
>  	switch (p) {
>  	case SSAM_TMP_PROFILE_NORMAL:
> @@ -83,7 +109,8 @@ static int convert_ssam_to_profile(struct ssam_device *sdev, enum ssam_tmp_profi
>  	}
>  }
>  
> -static int convert_profile_to_ssam(struct ssam_device *sdev, enum platform_profile_option p)
> +
> +static int convert_profile_to_ssam_tmp(struct ssam_device *sdev, enum platform_profile_option p)
>  {
>  	switch (p) {
>  	case PLATFORM_PROFILE_LOW_POWER:
> @@ -105,20 +132,42 @@ static int convert_profile_to_ssam(struct ssam_device *sdev, enum platform_profi
>  	}
>  }
>  
> +static int convert_profile_to_ssam_fan(struct ssam_device *sdev, enum platform_profile_option p)
> +{
> +	switch (p) {
> +	case PLATFORM_PROFILE_LOW_POWER:
> +		return SSAM_FAN_PROFILE_BATTERY_SAVER;
> +
> +	case PLATFORM_PROFILE_BALANCED:
> +		return SSAM_FAN_PROFILE_NORMAL;
> +
> +	case PLATFORM_PROFILE_BALANCED_PERFORMANCE:
> +		return SSAM_FAN_PROFILE_BETTER_PERFORMANCE;
> +
> +	case PLATFORM_PROFILE_PERFORMANCE:
> +		return SSAM_FAN_PROFILE_BEST_PERFORMANCE;
> +
> +	default:
> +		/* This should have already been caught by platform_profile_store(). */
> +		WARN(true, "unsupported platform profile");
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
>  static int ssam_platform_profile_get(struct platform_profile_handler *pprof,
>  				     enum platform_profile_option *profile)
>  {
> -	struct ssam_tmp_profile_device *tpd;
> +	struct ssam_platform_profile_device *tpd;
>  	enum ssam_tmp_profile tp;
>  	int status;
>  
> -	tpd = container_of(pprof, struct ssam_tmp_profile_device, handler);
> +	tpd = container_of(pprof, struct ssam_platform_profile_device, handler);
>  
>  	status = ssam_tmp_profile_get(tpd->sdev, &tp);
>  	if (status)
>  		return status;
>  
> -	status = convert_ssam_to_profile(tpd->sdev, tp);
> +	status = convert_ssam_tmp_to_profile(tpd->sdev, tp);
>  	if (status < 0)
>  		return status;
>  
> @@ -129,21 +178,32 @@ static int ssam_platform_profile_get(struct platform_profile_handler *pprof,
>  static int ssam_platform_profile_set(struct platform_profile_handler *pprof,
>  				     enum platform_profile_option profile)
>  {
> -	struct ssam_tmp_profile_device *tpd;
> +	struct ssam_platform_profile_device *tpd;
>  	int tp;
>  
> -	tpd = container_of(pprof, struct ssam_tmp_profile_device, handler);
> +	tpd = container_of(pprof, struct ssam_platform_profile_device, handler);
> +
> +	tp = convert_profile_to_ssam_tmp(tpd->sdev, profile);
> +	if (tp < 0)
> +		return tp;
>  
> -	tp = convert_profile_to_ssam(tpd->sdev, profile);
> +	tp = ssam_tmp_profile_set(tpd->sdev, tp);
>  	if (tp < 0)
>  		return tp;
>  
> -	return ssam_tmp_profile_set(tpd->sdev, tp);
> +	if (tpd->has_fan) {
> +		tp = convert_profile_to_ssam_fan(tpd->sdev, profile);
> +		if (tp < 0)
> +			return tp;
> +		tp = ssam_fan_profile_set(tpd->sdev, tp);
> +	}
> +
> +	return tp;
>  }
>  
>  static int surface_platform_profile_probe(struct ssam_device *sdev)
>  {
> -	struct ssam_tmp_profile_device *tpd;
> +	struct ssam_platform_profile_device *tpd;
>  
>  	tpd = devm_kzalloc(&sdev->dev, sizeof(*tpd), GFP_KERNEL);
>  	if (!tpd)
> @@ -154,6 +214,8 @@ static int surface_platform_profile_probe(struct ssam_device *sdev)
>  	tpd->handler.profile_get = ssam_platform_profile_get;
>  	tpd->handler.profile_set = ssam_platform_profile_set;
>  
> +	tpd->has_fan = device_property_read_bool(&sdev->dev, "has_fan");
> +
>  	set_bit(PLATFORM_PROFILE_LOW_POWER, tpd->handler.choices);
>  	set_bit(PLATFORM_PROFILE_BALANCED, tpd->handler.choices);
>  	set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE, tpd->handler.choices);

Re: [PATCH v2 1/1] platform/surface: platform_profile: add fan profile switching

[Ivor Wanders]

> Once I've run some tests on this branch the patches there will be
> added to the platform-drivers-x86/for-next branch and eventually
> will be included in the pdx86 pull-request to Linus for the next
> merge-window.

Sounds good, thank you for describing the process.

~Ivor