[PATCH v3 0/1] Pinctrl/gpio driver for Intel Baytrail platforms

[PATCH v3 0/1] Pinctrl/gpio driver for Intel Baytrail platforms

[Mathias Nyman]

This is the third version of the Intel baytrail GPIO driver, now moved to Pinctrl
subsystem.

As Linus W. pointed out the Intel Baytrail gpio looks more like a pin controller with
pad muxing and gpio-to-pad mappings than a pure gpio controller.
Even if firmware does its best to make sure we don't need to worry about the muxings
and other pinctrl features, there's a possibility these features might be useful in
the future and then pinctrl is easier to build on. 

Pinctrl also provides some generic gpio structures that are useful for this driver.        

This driver is built on top of linux-pinctrl tree and needs the 
"pinctrl: add pin list based GPIO ranges" patch by Christian Ruppert
(commit 2ff3477efd7086544b9e298fc63afab0645921b4)

Changes since v2:
- move driver from gpio/gpio-baytrail to drivers/pinctrl/pinctrl-baytrail
- use generic pinctrl_gpio_range structures from pinctrl instead of custom gpio_banks

Changes since v1:
- generic cleanups suggested by Andy S.
- removed text about pin muxing
- added missing 44:th pin to SUS controller
- added level triggering option
- prevent "forever loop" in case pin is stuck in "interrupt triggered" status,     

Mathias Nyman (1):
  pinctrl: add Intel BayTrail GPIO/pinctrl support

 drivers/pinctrl/Kconfig            |   12 +
 drivers/pinctrl/Makefile           |    1 +
 drivers/pinctrl/pinctrl-baytrail.c |  543 ++++++++++++++++++++++++++++++++++++
 3 files changed, 556 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pinctrl/pinctrl-baytrail.c

-- 
1.7.4.1

[PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Mathias Nyman]

Add support for gpio on Intel BayTrail platforms. BayTrail supports 3 banks
of gpios called SCORE, NCORE ans SUS with 102, 28 and 44 gpio pins.
Supports gpio interrupts and ACPI gpio events

Pins may be muxed to alternate function instead of gpio by firmware.
This driver does not touch the pin muxing and expect firmare
to set pin muxing and pullup/down properties properly.

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/pinctrl/Kconfig            |   12 +
 drivers/pinctrl/Makefile           |    1 +
 drivers/pinctrl/pinctrl-baytrail.c |  543 ++++++++++++++++++++++++++++++++++++
 3 files changed, 556 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pinctrl/pinctrl-baytrail.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 269c040..e01976f 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -58,6 +58,18 @@ config PINCTRL_AT91
 	help
 	  Say Y here to enable the at91 pinctrl driver
 
+config PINCTRL_BAYTRAIL
+	bool "Intel Baytrail GPIO pin control"
+	depends on GPIOLIB && ACPI && X86
+        select IRQ_DOMAIN
+	help
+	  driver for memory mapped GPIO functionality on Intel Baytrail
+	  platforms. Supports 3 banks with 102, 28 and 44 gpios.
+	  Most pins are usually muxed to some other functionality by firmware,
+	  so only a small amount is available for gpio use.
+
+	  Requires ACPI device enumeration code to set up a platform device.
+
 config PINCTRL_BCM2835
 	bool
 	select PINMUX
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index ff38aca..9031afd 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_PINCTRL_AB9540)	+= pinctrl-ab9540.o
 obj-$(CONFIG_PINCTRL_AB8505)	+= pinctrl-ab8505.o
 obj-$(CONFIG_PINCTRL_AT91)	+= pinctrl-at91.o
 obj-$(CONFIG_PINCTRL_BCM2835)	+= pinctrl-bcm2835.o
+obj-$(CONFIG_PINCTRL_BAYTRAIL)	+= pinctrl-baytrail.o
 obj-$(CONFIG_PINCTRL_IMX)	+= pinctrl-imx.o
 obj-$(CONFIG_PINCTRL_IMX35)	+= pinctrl-imx35.o
 obj-$(CONFIG_PINCTRL_IMX51)	+= pinctrl-imx51.o
