Re: [PATCH v5 2/4] leds: simatic-ipc-leds: add new driver for Siemens Industial PCs

[Pavel Machek <pavel@ucw.cz>]

[-- Attachment #1: Type: text/plain, Size: 3922 bytes --]

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
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]