[PATCH 0/2] platform/x86: simatic-ipc: add another model and hwmon module loading

[PATCH 0/2] platform/x86: simatic-ipc: add another model and hwmon module loading

[Henning Schild]

The second patch loads modules for hwmon support, should they be
available. That will save users the need to detect and manually load
those modules after a machine has been clearly identified by its Siemens
Simatic IPC station id.

The first patch just adds a device that is pretty similar to another one
we already had here.

Henning Schild (2):
  platform/x86: simatic-ipc: add another model
  platform/x86: simatic-ipc: add auto-loading of hwmon modules

 drivers/platform/x86/simatic-ipc.c            | 19 +++++++++++++++++++
 include/linux/platform_data/x86/simatic-ipc.h |  1 +
 2 files changed, 20 insertions(+)

-- 
2.41.0

[PATCH 1/2] platform/x86: simatic-ipc: add another model

[Henning Schild]

This is the panel variant of a device we already did have. All the same,
just no LEDs.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 drivers/platform/x86/simatic-ipc.c            | 2 ++
 include/linux/platform_data/x86/simatic-ipc.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/platform/x86/simatic-ipc.c b/drivers/platform/x86/simatic-ipc.c
index 6d2c94c17414..71487216d33f 100644
--- a/drivers/platform/x86/simatic-ipc.c
+++ b/drivers/platform/x86/simatic-ipc.c
@@ -48,6 +48,8 @@ static struct {
 		SIMATIC_IPC_DEVICE_427E, SIMATIC_IPC_DEVICE_227E, SIMATIC_IPC_DEVICE_227E},
 	{SIMATIC_IPC_IPC227G,
 		SIMATIC_IPC_DEVICE_227G, SIMATIC_IPC_DEVICE_227G, SIMATIC_IPC_DEVICE_227G},
+	{SIMATIC_IPC_IPC277G,
+		SIMATIC_IPC_DEVICE_NONE, SIMATIC_IPC_DEVICE_227G, SIMATIC_IPC_DEVICE_227G},
 	{SIMATIC_IPC_IPC277E,
 		SIMATIC_IPC_DEVICE_NONE, SIMATIC_IPC_DEVICE_227E, SIMATIC_IPC_DEVICE_227E},
 	{SIMATIC_IPC_IPC427D,
diff --git a/include/linux/platform_data/x86/simatic-ipc.h b/include/linux/platform_data/x86/simatic-ipc.h
index 1a8e4c1099e3..f2eafa43a605 100644
--- a/include/linux/platform_data/x86/simatic-ipc.h
+++ b/include/linux/platform_data/x86/simatic-ipc.h
@@ -32,6 +32,7 @@ enum simatic_ipc_station_ids {
 	SIMATIC_IPC_IPC477E = 0x00000A02,
 	SIMATIC_IPC_IPC127E = 0x00000D01,
 	SIMATIC_IPC_IPC227G = 0x00000F01,
+	SIMATIC_IPC_IPC277G = 0x00000F02,
 	SIMATIC_IPC_IPCBX_39A = 0x00001001,
 	SIMATIC_IPC_IPCPX_39A = 0x00001002,
 	SIMATIC_IPC_IPCBX_21A = 0x00001101,
-- 
2.41.0

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

[Henning Schild]

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@siemens.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;
+	}
+}
+
 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);
 }
 
-- 
2.41.0

Re: [PATCH 0/2] platform/x86: simatic-ipc: add another model and hwmon module loading

[Henning Schild]

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

> The second patch loads modules for hwmon support, should they be
> available. That will save users the need to detect and manually load
> those modules after a machine has been clearly identified by its
> Siemens Simatic IPC station id.
> 
> The first patch just adds a device that is pretty similar to another
> one we already had here.

Note that this is based on
"platform/x86: add CMOS battery monitoring for simatic  IPCs"

Henning

> Henning Schild (2):
>   platform/x86: simatic-ipc: add another model
>   platform/x86: simatic-ipc: add auto-loading of hwmon modules
> 
>  drivers/platform/x86/simatic-ipc.c            | 19
> +++++++++++++++++++ include/linux/platform_data/x86/simatic-ipc.h |
> 1 + 2 files changed, 20 insertions(+)
> 

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

[Henning Schild]

Am Tue, 11 Jul 2023 14:08:42 +0200
schrieb Henning Schild <henning.schild@siemens.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@siemens.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);
>  }
>  

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

[Hans de Goede]

Hi,

On 7/12/23 09:21, Henning Schild wrote:
> Am Tue, 11 Jul 2023 14:08:42 +0200
> schrieb Henning Schild <henning.schild@siemens.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@siemens.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.

This sounds like a good plan to me, no objections from my side.

Regards,

Hans




>> +	}
>> +}
>> +
>>  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);
>>  }
>>  
>