| Message ID | 20211126141027.16161-1-henning.schild@siemens.com |
|---|---|
| Headers | show |
| Series | add device drivers for Siemens Industrial PCs | expand |
Hi all, in p3 not too much was left open so i hope this might be the last and might be quick. The two points that have been open where: 1 wish to split wdt driver into two 2 wish to use pinctrl for LEDs/WDT 1 was ignored for now. We can split later when we add more devices. It remains unclear if splitting is the way to go when more devices come. Could also be that it would become too many and spreading code over many files could make maint harder. Plus a split of wdt alone would make things inconsistent with LEDs. And if we did split that we would end up with even more files/drivers. 2 can not be done because pinctrl does not probe on any of the PCs. There are patches floating around but still under review/construction. There is no technical conflict it is more an ordering issue. So i kindly ask to let these drivers in and not have them stuck behind the P2SB patches. I once offered to wait, but its been many months. regards, Henning Am Fri, 26 Nov 2021 15:10:23 +0100 schrieb Henning Schild <henning.schild@siemens.com>: > changes since v3: > > - fix io access width and region reservations > - fix style in p1 > > changes since v2: > > - remove "simatic-ipc" prefix from LED names > - fix style issues found in v2, mainly LED driver > - fix OEM specific dmi code, and remove magic numbers > - more "simatic_ipc" name prefixing > - improved pmc quirk code using callbacks > > 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 | 202 ++++++++++++++++ > drivers/platform/x86/Kconfig | 12 + > drivers/platform/x86/Makefile | 3 + > drivers/platform/x86/pmc_atom.c | 54 +++-- > drivers/platform/x86/simatic-ipc.c | 168 +++++++++++++ > drivers/watchdog/Kconfig | 11 + > drivers/watchdog/Makefile | 1 + > drivers/watchdog/simatic-ipc-wdt.c | 228 > ++++++++++++++++++ .../platform_data/x86/simatic-ipc-base.h | > 29 +++ include/linux/platform_data/x86/simatic-ipc.h | 72 ++++++ > 14 files changed, 778 insertions(+), 21 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 >
On 11/26/21 7:34 AM, Henning Schild wrote: > Hi all, > > in p3 not too much was left open so i hope this might be the last and > might be quick. > > The two points that have been open where: > 1 wish to split wdt driver into two > 2 wish to use pinctrl for LEDs/WDT > > 1 was ignored for now. We can split later when we add more devices. It > remains unclear if splitting is the way to go when more devices come. The code is already quite messy, in part because memory regions are declared locally and not passed through the parent device as they should. I don't see how splitting the driver into multiple drivers would improve the situation. The platform code claims to be inspired by the lpc_ich driver. Maybe it should take a real example from that and pass version or variant specific details through platform data instead of maintaining it in the watchdog driver. Guenter
Hi all, to me it currently looks like a v5 will only differ in terms of documentation and could be acceptable to all people who have raised concerns on v4. Will send that v5 soon. And invite anyone to please raise further concerns, or review again. regards, Henning Am Fri, 26 Nov 2021 15:10:23 +0100 schrieb Henning Schild <henning.schild@siemens.com>: > changes since v3: > > - fix io access width and region reservations > - fix style in p1 > > changes since v2: > > - remove "simatic-ipc" prefix from LED names > - fix style issues found in v2, mainly LED driver > - fix OEM specific dmi code, and remove magic numbers > - more "simatic_ipc" name prefixing > - improved pmc quirk code using callbacks > > 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 | 202 ++++++++++++++++ > drivers/platform/x86/Kconfig | 12 + > drivers/platform/x86/Makefile | 3 + > drivers/platform/x86/pmc_atom.c | 54 +++-- > drivers/platform/x86/simatic-ipc.c | 168 +++++++++++++ > drivers/watchdog/Kconfig | 11 + > drivers/watchdog/Makefile | 1 + > drivers/watchdog/simatic-ipc-wdt.c | 228 > ++++++++++++++++++ .../platform_data/x86/simatic-ipc-base.h | > 29 +++ include/linux/platform_data/x86/simatic-ipc.h | 72 ++++++ > 14 files changed, 778 insertions(+), 21 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 >
changes since v3: - fix io access width and region reservations - fix style in p1 changes since v2: - remove "simatic-ipc" prefix from LED names - fix style issues found in v2, mainly LED driver - fix OEM specific dmi code, and remove magic numbers - more "simatic_ipc" name prefixing - improved pmc quirk code using callbacks 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 | 202 ++++++++++++++++ drivers/platform/x86/Kconfig | 12 + drivers/platform/x86/Makefile | 3 + drivers/platform/x86/pmc_atom.c | 54 +++-- drivers/platform/x86/simatic-ipc.c | 168 +++++++++++++ drivers/watchdog/Kconfig | 11 + drivers/watchdog/Makefile | 1 + drivers/watchdog/simatic-ipc-wdt.c | 228 ++++++++++++++++++ .../platform_data/x86/simatic-ipc-base.h | 29 +++ include/linux/platform_data/x86/simatic-ipc.h | 72 ++++++ 14 files changed, 778 insertions(+), 21 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