2021-03-15 10:14:30

by Henning Schild

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

changes since v1:

- fixed lots of style issues found in v1
- (debug) printing
- header ordering
- fixed license issues GPLv2 and SPDX in all files
- module_platform_driver instead of __init __exit
- wdt simplifications cleanup
- lots of fixes in wdt driver, all that was found in v1
- fixed dmi length in dmi helper
- changed LED names to allowed ones
- move led driver to simple/
- switched pmc_atom to dmi callback with global variable

--

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.

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 | 3 +
drivers/leds/Makefile | 3 +
drivers/leds/simple/Kconfig | 11 +
drivers/leds/simple/Makefile | 2 +
drivers/leds/simple/simatic-ipc-leds.c | 210 +++++++++++++++++
drivers/platform/x86/Kconfig | 12 +
drivers/platform/x86/Makefile | 3 +
drivers/platform/x86/pmc_atom.c | 47 ++--
drivers/platform/x86/simatic-ipc.c | 168 ++++++++++++++
drivers/watchdog/Kconfig | 11 +
drivers/watchdog/Makefile | 1 +
drivers/watchdog/simatic-ipc-wdt.c | 215 ++++++++++++++++++
.../platform_data/x86/simatic-ipc-base.h | 29 +++
include/linux/platform_data/x86/simatic-ipc.h | 66 ++++++
14 files changed, 761 insertions(+), 20 deletions(-)
create mode 100644 drivers/leds/simple/Kconfig
create mode 100644 drivers/leds/simple/Makefile
create mode 100644 drivers/leds/simple/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-15 10:58:11

by Andy Shevchenko

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

On Mon, Mar 15, 2021 at 12:12 PM Henning Schild
<[email protected]> wrote:
>
> changes since v1:
>
> - fixed lots of style issues found in v1
> - (debug) printing
> - header ordering
> - fixed license issues GPLv2 and SPDX in all files
> - module_platform_driver instead of __init __exit
> - wdt simplifications cleanup
> - lots of fixes in wdt driver, all that was found in v1
> - fixed dmi length in dmi helper
> - changed LED names to allowed ones
> - move led driver to simple/
> - switched pmc_atom to dmi callback with global variable
>
> --
>
> 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.

Thanks for an update!

I did review more thoughtfully the series and realized that you can
avoid that P2SB thingy and may the approach be much cleaner if you
register the real GPIO driver and convert your LEDs to be a GPIO LEDs.
Then you won't need any custom code for it (except some board file, or
what would be better to file _DSD in your ACPI tables.

--
With Best Regards,
Andy Shevchenko

2021-03-15 18:36:53

by Henning Schild

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

Am Mon, 15 Mar 2021 12:55:13 +0200
schrieb Andy Shevchenko <[email protected]>:

> On Mon, Mar 15, 2021 at 12:12 PM Henning Schild
> <[email protected]> wrote:
> >
> > changes since v1:
> >
> > - fixed lots of style issues found in v1
> > - (debug) printing
> > - header ordering
> > - fixed license issues GPLv2 and SPDX in all files
> > - module_platform_driver instead of __init __exit
> > - wdt simplifications cleanup
> > - lots of fixes in wdt driver, all that was found in v1
> > - fixed dmi length in dmi helper
> > - changed LED names to allowed ones
> > - move led driver to simple/
> > - switched pmc_atom to dmi callback with global variable
> >
> > --
> >
> > 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.
>
> Thanks for an update!
>
> I did review more thoughtfully the series and realized that you can
> avoid that P2SB thingy and may the approach be much cleaner if you
> register the real GPIO driver and convert your LEDs to be a GPIO LEDs.
> Then you won't need any custom code for it (except some board file, or
> what would be better to file _DSD in your ACPI tables.

Thanks Andy. I will need to read through your comments and existing
code. Are you saying there already is a GPIO driver that i should
rather hook into ... given there is and will not be WDAT any time soon?
Can you please point it out to me, the driver and maybe an example.

If you are suggesting to write a generic GPIO driver, i would probably
say that this can hopefully wait until we have a second user and need
that level of abstraction.

regards,
Henning

> --
> With Best Regards,
> Andy Shevchenko

2021-08-02 09:43:29

by Henning Schild

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

Am Mon, 15 Mar 2021 12:55:13 +0200
schrieb Andy Shevchenko <[email protected]>:

> On Mon, Mar 15, 2021 at 12:12 PM Henning Schild
> <[email protected]> wrote:
> >
> > changes since v1:
> >
> > - fixed lots of style issues found in v1
> > - (debug) printing
> > - header ordering
> > - fixed license issues GPLv2 and SPDX in all files
> > - module_platform_driver instead of __init __exit
> > - wdt simplifications cleanup
> > - lots of fixes in wdt driver, all that was found in v1
> > - fixed dmi length in dmi helper
> > - changed LED names to allowed ones
> > - move led driver to simple/
> > - switched pmc_atom to dmi callback with global variable
> >
> > --
> >
> > 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.
>
> Thanks for an update!
>
> I did review more thoughtfully the series and realized that you can
> avoid that P2SB thingy and may the approach be much cleaner if you
> register the real GPIO driver and convert your LEDs to be a GPIO LEDs.
> Then you won't need any custom code for it (except some board file, or
> what would be better to file _DSD in your ACPI tables.

For the next generation of these machines i managed to involve the BIOS
guys. Goal would be to describe as much as possible in a generic and
standard way in ACPI, to reduce cost on driver dev and maint in the
long run. Hopefully across OSs.

The first thing we wanted to look into is LEDs. The way they can be
described for leds-gpio does not seem to be standard but at least seems
generic. At the same time we contemplated whether to model the LEDs
using the multicolor class.

One thing that seems to speak against using multicolor seems to be
missing ACPI "support", while regular LEDs can be described in ACPI, it
does not seem like multicolor can. Or did we miss something?

regards,
Henning

> --
> With Best Regards,
> Andy Shevchenko