[PATCH 1/2] x86: gpio: AMD G-Series pch gpio platform driver

[PATCH 1/2] x86: gpio: AMD G-Series pch gpio platform driver

[Enrico Weigelt, metux IT consult]

GPIO platform driver for the AMD G-series PCH (eg. on GX-412TC)

This driver doesn't registers itself automatically, as it needs to
be provided with platform specific configuration, provided by some
board driver setup code.

Didn't implement oftree probing yet, as it's rarely found on x86.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 MAINTAINERS                                        |   7 +
 drivers/gpio/Kconfig                               |  10 ++
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-amd-fch.c                        | 171 +++++++++++++++++++++
 .../linux/platform_data/x86/amd-fch-gpio-pdata.h   |  41 +++++
 5 files changed, 230 insertions(+)
 create mode 100644 drivers/gpio/gpio-amd-fch.c
 create mode 100644 include/linux/platform_data/x86/amd-fch-gpio-pdata.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 8c68de3c..a693f39 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -766,6 +766,13 @@ S:	Supported
 F:	Documentation/hwmon/fam15h_power
 F:	drivers/hwmon/fam15h_power.c
 
+AMD FCH GPIO DRIVER
+M:	Enrico Weigelt, metux IT consult <info@metux.net>
+L:	linux-gpio@vger.kernel.org
+S:	Maintanced
+F:	drivers/gpio/gpio-amd-fch.c
+F:	include/linux/platform_data/x86/amd-fch-gpio-pdata.h
+
 AMD GEODE CS5536 USB DEVICE CONTROLLER DRIVER
 L:	linux-geode@lists.infradead.org (moderated for non-subscribers)
 S:	Orphan
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index b5a2845..a3e47c8 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -654,6 +654,16 @@ config GPIO_LOONGSON1
 	help
 	  Say Y or M here to support GPIO on Loongson1 SoCs.
 
+config GPIO_AMD_FCH
+	tristate "GPIO support for AMD Fusion Controller Hub (G-series SOCs)"
+	select GPIO_GENERIC
+	help
+	  This option enables driver for GPIO on AMDs Fusion Controller Hub,
+	  as found on G-series SOCs (eg. GX-412TC)
+
+	  Note: This driver doesn't registers itself automatically, as it
+	  needs to be provided with platform specific configuration.
+	  (See eg. CONFIG_PCENGINES_APU2.)
 endmenu
 
 menu "Port-mapped I/O GPIO drivers"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 37628f8..bb48fd2 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_GPIO_ADP5520)	+= gpio-adp5520.o
 obj-$(CONFIG_GPIO_ADP5588)	+= gpio-adp5588.o
 obj-$(CONFIG_GPIO_ALTERA)  	+= gpio-altera.o
 obj-$(CONFIG_GPIO_ALTERA_A10SR)	+= gpio-altera-a10sr.o
+obj-$(CONFIG_GPIO_AMD_FCH)	+= gpio-amd-fch.o
 obj-$(CONFIG_GPIO_AMD8111)	+= gpio-amd8111.o
 obj-$(CONFIG_GPIO_AMDPT)	+= gpio-amdpt.o
 obj-$(CONFIG_GPIO_ARIZONA)	+= gpio-arizona.o
diff --git a/drivers/gpio/gpio-amd-fch.c b/drivers/gpio/gpio-amd-fch.c
new file mode 100644
index 0000000..8a002453
--- /dev/null
+++ b/drivers/gpio/gpio-amd-fch.c
@@ -0,0 +1,171 @@
+/*
+ * GPIO driver for the AMD G series FCH (eg. GX-412TC)
+ *
+ * Copyright (C) 2018 metux IT consult
+ * Author: Enrico Weigelt <info@metux.net>
+ *
+ * SPDX-License-Identifier: GPL+
+ */
+
+// FIXME: add spinlocks
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/gpio/driver.h>
+#include <linux/platform_data/x86/amd-fch-gpio-pdata.h>
+
+
+#define GPIO_BIT_DIR		23
+#define GPIO_BIT_WRITE		22
+#define GPIO_BIT_READ		16
+
+
+struct amd_fch_gpio_priv {
+	struct platform_device		*pdev;
+	struct gpio_chip		gc;
+	void __iomem			*base;
+	struct amd_fch_gpio_pdata	*pdata;
+};
+
+static uint32_t *amd_fch_gpio_addr(struct gpio_chip *gc, unsigned gpio)
+{
+	struct amd_fch_gpio_priv *priv = gpiochip_get_data(gc);
+
+	if (gpio > priv->pdata->gpio_num) {
+		dev_err(&priv->pdev->dev, "gpio number %d out of range\n", gpio);
+		return NULL;
+	}
+
+	return priv->base + priv->pdata->gpio_reg[gpio].reg*sizeof(u32);
+}
+
+static int amd_fch_gpio_direction_input(struct gpio_chip *gc, unsigned offset)
+{
+	volatile uint32_t *ptr = amd_fch_gpio_addr(gc, offset);
+	if (!ptr) return -EINVAL;
+
+	*ptr &= ~(1 << GPIO_BIT_DIR);
+	return 0;
+}
+
+static int amd_fch_gpio_direction_output(struct gpio_chip *gc, unsigned gpio, int value)
+{
+	volatile uint32_t *ptr = amd_fch_gpio_addr(gc, gpio);
+	if (!ptr) return -EINVAL;
+
+	*ptr |= (1 << GPIO_BIT_DIR);
+	return 0;
+}
+
+static int amd_fch_gpio_get_direction(struct gpio_chip *gc, unsigned gpio)
+{
+	volatile uint32_t *ptr = amd_fch_gpio_addr(gc, gpio);
+	if (!ptr) return -EINVAL;
+
+	return (*ptr >> GPIO_BIT_DIR) & 1;
+}
+
+static void amd_fch_gpio_set(struct gpio_chip *gc, unsigned gpio, int value)
+{
+	volatile uint32_t *ptr = amd_fch_gpio_addr(gc, gpio);
+	if (!ptr) return;
+
+	if (value)
+		*ptr |= (1 << GPIO_BIT_WRITE);
+	else
+		*ptr &= ~(1 << GPIO_BIT_WRITE);
+}
+
+static int amd_fch_gpio_get(struct gpio_chip *gc, unsigned offset)
+{
+	volatile uint32_t *ptr = amd_fch_gpio_addr(gc, offset);
+	if (!ptr) return -EINVAL;
+
+	return ((*ptr) >> GPIO_BIT_READ) & 1;
+}
+
+static void amd_fch_gpio_dbg_show(struct seq_file *s, struct gpio_chip *gc)
+{
+	struct amd_fch_gpio_priv *priv = gpiochip_get_data(gc);
+	(void)priv;
+
+	seq_printf(s, "debug info not implemented yet\n");
+}
+
+static int amd_fch_gpio_request(struct gpio_chip *chip, unsigned gpio_pin)
+{
+	if (gpio_pin < chip->ngpio)
+		return 0;
+
+	return -EINVAL;
+}
+
+static int amd_fch_gpio_probe(struct platform_device *pdev)
+{
+	struct amd_fch_gpio_priv *priv;
+	struct amd_fch_gpio_pdata *pdata = pdev->dev.platform_data;
+	int err;
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "no platform_data\n");
+		return -ENOENT;
+	}
+
+	if (!(priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL))) {
+		dev_err(&pdev->dev, "failed to allocate priv struct\n");
+		return -ENOMEM;
+	}
+
+	priv->pdata	= pdata;
+	priv->pdev	= pdev;
+
+	priv->gc.owner			= THIS_MODULE;
+	priv->gc.parent			= &pdev->dev;
+	priv->gc.label			= dev_name(&pdev->dev);
+	priv->gc.base			= priv->pdata->gpio_base;
+	priv->gc.ngpio			= priv->pdata->gpio_num;
+	priv->gc.request		= amd_fch_gpio_request;
+	priv->gc.direction_input	= amd_fch_gpio_direction_input;
+	priv->gc.direction_output	= amd_fch_gpio_direction_output;
+	priv->gc.get_direction		= amd_fch_gpio_get_direction;
+	priv->gc.get			= amd_fch_gpio_get;
+	priv->gc.set			= amd_fch_gpio_set;
+
+	spin_lock_init(&priv->gc.bgpio_lock);
+
+	if (IS_ERR(priv->base = devm_ioremap_resource(&pdev->dev, &priv->pdata->res))) {
+		dev_err(&pdev->dev, "failed to map iomem\n");
+		return -ENXIO;
+	}
+
+	dev_info(&pdev->dev, "initializing on my own II\n");
+
+	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
+		dev_info(&pdev->dev, "enabling debugfs\n");
+		priv->gc.dbg_show = amd_fch_gpio_dbg_show;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	err = devm_gpiochip_add_data(&pdev->dev, &priv->gc, priv);
+	dev_info(&pdev->dev, "probe finished\n");
+	return err;
+}
+
+static struct platform_driver amd_fch_gpio_driver = {
+	.driver = {
+		.name = AMD_FCH_GPIO_DRIVER_NAME,
+	},
+	.probe = amd_fch_gpio_probe,
+};
+
+module_platform_driver(amd_fch_gpio_driver);
+
+MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
+MODULE_DESCRIPTION("AMD G-series FCH GPIO driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:gpio_amd_fch");
diff --git a/include/linux/platform_data/x86/amd-fch-gpio-pdata.h b/include/linux/platform_data/x86/amd-fch-gpio-pdata.h
new file mode 100644
index 0000000..68c1730
--- /dev/null
+++ b/include/linux/platform_data/x86/amd-fch-gpio-pdata.h
@@ -0,0 +1,41 @@
+/*
+ * AMD FCH gpio driver platform-data
+ *
+ * Copyright (C) 2018 metux IT consult
+ * Author: Enrico Weigelt <info@metux.net>
+ *
+ * SPDX-License-Identifier: GPL
+ */
+
+#ifndef AMD_FCH_PDATA_H
+#define AMD_FCH_PDATA_H
+
+
+#include <linux/ioport.h>
+
+#define AMD_FCH_GPIO_DRIVER_NAME "gpio_amd_fch"
+
+/*
+ * struct amd_fch_gpio_reg - GPIO register definition
+ * @reg: register index
+ * @name: signal name
+ */
+struct amd_fch_gpio_reg {
+    int         reg;
+    const char* name;
+};
+
+/*
+ * struct amd_fch_gpio_pdata - GPIO chip platform data
+ * @resource: iomem range
+ * @gpio_reg: array of gpio registers
+ * @gpio_num: number of entries
+ */
+struct amd_fch_gpio_pdata {
+    struct resource          res;
+    int                      gpio_num;
+    struct amd_fch_gpio_reg *gpio_reg;
+    int                      gpio_base;
+};
+
+#endif /* AMD_FCH_PDATA_H */
-- 
1.9.1

