| Message ID | 20210311174843.3161-1-markpearson@lenovo.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | platform/x86: thinkpad_acpi: check dytc version for lapmode sysfs | expand |
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; > } >
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
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; }
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(-)