* [PATCH] platform/x86: thinkpad_acpi: check dytc version for lapmode sysfs
@ 2021-03-11 17:48 Mark Pearson
2021-03-18 12:17 ` Hans de Goede
0 siblings, 1 reply; 3+ messages in thread
From: Mark Pearson @ 2021-03-11 17:48 UTC (permalink / raw)
To: markpearson; +Cc: hdegoede, mgross, platform-driver-x86
Lenovo platforms with DYTC versions earlier than version 5 don't set
the lapmode interface correctly, causing issues with thermald on
older platforms.
Add checking to only create the dytc_lapmode interface for version
5 and later.
Signed-off-by: Mark Pearson <markpearson@lenovo.com>
---
drivers/platform/x86/thinkpad_acpi.c | 91 ++++++++++++++++++++--------
1 file changed, 65 insertions(+), 26 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index b881044b31b0..f7de90a47e28 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9845,6 +9845,11 @@ static struct ibm_struct lcdshadow_driver_data = {
* Thinkpad sensor interfaces
*/
+#define DYTC_CMD_QUERY 0 /* To get DYTC status - enable/revision */
+#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_CMD_GET 2 /* To get current IC function and mode */
#define DYTC_GET_LAPMODE_BIT 17 /* Set when in lapmode */
@@ -9855,6 +9860,7 @@ static bool has_palmsensor;
static bool has_lapsensor;
static bool palm_state;
static bool lap_state;
+static int dytc_version;
static int dytc_command(int command, int *output)
{
@@ -9869,6 +9875,33 @@ static int dytc_command(int command, int *output)
return 0;
}
+static int dytc_get_version(void)
+{
+ int err, output;
+
+ /* Check if we've been called before - and just return cached value */
+ if (dytc_version)
+ return dytc_version;
+
+ /* Otherwise query DYTC and extract version information */
+ 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))
+ dytc_version = (output >> DYTC_QUERY_REV_BIT) & 0xF;
+
+ return 0;
+}
+
static int lapsensor_get(bool *present, bool *state)
{
int output, err;
@@ -9974,7 +10007,18 @@ static int tpacpi_proxsensor_init(struct ibm_init_struct *iibm)
if (err)
return err;
}
- if (has_lapsensor) {
+
+ /* Check if we know the DYTC version, if we don't then get it */
+ if (!dytc_version) {
+ err = dytc_get_version();
+ if (err)
+ return err;
+ }
+ /*
+ * Platforms before DYTC version 5 claim to have a lap sensor, but it doesn't work, so we
+ * ignore them
+ */
+ if (has_lapsensor && (dytc_version >= 5)) {
err = sysfs_create_file(&tpacpi_pdev->dev.kobj, &dev_attr_dytc_lapmode.attr);
if (err)
return err;
@@ -9999,14 +10043,9 @@ static struct ibm_struct proxsensor_driver_data = {
* 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 */
@@ -10211,28 +10250,28 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
if (err)
return err;
+ /* Check if we know the DYTC version, if we don't then get it */
+ if (!dytc_version) {
+ err = dytc_get_version();
+ 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;
+ 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();
- }
+ dytc_profile_available = true;
+ /* Ensure initial values are correct */
+ dytc_profile_refresh();
}
return 0;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] platform/x86: thinkpad_acpi: check dytc version for lapmode sysfs
2021-03-11 17:48 [PATCH] platform/x86: thinkpad_acpi: check dytc version for lapmode sysfs Mark Pearson
@ 2021-03-18 12:17 ` Hans de Goede
2021-03-18 12:43 ` [External] " Mark Pearson
0 siblings, 1 reply; 3+ messages in thread
From: Hans de Goede @ 2021-03-18 12:17 UTC (permalink / raw)
To: Mark Pearson; +Cc: mgross, platform-driver-x86
Hi,
On 3/11/21 6:48 PM, Mark Pearson wrote:
> Lenovo platforms with DYTC versions earlier than version 5 don't set
> the lapmode interface correctly, causing issues with thermald on
> older platforms.
>
> Add checking to only create the dytc_lapmode interface for version
> 5 and later.
>
> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
I've added a:
Fixes: 1ac09656bded ("platform/x86: thinkpad_acpi: Add palm sensor support")
Tag, this will help with the patch automatically getting selected for
stable kernel-series which contain the patch pointed to by the Fixes: tag.
In this case this won't work since this patch seems to depend on code
introduced in:
commit c3bfcd4c676238 ("platform/x86: thinkpad_acpi: lap or desk mode interface")
So you will need to manually backport this and submit it to stable@vger.kernel.org
if you want it to be included in any of the stable kernel series.
Still it is always good to have the Fixes: tag present when a commit is actually
fixing a previous commit.
###
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
> ---
> drivers/platform/x86/thinkpad_acpi.c | 91 ++++++++++++++++++++--------
> 1 file changed, 65 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index b881044b31b0..f7de90a47e28 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -9845,6 +9845,11 @@ static struct ibm_struct lcdshadow_driver_data = {
> * Thinkpad sensor interfaces
> */
>
> +#define DYTC_CMD_QUERY 0 /* To get DYTC status - enable/revision */
> +#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_CMD_GET 2 /* To get current IC function and mode */
> #define DYTC_GET_LAPMODE_BIT 17 /* Set when in lapmode */
>
> @@ -9855,6 +9860,7 @@ static bool has_palmsensor;
> static bool has_lapsensor;
> static bool palm_state;
> static bool lap_state;
> +static int dytc_version;
>
> static int dytc_command(int command, int *output)
> {
> @@ -9869,6 +9875,33 @@ static int dytc_command(int command, int *output)
> return 0;
> }
>
> +static int dytc_get_version(void)
> +{
> + int err, output;
> +
> + /* Check if we've been called before - and just return cached value */
> + if (dytc_version)
> + return dytc_version;
> +
> + /* Otherwise query DYTC and extract version information */
> + 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))
> + dytc_version = (output >> DYTC_QUERY_REV_BIT) & 0xF;
> +
> + return 0;
> +}
> +
> static int lapsensor_get(bool *present, bool *state)
> {
> int output, err;
> @@ -9974,7 +10007,18 @@ static int tpacpi_proxsensor_init(struct ibm_init_struct *iibm)
> if (err)
> return err;
> }
> - if (has_lapsensor) {
> +
> + /* Check if we know the DYTC version, if we don't then get it */
> + if (!dytc_version) {
> + err = dytc_get_version();
> + if (err)
> + return err;
> + }
> + /*
> + * Platforms before DYTC version 5 claim to have a lap sensor, but it doesn't work, so we
> + * ignore them
> + */
> + if (has_lapsensor && (dytc_version >= 5)) {
> err = sysfs_create_file(&tpacpi_pdev->dev.kobj, &dev_attr_dytc_lapmode.attr);
> if (err)
> return err;
> @@ -9999,14 +10043,9 @@ static struct ibm_struct proxsensor_driver_data = {
> * 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 */
>
> @@ -10211,28 +10250,28 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
> if (err)
> return err;
>
> + /* Check if we know the DYTC version, if we don't then get it */
> + if (!dytc_version) {
> + err = dytc_get_version();
> + 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;
> + 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();
> - }
> + dytc_profile_available = true;
> + /* Ensure initial values are correct */
> + dytc_profile_refresh();
> }
> return 0;
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [External] Re: [PATCH] platform/x86: thinkpad_acpi: check dytc version for lapmode sysfs
2021-03-18 12:17 ` Hans de Goede
@ 2021-03-18 12:43 ` Mark Pearson
0 siblings, 0 replies; 3+ messages in thread
From: Mark Pearson @ 2021-03-18 12:43 UTC (permalink / raw)
To: Hans de Goede; +Cc: mgross, platform-driver-x86
On 18/03/2021 08:17, Hans de Goede wrote:
> Hi,
>
> On 3/11/21 6:48 PM, Mark Pearson wrote:
>> Lenovo platforms with DYTC versions earlier than version 5 don't set
>> the lapmode interface correctly, causing issues with thermald on
>> older platforms.
>>
>> Add checking to only create the dytc_lapmode interface for version
>> 5 and later.
>>
>> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
>
> I've added a:
>
> Fixes: 1ac09656bded ("platform/x86: thinkpad_acpi: Add palm sensor support")
>
> Tag, this will help with the patch automatically getting selected for
> stable kernel-series which contain the patch pointed to by the Fixes: tag.
>
> In this case this won't work since this patch seems to depend on code
> introduced in:
>
> commit c3bfcd4c676238 ("platform/x86: thinkpad_acpi: lap or desk mode interface")
>
> So you will need to manually backport this and submit it to stable@vger.kernel.org
> if you want it to be included in any of the stable kernel series.
>
> Still it is always good to have the Fixes: tag present when a commit is actually
> fixing a previous commit.
>
I wasn't aware of the linux-stable kernel so it was good to learn
about. I'll have a look at doing that process - I don't know how many
people it will really make a difference for, but if it saves a few folk
some headaches it seems worth trying.
Thank you!
Mark
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-18 12:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11 17:48 [PATCH] platform/x86: thinkpad_acpi: check dytc version for lapmode sysfs Mark Pearson
2021-03-18 12:17 ` Hans de Goede
2021-03-18 12:43 ` [External] " Mark Pearson
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).