[PATCH 2/2] x86: pcengines apuv2 gpio/leds/keys platform driver

[Enrico Weigelt, metux IT consult]

Driver for PCengines APUv2 board that supports GPIOs via AMD PCH
and attached LEDs and keys.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 MAINTAINERS                            |   5 +
 drivers/platform/x86/Kconfig           |   9 ++
 drivers/platform/x86/Makefile          |   1 +
 drivers/platform/x86/pcengines-apuv2.c | 263 +++++++++++++++++++++++++++++++++
 4 files changed, 278 insertions(+)
 create mode 100644 drivers/platform/x86/pcengines-apuv2.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a693f39..2bdc975 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11515,6 +11515,11 @@ F:	lib/parman.c
 F:	lib/test_parman.c
 F:	include/linux/parman.h
 
+PC ENGINES APU BOARD DRIVER
+M:	Enrico Weigelt, metux IT consult <info@metux.net>
+S:	Maintained
+F:	drivers/platform/x86/pcengines-apuv2.c
+
 PC87360 HARDWARE MONITORING DRIVER
 M:	Jim Cromie <jim.cromie@gmail.com>
 L:	linux-hwmon@vger.kernel.org
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index b5e9db8..e424643 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1303,6 +1303,15 @@ config HUAWEI_WMI
 	  To compile this driver as a module, choose M here: the module
 	  will be called huawei-wmi.
 
+config PCENGINES_APU2
+	tristate "LEDs and buttons driver forPC Engines APUv2 board"
+	depends on GPIO_AMD_FCH
+	depends on KEYBOARD_GPIO
+	depends on KEYBOARD_GPIO_POLLED
+	depends on LEDS_GPIO
+	---help---
+	  This options adds APUv2 board support for LEDs and keys
+
 endif # X86_PLATFORM_DEVICES
 
 config PMC_ATOM
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index ce8da26..86cb766 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -96,3 +96,4 @@ obj-$(CONFIG_INTEL_TURBO_MAX_3) += intel_turbo_max_3.o
 obj-$(CONFIG_INTEL_CHTDC_TI_PWRBTN)	+= intel_chtdc_ti_pwrbtn.o
 obj-$(CONFIG_I2C_MULTI_INSTANTIATE)	+= i2c-multi-instantiate.o
 obj-$(CONFIG_INTEL_ATOMISP2_PM)	+= intel_atomisp2_pm.o
+obj-$(CONFIG_PCENGINES_APU2)	+= pcengines-apuv2.o
diff --git a/drivers/platform/x86/pcengines-apuv2.c b/drivers/platform/x86/pcengines-apuv2.c
new file mode 100644
index 0000000..9bb89d6
--- /dev/null
+++ b/drivers/platform/x86/pcengines-apuv2.c
@@ -0,0 +1,263 @@
+/*
+ * PC-Engines APUv2 board platform driver for gpio buttons and LEDs
+ *
+ * Copyright (C) 2018 metux IT consult
+ * Author: Enrico Weigelt <info@metux.net>
+ */
+
+// SPDX-License-Identifier: GPL+
+
+#include <linux/dmi.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/gpio_keys.h>
+#include <linux/input.h>
+#include <linux/platform_data/x86/amd-fch-gpio-pdata.h>
+
+/* TODO
+   * support apu1 board (different fch, different register layouts
+   * add spinlocks
+*/
+
+#define FCH_ACPI_MMIO_BASE	0xFED80000
+#define FCH_GPIO_OFFSET		0x1500
+#define FCH_GPIO_SIZE		0x300
+
+#define GPIO_BASE		100
+
+#define GPIO_LED1		(GPIO_BASE+0)
+#define GPIO_LED2		(GPIO_BASE+1)
+#define GPIO_LED3		(GPIO_BASE+2)
+#define GPIO_MODESW		(GPIO_BASE+3)
+#define GPIO_SIMSWAP		(GPIO_BASE+4)
+
+struct board_data {
+	const char		*name;
+	struct resource		res;
+	int			gpio_num;
+	int			gpio_base;
+	struct amd_fch_gpio_reg	*gpio_regs;
+};
+
+static const struct gpio_led apu2_leds[] /* __initconst */ = {
+	{ .name = "apu:green:1", .gpio = GPIO_LED1, .active_low = 1, },
+	{ .name = "apu:green:2", .gpio = GPIO_LED2, .active_low = 1, },
+	{ .name = "apu:green:3", .gpio = GPIO_LED3, .active_low = 1, }
+};
+
+static const struct gpio_led_platform_data apu2_leds_pdata /* __initconst */ = {
+	.num_leds	= ARRAY_SIZE(apu2_leds),
+	.leds		= apu2_leds,
+};
+
+static struct amd_fch_gpio_reg apu2_gpio_regs[] = {
+	{ 0x44 }, // GPIO_57 -- LED1
+	{ 0x45 }, // GPIO_58 -- LED2
+	{ 0x46 }, // GPIO_59 -- LED3
+	{ 0x59 }, // GPIO_32 -- LED4 -- GE32 -- #modesw
+	{ 0x5A }, // GPIO_33 -- LED5 -- GE33 -- simswap
+	{ 0x42 }, // GPIO_51 -- LED6
+	{ 0x43 }, // GPIO_55 -- LED7
+	{ 0x47 }, // GPIO_64 -- LED8
+	{ 0x48 }, // GPIO_68 -- LED9
+	{ 0x4C }, // GPIO_70 -- LED10
+};
+
+static struct gpio_keys_button apu2_keys_buttons[] = {
+	{
+		.code			= KEY_A,
+		.gpio			= GPIO_MODESW,
+		.active_low		= 1,
+		.desc			= "modeswitch",
+		.type			= EV_KEY, /* or EV_SW ? */
+		.debounce_interval	= 10,
+		.value			= 1,
+	}
+};
+
+static const struct gpio_keys_platform_data apu2_keys_pdata = {
+	.buttons	= apu2_keys_buttons,
+	.nbuttons	= ARRAY_SIZE(apu2_keys_buttons),
+	.poll_interval	= 100,
+	.rep		= 0,
+	.name		= "apu2-keys",
+};
+
+static const struct amd_fch_gpio_pdata board_apu2 = {
+	.res		= DEFINE_RES_MEM_NAMED(FCH_ACPI_MMIO_BASE + FCH_GPIO_OFFSET,
+					       FCH_GPIO_SIZE,
+					       "apu2-gpio-iomem"),
+	.gpio_num	= ARRAY_SIZE(apu2_gpio_regs),
+	.gpio_reg	= apu2_gpio_regs,
+	.gpio_base	= GPIO_BASE,
+};
+
+/* note: matching works on string prefix, so "apu2" must come before "apu" */
+static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
+
+	/* APU2 w/ legacy bios < 4.0.8 */
+	{
+		.ident		= "apu2",
+		.matches	= {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "APU2")
+		},
+		.driver_data	= (void*)&board_apu2,
+	},
+	/* APU2 w/ legacy bios >= 4.0.8 */
+	{
+		.ident		= "apu2",
+		.matches	= {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "apu2")
+		},
+		.driver_data	= (void*)&board_apu2,
+	},
+	/* APU2 w/ maainline bios */
+	{
+		.ident		= "apu2",
+		.matches	= {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "PC Engines apu2")
+		},
+		.driver_data	= (void*)&board_apu2,
+	},
+
+	/* APU3 w/ legacy bios < 4.0.8 */
+	{
+		.ident		= "apu3",
+		.matches	= {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "APU3")
+		},
+		.driver_data = (void*)&board_apu2,
+	},
+	/* APU3 w/ legacy bios >= 4.0.8 */
+	{
+		.ident       = "apu3",
+		.matches     = {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "apu3")
+		},
+		.driver_data = (void*)&board_apu2,
+	},
+	/* APU3 w/ mainline bios */
+	{
+		.ident       = "apu3",
+		.matches     = {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "PC Engines apu3")
+		},
+		.driver_data = (void*)&board_apu2,
+	},
+
+	/* APU1 */
+	/* not supported yet - the register set is pretty different
+	{
+		.ident       = "apu",
+		.matches     = {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "APU")
+		},
+		.driver_data = (void*)&board_apu1,
+	},
+	*/
+	{}
+};
+
+static struct platform_device *apu_gpio_pdev = NULL;
+static struct platform_device *apu_leds_pdev = NULL;
+static struct platform_device *apu_keys_pdev = NULL;
+
+static int __init apu_gpio_init(void)
+{
+	int rc;
+	const struct dmi_system_id *dmi = dmi_first_match(apu_gpio_dmi_table);
+
+	if (!dmi) {
+		pr_err(KBUILD_MODNAME ": failed to detect apu board via dmi\n");
+		return -ENODEV;
+	}
+
+	pr_info(KBUILD_MODNAME ": registering gpio\n");
+	if (IS_ERR(apu_gpio_pdev = platform_device_register_resndata(
+			NULL,					/* parent */
+			AMD_FCH_GPIO_DRIVER_NAME,		/* name */
+			-1,					/* id */
+			NULL,					/* res */
+			0,					/* res_num */
+			dmi->driver_data,			/* platform_data */
+			sizeof(struct amd_fch_gpio_pdata)))) {
+		pr_err(KBUILD_MODNAME ": failed registering gpio device\n");
+		rc = PTR_ERR(apu_gpio_pdev);
+		goto fail;
+	}
+
+	pr_info(KBUILD_MODNAME ": registering leds\n");
+	if (IS_ERR(apu_leds_pdev = platform_device_register_resndata(
+			NULL,					/* parent */
+			"leds-gpio",				/* driver name */
+			-1,					/* id */
+			NULL,					/* res */
+			0,					/* ren_num */
+			&apu2_leds_pdata,			/* platform data */
+			sizeof(apu2_leds_pdata)))) {
+		pr_err(KBUILD_MODNAME ": failed registering leds device\n");
+		rc = PTR_ERR(apu_leds_pdev);
+		goto fail;
+	}
+
+	pr_info(KBUILD_MODNAME ": registering keys\n");
+	if (IS_ERR(apu_keys_pdev = platform_device_register_resndata(
+			NULL,					/* parent */
+			"gpio-keys-polled",			/* driver name */
+			-1,					/* id */
+			NULL,					/* res */
+			0,					/* res_num */
+			&apu2_keys_pdata,			/* platform_data */
+			sizeof(apu2_keys_pdata)))) {
+		pr_err(KBUILD_MODNAME ": failed registering keys device\n");
+		rc = PTR_ERR(apu_keys_pdev);
+		goto fail;
+	}
+
+	pr_info(KBUILD_MODNAME ": initialized: gpio, leds, keys\n");
+	return 0;
+
+fail:
+	if (!IS_ERR(apu_keys_pdev))
+		platform_device_unregister(apu_keys_pdev);
+	if (!IS_ERR(apu_leds_pdev))
+		platform_device_unregister(apu_leds_pdev);
+	if (!IS_ERR(apu_gpio_pdev))
+		platform_device_unregister(apu_gpio_pdev);
+
+	pr_err(KBUILD_MODNAME ": probe FAILED: %d\n", rc);
+	return rc;
+}
+
+static void __exit apu_gpio_exit(void)
+{
+	if (!IS_ERR(apu_keys_pdev))
+		platform_device_unregister(apu_keys_pdev);
+	if (!IS_ERR(apu_leds_pdev))
+		platform_device_unregister(apu_leds_pdev);
+	if (!IS_ERR(apu_gpio_pdev))
+		platform_device_unregister(apu_gpio_pdev);
+}
+
+module_init(apu_gpio_init);
+module_exit(apu_gpio_exit);
+
+MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
+MODULE_DESCRIPTION("PC Engines APUv2 board GPIO/LED/keys driver");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(dmi, apu_gpio_dmi_table);
+MODULE_ALIAS("platform:apu-board");
+MODULE_SOFTDEP("pre: gpio_amd_fch");
+MODULE_SOFTDEP("pre: gpio_keys");
+MODULE_SOFTDEP("pre: gpio_keys_polled");
-- 
1.9.1

