[RESEND RFC PATCH 0/5] platform drivers for UP Board

[RESEND RFC PATCH 0/5] platform drivers for UP Board

[Dan O'Donovan]

[Re-sending to a wider audience suggested by Darren Hart]

The UP Board is a new SBC based on the Intel Atom X5-Z8350 "Cherry 
Trail" SoC and features a 40-pin I/O pin header and form-factor 
inspired by the Raspberry Pi 2.

It utilises a CPLD between the SoC and the external 40-pin header
to provide buffered voltage level-shifting of the I/O signals, mux
switching and LED control, and programmable pin mapping between the
SoC and the external pin header.

The gpio, pinctrl and led drivers provided in this patch series 
enable and manage the functions provided by that CPLD.

I have some open questions about this patch series:
 * Is it ok to place all of these various UP board drivers together
   in drivers/platform/x86/, or would it be preferable to place them
   in the respective sub-system directories (gpio, pinctrl, etc.)?
   My rationale for keeping them together here is that they are all
   specific to this UP Board platform and not expected to be
   generally useful on any other platforms (except variants of UP).
 * Is it acceptable to include hard-coded references to ACPI device
   IDs (representing devices integrated on the SoC devices) for the
   purpose of pin map and gpio references? Or is it required to
   use only named gpio pins?

Any feedback/suggestions on the questions above, and the patch series
in general, would be greatly appreciated!

Further information on the UP board can be obtained from [1] and [2].

[1] https://www.up-board.org
[2] https://up-community.org

Dan O'Donovan (5):
  platform: x86: add driver for UP Board I/O CPLD
  platform: x86: add UP Board I/O pinctrl driver
  platform: x86: add UP Board I/O gpio driver
  platform: x86: add UP Board CPLD LED driver
  platform: x86: add platform driver for UP Board

 drivers/platform/x86/Kconfig            |  13 +
 drivers/platform/x86/Makefile           |   5 +
 drivers/platform/x86/up_board.c         | 167 ++++++++++
 drivers/platform/x86/up_board_cpld.c    | 560 ++++++++++++++++++++++++++++++++
 drivers/platform/x86/up_board_cpld.h    |  38 +++
 drivers/platform/x86/up_board_gpio.c    | 254 +++++++++++++++
 drivers/platform/x86/up_board_gpio.h    |  59 ++++
 drivers/platform/x86/up_board_leds.c    |  85 +++++
 drivers/platform/x86/up_board_leds.h    |  50 +++
 drivers/platform/x86/up_board_pinctrl.c | 285 ++++++++++++++++
 drivers/platform/x86/up_board_pinctrl.h | 102 ++++++
 11 files changed, 1618 insertions(+)
 create mode 100644 drivers/platform/x86/up_board.c
 create mode 100644 drivers/platform/x86/up_board_cpld.c
 create mode 100644 drivers/platform/x86/up_board_cpld.h
 create mode 100644 drivers/platform/x86/up_board_gpio.c
 create mode 100644 drivers/platform/x86/up_board_gpio.h
 create mode 100644 drivers/platform/x86/up_board_leds.c
 create mode 100644 drivers/platform/x86/up_board_leds.h
 create mode 100644 drivers/platform/x86/up_board_pinctrl.c
 create mode 100644 drivers/platform/x86/up_board_pinctrl.h

-- 
2.1.4

[RESEND RFC PATCH 1/5] platform: x86: add driver for UP Board I/O CPLD

[Dan O'Donovan]

The UP board utilises a CPLD between its Intel X5-Z8350 SoC and an
external 40-pin header, to provide buffered voltage level-shifting
of the I/O signals, mux switching and LED control, and programmable
pin mapping between the SoC and the external pin header.

gpio, pinctrl and led drivers are provided in separate commits
to manage the functions provided by that CPLD.

This commit registers a platform driver to manage the configuration
of the CPLD itself.

Signed-off-by: Dan O'Donovan <dan@emutex.com>
---
 drivers/platform/x86/up_board_cpld.c | 560 +++++++++++++++++++++++++++++++++++
 drivers/platform/x86/up_board_cpld.h |  38 +++
 2 files changed, 598 insertions(+)
 create mode 100644 drivers/platform/x86/up_board_cpld.c
 create mode 100644 drivers/platform/x86/up_board_cpld.h

diff --git a/drivers/platform/x86/up_board_cpld.c b/drivers/platform/x86/up_board_cpld.c
new file mode 100644
index 0000000..dc36d6a
--- /dev/null
+++ b/drivers/platform/x86/up_board_cpld.c
@@ -0,0 +1,560 @@
+/*
+ * UP Board I/O Header CPLD driver.
+ *
+ * Copyright (c) 2016, Emutex Ltd.  All rights reserved.
+ *
+ * Author: Dan O'Donovan <dan@emutex.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/gpio.h>
+
+#include "up_board_cpld.h"
+#include "up_board_pinctrl.h"
+#include "up_board_gpio.h"
+#include "up_board_leds.h"
+
+/*
+ * The UP Board features an external 40-pin header for I/O functions including
+ * GPIO, I2C, UART, SPI, PWM and I2S, similar in layout to the Raspberry Pi 2.
+ * At the heart of the UP Board is an Intel X5-Z8350 "Cherry Trail" SoC, which
+ * provides the I/O functions for these pins at 1.8V logic levels.
+ *
+ * Additional buffers and mux switches are used between the SoC and the I/O pin
+ * header to convert between the 1.8V SoC I/O and the 3.3V levels required at
+ * the pin header, with sufficient current source/sink capability for LV-TTL
+ * compatibility.  These buffers and mux switches require run-time configuration
+ * based on the pin function or GPIO direction selected by the user.
+ *
+ * The purpose of this driver is to manage the complexity of the buffer
+ * configuration so that application code can transparently access the I/O
+ * functions on the external pins through standard kernel interfaces.  It
+ * instantiates a gpio and pinctrl device, and effectively acts as a "shim"
+ * between application code and the underlying Cherry Trail GPIO driver.
+ */
+
+/* The Cherry Trail SoC has 4 independent GPIO pin controllers */
+#define SOC_GC_SW	"INT33FF:00"
+#define SOC_GC_N	"INT33FF:01"
+#define SOC_GC_E	"INT33FF:02"
+#define SOC_GC_SE	"INT33FF:03"
+
+#define SOC_GPIO(n, o, f)		\
+	{				\
+		.soc_gc_name	= (n),	\
+		.soc_gc_offset	= (o),	\
+		.soc_gpio_flags	= (f),	\
+	}
+#define SOC_GPIO_INPUT(c, o) SOC_GPIO(c, o, GPIOF_IN)
+#define SOC_GPIO_OUTPUT(c, o) SOC_GPIO(c, o, GPIOF_OUT_INIT_LOW)
+
+#define GPIO_PIN_INFO(d, m, f)				\
+	{						\
+		.dir_ctrl_offset	= (d),		\
+		.mux_ctrl_offset	= (m),		\
+		.func_dir		= (f),		\
+		.func_enabled		= false,	\
+	}
+
+#define GPIO_PIN_INFO_NO_MUX(d, f)		\
+	GPIO_PIN_INFO(d, UP_BOARD_UNASSIGNED, f)
+
+#define PIN_GROUP(n, p)				\
+	{					\
+		.name = (n),			\
+		.pins = (p),			\
+		.npin = ARRAY_SIZE((p)),	\
+	}
+
+#define FUNCTION(n, g)				\
+	{					\
+		.name = (n),			\
+		.groups = (g),			\
+		.ngroup = ARRAY_SIZE((g)),	\
+	}
+
+/* Initial configuration assumes all pins as GPIO inputs */
+#define CPLD_DIR_REG_INIT	(0x00FFFFFFFULL)
+
+/* Internal context information for this driver */
+struct up_board_cpld {
+	struct device *dev;
+	struct platform_device *pinctrl_pdev;
+	struct platform_device *gpio_pdev;
+	struct platform_device *leds_pdev;
+	struct up_board_gpio_info strobe_gpio;
+	struct up_board_gpio_info reset_gpio;
+	struct up_board_gpio_info data_in_gpio;
+	struct up_board_gpio_info data_out_gpio;
+	struct up_board_gpio_info oe_gpio;
+	u64 dir_reg;
+	unsigned int dir_reg_size;
+	/* Lock to prevent concurrent access to CPLD */
+	spinlock_t lock;
+};
+
+static int up_board_cpld_reg_set_bit(struct up_board_cpld *cpld,
+				     unsigned int offset, int value);
+
+static struct up_board_cpld up_board_cpld = {
+	.strobe_gpio		= SOC_GPIO_OUTPUT(SOC_GC_N, 21),
+	.reset_gpio		= SOC_GPIO_OUTPUT(SOC_GC_E, 15),
+	.data_in_gpio		= SOC_GPIO_OUTPUT(SOC_GC_E, 13),
+	.data_out_gpio		= SOC_GPIO_INPUT(SOC_GC_E, 23),
+	.oe_gpio		= SOC_GPIO_OUTPUT(SOC_GC_SW, 43),
+	.dir_reg		= CPLD_DIR_REG_INIT,
+	.dir_reg_size		= 34,
+};
+
+/* Pin control information for the 28 GPIO pins on the UP Board I/O header */
+static struct up_board_pin_info up_board_pins[] = {
+	GPIO_PIN_INFO(9,  28, UP_BOARD_PDIR_OUT),	/*  0 */
+	GPIO_PIN_INFO(23, 28, UP_BOARD_PDIR_OUT),	/*  1 */
+	GPIO_PIN_INFO(0,  29, UP_BOARD_PDIR_OUT),	/*  2 */
+	GPIO_PIN_INFO(1,  29, UP_BOARD_PDIR_OUT),	/*  3 */
+	GPIO_PIN_INFO(2,  30, UP_BOARD_PDIR_IN),	/*  4 */
+	GPIO_PIN_INFO_NO_MUX(10, UP_BOARD_PDIR_NONE),	/*  5 */
+	GPIO_PIN_INFO_NO_MUX(11, UP_BOARD_PDIR_NONE),	/*  6 */
+	GPIO_PIN_INFO_NO_MUX(22, UP_BOARD_PDIR_NONE),	/*  7 */
+	GPIO_PIN_INFO_NO_MUX(21, UP_BOARD_PDIR_OUT),	/*  8 */
+	GPIO_PIN_INFO_NO_MUX(7,  UP_BOARD_PDIR_IN),	/*  9 */
+	GPIO_PIN_INFO_NO_MUX(6,  UP_BOARD_PDIR_OUT),	/* 10 */
+	GPIO_PIN_INFO_NO_MUX(8,  UP_BOARD_PDIR_OUT),	/* 11 */
+	GPIO_PIN_INFO_NO_MUX(24, UP_BOARD_PDIR_OUT),	/* 12 */
+	GPIO_PIN_INFO_NO_MUX(12, UP_BOARD_PDIR_OUT),	/* 13 */
+	GPIO_PIN_INFO_NO_MUX(15, UP_BOARD_PDIR_OUT),	/* 14 */
+	GPIO_PIN_INFO_NO_MUX(16, UP_BOARD_PDIR_IN),	/* 15 */
+	GPIO_PIN_INFO_NO_MUX(25, UP_BOARD_PDIR_IN),	/* 16 */
+	GPIO_PIN_INFO_NO_MUX(3,  UP_BOARD_PDIR_OUT),	/* 17 */
+	GPIO_PIN_INFO_NO_MUX(17, UP_BOARD_PDIR_OUT),	/* 18 */
+	GPIO_PIN_INFO_NO_MUX(13, UP_BOARD_PDIR_OUT),	/* 19 */
+	GPIO_PIN_INFO_NO_MUX(26, UP_BOARD_PDIR_IN),	/* 20 */
+	GPIO_PIN_INFO_NO_MUX(27, UP_BOARD_PDIR_OUT),	/* 21 */
+	GPIO_PIN_INFO_NO_MUX(5,  UP_BOARD_PDIR_OUT),	/* 22 */
+	GPIO_PIN_INFO_NO_MUX(18, UP_BOARD_PDIR_OUT),	/* 23 */
+	GPIO_PIN_INFO_NO_MUX(19, UP_BOARD_PDIR_OUT),	/* 24 */
+	GPIO_PIN_INFO_NO_MUX(20, UP_BOARD_PDIR_OUT),	/* 25 */
+	GPIO_PIN_INFO_NO_MUX(14, UP_BOARD_PDIR_OUT),	/* 26 */
+	GPIO_PIN_INFO_NO_MUX(4,  UP_BOARD_PDIR_OUT),	/* 27 */
+};
+
+/* SoC GPIO mapping for the 28 GPIO pins on the UP Board I/O header */
+static struct up_board_gpio_info up_board_gpios[] = {
+	SOC_GPIO(SOC_GC_SW, 33, 0),	/*  0 */
+	SOC_GPIO(SOC_GC_SW, 37, 0),	/*  1 */
+	SOC_GPIO(SOC_GC_SW, 32, 0),	/*  2 */
+	SOC_GPIO(SOC_GC_SW, 35, 0),	/*  3 */
+	SOC_GPIO(SOC_GC_E,  18, 0),	/*  4 */
+	SOC_GPIO(SOC_GC_E,  21, 0),	/*  5 */
+	SOC_GPIO(SOC_GC_E,  12, 0),	/*  6 */
+	SOC_GPIO(SOC_GC_SE, 48, 0),	/*  7 */
+	SOC_GPIO(SOC_GC_SE,  7, 0),	/*  8 */
+	SOC_GPIO(SOC_GC_SE,  3, 0),	/*  9 */
+	SOC_GPIO(SOC_GC_SE,  6, 0),	/* 10 */
+	SOC_GPIO(SOC_GC_SE,  4, 0),	/* 11 */
+	SOC_GPIO(SOC_GC_SE,  5, 0),	/* 12 */
+	SOC_GPIO(SOC_GC_SE,  1, 0),	/* 13 */
+	SOC_GPIO(SOC_GC_SW, 13, 0),	/* 14 */
+	SOC_GPIO(SOC_GC_SW,  9, 0),	/* 15 */
+	SOC_GPIO(SOC_GC_SW, 11, 0),	/* 16 */
+	SOC_GPIO(SOC_GC_SW,  8, 0),	/* 17 */
+	SOC_GPIO(SOC_GC_SW, 50, 0),	/* 18 */
+	SOC_GPIO(SOC_GC_SW, 54, 0),	/* 19 */
+	SOC_GPIO(SOC_GC_SW, 52, 0),	/* 20 */
+	SOC_GPIO(SOC_GC_SW, 55, 0),	/* 21 */
+	SOC_GPIO(SOC_GC_SE, 12, 0),	/* 22 */
+	SOC_GPIO(SOC_GC_SE, 15, 0),	/* 23 */
+	SOC_GPIO(SOC_GC_SE, 18, 0),	/* 24 */
+	SOC_GPIO(SOC_GC_SE, 11, 0),	/* 25 */
+	SOC_GPIO(SOC_GC_SE, 14, 0),	/* 26 */
+	SOC_GPIO(SOC_GC_SE,  8, 0),	/* 27 */
+};
+
+/* pinctrl descriptors for the 28 GPIO pins on the UP Board I/O header */
+static const struct pinctrl_pin_desc up_board_pinctrl_descs[] = {
+	PINCTRL_PIN(0,  "I2C0_SDA"),
+	PINCTRL_PIN(1,  "I2C0_SCL"),
+	PINCTRL_PIN(2,  "I2C1_SDA"),
+	PINCTRL_PIN(3,  "I2C1_SCL"),
+	PINCTRL_PIN(4,  "ADC"),
+	PINCTRL_PIN(5,  "GPIO5"),
+	PINCTRL_PIN(6,  "GPIO6"),
+	PINCTRL_PIN(7,  "SPI_CS1"),
+	PINCTRL_PIN(8,  "SPI_CS0"),
+	PINCTRL_PIN(9,  "SPI_MISO"),
+	PINCTRL_PIN(10, "SPI_MOSI"),
+	PINCTRL_PIN(11, "SPI_CLK"),
+	PINCTRL_PIN(12, "PWM0"),
+	PINCTRL_PIN(13, "PWM1"),
+	PINCTRL_PIN(14, "UART1_TX"),
+	PINCTRL_PIN(15, "UART1_RX"),
+	PINCTRL_PIN(16, "UART1_CTS"),
+	PINCTRL_PIN(17, "UART1_RTS"),
+	PINCTRL_PIN(18, "I2S_CLK"),
+	PINCTRL_PIN(19, "I2S_FRM"),
+	PINCTRL_PIN(20, "I2S_DIN"),
+	PINCTRL_PIN(21, "I2S_DOUT"),
+	PINCTRL_PIN(22, "GPIO22"),
+	PINCTRL_PIN(23, "GPIO23"),
+	PINCTRL_PIN(24, "GPIO24"),
+	PINCTRL_PIN(25, "GPIO25"),
+	PINCTRL_PIN(26, "GPIO26"),
+	PINCTRL_PIN(27, "GPIO27"),
+};
+
+static const unsigned int uart1_pins[] = { 14, 15, 16, 17 };
+static const unsigned int uart2_pins[] = { 25, 27 };
+static const unsigned int i2c0_pins[]  = { 0, 1 };
+static const unsigned int i2c1_pins[]  = { 2, 3 };
+static const unsigned int spi2_pins[]  = { 8, 9, 10, 11 };
+static const unsigned int i2s2_pins[]  = { 18, 19, 20, 21 };
+static const unsigned int pwm0_pins[]  = { 12 };
+static const unsigned int pwm1_pins[]  = { 13 };
+static const unsigned int adc0_pins[]  = { 4 };
+
+static const struct up_board_pinctrl_group up_board_pinctrl_groups[] = {
+	PIN_GROUP("uart1_grp", uart1_pins),
+	PIN_GROUP("uart2_grp", uart2_pins),
+	PIN_GROUP("i2c0_grp", i2c0_pins),
+	PIN_GROUP("i2c1_grp", i2c1_pins),
+	PIN_GROUP("spi2_grp", spi2_pins),
+	PIN_GROUP("i2s2_grp", i2s2_pins),
+	PIN_GROUP("pwm0_grp", pwm0_pins),
+	PIN_GROUP("pwm1_grp", pwm1_pins),
+	PIN_GROUP("adc0_grp", adc0_pins),
+};
+
+static const char * const uart1_groups[] = { "uart1_grp" };
+static const char * const uart2_groups[] = { "uart2_grp" };
+static const char * const i2c0_groups[]  = { "i2c0_grp" };
+static const char * const i2c1_groups[]  = { "i2c1_grp" };
+static const char * const spi2_groups[]  = { "spi2_grp" };
+static const char * const i2s2_groups[]  = { "i2s2_grp" };
+static const char * const pwm0_groups[]  = { "pwm0_grp" };
+static const char * const pwm1_groups[]  = { "pwm1_grp" };
+static const char * const adc0_groups[]  = { "adc0_grp" };
+
+static const struct up_board_pinctrl_function up_board_pinctrl_functions[] = {
+	FUNCTION("uart1", uart1_groups),
+	FUNCTION("uart2", uart2_groups),
+	FUNCTION("i2c0",  i2c0_groups),
+	FUNCTION("i2c1",  i2c1_groups),
+	FUNCTION("spi2",  spi2_groups),
+	FUNCTION("i2s2",  i2s2_groups),
+	FUNCTION("pwm0",  pwm0_groups),
+	FUNCTION("pwm1",  pwm1_groups),
+	FUNCTION("adc0",  adc0_groups),
+};
+
+/* The CPLD controls the following 3 LEDs on the UP board */
+static struct up_board_led_info up_board_leds[] = {
+	{ .cpld_offset = 31, .name = "upboard:yellow:", },
+	{ .cpld_offset = 32, .name = "upboard:green:", },
+	{ .cpld_offset = 33, .name = "upboard:red:", },
+};
+
+static struct up_board_pinctrl_pdata up_board_pinctrl_pdata = {
+	.cpld_info.cpld = &up_board_cpld,
+	.cpld_info.reg_set_bit = up_board_cpld_reg_set_bit,
+	.pins = up_board_pins,
+	.npin = ARRAY_SIZE(up_board_pins),
+	.descs = up_board_pinctrl_descs,
+	.ndesc = ARRAY_SIZE(up_board_pinctrl_descs),
+	.groups = up_board_pinctrl_groups,
+	.ngroup = ARRAY_SIZE(up_board_pinctrl_groups),
+	.functions = up_board_pinctrl_functions,
+	.nfunction = ARRAY_SIZE(up_board_pinctrl_functions),
+};
+
+static struct up_board_gpio_pdata up_board_gpio_pdata = {
+	.gpios = up_board_gpios,
+	.ngpio = ARRAY_SIZE(up_board_gpios),
+};
+
+static struct up_board_leds_pdata up_board_leds_pdata = {
+	.cpld_info.cpld = &up_board_cpld,
+	.cpld_info.reg_set_bit = up_board_cpld_reg_set_bit,
+	.leds = up_board_leds,
+	.nled = ARRAY_SIZE(up_board_leds),
+};
+
+/*
+ * On the UP board, the header pin level shifting and mux switching is
+ * controlled by a dedicated CPLD with proprietary firmware.
+ *
+ * The CPLD is responsible for connecting and translating 1.8V GPIO signals from
+ * the SoC to the 28 GPIO header pins at 3.3V, and for this it needs to be
+ * configured with direction (input/output) for each GPIO.  In addition, it
+ * manages 3 mux switches (2 for I2C bus pins, 1 for ADC pin) which need to be
+ * configured on/off, and 3 LEDs.  A register value is loaded into the CPLD to
+ * dynamically configure each of these.
+ */
+static int cpld_reg_update(struct up_board_cpld *cpld)
+{
+	u64 dir_reg_verify = 0;
+	int i;
+
+	/* Reset the CPLD internal counters */
+	gpiod_set_value(cpld->reset_gpio.soc_gpiod, 0);
+	gpiod_set_value(cpld->reset_gpio.soc_gpiod, 1);
+
+	/*
+	 * Update the CPLD dir register
+	 * data_in will be sampled on each rising edge of the strobe signal
+	 */
+	for (i = cpld->dir_reg_size - 1; i >= 0; i--) {
+		gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 0);
+		gpiod_set_value(cpld->data_in_gpio.soc_gpiod,
+				(cpld->dir_reg >> i) & 0x1);
+		gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 1);
+	}
+
+	/*
+	 * Read back and verify the value
+	 * data_out will be set on each rising edge of the strobe signal
+	 */
+	for (i = cpld->dir_reg_size - 1; i >= 0; i--) {
+		int data_out;
+
+		gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 0);
+		gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 1);
+		data_out = gpiod_get_value(cpld->data_out_gpio.soc_gpiod);
+		dir_reg_verify |= (u64)data_out << i;
+	}
+
+	if (dir_reg_verify != cpld->dir_reg) {
+		pr_err("CPLD verify error (expected: %llX, actual: %llX)\n",
+		       cpld->dir_reg, dir_reg_verify);
+		return -EIO;
+	}
+
+	/* Issue a dummy STB cycle to latch the dir register updates */
+	gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 0);
+	gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 1);
+
+	return 0;
+}
+
+/**
+ * up_board_cpld_reg_set_bit() - update CPLD configuration
+ * @cpld:	CPLD internal context info reference
+ * @offset:	bit offset in CPLD register to set
+ * @value:	boolean value to set in CPLD register bit selected by offset
+ *
+ * Return:	Returns 0 if successful, or negative error value otherwise
+ */
+static int up_board_cpld_reg_set_bit(struct up_board_cpld *cpld,
+				     unsigned int offset, int value)
+{
+	u64 old_regval;
+	int ret = 0;
+
+	spin_lock(&cpld->lock);
+
+	old_regval = cpld->dir_reg;
+
+	if (value)
+		cpld->dir_reg |= 1ULL << offset;
+	else
+		cpld->dir_reg &= ~(1ULL << offset);
+
+	/* Only update the CPLD register if it has changed */
+	if (cpld->dir_reg != old_regval)
+		ret = cpld_reg_update(cpld);
+
+	spin_unlock(&cpld->lock);
+
+	return ret;
+}
+
+static int up_gpiochip_match(struct gpio_chip *chip, void *data)
+{
+	return !strcmp(chip->label, data);
+}
+
+static int up_board_soc_gpio_setup(struct up_board_cpld *cpld,
+				   struct up_board_gpio_info *gpio)
+{
+	gpio->soc_gc = gpiochip_find(gpio->soc_gc_name, up_gpiochip_match);
+	if (!gpio->soc_gc)
+		return -EPROBE_DEFER;
+
+	gpio->soc_gpio = gpio->soc_gc->base + gpio->soc_gc_offset;
+	gpio->soc_gpiod = gpio_to_desc(gpio->soc_gpio);
+	if (!gpio->soc_gpiod) {
+		dev_err(cpld->dev, "Failed to get descriptor for gpio %d\n",
+			gpio->soc_gpio);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int up_board_cpld_setup(struct up_board_cpld *cpld)
+{
+	struct up_board_gpio_info *cpld_gpios[] = {
+		&cpld->strobe_gpio,
+		&cpld->reset_gpio,
+		&cpld->data_in_gpio,
+		&cpld->data_out_gpio,
+		&cpld->oe_gpio,
+	};
+	int i, ret;
+
+	spin_lock_init(&cpld->lock);
+
+	/* Initialise the CPLD config input GPIOs as outputs, initially low */
+	for (i = 0; i < ARRAY_SIZE(cpld_gpios); i++) {
+		struct up_board_gpio_info *gpio = cpld_gpios[i];
+
+		ret = up_board_soc_gpio_setup(cpld, gpio);
+		if (ret)
+			return ret;
+
+		ret = devm_gpio_request_one(cpld->dev, gpio->soc_gpio,
+					    gpio->soc_gpio_flags,
+					    dev_name(cpld->dev));
+		if (ret)
+			return ret;
+	}
+
+	/* Load initial CPLD configuration (all pins set for GPIO input) */
+	ret = cpld_reg_update(cpld);
+	if (ret) {
+		dev_err(cpld->dev, "CPLD initialisation failed\n");
+		return ret;
+	}
+
+	/* Enable the CPLD outputs after a valid configuration has been set */
+	gpiod_set_value(cpld->oe_gpio.soc_gpiod, 1);
+
+	return 0;
+}
+
+static int up_board_setup(struct up_board_cpld *cpld,
+			  struct up_board_gpio_pdata *gpio_pdata)
+{
+	size_t i;
+	int ret;
+
+	/* Ensure the GPIO pins are configured as inputs initially */
+	for (i = 0; i < gpio_pdata->ngpio; i++) {
+		struct up_board_gpio_info *gpio = &gpio_pdata->gpios[i];
+
+		ret = up_board_soc_gpio_setup(cpld, gpio);
+		if (ret)
+			return ret;
+
+		ret = gpiod_direction_input(gpio->soc_gpiod);
+		if (ret) {
+			dev_err(cpld->dev, "GPIO direction init failed\n");
+			return ret;
+		}
+	}
+
+	return up_board_cpld_setup(cpld);
+}
+
+static int up_board_cpld_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct up_board_cpld *cpld = &up_board_cpld;
+	int ret;
+
+	cpld->dev = dev;
+	ret = up_board_setup(cpld, &up_board_gpio_pdata);
+	if (ret)
+		return ret;
+
+	cpld->pinctrl_pdev =
+		platform_device_register_data(dev, "up-board-pinctrl",
+					      PLATFORM_DEVID_NONE,
+					      &up_board_pinctrl_pdata,
+					      sizeof(up_board_pinctrl_pdata));
+	if (IS_ERR(cpld->pinctrl_pdev)) {
+		ret = PTR_ERR(cpld->pinctrl_pdev);
+		goto fail_register_pinctrl_pdev;
+	}
+
+	cpld->gpio_pdev =
+		platform_device_register_data(dev, "up-board-gpio",
+					      PLATFORM_DEVID_NONE,
+					      &up_board_gpio_pdata,
+					      sizeof(up_board_gpio_pdata));
+	if (IS_ERR(cpld->gpio_pdev)) {
+		ret = PTR_ERR(cpld->gpio_pdev);
+		goto fail_register_gpio_pdev;
+	}
+
+	cpld->leds_pdev =
+		platform_device_register_data(dev, "up-board-leds",
+					      PLATFORM_DEVID_NONE,
+					      &up_board_leds_pdata,
+					      sizeof(up_board_leds_pdata));
+	if (IS_ERR(cpld->leds_pdev)) {
+		ret = PTR_ERR(cpld->leds_pdev);
+		goto fail_register_leds_pdev;
+	}
+
+	return 0;
+
+fail_register_leds_pdev:
+	platform_device_unregister(cpld->gpio_pdev);
+fail_register_gpio_pdev:
+	platform_device_unregister(cpld->pinctrl_pdev);
+fail_register_pinctrl_pdev:
+
+	return ret;
+}
+
+static int up_board_cpld_remove(struct platform_device *pdev)
+{
+	struct up_board_cpld *cpld = &up_board_cpld;
+
+	platform_device_unregister(cpld->leds_pdev);
+	platform_device_unregister(cpld->gpio_pdev);
+	platform_device_unregister(cpld->pinctrl_pdev);
+
+	/* Disable the CPLD outputs */
+	gpiod_set_value(cpld->oe_gpio.soc_gpiod, 0);
+
+	return 0;
+}
+
+static struct platform_driver up_board_cpld_driver = {
+	.driver.name	= "up-board-cpld",
+	.driver.owner	= THIS_MODULE,
+	.probe		= up_board_cpld_probe,
+	.remove		= up_board_cpld_remove,
+};
+
+static int __init up_board_cpld_init(void)
+{
+	return platform_driver_register(&up_board_cpld_driver);
+}
+subsys_initcall(up_board_cpld_init);
+
+static void __exit up_board_cpld_exit(void)
+{
+	platform_driver_unregister(&up_board_cpld_driver);
+}
+module_exit(up_board_cpld_exit);
+
+MODULE_AUTHOR("Dan O'Donovan <dan@emutex.com>");
+MODULE_DESCRIPTION("UP Board I/O Header CPLD driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:up-board-cpld");
diff --git a/drivers/platform/x86/up_board_cpld.h b/drivers/platform/x86/up_board_cpld.h
new file mode 100644
index 0000000..f635435
--- /dev/null
+++ b/drivers/platform/x86/up_board_cpld.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2016, Emutex Ltd.  All rights reserved.
+ *
+ * Author: Dan O'Donovan <dan@emutex.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef _UP_BOARD_CPLD_H_
+#define _UP_BOARD_CPLD_H_
+
+/* Forward declaration to internal CPLD info structure */
+struct up_board_cpld;
+
+/**
+ * struct up_board_cpld_info - abstract interface for CPLD configuration
+ * @cpld:		Opaque reference to internal CPLD info structure
+ * @reg_set_bit:	Callback to update internal CPLD register bits
+ *
+ * Information passed to UP Board CPLD users to provide a method for updating
+ * the CPLD configuration register
+ */
+struct up_board_cpld_info {
+	struct up_board_cpld *cpld;
+	int (*reg_set_bit)(struct up_board_cpld *cpld,
+			   unsigned int offset, int value);
+};
+
+#endif /* _UP_BOARD_CPLD_H_ */
-- 
2.1.4

Re: [RESEND RFC PATCH 1/5] platform: x86: add driver for UP Board I/O CPLD

[Bryan O'Donoghue]

On Mon, 2016-07-04 at 17:07 +0100, Dan O'Donovan wrote:
> +static int cpld_reg_update(struct up_board_cpld *cpld)
> +{
> +	u64 dir_reg_verify = 0;
> +	int i;
> +
> +	/* Reset the CPLD internal counters */
> +	gpiod_set_value(cpld->reset_gpio.soc_gpiod, 0);
> +	gpiod_set_value(cpld->reset_gpio.soc_gpiod, 1);
> +
> +	/*
> +	 * Update the CPLD dir register
> +	 * data_in will be sampled on each rising edge of the strobe
> signal
> +	 */
> +	for (i = cpld->dir_reg_size - 1; i >= 0; i--) {
> +		gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 0);
> +		gpiod_set_value(cpld->data_in_gpio.soc_gpiod,
> +				(cpld->dir_reg >> i) & 0x1);
> +		gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 1);
> +	}
> +
> +	/*
> +	 * Read back and verify the value
> +	 * data_out will be set on each rising edge of the strobe
> signal
> +	 */
> +	for (i = cpld->dir_reg_size - 1; i >= 0; i--) {
> +		int data_out;
> +
> +		gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 0);
> +		gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 1);
> +		data_out = gpiod_get_value(cpld-
> >data_out_gpio.soc_gpiod);
> +		dir_reg_verify |= (u64)data_out << i;
> +	}
> +
> +	if (dir_reg_verify != cpld->dir_reg) {
> +		pr_err("CPLD verify error (expected: %llX, actual:
> %llX)\n",
> +		       cpld->dir_reg, dir_reg_verify);

