On Fri, 9 Aug 2024, Armin Wolf wrote: > The LEGX0820 ACPI device is expected to provide a custom operation > region: > > OperationRegion (XIN1, 0x8F, Zero, 0x04B0) > Field (XIN1, AnyAcc, Lock, Preserve) > { > DMSG, 8, > HDAP, 8, > Offset (0x03), > AFNM, 8, > Offset (0x10), > P80B, 8, > P81B, 8, > P82B, 8, > P83B, 8, > P84B, 8, > P85B, 8, > P86B, 8, > P87B, 8, > Offset (0x20), > DTTM, 8, > TMP1, 8, > LTP1, 8, > HTP1, 8, > TMP2, 8, > LTP2, 8, > HTP2, 8, > Offset (0x3E8), > PMSG, 1600 > } > > The PMSG field is used by AML code to log debug messages when DMSG is > true. Since those debug messages are already logged using the standard > ACPI Debug object, we set DMSG unconditionally to 0x00 and ignore any > writes to PMSG. > > The TMPx, LTPx, HTPx and AFNM fields are used to inform the driver when > the temperature/(presumably) trip points/fan mode changes. This only > happens when the DTTM flag is set. > > Unfortunately we have to implement support for this operation region > because the AML codes uses code constructs like this one: > > If (((\_SB.XINI.PLAV != Zero) && (\_SB.XINI.DTTM != Zero))) > > The PLAV field gets set to 1 when the driver registers its address space > handler, so by default XIN1 should not be accessed. > > However ACPI does not use short-circuit evaluation when evaluating > logical conditions. This causes the DTTM field to be accessed even > when PLAV is 0, which results in an ACPI error. > Since this check happens inside various thermal-related ACPI control > methods, various thermal zone become unusable since any attempts to attempt > read their temperature results in an ACPI error. > > Fix this by providing support for this operation region. I suspect > that the problem does not happen under Windows (which seemingly does > not use short-circuit evaluation either) because the necessary driver > comes preinstalled with the machine. > > Tested-by: Chris <ghostwind@xxxxxxxxx> > Signed-off-by: Armin Wolf <W_Armin@xxxxxx> > --- > drivers/platform/x86/lg-laptop.c | 128 +++++++++++++++++++++++++++++++ > 1 file changed, 128 insertions(+) > > diff --git a/drivers/platform/x86/lg-laptop.c b/drivers/platform/x86/lg-laptop.c > index 9c7857842caf..6310a23f808c 100644 > --- a/drivers/platform/x86/lg-laptop.c > +++ b/drivers/platform/x86/lg-laptop.c > @@ -8,6 +8,8 @@ > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > #include <linux/acpi.h> > +#include <linux/device.h> > +#include <linux/dev_printk.h> > #include <linux/dmi.h> > #include <linux/input.h> > #include <linux/input/sparse-keymap.h> > @@ -31,6 +33,25 @@ MODULE_AUTHOR("Matan Ziv-Av"); > MODULE_DESCRIPTION("LG WMI Hotkey Driver"); > MODULE_LICENSE("GPL"); > > +static bool fw_debug; > +module_param(fw_debug, bool, 0); > +MODULE_PARM_DESC(fw_debug, "Enable printing of firmware debug messages"); > + > +#define LG_ADDRESS_SPACE_ID 0x8F > + > +#define LG_ADDRESS_SPACE_DEBUG_FLAG_ADR 0x00 > +#define LG_ADDRESS_SPACE_DEBUG_MSG_START_ADR 0x3E8 > +#define LG_ADDRESS_SPACE_DEBUG_MSG_END_ADR 0x5E8 > + > +#define LG_ADDRESS_SPACE_DTTM_FLAG_ADR 0x20 > +#define LG_ADDRESS_SPACE_FAN_MODE_ADR 0x03 Any particular reason why there are not in numerical order? > +#define LG_ADDRESS_SPACE_CPU_TEMP_ADR 0x21 > +#define LG_ADDRESS_SPACE_CPU_TRIP_LOW_ADR 0x22 > +#define LG_ADDRESS_SPACE_CPU_TRIP_HIGH_ADR 0x23 > +#define LG_ADDRESS_SPACE_MB_TEMP_ADR 0x24 > +#define LG_ADDRESS_SPACE_MB_TRIP_LOW_ADR 0x25 > +#define LG_ADDRESS_SPACE_MB_TRIP_HIGH_ADR 0x26 > + > #define WMI_EVENT_GUID0 "E4FB94F9-7F2B-4173-AD1A-CD1D95086248" > #define WMI_EVENT_GUID1 "023B133E-49D1-4E10-B313-698220140DC2" > #define WMI_EVENT_GUID2 "37BE1AC0-C3F2-4B1F-BFBE-8FDEAF2814D6" > @@ -646,6 +667,101 @@ static struct platform_driver pf_driver = { > } > }; > > +static acpi_status lg_laptop_address_space_write(struct device *dev, acpi_physical_address address, > + size_t size, u64 *value) Why is this write function taking u64 _pointer_? > +{ > + u8 byte; > + > + /* Ignore any debug messages */ > + if (address >= LG_ADDRESS_SPACE_DEBUG_MSG_START_ADR && > + address <= LG_ADDRESS_SPACE_DEBUG_MSG_END_ADR) > + return AE_OK; > + > + if (size != sizeof(byte)) > + return AE_BAD_PARAMETER; > + > + byte = *value & 0xFF; > + > + switch (address) { > + case LG_ADDRESS_SPACE_FAN_MODE_ADR: > + dev_dbg(dev, "Fan mode set to mode %u\n", byte); > + return AE_OK; > + case LG_ADDRESS_SPACE_CPU_TEMP_ADR: > + dev_dbg(dev, "CPU temperature is %u °C\n", byte); > + return AE_OK; > + case LG_ADDRESS_SPACE_CPU_TRIP_LOW_ADR: > + dev_dbg(dev, "CPU lower trip point set to %u °C\n", byte); > + return AE_OK; > + case LG_ADDRESS_SPACE_CPU_TRIP_HIGH_ADR: > + dev_dbg(dev, "CPU higher trip point set to %u °C\n", byte); > + return AE_OK; > + case LG_ADDRESS_SPACE_MB_TEMP_ADR: > + dev_dbg(dev, "Motherboard temperature is %u °C\n", byte); > + return AE_OK; > + case LG_ADDRESS_SPACE_MB_TRIP_LOW_ADR: > + dev_dbg(dev, "Motherboard lower trip point set to %u °C\n", byte); > + return AE_OK; > + case LG_ADDRESS_SPACE_MB_TRIP_HIGH_ADR: > + dev_dbg(dev, "Motherboard higher trip point set to %u °C\n", byte); > + return AE_OK; > + default: > + dev_notice_ratelimited(dev, "Ignoring write to unknown opregion address %llu\n", > + address); > + return AE_OK; > + } > +} > + > +static acpi_status lg_laptop_address_space_read(struct device *dev, acpi_physical_address address, > + size_t size, u64 *value) > +{ > + if (size != 1) > + return AE_BAD_PARAMETER; > + > + switch (address) { > + case LG_ADDRESS_SPACE_DEBUG_FLAG_ADR: > + /* Debug messages are already printed using the standard ACPI Debug object */ > + *value = 0x00; > + return AE_OK; > + case LG_ADDRESS_SPACE_DTTM_FLAG_ADR: > + *value = fw_debug; > + return AE_OK; > + default: > + dev_notice_ratelimited(dev, "Attempt to read unknown opregion address %llu\n", > + address); > + return AE_BAD_PARAMETER; > + } > +} > + > +static acpi_status lg_laptop_address_space_handler(u32 function, acpi_physical_address address, > + u32 bits, u64 *value, void *handler_context, > + void *region_context) > +{ > + struct device *dev = handler_context; > + size_t size; > + > + if (bits % 8) > + return AE_BAD_PARAMETER; > + > + size = bits / 8; BITS_PER_BYTE x2 -- i. > + > + switch (function) { > + case ACPI_READ: > + return lg_laptop_address_space_read(dev, address, size, value); > + case ACPI_WRITE: > + return lg_laptop_address_space_write(dev, address, size, value); > + default: > + return AE_BAD_PARAMETER; > + } > +} > + > +static void lg_laptop_remove_address_space_handler(void *data) > +{ > + struct acpi_device *device = data; > + > + acpi_remove_address_space_handler(device->handle, LG_ADDRESS_SPACE_ID, > + &lg_laptop_address_space_handler); > +} > + > static int acpi_add(struct acpi_device *device) > { > struct platform_device_info pdev_info = { > @@ -653,6 +769,7 @@ static int acpi_add(struct acpi_device *device) > .name = PLATFORM_NAME, > .id = PLATFORM_DEVID_NONE, > }; > + acpi_status status; > int ret; > const char *product; > int year = 2017; > @@ -660,6 +777,17 @@ static int acpi_add(struct acpi_device *device) > if (pf_device) > return 0; > > + status = acpi_install_address_space_handler(device->handle, LG_ADDRESS_SPACE_ID, > + &lg_laptop_address_space_handler, > + NULL, &device->dev); > + if (ACPI_FAILURE(status)) > + return -ENODEV; > + > + ret = devm_add_action_or_reset(&device->dev, lg_laptop_remove_address_space_handler, > + device); > + if (ret < 0) > + return ret; > + > ret = platform_driver_register(&pf_driver); > if (ret) > return ret; > -- > 2.39.2 >