Re: [PATCH 2/2] x86: pcengines apuv2 gpio/leds/keys platform driver

[Andy Shevchenko]

On Thu, Feb 7, 2019 at 7:14 PM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:

Thanks for the patch, my comments below.

> Driver for PCengines APUv2 board that supports GPIOs via AMD PCH
> and attached LEDs and keys.

You should put here why ACPI can't be used for providing a necessary
information.
For example, 'The driver is targeting old platforms that do not have a
proper entry in ACPI tables for these devices. In new version of BIOS
this going to be fixed."

Besides same comments as per other patch in the series, can you tell
why it's not an MFD driver?

> +       depends on KEYBOARD_GPIO
> +       depends on KEYBOARD_GPIO_POLLED
> +       depends on LEDS_GPIO

Why?
Is any of this fatal to have driver not registered?


> +/* TODO
> +   * support apu1 board (different fch, different register layouts
> +   * add spinlocks
> +*/

Either remove this, or fix it properly. TODO is not going to be upstream.

> +#define FCH_ACPI_MMIO_BASE     0xFED80000
> +#define FCH_GPIO_OFFSET                0x1500
> +#define FCH_GPIO_SIZE          0x300

0x0300

> +#define GPIO_BASE              100
> +
> +#define GPIO_LED1              (GPIO_BASE+0)
> +#define GPIO_LED2              (GPIO_BASE+1)
> +#define GPIO_LED3              (GPIO_BASE+2)
> +#define GPIO_MODESW            (GPIO_BASE+3)
> +#define GPIO_SIMSWAP           (GPIO_BASE+4)

Have namespace issues.

> +static struct amd_fch_gpio_reg apu2_gpio_regs[] = {
> +       { 0x44 }, // GPIO_57 -- LED1
> +       { 0x45 }, // GPIO_58 -- LED2
> +       { 0x46 }, // GPIO_59 -- LED3
> +       { 0x59 }, // GPIO_32 -- LED4 -- GE32 -- #modesw
> +       { 0x5A }, // GPIO_33 -- LED5 -- GE33 -- simswap
> +       { 0x42 }, // GPIO_51 -- LED6
> +       { 0x43 }, // GPIO_55 -- LED7
> +       { 0x47 }, // GPIO_64 -- LED8
> +       { 0x48 }, // GPIO_68 -- LED9
> +       { 0x4C }, // GPIO_70 -- LED10

Keep it sorted and consider to use 'gpio-names' property.

> +};

> +static struct platform_device *apu_gpio_pdev = NULL;
> +static struct platform_device *apu_leds_pdev = NULL;
> +static struct platform_device *apu_keys_pdev = NULL;

Redundant assignments.

> +static int __init apu_gpio_init(void)
> +{
> +       int rc;

> +       const struct dmi_system_id *dmi = dmi_first_match(apu_gpio_dmi_table);

Split it to three lines.

const struct ... *id;

id = dmi_first_match(...);
if (!id)

> +       if (!dmi) {
> +               pr_err(KBUILD_MODNAME ": failed to detect apu board via dmi\n");
> +               return -ENODEV;
> +       }

> +                       -1,                                     /* id */

PLATFORM_DEVID_NONE ?

> +fail:
> +       if (!IS_ERR(apu_keys_pdev))
> +               platform_device_unregister(apu_keys_pdev);
> +       if (!IS_ERR(apu_leds_pdev))
> +               platform_device_unregister(apu_leds_pdev);
> +       if (!IS_ERR(apu_gpio_pdev))
> +               platform_device_unregister(apu_gpio_pdev);

So, why the failure of the registration of any of these devices is fatal?

> +}

> +static void __exit apu_gpio_exit(void)
> +{

> +       if (!IS_ERR(apu_keys_pdev))

Redundant.
See commit 99fef587ff98 ("driver core: platform: Respect return code
of platform_device_register_full()") for the details.

> +               platform_device_unregister(apu_keys_pdev);
> +       if (!IS_ERR(apu_leds_pdev))
> +               platform_device_unregister(apu_leds_pdev);
> +       if (!IS_ERR(apu_gpio_pdev))
> +               platform_device_unregister(apu_gpio_pdev);
> +}

> +MODULE_LICENSE("GPL");

License mismatch.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] x86: gpio: AMD G-Series pch gpio platform driver

[Andy Shevchenko]

On Thu, Feb 7, 2019 at 7:14 PM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
>
> GPIO platform driver for the AMD G-series PCH (eg. on GX-412TC)
>
> This driver doesn't registers itself automatically, as it needs to
> be provided with platform specific configuration, provided by some
> board driver setup code.
>
> Didn't implement oftree probing yet, as it's rarely found on x86.

Thanks for the patch, see my comments below.

Overall I have a feeling that this driver can be replaced with
existing generic one where one register per pin is allocated.
Unfortunately, I didn't look deep into this and hope Linus will help
to figure this out.

> @@ -0,0 +1,171 @@
> +/*
> + * GPIO driver for the AMD G series FCH (eg. GX-412TC)
> + *
> + * Copyright (C) 2018 metux IT consult
> + * Author: Enrico Weigelt <info@metux.net>
> + *

> + * SPDX-License-Identifier: GPL+

SPDX should go as a separate first line in a proper format.

> + */

> +// FIXME: add spinlocks

Then fix them and come again.

> +#include <linux/init.h>
> +#include <linux/module.h>

One of them should be present, another one dropped.

> +#define GPIO_BIT_DIR           23
> +#define GPIO_BIT_WRITE         22
> +#define GPIO_BIT_READ          16

Oh, namespace issues.
What about using BIT() macro?

> +
> +

Why two blank lines?

> +static uint32_t *amd_fch_gpio_addr(struct gpio_chip *gc, unsigned gpio)
> +{
> +       struct amd_fch_gpio_priv *priv = gpiochip_get_data(gc);
> +

> +       if (gpio > priv->pdata->gpio_num) {
> +               dev_err(&priv->pdev->dev, "gpio number %d out of range\n", gpio);
> +               return NULL;
> +       }

On which circumstances it may happen?

> +
> +       return priv->base + priv->pdata->gpio_reg[gpio].reg*sizeof(u32);
> +}
> +
> +static int amd_fch_gpio_direction_input(struct gpio_chip *gc, unsigned offset)
> +{

> +       volatile uint32_t *ptr = amd_fch_gpio_addr(gc, offset);

volatile?!

I think you need to use readl()/writel() (or their _relaxed variants) instead.
Same applies for entire code.

> +       if (!ptr) return -EINVAL;

This code has style issues.
Check your entire file.

> +
> +       *ptr &= ~(1 << GPIO_BIT_DIR);
> +       return 0;
> +}

> +static void amd_fch_gpio_dbg_show(struct seq_file *s, struct gpio_chip *gc)
> +{
> +       struct amd_fch_gpio_priv *priv = gpiochip_get_data(gc);
> +       (void)priv;
> +
> +       seq_printf(s, "debug info not implemented yet\n");
> +}

Remove whatever is not implemented and not required to have a stub.

> +static int amd_fch_gpio_request(struct gpio_chip *chip, unsigned gpio_pin)
> +{

> +       if (gpio_pin < chip->ngpio)
> +               return 0;

Is it even possible?

> +
> +       return -EINVAL;
> +}


> +
> +static int amd_fch_gpio_probe(struct platform_device *pdev)
> +{
> +       struct amd_fch_gpio_priv *priv;

> +       struct amd_fch_gpio_pdata *pdata = pdev->dev.platform_data;

We have a helper to get this. platform_get_data() IIRC.

> +       int err;
> +
> +       if (!pdata) {
> +               dev_err(&pdev->dev, "no platform_data\n");
> +               return -ENOENT;
> +       }
> +

> +       if (!(priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL))) {

Should be two lines.

> +               dev_err(&pdev->dev, "failed to allocate priv struct\n");

Noise.

> +               return -ENOMEM;
> +       }
> +

> +       if (IS_ERR(priv->base = devm_ioremap_resource(&pdev->dev, &priv->pdata->res))) {

> +               dev_err(&pdev->dev, "failed to map iomem\n");

Noise (that function will print a message)

> +               return -ENXIO;

Shadowed error code.

> +       }
> +

