linux-kernel - Re: [PATCH 2/2] platform/x86: simatic-ipc: add auto-loading of hwmon modules

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: <20230712092147.061009b2@md1za8fc.ad001.siemens.net>
Date:   Wed, 12 Jul 2023 09:21:47 +0200
From:   Henning Schild <henning.schild@...mens.com>
To:     Hans de Goede <hdegoede@...hat.com>,
        Mark Gross <markgross@...nel.org>,
        <platform-driver-x86@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Cc:     Tobias Schaffner <tobias.schaffner@...mens.com>,
        Gerd Haeussler <gerd.haeussler.ext@...mens.com>
Subject: Re: [PATCH 2/2] platform/x86: simatic-ipc: add auto-loading of
 hwmon modules

Am Tue, 11 Jul 2023 14:08:42 +0200
schrieb Henning Schild <henning.schild@...mens.com>:

> In order to know which hwmon modules to load one would have to usually
> first probe from user-land i.e. with sensors-detect and create a
> config for each machine. But here we know exactly what machines we
> are dealing with, so we can request those howmon modules without
> user-mode detection and config files.
> 
> Signed-off-by: Henning Schild <henning.schild@...mens.com>
> ---
>  drivers/platform/x86/simatic-ipc.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/platform/x86/simatic-ipc.c
> b/drivers/platform/x86/simatic-ipc.c index 71487216d33f..403dc231bef7
> 100644 --- a/drivers/platform/x86/simatic-ipc.c
> +++ b/drivers/platform/x86/simatic-ipc.c
> @@ -153,6 +153,21 @@ static int register_platform_devices(u32
> station_id) return 0;
>  }
>  
> +static void request_additional_modules(u32 station_id)
> +{
> +	switch (station_id) {
> +	case SIMATIC_IPC_IPC227G:
> +	case SIMATIC_IPC_IPC277G:
> +	case SIMATIC_IPC_IPCBX_39A:
> +	case SIMATIC_IPC_IPCPX_39A:
> +		request_module("nct6775");
> +		break;
> +	default:
> +		request_module("emc1403");
> +		break;

This one will be hard to maintain since every new model would choose
the default path. Requesting emc1403 on a device where that would not
do anything is not a problem, but still. And people might forget to
even look at this and maybe name a module that should be used instead.

I will send a v2 where an array of module names will become part of
device_modes. The array would hold all additional modules which do not
autoload. Then the module w83627hf_wdt used for some models can also be
part of that and no longer be modeled with wdtmode.

Should anyone have objections on the whole idea of requesting additional
modules, please already speak up.

Henning

> +	}
> +}
> +
>  static int __init simatic_ipc_init_module(void)
>  {
>  	const struct dmi_system_id *match;
> @@ -170,6 +185,8 @@ static int __init simatic_ipc_init_module(void)
>  		return 0;
>  	}
>  
> +	request_additional_modules(station_id);
> +
>  	return register_platform_devices(station_id);
>  }
>