dev_err();

> +		return -EIO;
> +	}
> +
> +	/* Issue a dummy STB cycle to latch the dir register updates
> */
> +	gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 0);
> +	gpiod_set_value(cpld->strobe_gpio.soc_gpiod, 1);
> +
> +	return 0;
> +}
> +
> +/**
> + * up_board_cpld_reg_set_bit() - update CPLD configuration
> + * @cpld:	CPLD internal context info reference
> + * @offset:	bit offset in CPLD register to set
> + * @value:	boolean value to set in CPLD register bit selected
> by offset
> + *
> + * Return:	Returns 0 if successful, or negative error value
> otherwise
> + */
> +static int up_board_cpld_reg_set_bit(struct up_board_cpld *cpld,
> +				     unsigned int offset, int value)
> +{
> +	u64 old_regval;
> +	int ret = 0;
> +
> +	spin_lock(&cpld->lock);
> +
> +	old_regval = cpld->dir_reg;
> +
> +	if (value)
> +		cpld->dir_reg |= 1ULL << offset;
> +	else
> +		cpld->dir_reg &= ~(1ULL << offset);
> +
> +	/* Only update the CPLD register if it has changed */
> +	if (cpld->dir_reg != old_regval)
> +		ret = cpld_reg_update(cpld);
> +
> +	spin_unlock(&cpld->lock);

Seems to me as though cpld_reg_update() could be quite lengthy. Would a
mutex be a better choice here ?


> +static int up_board_cpld_setup(struct up_board_cpld *cpld)
> +{
> +	struct up_board_gpio_info *cpld_gpios[] = {
> +		&cpld->strobe_gpio,
> +		&cpld->reset_gpio,
> +		&cpld->data_in_gpio,
> +		&cpld->data_out_gpio,
> +		&cpld->oe_gpio,
> +	};
> +	int i, ret;
> +
> +	spin_lock_init(&cpld->lock);
> +
> +	/* Initialise the CPLD config input GPIOs as outputs,
> initially low */
> +	for (i = 0; i < ARRAY_SIZE(cpld_gpios); i++) {
> +		struct up_board_gpio_info *gpio = cpld_gpios[i];
> +
> +		ret = up_board_soc_gpio_setup(cpld, gpio);
> +		if (ret)
> +			return ret;
> +
> +		ret = devm_gpio_request_one(cpld->dev, gpio-
> >soc_gpio,
> +					    gpio->soc_gpio_flags,
> +					    dev_name(cpld->dev));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* Load initial CPLD configuration (all pins set for GPIO
> input) */
> +	ret = cpld_reg_update(cpld);
> +	if (ret) {

devm_gpio_free() ?

---
bod

Re: [RESEND RFC PATCH 1/5] platform: x86: add driver for UP Board I/O CPLD

[Bryan O'Donoghue]

On Mon, 2016-07-04 at 17:07 +0100, Dan O'Donovan wrote:
> +	cpld->pinctrl_pdev =
> +		platform_device_register_data(dev, "up-board-
> pinctrl",
> +					      PLATFORM_DEVID_NONE,
> +					      &up_board_pinctrl_pdat
> a,
> +					      sizeof(up_board_pinctr
> l_pdata));
> +	if (IS_ERR(cpld->pinctrl_pdev)) {
> +		ret = PTR_ERR(cpld->pinctrl_pdev);
> +		goto fail_register_pinctrl_pdev;
> +	}
> +
> +	cpld->gpio_pdev =
> +		platform_device_register_data(dev, "up-board-gpio",
> +					      PLATFORM_DEVID_NONE,
> +					      &up_board_gpio_pdata,
> +					      sizeof(up_board_gpio_p
> data));
> +	if (IS_ERR(cpld->gpio_pdev)) {
> +		ret = PTR_ERR(cpld->gpio_pdev);
> +		goto fail_register_gpio_pdev;
> +	}
> +
> +	cpld->leds_pdev =
> +		platform_device_register_data(dev, "up-board-leds",
> +					      PLATFORM_DEVID_NONE,
> +					      &up_board_leds_pdata,
> +					      sizeof(up_board_leds_p
> data));
> +	if (IS_ERR(cpld->leds_pdev)) {
> +		ret = PTR_ERR(cpld->leds_pdev);
> +		goto fail_register_leds_pdev;
> +	}
> +
> +	return 0;
> +
> +fail_register_leds_pdev:
> +	platform_device_unregister(cpld->gpio_pdev);
> +fail_register_gpio_pdev:
> +	platform_device_unregister(cpld->pinctrl_pdev);
> +fail_register_pinctrl_pdev:

Because the subordinate drivers don't have remove routines if any of
these error paths are triggered you'll leak resources. Recommend adding
cleanup exit routines to the subordinate drivers which should then be
triggered on the failure jumps here.

---
bod

Re: [RESEND RFC PATCH 1/5] platform: x86: add driver for UP Board I/O CPLD

[Darren Hart]

On Mon, Jul 04, 2016 at 05:07:10PM +0100, Dan O'Donovan wrote:
> The UP board utilises a CPLD between its Intel X5-Z8350 SoC and an
> external 40-pin header, to provide buffered voltage level-shifting
> of the I/O signals, mux switching and LED control, and programmable
> pin mapping between the SoC and the external pin header.
> 
> gpio, pinctrl and led drivers are provided in separate commits
> to manage the functions provided by that CPLD.
> 
> This commit registers a platform driver to manage the configuration
> of the CPLD itself.
> 
> Signed-off-by: Dan O'Donovan <dan@emutex.com>
> ---
>  drivers/platform/x86/up_board_cpld.c | 560 +++++++++++++++++++++++++++++++++++
>  drivers/platform/x86/up_board_cpld.h |  38 +++
>  2 files changed, 598 insertions(+)
>  create mode 100644 drivers/platform/x86/up_board_cpld.c
>  create mode 100644 drivers/platform/x86/up_board_cpld.h
> 
> diff --git a/drivers/platform/x86/up_board_cpld.c b/drivers/platform/x86/up_board_cpld.c
> new file mode 100644
> index 0000000..dc36d6a
> --- /dev/null
> +++ b/drivers/platform/x86/up_board_cpld.c
> @@ -0,0 +1,560 @@
> +/*
> + * UP Board I/O Header CPLD driver.
> + *
> + * Copyright (c) 2016, Emutex Ltd.  All rights reserved.
> + *
> + * Author: Dan O'Donovan <dan@emutex.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include <linux/gpio.h>
> +
> +#include "up_board_cpld.h"
> +#include "up_board_pinctrl.h"

Here you include up_board_pinctrl.h, but this file has not been introduced. The
build fails with 1/5 applied, breaking bisection.

Something to keep in mind as you relocate and refactor these drivers as we hear
back from the other maintainers. Each patch should build as you know, I'm sure
this was an error following a refactoring or something similar.

-- 
Darren Hart
Intel Open Source Technology Center

Re: [RESEND RFC PATCH 1/5] platform: x86: add driver for UP Board I/O CPLD

[Paul Gortmaker]

On Fri, Jul 22, 2016 at 4:52 PM, Darren Hart <dvhart@infradead.org> wrote:
> On Mon, Jul 04, 2016 at 05:07:10PM +0100, Dan O'Donovan wrote:
>> The UP board utilises a CPLD between its Intel X5-Z8350 SoC and an
>> external 40-pin header, to provide buffered voltage level-shifting
>> of the I/O signals, mux switching and LED control, and programmable
>> pin mapping between the SoC and the external pin header.
>>
>> gpio, pinctrl and led drivers are provided in separate commits
>> to manage the functions provided by that CPLD.
>>
>> This commit registers a platform driver to manage the configuration
>> of the CPLD itself.
>>
>> Signed-off-by: Dan O'Donovan <dan@emutex.com>
>> ---
>>  drivers/platform/x86/up_board_cpld.c | 560 +++++++++++++++++++++++++++++++++++
>>  drivers/platform/x86/up_board_cpld.h |  38 +++
>>  2 files changed, 598 insertions(+)

A variation on what Darren said but leading to the same issue;
i.e. importance of bisection is the presence of only two files here.

Normally there should be a Makefile change and possibly a
Kconfig change (addition) that bind the driver in so it can be
built.

However, since those are not shown here, I can't tell if the use
of module.h makes sense, because I can't see if the Kconfig
is bool or tristate.   So it goes beyond bisectability and also
impacts ability to properly review -- the "it" here  being the
proper segmentation of a patch series.

Thanks,
Paul.
--

>>  create mode 100644 drivers/platform/x86/up_board_cpld.c
>>  create mode 100644 drivers/platform/x86/up_board_cpld.h
>>
>> diff --git a/drivers/platform/x86/up_board_cpld.c b/drivers/platform/x86/up_board_cpld.c
>> new file mode 100644
>> index 0000000..dc36d6a
>> --- /dev/null
>> +++ b/drivers/platform/x86/up_board_cpld.c
>> @@ -0,0 +1,560 @@
>> +/*
>> + * UP Board I/O Header CPLD driver.
>> + *
>> + * Copyright (c) 2016, Emutex Ltd.  All rights reserved.
>> + *
>> + * Author: Dan O'Donovan <dan@emutex.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/gpio.h>
>> +
>> +#include "up_board_cpld.h"
>> +#include "up_board_pinctrl.h"
>
> Here you include up_board_pinctrl.h, but this file has not been introduced. The
> build fails with 1/5 applied, breaking bisection.
>
> Something to keep in mind as you relocate and refactor these drivers as we hear
> back from the other maintainers. Each patch should build as you know, I'm sure
> this was an error following a refactoring or something similar.
>
> --
> Darren Hart
> Intel Open Source Technology Center

[RESEND RFC PATCH 2/5] platform: x86: add UP Board I/O pinctrl driver

[Dan O'Donovan]

This pinctrl driver manages the Pin Control functions provided by
the I/O CPLD integrated on the UP Board.  This includes dynamic pin
direction and pin mux configuration.

Signed-off-by: Dan O'Donovan <dan@emutex.com>
---
 drivers/platform/x86/up_board_pinctrl.c | 285 ++++++++++++++++++++++++++++++++
 drivers/platform/x86/up_board_pinctrl.h | 102 ++++++++++++
 2 files changed, 387 insertions(+)
 create mode 100644 drivers/platform/x86/up_board_pinctrl.c
 create mode 100644 drivers/platform/x86/up_board_pinctrl.h

diff --git a/drivers/platform/x86/up_board_pinctrl.c b/drivers/platform/x86/up_board_pinctrl.c
new file mode 100644
index 0000000..be95837
--- /dev/null
+++ b/drivers/platform/x86/up_board_pinctrl.c
@@ -0,0 +1,285 @@
+/*
+ * UP Board I/O Header CPLD Pin Control driver.
+ *
+ * Copyright (c) 2016, Emutex Ltd.  All rights reserved.
+ *
+ * Author: Dan O'Donovan <dan@emutex.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/platform_device.h>
+
+#include "up_board_pinctrl.h"
+
+/* Internal context information for this driver */
+struct up_board_pinctrl {
+	struct up_board_pinctrl_pdata *pdata;
+	struct pinctrl_desc pctldesc;
+	struct pinctrl_dev *pctldev;
+};
+
+static int up_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	struct up_board_pinctrl *up_pinctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	return up_pinctrl->pdata->ngroup;
+}
+
+static const char *up_get_group_name(struct pinctrl_dev *pctldev,
+				     unsigned int group)
+{
+	struct up_board_pinctrl *up_pinctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	return up_pinctrl->pdata->groups[group].name;
+}
+
+static int up_get_group_pins(struct pinctrl_dev *pctldev, unsigned int group,
+			     const unsigned int **pins, unsigned int *npins)
+{
+	struct up_board_pinctrl *up_pinctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	*pins = up_pinctrl->pdata->groups[group].pins;
+	*npins = up_pinctrl->pdata->groups[group].npin;
+
+	return 0;
+}
+
+static const struct pinctrl_ops up_pinctrl_ops = {
+	.get_groups_count = up_get_groups_count,
+	.get_group_name = up_get_group_name,
+	.get_group_pins = up_get_group_pins,
+};
+
+static int up_get_functions_count(struct pinctrl_dev *pctldev)
+{
+	struct up_board_pinctrl *up_pinctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	return up_pinctrl->pdata->nfunction;
+}
+
+static const char *up_get_function_name(struct pinctrl_dev *pctldev,
+					unsigned int function)
+{
+	struct up_board_pinctrl *up_pinctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	return up_pinctrl->pdata->functions[function].name;
+}
+
+static int up_get_function_groups(struct pinctrl_dev *pctldev,
+				  unsigned int function,
+				  const char * const **groups,
+				  unsigned int * const ngroups)
+{
+	struct up_board_pinctrl *up_pinctrl = pinctrl_dev_get_drvdata(pctldev);
+
+	*groups = up_pinctrl->pdata->functions[function].groups;
+	*ngroups = up_pinctrl->pdata->functions[function].ngroup;
+
+	return 0;
+}
+
+static int up_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
+			     unsigned int group)
+{
+	struct up_board_pinctrl *up_pinctrl = pinctrl_dev_get_drvdata(pctldev);
+	struct up_board_pinctrl_pdata *pdata = up_pinctrl->pdata;
+	struct up_board_cpld_info *cpld_info = &up_pinctrl->pdata->cpld_info;
+	const struct up_board_pinctrl_group *grp = &pdata->groups[group];
+	int i, ret;
+
+	for (i = 0; i < grp->npin; i++) {
+		int offset = grp->pins[i];
+		struct up_board_pin_info *pin = &pdata->pins[offset];
+
+		if (pin->func_dir != UP_BOARD_PDIR_NONE) {
+			ret = cpld_info->reg_set_bit(cpld_info->cpld,
+						     pin->dir_ctrl_offset,
+						     pin->func_dir);
+			if (ret)
+				return ret;
+		}
+		if (pin->mux_ctrl_offset != UP_BOARD_UNASSIGNED) {
+			ret = cpld_info->reg_set_bit(cpld_info->cpld,
+						     pin->mux_ctrl_offset,
+						     UP_BOARD_PMUX_FUNC);
+			if (ret)
+				return ret;
+		}
+		pin->func_enabled = true;
+	}
+
+	return 0;
+}
+
+static int up_gpio_set_direction(struct pinctrl_dev *pctldev,
+				 struct pinctrl_gpio_range *range,
+				 unsigned int offset, bool input)
+{
+	struct up_board_pinctrl *up_pinctrl = pinctrl_dev_get_drvdata(pctldev);
+	struct up_board_pinctrl_pdata *pdata = up_pinctrl->pdata;
+	struct up_board_cpld_info *cpld_info = &up_pinctrl->pdata->cpld_info;
+	struct up_board_pin_info *pin = &pdata->pins[offset];
+	int dir = input ? UP_BOARD_PDIR_IN : UP_BOARD_PDIR_OUT;
+
+	return cpld_info->reg_set_bit(cpld_info->cpld,
+				      pin->dir_ctrl_offset,
+				      dir);
+}
+
+static int up_gpio_request_enable(struct pinctrl_dev *pctldev,
+				  struct pinctrl_gpio_range *range,
+				  unsigned int offset)
+{
+	struct up_board_pinctrl *up_pinctrl = pinctrl_dev_get_drvdata(pctldev);
+	struct up_board_pinctrl_pdata *pdata = up_pinctrl->pdata;
+	struct up_board_cpld_info *cpld_info = &up_pinctrl->pdata->cpld_info;
+	struct up_board_pin_info *pin = &pdata->pins[offset];
+	int ret;
+
+	if (pin->mux_ctrl_offset != UP_BOARD_UNASSIGNED) {
+		ret = cpld_info->reg_set_bit(cpld_info->cpld,
+					     pin->mux_ctrl_offset,
+					     UP_BOARD_PMUX_GPIO);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static void up_gpio_disable_free(struct pinctrl_dev *pctldev,
+				 struct pinctrl_gpio_range *range,
+				 unsigned int offset)
+{
+	struct up_board_pinctrl *up_pinctrl = pinctrl_dev_get_drvdata(pctldev);
+	struct up_board_pinctrl_pdata *pdata = up_pinctrl->pdata;
+	struct up_board_cpld_info *cpld_info = &up_pinctrl->pdata->cpld_info;
+	struct up_board_pin_info *pin = &pdata->pins[offset];
+
+	if (pin->func_enabled) {
+		if (pin->func_dir != UP_BOARD_PDIR_NONE) {
+			cpld_info->reg_set_bit(cpld_info->cpld,
+					       pin->dir_ctrl_offset,
+					       pin->func_dir);
+		}
+		if (pin->mux_ctrl_offset != UP_BOARD_UNASSIGNED) {
+			cpld_info->reg_set_bit(cpld_info->cpld,
+					       pin->mux_ctrl_offset,
+					       UP_BOARD_PMUX_FUNC);
+		}
+	}
+}
+
+static const struct pinmux_ops up_pinmux_ops = {
+	.get_functions_count = up_get_functions_count,
+	.get_function_name = up_get_function_name,
+	.get_function_groups = up_get_function_groups,
+	.set_mux = up_pinmux_set_mux,
+	.gpio_request_enable = up_gpio_request_enable,
+	.gpio_disable_free = up_gpio_disable_free,
+	.gpio_set_direction = up_gpio_set_direction,
+};
+
+static int up_config_get(struct pinctrl_dev *pctldev, unsigned int pin,
+			 unsigned long *config)
+{
+	return -ENOTSUPP;
+}
+
+static int up_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
+			 unsigned long *configs, unsigned int nconfigs)
+{
+	return 0;
+}
+
+static const struct pinconf_ops up_pinconf_ops = {
+	.is_generic = true,
+	.pin_config_set = up_config_set,
+	.pin_config_get = up_config_get,
+};
+
+static struct pinctrl_desc up_pinctrl_desc = {
+	.owner = THIS_MODULE,
+	.pctlops = &up_pinctrl_ops,
+	.pmxops = &up_pinmux_ops,
+	.confops = &up_pinconf_ops,
+};
+
+static int up_board_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct up_board_pinctrl_pdata *pdata = dev_get_platdata(dev);
+	struct up_board_pinctrl *up_pinctrl;
+
+	if (!pdata)
+		return -EINVAL;
+
+	up_pinctrl = devm_kzalloc(dev, sizeof(*up_pinctrl), GFP_KERNEL);
+	if (!up_pinctrl)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, up_pinctrl);
+
+	up_pinctrl->pdata = pdata;
+	up_pinctrl->pctldesc = up_pinctrl_desc;
+	up_pinctrl->pctldesc.pins = pdata->descs;
+	up_pinctrl->pctldesc.npins = pdata->ndesc;
+	up_pinctrl->pctldesc.name = dev_name(dev);
+	up_pinctrl->pctldev = pinctrl_register(&up_pinctrl->pctldesc,
+					       dev, up_pinctrl);
+	if (IS_ERR(up_pinctrl->pctldev)) {
+		dev_err(dev, "failed to register pinctrl driver\n");
+		return PTR_ERR(up_pinctrl->pctldev);
+	}
+
+	return 0;
+}
+
+static int up_board_pinctrl_remove(struct platform_device *pdev)
+{
+	struct up_board_pinctrl *up_pinctrl = platform_get_drvdata(pdev);
+
+	pinctrl_unregister(up_pinctrl->pctldev);
+
+	return 0;
+}
+
+static struct platform_driver up_board_pinctrl_driver = {
+	.driver.name	= "up-board-pinctrl",
+	.driver.owner	= THIS_MODULE,
+	.probe		= up_board_pinctrl_probe,
+	.remove		= up_board_pinctrl_remove,
+};
+
+static int __init up_board_pinctrl_init(void)
+{
+	return platform_driver_register(&up_board_pinctrl_driver);
+}
+subsys_initcall(up_board_pinctrl_init);
+
+static void __exit up_board_pinctrl_exit(void)
+{
+	platform_driver_unregister(&up_board_pinctrl_driver);
+}
+module_exit(up_board_pinctrl_exit);
+
+MODULE_AUTHOR("Dan O'Donovan <dan@emutex.com>");
+MODULE_DESCRIPTION("UP Board I/O Header CPLD Pin Control driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:up-board-pinctrl");
diff --git a/drivers/platform/x86/up_board_pinctrl.h b/drivers/platform/x86/up_board_pinctrl.h
new file mode 100644
index 0000000..ce46886
--- /dev/null
+++ b/drivers/platform/x86/up_board_pinctrl.h
@@ -0,0 +1,102 @@
+/*
+ * Copyright (c) 2016, Emutex Ltd.  All rights reserved.
+ *
+ * Author: Dan O'Donovan <dan@emutex.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef _UP_BOARD_PINCTRL_H_
+#define _UP_BOARD_PINCTRL_H_
+
+#include <linux/pinctrl/pinctrl.h>
+
+#include "up_board_cpld.h"
+
+#define UP_BOARD_PDIR_NONE	-1
+#define UP_BOARD_PDIR_OUT	 0
+#define UP_BOARD_PDIR_IN	 1
+
+#define UP_BOARD_PMUX_GPIO	 0
+#define UP_BOARD_PMUX_FUNC	 1
+
+#define UP_BOARD_UNASSIGNED	-1
+
+/**
+ * struct up_board_pinctrl_group - information for a single pinctrl group
+ * @name: group name
+ * @pins: array of pins associated with this group
+ * @npin: size of pins array
+ */
+struct up_board_pinctrl_group {
+	const char *name;
+	const unsigned int *pins;
+	size_t npin;
+};
+
+/**
+ * struct up_board_pinctrl_function - information for a single pinctrl function
+ * @name:	function name
+ * @groups:	array of groups associated with this function
+ * @ngroup:	size of groups array
+ */
+struct up_board_pinctrl_function {
+	const char *name;
+	const char * const *groups;
+	size_t ngroup;
+};
+
+/**
+ * struct up_board_pin_info - information for each UP Board GPIO pin
+ * @dir_ctrl_offset:	CPLD register bit offset for pin direction control
+ * @mux_ctrl_offset:	CPLD register bit offset for pin mux control
+ * @func_dir:		Pin dir to set when alternate pin function is selected
+ * @func_enabled:	Flag to indicate if alternate pin function is enabled
+ *
+ * Information for a single GPIO pin on the UP Board I/O header, including
+ * details of CPLD parameters for managing pin direction and function selection.
+ */
+struct up_board_pin_info {
+	int dir_ctrl_offset;
+	int mux_ctrl_offset;
+	int func_dir;
+	bool func_enabled;
+};
+
+/**
+ * struct up_board_pinctrl_pdata - platform driver data
+ * @cpld_info:	CPLD configuration interface information
+ * @pins:	Array of pin information structures
+ * @npin:	Number of entries in pins array
+ * @descs:	Array of pinctrl pin descriptors
+ * @ndesc:	Number of entries in pin_descs array
+ * @groups:	Array of pin groups
+ * @ngroup:	Number of entries in groups array
+ * @functions:	Array of pin functions
+ * @nfunction:	Number of entries in functions array
+ *
+ * Platform data provided to UP Board CPLD pinctrl platform device driver.
+ * Provides information for each GPIO pin on the UP Board I/O header.
+ */
+struct up_board_pinctrl_pdata {
+	struct up_board_cpld_info cpld_info;
+	struct up_board_pin_info *pins;
+	size_t npin;
+	const struct pinctrl_pin_desc *descs;
+	size_t ndesc;
+	const struct up_board_pinctrl_group *groups;
+	size_t ngroup;
+	const struct up_board_pinctrl_function *functions;
+	size_t nfunction;
+};
+
+#endif /* _UP_BOARD_PINCTRL_H_ */
-- 
2.1.4

[RESEND RFC PATCH 3/5] platform: x86: add UP Board I/O gpio driver

[Dan O'Donovan]

This gpio driver encapsulates the GPIO pin mapping function provided
by the I/O CPLD integrated on the UP Board.  This makes possible the
following features:
- integration of UP Board pin control functions with GPIO run-time
  configuration hooks, to allow run-time pin direction and pin mux
  selection
- logical numbering scheme for the external GPIO pins which is
  independent from the internal mapping to SoC GPIO pins.  This
  allows for variations in CPLD pin mapping configurations.
- numbering scheme which mimicks the Raspberry Pi GPIO numbering to
  facilitate application portability, reflecting the hardware design
  goal for the UP Board I/O pin header.

In essence, this driver is implemented as a thin layer on top of the
underlying Cherry Trail SoC GPIO driver, and instantiates a logical
gpiochip device.  The intention is that the user at application level
would utilise this logical GPIO chip to access the GPIO functions on
the UP Board I/O pins.

Signed-off-by: Dan O'Donovan <dan@emutex.com>
---
 drivers/platform/x86/up_board_gpio.c | 254 +++++++++++++++++++++++++++++++++++
 drivers/platform/x86/up_board_gpio.h |  59 ++++++++
 2 files changed, 313 insertions(+)
 create mode 100644 drivers/platform/x86/up_board_gpio.c
 create mode 100644 drivers/platform/x86/up_board_gpio.h

diff --git a/drivers/platform/x86/up_board_gpio.c b/drivers/platform/x86/up_board_gpio.c
new file mode 100644
index 0000000..c30c64b
--- /dev/null
+++ b/drivers/platform/x86/up_board_gpio.c
@@ -0,0 +1,254 @@
+/*
+ * UP Board I/O Header CPLD GPIO driver.
+ *
+ * Copyright (c) 2016, Emutex Ltd.  All rights reserved.
+ *
+ * Author: Dan O'Donovan <dan@emutex.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/gpio.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+
+#include "up_board_gpio.h"
+
+/* Internal context information for this driver */
+struct up_board_gpio {
+	struct up_board_gpio_pdata *pdata;
+	struct gpio_chip chip;
+};
+
+static irqreturn_t up_gpio_irq_handler(int irq, void *data)
+{
+	struct up_board_gpio_info *gpio = data;
+
+	generic_handle_irq(gpio->irq);
+	return IRQ_HANDLED;
+}
+
+static unsigned int up_gpio_irq_startup(struct irq_data *data)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+	struct up_board_gpio *up_gpio = gpiochip_get_data(gc);
+	unsigned int offset = irqd_to_hwirq(data);
+	struct up_board_gpio_info *gpio = &up_gpio->pdata->gpios[offset];
+
+	return request_irq(gpio->soc_gpio_irq, up_gpio_irq_handler,
+			   IRQF_ONESHOT, gc->label, gpio);
+}
+
+static void up_gpio_irq_shutdown(struct irq_data *data)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+	struct up_board_gpio *up_gpio = gpiochip_get_data(gc);
+	unsigned int offset = irqd_to_hwirq(data);
+	struct up_board_gpio_info *gpio = &up_gpio->pdata->gpios[offset];
+
+	free_irq(gpio->soc_gpio_irq, gpio);
+}
+
+static struct irq_chip up_gpio_irqchip = {
+	.irq_startup = up_gpio_irq_startup,
+	.irq_shutdown = up_gpio_irq_shutdown,
+	.irq_enable = irq_chip_enable_parent,
+	.irq_disable = irq_chip_disable_parent,
+	.irq_mask = irq_chip_mask_parent,
+	.irq_unmask = irq_chip_unmask_parent,
+	.irq_ack = irq_chip_ack_parent,
+	.irq_set_type = irq_chip_set_type_parent,
+};
+
+static int up_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
+{
+	struct up_board_gpio *up_gpio = gpiochip_get_data(gc);
+	struct up_board_gpio_info *gpio = &up_gpio->pdata->gpios[offset];
+	int ret;
+
+	ret = gpiod_direction_input(gpio->soc_gpiod);
+	if (ret)
+		return ret;
+
+	return pinctrl_gpio_direction_input(gc->base + offset);
+}
+
+static int up_gpio_dir_out(struct gpio_chip *gc, unsigned int offset, int value)
+{
+	struct up_board_gpio *up_gpio = gpiochip_get_data(gc);
+	struct up_board_gpio_info *gpio = &up_gpio->pdata->gpios[offset];
+	int ret;
+
+	ret = pinctrl_gpio_direction_output(gc->base + offset);
+	if (ret)
+		return ret;
+
+	return gpiod_direction_output(gpio->soc_gpiod, value);
+}
+
+static int up_gpio_get_dir(struct gpio_chip *gc, unsigned int offset)
+{
+	struct up_board_gpio *up_gpio = gpiochip_get_data(gc);
+	struct up_board_gpio_info *gpio = &up_gpio->pdata->gpios[offset];
+
+	return gpiod_get_direction(gpio->soc_gpiod);
+}
+
+static int up_gpio_request(struct gpio_chip *gc, unsigned int offset)
+{
+	struct up_board_gpio *up_gpio = gpiochip_get_data(gc);
+	struct up_board_gpio_info *gpio = &up_gpio->pdata->gpios[offset];
+	int ret;
+
+	ret = pinctrl_request_gpio(gc->base + offset);
+	if (ret)
+		return ret;
+
+	if (gpiod_get_direction(gpio->soc_gpiod))
+		ret = pinctrl_gpio_direction_input(gc->base + offset);
+	else
+		ret = pinctrl_gpio_direction_output(gc->base + offset);
+	if (ret)
+		return ret;
+
+	return gpio_request(gpio->soc_gpio, gc->label);
+}
+
+static void up_gpio_free(struct gpio_chip *gc, unsigned int offset)
+{
+	struct up_board_gpio *up_gpio = gpiochip_get_data(gc);
+	struct up_board_gpio_info *gpio = &up_gpio->pdata->gpios[offset];
+
+	pinctrl_free_gpio(gc->base + offset);
+	gpio_free(gpio->soc_gpio);
+}
+
+static int up_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+	struct up_board_gpio *up_gpio = gpiochip_get_data(gc);
+	struct up_board_gpio_info *gpio = &up_gpio->pdata->gpios[offset];
+
+	return gpiod_get_value(gpio->soc_gpiod);
+}
+
+static void up_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
+{
+	struct up_board_gpio *up_gpio = gpiochip_get_data(gc);
+	struct up_board_gpio_info *gpio = &up_gpio->pdata->gpios[offset];
+
+	gpiod_set_value(gpio->soc_gpiod, value);
+}
+
+static struct gpio_chip up_gpio_chip = {
+	.owner			= THIS_MODULE,
+	.request		= up_gpio_request,
+	.free			= up_gpio_free,
+	.get_direction		= up_gpio_get_dir,
+	.direction_input	= up_gpio_dir_in,
+	.direction_output	= up_gpio_dir_out,
+	.get			= up_gpio_get,
+	.set			= up_gpio_set,
+};
+
+static int up_board_gpio_setup(struct up_board_gpio *up_gpio)
+{
+	struct up_board_gpio_pdata *pdata = up_gpio->pdata;
+	size_t i;
+
+	for (i = 0; i < pdata->ngpio; i++) {
+		struct up_board_gpio_info *gpio = &pdata->gpios[i];
+		struct irq_data *irq_data;
+
+		/*
+		 * Create parent linkage with SoC GPIO IRQs to simplify
+		 * IRQ handling by enabling use of irq_chip_*_parent()
+		 * functions
+		 */
+		gpio->soc_gpio_irq = gpiod_to_irq(gpio->soc_gpiod);
+		gpio->irq = irq_find_mapping(up_gpio->chip.irqdomain, i);
+		irq_set_parent(gpio->irq, gpio->soc_gpio_irq);
+		irq_data = irq_get_irq_data(gpio->irq);
+		irq_data->parent_data = irq_get_irq_data(gpio->soc_gpio_irq);
+	}
+
+	return 0;
+}
+
+static int up_board_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct up_board_gpio_pdata *pdata = dev_get_platdata(dev);
+	struct up_board_gpio *up_gpio;
+	int ret;
+
+	if (!pdata)
+		return -EINVAL;
+
+	up_gpio = devm_kzalloc(dev, sizeof(*up_gpio), GFP_KERNEL);
+	if (!up_gpio)
+		return -ENOMEM;
+
+	up_gpio->pdata = pdata;
+	up_gpio->chip = up_gpio_chip;
+	up_gpio->chip.parent = dev;
+	up_gpio->chip.ngpio = pdata->ngpio;
+	up_gpio->chip.label = dev_name(dev);
+
+	ret = devm_gpiochip_add_data(dev, &up_gpio->chip, up_gpio);
+	if (ret) {
+		dev_err(dev, "failed to add gpio chip: %d\n", ret);
+		return ret;
+	}
+
+	ret = gpiochip_add_pin_range(&up_gpio->chip, "up-board-pinctrl", 0, 0,
+				     pdata->ngpio);
+	if (ret) {
+		dev_err(dev, "failed to add GPIO pin range\n");
+		return ret;
+	}
+
+	up_gpio_irqchip.name = up_gpio->chip.label;
+	ret = gpiochip_irqchip_add(&up_gpio->chip, &up_gpio_irqchip, 0,
+				   handle_simple_irq, IRQ_TYPE_NONE);
+	if (ret) {
+		dev_err(dev, "failed to add IRQ chip\n");
+		goto fail_irqchip_add;
+	}
+
+	ret = up_board_gpio_setup(up_gpio);
+	if (ret)
+		goto fail_gpio_setup;
+
+	return 0;
+
+fail_gpio_setup:
+fail_irqchip_add:
+	gpiochip_remove_pin_ranges(&up_gpio->chip);
+
+	return ret;
+}
+
+static struct platform_driver up_board_gpio_driver = {
+	.driver.name	= "up-board-gpio",
+	.driver.owner	= THIS_MODULE,
+	.probe		= up_board_gpio_probe,
+};
+
+module_platform_driver(up_board_gpio_driver);
+
+MODULE_AUTHOR("Dan O'Donovan <dan@emutex.com>");
+MODULE_DESCRIPTION("UP Board I/O Header CPLD GPIO driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:up-board-gpio");
diff --git a/drivers/platform/x86/up_board_gpio.h b/drivers/platform/x86/up_board_gpio.h
new file mode 100644
index 0000000..ada7d2e
--- /dev/null
+++ b/drivers/platform/x86/up_board_gpio.h
@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2016, Emutex Ltd.  All rights reserved.
+ *
+ * Author: Dan O'Donovan <dan@emutex.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef _UP_BOARD_GPIO_H_
+#define _UP_BOARD_GPIO_H_
+
+/**
+ * struct up_board_gpio_info - information for an UP Board GPIO pin
+ * @soc_gc_name:	Device name for corresponding SoC GPIO chip
+ * @soc_gc_offset:	GPIO chip offset of corresponding SoC GPIO pin
+ * @soc_gc:		SoC GPIO chip reference
+ * @soc_gpiod:		SoC GPIO descriptor reference
+ * @soc_gpio:		SoC GPIO assigned pin number
+ * @soc_gpio_irq:	SoC GPIO assigned IRQ number
+ * @soc_gpio_flags:	Optional GPIO flags to apply to SoC GPIO
+ * @irq:		Assigned IRQ number for this GPIO pin
+ *
+ * Information for a single GPIO pin on the UP Board I/O header, including
+ * details of the corresponding SoC GPIO mapped to this I/O header GPIO.
+ */
+struct up_board_gpio_info {
+	char *soc_gc_name;
+	unsigned int soc_gc_offset;
+	struct gpio_chip *soc_gc;
+	struct gpio_desc *soc_gpiod;
+	int soc_gpio;
+	int soc_gpio_irq;
+	int soc_gpio_flags;
+	int irq;
+};
+
+/**
+ * struct up_board_gpio_pdata - platform driver data
+ * @gpios:	Array of GPIO information structures.
+ * @ngpio:	Number of entries in gpios array.
+ *
+ * Platform data provided to UP Board CPLD GPIO platform device driver.
+ * Provides information for each GPIO pin on the UP Board I/O header.
+ */
+struct up_board_gpio_pdata {
+	struct up_board_gpio_info *gpios;
+	size_t ngpio;
+};
+
+#endif /* _UP_BOARD_GPIO_H_ */
-- 
2.1.4

