* [PATCH v3 0/1] Pinctrl/gpio driver for Intel Baytrail platforms @ 2013-06-18 11:33 Mathias Nyman 2013-06-18 11:33 ` [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support Mathias Nyman 0 siblings, 1 reply; 30+ messages in thread From: Mathias Nyman @ 2013-06-18 11:33 UTC (permalink / raw) To: Linus; +Cc: grant.likely, linux-kernel, 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2013-06-18 11:33 [PATCH v3 0/1] Pinctrl/gpio driver for Intel Baytrail platforms Mathias Nyman @ 2013-06-18 11:33 ` Mathias Nyman 2013-06-18 15:17 ` Linus Walleij 2014-04-11 22:54 ` Timur Tabi 0 siblings, 2 replies; 30+ messages in thread From: Mathias Nyman @ 2013-06-18 11:33 UTC (permalink / raw) To: Linus; +Cc: grant.likely, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2013-06-18 11:33 ` [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support Mathias Nyman @ 2013-06-18 15:17 ` Linus Walleij 2013-06-19 10:28 ` Mathias Nyman 2014-04-11 22:54 ` Timur Tabi 1 sibling, 1 reply; 30+ messages in thread From: Linus Walleij @ 2013-06-18 15:17 UTC (permalink / raw) To: Mathias Nyman; +Cc: Grant Likely, linux-kernel 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2013-06-18 15:17 ` Linus Walleij @ 2013-06-19 10:28 ` Mathias Nyman 0 siblings, 0 replies; 30+ messages in thread From: Mathias Nyman @ 2013-06-19 10:28 UTC (permalink / raw) To: Linus Walleij; +Cc: Grant Likely, linux-kernel 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2013-06-18 11:33 ` [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support Mathias Nyman 2013-06-18 15:17 ` Linus Walleij @ 2014-04-11 22:54 ` Timur Tabi 2014-04-14 7:52 ` Mathias Nyman 2014-04-23 22:17 ` Linus Walleij 1 sibling, 2 replies; 30+ messages in thread From: Timur Tabi @ 2014-04-11 22:54 UTC (permalink / raw) To: Mathias Nyman; +Cc: Linus, Grant Likely, lkml 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-11 22:54 ` Timur Tabi @ 2014-04-14 7:52 ` Mathias Nyman 2014-04-14 15:11 ` Timur Tabi 2014-04-23 22:17 ` Linus Walleij 1 sibling, 1 reply; 30+ messages in thread From: Mathias Nyman @ 2014-04-14 7:52 UTC (permalink / raw) To: Timur Tabi; +Cc: Linus, Grant Likely, lkml 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-14 7:52 ` Mathias Nyman @ 2014-04-14 15:11 ` Timur Tabi 2014-04-15 10:01 ` Mathias Nyman 0 siblings, 1 reply; 30+ messages in thread From: Timur Tabi @ 2014-04-14 15:11 UTC (permalink / raw) To: Mathias Nyman; +Cc: Linus, Grant Likely, lkml 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-14 15:11 ` Timur Tabi @ 2014-04-15 10:01 ` Mathias Nyman 2014-04-17 16:47 ` Timur Tabi 0 siblings, 1 reply; 30+ messages in thread From: Mathias Nyman @ 2014-04-15 10:01 UTC (permalink / raw) To: Timur Tabi; +Cc: Linus, Grant Likely, lkml 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-15 10:01 ` Mathias Nyman @ 2014-04-17 16:47 ` Timur Tabi 2014-04-23 11:46 ` Mathias Nyman 0 siblings, 1 reply; 30+ messages in thread From: Timur Tabi @ 2014-04-17 16:47 UTC (permalink / raw) To: Mathias Nyman; +Cc: Linus, Grant Likely, lkml 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-17 16:47 ` Timur Tabi @ 2014-04-23 11:46 ` Mathias Nyman 2014-04-23 12:07 ` Timur Tabi 0 siblings, 1 reply; 30+ messages in thread From: Mathias Nyman @ 2014-04-23 11:46 UTC (permalink / raw) To: Timur Tabi; +Cc: Linus, Grant Likely, lkml, Westerberg, Mika, Rafael J. Wysocki 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-23 11:46 ` Mathias Nyman @ 2014-04-23 12:07 ` Timur Tabi 2014-04-23 13:59 ` Westerberg, Mika 0 siblings, 1 reply; 30+ messages in thread From: Timur Tabi @ 2014-04-23 12:07 UTC (permalink / raw) To: Mathias Nyman Cc: Linus, Grant Likely, lkml, Westerberg, Mika, Rafael J. Wysocki 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-23 12:07 ` Timur Tabi @ 2014-04-23 13:59 ` Westerberg, Mika 2014-04-23 15:14 ` Timur Tabi 0 siblings, 1 reply; 30+ messages in thread From: Westerberg, Mika @ 2014-04-23 13:59 UTC (permalink / raw) To: Timur Tabi; +Cc: Mathias Nyman, Linus, Grant Likely, lkml, Rafael J. Wysocki 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? ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-23 13:59 ` Westerberg, Mika @ 2014-04-23 15:14 ` Timur Tabi 2014-04-23 22:20 ` Linus Walleij 2014-04-24 6:35 ` Westerberg, Mika 0 siblings, 2 replies; 30+ messages in thread From: Timur Tabi @ 2014-04-23 15:14 UTC (permalink / raw) To: Westerberg, Mika Cc: Mathias Nyman, Linus, Grant Likely, lkml, Rafael J. Wysocki 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-23 15:14 ` Timur Tabi @ 2014-04-23 22:20 ` Linus Walleij 2014-04-23 22:54 ` Timur Tabi 2014-04-24 6:27 ` Westerberg, Mika 2014-04-24 6:35 ` Westerberg, Mika 1 sibling, 2 replies; 30+ messages in thread From: Linus Walleij @ 2014-04-23 22:20 UTC (permalink / raw) To: Timur Tabi Cc: Westerberg, Mika, Mathias Nyman, Grant Likely, lkml, Rafael J. Wysocki 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-23 22:20 ` Linus Walleij @ 2014-04-23 22:54 ` Timur Tabi 2014-04-24 6:27 ` Westerberg, Mika 1 sibling, 0 replies; 30+ messages in thread From: Timur Tabi @ 2014-04-23 22:54 UTC (permalink / raw) To: Linus Walleij Cc: Westerberg, Mika, Mathias Nyman, Grant Likely, lkml, Rafael J. Wysocki 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-23 22:20 ` Linus Walleij 2014-04-23 22:54 ` Timur Tabi @ 2014-04-24 6:27 ` Westerberg, Mika 2014-04-24 11:20 ` Timur Tabi 1 sibling, 1 reply; 30+ messages in thread From: Westerberg, Mika @ 2014-04-24 6:27 UTC (permalink / raw) To: Linus Walleij Cc: Timur Tabi, Mathias Nyman, Grant Likely, lkml, Rafael J. Wysocki 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-24 6:27 ` Westerberg, Mika @ 2014-04-24 11:20 ` Timur Tabi 2014-04-24 11:38 ` Westerberg, Mika 0 siblings, 1 reply; 30+ messages in thread From: Timur Tabi @ 2014-04-24 11:20 UTC (permalink / raw) To: Westerberg, Mika, Linus Walleij Cc: Mathias Nyman, Grant Likely, lkml, Rafael J. Wysocki 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-24 11:20 ` Timur Tabi @ 2014-04-24 11:38 ` Westerberg, Mika 0 siblings, 0 replies; 30+ messages in thread From: Westerberg, Mika @ 2014-04-24 11:38 UTC (permalink / raw) To: Timur Tabi Cc: Linus Walleij, Mathias Nyman, Grant Likely, lkml, Rafael J. Wysocki 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-23 15:14 ` Timur Tabi 2014-04-23 22:20 ` Linus Walleij @ 2014-04-24 6:35 ` Westerberg, Mika 2014-04-24 11:18 ` Timur Tabi 1 sibling, 1 reply; 30+ messages in thread From: Westerberg, Mika @ 2014-04-24 6:35 UTC (permalink / raw) To: Timur Tabi; +Cc: Mathias Nyman, Linus, Grant Likely, lkml, Rafael J. Wysocki 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? ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-24 6:35 ` Westerberg, Mika @ 2014-04-24 11:18 ` Timur Tabi 2014-04-24 11:58 ` Westerberg, Mika 0 siblings, 1 reply; 30+ messages in thread From: Timur Tabi @ 2014-04-24 11:18 UTC (permalink / raw) To: Westerberg, Mika Cc: Mathias Nyman, Linus, Grant Likely, lkml, Rafael J. Wysocki 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-24 11:18 ` Timur Tabi @ 2014-04-24 11:58 ` Westerberg, Mika 2014-04-24 13:29 ` Linus Walleij 2014-04-24 15:25 ` Timur Tabi 0 siblings, 2 replies; 30+ messages in thread From: Westerberg, Mika @ 2014-04-24 11:58 UTC (permalink / raw) To: Timur Tabi; +Cc: Mathias Nyman, Linus, Grant Likely, lkml, Rafael J. Wysocki 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-24 11:58 ` Westerberg, Mika @ 2014-04-24 13:29 ` Linus Walleij 2014-04-24 15:25 ` Timur Tabi 1 sibling, 0 replies; 30+ messages in thread From: Linus Walleij @ 2014-04-24 13:29 UTC (permalink / raw) To: Westerberg, Mika Cc: Timur Tabi, Mathias Nyman, Grant Likely, lkml, Rafael J. Wysocki 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-24 11:58 ` Westerberg, Mika 2014-04-24 13:29 ` Linus Walleij @ 2014-04-24 15:25 ` Timur Tabi 2014-04-25 7:41 ` Westerberg, Mika 1 sibling, 1 reply; 30+ messages in thread From: Timur Tabi @ 2014-04-24 15:25 UTC (permalink / raw) To: Westerberg, Mika Cc: Mathias Nyman, Linus, Grant Likely, lkml, Rafael J. Wysocki 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-24 15:25 ` Timur Tabi @ 2014-04-25 7:41 ` Westerberg, Mika 2014-04-25 10:36 ` Linus Walleij 2014-04-25 16:13 ` Timur Tabi 0 siblings, 2 replies; 30+ messages in thread From: Westerberg, Mika @ 2014-04-25 7:41 UTC (permalink / raw) To: Timur Tabi; +Cc: Mathias Nyman, Linus, Grant Likely, lkml, Rafael J. Wysocki 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-25 7:41 ` Westerberg, Mika @ 2014-04-25 10:36 ` Linus Walleij 2014-04-25 16:13 ` Timur Tabi 1 sibling, 0 replies; 30+ messages in thread From: Linus Walleij @ 2014-04-25 10:36 UTC (permalink / raw) To: Westerberg, Mika Cc: Timur Tabi, Mathias Nyman, Grant Likely, lkml, Rafael J. Wysocki 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-25 7:41 ` Westerberg, Mika 2014-04-25 10:36 ` Linus Walleij @ 2014-04-25 16:13 ` Timur Tabi 2014-04-25 16:21 ` Rafael J. Wysocki 1 sibling, 1 reply; 30+ messages in thread From: Timur Tabi @ 2014-04-25 16:13 UTC (permalink / raw) To: Westerberg, Mika Cc: Mathias Nyman, Linus, Grant Likely, lkml, Rafael J. Wysocki 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-25 16:13 ` Timur Tabi @ 2014-04-25 16:21 ` Rafael J. Wysocki 2014-04-25 16:31 ` Timur Tabi 0 siblings, 1 reply; 30+ messages in thread From: Rafael J. Wysocki @ 2014-04-25 16:21 UTC (permalink / raw) To: Timur Tabi, Westerberg, Mika Cc: Mathias Nyman, Linus, Grant Likely, lkml, 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-25 16:21 ` Rafael J. Wysocki @ 2014-04-25 16:31 ` Timur Tabi 2014-05-02 22:31 ` Linus Walleij 0 siblings, 1 reply; 30+ messages in thread From: Timur Tabi @ 2014-04-25 16:31 UTC (permalink / raw) To: Rafael J. Wysocki, Westerberg, Mika Cc: Mathias Nyman, Linus, Grant Likely, lkml, Rafael J. Wysocki 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-25 16:31 ` Timur Tabi @ 2014-05-02 22:31 ` Linus Walleij 0 siblings, 0 replies; 30+ messages in thread From: Linus Walleij @ 2014-05-02 22:31 UTC (permalink / raw) To: Timur Tabi Cc: Rafael J. Wysocki, Westerberg, Mika, Mathias Nyman, Grant Likely, lkml, Rafael J. Wysocki 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) ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support 2014-04-11 22:54 ` Timur Tabi 2014-04-14 7:52 ` Mathias Nyman @ 2014-04-23 22:17 ` Linus Walleij 1 sibling, 0 replies; 30+ messages in thread From: Linus Walleij @ 2014-04-23 22:17 UTC (permalink / raw) To: Timur Tabi; +Cc: Mathias Nyman, Grant Likely, lkml 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2014-05-02 22:31 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-06-18 11:33 [PATCH v3 0/1] Pinctrl/gpio driver for Intel Baytrail platforms Mathias Nyman 2013-06-18 11:33 ` [PATCH v3 1/1] pinctrl: add Intel BayTrail GPIO/pinctrl support Mathias Nyman 2013-06-18 15:17 ` Linus Walleij 2013-06-19 10:28 ` Mathias Nyman 2014-04-11 22:54 ` Timur Tabi 2014-04-14 7:52 ` Mathias Nyman 2014-04-14 15:11 ` Timur Tabi 2014-04-15 10:01 ` Mathias Nyman 2014-04-17 16:47 ` Timur Tabi 2014-04-23 11:46 ` Mathias Nyman 2014-04-23 12:07 ` Timur Tabi 2014-04-23 13:59 ` Westerberg, Mika 2014-04-23 15:14 ` Timur Tabi 2014-04-23 22:20 ` Linus Walleij 2014-04-23 22:54 ` Timur Tabi 2014-04-24 6:27 ` Westerberg, Mika 2014-04-24 11:20 ` Timur Tabi 2014-04-24 11:38 ` Westerberg, Mika 2014-04-24 6:35 ` Westerberg, Mika 2014-04-24 11:18 ` Timur Tabi 2014-04-24 11:58 ` Westerberg, Mika 2014-04-24 13:29 ` Linus Walleij 2014-04-24 15:25 ` Timur Tabi 2014-04-25 7:41 ` Westerberg, Mika 2014-04-25 10:36 ` Linus Walleij 2014-04-25 16:13 ` Timur Tabi 2014-04-25 16:21 ` Rafael J. Wysocki 2014-04-25 16:31 ` Timur Tabi 2014-05-02 22:31 ` Linus Walleij 2014-04-23 22:17 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).