| Message ID | 20240813022903.20567-1-W_Armin@gmx.de (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [v2] platform/x86: lg-laptop: Add operation region support | expand |
Hi, On 8/13/24 4:29 AM, 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 attempt to > 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> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > Changes since v1: > - attempts -> attempt > - sort defines in numerical order > - make lg_laptop_address_space_write() take a plain u64 > - use BITS_PER_BYTE > - manually check fw_debug when handling fan mode updates > --- > drivers/platform/x86/lg-laptop.c | 136 +++++++++++++++++++++++++++++++ > 1 file changed, 136 insertions(+) > > diff --git a/drivers/platform/x86/lg-laptop.c b/drivers/platform/x86/lg-laptop.c > index 9c7857842caf..55d31d4fefd6 100644 > --- a/drivers/platform/x86/lg-laptop.c > +++ b/drivers/platform/x86/lg-laptop.c > @@ -8,6 +8,9 @@ > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > #include <linux/acpi.h> > +#include <linux/bits.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 +34,26 @@ 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_FAN_MODE_ADR 0x03 > + > +#define LG_ADDRESS_SPACE_DTTM_FLAG_ADR 0x20 > +#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 LG_ADDRESS_SPACE_DEBUG_MSG_START_ADR 0x3E8 > +#define LG_ADDRESS_SPACE_DEBUG_MSG_END_ADR 0x5E8 > + > #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 +669,107 @@ 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: > + /* > + * The fan mode field is not affected by the DTTM flag, so we > + * have to manually check fw_debug. > + */ > + if (fw_debug) > + 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 % BITS_PER_BYTE) > + return AE_BAD_PARAMETER; > + > + size = bits / BITS_PER_BYTE; > + > + 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 +777,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 +785,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 >
Am 19.08.24 um 13:37 schrieb Hans de Goede: > Hi, > > On 8/13/24 4:29 AM, 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 attempt to >> 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> > Thanks, patch looks good to me: > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > > Regards, > > Hans > Any updates on this? I would prefer to have this merged for the upcoming 6.12 kernel since without this patch many LG notebooks have unusable thermal zones. Thanks, Armin Wolf > >> --- >> Changes since v1: >> - attempts -> attempt >> - sort defines in numerical order >> - make lg_laptop_address_space_write() take a plain u64 >> - use BITS_PER_BYTE >> - manually check fw_debug when handling fan mode updates >> --- >> drivers/platform/x86/lg-laptop.c | 136 +++++++++++++++++++++++++++++++ >> 1 file changed, 136 insertions(+) >> >> diff --git a/drivers/platform/x86/lg-laptop.c b/drivers/platform/x86/lg-laptop.c >> index 9c7857842caf..55d31d4fefd6 100644 >> --- a/drivers/platform/x86/lg-laptop.c >> +++ b/drivers/platform/x86/lg-laptop.c >> @@ -8,6 +8,9 @@ >> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >> >> #include <linux/acpi.h> >> +#include <linux/bits.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 +34,26 @@ 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_FAN_MODE_ADR 0x03 >> + >> +#define LG_ADDRESS_SPACE_DTTM_FLAG_ADR 0x20 >> +#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 LG_ADDRESS_SPACE_DEBUG_MSG_START_ADR 0x3E8 >> +#define LG_ADDRESS_SPACE_DEBUG_MSG_END_ADR 0x5E8 >> + >> #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 +669,107 @@ 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: >> + /* >> + * The fan mode field is not affected by the DTTM flag, so we >> + * have to manually check fw_debug. >> + */ >> + if (fw_debug) >> + 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 % BITS_PER_BYTE) >> + return AE_BAD_PARAMETER; >> + >> + size = bits / BITS_PER_BYTE; >> + >> + 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 +777,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 +785,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 >> >
Hi Armin, On 8/26/24 10:09 PM, Armin Wolf wrote: > Am 19.08.24 um 13:37 schrieb Hans de Goede: > >> Hi, >> >> On 8/13/24 4:29 AM, 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 attempt to >>> 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> >> Thanks, patch looks good to me: >> >> Reviewed-by: Hans de Goede <hdegoede@redhat.com> >> >> Regards, >> >> Hans >> > Any updates on this? I would prefer to have this merged for the upcoming 6.12 kernel since > without this patch many LG notebooks have unusable thermal zones. This patch has already been merged and already is in linux-next, but I see that I forgot to send my usual email confirming that I merged it, sorry. Regards, Hans >>> --- >>> Changes since v1: >>> - attempts -> attempt >>> - sort defines in numerical order >>> - make lg_laptop_address_space_write() take a plain u64 >>> - use BITS_PER_BYTE >>> - manually check fw_debug when handling fan mode updates >>> --- >>> drivers/platform/x86/lg-laptop.c | 136 +++++++++++++++++++++++++++++++ >>> 1 file changed, 136 insertions(+) >>> >>> diff --git a/drivers/platform/x86/lg-laptop.c b/drivers/platform/x86/lg-laptop.c >>> index 9c7857842caf..55d31d4fefd6 100644 >>> --- a/drivers/platform/x86/lg-laptop.c >>> +++ b/drivers/platform/x86/lg-laptop.c >>> @@ -8,6 +8,9 @@ >>> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >>> >>> #include <linux/acpi.h> >>> +#include <linux/bits.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 +34,26 @@ 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_FAN_MODE_ADR 0x03 >>> + >>> +#define LG_ADDRESS_SPACE_DTTM_FLAG_ADR 0x20 >>> +#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 LG_ADDRESS_SPACE_DEBUG_MSG_START_ADR 0x3E8 >>> +#define LG_ADDRESS_SPACE_DEBUG_MSG_END_ADR 0x5E8 >>> + >>> #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 +669,107 @@ 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: >>> + /* >>> + * The fan mode field is not affected by the DTTM flag, so we >>> + * have to manually check fw_debug. >>> + */ >>> + if (fw_debug) >>> + 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 % BITS_PER_BYTE) >>> + return AE_BAD_PARAMETER; >>> + >>> + size = bits / BITS_PER_BYTE; >>> + >>> + 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 +777,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 +785,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 >>> >> >
Am 27.08.24 um 10:33 schrieb Hans de Goede: > Hi Armin, > > On 8/26/24 10:09 PM, Armin Wolf wrote: >> Am 19.08.24 um 13:37 schrieb Hans de Goede: >> >>> Hi, >>> >>> On 8/13/24 4:29 AM, 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 attempt to >>>> 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> >>> Thanks, patch looks good to me: >>> >>> Reviewed-by: Hans de Goede <hdegoede@redhat.com> >>> >>> Regards, >>> >>> Hans >>> >> Any updates on this? I would prefer to have this merged for the upcoming 6.12 kernel since >> without this patch many LG notebooks have unusable thermal zones. > This patch has already been merged and already is in linux-next, > but I see that I forgot to send my usual email confirming that > I merged it, sorry. > > Regards, > > Hans Thank you :) > >>>> --- >>>> Changes since v1: >>>> - attempts -> attempt >>>> - sort defines in numerical order >>>> - make lg_laptop_address_space_write() take a plain u64 >>>> - use BITS_PER_BYTE >>>> - manually check fw_debug when handling fan mode updates >>>> --- >>>> drivers/platform/x86/lg-laptop.c | 136 +++++++++++++++++++++++++++++++ >>>> 1 file changed, 136 insertions(+) >>>> >>>> diff --git a/drivers/platform/x86/lg-laptop.c b/drivers/platform/x86/lg-laptop.c >>>> index 9c7857842caf..55d31d4fefd6 100644 >>>> --- a/drivers/platform/x86/lg-laptop.c >>>> +++ b/drivers/platform/x86/lg-laptop.c >>>> @@ -8,6 +8,9 @@ >>>> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >>>> >>>> #include <linux/acpi.h> >>>> +#include <linux/bits.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 +34,26 @@ 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_FAN_MODE_ADR 0x03 >>>> + >>>> +#define LG_ADDRESS_SPACE_DTTM_FLAG_ADR 0x20 >>>> +#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 LG_ADDRESS_SPACE_DEBUG_MSG_START_ADR 0x3E8 >>>> +#define LG_ADDRESS_SPACE_DEBUG_MSG_END_ADR 0x5E8 >>>> + >>>> #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 +669,107 @@ 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: >>>> + /* >>>> + * The fan mode field is not affected by the DTTM flag, so we >>>> + * have to manually check fw_debug. >>>> + */ >>>> + if (fw_debug) >>>> + 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 % BITS_PER_BYTE) >>>> + return AE_BAD_PARAMETER; >>>> + >>>> + size = bits / BITS_PER_BYTE; >>>> + >>>> + 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 +777,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 +785,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 >>>> >
diff --git a/drivers/platform/x86/lg-laptop.c b/drivers/platform/x86/lg-laptop.c index 9c7857842caf..55d31d4fefd6 100644 --- a/drivers/platform/x86/lg-laptop.c +++ b/drivers/platform/x86/lg-laptop.c @@ -8,6 +8,9 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include <linux/acpi.h> +#include <linux/bits.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 +34,26 @@ 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_FAN_MODE_ADR 0x03 + +#define LG_ADDRESS_SPACE_DTTM_FLAG_ADR 0x20 +#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 LG_ADDRESS_SPACE_DEBUG_MSG_START_ADR 0x3E8 +#define LG_ADDRESS_SPACE_DEBUG_MSG_END_ADR 0x5E8 + #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 +669,107 @@ 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: + /* + * The fan mode field is not affected by the DTTM flag, so we + * have to manually check fw_debug. + */ + if (fw_debug) + 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 % BITS_PER_BYTE) + return AE_BAD_PARAMETER; + + size = bits / BITS_PER_BYTE; + + 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 +777,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 +785,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;
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 attempt to 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> --- Changes since v1: - attempts -> attempt - sort defines in numerical order - make lg_laptop_address_space_write() take a plain u64 - use BITS_PER_BYTE - manually check fw_debug when handling fan mode updates --- drivers/platform/x86/lg-laptop.c | 136 +++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) -- 2.39.2