diff --git a/drivers/pinctrl/pinctrl-baytrail.c b/drivers/pinctrl/pinctrl-baytrail.c
new file mode 100644
index 0000000..e9d735d
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-baytrail.c
@@ -0,0 +1,543 @@
+/*
+ * Pinctrl GPIO driver for Intel Baytrail
+ * Copyright (c) 2012-2013, Intel Corporation.
+ *
+ * Author: Mathias Nyman <mathias.nyman@linux.intel.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, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/bitops.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/gpio.h>
+#include <linux/irqdomain.h>
+#include <linux/acpi.h>
+#include <linux/acpi_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/seq_file.h>
+#include <linux/io.h>
+#include <linux/pm_runtime.h>
+#include <linux/pinctrl/pinctrl.h>
+
+/* memory mapped register offsets */
+#define BYT_CONF0_REG		0x000
+#define BYT_CONF1_REG		0x004
+#define BYT_VAL_REG		0x008
+#define BYT_DFT_REG		0x00c
+#define BYT_INT_STAT_REG	0x800
+
+/* BYT_CONF0_REG register bits */
+#define BYT_TRIG_NEG		BIT(26)
+#define BYT_TRIG_POS		BIT(25)
+#define BYT_TRIG_LVL		BIT(24)
+#define BYT_PIN_MUX		0x07
+
+/* BYT_VAL_REG register bits */
+#define BYT_INPUT_EN		BIT(2)  /* 0: input enabled (active low)*/
+#define BYT_OUTPUT_EN		BIT(1)  /* 0: output enabled (active low)*/
+#define BYT_LEVEL		BIT(0)
+
+#define BYT_DIR_MASK		(BIT(1) | BIT(2))
+#define BYT_TRIG_MASK		(BIT(26) | BIT(25) | BIT(24))
+
+#define BYT_NGPIO_SCORE		102
+#define BYT_NGPIO_NCORE		28
+#define BYT_NGPIO_SUS		44
+
+/*
+ * Baytrail gpio controller consist of three separate sub-controllers called
+ * SCORE, NCORE and SUS. The sub-controllers are identified by their acpi UID.
+ *
+ * GPIO numbering is _not_ ordered meaning that gpio # 0 in ACPI namespace does
+ * _not_ correspond to the first gpio register at controller's gpio base.
+ * There is no logic or pattern in mapping gpio numbers to registers (pads) so
+ * each sub-controller needs to have its own mapping table
+ */
+
+/* score_pins[gpio_nr] = pad_nr */
+
+static unsigned const score_pins[BYT_NGPIO_SCORE] = {
+	85, 89, 93, 96, 99, 102, 98, 101, 34, 37,
+	36, 38, 39, 35, 40, 84, 62, 61, 64, 59,
+	54, 56, 60, 55, 63, 57, 51, 50, 53, 47,
+	52, 49, 48, 43, 46, 41, 45, 42, 58, 44,
+	95, 105, 70, 68, 67, 66, 69, 71, 65, 72,
+	86, 90, 88, 92, 103, 77, 79, 83, 78, 81,
+	80, 82, 13, 12, 15, 14, 17, 18, 19, 16,
+	2, 1, 0, 4, 6, 7, 9, 8, 33, 32,
+	31, 30, 29, 27, 25, 28, 26, 23, 21, 20,
+	24, 22, 5, 3, 10, 11, 106, 87, 91, 104,
+	97, 100,
+};
+
+static unsigned const ncore_pins[BYT_NGPIO_NCORE] = {
+	19, 18, 17, 20, 21, 22, 24, 25, 23, 16,
+	14, 15, 12, 26, 27, 1, 4, 8, 11, 0,
+	3, 6, 10, 13, 2, 5, 9, 7,
+};
+
+static unsigned const sus_pins[BYT_NGPIO_SUS] = {
+	29, 33, 30, 31, 32, 34, 36, 35, 38, 37,
+	18, 7, 11, 20, 17, 1, 8, 10, 19, 12,
+	0, 2, 23, 39, 28, 27, 22, 21, 24, 25,
+	26, 51, 56, 54, 49, 55, 48, 57, 50, 58,
+	52, 53, 59, 40,
+};
+
+static struct pinctrl_gpio_range byt_ranges[] = {
+	{
+		.name = "1", /* match with acpi _UID in probe */
+		.npins = BYT_NGPIO_SCORE,
+		.pins = score_pins,
+	},
+	{
+		.name = "2",
+		.npins = BYT_NGPIO_NCORE,
+		.pins = ncore_pins,
+	},
+	{
+		.name = "3",
+		.npins = BYT_NGPIO_SUS,
+		.pins = sus_pins,
+	},
+	{
+	},
+};
+
+struct byt_gpio {
+	struct gpio_chip		chip;
+	struct irq_domain		*domain;
+	struct platform_device		*pdev;
+	spinlock_t			lock;
+	void __iomem			*reg_base;
+	struct pinctrl_gpio_range	*range;
+};
+
+static void __iomem *byt_gpio_reg(struct gpio_chip *chip, unsigned offset,
+				 int reg)
+{
+	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	u32 reg_offset;
+	void __iomem *ptr;
+
+	if (reg == BYT_INT_STAT_REG)
+		reg_offset = (offset / 32) * 4;
+	else
+		reg_offset = vg->range->pins[offset] * 16;
+
+	ptr = (void __iomem *) (vg->reg_base + reg_offset + reg);
+	return ptr;
+}
+
+static int byt_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+
+	pm_runtime_get(&vg->pdev->dev);
+
+	return 0;
+}
+
+static void byt_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	void __iomem *reg = byt_gpio_reg(&vg->chip, offset, BYT_CONF0_REG);
+	u32 value;
+
+	/* clear interrupt triggering */
+	value = readl(reg);
+	value &= ~(BYT_TRIG_POS | BYT_TRIG_NEG | BYT_TRIG_LVL);
+	writel(value, reg);
+
+	pm_runtime_put(&vg->pdev->dev);
+}
+
+static int byt_irq_type(struct irq_data *d, unsigned type)
+{
+	struct byt_gpio *vg = irq_data_get_irq_chip_data(d);
+	u32 offset = irqd_to_hwirq(d);
+	u32 value;
+	unsigned long flags;
+	void __iomem *reg = byt_gpio_reg(&vg->chip, offset, BYT_CONF0_REG);
+
+	if (offset >= vg->chip.ngpio)
+		return -EINVAL;
+
+	spin_lock_irqsave(&vg->lock, flags);
+	value = readl(reg);
+
+	/* For level trigges the BYT_TRIG_POS and BYT_TRIG_NEG bits
+	 * are used to indicate high and low level triggering
+	 */
+	value &= ~(BYT_TRIG_POS | BYT_TRIG_NEG | BYT_TRIG_LVL);
+
+	switch (type) {
+	case IRQ_TYPE_LEVEL_HIGH:
+		value |= BYT_TRIG_LVL;
+	case IRQ_TYPE_EDGE_RISING:
+		value |= BYT_TRIG_POS;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		value |= BYT_TRIG_LVL;
+	case IRQ_TYPE_EDGE_FALLING:
+		value |= BYT_TRIG_NEG;
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		value |= (BYT_TRIG_NEG | BYT_TRIG_POS);
+		break;
+	}
+	writel(value, reg);
+
+	spin_unlock_irqrestore(&vg->lock, flags);
+
+	return 0;
+}
+
+static int byt_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+	void __iomem *reg = byt_gpio_reg(chip, offset, BYT_VAL_REG);
+	return readl(reg) & BYT_LEVEL;
+}
+
+static void byt_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	void __iomem *reg = byt_gpio_reg(chip, offset, BYT_VAL_REG);
+	unsigned long flags;
+	u32 old_val;
+
+	spin_lock_irqsave(&vg->lock, flags);
+
+	old_val = readl(reg);
+
+	if (value)
+		writel(old_val | BYT_LEVEL, reg);
+	else
+		writel(old_val & ~BYT_LEVEL, reg);
+
+	spin_unlock_irqrestore(&vg->lock, flags);
+}
+
+static int byt_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	void __iomem *reg = byt_gpio_reg(chip, offset, BYT_VAL_REG);
+	unsigned long flags;
+	u32 value;
+
+	spin_lock_irqsave(&vg->lock, flags);
+
+	value = readl(reg) | BYT_DIR_MASK;
+	value = value & (~BYT_INPUT_EN); /* active low */
+	writel(value, reg);
+
+	spin_unlock_irqrestore(&vg->lock, flags);
+
+	return 0;
+}
+
+static int byt_gpio_direction_output(struct gpio_chip *chip,
+				     unsigned gpio, int value)
+{
+	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	void __iomem *reg = byt_gpio_reg(chip, gpio, BYT_VAL_REG);
+	unsigned long flags;
+	u32 reg_val;
+
+	spin_lock_irqsave(&vg->lock, flags);
+
+	reg_val = readl(reg) | (BYT_DIR_MASK | !!value);
+	reg_val &= ~(BYT_OUTPUT_EN | !value);
+	writel(reg_val, reg);
+
+	spin_unlock_irqrestore(&vg->lock, flags);
+
+	return 0;
+}
+
+static void byt_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
+{
+	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	int i;
+	unsigned long flags;
+	u32 conf0, val, offs;
+
+	spin_lock_irqsave(&vg->lock, flags);
+
+	for (i = 0; i < vg->chip.ngpio; i++) {
+		offs = vg->range->pins[i] * 16;
+		conf0 = readl(vg->reg_base + offs + BYT_CONF0_REG);
+		val = readl(vg->reg_base + offs + BYT_VAL_REG);
+
+		seq_printf(s,
+			   " gpio-%-3d %s %s %s pad-%-3d offset:0x%03x mux:%d %s%s%s\n",
+			   i,
+			   val & BYT_INPUT_EN ? "  " : "in",
+			   val & BYT_OUTPUT_EN ? "   " : "out",
+			   val & BYT_LEVEL ? "hi" : "lo",
+			   vg->range->pins[i], offs,
+			   conf0 & 0x7,
+			   conf0 & BYT_TRIG_NEG ? "fall " : "",
+			   conf0 & BYT_TRIG_POS ? "rise " : "",
+			   conf0 & BYT_TRIG_LVL ? "lvl " : "");
+	}
+	spin_unlock_irqrestore(&vg->lock, flags);
+}
+
+static int byt_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+	struct byt_gpio *vg = container_of(chip, struct byt_gpio, chip);
+	return irq_create_mapping(vg->domain, offset);
+}
+
+static void byt_gpio_irq_handler(unsigned irq, struct irq_desc *desc)
+{
+	struct irq_data *data = irq_desc_get_irq_data(desc);
+	struct byt_gpio *vg = irq_data_get_irq_handler_data(data);
+	struct irq_chip *chip = irq_data_get_irq_chip(data);
+	u32 base, pin, mask;
+	void __iomem *reg;
+	u32 pending;
+	unsigned virq;
+	int looplimit = 0;
+
+	/* check from GPIO controller which pin triggered the interrupt */
+	for (base = 0; base < vg->chip.ngpio; base += 32) {
+
+		reg = byt_gpio_reg(&vg->chip, base, BYT_INT_STAT_REG);
+
+		while ((pending = readl(reg))) {
+			pin = __ffs(pending);
+			mask = BIT(pin);
+			/* Clear before handling so we can't lose an edge */
+			writel(mask, reg);
+
+			virq = irq_find_mapping(vg->domain, base + pin);
+			generic_handle_irq(virq);
+
+			/* In case bios or user sets triggering incorretly a pin
+			 * might remain in "interrupt triggered" state.
+			 */
+			if (looplimit++ > 32) {
+				dev_err(&vg->pdev->dev,
+					"Gpio %d interrupt flood, disabling\n",
+					base + pin);
+
+				reg = byt_gpio_reg(&vg->chip, base + pin,
+						   BYT_CONF0_REG);
+				mask = readl(reg);
+				mask &= ~(BYT_TRIG_NEG | BYT_TRIG_POS |
+					  BYT_TRIG_LVL);
+				writel(mask, reg);
+				mask = readl(reg); /* flush */
+				break;
+			}
+		}
+	}
+	chip->irq_eoi(data);
+}
+
+static void byt_irq_unmask(struct irq_data *d)
+{
+}
+
+static void byt_irq_mask(struct irq_data *d)
+{
+}
+
+static struct irq_chip byt_irqchip = {
+	.name = "BYT-GPIO",
+	.irq_mask = byt_irq_mask,
+	.irq_unmask = byt_irq_unmask,
+	.irq_set_type = byt_irq_type,
+};
+
+static void byt_gpio_irq_init_hw(struct byt_gpio *vg)
+{
+	void __iomem *reg;
+	u32 base, value;
+
+	/* clear interrupt status trigger registers */
+	for (base = 0; base < vg->chip.ngpio; base += 32) {
+		reg = byt_gpio_reg(&vg->chip, base, BYT_INT_STAT_REG);
+		writel(0xffffffff, reg);
+		/* make sure trigger bits are cleared, if not then a pin
+		   might be misconfigured in bios */
+		value = readl(reg);
+		if (value)
+			dev_err(&vg->pdev->dev,
+				"GPIO interrupt error, pins misconfigured\n");
+	}
+}
+
+static int byt_gpio_irq_map(struct irq_domain *d, unsigned int virq,
+			    irq_hw_number_t hw)
+{
+	struct byt_gpio *vg = d->host_data;
+
+	irq_set_chip_and_handler_name(virq, &byt_irqchip, handle_simple_irq,
+				      "demux");
+	irq_set_chip_data(virq, vg);
+	irq_set_irq_type(virq, IRQ_TYPE_NONE);
+
+	return 0;
+}
+
+static const struct irq_domain_ops byt_gpio_irq_ops = {
+	.map = byt_gpio_irq_map,
+};
+
+static int byt_gpio_probe(struct platform_device *pdev)
+{
+	struct byt_gpio *vg;
+	struct gpio_chip *gc;
+	struct resource *mem_rc, *irq_rc;
+	struct device *dev = &pdev->dev;
+	struct acpi_device *acpi_dev;
+	struct pinctrl_gpio_range *range;
+	acpi_handle handle = ACPI_HANDLE(dev);
+	unsigned hwirq;
+	int ret;
+
+	if (acpi_bus_get_device(handle, &acpi_dev))
+		return -ENODEV;
+
+	vg = devm_kzalloc(dev, sizeof(struct byt_gpio), GFP_KERNEL);
+	if (!vg) {
+		dev_err(&pdev->dev, "can't allocate byt_gpio chip data\n");
+		return -ENOMEM;
+	}
+
+	for (range = byt_ranges; range->name; range++) {
+		if (!strcmp(acpi_dev->pnp.unique_id, range->name)) {
+			vg->chip.ngpio = range->npins;
+			vg->range = range;
+			break;
+		}
+	}
+
+	if (!vg->chip.ngpio || !vg->range)
+		return -ENODEV;
+
+	vg->pdev = pdev;
+	platform_set_drvdata(pdev, vg);
+
+	mem_rc = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	vg->reg_base = devm_ioremap_resource(dev, mem_rc);
+	if (IS_ERR(vg->reg_base))
+		return PTR_ERR(vg->reg_base);
+
+	spin_lock_init(&vg->lock);
+
+	gc = &vg->chip;
+	gc->label = dev_name(&pdev->dev);
+	gc->owner = THIS_MODULE;
+	gc->request = byt_gpio_request;
+	gc->free = byt_gpio_free;
+	gc->direction_input = byt_gpio_direction_input;
+	gc->direction_output = byt_gpio_direction_output;
+	gc->get = byt_gpio_get;
+	gc->set = byt_gpio_set;
+	gc->dbg_show = byt_gpio_dbg_show;
+	gc->base = -1;
+	gc->can_sleep = 0;
+	gc->dev = dev;
+
+	ret = gpiochip_add(gc);
+	if (ret) {
+		dev_err(&pdev->dev, "failed adding byt-gpio chip\n");
+		return ret;
+	}
+
+	/* set up interrupts  */
+	irq_rc = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (irq_rc && irq_rc->start) {
+		hwirq = irq_rc->start;
+		gc->to_irq = byt_gpio_to_irq;
+
+		vg->domain = irq_domain_add_linear(NULL, gc->ngpio,
+						   &byt_gpio_irq_ops, vg);
+		if (!vg->domain)
+			return -ENXIO;
+
+		byt_gpio_irq_init_hw(vg);
+
+		irq_set_handler_data(hwirq, vg);
+		irq_set_chained_handler(hwirq, byt_gpio_irq_handler);
+
+		/* Register interrupt handlers for gpio signaled acpi events */
+		acpi_gpiochip_request_interrupts(gc);
+	}
+
+	pm_runtime_enable(dev);
+
+	return 0;
+}
+
+static int byt_gpio_runtime_suspend(struct device *dev)
+{
+	return 0;
+}
+
+static int byt_gpio_runtime_resume(struct device *dev)
+{
+	return 0;
+}
+
+static const struct dev_pm_ops byt_gpio_pm_ops = {
+	.runtime_suspend = byt_gpio_runtime_suspend,
+	.runtime_resume = byt_gpio_runtime_resume,
+};
+
+static const struct acpi_device_id byt_gpio_acpi_match[] = {
+	{ "INT33B2", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, byt_gpio_acpi_match);
+
+static int byt_gpio_remove(struct platform_device *pdev)
+{
+	struct byt_gpio *vg = platform_get_drvdata(pdev);
+	int err;
+	pm_runtime_disable(&pdev->dev);
+	err = gpiochip_remove(&vg->chip);
+	if (err)
+		dev_warn(&pdev->dev, "failed to remove gpio_chip.\n");
+
+	return 0;
+}
+
+static struct platform_driver byt_gpio_driver = {
+	.probe          = byt_gpio_probe,
+	.remove         = byt_gpio_remove,
+	.driver         = {
+		.name   = "byt_gpio",
+		.owner  = THIS_MODULE,
+		.pm	= &byt_gpio_pm_ops,
+		.acpi_match_table = ACPI_PTR(byt_gpio_acpi_match),
+	},
+};
+
+static int __init byt_gpio_init(void)
+{
+	return platform_driver_register(&byt_gpio_driver);
+}
+
+subsys_initcall(byt_gpio_init);
-- 
1.7.4.1

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Linus Walleij]