> +       dev_info(&pdev->dev, "initializing on my own II\n");

Noise.

> +

> +       if (IS_ENABLED(CONFIG_DEBUG_FS)) {

Do you really care?

> +               dev_info(&pdev->dev, "enabling debugfs\n");

Noise.

> +               priv->gc.dbg_show = amd_fch_gpio_dbg_show;
> +       }
> +
> +       platform_set_drvdata(pdev, priv);
> +

> +       err = devm_gpiochip_add_data(&pdev->dev, &priv->gc, priv);
> +       dev_info(&pdev->dev, "probe finished\n");
> +       return err;

return devm_gpiochip_add_data(...);

> +}

> +MODULE_LICENSE("GPL");

License mismatch. I really don't look what 'GPL+' means. OTOH I know
this one corresponds to GPL-2.0+.

> +++ b/include/linux/platform_data/x86/amd-fch-gpio-pdata.h
> @@ -0,0 +1,41 @@
> +/*
> + * AMD FCH gpio driver platform-data
> + *
> + * Copyright (C) 2018 metux IT consult
> + * Author: Enrico Weigelt <info@metux.net>
> + *

> + * SPDX-License-Identifier: GPL

Same comments.

> + */

> +/*

It's not marked as kernel doc.

> + * struct amd_fch_gpio_reg - GPIO register definition
> + * @reg: register index
> + * @name: signal name
> + */
> +struct amd_fch_gpio_reg {
> +    int         reg;
> +    const char* name;
> +};

Isn't this provided by GPIO library? We have so called labels.

> +/*
> + * struct amd_fch_gpio_pdata - GPIO chip platform data
> + * @resource: iomem range
> + * @gpio_reg: array of gpio registers
> + * @gpio_num: number of entries
> + */
> +struct amd_fch_gpio_pdata {
> +    struct resource          res;
> +    int                      gpio_num;
> +    struct amd_fch_gpio_reg *gpio_reg;
> +    int                      gpio_base;
> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] x86: gpio: AMD G-Series pch gpio platform driver

[Enrico Weigelt, metux IT consult]

On 07.02.19 19:06, Andy Shevchenko wrote:

Hi,

> Overall I have a feeling that this driver can be replaced with> existing generic one where one register per pin is allocated.>
Unfortunately, I didn't look deep into this and hope Linus will help> to
figure this out.
this also was my first thought, but i recall the generic one copes w/
devices that have per-flag- instead of per channel-registers
(IOW: all direction flags in one register, all level flags in another).
correct me if I'm wrong ..

I actually considered writing a generic per-register driver, where one
just configures which bit does what. But it didn't feel worth the extra
effort yet. OTOH, if we would have lots of consumers, the situation
would be different.

>> @@ -0,0 +1,171 @@
>> +/*
>> + * GPIO driver for the AMD G series FCH (eg. GX-412TC)
>> + *
>> + * Copyright (C) 2018 metux IT consult
>> + * Author: Enrico Weigelt <info@metux.net>
>> + *
> 
>> + * SPDX-License-Identifier: GPL+
> 
> SPDX should go as a separate first line in a proper format.

Fixed.
Maybe I should write an automatic check for that ;-)

By the way: are there already some tools that actually operate on that ?

>> +#define GPIO_BIT_DIR           23
>> +#define GPIO_BIT_WRITE         22
>> +#define GPIO_BIT_READ          16
> 
> Oh, namespace issues.
> What about using BIT() macro?

Ah, thanks, forgot that.

By the way: would it be a good idea to define a struct w/ bitfields
for the registers, instead of directly doing bitmask operations ?


>> +static uint32_t *amd_fch_gpio_addr(struct gpio_chip *gc, unsigned gpio)
>> +{
>> +       struct amd_fch_gpio_priv *priv = gpiochip_get_data(gc);
>> +
> 
>> +       if (gpio > priv->pdata->gpio_num) {
>> +               dev_err(&priv->pdev->dev, "gpio number %d out of range\n", gpio);
>> +               return NULL;
>> +       }
> 
> On which circumstances it may happen?

hopefully never, but I'm a bit paranoid ;-)
Shall I kick out that check ?

>> +       return priv->base + priv->pdata->gpio_reg[gpio].reg*sizeof(u32);
>> +}
>> +
>> +static int amd_fch_gpio_direction_input(struct gpio_chip *gc, unsigned offset)
>> +{
> 
>> +       volatile uint32_t *ptr = amd_fch_gpio_addr(gc, offset);
> 
> volatile?!
> 
> I think you need to use readl()/writel() (or their _relaxed variants) instead.

I assumed the compiler would already emit the correct code (and not try
any optimizations) if the field is declared volatile. But I'll change it
to readl()/writel().

By the way: do we already have helpers for doing such bit operations on
mmapped registers ? (eg. similar to those in bitops.h)

> Same applies for entire code.
> 
>> +       if (!ptr) return -EINVAL;
> 
> This code has style issues.
> Check your entire file.

Should it be written in two lines - like that ?

    if (!ptr)
        return -EINVAL;

>> +static int amd_fch_gpio_request(struct gpio_chip *chip, unsigned gpio_pin)
>> +{
> 
>> +       if (gpio_pin < chip->ngpio)
>> +               return 0;
> 
> Is it even possible?

Not sure. AFAIK, this function should check whether the requested pin is
available. Feels safer to me having this check.

>> +static int amd_fch_gpio_probe(struct platform_device *pdev)
>> +{
>> +       struct amd_fch_gpio_priv *priv;
> 
>> +       struct amd_fch_gpio_pdata *pdata = pdev->dev.platform_data;
> 
> We have a helper to get this. platform_get_data() IIRC.

found dev_get_platdata(const struct device *dev).

but nothing for working on struct platform_device directly - should we
introduce one ?

>> +       if (IS_ERR(priv->base = devm_ioremap_resource(&pdev->dev, &priv->pdata->res))) {
> 
>> +               dev_err(&pdev->dev, "failed to map iomem\n");
> 
> Noise (that function will print a message)
> 
>> +               return -ENXIO;
> 
> Shadowed error code.

Which one shall I use instead ?

>> +MODULE_LICENSE("GPL");
> 
> License mismatch. I really don't look what 'GPL+' means. OTOH I know
> this one corresponds to GPL-2.0+.

Typo, should have been GPL-2.0+.

>> + * struct amd_fch_gpio_reg - GPIO register definition
>> + * @reg: register index
>> + * @name: signal name
>> + */
>> +struct amd_fch_gpio_reg {
>> +    int         reg;
>> +    const char* name;
>> +};
> 
> Isn't this provided by GPIO library? We have so called labels.

hmm, haven't found a proper struct yet.

struct gpio indeed has a label and two int fields. but it doesn't seem
to be designed for holding register addresses ... using this one here
feels quite abusive. (and a waste of memory, too).

for consistency, I could rename 'name' to 'label', if you wish.


thanks for your review.


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

[PATCH 1/2] x86: gpio: AMD G-Series pch gpio platform driver

[Enrico Weigelt, metux IT consult]

From: "Enrico Weigelt, metux IT consult" <info@metux.net>

GPIO platform driver for the AMD G-series PCH (eg. on GX-412TC)

This driver doesn't registers itself automatically, as it needs to
be provided with platform specific configuration, provided by some
board driver setup code.

Didn't implement oftree probing yet, as it's rarely found on x86.

Cc: linux-gpio@vger.kernel.org
Cc: linus.walleij@linaro.org
Cc: bgolaszewski@baylibre.com
Cc: dvhart@infradead.org
Cc: andy@infradead.org
Cc: platform-driver-x86@vger.kernel.org

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 MAINTAINERS                                        |   7 +
 drivers/gpio/Kconfig                               |  10 ++
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-amd-fch.c                        | 169 +++++++++++++++++++++
 .../linux/platform_data/x86/amd-fch-gpio-pdata.h   |  41 +++++
 5 files changed, 228 insertions(+)
 create mode 100644 drivers/gpio/gpio-amd-fch.c
 create mode 100644 include/linux/platform_data/x86/amd-fch-gpio-pdata.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 8c68de3c..b9bc500 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -766,6 +766,13 @@ S:	Supported
 F:	Documentation/hwmon/fam15h_power
 F:	drivers/hwmon/fam15h_power.c
 
+AMD FCH GPIO DRIVER
+M:	Enrico Weigelt, metux IT consult <info@metux.net>
+L:	linux-gpio@vger.kernel.org
+S:	Maintained
+F:	drivers/gpio/gpio-amd-fch.c
+F:	include/linux/platform_data/x86/amd-fch-gpio-pdata.h
+
 AMD GEODE CS5536 USB DEVICE CONTROLLER DRIVER
 L:	linux-geode@lists.infradead.org (moderated for non-subscribers)
 S:	Orphan
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index b5a2845..a3e47c8 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -654,6 +654,16 @@ config GPIO_LOONGSON1
 	help
 	  Say Y or M here to support GPIO on Loongson1 SoCs.
 
+config GPIO_AMD_FCH
+	tristate "GPIO support for AMD Fusion Controller Hub (G-series SOCs)"
+	select GPIO_GENERIC
+	help
+	  This option enables driver for GPIO on AMDs Fusion Controller Hub,
+	  as found on G-series SOCs (eg. GX-412TC)
+
+	  Note: This driver doesn't registers itself automatically, as it
+	  needs to be provided with platform specific configuration.
+	  (See eg. CONFIG_PCENGINES_APU2.)
 endmenu
 
 menu "Port-mapped I/O GPIO drivers"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 37628f8..bb48fd2 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_GPIO_ADP5520)	+= gpio-adp5520.o
 obj-$(CONFIG_GPIO_ADP5588)	+= gpio-adp5588.o
 obj-$(CONFIG_GPIO_ALTERA)  	+= gpio-altera.o
 obj-$(CONFIG_GPIO_ALTERA_A10SR)	+= gpio-altera-a10sr.o
+obj-$(CONFIG_GPIO_AMD_FCH)	+= gpio-amd-fch.o
 obj-$(CONFIG_GPIO_AMD8111)	+= gpio-amd8111.o
 obj-$(CONFIG_GPIO_AMDPT)	+= gpio-amdpt.o
 obj-$(CONFIG_GPIO_ARIZONA)	+= gpio-arizona.o
