2021-03-04 20:27:33

by Henning Schild

[permalink] [raw]
Subject: [PATCH 0/4] add device drivers for Siemens Industrial PCs

From: Henning Schild <[email protected]>

This series adds support for watchdogs and leds of several x86 devices
from Siemens.

It is structured with a platform driver that mainly does identification
of the machines. It might trigger loading of the actual device drivers
by attaching devices to the platform bus.

The identification is vendor specific, parsing a special binary DMI
entry. The implementation of that platform identification is applied on
pmc_atom clock quirks in the final patch.

It is all structured in a way that we can easily add more devices and
more platform drivers later. Internally we have some more code for
hardware monitoring, more leds, watchdogs etc. This will follow some
day.

But the idea here is to share early, and hopefully not fail early.

Henning Schild (4):
platform/x86: simatic-ipc: add main driver for Siemens devices
leds: simatic-ipc-leds: add new driver for Siemens Industial PCs
watchdog: simatic-ipc-wdt: add new driver for Siemens Industrial PCs
platform/x86: pmc_atom: improve critclk_systems matching for Siemens
PCs

drivers/leds/Kconfig | 11 +
drivers/leds/Makefile | 1 +
drivers/leds/simatic-ipc-leds.c | 224 +++++++++++++
drivers/platform/x86/Kconfig | 9 +
drivers/platform/x86/Makefile | 3 +
drivers/platform/x86/pmc_atom.c | 39 +--
drivers/platform/x86/simatic-ipc.c | 166 ++++++++++
drivers/watchdog/Kconfig | 11 +
drivers/watchdog/Makefile | 1 +
drivers/watchdog/simatic-ipc-wdt.c | 305 ++++++++++++++++++
.../platform_data/x86/simatic-ipc-base.h | 33 ++
include/linux/platform_data/x86/simatic-ipc.h | 68 ++++
12 files changed, 853 insertions(+), 18 deletions(-)
create mode 100644 drivers/leds/simatic-ipc-leds.c
create mode 100644 drivers/platform/x86/simatic-ipc.c
create mode 100644 drivers/watchdog/simatic-ipc-wdt.c
create mode 100644 include/linux/platform_data/x86/simatic-ipc-base.h
create mode 100644 include/linux/platform_data/x86/simatic-ipc.h

--
2.26.2


2021-03-04 20:29:18

by Henning Schild

[permalink] [raw]
Subject: [PATCH 4/4] platform/x86: pmc_atom: improve critclk_systems matching for Siemens PCs

From: Henning Schild <[email protected]>

Siemens industrial PCs unfortunately can not always be properly
identified the way we used to. An earlier commit introduced code that
allows proper identification without looking at DMI strings that could
differ based on product branding.
Switch over to that proper way and revert commits that used to collect
the machines based on unstable strings.

Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
Fixes: e8796c6c69d1 ("platform/x86: pmc_atom: Add Siemens CONNECT ...")
Fixes: f110d252ae79 ("platform/x86: pmc_atom: Add Siemens SIMATIC ...")
Fixes: ad0d315b4d4e ("platform/x86: pmc_atom: Add Siemens SIMATIC ...")
Tested-by: Michael Haener <[email protected]>
Signed-off-by: Henning Schild <[email protected]>
---
drivers/platform/x86/pmc_atom.c | 39 ++++++++++++++++++---------------
1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index ca684ed760d1..03344f5502ad 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -13,6 +13,7 @@
#include <linux/io.h>
#include <linux/platform_data/x86/clk-pmc-atom.h>
#include <linux/platform_data/x86/pmc_atom.h>
+#include <linux/platform_data/x86/simatic-ipc.h>
#include <linux/platform_device.h>
#include <linux/pci.h>
#include <linux/seq_file.h>
@@ -424,30 +425,31 @@ static const struct dmi_system_id critclk_systems[] = {
},
},
{
- .ident = "SIMATIC IPC227E",
+ .ident = "SIEMENS AG",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
- DMI_MATCH(DMI_PRODUCT_VERSION, "6ES7647-8B"),
- },
- },
- {
- .ident = "SIMATIC IPC277E",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
- DMI_MATCH(DMI_PRODUCT_VERSION, "6AV7882-0"),
- },
- },
- {
- .ident = "CONNECT X300",
- .matches = {
- DMI_MATCH(DMI_SYS_VENDOR, "SIEMENS AG"),
- DMI_MATCH(DMI_PRODUCT_VERSION, "A5E45074588"),
},
},

{ /*sentinel*/ }
};

+static int pmc_clk_is_critical(const struct dmi_system_id *d)
+{
+ int ret = true;
+ u32 station_id;
+
+ if (!strcmp(d->ident, "SIEMENS AG")) {
+ if (dmi_walk(simatic_ipc_find_dmi_entry_helper, &station_id))
+ ret = false;
+ else
+ ret = (station_id == SIMATIC_IPC_IPC227E ||
+ station_id == SIMATIC_IPC_IPC277E);
+ }
+
+ return ret;
+}
+
static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
const struct pmc_data *pmc_data)
{
@@ -462,8 +464,9 @@ static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
clk_data->base = pmc_regmap; /* offset is added by client */
clk_data->clks = pmc_data->clks;
if (d) {
- clk_data->critical = true;
- pr_info("%s critclks quirk enabled\n", d->ident);
+ clk_data->critical = pmc_clk_is_critical(d);
+ if (clk_data->critical)
+ pr_info("%s critclks quirk enabled\n", d->ident);
}

clkdev = platform_device_register_data(&pdev->dev, "clk-pmc-atom",
--
2.26.2

2021-03-05 00:09:40

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 4/4] platform/x86: pmc_atom: improve critclk_systems matching for Siemens PCs

On Thu, Mar 4, 2021 at 9:27 AM Henning Schild
<[email protected]> wrote:

> Siemens industrial PCs unfortunately can not always be properly
> identified the way we used to. An earlier commit introduced code that
> allows proper identification without looking at DMI strings that could
> differ based on product branding.
> Switch over to that proper way and revert commits that used to collect
> the machines based on unstable strings.

> +#include <linux/platform_data/x86/simatic-ipc.h>

> +static int pmc_clk_is_critical(const struct dmi_system_id *d)
> +{
> + int ret = true;
> + u32 station_id;
> +
> + if (!strcmp(d->ident, "SIEMENS AG")) {
> + if (dmi_walk(simatic_ipc_find_dmi_entry_helper, &station_id))
> + ret = false;
> + else
> + ret = (station_id == SIMATIC_IPC_IPC227E ||
> + station_id == SIMATIC_IPC_IPC277E);
> + }
> +
> + return ret;

Much easier to rewrite it as

if (strcmp(...)) // BTW, do we have a dmi_* helper for that?
return true;

if (dmi_walk)
return false;

return station_id == || ...;

> +}

Maybe instead you can rewrite it as a callback in DMI table which
changes a (global, yeah) variable that you simply reassign...



> if (d) {
> - clk_data->critical = true;
> - pr_info("%s critclks quirk enabled\n", d->ident);
> + clk_data->critical = pmc_clk_is_critical(d);
> + if (clk_data->critical)
> + pr_info("%s critclks quirk enabled\n", d->ident);
> }

...somewhere here?

Like
clk_data->critical = global_var;
if (...)
pr_info();

It seems it will reduce burden on a callback by dropping strcmp() call.

--
With Best Regards,
Andy Shevchenko