On Tue, Jun 18, 2013 at 1:33 PM, Mathias Nyman
<mathias.nyman@linux.intel.com> wrote:

> Add support for gpio on Intel BayTrail platforms. BayTrail supports 3 banks
> of gpios called SCORE, NCORE ans SUS with 102, 28 and 44 gpio pins.
> Supports gpio interrupts and ACPI gpio events
>
> Pins may be muxed to alternate function instead of gpio by firmware.
> This driver does not touch the pin muxing and expect firmare
> to set pin muxing and pullup/down properties properly.
>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>

I have a feeling this driver will evolve quite a bit and eventually
register a pure pinctrl interface as well (currently it's only using
the ranges as some data container...)

Anyway, it's a good starting point and obviously (I guess?)
gets your hardware up an ticking, so let's take this as a
starting point.

So patch applied.

This thing only seems to use gpiolib-acpi.c for the basic
device matching and IRQ handling backend, is that correct?

What I'm thinking of moving forward is that I have seen
ACPI fragments with things like "PullUp" etc, which is pinctrl
domain, so we may come to need some generic ACPI helpers
inside drivers/pinctrl as well sooner or later.

Yours,
Linus Walleij

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Mathias Nyman]

On 06/18/2013 06:17 PM, Linus Walleij wrote:
> On Tue, Jun 18, 2013 at 1:33 PM, Mathias Nyman
> <mathias.nyman@linux.intel.com>  wrote:
>
>> Add support for gpio on Intel BayTrail platforms. BayTrail supports 3 banks
>> of gpios called SCORE, NCORE ans SUS with 102, 28 and 44 gpio pins.
>> Supports gpio interrupts and ACPI gpio events
>>
>> Pins may be muxed to alternate function instead of gpio by firmware.
>> This driver does not touch the pin muxing and expect firmare
>> to set pin muxing and pullup/down properties properly.
>>
>> Signed-off-by: Mathias Nyman<mathias.nyman@linux.intel.com>
>
> I have a feeling this driver will evolve quite a bit and eventually
> register a pure pinctrl interface as well (currently it's only using
> the ranges as some data container...)
>
> Anyway, it's a good starting point and obviously (I guess?)
> gets your hardware up an ticking, so let's take this as a
> starting point.
>
> So patch applied.
>

