linux-kernel - Re: [PATCH 6/6] platform/x86/uniwill: Set cTGP support based on EC for TUXEDO IBP Gen7 MK1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2872e563-8973-d501-764c-920c18afd58d@linux.intel.com>
Date: Tue, 18 Nov 2025 12:43:02 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Werner Sembach <wse@...edocomputers.com>
cc: W_Armin@....de, Hans de Goede <hansg@...nel.org>, 
    platform-driver-x86@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 6/6] platform/x86/uniwill: Set cTGP support based on EC
 for TUXEDO IBP Gen7 MK1

On Mon, 17 Nov 2025, Werner Sembach wrote:

> The TUXEDO InfinityBook 14 Gen7 shares a boardname between the variant with
> and without an NVIDIA GPU.
> 
> Dynamically read which variant is present from EC to determine cTGP
> support.
> 
> Signed-off-by: Werner Sembach <wse@...edocomputers.com>
> ---
>  drivers/platform/x86/uniwill/uniwill-acpi.c | 22 ++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c b/drivers/platform/x86/uniwill/uniwill-acpi.c
> index 15a94c256f728..f6b24d2c28b89 100644
> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
> @@ -88,6 +88,9 @@
>  
>  #define EC_ADDR_GPU_TEMP		0x044F
>  
> +#define EC_ADDR_SYSTEM_ID		0x0456
> +#define HAS_GPU				BIT(7)
> +
>  #define EC_ADDR_MAIN_FAN_RPM_1		0x0464
>  
>  #define EC_ADDR_MAIN_FAN_RPM_2		0x0465
> @@ -1406,6 +1409,23 @@ static int uniwill_ec_init(struct uniwill_data *data)
>  	return devm_add_action_or_reset(data->dev, uniwill_disable_manual_control, data);
>  }
>  
> +static int quirk_ibp_gen7_ctgp_supported(const struct dmi_system_id *d)
> +{
> +	struct uniwill_data *data = container_of(d, struct uniwill_data, id);
> +	unsigned int value;
> +	int ret;
> +
> +	ret = regmap_read(data->regmap, EC_ADDR_SYSTEM_ID, &value);
> +	if (ret < 0)
> +		return ret;
> +	if (value & HAS_GPU) {
> +		data->id.driver_data =
> +			(void *)((unsigned long)data->id.driver_data |
> +				 UNIWILL_FEATURE_NVIDIA_CTGP_CONTROL);

Hi,

Mutating .driver_data like this is a no go. Please store information that 
isn't bound to the match properly, not with hacks like this.

There seems to be already struct uniwill_data where you can add things 
properly. If you want to use .driver_data as mutatable feature flags, you 
need to make a copy of it into a properly named variable (e.g. 
data->features). Then you can do
	data->features |= UNIWILL_FEATURE_NVIDIA_CTGP_CONTROL;
here.

> +	}
> +	return 0;
> +}
> +
>  static const struct dmi_system_id uniwill_dmi_table[] = {
>  	{
>  		.ident = "XMG FUSION 15",
> @@ -1462,12 +1482,12 @@ static const struct dmi_system_id uniwill_dmi_table[] = {
>  		.driver_data = (void *)(UNIWILL_FEATURE_NVIDIA_CTGP_CONTROL),
>  	},
>  	{
> +		.callback = quirk_ibp_gen7_ctgp_supported,
>  		.ident = "TUXEDO InfinityBook Pro 14/16 Gen7 Intel",
>  		.matches = {
>  			DMI_MATCH(DMI_SYS_VENDOR, "TUXEDO"),
>  			DMI_EXACT_MATCH(DMI_BOARD_NAME, "PHxARX1_PHxAQF1"),
>  		},
> -		.driver_data = (void *)(UNIWILL_FEATURE_NVIDIA_CTGP_CONTROL),

Please always order your series so you don't need to correct things you've 
just added.

-- 
 i.