Hi, On 4/7/21 11:20 PM, Mark Pearson wrote: > On recent Thinkpad platforms it was reported that temp sensor 11 was > always incorrectly displaying 66C. It turns out the reason for this is > that this location in EC RAM is not a temperature sensor but is the > power supply ID (offset 0xC2). > > Based on feedback from the Lenovo firmware team the EC RAM version can > be determined and for the current version (3) only the 0x78 to 0x7F > range is used for temp sensors. I don't have any details for earlier > versions so I have left the implementation unaltered there. > > Note - in this block only 0x78 and 0x79 are officially designated (CPU & > GPU sensors). The use of the other locations in the block will vary from > platform to platform; but the existing logic to detect a sensor presence > holds. > > Signed-off-by: Mark Pearson <markpearson@xxxxxxxxxx> I've merged this, note I've added one small fixup to initialize ver to 0 when it is declared, also see a remark inline below. 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. > --- > drivers/platform/x86/thinkpad_acpi.c | 31 ++++++++++++++++++++-------- > 1 file changed, 22 insertions(+), 9 deletions(-) > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index 07de21941..8fbe4d3d9 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -6260,6 +6260,7 @@ enum thermal_access_mode { > enum { /* TPACPI_THERMAL_TPEC_* */ > TP_EC_THERMAL_TMP0 = 0x78, /* ACPI EC regs TMP 0..7 */ > TP_EC_THERMAL_TMP8 = 0xC0, /* ACPI EC regs TMP 8..15 */ > + TP_EC_FUNCREV = 0xEF, /* ACPI EC Functional revision */ > TP_EC_THERMAL_TMP_NA = -128, /* ACPI EC sensor not available */ > > TPACPI_THERMAL_SENSOR_NA = -128000, /* Sensor not available */ > @@ -6458,7 +6459,7 @@ static const struct attribute_group thermal_temp_input8_group = { > > static int __init thermal_init(struct ibm_init_struct *iibm) > { > - u8 t, ta1, ta2; > + u8 t, ta1, ta2, ver; > int i; > int acpi_tmp7; > int res; > @@ -6473,7 +6474,14 @@ static int __init thermal_init(struct ibm_init_struct *iibm) > * 0x78-0x7F, 0xC0-0xC7. Registers return 0x00 for > * non-implemented, thermal sensors return 0x80 when > * not available > + * The above rule is unfortunately flawed. This has been seen with > + * 0xC2 (power supply ID) causing thermal control problems. > + * The EC version can be determined by offset 0xEF and at least for > + * version 3 the Lenovo firmware team confirmed that registers 0xC0-0xC7 > + * are not thermal registers. > */ > + if (!acpi_ec_read(TP_EC_FUNCREV, &ver)) > + pr_warn("Thinkpad ACPI EC unable to access EC version\n"); So what happens with ver in case the pr_warn here triggers? As said above I've added a small change to initialize ver to 0 when declared, so that we keep the old behavior when this pr_warn triggers. > > ta1 = ta2 = 0; > for (i = 0; i < 8; i++) { > @@ -6483,11 +6491,13 @@ static int __init thermal_init(struct ibm_init_struct *iibm) > ta1 = 0; > break; > } > - if (acpi_ec_read(TP_EC_THERMAL_TMP8 + i, &t)) { > - ta2 |= t; > - } else { > - ta1 = 0; > - break; > + if (ver < 3) { > + if (acpi_ec_read(TP_EC_THERMAL_TMP8 + i, &t)) { > + ta2 |= t; > + } else { > + ta1 = 0; > + break; > + } > } > } > if (ta1 == 0) { > @@ -6500,9 +6510,12 @@ static int __init thermal_init(struct ibm_init_struct *iibm) > thermal_read_mode = TPACPI_THERMAL_NONE; > } > } else { > - thermal_read_mode = > - (ta2 != 0) ? > - TPACPI_THERMAL_TPEC_16 : TPACPI_THERMAL_TPEC_8; > + if (ver >= 3) > + thermal_read_mode = TPACPI_THERMAL_TPEC_8; > + else > + thermal_read_mode = > + (ta2 != 0) ? > + TPACPI_THERMAL_TPEC_16 : TPACPI_THERMAL_TPEC_8; > } > } else if (acpi_tmp7) { > if (tpacpi_is_ibm() && > Regards, Hans