Thanks, much appreciated.
We'll see how it evolves.

> This thing only seems to use gpiolib-acpi.c for the basic
> device matching and IRQ handling backend, is that correct?

It only uses gpiolib-acpi.c for handling GPIO-signaled ACPI Events,
which are like SCI events on hardware reduced ACPI platforms.

Basically It's ACPI saying "I have a firmware method that needs to be 
run if a certain gpio is triggered, please do that for me"

So the acpi_gpiochip_request_interrupts() registers interrupt handlers 
which call ACPI firmware methods for those gpio interrupts.

Otherwise gpiolib-acpi.c is useful for other device drivers to translate 
ACPI gpio resource numbers to linux gpio numbers. In ACPI tables the 
gpio resource numbers are per controller and zero based.
>
> What I'm thinking of moving forward is that I have seen
> ACPI fragments with things like "PullUp" etc, which is pinctrl
> domain, so we may come to need some generic ACPI helpers
> inside drivers/pinctrl as well sooner or later.

Probably yes, I'm not an expert on ACPI (or pinctrl), but ACPI5 added 
the GpioInt and GpioIo resources for devices. GpioInt resource 
descriptor has the follwing arguments: GpioInt(EdgeLevel, ActiveLevel, 
Shared, PinConfig, DebounceTimeout, ResourceSource,
esourceSourceIndex, ResourceUsage, DescriptorName, VendorData) {PinList}

