Re: [PATCH v2 6/7] platform/x86: uniwill-laptop: Mark EC_ADDR_OEM_4 as volatile

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 5591 bytes --]

On Fri, 17 Apr 2026, Armin Wolf wrote:

> It turned out that EC_ADDR_OEM_4 also contains bits with a volatile
> nature. Mark the whole register as volatile to prepare for the usage
> of said bits. This also means that we now have to save/restore the
> touchpad toggle state ourself.
> 
> Reviewed-by: Werner Sembach <wse@tuxedocomputers.com>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/platform/x86/uniwill/uniwill-acpi.c | 70 ++++++++++++++++++---
>  1 file changed, 60 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
> index 8c00d762ab08..d4abcaf87e39 100644
> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
> @@ -343,6 +343,7 @@ struct uniwill_data {
>  	struct mutex battery_lock;	/* Protects the list of currently registered batteries */
>  	bool last_fn_lock_state;
>  	bool last_super_key_enable_state;
> +	bool last_touchpad_toggle_enable_state;
>  	struct mutex super_key_lock;	/* Protects the toggling of the super key lock state */
>  	struct list_head batteries;
>  	struct mutex led_lock;		/* Protects writes to the lightbar registers */
> @@ -598,6 +599,7 @@ static bool uniwill_volatile_reg(struct device *dev, unsigned int reg)
>  	case EC_ADDR_PWM_2:
>  	case EC_ADDR_TRIGGER:
>  	case EC_ADDR_SWITCH_STATUS:
> +	case EC_ADDR_OEM_4:
>  	case EC_ADDR_CHARGE_CTRL:
>  	case EC_ADDR_USB_C_POWER_PRIORITY:
>  		return true;
> @@ -747,11 +749,22 @@ static ssize_t super_key_enable_show(struct device *dev, struct device_attribute
>  
>  static DEVICE_ATTR_RW(super_key_enable);
>  
> +static int uniwill_write_touchpad_toggle_enable(struct uniwill_data *data, bool status)
> +{
> +	unsigned int value;
> +
> +	if (status)
> +		value = 0;
> +	else
> +		value = TOUCHPAD_TOGGLE_OFF;
> +
> +	return regmap_update_bits(data->regmap, EC_ADDR_OEM_4, TOUCHPAD_TOGGLE_OFF, value);
> +}
> +
>  static ssize_t touchpad_toggle_enable_store(struct device *dev, struct device_attribute *attr,
>  					    const char *buf, size_t count)
>  {
>  	struct uniwill_data *data = dev_get_drvdata(dev);
> -	unsigned int value;
>  	bool enable;
>  	int ret;
>  
> @@ -759,30 +772,39 @@ static ssize_t touchpad_toggle_enable_store(struct device *dev, struct device_at
>  	if (ret < 0)
>  		return ret;
>  
> -	if (enable)
> -		value = 0;
> -	else
> -		value = TOUCHPAD_TOGGLE_OFF;
> -
> -	ret = regmap_update_bits(data->regmap, EC_ADDR_OEM_4, TOUCHPAD_TOGGLE_OFF, value);
> +	ret = uniwill_write_touchpad_toggle_enable(data, enable);
>  	if (ret < 0)
>  		return ret;
>  
>  	return count;
>  }
>  
> +static int uniwill_read_touchpad_toggle_enable(struct uniwill_data *data, bool *status)
> +{
> +	unsigned int value;
> +	int ret;
> +
> +	ret = regmap_read(data->regmap, EC_ADDR_OEM_4, &value);
> +	if (ret < 0)
> +		return ret;
> +
> +	*status = !(value & TOUCHPAD_TOGGLE_OFF);
> +
> +	return 0;
> +}
> +
>  static ssize_t touchpad_toggle_enable_show(struct device *dev, struct device_attribute *attr,
>  					   char *buf)
>  {
>  	struct uniwill_data *data = dev_get_drvdata(dev);
> -	unsigned int value;
> +	bool status;
>  	int ret;
>  
> -	ret = regmap_read(data->regmap, EC_ADDR_OEM_4, &value);
> +	ret = uniwill_read_touchpad_toggle_enable(data, &status);
>  	if (ret < 0)
>  		return ret;
>  
> -	return sysfs_emit(buf, "%d\n", !(value & TOUCHPAD_TOGGLE_OFF));
> +	return sysfs_emit(buf, "%d\n", status);
>  }
>  
>  static DEVICE_ATTR_RW(touchpad_toggle_enable);
> @@ -1774,6 +1796,18 @@ static int uniwill_suspend_super_key(struct uniwill_data *data)
>  	return uniwill_read_super_key_enable(data, &data->last_super_key_enable_state);
>  }
>  
> +static int uniwill_suspend_touchpad_toggle(struct uniwill_data *data)
> +{
> +	if (!uniwill_device_supports(data, UNIWILL_FEATURE_TOUCHPAD_TOGGLE))
> +		return 0;
> +
> +	/*
> +	 * EC_ADDR_OEM_4 is marked as volatile, so we have to restore it
> +	 * ourselves.
> +	 */
> +	return uniwill_read_touchpad_toggle_enable(data, &data->last_touchpad_toggle_enable_state);
> +}
> +
>  static int uniwill_suspend_battery(struct uniwill_data *data)
>  {
>  	if (!uniwill_device_supports(data, UNIWILL_FEATURE_BATTERY))
> @@ -1809,6 +1843,10 @@ static int uniwill_suspend(struct device *dev)
>  	if (ret < 0)
>  		return ret;
>  
> +	ret = uniwill_suspend_touchpad_toggle(data);
> +	if (ret < 0)
> +		return ret;
> +
>  	ret = uniwill_suspend_battery(data);
>  	if (ret < 0)
>  		return ret;
> @@ -1839,6 +1877,14 @@ static int uniwill_resume_super_key(struct uniwill_data *data)
>  	return uniwill_write_super_key_enable(data, data->last_super_key_enable_state);
>  }
>  
> +static int uniwill_resume_touchpad_toggle(struct uniwill_data *data)
> +{
> +	if (!uniwill_device_supports(data, UNIWILL_FEATURE_TOUCHPAD_TOGGLE))
> +		return 0;
> +
> +	return uniwill_write_touchpad_toggle_enable(data, data->last_touchpad_toggle_enable_state);
> +}
> +
>  static int uniwill_resume_battery(struct uniwill_data *data)
>  {
>  	if (!uniwill_device_supports(data, UNIWILL_FEATURE_BATTERY))
> @@ -1884,6 +1930,10 @@ static int uniwill_resume(struct device *dev)
>  	if (ret < 0)
>  		return ret;
>  
> +	ret = uniwill_resume_touchpad_toggle(data);
> +	if (ret < 0)
> +		return ret;
> +
>  	ret = uniwill_resume_battery(data);
>  	if (ret < 0)
>  		return ret;
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.