| Message ID | 20211213120502.20661-3-henning.schild@siemens.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | add device drivers for Siemens Industrial PCs | expand |
On Mon 2021-12-13 13:05:00, Henning Schild wrote: > This driver adds initial support for several devices from Siemens. It is > based on a platform driver introduced in an earlier commit. > > One of the supported machines has GPIO connected LEDs, here we poke GPIO > memory directly because pinctrl does not come up. > > Signed-off-by: Henning Schild <henning.schild@siemens.com> Acked-by: Pavel Machek <pavel@ucw.cz> > index c636ec069612..1a719caf14c0 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -105,3 +105,6 @@ obj-$(CONFIG_LEDS_TRIGGERS) += trigger/ > > # LED Blink > obj-y += blink/ > + > +# Simple LED drivers > +obj-y += simple/ > diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig > new file mode 100644 > index 000000000000..9f6a68336659 > --- /dev/null > +++ b/drivers/leds/simple/Kconfig > @@ -0,0 +1,11 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +config LEDS_SIEMENS_SIMATIC_IPC > + tristate "LED driver for Siemens Simatic IPCs" > + depends on LEDS_CLASS > + depends on SIEMENS_SIMATIC_IPC > + help > + This option enables support for the LEDs of several Industrial PCs > + from Siemens. > + > + To compile this driver as a module, choose M here: the module > + will be called simatic-ipc-leds. > diff --git a/drivers/leds/simple/Makefile b/drivers/leds/simple/Makefile > new file mode 100644 > index 000000000000..8481f1e9e360 > --- /dev/null > +++ b/drivers/leds/simple/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0 > +obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC) += simatic-ipc-leds.o > diff --git a/drivers/leds/simple/simatic-ipc-leds.c b/drivers/leds/simple/simatic-ipc-leds.c > new file mode 100644 > index 000000000000..ff2c96e73241 > --- /dev/null > +++ b/drivers/leds/simple/simatic-ipc-leds.c > @@ -0,0 +1,202 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Siemens SIMATIC IPC driver for LEDs > + * > + * Copyright (c) Siemens AG, 2018-2021 > + * > + * Authors: > + * Henning Schild <henning.schild@siemens.com> > + * Jan Kiszka <jan.kiszka@siemens.com> > + * Gerd Haeussler <gerd.haeussler.ext@siemens.com> > + */ > + > +#include <linux/ioport.h> > +#include <linux/kernel.h> > +#include <linux/leds.h> > +#include <linux/module.h> > +#include <linux/pci.h> > +#include <linux/platform_data/x86/simatic-ipc-base.h> > +#include <linux/platform_device.h> > +#include <linux/sizes.h> > +#include <linux/spinlock.h> > + > +#define SIMATIC_IPC_LED_PORT_BASE 0x404E > + > +struct simatic_ipc_led { > + unsigned int value; /* mask for io and offset for mem */ > + char *name; > + struct led_classdev cdev; > +}; > + > +static struct simatic_ipc_led simatic_ipc_leds_io[] = { > + {1 << 15, "green:" LED_FUNCTION_STATUS "-1" }, > + {1 << 7, "yellow:" LED_FUNCTION_STATUS "-1" }, > + {1 << 14, "red:" LED_FUNCTION_STATUS "-2" }, > + {1 << 6, "yellow:" LED_FUNCTION_STATUS "-2" }, > + {1 << 13, "red:" LED_FUNCTION_STATUS "-3" }, > + {1 << 5, "yellow:" LED_FUNCTION_STATUS "-3" }, > + { } > +}; > + > +/* the actual start will be discovered with PCI, 0 is a placeholder */ > +struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, SZ_4K, KBUILD_MODNAME); > + > +static void *simatic_ipc_led_memory; > + > +static struct simatic_ipc_led simatic_ipc_leds_mem[] = { > + {0x500 + 0x1A0, "red:" LED_FUNCTION_STATUS "-1"}, > + {0x500 + 0x1A8, "green:" LED_FUNCTION_STATUS "-1"}, > + {0x500 + 0x1C8, "red:" LED_FUNCTION_STATUS "-2"}, > + {0x500 + 0x1D0, "green:" LED_FUNCTION_STATUS "-2"}, > + {0x500 + 0x1E0, "red:" LED_FUNCTION_STATUS "-3"}, > + {0x500 + 0x198, "green:" LED_FUNCTION_STATUS "-3"}, > + { } > +}; Would it be possible to get some better naming for leds? status-1 to status-3 is not quite useful. Best regards, Pavel
Hi, On 12/15/21 21:18, Pavel Machek wrote: > On Mon 2021-12-13 13:05:00, Henning Schild wrote: >> This driver adds initial support for several devices from Siemens. It is >> based on a platform driver introduced in an earlier commit. >> >> One of the supported machines has GPIO connected LEDs, here we poke GPIO >> memory directly because pinctrl does not come up. >> >> Signed-off-by: Henning Schild <henning.schild@siemens.com> > > Acked-by: Pavel Machek <pavel@ucw.cz> I see that this patch #includes linux/platform_data/x86/simatic-ipc-base.h which gets added by patch 1/4. Pavel, can I take this patch upstream through the pdx86 tree (with you Ack added)? Or shall I prepare an immutable branch with patch 1 for you to merge ? Regards, Hans > >> index c636ec069612..1a719caf14c0 100644 >> --- a/drivers/leds/Makefile >> +++ b/drivers/leds/Makefile >> @@ -105,3 +105,6 @@ obj-$(CONFIG_LEDS_TRIGGERS) += trigger/ >> >> # LED Blink >> obj-y += blink/ >> + >> +# Simple LED drivers >> +obj-y += simple/ >> diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig >> new file mode 100644 >> index 000000000000..9f6a68336659 >> --- /dev/null >> +++ b/drivers/leds/simple/Kconfig >> @@ -0,0 +1,11 @@ >> +# SPDX-License-Identifier: GPL-2.0-only >> +config LEDS_SIEMENS_SIMATIC_IPC >> + tristate "LED driver for Siemens Simatic IPCs" >> + depends on LEDS_CLASS >> + depends on SIEMENS_SIMATIC_IPC >> + help >> + This option enables support for the LEDs of several Industrial PCs >> + from Siemens. >> + >> + To compile this driver as a module, choose M here: the module >> + will be called simatic-ipc-leds. >> diff --git a/drivers/leds/simple/Makefile b/drivers/leds/simple/Makefile >> new file mode 100644 >> index 000000000000..8481f1e9e360 >> --- /dev/null >> +++ b/drivers/leds/simple/Makefile >> @@ -0,0 +1,2 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC) += simatic-ipc-leds.o >> diff --git a/drivers/leds/simple/simatic-ipc-leds.c b/drivers/leds/simple/simatic-ipc-leds.c >> new file mode 100644 >> index 000000000000..ff2c96e73241 >> --- /dev/null >> +++ b/drivers/leds/simple/simatic-ipc-leds.c >> @@ -0,0 +1,202 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Siemens SIMATIC IPC driver for LEDs >> + * >> + * Copyright (c) Siemens AG, 2018-2021 >> + * >> + * Authors: >> + * Henning Schild <henning.schild@siemens.com> >> + * Jan Kiszka <jan.kiszka@siemens.com> >> + * Gerd Haeussler <gerd.haeussler.ext@siemens.com> >> + */ >> + >> +#include <linux/ioport.h> >> +#include <linux/kernel.h> >> +#include <linux/leds.h> >> +#include <linux/module.h> >> +#include <linux/pci.h> >> +#include <linux/platform_data/x86/simatic-ipc-base.h> >> +#include <linux/platform_device.h> >> +#include <linux/sizes.h> >> +#include <linux/spinlock.h> >> + >> +#define SIMATIC_IPC_LED_PORT_BASE 0x404E >> + >> +struct simatic_ipc_led { >> + unsigned int value; /* mask for io and offset for mem */ >> + char *name; >> + struct led_classdev cdev; >> +}; >> + >> +static struct simatic_ipc_led simatic_ipc_leds_io[] = { >> + {1 << 15, "green:" LED_FUNCTION_STATUS "-1" }, >> + {1 << 7, "yellow:" LED_FUNCTION_STATUS "-1" }, >> + {1 << 14, "red:" LED_FUNCTION_STATUS "-2" }, >> + {1 << 6, "yellow:" LED_FUNCTION_STATUS "-2" }, >> + {1 << 13, "red:" LED_FUNCTION_STATUS "-3" }, >> + {1 << 5, "yellow:" LED_FUNCTION_STATUS "-3" }, >> + { } >> +}; >> + >> +/* the actual start will be discovered with PCI, 0 is a placeholder */ >> +struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, SZ_4K, KBUILD_MODNAME); >> + >> +static void *simatic_ipc_led_memory; >> + >> +static struct simatic_ipc_led simatic_ipc_leds_mem[] = { >> + {0x500 + 0x1A0, "red:" LED_FUNCTION_STATUS "-1"}, >> + {0x500 + 0x1A8, "green:" LED_FUNCTION_STATUS "-1"}, >> + {0x500 + 0x1C8, "red:" LED_FUNCTION_STATUS "-2"}, >> + {0x500 + 0x1D0, "green:" LED_FUNCTION_STATUS "-2"}, >> + {0x500 + 0x1E0, "red:" LED_FUNCTION_STATUS "-3"}, >> + {0x500 + 0x198, "green:" LED_FUNCTION_STATUS "-3"}, >> + { } >> +}; > > Would it be possible to get some better naming for leds? status-1 to > status-3 is not quite useful. > > Best regards, > Pavel >
On Wed 2021-12-15 21:53:56, Hans de Goede wrote: > Hi, > > On 12/15/21 21:18, Pavel Machek wrote: > > On Mon 2021-12-13 13:05:00, Henning Schild wrote: > >> This driver adds initial support for several devices from Siemens. It is > >> based on a platform driver introduced in an earlier commit. > >> > >> One of the supported machines has GPIO connected LEDs, here we poke GPIO > >> memory directly because pinctrl does not come up. > >> > >> Signed-off-by: Henning Schild <henning.schild@siemens.com> > > > > Acked-by: Pavel Machek <pavel@ucw.cz> > > I see that this patch #includes linux/platform_data/x86/simatic-ipc-base.h > which gets added by patch 1/4. > > Pavel, can I take this patch upstream through the pdx86 tree (with you Ack > added)? Or shall I prepare an immutable branch with patch 1 for you to > merge ? Yes, you can. > >> + > >> +static struct simatic_ipc_led simatic_ipc_leds_io[] = { > >> + {1 << 15, "green:" LED_FUNCTION_STATUS "-1" }, > >> + {1 << 7, "yellow:" LED_FUNCTION_STATUS "-1" }, > >> + {1 << 14, "red:" LED_FUNCTION_STATUS "-2" }, > >> + {1 << 6, "yellow:" LED_FUNCTION_STATUS "-2" }, > >> + {1 << 13, "red:" LED_FUNCTION_STATUS "-3" }, > >> + {1 << 5, "yellow:" LED_FUNCTION_STATUS "-3" }, > >> + { } > >> +}; But I'd still like better naming than red:status-2. Pavel
Hi, On 12/19/21 17:49, Pavel Machek wrote: > On Wed 2021-12-15 21:53:56, Hans de Goede wrote: >> Hi, >> >> On 12/15/21 21:18, Pavel Machek wrote: >>> On Mon 2021-12-13 13:05:00, Henning Schild wrote: >>>> This driver adds initial support for several devices from Siemens. It is >>>> based on a platform driver introduced in an earlier commit. >>>> >>>> One of the supported machines has GPIO connected LEDs, here we poke GPIO >>>> memory directly because pinctrl does not come up. >>>> >>>> Signed-off-by: Henning Schild <henning.schild@siemens.com> >>> >>> Acked-by: Pavel Machek <pavel@ucw.cz> >> >> I see that this patch #includes linux/platform_data/x86/simatic-ipc-base.h >> which gets added by patch 1/4. >> >> Pavel, can I take this patch upstream through the pdx86 tree (with you Ack >> added)? Or shall I prepare an immutable branch with patch 1 for you to >> merge ? > > Yes, you can. > > >>>> + >>>> +static struct simatic_ipc_led simatic_ipc_leds_io[] = { >>>> + {1 << 15, "green:" LED_FUNCTION_STATUS "-1" }, >>>> + {1 << 7, "yellow:" LED_FUNCTION_STATUS "-1" }, >>>> + {1 << 14, "red:" LED_FUNCTION_STATUS "-2" }, >>>> + {1 << 6, "yellow:" LED_FUNCTION_STATUS "-2" }, >>>> + {1 << 13, "red:" LED_FUNCTION_STATUS "-3" }, >>>> + {1 << 5, "yellow:" LED_FUNCTION_STATUS "-3" }, >>>> + { } >>>> +}; > > But I'd still like better naming than red:status-2. Hmm, if I merge this and no patch setting a better name is merged before 5.17 final, then we are stuck with this. OTOH I think that given the use of this driver, where AFAIK the use of the LEDs in the field varies, I think this might be the best name we can come up with. Regards, Hans
Am Sun, 19 Dec 2021 17:49:03 +0100 schrieb Pavel Machek <pavel@ucw.cz>: > On Wed 2021-12-15 21:53:56, Hans de Goede wrote: > > Hi, > > > > On 12/15/21 21:18, Pavel Machek wrote: > > > On Mon 2021-12-13 13:05:00, Henning Schild wrote: > > >> This driver adds initial support for several devices from > > >> Siemens. It is based on a platform driver introduced in an > > >> earlier commit. > > >> > > >> One of the supported machines has GPIO connected LEDs, here we > > >> poke GPIO memory directly because pinctrl does not come up. > > >> > > >> Signed-off-by: Henning Schild <henning.schild@siemens.com> > > > > > > Acked-by: Pavel Machek <pavel@ucw.cz> > > > > I see that this patch #includes > > linux/platform_data/x86/simatic-ipc-base.h which gets added by > > patch 1/4. > > > > Pavel, can I take this patch upstream through the pdx86 tree (with > > you Ack added)? Or shall I prepare an immutable branch with patch 1 > > for you to merge ? > > Yes, you can. > > > > >> + > > >> +static struct simatic_ipc_led simatic_ipc_leds_io[] = { > > >> + {1 << 15, "green:" LED_FUNCTION_STATUS "-1" }, > > >> + {1 << 7, "yellow:" LED_FUNCTION_STATUS "-1" }, > > >> + {1 << 14, "red:" LED_FUNCTION_STATUS "-2" }, > > >> + {1 << 6, "yellow:" LED_FUNCTION_STATUS "-2" }, > > >> + {1 << 13, "red:" LED_FUNCTION_STATUS "-3" }, > > >> + {1 << 5, "yellow:" LED_FUNCTION_STATUS "-3" }, > > >> + { } > > >> +}; > > But I'd still like better naming than red:status-2. We had the name discussion already several times, and i have to admit i am not too happy either. But my impression was that this is an acceptable compromise. I am not happy because the names lack scope, which i had in the first round with "simatic-ipc:red:...". Function is also a bit unclear, but with the numbers and the user manual, or looking at the chassis it kind of adds up and should be clear to users which is which. But i agree with Hans that we should sort this out before merge. So please say what makes you unhappy, maybe that can be fixed ... might even make me happier about the names i feel i had to choose. The LEDs are per definition of the manuals meant for users/applications to signal whatever the use-case might want to signal. There are 3 of them numbered 1-3 on the chassis, and next to the number can often (not always) be found a string like "error", "maint", "run-stop" So a function suggestion i would say. I could envision to use "fault" or "alarm" instead of "status" for the one labeled "error". And maybe "standby" for the one called "maint" but i would really like to keep the numbers. Which would look like status-1 alarm-2 standby-3 But still i have to clue what those names stand for and choosing and of those "undefined" names could just suggest things and break expectations. Calling them all "status" is neutral ... Or can you explain the difference between "fault", "panic" and "alarm". Ask 5 people and get at least 3 different expectations ... i guess. Henning > Pavel
Am Mon, 20 Dec 2021 08:53:55 +0100 schrieb Henning Schild <henning.schild@siemens.com>: > Am Sun, 19 Dec 2021 17:49:03 +0100 > schrieb Pavel Machek <pavel@ucw.cz>: > > > On Wed 2021-12-15 21:53:56, Hans de Goede wrote: > > > Hi, > > > > > > On 12/15/21 21:18, Pavel Machek wrote: > > > > On Mon 2021-12-13 13:05:00, Henning Schild wrote: > > > >> This driver adds initial support for several devices from > > > >> Siemens. It is based on a platform driver introduced in an > > > >> earlier commit. > > > >> > > > >> One of the supported machines has GPIO connected LEDs, here we > > > >> poke GPIO memory directly because pinctrl does not come up. > > > >> > > > >> Signed-off-by: Henning Schild <henning.schild@siemens.com> > > > > > > > > Acked-by: Pavel Machek <pavel@ucw.cz> > > > > > > I see that this patch #includes > > > linux/platform_data/x86/simatic-ipc-base.h which gets added by > > > patch 1/4. > > > > > > Pavel, can I take this patch upstream through the pdx86 tree (with > > > you Ack added)? Or shall I prepare an immutable branch with patch > > > 1 for you to merge ? > > > > Yes, you can. > > > > > > > >> + > > > >> +static struct simatic_ipc_led simatic_ipc_leds_io[] = { > > > >> + {1 << 15, "green:" LED_FUNCTION_STATUS "-1" }, > > > >> + {1 << 7, "yellow:" LED_FUNCTION_STATUS "-1" }, > > > >> + {1 << 14, "red:" LED_FUNCTION_STATUS "-2" }, > > > >> + {1 << 6, "yellow:" LED_FUNCTION_STATUS "-2" }, > > > >> + {1 << 13, "red:" LED_FUNCTION_STATUS "-3" }, > > > >> + {1 << 5, "yellow:" LED_FUNCTION_STATUS "-3" }, > > > >> + { } > > > >> +}; > > > > But I'd still like better naming than red:status-2. > > We had the name discussion already several times, and i have to admit > i am not too happy either. > > But my impression was that this is an acceptable compromise. I am not > happy because the names lack scope, which i had in the first round > with "simatic-ipc:red:...". > > Function is also a bit unclear, but with the numbers and the user > manual, or looking at the chassis it kind of adds up and should be > clear to users which is which. > > But i agree with Hans that we should sort this out before merge. So > please say what makes you unhappy, maybe that can be fixed ... might > even make me happier about the names i feel i had to choose. > > The LEDs are per definition of the manuals meant for > users/applications to signal whatever the use-case might want to > signal. There are 3 of them numbered 1-3 on the chassis, and next to > the number can often (not always) be found a string like "error", > "maint", "run-stop" So a function suggestion i would say. > > I could envision to use "fault" or "alarm" instead of "status" for the > one labeled "error". And maybe "standby" for the one called "maint" > but i would really like to keep the numbers. > > Which would look like > > status-1 > alarm-2 > standby-3 > > But still i have to clue what those names stand for and choosing > and of those "undefined" names could just suggest things and break > expectations. Calling them all "status" is neutral ... > > Or can you explain the difference between "fault", "panic" and > "alarm". Ask 5 people and get at least 3 different expectations ... i > guess. Long story short, i am also not happy but the current suggestion is the most generic and least "expectation-creating" i could come up with. While keeping a mapping between the name and the chassis/manual. So i will stick with it, unless i get concrete suggestions on how to improve. The misc functions https://elixir.bootlin.com/linux/latest/source/include/dt-bindings/leds/common.h#L63 do not seem usable. Without a set of conventions they are nothing but "allowed but undefined strings". I could however introduce FUNCTION_ERROR FUNCTION_MAINT FUNCTION_RUN_STOP ... add more such random strings. Which would probably make me happy because it would create a better mapping between the names and the chassis ... but it would worsen the problem of "what are those misc functions anyway?" Henning > Henning > > > > Pavel > > >
Hi, On 12/20/21 09:14, Henning Schild wrote: > Am Mon, 20 Dec 2021 08:53:55 +0100 > schrieb Henning Schild <henning.schild@siemens.com>: > >> Am Sun, 19 Dec 2021 17:49:03 +0100 >> schrieb Pavel Machek <pavel@ucw.cz>: >> >>> On Wed 2021-12-15 21:53:56, Hans de Goede wrote: >>>> Hi, >>>> >>>> On 12/15/21 21:18, Pavel Machek wrote: >>>>> On Mon 2021-12-13 13:05:00, Henning Schild wrote: >>>>>> This driver adds initial support for several devices from >>>>>> Siemens. It is based on a platform driver introduced in an >>>>>> earlier commit. >>>>>> >>>>>> One of the supported machines has GPIO connected LEDs, here we >>>>>> poke GPIO memory directly because pinctrl does not come up. >>>>>> >>>>>> Signed-off-by: Henning Schild <henning.schild@siemens.com> >>>>> >>>>> Acked-by: Pavel Machek <pavel@ucw.cz> >>>> >>>> I see that this patch #includes >>>> linux/platform_data/x86/simatic-ipc-base.h which gets added by >>>> patch 1/4. >>>> >>>> Pavel, can I take this patch upstream through the pdx86 tree (with >>>> you Ack added)? Or shall I prepare an immutable branch with patch >>>> 1 for you to merge ? >>> >>> Yes, you can. >>> >>> >>>>>> + >>>>>> +static struct simatic_ipc_led simatic_ipc_leds_io[] = { >>>>>> + {1 << 15, "green:" LED_FUNCTION_STATUS "-1" }, >>>>>> + {1 << 7, "yellow:" LED_FUNCTION_STATUS "-1" }, >>>>>> + {1 << 14, "red:" LED_FUNCTION_STATUS "-2" }, >>>>>> + {1 << 6, "yellow:" LED_FUNCTION_STATUS "-2" }, >>>>>> + {1 << 13, "red:" LED_FUNCTION_STATUS "-3" }, >>>>>> + {1 << 5, "yellow:" LED_FUNCTION_STATUS "-3" }, >>>>>> + { } >>>>>> +}; >>> >>> But I'd still like better naming than red:status-2. >> >> We had the name discussion already several times, and i have to admit >> i am not too happy either. >> >> But my impression was that this is an acceptable compromise. I am not >> happy because the names lack scope, which i had in the first round >> with "simatic-ipc:red:...". >> >> Function is also a bit unclear, but with the numbers and the user >> manual, or looking at the chassis it kind of adds up and should be >> clear to users which is which. >> >> But i agree with Hans that we should sort this out before merge. So >> please say what makes you unhappy, maybe that can be fixed ... might >> even make me happier about the names i feel i had to choose. >> >> The LEDs are per definition of the manuals meant for >> users/applications to signal whatever the use-case might want to >> signal. There are 3 of them numbered 1-3 on the chassis, and next to >> the number can often (not always) be found a string like "error", >> "maint", "run-stop" So a function suggestion i would say. >> >> I could envision to use "fault" or "alarm" instead of "status" for the >> one labeled "error". And maybe "standby" for the one called "maint" >> but i would really like to keep the numbers. >> >> Which would look like >> >> status-1 >> alarm-2 >> standby-3 >> >> But still i have to clue what those names stand for and choosing >> and of those "undefined" names could just suggest things and break >> expectations. Calling them all "status" is neutral ... >> >> Or can you explain the difference between "fault", "panic" and >> "alarm". Ask 5 people and get at least 3 different expectations ... i >> guess. > > Long story short, i am also not happy but the current suggestion is the > most generic and least "expectation-creating" i could come up with. > While keeping a mapping between the name and the chassis/manual. > > So i will stick with it, unless i get concrete suggestions on how to > improve. Ok, given the above I've gone ahead and merged this series. I too believe the current status names are fine, but if someone disagrees, they still have the entire 5.17 cycle to come up with something better. Regards, Hans
Am Mon, 13 Dec 2021 13:05:00 +0100 schrieb Henning Schild <henning.schild@siemens.com>: > This driver adds initial support for several devices from Siemens. It > is based on a platform driver introduced in an earlier commit. > > One of the supported machines has GPIO connected LEDs, here we poke > GPIO memory directly because pinctrl does not come up. > > Signed-off-by: Henning Schild <henning.schild@siemens.com> > --- > 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 > +++++++++++++++++++++++++ 5 files changed, 221 insertions(+) > create mode 100644 drivers/leds/simple/Kconfig > create mode 100644 drivers/leds/simple/Makefile > create mode 100644 drivers/leds/simple/simatic-ipc-leds.c > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index ed800f5da7d8..ac6688d7a3f4 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -879,4 +879,7 @@ source "drivers/leds/flash/Kconfig" > comment "LED Triggers" > source "drivers/leds/trigger/Kconfig" > > +comment "Simple LED drivers" > +source "drivers/leds/simple/Kconfig" > + > endif # NEW_LEDS > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index c636ec069612..1a719caf14c0 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -105,3 +105,6 @@ obj-$(CONFIG_LEDS_TRIGGERS) += > trigger/ > # LED Blink > obj-y += blink/ > + > +# Simple LED drivers > +obj-y += simple/ > diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig > new file mode 100644 > index 000000000000..9f6a68336659 > --- /dev/null > +++ b/drivers/leds/simple/Kconfig > @@ -0,0 +1,11 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +config LEDS_SIEMENS_SIMATIC_IPC > + tristate "LED driver for Siemens Simatic IPCs" > + depends on LEDS_CLASS > + depends on SIEMENS_SIMATIC_IPC Trying to bring this into distros i think this one should rather be "select SIEMENS_SIMATIC_IPC" and the same thought goes for wdt. Users care about enabling a feature and maybe not too much about having to enable infra, enabling SIEMENS_SIMATIC_IPC is just infra and useless without LEDS_SIEMENS_SIMATIC_IPC and/or SIEMENS_SIMATIC_IPC_WDT. So those two should probably "select" and not "depend". I will send a patch, feel free to tell me that i am wrong and teach me about how deps should work. regards, Henning > + help > + This option enables support for the LEDs of several > Industrial PCs > + from Siemens. > + > + To compile this driver as a module, choose M here: the > module > + will be called simatic-ipc-leds. > diff --git a/drivers/leds/simple/Makefile > b/drivers/leds/simple/Makefile new file mode 100644 > index 000000000000..8481f1e9e360 > --- /dev/null > +++ b/drivers/leds/simple/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0 > +obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC) += simatic-ipc-leds.o > diff --git a/drivers/leds/simple/simatic-ipc-leds.c > b/drivers/leds/simple/simatic-ipc-leds.c new file mode 100644 > index 000000000000..ff2c96e73241 > --- /dev/null > +++ b/drivers/leds/simple/simatic-ipc-leds.c > @@ -0,0 +1,202 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Siemens SIMATIC IPC driver for LEDs > + * > + * Copyright (c) Siemens AG, 2018-2021 > + * > + * Authors: > + * Henning Schild <henning.schild@siemens.com> > + * Jan Kiszka <jan.kiszka@siemens.com> > + * Gerd Haeussler <gerd.haeussler.ext@siemens.com> > + */ > + > +#include <linux/ioport.h> > +#include <linux/kernel.h> > +#include <linux/leds.h> > +#include <linux/module.h> > +#include <linux/pci.h> > +#include <linux/platform_data/x86/simatic-ipc-base.h> > +#include <linux/platform_device.h> > +#include <linux/sizes.h> > +#include <linux/spinlock.h> > + > +#define SIMATIC_IPC_LED_PORT_BASE 0x404E > + > +struct simatic_ipc_led { > + unsigned int value; /* mask for io and offset for mem */ > + char *name; > + struct led_classdev cdev; > +}; > + > +static struct simatic_ipc_led simatic_ipc_leds_io[] = { > + {1 << 15, "green:" LED_FUNCTION_STATUS "-1" }, > + {1 << 7, "yellow:" LED_FUNCTION_STATUS "-1" }, > + {1 << 14, "red:" LED_FUNCTION_STATUS "-2" }, > + {1 << 6, "yellow:" LED_FUNCTION_STATUS "-2" }, > + {1 << 13, "red:" LED_FUNCTION_STATUS "-3" }, > + {1 << 5, "yellow:" LED_FUNCTION_STATUS "-3" }, > + { } > +}; > + > +/* the actual start will be discovered with PCI, 0 is a placeholder > */ +struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, > SZ_4K, KBUILD_MODNAME); + > +static void *simatic_ipc_led_memory; > + > +static struct simatic_ipc_led simatic_ipc_leds_mem[] = { > + {0x500 + 0x1A0, "red:" LED_FUNCTION_STATUS "-1"}, > + {0x500 + 0x1A8, "green:" LED_FUNCTION_STATUS "-1"}, > + {0x500 + 0x1C8, "red:" LED_FUNCTION_STATUS "-2"}, > + {0x500 + 0x1D0, "green:" LED_FUNCTION_STATUS "-2"}, > + {0x500 + 0x1E0, "red:" LED_FUNCTION_STATUS "-3"}, > + {0x500 + 0x198, "green:" LED_FUNCTION_STATUS "-3"}, > + { } > +}; > + > +static struct resource simatic_ipc_led_io_res = > + DEFINE_RES_IO_NAMED(SIMATIC_IPC_LED_PORT_BASE, SZ_2, > KBUILD_MODNAME); + > +static DEFINE_SPINLOCK(reg_lock); > + > +static inline struct simatic_ipc_led *cdev_to_led(struct > led_classdev *led_cd) +{ > + return container_of(led_cd, struct simatic_ipc_led, cdev); > +} > + > +static void simatic_ipc_led_set_io(struct led_classdev *led_cd, > + enum led_brightness brightness) > +{ > + struct simatic_ipc_led *led = cdev_to_led(led_cd); > + unsigned long flags; > + unsigned int val; > + > + spin_lock_irqsave(®_lock, flags); > + > + val = inw(SIMATIC_IPC_LED_PORT_BASE); > + if (brightness == LED_OFF) > + outw(val | led->value, SIMATIC_IPC_LED_PORT_BASE); > + else > + outw(val & ~led->value, SIMATIC_IPC_LED_PORT_BASE); > + > + spin_unlock_irqrestore(®_lock, flags); > +} > + > +static enum led_brightness simatic_ipc_led_get_io(struct > led_classdev *led_cd) +{ > + struct simatic_ipc_led *led = cdev_to_led(led_cd); > + > + return inw(SIMATIC_IPC_LED_PORT_BASE) & led->value ? LED_OFF > : led_cd->max_brightness; +} > + > +static void simatic_ipc_led_set_mem(struct led_classdev *led_cd, > + enum led_brightness brightness) > +{ > + struct simatic_ipc_led *led = cdev_to_led(led_cd); > + > + u32 *p; > + > + p = simatic_ipc_led_memory + led->value; > + *p = (*p & ~1) | (brightness == LED_OFF); > +} > + > +static enum led_brightness simatic_ipc_led_get_mem(struct > led_classdev *led_cd) +{ > + struct simatic_ipc_led *led = cdev_to_led(led_cd); > + > + u32 *p; > + > + p = simatic_ipc_led_memory + led->value; > + return (*p & 1) ? LED_OFF : led_cd->max_brightness; > +} > + > +static int simatic_ipc_leds_probe(struct platform_device *pdev) > +{ > + const struct simatic_ipc_platform *plat = > pdev->dev.platform_data; > + struct device *dev = &pdev->dev; > + struct simatic_ipc_led *ipcled; > + struct led_classdev *cdev; > + struct resource *res; > + int err, type; > + u32 *p; > + > + switch (plat->devmode) { > + case SIMATIC_IPC_DEVICE_227D: > + case SIMATIC_IPC_DEVICE_427E: > + res = &simatic_ipc_led_io_res; > + ipcled = simatic_ipc_leds_io; > + /* on 227D the two bytes work the other way araound > */ > + if (plat->devmode == SIMATIC_IPC_DEVICE_227D) { > + while (ipcled->value) { > + ipcled->value = > swab16(ipcled->value); > + ipcled++; > + } > + ipcled = simatic_ipc_leds_io; > + } > + type = IORESOURCE_IO; > + if (!devm_request_region(dev, res->start, > resource_size(res), KBUILD_MODNAME)) { > + dev_err(dev, "Unable to register IO resource > at %pR\n", res); > + return -EBUSY; > + } > + break; > + case SIMATIC_IPC_DEVICE_127E: > + res = &simatic_ipc_led_mem_res; > + ipcled = simatic_ipc_leds_mem; > + type = IORESOURCE_MEM; > + > + /* get GPIO base from PCI */ > + res->start = simatic_ipc_get_membase0(PCI_DEVFN(13, > 0)); > + if (res->start == 0) > + return -ENODEV; > + > + /* do the final address calculation */ > + res->start = res->start + (0xC5 << 16); > + res->end += res->start; > + > + simatic_ipc_led_memory = devm_ioremap_resource(dev, > res); > + if (IS_ERR(simatic_ipc_led_memory)) > + return PTR_ERR(simatic_ipc_led_memory); > + > + /* initialize power/watchdog LED */ > + p = simatic_ipc_led_memory + 0x500 + 0x1D8; /* > PM_WDT_OUT */ > + *p = (*p & ~1); > + p = simatic_ipc_led_memory + 0x500 + 0x1C0; /* > PM_BIOS_BOOT_N */ > + *p = (*p | 1); > + > + break; > + default: > + return -ENODEV; > + } > + > + while (ipcled->value) { > + cdev = &ipcled->cdev; > + if (type == IORESOURCE_MEM) { > + cdev->brightness_set = > simatic_ipc_led_set_mem; > + cdev->brightness_get = > simatic_ipc_led_get_mem; > + } else { > + cdev->brightness_set = > simatic_ipc_led_set_io; > + cdev->brightness_get = > simatic_ipc_led_get_io; > + } > + cdev->max_brightness = LED_ON; > + cdev->name = ipcled->name; > + > + err = devm_led_classdev_register(dev, cdev); > + if (err < 0) > + return err; > + ipcled++; > + } > + > + return 0; > +} > + > +static struct platform_driver simatic_ipc_led_driver = { > + .probe = simatic_ipc_leds_probe, > + .driver = { > + .name = KBUILD_MODNAME, > + } > +}; > + > +module_platform_driver(simatic_ipc_led_driver); > + > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("platform:" KBUILD_MODNAME); > +MODULE_AUTHOR("Henning Schild <henning.schild@siemens.com>");
On 5/13/22 12:40, Henning Schild wrote: > Am Mon, 13 Dec 2021 13:05:00 +0100 > schrieb Henning Schild <henning.schild@siemens.com>: > >> This driver adds initial support for several devices from Siemens. It >> is based on a platform driver introduced in an earlier commit. >> >> One of the supported machines has GPIO connected LEDs, here we poke >> GPIO memory directly because pinctrl does not come up. >> >> Signed-off-by: Henning Schild <henning.schild@siemens.com> >> --- >> 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 >> +++++++++++++++++++++++++ 5 files changed, 221 insertions(+) >> create mode 100644 drivers/leds/simple/Kconfig >> create mode 100644 drivers/leds/simple/Makefile >> create mode 100644 drivers/leds/simple/simatic-ipc-leds.c >> >> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig >> index ed800f5da7d8..ac6688d7a3f4 100644 >> --- a/drivers/leds/Kconfig >> +++ b/drivers/leds/Kconfig >> @@ -879,4 +879,7 @@ source "drivers/leds/flash/Kconfig" >> comment "LED Triggers" >> source "drivers/leds/trigger/Kconfig" >> >> +comment "Simple LED drivers" >> +source "drivers/leds/simple/Kconfig" >> + >> endif # NEW_LEDS >> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile >> index c636ec069612..1a719caf14c0 100644 >> --- a/drivers/leds/Makefile >> +++ b/drivers/leds/Makefile >> @@ -105,3 +105,6 @@ obj-$(CONFIG_LEDS_TRIGGERS) += >> trigger/ >> # LED Blink >> obj-y += blink/ >> + >> +# Simple LED drivers >> +obj-y += simple/ >> diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig >> new file mode 100644 >> index 000000000000..9f6a68336659 >> --- /dev/null >> +++ b/drivers/leds/simple/Kconfig >> @@ -0,0 +1,11 @@ >> +# SPDX-License-Identifier: GPL-2.0-only >> +config LEDS_SIEMENS_SIMATIC_IPC >> + tristate "LED driver for Siemens Simatic IPCs" >> + depends on LEDS_CLASS >> + depends on SIEMENS_SIMATIC_IPC > > Trying to bring this into distros i think this one should rather be > "select SIEMENS_SIMATIC_IPC" and the same thought goes for wdt. > > Users care about enabling a feature and maybe not too much about having > to enable infra, enabling SIEMENS_SIMATIC_IPC is just infra and useless > without LEDS_SIEMENS_SIMATIC_IPC and/or SIEMENS_SIMATIC_IPC_WDT. > > So those two should probably "select" and not "depend". > > I will send a patch, feel free to tell me that i am wrong and teach me > about how deps should work. > I disagree. Normally drivers depend on the platform and don't select it. It would make more sense to specify something like depends on SIEMENS_SIMATIC_IPC default SIEMENS_SIMATIC_IPC In this context, the description for SIEMENS_SIMATIC_IPC is a bit misleading. Something like "Platform driver for Simatic Industrial PC" or just "Support for Simatic Industrial PC" would be a better description. Guenter > regards, > Henning > >> + help >> + This option enables support for the LEDs of several >> Industrial PCs >> + from Siemens. >> + >> + To compile this driver as a module, choose M here: the >> module >> + will be called simatic-ipc-leds. >> diff --git a/drivers/leds/simple/Makefile >> b/drivers/leds/simple/Makefile new file mode 100644 >> index 000000000000..8481f1e9e360 >> --- /dev/null >> +++ b/drivers/leds/simple/Makefile >> @@ -0,0 +1,2 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> +obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC) += simatic-ipc-leds.o >> diff --git a/drivers/leds/simple/simatic-ipc-leds.c >> b/drivers/leds/simple/simatic-ipc-leds.c new file mode 100644 >> index 000000000000..ff2c96e73241 >> --- /dev/null >> +++ b/drivers/leds/simple/simatic-ipc-leds.c >> @@ -0,0 +1,202 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Siemens SIMATIC IPC driver for LEDs >> + * >> + * Copyright (c) Siemens AG, 2018-2021 >> + * >> + * Authors: >> + * Henning Schild <henning.schild@siemens.com> >> + * Jan Kiszka <jan.kiszka@siemens.com> >> + * Gerd Haeussler <gerd.haeussler.ext@siemens.com> >> + */ >> + >> +#include <linux/ioport.h> >> +#include <linux/kernel.h> >> +#include <linux/leds.h> >> +#include <linux/module.h> >> +#include <linux/pci.h> >> +#include <linux/platform_data/x86/simatic-ipc-base.h> >> +#include <linux/platform_device.h> >> +#include <linux/sizes.h> >> +#include <linux/spinlock.h> >> + >> +#define SIMATIC_IPC_LED_PORT_BASE 0x404E >> + >> +struct simatic_ipc_led { >> + unsigned int value; /* mask for io and offset for mem */ >> + char *name; >> + struct led_classdev cdev; >> +}; >> + >> +static struct simatic_ipc_led simatic_ipc_leds_io[] = { >> + {1 << 15, "green:" LED_FUNCTION_STATUS "-1" }, >> + {1 << 7, "yellow:" LED_FUNCTION_STATUS "-1" }, >> + {1 << 14, "red:" LED_FUNCTION_STATUS "-2" }, >> + {1 << 6, "yellow:" LED_FUNCTION_STATUS "-2" }, >> + {1 << 13, "red:" LED_FUNCTION_STATUS "-3" }, >> + {1 << 5, "yellow:" LED_FUNCTION_STATUS "-3" }, >> + { } >> +}; >> + >> +/* the actual start will be discovered with PCI, 0 is a placeholder >> */ +struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, >> SZ_4K, KBUILD_MODNAME); + >> +static void *simatic_ipc_led_memory; >> + >> +static struct simatic_ipc_led simatic_ipc_leds_mem[] = { >> + {0x500 + 0x1A0, "red:" LED_FUNCTION_STATUS "-1"}, >> + {0x500 + 0x1A8, "green:" LED_FUNCTION_STATUS "-1"}, >> + {0x500 + 0x1C8, "red:" LED_FUNCTION_STATUS "-2"}, >> + {0x500 + 0x1D0, "green:" LED_FUNCTION_STATUS "-2"}, >> + {0x500 + 0x1E0, "red:" LED_FUNCTION_STATUS "-3"}, >> + {0x500 + 0x198, "green:" LED_FUNCTION_STATUS "-3"}, >> + { } >> +}; >> + >> +static struct resource simatic_ipc_led_io_res = >> + DEFINE_RES_IO_NAMED(SIMATIC_IPC_LED_PORT_BASE, SZ_2, >> KBUILD_MODNAME); + >> +static DEFINE_SPINLOCK(reg_lock); >> + >> +static inline struct simatic_ipc_led *cdev_to_led(struct >> led_classdev *led_cd) +{ >> + return container_of(led_cd, struct simatic_ipc_led, cdev); >> +} >> + >> +static void simatic_ipc_led_set_io(struct led_classdev *led_cd, >> + enum led_brightness brightness) >> +{ >> + struct simatic_ipc_led *led = cdev_to_led(led_cd); >> + unsigned long flags; >> + unsigned int val; >> + >> + spin_lock_irqsave(®_lock, flags); >> + >> + val = inw(SIMATIC_IPC_LED_PORT_BASE); >> + if (brightness == LED_OFF) >> + outw(val | led->value, SIMATIC_IPC_LED_PORT_BASE); >> + else >> + outw(val & ~led->value, SIMATIC_IPC_LED_PORT_BASE); >> + >> + spin_unlock_irqrestore(®_lock, flags); >> +} >> + >> +static enum led_brightness simatic_ipc_led_get_io(struct >> led_classdev *led_cd) +{ >> + struct simatic_ipc_led *led = cdev_to_led(led_cd); >> + >> + return inw(SIMATIC_IPC_LED_PORT_BASE) & led->value ? LED_OFF >> : led_cd->max_brightness; +} >> + >> +static void simatic_ipc_led_set_mem(struct led_classdev *led_cd, >> + enum led_brightness brightness) >> +{ >> + struct simatic_ipc_led *led = cdev_to_led(led_cd); >> + >> + u32 *p; >> + >> + p = simatic_ipc_led_memory + led->value; >> + *p = (*p & ~1) | (brightness == LED_OFF); >> +} >> + >> +static enum led_brightness simatic_ipc_led_get_mem(struct >> led_classdev *led_cd) +{ >> + struct simatic_ipc_led *led = cdev_to_led(led_cd); >> + >> + u32 *p; >> + >> + p = simatic_ipc_led_memory + led->value; >> + return (*p & 1) ? LED_OFF : led_cd->max_brightness; >> +} >> + >> +static int simatic_ipc_leds_probe(struct platform_device *pdev) >> +{ >> + const struct simatic_ipc_platform *plat = >> pdev->dev.platform_data; >> + struct device *dev = &pdev->dev; >> + struct simatic_ipc_led *ipcled; >> + struct led_classdev *cdev; >> + struct resource *res; >> + int err, type; >> + u32 *p; >> + >> + switch (plat->devmode) { >> + case SIMATIC_IPC_DEVICE_227D: >> + case SIMATIC_IPC_DEVICE_427E: >> + res = &simatic_ipc_led_io_res; >> + ipcled = simatic_ipc_leds_io; >> + /* on 227D the two bytes work the other way araound >> */ >> + if (plat->devmode == SIMATIC_IPC_DEVICE_227D) { >> + while (ipcled->value) { >> + ipcled->value = >> swab16(ipcled->value); >> + ipcled++; >> + } >> + ipcled = simatic_ipc_leds_io; >> + } >> + type = IORESOURCE_IO; >> + if (!devm_request_region(dev, res->start, >> resource_size(res), KBUILD_MODNAME)) { >> + dev_err(dev, "Unable to register IO resource >> at %pR\n", res); >> + return -EBUSY; >> + } >> + break; >> + case SIMATIC_IPC_DEVICE_127E: >> + res = &simatic_ipc_led_mem_res; >> + ipcled = simatic_ipc_leds_mem; >> + type = IORESOURCE_MEM; >> + >> + /* get GPIO base from PCI */ >> + res->start = simatic_ipc_get_membase0(PCI_DEVFN(13, >> 0)); >> + if (res->start == 0) >> + return -ENODEV; >> + >> + /* do the final address calculation */ >> + res->start = res->start + (0xC5 << 16); >> + res->end += res->start; >> + >> + simatic_ipc_led_memory = devm_ioremap_resource(dev, >> res); >> + if (IS_ERR(simatic_ipc_led_memory)) >> + return PTR_ERR(simatic_ipc_led_memory); >> + >> + /* initialize power/watchdog LED */ >> + p = simatic_ipc_led_memory + 0x500 + 0x1D8; /* >> PM_WDT_OUT */ >> + *p = (*p & ~1); >> + p = simatic_ipc_led_memory + 0x500 + 0x1C0; /* >> PM_BIOS_BOOT_N */ >> + *p = (*p | 1); >> + >> + break; >> + default: >> + return -ENODEV; >> + } >> + >> + while (ipcled->value) { >> + cdev = &ipcled->cdev; >> + if (type == IORESOURCE_MEM) { >> + cdev->brightness_set = >> simatic_ipc_led_set_mem; >> + cdev->brightness_get = >> simatic_ipc_led_get_mem; >> + } else { >> + cdev->brightness_set = >> simatic_ipc_led_set_io; >> + cdev->brightness_get = >> simatic_ipc_led_get_io; >> + } >> + cdev->max_brightness = LED_ON; >> + cdev->name = ipcled->name; >> + >> + err = devm_led_classdev_register(dev, cdev); >> + if (err < 0) >> + return err; >> + ipcled++; >> + } >> + >> + return 0; >> +} >> + >> +static struct platform_driver simatic_ipc_led_driver = { >> + .probe = simatic_ipc_leds_probe, >> + .driver = { >> + .name = KBUILD_MODNAME, >> + } >> +}; >> + >> +module_platform_driver(simatic_ipc_led_driver); >> + >> +MODULE_LICENSE("GPL v2"); >> +MODULE_ALIAS("platform:" KBUILD_MODNAME); >> +MODULE_AUTHOR("Henning Schild <henning.schild@siemens.com>"); >
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index ed800f5da7d8..ac6688d7a3f4 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -879,4 +879,7 @@ source "drivers/leds/flash/Kconfig" comment "LED Triggers" source "drivers/leds/trigger/Kconfig" +comment "Simple LED drivers" +source "drivers/leds/simple/Kconfig" + endif # NEW_LEDS diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index c636ec069612..1a719caf14c0 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -105,3 +105,6 @@ obj-$(CONFIG_LEDS_TRIGGERS) += trigger/ # LED Blink obj-y += blink/ + +# Simple LED drivers +obj-y += simple/ diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig new file mode 100644 index 000000000000..9f6a68336659 --- /dev/null +++ b/drivers/leds/simple/Kconfig @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: GPL-2.0-only +config LEDS_SIEMENS_SIMATIC_IPC + tristate "LED driver for Siemens Simatic IPCs" + depends on LEDS_CLASS + depends on SIEMENS_SIMATIC_IPC + help + This option enables support for the LEDs of several Industrial PCs + from Siemens. + + To compile this driver as a module, choose M here: the module + will be called simatic-ipc-leds. diff --git a/drivers/leds/simple/Makefile b/drivers/leds/simple/Makefile new file mode 100644 index 000000000000..8481f1e9e360 --- /dev/null +++ b/drivers/leds/simple/Makefile @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0 +obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC) += simatic-ipc-leds.o diff --git a/drivers/leds/simple/simatic-ipc-leds.c b/drivers/leds/simple/simatic-ipc-leds.c new file mode 100644 index 000000000000..ff2c96e73241 --- /dev/null +++ b/drivers/leds/simple/simatic-ipc-leds.c @@ -0,0 +1,202 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Siemens SIMATIC IPC driver for LEDs + * + * Copyright (c) Siemens AG, 2018-2021 + * + * Authors: + * Henning Schild <henning.schild@siemens.com> + * Jan Kiszka <jan.kiszka@siemens.com> + * Gerd Haeussler <gerd.haeussler.ext@siemens.com> + */ + +#include <linux/ioport.h> +#include <linux/kernel.h> +#include <linux/leds.h> +#include <linux/module.h> +#include <linux/pci.h> +#include <linux/platform_data/x86/simatic-ipc-base.h> +#include <linux/platform_device.h> +#include <linux/sizes.h> +#include <linux/spinlock.h> + +#define SIMATIC_IPC_LED_PORT_BASE 0x404E + +struct simatic_ipc_led { + unsigned int value; /* mask for io and offset for mem */ + char *name; + struct led_classdev cdev; +}; + +static struct simatic_ipc_led simatic_ipc_leds_io[] = { + {1 << 15, "green:" LED_FUNCTION_STATUS "-1" }, + {1 << 7, "yellow:" LED_FUNCTION_STATUS "-1" }, + {1 << 14, "red:" LED_FUNCTION_STATUS "-2" }, + {1 << 6, "yellow:" LED_FUNCTION_STATUS "-2" }, + {1 << 13, "red:" LED_FUNCTION_STATUS "-3" }, + {1 << 5, "yellow:" LED_FUNCTION_STATUS "-3" }, + { } +}; + +/* the actual start will be discovered with PCI, 0 is a placeholder */ +struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, SZ_4K, KBUILD_MODNAME); + +static void *simatic_ipc_led_memory; + +static struct simatic_ipc_led simatic_ipc_leds_mem[] = { + {0x500 + 0x1A0, "red:" LED_FUNCTION_STATUS "-1"}, + {0x500 + 0x1A8, "green:" LED_FUNCTION_STATUS "-1"}, + {0x500 + 0x1C8, "red:" LED_FUNCTION_STATUS "-2"}, + {0x500 + 0x1D0, "green:" LED_FUNCTION_STATUS "-2"}, + {0x500 + 0x1E0, "red:" LED_FUNCTION_STATUS "-3"}, + {0x500 + 0x198, "green:" LED_FUNCTION_STATUS "-3"}, + { } +}; + +static struct resource simatic_ipc_led_io_res = + DEFINE_RES_IO_NAMED(SIMATIC_IPC_LED_PORT_BASE, SZ_2, KBUILD_MODNAME); + +static DEFINE_SPINLOCK(reg_lock); + +static inline struct simatic_ipc_led *cdev_to_led(struct led_classdev *led_cd) +{ + return container_of(led_cd, struct simatic_ipc_led, cdev); +} + +static void simatic_ipc_led_set_io(struct led_classdev *led_cd, + enum led_brightness brightness) +{ + struct simatic_ipc_led *led = cdev_to_led(led_cd); + unsigned long flags; + unsigned int val; + + spin_lock_irqsave(®_lock, flags); + + val = inw(SIMATIC_IPC_LED_PORT_BASE); + if (brightness == LED_OFF) + outw(val | led->value, SIMATIC_IPC_LED_PORT_BASE); + else + outw(val & ~led->value, SIMATIC_IPC_LED_PORT_BASE); + + spin_unlock_irqrestore(®_lock, flags); +} + +static enum led_brightness simatic_ipc_led_get_io(struct led_classdev *led_cd) +{ + struct simatic_ipc_led *led = cdev_to_led(led_cd); + + return inw(SIMATIC_IPC_LED_PORT_BASE) & led->value ? LED_OFF : led_cd->max_brightness; +} + +static void simatic_ipc_led_set_mem(struct led_classdev *led_cd, + enum led_brightness brightness) +{ + struct simatic_ipc_led *led = cdev_to_led(led_cd); + + u32 *p; + + p = simatic_ipc_led_memory + led->value; + *p = (*p & ~1) | (brightness == LED_OFF); +} + +static enum led_brightness simatic_ipc_led_get_mem(struct led_classdev *led_cd) +{ + struct simatic_ipc_led *led = cdev_to_led(led_cd); + + u32 *p; + + p = simatic_ipc_led_memory + led->value; + return (*p & 1) ? LED_OFF : led_cd->max_brightness; +} + +static int simatic_ipc_leds_probe(struct platform_device *pdev) +{ + const struct simatic_ipc_platform *plat = pdev->dev.platform_data; + struct device *dev = &pdev->dev; + struct simatic_ipc_led *ipcled; + struct led_classdev *cdev; + struct resource *res; + int err, type; + u32 *p; + + switch (plat->devmode) { + case SIMATIC_IPC_DEVICE_227D: + case SIMATIC_IPC_DEVICE_427E: + res = &simatic_ipc_led_io_res; + ipcled = simatic_ipc_leds_io; + /* on 227D the two bytes work the other way araound */ + if (plat->devmode == SIMATIC_IPC_DEVICE_227D) { + while (ipcled->value) { + ipcled->value = swab16(ipcled->value); + ipcled++; + } + ipcled = simatic_ipc_leds_io; + } + type = IORESOURCE_IO; + if (!devm_request_region(dev, res->start, resource_size(res), KBUILD_MODNAME)) { + dev_err(dev, "Unable to register IO resource at %pR\n", res); + return -EBUSY; + } + break; + case SIMATIC_IPC_DEVICE_127E: + res = &simatic_ipc_led_mem_res; + ipcled = simatic_ipc_leds_mem; + type = IORESOURCE_MEM; + + /* get GPIO base from PCI */ + res->start = simatic_ipc_get_membase0(PCI_DEVFN(13, 0)); + if (res->start == 0) + return -ENODEV; + + /* do the final address calculation */ + res->start = res->start + (0xC5 << 16); + res->end += res->start; + + simatic_ipc_led_memory = devm_ioremap_resource(dev, res); + if (IS_ERR(simatic_ipc_led_memory)) + return PTR_ERR(simatic_ipc_led_memory); + + /* initialize power/watchdog LED */ + p = simatic_ipc_led_memory + 0x500 + 0x1D8; /* PM_WDT_OUT */ + *p = (*p & ~1); + p = simatic_ipc_led_memory + 0x500 + 0x1C0; /* PM_BIOS_BOOT_N */ + *p = (*p | 1); + + break; + default: + return -ENODEV; + } + + while (ipcled->value) { + cdev = &ipcled->cdev; + if (type == IORESOURCE_MEM) { + cdev->brightness_set = simatic_ipc_led_set_mem; + cdev->brightness_get = simatic_ipc_led_get_mem; + } else { + cdev->brightness_set = simatic_ipc_led_set_io; + cdev->brightness_get = simatic_ipc_led_get_io; + } + cdev->max_brightness = LED_ON; + cdev->name = ipcled->name; + + err = devm_led_classdev_register(dev, cdev); + if (err < 0) + return err; + ipcled++; + } + + return 0; +} + +static struct platform_driver simatic_ipc_led_driver = { + .probe = simatic_ipc_leds_probe, + .driver = { + .name = KBUILD_MODNAME, + } +}; + +module_platform_driver(simatic_ipc_led_driver); + +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:" KBUILD_MODNAME); +MODULE_AUTHOR("Henning Schild <henning.schild@siemens.com>");
This driver adds initial support for several devices from Siemens. It is based on a platform driver introduced in an earlier commit. One of the supported machines has GPIO connected LEDs, here we poke GPIO memory directly because pinctrl does not come up. Signed-off-by: Henning Schild <henning.schild@siemens.com> --- 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 +++++++++++++++++++++++++ 5 files changed, 221 insertions(+) create mode 100644 drivers/leds/simple/Kconfig create mode 100644 drivers/leds/simple/Makefile create mode 100644 drivers/leds/simple/simatic-ipc-leds.c