Where the PinConfig argument can be PullDefault, PullUp, PullDown, 
PullNone or some vendor specific value.

ResourceSource tells which gpio controller the pin belongs to.

Right now drivers only have helpers for translating the ACPI gpio pin 
number to linux gpio number.

-Mathias

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Timur Tabi]

On Tue, Jun 18, 2013 at 6:33 AM, Mathias Nyman
<mathias.nyman@linux.intel.com> wrote:
>
> Pins may be muxed to alternate function instead of gpio by firmware.
> This driver does not touch the pin muxing and expect firmare
> to set pin muxing and pullup/down properties properly.
>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
>  drivers/pinctrl/Kconfig            |   12 +
>  drivers/pinctrl/Makefile           |    1 +
>  drivers/pinctrl/pinctrl-baytrail.c |  543 ++++++++++++++++++++++++++++++++++++


I know it's been ten months since you posted this driver, but I have a
question.  If this driver does not touch the pin muxing, and it
doesn't even call pinctrl_register(), then why is it in
drivers/pinctrl?  It's not a pinctrl driver.  Why isn't this a regular
GPIO drivers in drivers/gpio?

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Mathias Nyman]

On 04/12/2014 01:54 AM, Timur Tabi wrote:
> On Tue, Jun 18, 2013 at 6:33 AM, Mathias Nyman
> <mathias.nyman@linux.intel.com> wrote:
>>
>> Pins may be muxed to alternate function instead of gpio by firmware.
>> This driver does not touch the pin muxing and expect firmare
>> to set pin muxing and pullup/down properties properly.
>>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> ---
>>   drivers/pinctrl/Kconfig            |   12 +
>>   drivers/pinctrl/Makefile           |    1 +
>>   drivers/pinctrl/pinctrl-baytrail.c |  543 ++++++++++++++++++++++++++++++++++++
>
>
> I know it's been ten months since you posted this driver, but I have a
> question.  If this driver does not touch the pin muxing, and it
> doesn't even call pinctrl_register(), then why is it in
> drivers/pinctrl?  It's not a pinctrl driver.  Why isn't this a regular
> GPIO drivers in drivers/gpio?
>

This was the conclusion we reached after some discussion with Linus W.
Initially this was just a GPIO driver, but Linus correctly spotted that 
Baytrail has many pinctrl-like features (like pin muxing, etc) that we 
might need to address in the future.

threads where this was discussed:

http://marc.info/?l=linux-kernel&m=136994203308585&w=2
http://marc.info/?l=linux-kernel&m=137113578604763&w=2

-Mathias

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Timur Tabi]

On 04/14/2014 02:52 AM, Mathias Nyman wrote:
>>
>
> This was the conclusion we reached after some discussion with Linus W.
> Initially this was just a GPIO driver, but Linus correctly spotted that
> Baytrail has many pinctrl-like features (like pin muxing, etc) that we
> might need to address in the future.
>
> threads where this was discussed:
>
> http://marc.info/?l=linux-kernel&m=136994203308585&w=2
> http://marc.info/?l=linux-kernel&m=137113578604763&w=2

So this is the interesting part:

> We expect BIOS to set all pin configurations correctly.

This device will only be used on an ACPI system, right?  And isn't ACPI 
supposed to hide all the pinctrl programming from the OS?  I thought 
that was the whole point behind ACPI and the reason why ARM64 isn't 
going to use device trees.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by the Linux Foundation.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Mathias Nyman]

On 04/14/2014 06:11 PM, Timur Tabi wrote:
> On 04/14/2014 02:52 AM, Mathias Nyman wrote:
>>>
>>
>> This was the conclusion we reached after some discussion with Linus W.
>> Initially this was just a GPIO driver, but Linus correctly spotted that
>> Baytrail has many pinctrl-like features (like pin muxing, etc) that we
>> might need to address in the future.
>>
>> threads where this was discussed:
>>
>> http://marc.info/?l=linux-kernel&m=136994203308585&w=2
>> http://marc.info/?l=linux-kernel&m=137113578604763&w=2
>
> So this is the interesting part:
>
>> We expect BIOS to set all pin configurations correctly.
>
> This device will only be used on an ACPI system, right?  And isn't ACPI
> supposed to hide all the pinctrl programming from the OS?  I thought
> that was the whole point behind ACPI and the reason why ARM64 isn't
> going to use device trees.
>

This was my starting point as well, and the driver was initially 
submitted as a GPIO driver. But Linus W. suggested pinctrl instead, and 
as he's the maintainer of both those subsystem I trust his judgment.


-Mathias

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Timur Tabi]

On 04/15/2014 05:01 AM, Mathias Nyman wrote:
>>
>> This device will only be used on an ACPI system, right?  And isn't ACPI
>> supposed to hide all the pinctrl programming from the OS?  I thought
>> that was the whole point behind ACPI and the reason why ARM64 isn't
>> going to use device trees.
>>
>
> This was my starting point as well, and the driver was initially
> submitted as a GPIO driver. But Linus W. suggested pinctrl instead, and
> as he's the maintainer of both those subsystem I trust his judgment.