diff --git a/drivers/gpio/gpio-amd-fch.c b/drivers/gpio/gpio-amd-fch.c
new file mode 100644
index 0000000..356bb21
--- /dev/null
+++ b/drivers/gpio/gpio-amd-fch.c
@@ -0,0 +1,169 @@
+/*
+ * GPIO driver for the AMD G series FCH (eg. GX-412TC)
+ *
+ * Copyright (C) 2018 metux IT consult
+ * Author: Enrico Weigelt <info@metux.net>
+ *
+ * SPDX-License-Identifier: GPL+
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/gpio/driver.h>
+#include <linux/platform_data/x86/amd-fch-gpio-pdata.h>
+
+
+#define GPIO_BIT_DIR		23
+#define GPIO_BIT_WRITE		22
+#define GPIO_BIT_READ		16
+
+
+struct amd_fch_gpio_priv {
+	struct platform_device		*pdev;
+	struct gpio_chip		gc;
+	void __iomem			*base;
+	struct amd_fch_gpio_pdata	*pdata;
+};
+
+static uint32_t *amd_fch_gpio_addr(struct gpio_chip *gc, unsigned gpio)
+{
+	struct amd_fch_gpio_priv *priv = gpiochip_get_data(gc);
+
+	if (gpio > priv->pdata->gpio_num) {
+		dev_err(&priv->pdev->dev, "gpio number %d out of range\n", gpio);
+		return NULL;
+	}
+
+	return priv->base + priv->pdata->gpio_reg[gpio].reg*sizeof(u32);
+}
+
+static int amd_fch_gpio_direction_input(struct gpio_chip *gc, unsigned offset)
+{
+	volatile uint32_t *ptr = amd_fch_gpio_addr(gc, offset);
+	if (!ptr) return -EINVAL;
+
+	*ptr &= ~(1 << GPIO_BIT_DIR);
+	return 0;
+}
+
+static int amd_fch_gpio_direction_output(struct gpio_chip *gc, unsigned gpio, int value)
+{
+	volatile uint32_t *ptr = amd_fch_gpio_addr(gc, gpio);
+	if (!ptr) return -EINVAL;
+
+	*ptr |= (1 << GPIO_BIT_DIR);
+	return 0;
+}
+
+static int amd_fch_gpio_get_direction(struct gpio_chip *gc, unsigned gpio)
+{
+	volatile uint32_t *ptr = amd_fch_gpio_addr(gc, gpio);
+	if (!ptr) return -EINVAL;
+
+	return (*ptr >> GPIO_BIT_DIR) & 1;
+}
+
+static void amd_fch_gpio_set(struct gpio_chip *gc, unsigned gpio, int value)
+{
+	volatile uint32_t *ptr = amd_fch_gpio_addr(gc, gpio);
+	if (!ptr) return;
+
+	if (value)
+		*ptr |= (1 << GPIO_BIT_WRITE);
+	else
+		*ptr &= ~(1 << GPIO_BIT_WRITE);
+}
+
+static int amd_fch_gpio_get(struct gpio_chip *gc, unsigned offset)
+{
+	volatile uint32_t *ptr = amd_fch_gpio_addr(gc, offset);
+	if (!ptr) return -EINVAL;
+
+	return ((*ptr) >> GPIO_BIT_READ) & 1;
+}
+
+static void amd_fch_gpio_dbg_show(struct seq_file *s, struct gpio_chip *gc)
+{
+	struct amd_fch_gpio_priv *priv = gpiochip_get_data(gc);
+	(void)priv;
+
+	seq_printf(s, "debug info not implemented yet\n");
+}
+
+static int amd_fch_gpio_request(struct gpio_chip *chip, unsigned gpio_pin)
+{
+	if (gpio_pin < chip->ngpio)
+		return 0;
+
+	return -EINVAL;
+}
+
+static int amd_fch_gpio_probe(struct platform_device *pdev)
+{
+	struct amd_fch_gpio_priv *priv;
+	struct amd_fch_gpio_pdata *pdata = pdev->dev.platform_data;
+	int err;
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "no platform_data\n");
+		return -ENOENT;
+	}
+
+	if (!(priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL))) {
+		dev_err(&pdev->dev, "failed to allocate priv struct\n");
+		return -ENOMEM;
+	}
+
+	priv->pdata	= pdata;
+	priv->pdev	= pdev;
+
+	priv->gc.owner			= THIS_MODULE;
+	priv->gc.parent			= &pdev->dev;
+	priv->gc.label			= dev_name(&pdev->dev);
+	priv->gc.base			= priv->pdata->gpio_base;
+	priv->gc.ngpio			= priv->pdata->gpio_num;
+	priv->gc.request		= amd_fch_gpio_request;
+	priv->gc.direction_input	= amd_fch_gpio_direction_input;
+	priv->gc.direction_output	= amd_fch_gpio_direction_output;
+	priv->gc.get_direction		= amd_fch_gpio_get_direction;
+	priv->gc.get			= amd_fch_gpio_get;
+	priv->gc.set			= amd_fch_gpio_set;
+
+	spin_lock_init(&priv->gc.bgpio_lock);
+
+	if (IS_ERR(priv->base = devm_ioremap_resource(&pdev->dev, &priv->pdata->res))) {
+		dev_err(&pdev->dev, "failed to map iomem\n");
+		return -ENXIO;
+	}
+
+	dev_info(&pdev->dev, "initializing on my own II\n");
+
+	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
+		dev_info(&pdev->dev, "enabling debugfs\n");
+		priv->gc.dbg_show = amd_fch_gpio_dbg_show;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	err = devm_gpiochip_add_data(&pdev->dev, &priv->gc, priv);
+	dev_info(&pdev->dev, "probe finished\n");
+	return err;
+}
+
+static struct platform_driver amd_fch_gpio_driver = {
+	.driver = {
+		.name = AMD_FCH_GPIO_DRIVER_NAME,
+	},
+	.probe = amd_fch_gpio_probe,
+};
+
+module_platform_driver(amd_fch_gpio_driver);
+
+MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
+MODULE_DESCRIPTION("AMD G-series FCH GPIO driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:gpio_amd_fch");
diff --git a/include/linux/platform_data/x86/amd-fch-gpio-pdata.h b/include/linux/platform_data/x86/amd-fch-gpio-pdata.h
new file mode 100644
index 0000000..68c1730
--- /dev/null
+++ b/include/linux/platform_data/x86/amd-fch-gpio-pdata.h
@@ -0,0 +1,41 @@
+/*
+ * AMD FCH gpio driver platform-data
+ *
+ * Copyright (C) 2018 metux IT consult
+ * Author: Enrico Weigelt <info@metux.net>
+ *
+ * SPDX-License-Identifier: GPL
+ */
+
+#ifndef AMD_FCH_PDATA_H
+#define AMD_FCH_PDATA_H
+
+
+#include <linux/ioport.h>
+
+#define AMD_FCH_GPIO_DRIVER_NAME "gpio_amd_fch"
+
+/*
+ * struct amd_fch_gpio_reg - GPIO register definition
+ * @reg: register index
+ * @name: signal name
+ */
+struct amd_fch_gpio_reg {
+    int         reg;
+    const char* name;
+};
+
+/*
+ * struct amd_fch_gpio_pdata - GPIO chip platform data
+ * @resource: iomem range
+ * @gpio_reg: array of gpio registers
+ * @gpio_num: number of entries
+ */
+struct amd_fch_gpio_pdata {
+    struct resource          res;
+    int                      gpio_num;
+    struct amd_fch_gpio_reg *gpio_reg;
+    int                      gpio_base;
+};
+
+#endif /* AMD_FCH_PDATA_H */
-- 
1.9.1

[PATCH 2/2] x86: pcengines apuv2 gpio/leds/keys platform driver

[Enrico Weigelt, metux IT consult]

From: "Enrico Weigelt, metux IT consult" <info@metux.net>

Driver for PCengines APUv2 board that supports GPIOs via AMD PCH
and attached LEDs and keys.

Cc: linux-gpio@vger.kernel.org
Cc: linus.walleij@linaro.org
Cc: bgolaszewski@baylibre.com
Cc: dvhart@infradead.org
Cc: andy@infradead.org
Cc: platform-driver-x86@vger.kernel.org

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 MAINTAINERS                            |   5 +
 drivers/platform/x86/Kconfig           |   9 ++
 drivers/platform/x86/Makefile          |   1 +
 drivers/platform/x86/pcengines-apuv2.c | 263 +++++++++++++++++++++++++++++++++
 4 files changed, 278 insertions(+)
 create mode 100644 drivers/platform/x86/pcengines-apuv2.c

diff --git a/MAINTAINERS b/MAINTAINERS
index b9bc500..9abcc47 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11515,6 +11515,11 @@ F:	lib/parman.c
 F:	lib/test_parman.c
 F:	include/linux/parman.h
 
+PC ENGINES APU BOARD DRIVER
+M:	Enrico Weigelt, metux IT consult <info@metux.net>
+S:	Maintained
+F:	drivers/platform/x86/pcengines-apuv2.c
+
 PC87360 HARDWARE MONITORING DRIVER
 M:	Jim Cromie <jim.cromie@gmail.com>
 L:	linux-hwmon@vger.kernel.org
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index b5e9db8..a77d705 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1303,6 +1303,15 @@ config HUAWEI_WMI
 	  To compile this driver as a module, choose M here: the module
 	  will be called huawei-wmi.
 
+config PCENGINES_APU2
+	tristate "LEDs and buttons driver for PC Engines APUv2 board"
+	depends on GPIO_AMD_FCH
+	depends on KEYBOARD_GPIO
+	depends on KEYBOARD_GPIO_POLLED
+	depends on LEDS_GPIO
+	---help---
+	  This options adds APUv2 board support for LEDs and keys
+
 endif # X86_PLATFORM_DEVICES
 
 config PMC_ATOM
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index ce8da26..86cb766 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -96,3 +96,4 @@ obj-$(CONFIG_INTEL_TURBO_MAX_3) += intel_turbo_max_3.o
 obj-$(CONFIG_INTEL_CHTDC_TI_PWRBTN)	+= intel_chtdc_ti_pwrbtn.o
 obj-$(CONFIG_I2C_MULTI_INSTANTIATE)	+= i2c-multi-instantiate.o
 obj-$(CONFIG_INTEL_ATOMISP2_PM)	+= intel_atomisp2_pm.o