[RESEND RFC PATCH 4/5] platform: x86: add UP Board CPLD LED driver

[Dan O'Donovan]

This pinctrl driver manages the LED Control function provided by
the I/O CPLD integrated on the UP Board.  This allows basic on/off
brightness control for each of the individual LEDs.

Signed-off-by: Dan O'Donovan <dan@emutex.com>
---
 drivers/platform/x86/up_board_leds.c | 85 ++++++++++++++++++++++++++++++++++++
 drivers/platform/x86/up_board_leds.h | 50 +++++++++++++++++++++
 2 files changed, 135 insertions(+)
 create mode 100644 drivers/platform/x86/up_board_leds.c
 create mode 100644 drivers/platform/x86/up_board_leds.h

diff --git a/drivers/platform/x86/up_board_leds.c b/drivers/platform/x86/up_board_leds.c
new file mode 100644
index 0000000..4186475
--- /dev/null
+++ b/drivers/platform/x86/up_board_leds.c
@@ -0,0 +1,85 @@
+/*
+ * UP Board CPLD LEDs driver.
+ *
+ * Copyright (c) 2016, Emutex Ltd.  All rights reserved.
+ *
+ * Author: Javier Arteaga <javier@emutex.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/leds.h>
+#include <linux/platform_device.h>
+
+#include "up_board_leds.h"
+
+/* Internal context information for this driver */
+struct up_board_led {
+	struct up_board_leds_pdata *pdata;
+	unsigned int offset;
+	const char *name;
+	struct led_classdev cdev;
+};
+
+static void up_led_brightness_set(struct led_classdev *cdev,
+				  enum led_brightness value)
+{
+	struct up_board_led *led = container_of(cdev,
+						struct up_board_led,
+						cdev);
+	struct up_board_cpld_info *cpld_info = &led->pdata->cpld_info;
+
+	cpld_info->reg_set_bit(cpld_info->cpld, led->offset, value != LED_OFF);
+}
+
+static int up_board_leds_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct up_board_leds_pdata *pdata = dev_get_platdata(dev);
+	struct up_board_led *up_led;
+	int ret = 0;
+	size_t i;
+
+	for (i = 0; i < pdata->nled; i++) {
+		struct up_board_led_info *led_info = &pdata->leds[i];
+
+		up_led = devm_kzalloc(dev, sizeof(*up_led), GFP_KERNEL);
+		if (!up_led)
+			return -ENOMEM;
+
+		up_led->pdata = pdata;
+		up_led->offset = led_info->cpld_offset;
+		up_led->cdev.brightness_set = up_led_brightness_set;
+		up_led->cdev.name = led_info->name;
+
+		ret = devm_led_classdev_register(dev, &up_led->cdev);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver up_board_leds_driver = {
+	.driver.name	= "up-board-leds",
+	.driver.owner	= THIS_MODULE,
+	.probe		= up_board_leds_probe,
+};
+
+module_platform_driver(up_board_leds_driver);
+
+MODULE_AUTHOR("Javier Arteaga <javier@emutex.com>");
+MODULE_DESCRIPTION("UP Board LEDs driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:up-board-leds");
diff --git a/drivers/platform/x86/up_board_leds.h b/drivers/platform/x86/up_board_leds.h
new file mode 100644
index 0000000..473b1ad
--- /dev/null
+++ b/drivers/platform/x86/up_board_leds.h
@@ -0,0 +1,50 @@
+/*
+ * Copyright (c) 2016, Emutex Ltd.  All rights reserved.
+ *
+ * Author: Dan O'Donovan <dan@emutex.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef _UP_BOARD_LED_H_
+#define _UP_BOARD_LED_H_
+
+#include "up_board_cpld.h"
+
+/**
+ * struct up_board_led_info - information for an UP Board LED
+ * @name:		LED name
+ * @cpld_offset:	CPLD register bit offset for LED control
+ *
+ * Information for a single CPLD-controlled LED on the UP Board.
+ */
+struct up_board_led_info {
+	const char *name;
+	unsigned int cpld_offset;
+};
+
+/**
+ * struct up_board_leds_pdata - platform driver data
+ * @cpld_info:	CPLD configuration interface information
+ * @leds:	Array of LED information structures
+ * @nled:	Number of entries in leds array
+ *
+ * Platform data provided to UP Board CPLD LEDs platform device driver.
+ * Provides information for each CPLD-controlled LED on the UP Board.
+ */
+struct up_board_leds_pdata {
+	struct up_board_cpld_info cpld_info;
+	struct up_board_led_info *leds;
+	size_t nled;
+};
+
+#endif /* _UP_BOARD_LED_H_ */
-- 
2.1.4

[RESEND RFC PATCH 5/5] platform: x86: add platform driver for UP Board

[Dan O'Donovan]

This platform driver instantiates a platform device relevant to the
UP board, in particular a device representing the unique I/O pin CPLD
controller on the UP board.

In addition, this driver registers pin maps to configure
appropriately the underlying SoC GPIO pins for use with the
UP Board I/O pin header.

Signed-off-by: Dan O'Donovan <dan@emutex.com>
---
 drivers/platform/x86/Kconfig    |  13 ++++
 drivers/platform/x86/Makefile   |   5 ++
 drivers/platform/x86/up_board.c | 167 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 185 insertions(+)
 create mode 100644 drivers/platform/x86/up_board.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 3ec0025..b579adb 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1011,4 +1011,17 @@ config INTEL_TELEMETRY
 	  used to get various SoC events and parameters
 	  directly via debugfs files. Various tools may use
 	  this interface for SoC state monitoring.
+
+config UP_BOARD
+	bool "UP Board Platform I/O Driver"
+	depends on ACPI && PINCTRL_CHERRYVIEW
+	select GPIOLIB_IRQCHIP
+	select LEDS_CLASS
+	select NEW_LEDS
+	---help---
+	  This driver provides support for the platform functions on the UP
+	  board.  It includes platform, pinctrl and gpio drivers for the CPLD
+	  that manages the external pin header, as well as a driver for the
+	  built-in LEDs.
+
 endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 9b11b40..687c583 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -70,3 +70,8 @@ obj-$(CONFIG_INTEL_TELEMETRY)	+= intel_telemetry_core.o \
 				   intel_telemetry_pltdrv.o \
 				   intel_telemetry_debugfs.o
 obj-$(CONFIG_INTEL_PMC_CORE)    += intel_pmc_core.o
+obj-$(CONFIG_UP_BOARD)		+= up_board.o \
+				   up_board_cpld.o \
+				   up_board_pinctrl.o \
+				   up_board_gpio.o \
+				   up_board_leds.o
diff --git a/drivers/platform/x86/up_board.c b/drivers/platform/x86/up_board.c
new file mode 100644
index 0000000..8635759
--- /dev/null
+++ b/drivers/platform/x86/up_board.c
@@ -0,0 +1,167 @@
+
+/*
+ * UP Board platform driver.
+ *
+ * Copyright (c) 2016, Emutex Ltd.  All rights reserved.
+ *
+ * Author: Dan O'Donovan <dan@emutex.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/module.h>
+#include <linux/dmi.h>
+#include <linux/platform_device.h>
+#include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/regulator/fixed.h>
+#include <linux/regulator/machine.h>
+
+/* Internal context information for this driver */
+struct up_board_info {
+	struct platform_device *cpld_pdev;
+	struct platform_device *vreg_pdev;
+	struct pinctrl_map *pinmux_maps;
+	unsigned int num_pinmux_maps;
+};
+
+/*
+ * On the UP board, if the ODEn bit is set on the pad configuration
+ * it seems to impair some functions on the I/O header such as UART, SPI
+ * and I2C.  So we disable it for all header pins by default.
+ */
+static unsigned long oden_disable_conf[] = {
+	PIN_CONF_PACKED(PIN_CONFIG_DRIVE_PUSH_PULL, 0),
+};
+
+#define UP_PIN_MAP_MUX_GROUP(d, p, f) \
+	PIN_MAP_MUX_GROUP_DEFAULT(d, p, f "_grp", f)
+
+#define UP_PIN_MAP_CONF_ODEN(d, p, f) \
+	PIN_MAP_CONFIGS_GROUP_DEFAULT(d, p, f "_grp", oden_disable_conf)
+
+/* Maps pin functions on UP Board I/O pin header to specific CHT SoC devices */
+static struct pinctrl_map up_pinmux_maps[] __initdata = {
+	UP_PIN_MAP_MUX_GROUP("8086228A:00", "up-board-pinctrl", "uart1"),
+	UP_PIN_MAP_MUX_GROUP("808622C1:00", "up-board-pinctrl", "i2c0"),
+	UP_PIN_MAP_MUX_GROUP("808622C1:01", "up-board-pinctrl", "i2c1"),
+	UP_PIN_MAP_MUX_GROUP("80862288:00", "up-board-pinctrl", "pwm0"),
+	UP_PIN_MAP_MUX_GROUP("80862288:01", "up-board-pinctrl", "pwm1"),
+	UP_PIN_MAP_MUX_GROUP("8086228E:01", "up-board-pinctrl", "spi2"),
+	UP_PIN_MAP_MUX_GROUP("808622A8:00", "up-board-pinctrl", "i2s2"),
+	UP_PIN_MAP_MUX_GROUP("i2c-ADC081C:00", "up-board-pinctrl", "adc0"),
+
+	UP_PIN_MAP_MUX_GROUP("8086228A:00", "INT33FF:00", "uart1"),
+	UP_PIN_MAP_MUX_GROUP("808622C1:00", "INT33FF:00", "i2c0"),
+	UP_PIN_MAP_MUX_GROUP("808622C1:01", "INT33FF:00", "i2c1"),
+	UP_PIN_MAP_MUX_GROUP("808622C1:02", "INT33FF:00", "i2c2"),
+	UP_PIN_MAP_MUX_GROUP("80862288:00", "INT33FF:03", "pwm0"),
+	UP_PIN_MAP_MUX_GROUP("80862288:01", "INT33FF:03", "pwm1"),
+	UP_PIN_MAP_MUX_GROUP("8086228E:01", "INT33FF:03", "spi2"),
+	UP_PIN_MAP_MUX_GROUP("808622A8:00", "INT33FF:00", "lpe"),
+
+	UP_PIN_MAP_CONF_ODEN("8086228A:00", "INT33FF:00", "uart1"),
+	UP_PIN_MAP_CONF_ODEN("808622C1:00", "INT33FF:00", "i2c0"),
+	UP_PIN_MAP_CONF_ODEN("808622C1:01", "INT33FF:00", "i2c1"),
+	UP_PIN_MAP_CONF_ODEN("80862288:00", "INT33FF:03", "pwm0"),
+	UP_PIN_MAP_CONF_ODEN("80862288:01", "INT33FF:03", "pwm1"),
+	UP_PIN_MAP_CONF_ODEN("8086228E:01", "INT33FF:03", "spi2"),
+	UP_PIN_MAP_CONF_ODEN("808622A8:00", "INT33FF:00", "lpe"),
+};
+
+static struct up_board_info up_board_info = {
+	.pinmux_maps = up_pinmux_maps,
+	.num_pinmux_maps = ARRAY_SIZE(up_pinmux_maps),
+};
+
+static const struct dmi_system_id up_board_id_table[] __initconst = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "AAEON"),
+			DMI_MATCH(DMI_BOARD_NAME, "UP-CHT01"),
+			DMI_MATCH(DMI_BOARD_VERSION, "V0.4"),
+		},
+		.driver_data = &up_board_info,
+	},
+	{ }
+};
+
+static struct regulator_consumer_supply vref3v3_consumers[] = {
+	REGULATOR_SUPPLY("vref", "i2c-ADC081C:00"),
+};
+
+static struct up_board_info *up_board;
+
+static int __init
+up_board_init_devices(void)
+{
+	const struct dmi_system_id *system_id;
+	int ret;
+
+	system_id = dmi_first_match(up_board_id_table);
+	if (!system_id)
+		return -ENXIO;
+
+	up_board = system_id->driver_data;
+
+	/* Register pin control mappings specific to board version */
+	if (up_board->pinmux_maps) {
+		ret = pinctrl_register_mappings(up_board->pinmux_maps,
+						up_board->num_pinmux_maps);
+		if (ret) {
+			pr_err("Failed to register UP Board pinctrl mapping");
+			return ret;
+		}
+	}
+
+	/* Create a platform device to manage the UP Board I/O header CPLD */
+	up_board->cpld_pdev =
+		platform_device_register_simple("up-board-cpld",
+						PLATFORM_DEVID_NONE,
+						NULL, 0);
+	if (IS_ERR(up_board->cpld_pdev)) {
+		pr_err("Failed to register UP Board I/O CPLD platform device");
+		return PTR_ERR(up_board->cpld_pdev);
+	}
+
+	up_board->vreg_pdev =
+		regulator_register_always_on(0, "fixed-3.3V",
+					     vref3v3_consumers,
+					     ARRAY_SIZE(vref3v3_consumers),
+					     3300000);
+	if (!up_board->vreg_pdev) {
+		pr_err("Failed to register UP Board ADC vref regulator");
+		platform_device_unregister(up_board->cpld_pdev);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static void __exit
+up_board_exit(void)
+{
+	platform_device_unregister(up_board->vreg_pdev);
+	platform_device_unregister(up_board->cpld_pdev);
+}
+
+/*
+ * Using arch_initcall to ensure that pinmux maps are registered
+ * before the relevant devices are initialised
+ */
+arch_initcall(up_board_init_devices);
+module_exit(up_board_exit);
+
+MODULE_AUTHOR("Dan O'Donovan <dan@emutex.com>");
+MODULE_DESCRIPTION("Platform driver for UP Board");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("dmi:*:svnAAEON*:rnUP-CHT01:*");
-- 
2.1.4

Re: [RESEND RFC PATCH 5/5] platform: x86: add platform driver for UP Board

[Bryan O'Donoghue]

On Mon, 2016-07-04 at 17:07 +0100, Dan O'Donovan wrote:
> This platform driver instantiates a platform device relevant to the
> UP board, in particular a device representing the unique I/O pin CPLD
> controller on the UP board.
> 
> In addition, this driver registers pin maps to configure
> appropriately the underlying SoC GPIO pins for use with the
> UP Board I/O pin header.
> 
> Signed-off-by: Dan O'Donovan <dan@emutex.com>
> ---
>  drivers/platform/x86/Kconfig    |  13 ++++
>  drivers/platform/x86/Makefile   |   5 ++
>  drivers/platform/x86/up_board.c | 167
> ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 185 insertions(+)
>  create mode 100644 drivers/platform/x86/up_board.c
> 
> diff --git a/drivers/platform/x86/Kconfig
> b/drivers/platform/x86/Kconfig
> index 3ec0025..b579adb 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -1011,4 +1011,17 @@ config INTEL_TELEMETRY
>  	  used to get various SoC events and parameters
>  	  directly via debugfs files. Various tools may use
>  	  this interface for SoC state monitoring.
> +
> +config UP_BOARD
> +	bool "UP Board Platform I/O Driver"

Addressing your question in the cover letter - I'm not sure where
up_board.c should go but, I reckon up_board_leds.c should go into
drivers/leds, up_board_gpio.c should go into drivers/gpio etc.

My highly scientific feeling is that up_board.c and up_board_cpld.c
shouldn't be in this directory but, that they would fit well in
drivers/mfd.

> +	depends on ACPI && PINCTRL_CHERRYVIEW
> +	select GPIOLIB_IRQCHIP
> +	select LEDS_CLASS
> +	select NEW_LEDS
> +	---help---
> +	  This driver provides support for the platform functions on
> the UP
> +	  board.  It includes platform, pinctrl and gpio drivers for
> the CPLD
> +	  that manages the external pin header, as well as a driver
> for the
> +	  built-in LEDs.
> +
>  endif # X86_PLATFORM_DEVICES
> diff --git a/drivers/platform/x86/Makefile
> b/drivers/platform/x86/Makefile
> index 9b11b40..687c583 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -70,3 +70,8 @@ obj-$(CONFIG_INTEL_TELEMETRY)	+=
> intel_telemetry_core.o \
>  				   intel_telemetry_pltdrv.o \
>  				   intel_telemetry_debugfs.o
>  obj-$(CONFIG_INTEL_PMC_CORE)    += intel_pmc_core.o
> +obj-$(CONFIG_UP_BOARD)		+= up_board.o \
> +				   up_board_cpld.o \
> +				   up_board_pinctrl.o \
> +				   up_board_gpio.o \
> +				   up_board_leds.o
> diff --git a/drivers/platform/x86/up_board.c
> b/drivers/platform/x86/up_board.c
> new file mode 100644
> index 0000000..8635759
> --- /dev/null
> +++ b/drivers/platform/x86/up_board.c
> @@ -0,0 +1,167 @@
> +

Dangling extra line.

> +
> +/*
> + * On the UP board, if the ODEn bit 

Do you mean Open Drain Enable bit ? I think this comment will parse
better if its called out explicitly.

> is set on the pad configuration
> + * it seems to impair some functions on the I/O header such as UART,
> SPI
> + * and I2C.  So we disable it for all header pins by default.
> + */

Seems to impair is a bit vague... what does impair mean - not working
or just wrong ?

> +static struct up_board_info *up_board;
> +
> +static int __init
> +up_board_init_devices(void)

Why are you breaking up the __init and the function name onto separate
lines ?

> +{
> +	const struct dmi_system_id *system_id;
> +	int ret;
> +
> +	system_id = dmi_first_match(up_board_id_table);
> +	if (!system_id)
> +		return -ENXIO;

How about -ENODEV

> +
> +
> +	up_board->vreg_pdev =
> +		regulator_register_always_on(0, "fixed-3.3V",
> +					     vref3v3_consumers,
> +					     ARRAY_SIZE(vref3v3_cons
> umers),
> +					     3300000);
> +	if (!up_board->vreg_pdev) {
> +		pr_err("Failed to register UP Board ADC vref
> regulator");

dev_err(&up_board->cpld_pdev.dev) ?

> +		platform_device_unregister(up_board->cpld_pdev);
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
> +static void __exit
> +up_board_exit(void)

Same comment on multiple line declarations.

---
bod

Re: [RESEND RFC PATCH 0/5] platform drivers for UP Board

[Andy Shevchenko]

+Ismo

On Mon, 2016-07-04 at 17:07 +0100, Dan O'Donovan wrote:
> [Re-sending to a wider audience suggested by Darren Hart]
> 
> The UP Board is a new SBC based on the Intel Atom X5-Z8350 "Cherry 
> Trail" SoC and features a 40-pin I/O pin header and form-factor 
> inspired by the Raspberry Pi 2.
> 
> It utilises a CPLD between the SoC and the external 40-pin header
> to provide buffered voltage level-shifting of the I/O signals, mux
> switching and LED control, and programmable pin mapping between the
> SoC and the external pin header.
> 
> The gpio, pinctrl and led drivers provided in this patch series 
> enable and manage the functions provided by that CPLD.
> 
> I have some open questions about this patch series:
>  * Is it ok to place all of these various UP board drivers together
>    in drivers/platform/x86/, or would it be preferable to place them
>    in the respective sub-system directories (gpio, pinctrl, etc.)?
>    My rationale for keeping them together here is that they are all
>    specific to this UP Board platform and not expected to be
>    generally useful on any other platforms (except variants of UP).
>  * Is it acceptable to include hard-coded references to ACPI device
>    IDs (representing devices integrated on the SoC devices) for the
>    purpose of pin map and gpio references? Or is it required to
>    use only named gpio pins?
> 
> Any feedback/suggestions on the questions above, and the patch series
> in general, would be greatly appreciated!
> 
> Further information on the UP board can be obtained from [1] and [2].

Ismo, since you are doing something regarding Intel Galileo open
connected board I think you would be interested in this discussion since
it brings for example pinctrl driver for a _board_ which Galileo and
Edison/Arduino need.

> 
> [1] https://www.up-board.org
> [2] https://up-community.org
> 
> Dan O'Donovan (5):
>   platform: x86: add driver for UP Board I/O CPLD
>   platform: x86: add UP Board I/O pinctrl driver
>   platform: x86: add UP Board I/O gpio driver
>   platform: x86: add UP Board CPLD LED driver
>   platform: x86: add platform driver for UP Board
> 
>  drivers/platform/x86/Kconfig            |  13 +
>  drivers/platform/x86/Makefile           |   5 +
>  drivers/platform/x86/up_board.c         | 167 ++++++++++
>  drivers/platform/x86/up_board_cpld.c    | 560
> ++++++++++++++++++++++++++++++++
>  drivers/platform/x86/up_board_cpld.h    |  38 +++
>  drivers/platform/x86/up_board_gpio.c    | 254 +++++++++++++++
>  drivers/platform/x86/up_board_gpio.h    |  59 ++++
>  drivers/platform/x86/up_board_leds.c    |  85 +++++
>  drivers/platform/x86/up_board_leds.h    |  50 +++
>  drivers/platform/x86/up_board_pinctrl.c | 285 ++++++++++++++++
>  drivers/platform/x86/up_board_pinctrl.h | 102 ++++++
>  11 files changed, 1618 insertions(+)
>  create mode 100644 drivers/platform/x86/up_board.c
>  create mode 100644 drivers/platform/x86/up_board_cpld.c
>  create mode 100644 drivers/platform/x86/up_board_cpld.h
>  create mode 100644 drivers/platform/x86/up_board_gpio.c
>  create mode 100644 drivers/platform/x86/up_board_gpio.h
>  create mode 100644 drivers/platform/x86/up_board_leds.c
>  create mode 100644 drivers/platform/x86/up_board_leds.h
>  create mode 100644 drivers/platform/x86/up_board_pinctrl.c
>  create mode 100644 drivers/platform/x86/up_board_pinctrl.h
> 

-- 

Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [RESEND RFC PATCH 0/5] platform drivers for UP Board

[Andy Shevchenko]

On Mon, 2016-07-04 at 17:07 +0100, Dan O'Donovan wrote:
> [Re-sending to a wider audience suggested by Darren Hart]
> 
> The UP Board is a new SBC based on the Intel Atom X5-Z8350 "Cherry 
> Trail" SoC and features a 40-pin I/O pin header and form-factor 
> inspired by the Raspberry Pi 2.
> 
> It utilises a CPLD between the SoC and the external 40-pin header
> to provide buffered voltage level-shifting of the I/O signals, mux
> switching and LED control, and programmable pin mapping between the
> SoC and the external pin header.
> 
> The gpio, pinctrl and led drivers provided in this patch series 
> enable and manage the functions provided by that CPLD.
> 
> I have some open questions about this patch series:
>  * Is it ok to place all of these various UP board drivers together
>    in drivers/platform/x86/, or would it be preferable to place them
>    in the respective sub-system directories (gpio, pinctrl, etc.)?
>    My rationale for keeping them together here is that they are all
>    specific to this UP Board platform and not expected to be
>    generally useful on any other platforms (except variants of UP).
>  * Is it acceptable to include hard-coded references to ACPI device
>    IDs (representing devices integrated on the SoC devices) for the
>    purpose of pin map and gpio references? Or is it required to
>    use only named gpio pins?
> 
> Any feedback/suggestions on the questions above, and the patch series
> in general, would be greatly appreciated!

Looking closer to this and taking into account what is going on with
ACPI support for open connected boards I think this patch set is not
needed at all.

Basically most (everything?) you are trying to do in C code may and
should be done in ASL.

Mika, can you correct me if I'm wrong?

> 
> Further information on the UP board can be obtained from [1] and [2].
> 
> [1] https://www.up-board.org
> [2] https://up-community.org
> 
> Dan O'Donovan (5):
>   platform: x86: add driver for UP Board I/O CPLD
>   platform: x86: add UP Board I/O pinctrl driver
>   platform: x86: add UP Board I/O gpio driver
>   platform: x86: add UP Board CPLD LED driver
>   platform: x86: add platform driver for UP Board
> 
>  drivers/platform/x86/Kconfig            |  13 +
>  drivers/platform/x86/Makefile           |   5 +
>  drivers/platform/x86/up_board.c         | 167 ++++++++++
>  drivers/platform/x86/up_board_cpld.c    | 560
> ++++++++++++++++++++++++++++++++
>  drivers/platform/x86/up_board_cpld.h    |  38 +++
>  drivers/platform/x86/up_board_gpio.c    | 254 +++++++++++++++
>  drivers/platform/x86/up_board_gpio.h    |  59 ++++
>  drivers/platform/x86/up_board_leds.c    |  85 +++++
>  drivers/platform/x86/up_board_leds.h    |  50 +++
>  drivers/platform/x86/up_board_pinctrl.c | 285 ++++++++++++++++
>  drivers/platform/x86/up_board_pinctrl.h | 102 ++++++
>  11 files changed, 1618 insertions(+)
>  create mode 100644 drivers/platform/x86/up_board.c
>  create mode 100644 drivers/platform/x86/up_board_cpld.c
>  create mode 100644 drivers/platform/x86/up_board_cpld.h
>  create mode 100644 drivers/platform/x86/up_board_gpio.c
>  create mode 100644 drivers/platform/x86/up_board_gpio.h
>  create mode 100644 drivers/platform/x86/up_board_leds.c
>  create mode 100644 drivers/platform/x86/up_board_leds.h
>  create mode 100644 drivers/platform/x86/up_board_pinctrl.c
>  create mode 100644 drivers/platform/x86/up_board_pinctrl.h
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [RESEND RFC PATCH 0/5] platform drivers for UP Board

[Mika Westerberg]

On Tue, Sep 13, 2016 at 12:42:52PM +0300, Andy Shevchenko wrote:
> On Mon, 2016-07-04 at 17:07 +0100, Dan O'Donovan wrote:
> > [Re-sending to a wider audience suggested by Darren Hart]
> > 
> > The UP Board is a new SBC based on the Intel Atom X5-Z8350 "Cherry 
> > Trail" SoC and features a 40-pin I/O pin header and form-factor 
> > inspired by the Raspberry Pi 2.
> > 
> > It utilises a CPLD between the SoC and the external 40-pin header
> > to provide buffered voltage level-shifting of the I/O signals, mux
> > switching and LED control, and programmable pin mapping between the
> > SoC and the external pin header.
> > 
> > The gpio, pinctrl and led drivers provided in this patch series 
> > enable and manage the functions provided by that CPLD.
> > 
> > I have some open questions about this patch series:
> >  * Is it ok to place all of these various UP board drivers together
> >    in drivers/platform/x86/, or would it be preferable to place them
> >    in the respective sub-system directories (gpio, pinctrl, etc.)?
> >    My rationale for keeping them together here is that they are all
> >    specific to this UP Board platform and not expected to be
> >    generally useful on any other platforms (except variants of UP).
> >  * Is it acceptable to include hard-coded references to ACPI device
> >    IDs (representing devices integrated on the SoC devices) for the
> >    purpose of pin map and gpio references? Or is it required to
> >    use only named gpio pins?
> > 
> > Any feedback/suggestions on the questions above, and the patch series
> > in general, would be greatly appreciated!
> 
> Looking closer to this and taking into account what is going on with
> ACPI support for open connected boards I think this patch set is not
> needed at all.
> 
> Basically most (everything?) you are trying to do in C code may and
> should be done in ASL.
> 
> Mika, can you correct me if I'm wrong?

Sounds correct to me.

We have been working on a Yocto layer which allows one to ship your own
peripherals as ACPI ASL fragments which targets all Intel boards which
have ACPI BIOS:

  https://github.com/westeri/meta-acpi

For example adding GPIO leds is just matter of adding:

  ACPI_FRAGMENTS = "leds.asl"
  MACHINE = "minnowboard"

to your Yocto local.conf. The resulting image has these fragments
compiled and included in initrd where the kernel is able to pick them
up.

It is still very much work in progress, though :)

Re: [RESEND RFC PATCH 0/5] platform drivers for UP Board

[Dan O'Donovan]

On 09/13/2016 10:42 AM, Andy Shevchenko wrote:
> On Mon, 2016-07-04 at 17:07 +0100, Dan O'Donovan wrote:
>> [Re-sending to a wider audience suggested by Darren Hart]
>>
>> The UP Board is a new SBC based on the Intel Atom X5-Z8350 "Cherry 
>> Trail" SoC and features a 40-pin I/O pin header and form-factor 
>> inspired by the Raspberry Pi 2.
>>
>> It utilises a CPLD between the SoC and the external 40-pin header
>> to provide buffered voltage level-shifting of the I/O signals, mux
>> switching and LED control, and programmable pin mapping between the
>> SoC and the external pin header.
>>
>> The gpio, pinctrl and led drivers provided in this patch series 
>> enable and manage the functions provided by that CPLD.
>>
>> I have some open questions about this patch series:
>>  * Is it ok to place all of these various UP board drivers together
>>    in drivers/platform/x86/, or would it be preferable to place them
>>    in the respective sub-system directories (gpio, pinctrl, etc.)?
>>    My rationale for keeping them together here is that they are all
>>    specific to this UP Board platform and not expected to be
>>    generally useful on any other platforms (except variants of UP).
>>  * Is it acceptable to include hard-coded references to ACPI device
>>    IDs (representing devices integrated on the SoC devices) for the
>>    purpose of pin map and gpio references? Or is it required to
>>    use only named gpio pins?
>>
>> Any feedback/suggestions on the questions above, and the patch series
>> in general, would be greatly appreciated!
Firstly, many thanks to everyone for the valuable feedback provided on
this patch set so far, and apologies for the very delayed follow-up.  I
haven't abandoned it, but unfortunately haven't been able to get near it
for the past few weeks for various reasons.  I'm looking forward to
addressing all comments received as soon as I can.
> Looking closer to this and taking into account what is going on with
> ACPI support for open connected boards I think this patch set is not
> needed at all.
I do agree with your point about shifting a lot of this into ASL.  I do
think a driver is still needed though to manage the pin/led controller
(implemented on a CPLD).  Some previous comments have suggested an MFD
driver structure for representing the CPLD functions, which seems to
make sense. 
> Basically most (everything?) you are trying to do in C code may and
> should be done in ASL.
Among other things, one primary concept was to create a "virtual"
gpio-chip on top of this pin controller to represent the external header
pins, so that I could catch the run-time pin configuration updates (e.g.
gpio direction changes) and reconfigure the pin controller accordingly. 
It also had some other benefits, from my perspective, such as allowing a
more logical pin numbering scheme for those external pins.  I haven't
seen ASL examples yet that would indicate that it is possible to do it
all in ASL, but I'll do some more research on that front now to see what
I might be missing there.
>
> Mika, can you correct me if I'm wrong?
>
>> Further information on the UP board can be obtained from [1] and [2].
>>
>> [1] https://www.up-board.org
>> [2] https://up-community.org
>>
>> Dan O'Donovan (5):
>>   platform: x86: add driver for UP Board I/O CPLD
>>   platform: x86: add UP Board I/O pinctrl driver
>>   platform: x86: add UP Board I/O gpio driver
>>   platform: x86: add UP Board CPLD LED driver
>>   platform: x86: add platform driver for UP Board
>>
>>  drivers/platform/x86/Kconfig            |  13 +
>>  drivers/platform/x86/Makefile           |   5 +
>>  drivers/platform/x86/up_board.c         | 167 ++++++++++
>>  drivers/platform/x86/up_board_cpld.c    | 560
>> ++++++++++++++++++++++++++++++++
>>  drivers/platform/x86/up_board_cpld.h    |  38 +++
>>  drivers/platform/x86/up_board_gpio.c    | 254 +++++++++++++++
>>  drivers/platform/x86/up_board_gpio.h    |  59 ++++
>>  drivers/platform/x86/up_board_leds.c    |  85 +++++
>>  drivers/platform/x86/up_board_leds.h    |  50 +++
>>  drivers/platform/x86/up_board_pinctrl.c | 285 ++++++++++++++++
>>  drivers/platform/x86/up_board_pinctrl.h | 102 ++++++
>>  11 files changed, 1618 insertions(+)
>>  create mode 100644 drivers/platform/x86/up_board.c
>>  create mode 100644 drivers/platform/x86/up_board_cpld.c
>>  create mode 100644 drivers/platform/x86/up_board_cpld.h
>>  create mode 100644 drivers/platform/x86/up_board_gpio.c
>>  create mode 100644 drivers/platform/x86/up_board_gpio.h
>>  create mode 100644 drivers/platform/x86/up_board_leds.c
>>  create mode 100644 drivers/platform/x86/up_board_leds.h
>>  create mode 100644 drivers/platform/x86/up_board_pinctrl.c
>>  create mode 100644 drivers/platform/x86/up_board_pinctrl.h
>>