Do you think, for an ACPI pinctrl driver, that we will need to specify 
any function groups?  When I look at the ASL that configures GPIOs, I 
see only lines like this:

GpioIo (Exclusive, PullDefault, , , , "\\GIO0") {0x1D, 0x1E}

This tells me that ACPI will never use any of the names that are 
defined.  I see that .get_function_name is called on my ACPI system, but 
I don't see where it is used.

The reason I ask is because I would like to make a "generic" ACPI 
pinctrl/gpio driver that doesn't specify any pin groups.  So if we use 
the same pinctrl/gpio hardware on multiple SOCs, the only thing that the 
driver needs from ACPI is the number of pins.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by the Linux Foundation.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Mathias Nyman]

On 04/17/2014 07:47 PM, Timur Tabi wrote:
> On 04/15/2014 05:01 AM, Mathias Nyman wrote:
>>>
>>> This device will only be used on an ACPI system, right?  And isn't ACPI
>>> supposed to hide all the pinctrl programming from the OS?  I thought
>>> that was the whole point behind ACPI and the reason why ARM64 isn't
>>> going to use device trees.
>>>
>>
>> This was my starting point as well, and the driver was initially
>> submitted as a GPIO driver. But Linus W. suggested pinctrl instead, and
>> as he's the maintainer of both those subsystem I trust his judgment.
>
> Do you think, for an ACPI pinctrl driver, that we will need to specify
> any function groups?  When I look at the ASL that configures GPIOs, I
> see only lines like this:
>
> GpioIo (Exclusive, PullDefault, , , , "\\GIO0") {0x1D, 0x1E}
>
> This tells me that ACPI will never use any of the names that are
> defined.  I see that .get_function_name is called on my ACPI system, but
> I don't see where it is used.
>
> The reason I ask is because I would like to make a "generic" ACPI
> pinctrl/gpio driver that doesn't specify any pin groups.  So if we use
> the same pinctrl/gpio hardware on multiple SOCs, the only thing that the
> driver needs from ACPI is the number of pins.
>

Mika, Rafael (and me) have done some effort already on supporting ACPI 
gpio resources.

ACPI code creates a platfrom device out of the GPIO device defined ACPI 
tables, with all the resources set, and a hook back to the ACPI node.

Helper functions to translate the ACPI GpioIO and GpioInt resources to 
linux gpio numbers can be found in gpio/gpiolib-acpi.c together with 
other ACPI and gpio related helper functions.

see also
Documentation/acpi/enumeration.txt for some GPIO and ACPI related info

-Mathias

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Timur Tabi]

Mathias Nyman wrote:
>
> Helper functions to translate the ACPI GpioIO and GpioInt resources to
> linux gpio numbers can be found in gpio/gpiolib-acpi.c together with
> other ACPI and gpio related helper functions.

Does this also cover pin control and pin muxing?  Sorry, but I sometimes 
I get confused.  It appears to me that the GpioIo and GpioInt functions 
require pin muxing, but I don't see that support in gpiolib-acpi.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Westerberg, Mika]

On Wed, Apr 23, 2014 at 07:07:13AM -0500, Timur Tabi wrote:
> Mathias Nyman wrote:
> >
> >Helper functions to translate the ACPI GpioIO and GpioInt resources to
> >linux gpio numbers can be found in gpio/gpiolib-acpi.c together with
> >other ACPI and gpio related helper functions.
> 
> Does this also cover pin control and pin muxing?  Sorry, but I
> sometimes I get confused.  It appears to me that the GpioIo and
> GpioInt functions require pin muxing, but I don't see that support
> in gpiolib-acpi.

It doesn't do any pin control nor muxing and I'm not sure if it is
required. Can you elaborate why you think pin muxing is required with
GpioIo/GpioInt resources?

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Timur Tabi]

Westerberg, Mika wrote:
> It doesn't do any pin control nor muxing and I'm not sure if it is
> required. Can you elaborate why you think pin muxing is required with
> GpioIo/GpioInt resources?

How are the pin muxes normally configured in ACPI?  All of our GPIOs 
have a pinmux on them, and so if you want to use the pin for the 
non-default functionality, you need to configure the mux.  Isn't that 
supposed to happen with the through the pinctrl driver?  That is, when 
the kernel parses the ASL, and it seems a command to configure pin #3 to 
function #4, it calls the local pinctrl driver to do that?

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Linus Walleij]

On Wed, Apr 23, 2014 at 5:14 PM, Timur Tabi <timur@codeaurora.org> wrote:

> How are the pin muxes normally configured in ACPI?

VERY good question.

>  All of our GPIOs have a
> pinmux on them, and so if you want to use the pin for the non-default
> functionality, you need to configure the mux.  Isn't that supposed to happen
> with the through the pinctrl driver?  That is, when the kernel parses the
> ASL, and it seems a command to configure pin #3 to function #4, it calls the
> local pinctrl driver to do that?

Let's suspect this is true. That is how GPIO work after all is it not?

I guess the other option would be that all systems must come with a
tailored set-up written in ASL, specific for each board configuration.

Yours,
Linus Walleij

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Timur Tabi]

Linus Walleij wrote:
>> >  All of our GPIOs have a
>> >pinmux on them, and so if you want to use the pin for the non-default
>> >functionality, you need to configure the mux.  Isn't that supposed to happen
>> >with the through the pinctrl driver?  That is, when the kernel parses the
>> >ASL, and it seems a command to configure pin #3 to function #4, it calls the
>> >local pinctrl driver to do that?
> Let's suspect this is true. That is how GPIO work after all is it not?

With device trees, function groups are defined by name in the driver, 
and the device tree contains these same names as links between nodes. 
This works well.

In ACPI, it appears that no such infrastructure exists.

> I guess the other option would be that all systems must come with a
> tailored set-up written in ASL, specific for each board configuration.

That makes sense (U-Boot does fixups of the device tree based on actual 
board config, so it's conceivable that UEFI could do the same for ASL). 
  However, that would require either

1) UEFI programs the GPIO muxes directly, without specifying any mux 
information in ASL and without using the Linux driver to program the muxes.

or

2) ASL contains code to program the muxes, and Linux parses that ASL and 
calls the pinctrl driver to program the hardware.