+obj-$(CONFIG_PCENGINES_APU2)	+= pcengines-apuv2.o
diff --git a/drivers/platform/x86/pcengines-apuv2.c b/drivers/platform/x86/pcengines-apuv2.c
new file mode 100644
index 0000000..9bb89d6
--- /dev/null
+++ b/drivers/platform/x86/pcengines-apuv2.c
@@ -0,0 +1,263 @@
+/*
+ * PC-Engines APUv2 board platform driver for gpio buttons and LEDs
+ *
+ * Copyright (C) 2018 metux IT consult
+ * Author: Enrico Weigelt <info@metux.net>
+ */
+
+// SPDX-License-Identifier: GPL+
+
+#include <linux/dmi.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/gpio_keys.h>
+#include <linux/input.h>
+#include <linux/platform_data/x86/amd-fch-gpio-pdata.h>
+
+/* TODO
+   * support apu1 board (different fch, different register layouts
+   * add spinlocks
+*/
+
+#define FCH_ACPI_MMIO_BASE	0xFED80000
+#define FCH_GPIO_OFFSET		0x1500
+#define FCH_GPIO_SIZE		0x300
+
+#define GPIO_BASE		100
+
+#define GPIO_LED1		(GPIO_BASE+0)
+#define GPIO_LED2		(GPIO_BASE+1)
+#define GPIO_LED3		(GPIO_BASE+2)
+#define GPIO_MODESW		(GPIO_BASE+3)
+#define GPIO_SIMSWAP		(GPIO_BASE+4)
+
+struct board_data {
+	const char		*name;
+	struct resource		res;
+	int			gpio_num;
+	int			gpio_base;
+	struct amd_fch_gpio_reg	*gpio_regs;
+};
+
+static const struct gpio_led apu2_leds[] /* __initconst */ = {
+	{ .name = "apu:green:1", .gpio = GPIO_LED1, .active_low = 1, },
+	{ .name = "apu:green:2", .gpio = GPIO_LED2, .active_low = 1, },
+	{ .name = "apu:green:3", .gpio = GPIO_LED3, .active_low = 1, }
+};
+
+static const struct gpio_led_platform_data apu2_leds_pdata /* __initconst */ = {
+	.num_leds	= ARRAY_SIZE(apu2_leds),
+	.leds		= apu2_leds,
+};
+
+static struct amd_fch_gpio_reg apu2_gpio_regs[] = {
+	{ 0x44 }, // GPIO_57 -- LED1
+	{ 0x45 }, // GPIO_58 -- LED2
+	{ 0x46 }, // GPIO_59 -- LED3
+	{ 0x59 }, // GPIO_32 -- LED4 -- GE32 -- #modesw
+	{ 0x5A }, // GPIO_33 -- LED5 -- GE33 -- simswap
+	{ 0x42 }, // GPIO_51 -- LED6
+	{ 0x43 }, // GPIO_55 -- LED7
+	{ 0x47 }, // GPIO_64 -- LED8
+	{ 0x48 }, // GPIO_68 -- LED9
+	{ 0x4C }, // GPIO_70 -- LED10
+};
+
+static struct gpio_keys_button apu2_keys_buttons[] = {
+	{
+		.code			= KEY_A,
+		.gpio			= GPIO_MODESW,
+		.active_low		= 1,
+		.desc			= "modeswitch",
+		.type			= EV_KEY, /* or EV_SW ? */
+		.debounce_interval	= 10,
+		.value			= 1,
+	}
+};
+
+static const struct gpio_keys_platform_data apu2_keys_pdata = {
+	.buttons	= apu2_keys_buttons,
+	.nbuttons	= ARRAY_SIZE(apu2_keys_buttons),
+	.poll_interval	= 100,
+	.rep		= 0,
+	.name		= "apu2-keys",
+};
+
+static const struct amd_fch_gpio_pdata board_apu2 = {
+	.res		= DEFINE_RES_MEM_NAMED(FCH_ACPI_MMIO_BASE + FCH_GPIO_OFFSET,
+					       FCH_GPIO_SIZE,
+					       "apu2-gpio-iomem"),
+	.gpio_num	= ARRAY_SIZE(apu2_gpio_regs),
+	.gpio_reg	= apu2_gpio_regs,
+	.gpio_base	= GPIO_BASE,
+};
+
+/* note: matching works on string prefix, so "apu2" must come before "apu" */
+static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
+
+	/* APU2 w/ legacy bios < 4.0.8 */
+	{
+		.ident		= "apu2",
+		.matches	= {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "APU2")
+		},
+		.driver_data	= (void*)&board_apu2,
+	},
+	/* APU2 w/ legacy bios >= 4.0.8 */
+	{
+		.ident		= "apu2",
+		.matches	= {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "apu2")
+		},
+		.driver_data	= (void*)&board_apu2,
+	},
+	/* APU2 w/ maainline bios */
+	{
+		.ident		= "apu2",
+		.matches	= {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "PC Engines apu2")
+		},
+		.driver_data	= (void*)&board_apu2,
+	},
+
+	/* APU3 w/ legacy bios < 4.0.8 */
+	{
+		.ident		= "apu3",
+		.matches	= {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "APU3")
+		},
+		.driver_data = (void*)&board_apu2,
+	},
+	/* APU3 w/ legacy bios >= 4.0.8 */
+	{
+		.ident       = "apu3",
+		.matches     = {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "apu3")
+		},
+		.driver_data = (void*)&board_apu2,
+	},
+	/* APU3 w/ mainline bios */
+	{
+		.ident       = "apu3",
+		.matches     = {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "PC Engines apu3")
+		},
+		.driver_data = (void*)&board_apu2,
+	},
+
+	/* APU1 */
+	/* not supported yet - the register set is pretty different
+	{
+		.ident       = "apu",
+		.matches     = {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "APU")
+		},
+		.driver_data = (void*)&board_apu1,
+	},
+	*/
+	{}
+};
+
+static struct platform_device *apu_gpio_pdev = NULL;
+static struct platform_device *apu_leds_pdev = NULL;
+static struct platform_device *apu_keys_pdev = NULL;
+
+static int __init apu_gpio_init(void)
+{
+	int rc;
+	const struct dmi_system_id *dmi = dmi_first_match(apu_gpio_dmi_table);
+
+	if (!dmi) {
+		pr_err(KBUILD_MODNAME ": failed to detect apu board via dmi\n");
+		return -ENODEV;
+	}
+
+	pr_info(KBUILD_MODNAME ": registering gpio\n");
+	if (IS_ERR(apu_gpio_pdev = platform_device_register_resndata(
+			NULL,					/* parent */
+			AMD_FCH_GPIO_DRIVER_NAME,		/* name */
+			-1,					/* id */
+			NULL,					/* res */
+			0,					/* res_num */
+			dmi->driver_data,			/* platform_data */
+			sizeof(struct amd_fch_gpio_pdata)))) {
+		pr_err(KBUILD_MODNAME ": failed registering gpio device\n");
+		rc = PTR_ERR(apu_gpio_pdev);
+		goto fail;
+	}
+
+	pr_info(KBUILD_MODNAME ": registering leds\n");
+	if (IS_ERR(apu_leds_pdev = platform_device_register_resndata(
+			NULL,					/* parent */
+			"leds-gpio",				/* driver name */
+			-1,					/* id */
+			NULL,					/* res */
+			0,					/* ren_num */
+			&apu2_leds_pdata,			/* platform data */
+			sizeof(apu2_leds_pdata)))) {
+		pr_err(KBUILD_MODNAME ": failed registering leds device\n");
+		rc = PTR_ERR(apu_leds_pdev);
+		goto fail;
+	}
+
+	pr_info(KBUILD_MODNAME ": registering keys\n");
+	if (IS_ERR(apu_keys_pdev = platform_device_register_resndata(
+			NULL,					/* parent */
+			"gpio-keys-polled",			/* driver name */
+			-1,					/* id */
+			NULL,					/* res */
+			0,					/* res_num */
+			&apu2_keys_pdata,			/* platform_data */
+			sizeof(apu2_keys_pdata)))) {
+		pr_err(KBUILD_MODNAME ": failed registering keys device\n");
+		rc = PTR_ERR(apu_keys_pdev);
+		goto fail;
+	}
+
+	pr_info(KBUILD_MODNAME ": initialized: gpio, leds, keys\n");
+	return 0;
+
+fail:
+	if (!IS_ERR(apu_keys_pdev))
+		platform_device_unregister(apu_keys_pdev);
+	if (!IS_ERR(apu_leds_pdev))
+		platform_device_unregister(apu_leds_pdev);
+	if (!IS_ERR(apu_gpio_pdev))
+		platform_device_unregister(apu_gpio_pdev);
+
+	pr_err(KBUILD_MODNAME ": probe FAILED: %d\n", rc);
+	return rc;
+}
+
+static void __exit apu_gpio_exit(void)
+{
+	if (!IS_ERR(apu_keys_pdev))
+		platform_device_unregister(apu_keys_pdev);
+	if (!IS_ERR(apu_leds_pdev))
+		platform_device_unregister(apu_leds_pdev);
+	if (!IS_ERR(apu_gpio_pdev))
+		platform_device_unregister(apu_gpio_pdev);
+}
+
+module_init(apu_gpio_init);
+module_exit(apu_gpio_exit);
+
+MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
+MODULE_DESCRIPTION("PC Engines APUv2 board GPIO/LED/keys driver");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(dmi, apu_gpio_dmi_table);
+MODULE_ALIAS("platform:apu-board");
+MODULE_SOFTDEP("pre: gpio_amd_fch");
+MODULE_SOFTDEP("pre: gpio_keys");
+MODULE_SOFTDEP("pre: gpio_keys_polled");
-- 
1.9.1

Re: [PATCH 2/2] x86: pcengines apuv2 gpio/leds/keys platform driver

[Linus Walleij]

On Fri, Feb 8, 2019 at 2:16 AM Enrico Weigelt, metux IT consult
<lkml@metux.net> wrote:

