Message ID | 20240809175211.5962-1-W_Armin@gmx.de (mailing list archive) |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | platform/x86: lg-laptop: Add operation region support | expand |
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@gmail.com> > Signed-off-by: Armin Wolf <W_Armin@gmx.de> > --- > 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
Am 12.08.24 um 14:19 schrieb Ilpo Järvinen: > 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@gmail.com> >> Signed-off-by: Armin Wolf <W_Armin@gmx.de> >> --- >> 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? Hi, i decided to group all fields connected to the DTTM flag together, but if you wish i can sort them 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_? Good point, i will change that. > >> +{ >> + 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 >
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 +#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) +{ + 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; + + 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;