I don't know which direction the Community wants to use.  If we go with 
option #1, then there is no need for the pinctrl driver to specify 
function groups at all.  This means we would not need a separate pinctrl 
driver for each SOC, like we do today.


-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Westerberg, Mika]

On Thu, Apr 24, 2014 at 12:20:12AM +0200, Linus Walleij wrote:
> On Wed, Apr 23, 2014 at 5:14 PM, Timur Tabi <timur@codeaurora.org> wrote:
> 
> > How are the pin muxes normally configured in ACPI?
> 
> VERY good question.

Typically this is done by the boot firmware (BIOS in this case). So the OS
doesn't need to deal with that.

AFAICT ACPI doesn't know anything about pin muxing.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Timur Tabi]

Westerberg, Mika wrote:
> Typically this is done by the boot firmware (BIOS in this case). So the OS
> doesn't need to deal with that.
>
> AFAICT ACPI doesn't know anything about pin muxing.

In that case, would it be correct to say that a Linux pinctrl driver for 
an ACPI-only platform should not define pinmux function groups, because 
they'll never be used?

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Westerberg, Mika]

On Thu, Apr 24, 2014 at 06:20:02AM -0500, Timur Tabi wrote:
> Westerberg, Mika wrote:
> >Typically this is done by the boot firmware (BIOS in this case). So the OS
> >doesn't need to deal with that.
> >
> >AFAICT ACPI doesn't know anything about pin muxing.
> 
> In that case, would it be correct to say that a Linux pinctrl driver
> for an ACPI-only platform should not define pinmux function groups,
> because they'll never be used?

No because you can have a GPIO controller on ACPI machine that allows you
to do some sort of pin muxing. What I'm saying is that ACPI itself doesn't
have such knowledge and hence we don't support such things in
gpiolib-acpi.c.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Westerberg, Mika]

On Wed, Apr 23, 2014 at 10:14:20AM -0500, Timur Tabi wrote:
> Westerberg, Mika wrote:
> >It doesn't do any pin control nor muxing and I'm not sure if it is
> >required. Can you elaborate why you think pin muxing is required with
> >GpioIo/GpioInt resources?
> 
> How are the pin muxes normally configured in ACPI?  All of our GPIOs
> have a pinmux on them, and so if you want to use the pin for the
> non-default functionality, you need to configure the mux.  Isn't
> that supposed to happen with the through the pinctrl driver?

It's the BIOS that handles all this even though there are several
"alternate functions" in the GPIO hardware. BIOS goes and configures those
according what the platform needs and those that are GPIOs/IRQs it will
create corresponding GpioIo/GpioInt along with the device that uses them.

Of course if you have a custom board and your BIOS doesn't handle this, you
are going to need some sort of pinctrl driver.

> That is, when the kernel parses the ASL, and it seems a command to
> configure pin #3 to function #4, it calls the local pinctrl driver to do
> that?

I'm not aware of ASL code that allows you to do that. Do you have examples?

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Timur Tabi]

Westerberg, Mika wrote:
>> >That is, when the kernel parses the ASL, and it seems a command to
>> >configure pin #3 to function #4, it calls the local pinctrl driver to do
>> >that?

> I'm not aware of ASL code that allows you to do that. Do you have examples?

No, that's my point.  I was expecting the pinmux functions of the 
pinctrl driver are used by ACPI, but apparently they aren't, and that's 
why I'm asking.  I'm wondering why a pinctrl driver for an ACPI platform 
should be defining pinmux function groups.  I haven't gotten a straight 
answer to that question.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Westerberg, Mika]

On Thu, Apr 24, 2014 at 06:18:38AM -0500, Timur Tabi wrote:
> Westerberg, Mika wrote:
> >>>That is, when the kernel parses the ASL, and it seems a command to
> >>>configure pin #3 to function #4, it calls the local pinctrl driver to do
> >>>that?
> 
> >I'm not aware of ASL code that allows you to do that. Do you have examples?
> 
> No, that's my point.  I was expecting the pinmux functions of the
> pinctrl driver are used by ACPI, but apparently they aren't, and
> that's why I'm asking.

Which functions?

> I'm wondering why a pinctrl driver for an
> ACPI platform should be defining pinmux function groups.  I haven't
> gotten a straight answer to that question.

Are you asking why pinctrl-baytrail.c uses pinctrl_gpio_ranges even if it
is a pretty much standard GPIO driver? Mathias probably can answer that
question better than me.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Linus Walleij]

On Thu, Apr 24, 2014 at 1:58 PM, Westerberg, Mika
<mika.westerberg@intel.com> wrote:
> On Thu, Apr 24, 2014 at 06:18:38AM -0500, Timur Tabi wrote:

>> I'm wondering why a pinctrl driver for an
>> ACPI platform should be defining pinmux function groups.  I haven't
>> gotten a straight answer to that question.
>
> Are you asking why pinctrl-baytrail.c uses pinctrl_gpio_ranges even if it
> is a pretty much standard GPIO driver? Mathias probably can answer that
> question better than me.

That was done on my request since any other approach obviously
would lead to code duplication of exactly what GPIO ranges does.

Yours,
Linus Walleij

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Timur Tabi]

On 04/24/2014 06:58 AM, Westerberg, Mika wrote:
>> >No, that's my point.  I was expecting the pinmux functions of the
>> >pinctrl driver are used by ACPI, but apparently they aren't, and
>> >that's why I'm asking.

> Which functions?

The functions in struct pinmux_ops, like get_function_groups.  Will 
these functions ever be called on an ACPI system?

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by the Linux Foundation.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Westerberg, Mika]

On Thu, Apr 24, 2014 at 10:25:56AM -0500, Timur Tabi wrote:
> On 04/24/2014 06:58 AM, Westerberg, Mika wrote:
> >>>No, that's my point.  I was expecting the pinmux functions of the
> >>>pinctrl driver are used by ACPI, but apparently they aren't, and
> >>>that's why I'm asking.
> 
> >Which functions?
> 
> The functions in struct pinmux_ops, like get_function_groups.  Will
> these functions ever be called on an ACPI system?

Well, if you have an ACPI system (like normal PC) it is perfectly fine to
have pin controller/mux hardware there which is not dependent at all on
ACPI.

If you happen to have pin controller/mux driver that drives that hardware,
I'm sure your pinmux functions gets called.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Linus Walleij]