> From: "Enrico Weigelt, metux IT consult" <info@metux.net>
>
> Driver for PCengines APUv2 board that supports GPIOs via AMD PCH
> and attached LEDs and keys.
>
> Cc: linux-gpio@vger.kernel.org
> Cc: linus.walleij@linaro.org
> Cc: bgolaszewski@baylibre.com
> Cc: dvhart@infradead.org
> Cc: andy@infradead.org
> Cc: platform-driver-x86@vger.kernel.org
>
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

Andy can provide more details on this patch here are some quick
remarks:

> +#define GPIO_BASE              100
> +
> +#define GPIO_LED1              (GPIO_BASE+0)
> +#define GPIO_LED2              (GPIO_BASE+1)
> +#define GPIO_LED3              (GPIO_BASE+2)
> +#define GPIO_MODESW            (GPIO_BASE+3)
> +#define GPIO_SIMSWAP           (GPIO_BASE+4)

Instead of hardcoding the GPIO base and offsets like this, use:

#include <linux/gpio/machine.h>

and define a descriptor table using the name of your gpiochip.
There should be examples of other board quirks doing this.
I have already patched gpio-leds.c to accept LEDs from
descriptor tables, see commit
commit 45d4c6de4e497e5b0026c77044ae5fcddf8fecd8
"leds: gpio: Try to lookup gpiod from device"

Yours,
Linus Walleij

Re: [PATCH 2/2] x86: pcengines apuv2 gpio/leds/keys platform driver

[Andy Shevchenko]

On Fri, Feb 8, 2019 at 4:31 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Feb 8, 2019 at 2:16 AM Enrico Weigelt, metux IT consult
> <lkml@metux.net> wrote:

> Andy can provide more details on this patch here are some quick
> remarks:

Already done this on the first submission of the same patch.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] x86: pcengines apuv2 gpio/leds/keys platform driver

[Enrico Weigelt, metux IT consult]

On 08.02.19 15:30, Linus Walleij wrote:

Hi,

> Instead of hardcoding the GPIO base and offsets like this, use:
> 
> #include <linux/gpio/machine.h>
> 
> and define a descriptor table using the name of your gpiochip.
> There should be examples of other board quirks doing this.
> I have already patched gpio-leds.c to accept LEDs from
> descriptor tables, see commit
> commit 45d4c6de4e497e5b0026c77044ae5fcddf8fecd8
> "leds: gpio: Try to lookup gpiod from device"

Still trying to understand how that actually works ...

I'm now defining the leds pdata and gpio mapping this way:

static const struct gpio_led apu2_leds[] = {
    { .name = "apu:green:1" },
    { .name = "apu:green:2" },
    { .name = "apu:green:3" }
};

struct gpiod_lookup_table gpios_led_table[] = {
    .dev_id = "leds-gpio.0",
    .table = {
        GPIO_LOOKUP_IDX("gpio.0", 0, "led", 0, GPIO_ACTIVE_LOW),
        GPIO_LOOKUP_IDX("gpio.0", 1, "led", 1, GPIO_ACTIVE_LOW),
        GPIO_LOOKUP_IDX("gpio.0", 2, "led", 2, GPIO_ACTIVE_LOW),
    }
};

But unsure now to determine the correct names for dev_id (the
leds-gpio instance ?) and the gpio chip. In the example, these
seem to be autogenerated - how can I retrieve them from my
actual devices ?

By the way: does that also work with gpio-keys-polled ?


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: [PATCH 2/2] x86: pcengines apuv2 gpio/leds/keys platform driver

[Linus Walleij]

On Mon, Feb 11, 2019 at 11:39 AM Enrico Weigelt, metux IT consult
<lkml@metux.net> wrote:

> struct gpiod_lookup_table gpios_led_table[] = {
>     .dev_id = "leds-gpio.0",
>     .table = {
>         GPIO_LOOKUP_IDX("gpio.0", 0, "led", 0, GPIO_ACTIVE_LOW),
>         GPIO_LOOKUP_IDX("gpio.0", 1, "led", 1, GPIO_ACTIVE_LOW),
>         GPIO_LOOKUP_IDX("gpio.0", 2, "led", 2, GPIO_ACTIVE_LOW),
>     }
> };
>
> But unsure now to determine the correct names for dev_id (the
> leds-gpio instance ?) and the gpio chip. In the example, these
> seem to be autogenerated - how can I retrieve them from my
> actual devices ?

It is a bit tricky.

For the dev_id you need to be aware of the following from
<linux/platform_device.h>:
#define PLATFORM_DEVID_NONE     (-1)
#define PLATFORM_DEVID_AUTO     (-2)

If the platform device has .id set to -1 it will be just "leds-gpio",
if it is -2 it will be whatever, take a chance on .0 or ideally fix
it up. Any positive number like .id = 4 becomes "leds-gpio.4".

So figure out the .id field on the platform device.

Yours,
Linus Walleij

Re: [PATCH 2/2] x86: pcengines apuv2 gpio/leds/keys platform driver

[Enrico Weigelt, metux IT consult]

On 13.02.19 10:35, Linus Walleij wrote:

> For the dev_id you need to be aware of the following from> <linux/platform_device.h>:> #define PLATFORM_DEVID_NONE     (-1)>
#define PLATFORM_DEVID_AUTO     (-2)>> If the platform device has .id
set to -1 it will be just "leds-gpio",> if it is -2 it will be whatever,
take a chance on .0 or ideally fix> it up. Any positive number like .id
= 4 becomes "leds-gpio.4".> > So figure out the .id field on the
platform device.

Hacked up some debuging into the lookup code and found that it's
just "leds-gpio" and "gpio-keys-polled" in my case. I've just got one
instance of each right now and used PLATFORM_DEVID_NONE.

--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

APUv2/v3 board support V2

[Enrico Weigelt, metux IT consult]

Hi folks,


this is version 2 of my APUv2/APUv3 series. Heavily reworked,
hope I've now addressed all issues of the last one.


