diff mbox series

[v2] platform/x86: lg-laptop: Add operation region support

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

Commit Message

Armin Wolf Aug. 13, 2024, 2:29 a.m. UTC
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

Comments

Hans de Goede Aug. 19, 2024, 11:37 a.m. UTC | #1
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
>
Armin Wolf Aug. 26, 2024, 8:09 p.m. UTC | #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
>>
>
Hans de Goede Aug. 27, 2024, 8:33 a.m. UTC | #3
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
>>>
>>
>
Armin Wolf Aug. 27, 2024, 10:03 p.m. UTC | #4
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 mbox series

Patch

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;