On Fri, Apr 25, 2014 at 9:41 AM, Westerberg, Mika
<mika.westerberg@intel.com> wrote:
> On Thu, Apr 24, 2014 at 10:25:56AM -0500, Timur Tabi wrote:
>> On 04/24/2014 06:58 AM, Westerberg, Mika wrote:
>> >>>No, that's my point.  I was expecting the pinmux functions of the
>> >>>pinctrl driver are used by ACPI, but apparently they aren't, and
>> >>>that's why I'm asking.
>>
>> >Which functions?
>>
>> The functions in struct pinmux_ops, like get_function_groups.  Will
>> these functions ever be called on an ACPI system?
>
> Well, if you have an ACPI system (like normal PC) it is perfectly fine to
> have pin controller/mux hardware there which is not dependent at all on
> ACPI.
>
> If you happen to have pin controller/mux driver that drives that hardware,
> I'm sure your pinmux functions gets called.

True as far as the driver itself goes.

Pin controllers are however configured into a certain state with
platform data or device tree data. This configuration would need
to be stored in ACPI to fill the equivalent role in such a system,
and that in turn implies some thought given to it during design
of any ACPI-pinctrl bindings.

Yours,
Linus Walleij

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Timur Tabi]

Westerberg, Mika wrote:
> If you happen to have pin controller/mux driver that drives that hardware,
> I'm sure your pinmux functions gets called.

Actually, I don't think they do.  On a device-tree system, the functions 
get called automatically by the pinctrl layer when it parses the device 
tree.  This happens in response to probe, remove, and various power 
management changes.

AFAIK, there is nothing at all like that for ACPI systems.  Do you have 
an example of a driver making pinmux calls directly?

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Rafael J. Wysocki]

On 4/25/2014 6:13 PM, Timur Tabi wrote:
> Westerberg, Mika wrote:
>> If you happen to have pin controller/mux driver that drives that 
>> hardware,
>> I'm sure your pinmux functions gets called.
>
> Actually, I don't think they do.  On a device-tree system, the 
> functions get called automatically by the pinctrl layer when it parses 
> the device tree.  This happens in response to probe, remove, and 
> various power management changes.
>

I would be interested in understanding what exactly the flow is in that 
situation, so care to educate me?  What does the driver do to trigger 
this and what exactly does happen in response to that?

Rafael

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Timur Tabi]

Rafael J. Wysocki wrote:
>>
>
> I would be interested in understanding what exactly the flow is in that
> situation, so care to educate me?  What does the driver do to trigger
> this and what exactly does happen in response to that?

I only just learned some of this myself, so I'm no expert.  My 
understanding is that the all of the pinctrl-* properties and nodes are 
scanned by the pinctrl layer itself.  So you could have a SATA 
controller node that points to a pin control node (via phandles).  When 
the SATA driver is probed, the pinctrl layer notices the phandles and 
automatically calls the pinctrl layer to configure the pins and pin muxes.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Linus Walleij]

On Fri, Apr 25, 2014 at 9:31 AM, Timur Tabi <timur@codeaurora.org> wrote:
> Rafael J. Wysocki wrote:
>>
>> I would be interested in understanding what exactly the flow is in that
>> situation, so care to educate me?  What does the driver do to trigger
>> this and what exactly does happen in response to that?
>
>
> I only just learned some of this myself, so I'm no expert.  My understanding
> is that the all of the pinctrl-* properties and nodes are scanned by the
> pinctrl layer itself.  So you could have a SATA controller node that points
> to a pin control node (via phandles).  When the SATA driver is probed, the
> pinctrl layer notices the phandles and automatically calls the pinctrl layer
> to configure the pins and pin muxes.

There is a global pin control map for the system spanning all possible
pin controller instances.

Right before a device probe() function is called, the device core will
attempt to grab and activate a pin control setting tied to this device
and named "default". (It can also handle some PM states.)

The table of states can come from:
(a) Platform data or
(b) Device tree

And if there was an ACPI pin controller it would be case (c) and need
to have intelligent bindings allowing such a table to be built so that
the device core can make use of it.

When the different states (such as "default") are enabled, this
results in calls into the pin control driver. In most cases that ends
up with simple register writes but I guess an ACPI driver would
result in calling some esoteric bytecode or whatever or both.

There is no escape from the fact that this needs being
handled from the pin control subsystem though, it can't
be sneaked into the ACPI core or something. You may want
to add new states apart from the ones defined in
include/linux/pinctrl/pinctrl-state.h, as I know ACPI is
a bit picky about it's states and what they are named
(D-states right?)

Yours,
Linus Walleij (pretending to understand ACPI)

Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support

[Linus Walleij]

On Sat, Apr 12, 2014 at 12:54 AM, Timur Tabi <timur@codeaurora.org> wrote:

> I know it's been ten months since you posted this driver, but I have a
> question.  If this driver does not touch the pin muxing, and it
> doesn't even call pinctrl_register(), then why is it in
> drivers/pinctrl?  It's not a pinctrl driver.  Why isn't this a regular
> GPIO drivers in drivers/gpio?

- It uses the GPIO ranges concept in the pinctrl framework, because
  it internally actually deals with pin numbers, not GPIO numbers
  or predictable offsets. This way the basic pin control properties
  that ACPI should "hide" have already leaked to the driver,
  see score_pins[], ncore_pins[] etc.

- Pin control is not only about muxing, it is also about electrical
  config, see e.g. include/linux/pinctrl/pinconf-generic.h
  ACPI contain very unnerving stuff like PullDefault, PullUp,
  etc. In Linux terms that stuff is NOT GPIO, it is pin control.
  That ACPI choose to call it "GPIO" does not matter one bit.
  Now I am informed that this is read-only information, yet
  you may want to model it to Linux in the end, and then
  pin control provides the right infrastructure.

- Since the driver already knows this, it can be augmented to
  register a pin controller struct and display the actual names
  of all the pins in debugfs. And the pull state of each maybe?

- The magic muxing and biasing etc registers obviously exist
  in the register range of this IP block. Someone may start
  hacking to do fun stuff. Like bypass ACPI when it's doing
  something wrong. (Oh that never would happen, right?)
  Then the driver is in the right place to start hacking.

Yours,
Linus Walleij