On Fri, Nov 26, 2021 at 04:21:09PM +0100, Hans de Goede wrote:
> On some devices with a X-Powers AXP288 PMIC the LPAT tables in the ACPI
> node for the AXP288 PMIC for some reason only describe a small temperature
> range, e.g. 27° - 37° Celcius (assuming the entries are in millidegrees).
>
> When the tablet is idle in a room at 21° degrees this is causing values
> outside the LPAT table to be read, causing e.g. the following 2 errors
> to get spammed to the logs every 4 seconds! :
>
> [ 7512.791316] ACPI Error: AE_ERROR, Returned by Handler for [UserDefinedRegion] (20210930/evregion-281)
> [ 7512.791611] ACPI Error: Aborting method \_SB.SXP1._TMP due to previous error (AE_ERROR) (20210930/psparse-529)
>
> Fix this by clamping the raw value to the LPAT table range before
> passing it to acpi_lpat_raw_to_temp().
>
> Note clamping has been chosen rather then extrapolating because it is
> unknown how other parts of the ACPI tables will respond to temperature
> values outside of the LPAT range.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
One nit-pick below.
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> drivers/acpi/pmic/intel_pmic_xpower.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c
> index e844bc1f3df5..61bbe4c24d87 100644
> --- a/drivers/acpi/pmic/intel_pmic_xpower.c
> +++ b/drivers/acpi/pmic/intel_pmic_xpower.c
> @@ -293,12 +293,33 @@ static int intel_xpower_exec_mipi_pmic_seq_element(struct regmap *regmap,
> return ret;
> }
>
> +static int intel_xpower_lpat_raw_to_temp(struct acpi_lpat_conversion_table *lpat_table,
> + int raw)
> +{
> + struct acpi_lpat first = lpat_table->lpat[0];
> + struct acpi_lpat last = lpat_table->lpat[lpat_table->lpat_count - 1];
> +
> + /*
> + * Some LPAT tables in the ACPI Device for the AXP288 PMIC for some
> + * reason only describe a small temperature range, e.g. 27° - 37°
> + * Celcius. Resulting in errors when the tablet is idle in a cool room.
> + *
> + * To avoid these errors clamp the raw value to be inside the LPAT.
> + */
> + if (first.raw < last.raw)
Wondering what that would mean if this condition is false.
> + raw = clamp(raw, first.raw, last.raw);
> + else
> + raw = clamp(raw, last.raw, first.raw);
clamp_value() slightly better due to type checking.
> +
> + return acpi_lpat_raw_to_temp(lpat_table, raw);
> +}
> +
> static const struct intel_pmic_opregion_data intel_xpower_pmic_opregion_data = {
> .get_power = intel_xpower_pmic_get_power,
> .update_power = intel_xpower_pmic_update_power,
> .get_raw_temp = intel_xpower_pmic_get_raw_temp,
> .exec_mipi_pmic_seq_element = intel_xpower_exec_mipi_pmic_seq_element,
> - .lpat_raw_to_temp = acpi_lpat_raw_to_temp,
> + .lpat_raw_to_temp = intel_xpower_lpat_raw_to_temp,
> .power_table = power_table,
> .power_table_count = ARRAY_SIZE(power_table),
> .thermal_table = thermal_table,
> --
> 2.33.1
>
--
With Best Regards,
Andy Shevchenko