Major changes are:
 * using gpiod_lookup_table instead of hardcoded gpio numbers
 * moved the PCHs gpio register definitions to the gpio drivers'
   header (but keeping the actual assignments in the board driver,
   as they're board speficic)
 * moved IO resource definition from board to gpio driver
   (the FCHs mmio base address could be changed, but nobody
   seems to ever do it)
 * added gpio line names
 * moved the gpio driver's pdata definitions to::
   include/linux/platform_data/gpio/gpio-amd-fch.h
 * reduced some code redundancies and various cleanups

Possibly debatable:
 * does is the assigned key code fit well ? (KEY_SETUP seemed the
   best candidate to me, but I'm open to suggestions)
 * should the pch gpio become a sub-device of some toplevel PCH
   device ? (which then could also maintain the base address, etc)

Note: the keyboard device needs separate patch for gpiod_lookup_table
support in gpio-keys-polled driver. I'll sent it separately.


--mtx


-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

[PATCH 2/2] x86: pcengines apuv2 gpio/leds/keys platform driver

[Enrico Weigelt, metux IT consult]

Driver for PCengines APUv2 board's front LEDs and Button,
which are attached to AMD PCH GPIOs. Due to lack of dedicated
ACPI entry, detecting the board via DMI.

Cc: linux-gpio@vger.kernel.org
Cc: linus.walleij@linaro.org
Cc: bgolaszewski@baylibre.com
Cc: dvhart@infradead.org
Cc: andy@infradead.org
Cc: platform-driver-x86@vger.kernel.org

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 MAINTAINERS                            |   5 +
 drivers/platform/x86/Kconfig           |  13 ++
 drivers/platform/x86/Makefile          |   1 +
 drivers/platform/x86/pcengines-apuv2.c | 259 +++++++++++++++++++++++++++++++++
 4 files changed, 278 insertions(+)
 create mode 100644 drivers/platform/x86/pcengines-apuv2.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d286e7d..d39e029 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11522,6 +11522,11 @@ F:	lib/parman.c
 F:	lib/test_parman.c
 F:	include/linux/parman.h
 
+PC ENGINES APU BOARD DRIVER
+M:	Enrico Weigelt, metux IT consult <info@metux.net>
+S:	Maintained
+F:	drivers/platform/x86/pcengines-apuv2.c
+
 PC87360 HARDWARE MONITORING DRIVER
 M:	Jim Cromie <jim.cromie@gmail.com>
 L:	linux-hwmon@vger.kernel.org
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index b5e9db8..10e78ad 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1303,6 +1303,19 @@ config HUAWEI_WMI
 	  To compile this driver as a module, choose M here: the module
 	  will be called huawei-wmi.
 
+config PCENGINES_APU2
+	tristate "PC Engines APUv2/3 front button and LEDs driver"
+	select GPIO_AMD_FCH
+	select KEYBOARD_GPIO
+	select KEYBOARD_GPIO_POLLED
+	select LEDS_GPIO
+	help
+	  This driver provides support for the front button and LEDs on
+	  PC Engines APUv2/APUv3 board.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pcengines-apuv2.
+
 endif # X86_PLATFORM_DEVICES
 
 config PMC_ATOM
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index ce8da26..86cb766 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -96,3 +96,4 @@ obj-$(CONFIG_INTEL_TURBO_MAX_3) += intel_turbo_max_3.o
 obj-$(CONFIG_INTEL_CHTDC_TI_PWRBTN)	+= intel_chtdc_ti_pwrbtn.o
 obj-$(CONFIG_I2C_MULTI_INSTANTIATE)	+= i2c-multi-instantiate.o
 obj-$(CONFIG_INTEL_ATOMISP2_PM)	+= intel_atomisp2_pm.o
+obj-$(CONFIG_PCENGINES_APU2)	+= pcengines-apuv2.o
diff --git a/drivers/platform/x86/pcengines-apuv2.c b/drivers/platform/x86/pcengines-apuv2.c
new file mode 100644
index 0000000..a8e8b0f
--- /dev/null
+++ b/drivers/platform/x86/pcengines-apuv2.c
@@ -0,0 +1,259 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * PC-Engines APUv2/APUv3 board platform driver
+ * for gpio buttons and LEDs
+ *
+ * Copyright (C) 2018 metux IT consult
+ * Author: Enrico Weigelt <info@metux.net>
+ */
+
+#include <linux/dmi.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/gpio_keys.h>
+#include <linux/gpio/machine.h>
+#include <linux/input.h>
+#include <linux/platform_data/gpio/gpio-amd-fch.h>
+
+/*
+ * NOTE: this driver only supports APUv2/3 - not APUv1, as this one
+ * has completely different register layouts
+ */
+
+/* register mappings */
+#define APU2_GPIO_REG_LED1		AMD_FCH_GPIO_REG_GPIO57
+#define APU2_GPIO_REG_LED2		AMD_FCH_GPIO_REG_GPIO58
+#define APU2_GPIO_REG_LED3		AMD_FCH_GPIO_REG_GPIO59_DEVSLP1
+#define APU2_GPIO_REG_MODESW		AMD_FCH_GPIO_REG_GPIO32_GE1
+#define APU2_GPIO_REG_SIMSWAP		AMD_FCH_GPIO_REG_GPIO33_GE2
+
+/* order in which the gpio lines are defined in the register list */
+#define APU2_GPIO_LINE_LED1		0
+#define APU2_GPIO_LINE_LED2		1
+#define APU2_GPIO_LINE_LED3		2
+#define APU2_GPIO_LINE_MODESW		3
+#define APU2_GPIO_LINE_SIMSWAP		4
+
+/* gpio device */
+
+static int apu2_gpio_regs[] = {
+	[APU2_GPIO_LINE_LED1]		= APU2_GPIO_REG_LED1,
+	[APU2_GPIO_LINE_LED2]		= APU2_GPIO_REG_LED2,
+	[APU2_GPIO_LINE_LED3]		= APU2_GPIO_REG_LED3,
+	[APU2_GPIO_LINE_MODESW]		= APU2_GPIO_REG_MODESW,
+	[APU2_GPIO_LINE_SIMSWAP]	= APU2_GPIO_REG_SIMSWAP,
+};
+
+static const char * const apu2_gpio_names[] = {
+	[APU2_GPIO_LINE_LED1]		= "front-led1",
+	[APU2_GPIO_LINE_LED2]		= "front-led2",
+	[APU2_GPIO_LINE_LED3]		= "front-led3",
+	[APU2_GPIO_LINE_MODESW]		= "front-button",
+	[APU2_GPIO_LINE_SIMSWAP]	= "simswap",
+};
+
+static const struct amd_fch_gpio_pdata board_apu2 = {
+	.gpio_num	= ARRAY_SIZE(apu2_gpio_regs),
+	.gpio_reg	= apu2_gpio_regs,
+	.gpio_names	= apu2_gpio_names,
+};
+
+/* gpio leds device */
+
+static const struct gpio_led apu2_leds[] = {
+	{ .name = "apu:green:1" },
+	{ .name = "apu:green:2" },
+	{ .name = "apu:green:3" }
+};
+
+static const struct gpio_led_platform_data apu2_leds_pdata = {
+	.num_leds	= ARRAY_SIZE(apu2_leds),
+	.leds		= apu2_leds,
+};
+
+struct gpiod_lookup_table gpios_led_table = {
+	.dev_id = "leds-gpio",
+	.table = {
+		GPIO_LOOKUP_IDX(AMD_FCH_GPIO_DRIVER_NAME, APU2_GPIO_LINE_LED1, NULL, 0, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX(AMD_FCH_GPIO_DRIVER_NAME, APU2_GPIO_LINE_LED2, NULL, 1, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX(AMD_FCH_GPIO_DRIVER_NAME, APU2_GPIO_LINE_LED3, NULL, 2, GPIO_ACTIVE_LOW),
+	}
+};
+
+/* gpio keyboard device */
+
+static struct gpio_keys_button apu2_keys_buttons[] = {
+	{
+		.code			= KEY_SETUP,
+		.active_low		= 1,
+		.desc			= "front button",
+		.type			= EV_KEY,
+		.debounce_interval	= 10,
+		.value			= 1,
+	},
+};
+
+static const struct gpio_keys_platform_data apu2_keys_pdata = {
+	.buttons	= apu2_keys_buttons,
+	.nbuttons	= ARRAY_SIZE(apu2_keys_buttons),
+	.poll_interval	= 100,
+	.rep		= 0,
+	.name		= "apu2-keys",
+};
+
+struct gpiod_lookup_table gpios_key_table = {
+	.dev_id = "gpio-keys-polled",
+	.table = {
+		GPIO_LOOKUP_IDX(AMD_FCH_GPIO_DRIVER_NAME, APU2_GPIO_LINE_MODESW, NULL, 0, GPIO_ACTIVE_LOW),
+	}
+};
+
+/* board setup */
+
+/* note: matching works on string prefix, so "apu2" must come before "apu" */
+static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
+
+	/* APU2 w/ legacy bios < 4.0.8 */
+	{
+		.ident		= "apu2",
+		.matches	= {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "APU2")
+		},
+		.driver_data	= (void *)&board_apu2,
+	},
+	/* APU2 w/ legacy bios >= 4.0.8 */
+	{
+		.ident		= "apu2",
+		.matches	= {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "apu2")
+		},
+		.driver_data	= (void *)&board_apu2,
+	},
+	/* APU2 w/ maainline bios */
+	{
+		.ident		= "apu2",
+		.matches	= {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "PC Engines apu2")
+		},
+		.driver_data	= (void *)&board_apu2,
+	},
+
+	/* APU3 w/ legacy bios < 4.0.8 */
+	{
+		.ident		= "apu3",
+		.matches	= {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "APU3")
+		},
+		.driver_data = (void *)&board_apu2,
+	},
+	/* APU3 w/ legacy bios >= 4.0.8 */
+	{
+		.ident       = "apu3",
+		.matches     = {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "apu3")
+		},
+		.driver_data = (void *)&board_apu2,
+	},
+	/* APU3 w/ mainline bios */
+	{
+		.ident       = "apu3",
+		.matches     = {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "PC Engines apu3")
+		},
+		.driver_data = (void *)&board_apu2,
+	},
+	{}
+};
+
+static struct platform_device *apu_gpio_pdev;
+static struct platform_device *apu_leds_pdev;
+static struct platform_device *apu_keys_pdev;
+
+static struct platform_device * __init apu_create_pdev(
+	const char* name,
+	const void *pdata,
+	size_t sz)
+{
+	struct platform_device *pdev;
+
+	pdev = platform_device_register_resndata(
+		NULL,			/* parent */
+		name,			/* name */
+		PLATFORM_DEVID_NONE,	/* id */
+		NULL,			/* res */
+		0,			/* res_num */
+		pdata,			/* platform_data */
+		sz);
+
+	if (IS_ERR(pdev))
+		pr_err(KBUILD_MODNAME ": failed registering %s: %ld\n", name, PTR_ERR(pdev));
+
+	return pdev;
+}
+
+static int __init apu_board_init(void)
+{
+	int rc;
+	const struct dmi_system_id *id;
+
+	id = dmi_first_match(apu_gpio_dmi_table);
+
+	if (!id) {
+		pr_err(KBUILD_MODNAME ": failed to detect apu board via dmi\n");
+		return -ENODEV;
+	}
+
+	gpiod_add_lookup_table(&gpios_led_table);
+	gpiod_add_lookup_table(&gpios_key_table);
+
+	apu_gpio_pdev = apu_create_pdev(
+		AMD_FCH_GPIO_DRIVER_NAME,
+		id->driver_data,
+		sizeof(struct amd_fch_gpio_pdata));
+
+	apu_leds_pdev = apu_create_pdev(
+		"leds-gpio",
+		&apu2_leds_pdata,
+		sizeof(apu2_leds_pdata));
+
+	apu_keys_pdev = apu_create_pdev(
+		"gpio-keys-polled",
+		&apu2_keys_pdata,
+		sizeof(apu2_keys_pdata));
+
+	pr_info(KBUILD_MODNAME ": initialized: gpio, leds, keys\n");
+	return 0;
+}
+
+static void __exit apu_board_exit(void)
+{
+	gpiod_remove_lookup_table(&gpios_led_table);
+	gpiod_remove_lookup_table(&gpios_key_table);
+
+	platform_device_unregister(apu_keys_pdev);
+	platform_device_unregister(apu_leds_pdev);
+	platform_device_unregister(apu_gpio_pdev);
+}
+
+module_init(apu_board_init);
+module_exit(apu_board_exit);
+
+MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
+MODULE_DESCRIPTION("PC Engines APUv2/APUv3 board GPIO/LED/keys driver");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(dmi, apu_gpio_dmi_table);
+MODULE_ALIAS("platform:pcengines-apuv2");
+MODULE_SOFTDEP("pre: platform:" AMD_FCH_GPIO_DRIVER_NAME);
+MODULE_SOFTDEP("pre: platform:leds-gpio");
+MODULE_SOFTDEP("pre: platform:gpio_keys_polled");
-- 
1.9.1

Re: [PATCH 2/2] x86: pcengines apuv2 gpio/leds/keys platform driver

[Andy Shevchenko]

On Wed, Feb 13, 2019 at 10:57 PM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
>
> Driver for PCengines APUv2 board's front LEDs and Button,
> which are attached to AMD PCH GPIOs. Due to lack of dedicated
> ACPI entry, detecting the board via DMI.

> +#include <linux/init.h>

> +#include <linux/module.h>

One of them is not needed.

> +static struct platform_device * __init apu_create_pdev(
> +       const char* name,
> +       const void *pdata,
> +       size_t sz)
> +{
> +       struct platform_device *pdev;
> +

> +       pdev = platform_device_register_resndata(
> +               NULL,                   /* parent */
> +               name,                   /* name */
> +               PLATFORM_DEVID_NONE,    /* id */
> +               NULL,                   /* res */
> +               0,                      /* res_num */
> +               pdata,                  /* platform_data */
> +               sz);

No need to comment the obvious.

> +
> +       if (IS_ERR(pdev))

> +               pr_err(KBUILD_MODNAME ": failed registering %s: %ld\n", name, PTR_ERR(pdev));

Better to set pr_fmt() macro at the top of source.

> +
> +       return pdev;
> +}
> +
> +static int __init apu_board_init(void)
> +{
> +       int rc;
> +       const struct dmi_system_id *id;
> +
> +       id = dmi_first_match(apu_gpio_dmi_table);

> +

Redundant blank line.

> +       if (!id) {
> +               pr_err(KBUILD_MODNAME ": failed to detect apu board via dmi\n");

pr_fmt()

> +               return -ENODEV;
> +       }

> +       pr_info(KBUILD_MODNAME ": initialized: gpio, leds, keys\n");

Noise.

> +       return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko