* [PATCH v9] platform/x86: thinkpad_acpi: Add platform profile support
@ 2021-01-11 16:22 Mark Pearson
2021-01-17 14:01 ` kernel test robot
2021-02-02 14:49 ` Hans de Goede
0 siblings, 2 replies; 7+ messages in thread
From: Mark Pearson @ 2021-01-11 16:22 UTC (permalink / raw)
To: markpearson
Cc: hdegoede, mgross, rjw, linux-acpi, platform-driver-x86, hadess, njoshi1
Add support to thinkpad_acpi for Lenovo platforms that have DYTC
version 5 support or newer to use the platform profile feature.
This will allow users to determine and control the platform modes
between low-power, balanced operation and performance modes.
Signed-off-by: Mark Pearson <markpearson@lenovo.com>
---
Changes in v2:
Address (hopefully) all recommendations from review including:
- use IS_ENABLED instead of IS_DEFINED
- update driver to work with all the fixes in platform_profile update
- improve error handling for invalid inputs
- move tracking of current profile mode into this driver
Changes in v3:
- version update for patch series
Changes in v4:
- Rebase on top of palm sensor patch which led to a little bit of file
restructuring/clean up
- Use BIT macro where applicable
- Formatting fixes
- Check sysfs node created on exit function
- implement and use DYTC_SET_COMMAND macro
- in case of failure setting performance mode make sure CQL mode is
enabled again before returning.
- Clean up initialisation and error handling code
Changes in v5:
- Use atomic_int with ignoring events
- Add mutex when accessing ACPI information
- Clean up registration code
- Use cached value in profile_get function
- Add dytc_cql_command function to clean up and provide common handler
- Update to work with changes in platform_profile implementation
Changes in v6:
- Update file to build against update in v6 platform_profile
Changes in v7 & v8:
- version bump along with rest of patch series
Changes in v9:
- Update define names to be correct with the linux-pm branches accepted
version
- Rename DYTC_MODE_QUIET to LOW_POWER just because it's confusing. It
was originally based on internal documentation.
Note - originally this patch was part of a series but with the other two
pieces being accepted into the acpi maintainers branch this one becomes
standalone.
drivers/platform/x86/thinkpad_acpi.c | 294 ++++++++++++++++++++++++++-
1 file changed, 288 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index e03df2881dc6..c5645331cfac 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -66,6 +66,7 @@
#include <linux/acpi.h>
#include <linux/pci.h>
#include <linux/power_supply.h>
+#include <linux/platform_profile.h>
#include <sound/core.h>
#include <sound/control.h>
#include <sound/initval.h>
@@ -9854,16 +9855,27 @@ static bool has_lapsensor;
static bool palm_state;
static bool lap_state;
-static int lapsensor_get(bool *present, bool *state)
+static int dytc_command(int command, int *output)
{
acpi_handle dytc_handle;
- int output;
- *present = false;
- if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", &dytc_handle)))
+ if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", &dytc_handle))) {
+ /* Platform doesn't support DYTC */
return -ENODEV;
- if (!acpi_evalf(dytc_handle, &output, NULL, "dd", DYTC_CMD_GET))
+ }
+ if (!acpi_evalf(dytc_handle, output, NULL, "dd", command))
return -EIO;
+ return 0;
+}
+
+static int lapsensor_get(bool *present, bool *state)
+{
+ int output, err;
+
+ *present = false;
+ err = dytc_command(DYTC_CMD_GET, &output);
+ if (err)
+ return err;
*present = true; /*If we get his far, we have lapmode support*/
*state = output & BIT(DYTC_GET_LAPMODE_BIT) ? true : false;
@@ -9982,6 +9994,264 @@ static struct ibm_struct proxsensor_driver_data = {
.exit = proxsensor_exit,
};
+#if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)
+
+/*************************************************************************
+ * DYTC Platform Profile interface
+ */
+
+#define DYTC_CMD_QUERY 0 /* To get DYTC status - enable/revision */
+#define DYTC_CMD_SET 1 /* To enable/disable IC function mode */
+#define DYTC_CMD_RESET 0x1ff /* To reset back to default */
+
+#define DYTC_QUERY_ENABLE_BIT 8 /* Bit 8 - 0 = disabled, 1 = enabled */
+#define DYTC_QUERY_SUBREV_BIT 16 /* Bits 16 - 27 - sub revision */
+#define DYTC_QUERY_REV_BIT 28 /* Bits 28 - 31 - revision */
+
+#define DYTC_GET_FUNCTION_BIT 8 /* Bits 8-11 - function setting */
+#define DYTC_GET_MODE_BIT 12 /* Bits 12-15 - mode setting */
+
+#define DYTC_SET_FUNCTION_BIT 12 /* Bits 12-15 - function setting */
+#define DYTC_SET_MODE_BIT 16 /* Bits 16-19 - mode setting */
+#define DYTC_SET_VALID_BIT 20 /* Bit 20 - 1 = on, 0 = off */
+
+#define DYTC_FUNCTION_STD 0 /* Function = 0, standard mode */
+#define DYTC_FUNCTION_CQL 1 /* Function = 1, lap mode */
+#define DYTC_FUNCTION_MMC 11 /* Function = 11, desk mode */
+
+#define DYTC_MODE_PERFORM 2 /* High power mode aka performance */
+#define DYTC_MODE_LOWPOWER 3 /* Low power mode */
+#define DYTC_MODE_BALANCE 0xF /* Default mode aka balanced */
+
+#define DYTC_SET_COMMAND(function, mode, on) \
+ (DYTC_CMD_SET | (function) << DYTC_SET_FUNCTION_BIT | \
+ (mode) << DYTC_SET_MODE_BIT | \
+ (on) << DYTC_SET_VALID_BIT)
+
+#define DYTC_DISABLE_CQL DYTC_SET_COMMAND(DYTC_FUNCTION_CQL, DYTC_MODE_BALANCE, 0)
+
+#define DYTC_ENABLE_CQL DYTC_SET_COMMAND(DYTC_FUNCTION_CQL, DYTC_MODE_BALANCE, 1)
+
+static bool dytc_profile_available;
+static enum platform_profile_option dytc_current_profile;
+static atomic_t dytc_ignore_event = ATOMIC_INIT(0);
+static DEFINE_MUTEX(dytc_mutex);
+
+static int convert_dytc_to_profile(int dytcmode, enum platform_profile_option *profile)
+{
+ switch (dytcmode) {
+ case DYTC_MODE_LOWPOWER:
+ *profile = PLATFORM_PROFILE_LOW_POWER;
+ break;
+ case DYTC_MODE_BALANCE:
+ *profile = PLATFORM_PROFILE_BALANCED;
+ break;
+ case DYTC_MODE_PERFORM:
+ *profile = PLATFORM_PROFILE_PERFORMANCE;
+ break;
+ default: /* Unknown mode */
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int convert_profile_to_dytc(enum platform_profile_option profile, int *perfmode)
+{
+ switch (profile) {
+ case PLATFORM_PROFILE_LOW_POWER:
+ *perfmode = DYTC_MODE_LOWPOWER;
+ break;
+ case PLATFORM_PROFILE_BALANCED:
+ *perfmode = DYTC_MODE_BALANCE;
+ break;
+ case PLATFORM_PROFILE_PERFORMANCE:
+ *perfmode = DYTC_MODE_PERFORM;
+ break;
+ default: /* Unknown profile */
+ return -EOPNOTSUPP;
+ }
+ return 0;
+}
+
+/*
+ * dytc_profile_get: Function to register with platform_profile
+ * handler. Returns current platform profile.
+ */
+int dytc_profile_get(struct platform_profile_handler *pprof,
+ enum platform_profile_option *profile)
+{
+ *profile = dytc_current_profile;
+ return 0;
+}
+
+/*
+ * Helper function - check if we are in CQL mode and if we are
+ * - disable CQL,
+ * - run the command
+ * - enable CQL
+ * If not in CQL mode, just run the command
+ */
+int dytc_cql_command(int command, int *output)
+{
+ int err, cmd_err, dummy;
+ int cur_funcmode;
+
+ /* Determine if we are in CQL mode. This alters the commands we do */
+ err = dytc_command(DYTC_CMD_GET, output);
+ if (err)
+ return err;
+
+ cur_funcmode = (*output >> DYTC_GET_FUNCTION_BIT) & 0xF;
+ /* Check if we're OK to return immediately */
+ if ((command == DYTC_CMD_GET) && (cur_funcmode != DYTC_FUNCTION_CQL))
+ return 0;
+
+ if (cur_funcmode == DYTC_FUNCTION_CQL) {
+ atomic_inc(&dytc_ignore_event);
+ err = dytc_command(DYTC_DISABLE_CQL, &dummy);
+ if (err)
+ return err;
+ }
+
+ cmd_err = dytc_command(command, output);
+ /* Check return condition after we've restored CQL state */
+
+ if (cur_funcmode == DYTC_FUNCTION_CQL) {
+ err = dytc_command(DYTC_ENABLE_CQL, &dummy);
+ if (err)
+ return err;
+ }
+
+ return cmd_err;
+}
+
+/*
+ * dytc_profile_set: Function to register with platform_profile
+ * handler. Sets current platform profile.
+ */
+int dytc_profile_set(struct platform_profile_handler *pprof,
+ enum platform_profile_option profile)
+{
+ int output;
+ int err;
+
+ if (!dytc_profile_available)
+ return -ENODEV;
+
+ err = mutex_lock_interruptible(&dytc_mutex);
+ if (err)
+ return err;
+
+ if (profile == PLATFORM_PROFILE_BALANCED) {
+ /* To get back to balanced mode we just issue a reset command */
+ err = dytc_command(DYTC_CMD_RESET, &output);
+ if (err)
+ goto unlock;
+ } else {
+ int perfmode;
+
+ err = convert_profile_to_dytc(profile, &perfmode);
+ if (err)
+ goto unlock;
+
+ /* Determine if we are in CQL mode. This alters the commands we do */
+ err = dytc_cql_command(DYTC_SET_COMMAND(DYTC_FUNCTION_MMC, perfmode, 1), &output);
+ if (err)
+ goto unlock;
+ }
+ /* Success - update current profile */
+ dytc_current_profile = profile;
+unlock:
+ mutex_unlock(&dytc_mutex);
+ return err;
+}
+
+static void dytc_profile_refresh(void)
+{
+ enum platform_profile_option profile;
+ int output, err;
+ int perfmode;
+
+ mutex_lock(&dytc_mutex);
+ err = dytc_cql_command(DYTC_CMD_GET, &output);
+ mutex_unlock(&dytc_mutex);
+ if (err)
+ return;
+
+ perfmode = (output >> DYTC_GET_MODE_BIT) & 0xF;
+ convert_dytc_to_profile(perfmode, &profile);
+ if (profile != dytc_current_profile) {
+ dytc_current_profile = profile;
+ platform_profile_notify();
+ }
+}
+
+static struct platform_profile_handler dytc_profile = {
+ .profile_get = dytc_profile_get,
+ .profile_set = dytc_profile_set,
+};
+
+static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
+{
+ int err, output;
+
+ /* Setup supported modes */
+ set_bit(PLATFORM_PROFILE_LOW_POWER, dytc_profile.choices);
+ set_bit(PLATFORM_PROFILE_BALANCED, dytc_profile.choices);
+ set_bit(PLATFORM_PROFILE_PERFORMANCE, dytc_profile.choices);
+
+ dytc_profile_available = false;
+ err = dytc_command(DYTC_CMD_QUERY, &output);
+ /*
+ * If support isn't available (ENODEV) then don't return an error
+ * and don't create the sysfs group
+ */
+ if (err == -ENODEV)
+ return 0;
+ /* For all other errors we can flag the failure */
+ if (err)
+ return err;
+
+ /* Check DYTC is enabled and supports mode setting */
+ if (output & BIT(DYTC_QUERY_ENABLE_BIT)) {
+ /* Only DYTC v5.0 and later has this feature. */
+ int dytc_version;
+
+ dytc_version = (output >> DYTC_QUERY_REV_BIT) & 0xF;
+ if (dytc_version >= 5) {
+ dbg_printk(TPACPI_DBG_INIT,
+ "DYTC version %d: thermal mode available\n", dytc_version);
+ /* Create platform_profile structure and register */
+ err = platform_profile_register(&dytc_profile);
+ /*
+ * If for some reason platform_profiles aren't enabled
+ * don't quit terminally.
+ */
+ if (err)
+ return 0;
+
+ dytc_profile_available = true;
+ /* Ensure initial values are correct */
+ dytc_profile_refresh();
+ }
+ }
+ return 0;
+}
+
+static void dytc_profile_exit(void)
+{
+ if (dytc_profile_available) {
+ dytc_profile_available = false;
+ platform_profile_remove();
+ }
+}
+
+static struct ibm_struct dytc_profile_driver_data = {
+ .name = "dytc-profile",
+ .exit = dytc_profile_exit,
+};
+#endif /* CONFIG_ACPI_PLATFORM_PROFILE */
+
/****************************************************************************
****************************************************************************
*
@@ -10030,8 +10300,14 @@ static void tpacpi_driver_event(const unsigned int hkey_event)
mutex_unlock(&kbdlight_mutex);
}
- if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED)
+ if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED) {
lapsensor_refresh();
+#if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)
+ /* If we are already accessing DYTC then skip dytc update */
+ if (!atomic_add_unless(&dytc_ignore_event, -1, 0))
+ dytc_profile_refresh();
+#endif
+ }
}
static void hotkey_driver_event(const unsigned int scancode)
@@ -10474,6 +10750,12 @@ static struct ibm_init_struct ibms_init[] __initdata = {
.init = tpacpi_proxsensor_init,
.data = &proxsensor_driver_data,
},
+#if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)
+ {
+ .init = tpacpi_dytc_profile_init,
+ .data = &dytc_profile_driver_data,
+ },
+#endif
};
static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
--
2.28.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v9] platform/x86: thinkpad_acpi: Add platform profile support
2021-01-11 16:22 [PATCH v9] platform/x86: thinkpad_acpi: Add platform profile support Mark Pearson
@ 2021-01-17 14:01 ` kernel test robot
2021-02-02 14:49 ` Hans de Goede
1 sibling, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-01-17 14:01 UTC (permalink / raw)
To: Mark Pearson
Cc: kbuild-all, hdegoede, mgross, rjw, linux-acpi,
platform-driver-x86, hadess, njoshi1
[-- Attachment #1: Type: text/plain, Size: 3923 bytes --]
Hi Mark,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.11-rc3 next-20210115]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Mark-Pearson/platform-x86-thinkpad_acpi-Add-platform-profile-support/20210112-003248
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 7c53f6b671f4aba70ff15e1b05148b10d58c2837
config: x86_64-rhel (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/958de6527b22cc8744001da18b8e9a9c0b85c201
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Mark-Pearson/platform-x86-thinkpad_acpi-Add-platform-profile-support/20210112-003248
git checkout 958de6527b22cc8744001da18b8e9a9c0b85c201
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/platform/x86/thinkpad_acpi.c:69:10: fatal error: linux/platform_profile.h: No such file or directory
69 | #include <linux/platform_profile.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
vim +69 drivers/platform/x86/thinkpad_acpi.c
13
14 /*
15 * Changelog:
16 * 2007-10-20 changelog trimmed down
17 *
18 * 2007-03-27 0.14 renamed to thinkpad_acpi and moved to
19 * drivers/misc.
20 *
21 * 2006-11-22 0.13 new maintainer
22 * changelog now lives in git commit history, and will
23 * not be updated further in-file.
24 *
25 * 2005-03-17 0.11 support for 600e, 770x
26 * thanks to Jamie Lentin <lentinj@dial.pipex.com>
27 *
28 * 2005-01-16 0.9 use MODULE_VERSION
29 * thanks to Henrik Brix Andersen <brix@gentoo.org>
30 * fix parameter passing on module loading
31 * thanks to Rusty Russell <rusty@rustcorp.com.au>
32 * thanks to Jim Radford <radford@blackbean.org>
33 * 2004-11-08 0.8 fix init error case, don't return from a macro
34 * thanks to Chris Wright <chrisw@osdl.org>
35 */
36
37 #include <linux/kernel.h>
38 #include <linux/module.h>
39 #include <linux/init.h>
40 #include <linux/types.h>
41 #include <linux/string.h>
42 #include <linux/list.h>
43 #include <linux/mutex.h>
44 #include <linux/sched.h>
45 #include <linux/sched/signal.h>
46 #include <linux/kthread.h>
47 #include <linux/freezer.h>
48 #include <linux/delay.h>
49 #include <linux/slab.h>
50 #include <linux/nvram.h>
51 #include <linux/proc_fs.h>
52 #include <linux/seq_file.h>
53 #include <linux/sysfs.h>
54 #include <linux/backlight.h>
55 #include <linux/bitops.h>
56 #include <linux/fb.h>
57 #include <linux/platform_device.h>
58 #include <linux/hwmon.h>
59 #include <linux/hwmon-sysfs.h>
60 #include <linux/input.h>
61 #include <linux/leds.h>
62 #include <linux/rfkill.h>
63 #include <linux/dmi.h>
64 #include <linux/jiffies.h>
65 #include <linux/workqueue.h>
66 #include <linux/acpi.h>
67 #include <linux/pci.h>
68 #include <linux/power_supply.h>
> 69 #include <linux/platform_profile.h>
70 #include <sound/core.h>
71 #include <sound/control.h>
72 #include <sound/initval.h>
73 #include <linux/uaccess.h>
74 #include <acpi/battery.h>
75 #include <acpi/video.h>
76
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45923 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v9] platform/x86: thinkpad_acpi: Add platform profile support
@ 2021-01-17 14:01 ` kernel test robot
0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-01-17 14:01 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4028 bytes --]
Hi Mark,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.11-rc3 next-20210115]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Mark-Pearson/platform-x86-thinkpad_acpi-Add-platform-profile-support/20210112-003248
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 7c53f6b671f4aba70ff15e1b05148b10d58c2837
config: x86_64-rhel (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/958de6527b22cc8744001da18b8e9a9c0b85c201
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Mark-Pearson/platform-x86-thinkpad_acpi-Add-platform-profile-support/20210112-003248
git checkout 958de6527b22cc8744001da18b8e9a9c0b85c201
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/platform/x86/thinkpad_acpi.c:69:10: fatal error: linux/platform_profile.h: No such file or directory
69 | #include <linux/platform_profile.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
vim +69 drivers/platform/x86/thinkpad_acpi.c
13
14 /*
15 * Changelog:
16 * 2007-10-20 changelog trimmed down
17 *
18 * 2007-03-27 0.14 renamed to thinkpad_acpi and moved to
19 * drivers/misc.
20 *
21 * 2006-11-22 0.13 new maintainer
22 * changelog now lives in git commit history, and will
23 * not be updated further in-file.
24 *
25 * 2005-03-17 0.11 support for 600e, 770x
26 * thanks to Jamie Lentin <lentinj@dial.pipex.com>
27 *
28 * 2005-01-16 0.9 use MODULE_VERSION
29 * thanks to Henrik Brix Andersen <brix@gentoo.org>
30 * fix parameter passing on module loading
31 * thanks to Rusty Russell <rusty@rustcorp.com.au>
32 * thanks to Jim Radford <radford@blackbean.org>
33 * 2004-11-08 0.8 fix init error case, don't return from a macro
34 * thanks to Chris Wright <chrisw@osdl.org>
35 */
36
37 #include <linux/kernel.h>
38 #include <linux/module.h>
39 #include <linux/init.h>
40 #include <linux/types.h>
41 #include <linux/string.h>
42 #include <linux/list.h>
43 #include <linux/mutex.h>
44 #include <linux/sched.h>
45 #include <linux/sched/signal.h>
46 #include <linux/kthread.h>
47 #include <linux/freezer.h>
48 #include <linux/delay.h>
49 #include <linux/slab.h>
50 #include <linux/nvram.h>
51 #include <linux/proc_fs.h>
52 #include <linux/seq_file.h>
53 #include <linux/sysfs.h>
54 #include <linux/backlight.h>
55 #include <linux/bitops.h>
56 #include <linux/fb.h>
57 #include <linux/platform_device.h>
58 #include <linux/hwmon.h>
59 #include <linux/hwmon-sysfs.h>
60 #include <linux/input.h>
61 #include <linux/leds.h>
62 #include <linux/rfkill.h>
63 #include <linux/dmi.h>
64 #include <linux/jiffies.h>
65 #include <linux/workqueue.h>
66 #include <linux/acpi.h>
67 #include <linux/pci.h>
68 #include <linux/power_supply.h>
> 69 #include <linux/platform_profile.h>
70 #include <sound/core.h>
71 #include <sound/control.h>
72 #include <sound/initval.h>
73 #include <linux/uaccess.h>
74 #include <acpi/battery.h>
75 #include <acpi/video.h>
76
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 45923 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v9] platform/x86: thinkpad_acpi: Add platform profile support
2021-01-11 16:22 [PATCH v9] platform/x86: thinkpad_acpi: Add platform profile support Mark Pearson
2021-01-17 14:01 ` kernel test robot
@ 2021-02-02 14:49 ` Hans de Goede
2021-02-03 14:46 ` [External] " Mark Pearson
1 sibling, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2021-02-02 14:49 UTC (permalink / raw)
To: Mark Pearson
Cc: mgross, rjw, linux-acpi, platform-driver-x86, hadess, njoshi1
Hi,
On 1/11/21 5:22 PM, Mark Pearson wrote:
> Add support to thinkpad_acpi for Lenovo platforms that have DYTC
> version 5 support or newer to use the platform profile feature.
>
> This will allow users to determine and control the platform modes
> between low-power, balanced operation and performance modes.
>
> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
Now that the acpi/platform_profile changes have landed I have
merged this patch (solving a trivial conflict caused by the
keyboard_lang changes).
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:
> Address (hopefully) all recommendations from review including:
> - use IS_ENABLED instead of IS_DEFINED
> - update driver to work with all the fixes in platform_profile update
> - improve error handling for invalid inputs
> - move tracking of current profile mode into this driver
>
> Changes in v3:
> - version update for patch series
>
> Changes in v4:
> - Rebase on top of palm sensor patch which led to a little bit of file
> restructuring/clean up
> - Use BIT macro where applicable
> - Formatting fixes
> - Check sysfs node created on exit function
> - implement and use DYTC_SET_COMMAND macro
> - in case of failure setting performance mode make sure CQL mode is
> enabled again before returning.
> - Clean up initialisation and error handling code
>
> Changes in v5:
> - Use atomic_int with ignoring events
> - Add mutex when accessing ACPI information
> - Clean up registration code
> - Use cached value in profile_get function
> - Add dytc_cql_command function to clean up and provide common handler
> - Update to work with changes in platform_profile implementation
>
> Changes in v6:
> - Update file to build against update in v6 platform_profile
>
> Changes in v7 & v8:
> - version bump along with rest of patch series
>
> Changes in v9:
> - Update define names to be correct with the linux-pm branches accepted
> version
> - Rename DYTC_MODE_QUIET to LOW_POWER just because it's confusing. It
> was originally based on internal documentation.
> Note - originally this patch was part of a series but with the other two
> pieces being accepted into the acpi maintainers branch this one becomes
> standalone.
>
> drivers/platform/x86/thinkpad_acpi.c | 294 ++++++++++++++++++++++++++-
> 1 file changed, 288 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index e03df2881dc6..c5645331cfac 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -66,6 +66,7 @@
> #include <linux/acpi.h>
> #include <linux/pci.h>
> #include <linux/power_supply.h>
> +#include <linux/platform_profile.h>
> #include <sound/core.h>
> #include <sound/control.h>
> #include <sound/initval.h>
> @@ -9854,16 +9855,27 @@ static bool has_lapsensor;
> static bool palm_state;
> static bool lap_state;
>
> -static int lapsensor_get(bool *present, bool *state)
> +static int dytc_command(int command, int *output)
> {
> acpi_handle dytc_handle;
> - int output;
>
> - *present = false;
> - if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", &dytc_handle)))
> + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", &dytc_handle))) {
> + /* Platform doesn't support DYTC */
> return -ENODEV;
> - if (!acpi_evalf(dytc_handle, &output, NULL, "dd", DYTC_CMD_GET))
> + }
> + if (!acpi_evalf(dytc_handle, output, NULL, "dd", command))
> return -EIO;
> + return 0;
> +}
> +
> +static int lapsensor_get(bool *present, bool *state)
> +{
> + int output, err;
> +
> + *present = false;
> + err = dytc_command(DYTC_CMD_GET, &output);
> + if (err)
> + return err;
>
> *present = true; /*If we get his far, we have lapmode support*/
> *state = output & BIT(DYTC_GET_LAPMODE_BIT) ? true : false;
> @@ -9982,6 +9994,264 @@ static struct ibm_struct proxsensor_driver_data = {
> .exit = proxsensor_exit,
> };
>
> +#if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)
> +
> +/*************************************************************************
> + * DYTC Platform Profile interface
> + */
> +
> +#define DYTC_CMD_QUERY 0 /* To get DYTC status - enable/revision */
> +#define DYTC_CMD_SET 1 /* To enable/disable IC function mode */
> +#define DYTC_CMD_RESET 0x1ff /* To reset back to default */
> +
> +#define DYTC_QUERY_ENABLE_BIT 8 /* Bit 8 - 0 = disabled, 1 = enabled */
> +#define DYTC_QUERY_SUBREV_BIT 16 /* Bits 16 - 27 - sub revision */
> +#define DYTC_QUERY_REV_BIT 28 /* Bits 28 - 31 - revision */
> +
> +#define DYTC_GET_FUNCTION_BIT 8 /* Bits 8-11 - function setting */
> +#define DYTC_GET_MODE_BIT 12 /* Bits 12-15 - mode setting */
> +
> +#define DYTC_SET_FUNCTION_BIT 12 /* Bits 12-15 - function setting */
> +#define DYTC_SET_MODE_BIT 16 /* Bits 16-19 - mode setting */
> +#define DYTC_SET_VALID_BIT 20 /* Bit 20 - 1 = on, 0 = off */
> +
> +#define DYTC_FUNCTION_STD 0 /* Function = 0, standard mode */
> +#define DYTC_FUNCTION_CQL 1 /* Function = 1, lap mode */
> +#define DYTC_FUNCTION_MMC 11 /* Function = 11, desk mode */
> +
> +#define DYTC_MODE_PERFORM 2 /* High power mode aka performance */
> +#define DYTC_MODE_LOWPOWER 3 /* Low power mode */
> +#define DYTC_MODE_BALANCE 0xF /* Default mode aka balanced */
> +
> +#define DYTC_SET_COMMAND(function, mode, on) \
> + (DYTC_CMD_SET | (function) << DYTC_SET_FUNCTION_BIT | \
> + (mode) << DYTC_SET_MODE_BIT | \
> + (on) << DYTC_SET_VALID_BIT)
> +
> +#define DYTC_DISABLE_CQL DYTC_SET_COMMAND(DYTC_FUNCTION_CQL, DYTC_MODE_BALANCE, 0)
> +
> +#define DYTC_ENABLE_CQL DYTC_SET_COMMAND(DYTC_FUNCTION_CQL, DYTC_MODE_BALANCE, 1)
> +
> +static bool dytc_profile_available;
> +static enum platform_profile_option dytc_current_profile;
> +static atomic_t dytc_ignore_event = ATOMIC_INIT(0);
> +static DEFINE_MUTEX(dytc_mutex);
> +
> +static int convert_dytc_to_profile(int dytcmode, enum platform_profile_option *profile)
> +{
> + switch (dytcmode) {
> + case DYTC_MODE_LOWPOWER:
> + *profile = PLATFORM_PROFILE_LOW_POWER;
> + break;
> + case DYTC_MODE_BALANCE:
> + *profile = PLATFORM_PROFILE_BALANCED;
> + break;
> + case DYTC_MODE_PERFORM:
> + *profile = PLATFORM_PROFILE_PERFORMANCE;
> + break;
> + default: /* Unknown mode */
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int convert_profile_to_dytc(enum platform_profile_option profile, int *perfmode)
> +{
> + switch (profile) {
> + case PLATFORM_PROFILE_LOW_POWER:
> + *perfmode = DYTC_MODE_LOWPOWER;
> + break;
> + case PLATFORM_PROFILE_BALANCED:
> + *perfmode = DYTC_MODE_BALANCE;
> + break;
> + case PLATFORM_PROFILE_PERFORMANCE:
> + *perfmode = DYTC_MODE_PERFORM;
> + break;
> + default: /* Unknown profile */
> + return -EOPNOTSUPP;
> + }
> + return 0;
> +}
> +
> +/*
> + * dytc_profile_get: Function to register with platform_profile
> + * handler. Returns current platform profile.
> + */
> +int dytc_profile_get(struct platform_profile_handler *pprof,
> + enum platform_profile_option *profile)
> +{
> + *profile = dytc_current_profile;
> + return 0;
> +}
> +
> +/*
> + * Helper function - check if we are in CQL mode and if we are
> + * - disable CQL,
> + * - run the command
> + * - enable CQL
> + * If not in CQL mode, just run the command
> + */
> +int dytc_cql_command(int command, int *output)
> +{
> + int err, cmd_err, dummy;
> + int cur_funcmode;
> +
> + /* Determine if we are in CQL mode. This alters the commands we do */
> + err = dytc_command(DYTC_CMD_GET, output);
> + if (err)
> + return err;
> +
> + cur_funcmode = (*output >> DYTC_GET_FUNCTION_BIT) & 0xF;
> + /* Check if we're OK to return immediately */
> + if ((command == DYTC_CMD_GET) && (cur_funcmode != DYTC_FUNCTION_CQL))
> + return 0;
> +
> + if (cur_funcmode == DYTC_FUNCTION_CQL) {
> + atomic_inc(&dytc_ignore_event);
> + err = dytc_command(DYTC_DISABLE_CQL, &dummy);
> + if (err)
> + return err;
> + }
> +
> + cmd_err = dytc_command(command, output);
> + /* Check return condition after we've restored CQL state */
> +
> + if (cur_funcmode == DYTC_FUNCTION_CQL) {
> + err = dytc_command(DYTC_ENABLE_CQL, &dummy);
> + if (err)
> + return err;
> + }
> +
> + return cmd_err;
> +}
> +
> +/*
> + * dytc_profile_set: Function to register with platform_profile
> + * handler. Sets current platform profile.
> + */
> +int dytc_profile_set(struct platform_profile_handler *pprof,
> + enum platform_profile_option profile)
> +{
> + int output;
> + int err;
> +
> + if (!dytc_profile_available)
> + return -ENODEV;
> +
> + err = mutex_lock_interruptible(&dytc_mutex);
> + if (err)
> + return err;
> +
> + if (profile == PLATFORM_PROFILE_BALANCED) {
> + /* To get back to balanced mode we just issue a reset command */
> + err = dytc_command(DYTC_CMD_RESET, &output);
> + if (err)
> + goto unlock;
> + } else {
> + int perfmode;
> +
> + err = convert_profile_to_dytc(profile, &perfmode);
> + if (err)
> + goto unlock;
> +
> + /* Determine if we are in CQL mode. This alters the commands we do */
> + err = dytc_cql_command(DYTC_SET_COMMAND(DYTC_FUNCTION_MMC, perfmode, 1), &output);
> + if (err)
> + goto unlock;
> + }
> + /* Success - update current profile */
> + dytc_current_profile = profile;
> +unlock:
> + mutex_unlock(&dytc_mutex);
> + return err;
> +}
> +
> +static void dytc_profile_refresh(void)
> +{
> + enum platform_profile_option profile;
> + int output, err;
> + int perfmode;
> +
> + mutex_lock(&dytc_mutex);
> + err = dytc_cql_command(DYTC_CMD_GET, &output);
> + mutex_unlock(&dytc_mutex);
> + if (err)
> + return;
> +
> + perfmode = (output >> DYTC_GET_MODE_BIT) & 0xF;
> + convert_dytc_to_profile(perfmode, &profile);
> + if (profile != dytc_current_profile) {
> + dytc_current_profile = profile;
> + platform_profile_notify();
> + }
> +}
> +
> +static struct platform_profile_handler dytc_profile = {
> + .profile_get = dytc_profile_get,
> + .profile_set = dytc_profile_set,
> +};
> +
> +static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
> +{
> + int err, output;
> +
> + /* Setup supported modes */
> + set_bit(PLATFORM_PROFILE_LOW_POWER, dytc_profile.choices);
> + set_bit(PLATFORM_PROFILE_BALANCED, dytc_profile.choices);
> + set_bit(PLATFORM_PROFILE_PERFORMANCE, dytc_profile.choices);
> +
> + dytc_profile_available = false;
> + err = dytc_command(DYTC_CMD_QUERY, &output);
> + /*
> + * If support isn't available (ENODEV) then don't return an error
> + * and don't create the sysfs group
> + */
> + if (err == -ENODEV)
> + return 0;
> + /* For all other errors we can flag the failure */
> + if (err)
> + return err;
> +
> + /* Check DYTC is enabled and supports mode setting */
> + if (output & BIT(DYTC_QUERY_ENABLE_BIT)) {
> + /* Only DYTC v5.0 and later has this feature. */
> + int dytc_version;
> +
> + dytc_version = (output >> DYTC_QUERY_REV_BIT) & 0xF;
> + if (dytc_version >= 5) {
> + dbg_printk(TPACPI_DBG_INIT,
> + "DYTC version %d: thermal mode available\n", dytc_version);
> + /* Create platform_profile structure and register */
> + err = platform_profile_register(&dytc_profile);
> + /*
> + * If for some reason platform_profiles aren't enabled
> + * don't quit terminally.
> + */
> + if (err)
> + return 0;
> +
> + dytc_profile_available = true;
> + /* Ensure initial values are correct */
> + dytc_profile_refresh();
> + }
> + }
> + return 0;
> +}
> +
> +static void dytc_profile_exit(void)
> +{
> + if (dytc_profile_available) {
> + dytc_profile_available = false;
> + platform_profile_remove();
> + }
> +}
> +
> +static struct ibm_struct dytc_profile_driver_data = {
> + .name = "dytc-profile",
> + .exit = dytc_profile_exit,
> +};
> +#endif /* CONFIG_ACPI_PLATFORM_PROFILE */
> +
> /****************************************************************************
> ****************************************************************************
> *
> @@ -10030,8 +10300,14 @@ static void tpacpi_driver_event(const unsigned int hkey_event)
> mutex_unlock(&kbdlight_mutex);
> }
>
> - if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED)
> + if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED) {
> lapsensor_refresh();
> +#if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)
> + /* If we are already accessing DYTC then skip dytc update */
> + if (!atomic_add_unless(&dytc_ignore_event, -1, 0))
> + dytc_profile_refresh();
> +#endif
> + }
> }
>
> static void hotkey_driver_event(const unsigned int scancode)
> @@ -10474,6 +10750,12 @@ static struct ibm_init_struct ibms_init[] __initdata = {
> .init = tpacpi_proxsensor_init,
> .data = &proxsensor_driver_data,
> },
> +#if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)
> + {
> + .init = tpacpi_dytc_profile_init,
> + .data = &dytc_profile_driver_data,
> + },
> +#endif
> };
>
> static int __init set_ibm_param(const char *val, const struct kernel_param *kp)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [External] Re: [PATCH v9] platform/x86: thinkpad_acpi: Add platform profile support
2021-02-02 14:49 ` Hans de Goede
@ 2021-02-03 14:46 ` Mark Pearson
2021-02-04 9:18 ` Hans de Goede
0 siblings, 1 reply; 7+ messages in thread
From: Mark Pearson @ 2021-02-03 14:46 UTC (permalink / raw)
To: Hans de Goede
Cc: mgross, rjw, linux-acpi, platform-driver-x86, hadess, njoshi1
On 02/02/2021 09:49, Hans de Goede wrote:
> Hi,
>
> On 1/11/21 5:22 PM, Mark Pearson wrote:
>> Add support to thinkpad_acpi for Lenovo platforms that have DYTC
>> version 5 support or newer to use the platform profile feature.
>>
>> This will allow users to determine and control the platform modes
>> between low-power, balanced operation and performance modes.
>>
>> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
>
> Now that the acpi/platform_profile changes have landed I have
> merged this patch (solving a trivial conflict caused by the
> keyboard_lang changes).
>
> 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.
>
Thanks Hans - sounds great.
Let me know if you find any issues or need any extra tests running.
Happy to do that
Mark
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [External] Re: [PATCH v9] platform/x86: thinkpad_acpi: Add platform profile support
2021-02-03 14:46 ` [External] " Mark Pearson
@ 2021-02-04 9:18 ` Hans de Goede
2021-02-04 15:40 ` Mark Pearson
0 siblings, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2021-02-04 9:18 UTC (permalink / raw)
To: Mark Pearson
Cc: mgross, rjw, linux-acpi, platform-driver-x86, hadess, njoshi1
Hi,
On 2/3/21 3:46 PM, Mark Pearson wrote:
> On 02/02/2021 09:49, Hans de Goede wrote:
>> Hi,
>>
>> On 1/11/21 5:22 PM, Mark Pearson wrote:
>>> Add support to thinkpad_acpi for Lenovo platforms that have DYTC
>>> version 5 support or newer to use the platform profile feature.
>>>
>>> This will allow users to determine and control the platform modes
>>> between low-power, balanced operation and performance modes.
>>>
>>> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
>>
>> Now that the acpi/platform_profile changes have landed I have
>> merged this patch (solving a trivial conflict caused by the
>> keyboard_lang changes).
>>
>> 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.
>>
> Thanks Hans - sounds great.
> Let me know if you find any issues or need any extra tests running.
So the build-bots have found 2 issues:
1. Some symbols which are not exported need to be marked static (I will fix this myself,
that is the easiest / fastest):
drivers/platform/x86/thinkpad_acpi.c:10081:5: warning: no previous prototype for 'dytc_profile_get' [-Wmissing-prototypes]
drivers/platform/x86/thinkpad_acpi.c:10095:5: warning: no previous prototype for 'dytc_cql_command' [-Wmissing-prototypes]
drivers/platform/x86/thinkpad_acpi.c:10133:5: warning: no previous prototype for 'dytc_profile_set' [-Wmissing-prototypes]
2. This is a bigger problem, this is the result of a random-config test-build and I'm
pretty sure that the issue is that acpi_platform was build as a module while
thinkpad_acpi was builtin and builtin code cannot rely on modules.
drivers/platform/x86/thinkpad_acpi.c:10186: undefined reference to `platform_profile_notify'
drivers/platform/x86/thinkpad_acpi.c:10226: undefined reference to `platform_profile_register'
drivers/platform/x86/thinkpad_acpi.c:10246: undefined reference to `platform_profile_remove'
There are 2 ways to solve this:
1. Change
#if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)
to:
#if IS_REACHABLE(CONFIG_ACPI_PLATFORM_PROFILE)
Which will disable the platform-profile support in acpi_thinkpad in the above scenario.
or.
2. Drop the #if IS_ENABLED(...) and add a
depends on ACPI_PLATFORM_PROFILE
To the THINKPAD_ACPI Kconfig symbol.
I personally think 2. would be slightly better as it ensures that platform-profile
support is always available when thinkpad_acpi is build, hopefully leading to less
confusing bug-reports about it sometimes not working.
If you can let me know what you want, then I can fix this locally too and get
the fix in place before the merge-window opens.
Regards,
Hans
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [External] Re: [PATCH v9] platform/x86: thinkpad_acpi: Add platform profile support
2021-02-04 9:18 ` Hans de Goede
@ 2021-02-04 15:40 ` Mark Pearson
0 siblings, 0 replies; 7+ messages in thread
From: Mark Pearson @ 2021-02-04 15:40 UTC (permalink / raw)
To: Hans de Goede
Cc: mgross, rjw, linux-acpi, platform-driver-x86, hadess, njoshi1
Hi Hans
On 04/02/2021 04:18, Hans de Goede wrote:
> Hi,
>
> On 2/3/21 3:46 PM, Mark Pearson wrote:
>> On 02/02/2021 09:49, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 1/11/21 5:22 PM, Mark Pearson wrote:
>>>> Add support to thinkpad_acpi for Lenovo platforms that have DYTC
>>>> version 5 support or newer to use the platform profile feature.
>>>>
>>>> This will allow users to determine and control the platform modes
>>>> between low-power, balanced operation and performance modes.
>>>>
>>>> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
>>>
>>> Now that the acpi/platform_profile changes have landed I have
>>> merged this patch (solving a trivial conflict caused by the
>>> keyboard_lang changes).
>>>
>>> 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.
>>>
>> Thanks Hans - sounds great.
>> Let me know if you find any issues or need any extra tests running.
>
> So the build-bots have found 2 issues:
>
> 1. Some symbols which are not exported need to be marked static (I will fix this myself,
> that is the easiest / fastest):
>
> drivers/platform/x86/thinkpad_acpi.c:10081:5: warning: no previous prototype for 'dytc_profile_get' [-Wmissing-prototypes]
> drivers/platform/x86/thinkpad_acpi.c:10095:5: warning: no previous prototype for 'dytc_cql_command' [-Wmissing-prototypes]
> drivers/platform/x86/thinkpad_acpi.c:10133:5: warning: no previous prototype for 'dytc_profile_set' [-Wmissing-prototypes]
>
> 2. This is a bigger problem, this is the result of a random-config test-build and I'm
> pretty sure that the issue is that acpi_platform was build as a module while
> thinkpad_acpi was builtin and builtin code cannot rely on modules.
>
> drivers/platform/x86/thinkpad_acpi.c:10186: undefined reference to `platform_profile_notify'
> drivers/platform/x86/thinkpad_acpi.c:10226: undefined reference to `platform_profile_register'
> drivers/platform/x86/thinkpad_acpi.c:10246: undefined reference to `platform_profile_remove'
>
> There are 2 ways to solve this:
>
> 1. Change
>
> #if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE)
>
> to:
>
> #if IS_REACHABLE(CONFIG_ACPI_PLATFORM_PROFILE)
>
> Which will disable the platform-profile support in acpi_thinkpad in the above scenario.
>
> or.
>
> 2. Drop the #if IS_ENABLED(...) and add a
>
> depends on ACPI_PLATFORM_PROFILE
>
> To the THINKPAD_ACPI Kconfig symbol.
>
>
> I personally think 2. would be slightly better as it ensures that platform-profile
> support is always available when thinkpad_acpi is build, hopefully leading to less
> confusing bug-reports about it sometimes not working.
>
> If you can let me know what you want, then I can fix this locally too and get
> the fix in place before the merge-window opens.
>
> Regards,
>
> Hans
>
I think you've fixed both of those already - and I agree with #2. Thanks
for jumping on these.
Let me know if I need to do anything to help.
Thanks!
Mark
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-02-04 15:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-11 16:22 [PATCH v9] platform/x86: thinkpad_acpi: Add platform profile support Mark Pearson
2021-01-17 14:01 ` kernel test robot
2021-01-17 14:01 ` kernel test robot
2021-02-02 14:49 ` Hans de Goede
2021-02-03 14:46 ` [External] " Mark Pearson
2021-02-04 9:18 ` Hans de Goede
2021-02-04 15:40 ` Mark Pearson
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.