* Right amount of info in the DT @ 2017-04-20 9:54 Yves Lefloch 2017-04-24 8:23 ` Linus Walleij 0 siblings, 1 reply; 17+ messages in thread From: Yves Lefloch @ 2017-04-20 9:54 UTC (permalink / raw) To: linux-gpio; +Cc: linus.walleij, gnurou, slash.tmp, Thibaud Cornic Hello everybody, I'm a kernel newbie trying to write a GPIO driver for my platform (ARM tango), and I'd need some advice regarding the oddity of my hardware. I have a bunch of GPIO groups all over the place (dedicated pins, UART pins, ethernet pins, etc.), and even though they mainly behave similarly (same way of handling direction and value), there are some differences between them: - IRQs, because some groups can generate interrupts and others can't; - Alternate functions (for instance GPIO mode vs UART mode), because obviously dedicated pins don't have an alternate function, and for the others which have it, the mode-changing register is sometimes before, sometimes after the other regs. My question is: where to encode this information? I've noticed that in other device trees, there's an irq-controller property that could probably handle the first case, but for the second, I'm not sure what I should do. I've thought of: 1) Making up a property that would tell the driver the offset of the mode register in the resource, but I'm afraid this is encoding too much HW info in the DT. 2) Making up a property or perhaps different compatible strings that would tell the driver which kind of GPIO group it's dealing with, but that would maybe make the driver quite ad-hoc. 3) Put in another resource to deal with the mode register, but the thing is, this register must not be written the same way for all groups. 4) Encoding all the information (offsets, number of GPIOs, etc.) in a const array within the driver, but that would defeat the purpose of a DT, would it not? Any help appreciated! Yves Lefloch ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-04-20 9:54 Right amount of info in the DT Yves Lefloch @ 2017-04-24 8:23 ` Linus Walleij 2017-04-24 13:03 ` Yves Lefloch 0 siblings, 1 reply; 17+ messages in thread From: Linus Walleij @ 2017-04-24 8:23 UTC (permalink / raw) To: Yves Lefloch; +Cc: linux-gpio, Alexandre Courbot, Mason, Thibaud Cornic On Thu, Apr 20, 2017 at 11:54 AM, Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> wrote: > I have a bunch of GPIO groups all over the place (dedicated pins, UART pins, > ethernet pins, etc.), and even though they mainly behave similarly (same way of > handling direction and value), there are some differences between them: > - IRQs, because some groups can generate interrupts and others can't; > - Alternate functions (for instance GPIO mode vs UART mode), because obviously > dedicated pins don't have an alternate function, and for the others which have > it, the mode-changing register is sometimes before, sometimes after the other > regs. The basic problem is that you conceptualize of these things as GPIO and not the more abstract concept of pins under pin control. For example, there are not GPIO groups, there are pin control pin multiplexing groups. Please read Documentation/pinctrl.txt https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/pinctrl.txt Also read the generic pin control device tree bindings: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt Further in the GPIO subsystem you will find references back to how it interacts with pin control. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/gpio/gpio.txt Yours, Linus Walleij ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-04-24 8:23 ` Linus Walleij @ 2017-04-24 13:03 ` Yves Lefloch 2017-04-24 16:45 ` Linus Walleij 0 siblings, 1 reply; 17+ messages in thread From: Yves Lefloch @ 2017-04-24 13:03 UTC (permalink / raw) To: Linus Walleij; +Cc: linux-gpio, Alexandre Courbot, Mason, Thibaud Cornic Hello Linus, thanks for reply. OK I see now that I have to do this kind of stuff in a pinctrl/pinmux driver. I've read the documentation you pointed to and browsed a few existing drivers, but I'm still a bit confused by how my device tree should look like. Indeed in my SoC there is no central hw pin controller, and pin control registers are scattered all over the address space because they're in different IPs. How am I supposed to deal with so many different addresses? I have considered hardcoding them in the driver but that seems counter-productive, so I thought I could do something like this, with the address of the function-select register as a resource (it's the only pin property I can change besides direction and data): pincontroller { compatible = "foo-pinctrl"; #address-cells = <1>; #size-cells = <0>; uart1 { reg = <0x10001>; state_0 { groups = "uart1"; function = "uart1"; }; state_1 { groups = "uart1"; function = "gpio"; }; ... other IPs (SPI, ethernet, etc.) ... }; Furthermore, I was wondering how it's possible for pinctrl drivers to map registers possibly used by other drivers. For example, the "GPIO pad mode" register of my ethernet pins is right in the middle of the address space requested by the ethernet driver. Meaning that if I try to "devm_ioremap_resource" when probing the GPIO or pinctrl drivers, I will get an error saying that the memory region is already taken. Do you have any insight on that? Regards, Yves. On 04/24/2017 10:23 AM, Linus Walleij wrote: > On Thu, Apr 20, 2017 at 11:54 AM, Yves Lefloch > <YvesMarie_Lefloch@sigmadesigns.com> wrote: > >> I have a bunch of GPIO groups all over the place (dedicated pins, UART pins, >> ethernet pins, etc.), and even though they mainly behave similarly (same way of >> handling direction and value), there are some differences between them: >> - IRQs, because some groups can generate interrupts and others can't; >> - Alternate functions (for instance GPIO mode vs UART mode), because obviously >> dedicated pins don't have an alternate function, and for the others which have >> it, the mode-changing register is sometimes before, sometimes after the other >> regs. > > The basic problem is that you conceptualize of these things as GPIO > and not the more abstract concept of pins under pin control. > For example, there are not GPIO groups, there are pin control pin > multiplexing groups. > > Please read Documentation/pinctrl.txt > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/pinctrl.txt > > Also read the generic pin control device tree bindings: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt > > Further in the GPIO subsystem you will find references back to how it > interacts with pin control. > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/gpio/gpio.txt > > Yours, > Linus Walleij > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-04-24 13:03 ` Yves Lefloch @ 2017-04-24 16:45 ` Linus Walleij 2017-04-26 15:47 ` Yves Lefloch 0 siblings, 1 reply; 17+ messages in thread From: Linus Walleij @ 2017-04-24 16:45 UTC (permalink / raw) To: Yves Lefloch; +Cc: linux-gpio, Alexandre Courbot, Mason, Thibaud Cornic On Mon, Apr 24, 2017 at 3:03 PM, Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> wrote: > OK I see now that I have to do this kind of stuff in a pinctrl/pinmux driver. Great! > in > my SoC there is no central hw pin controller, and pin control registers are > scattered all over the address space because they're in different IPs. How am I > supposed to deal with so many different addresses? I've herd about this happening sometimes... I haven't seen a perfect solution for it. > I have considered hardcoding > them in the driver but that seems counter-productive, so I thought I could do > something like this, with the address of the function-select register as a > resource (it's the only pin property I can change besides direction and data): > > > pincontroller { > compatible = "foo-pinctrl"; > #address-cells = <1>; > #size-cells = <0>; > > uart1 { > reg = <0x10001>; > > state_0 { > groups = "uart1"; > function = "uart1"; > }; > state_1 { > groups = "uart1"; > function = "gpio"; > }; > > ... other IPs (SPI, ethernet, etc.) ... > > }; Maybe, I would have to check the final result. It will need some serious documentation so that we can establish this way of dealing with this type of hardware. > Furthermore, I was wondering how it's possible for pinctrl drivers to map registers > possibly used by other drivers. We usually use syscon approaches for that. Especially when it considers individual bits in a single register. If you just have one single 32bit register in each hardware blocks, then certainly it can be remapped by several drivers, or you can leave a "hole" there in the main driver register definitions, but that seems very awkward. > For example, the "GPIO pad mode" register of my > ethernet pins is right in the middle of the address space requested by the ethernet > driver. Meaning that if I try to "devm_ioremap_resource" when probing the GPIO or > pinctrl drivers, I will get an error saying that the memory region is already taken. > Do you have any insight on that? Isn't the best approach to add a pin controller as a subnode of the ethernet controller in the device tree, so that the probe() of you main driver looks for it and instantiates a pin controller that is then referenced by the driver itself? We have similar constructions with e.g. PCI irq controllers inside of PCI bridge nodes. Then, if the same type of pin control is sprinkled all over your drivers, maybe turn it into a library in drivers/pinctrl that you can reuse from each driver so as to centralize the code? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-04-24 16:45 ` Linus Walleij @ 2017-04-26 15:47 ` Yves Lefloch 2017-05-07 10:26 ` Linus Walleij 0 siblings, 1 reply; 17+ messages in thread From: Yves Lefloch @ 2017-04-26 15:47 UTC (permalink / raw) To: Linus Walleij; +Cc: linux-gpio, Alexandre Courbot, Mason, Thibaud Cornic On 04/24/2017 06:45 PM, Linus Walleij wrote: > Isn't the best approach to add a pin controller as a subnode of the ethernet > controller in the device tree, so that the probe() of you main driver looks > for it and instantiates a pin controller that is then referenced by the > driver itself? I guess that could do it, but I'm not confident enough in my kernel abilities to modify such a big driver just yet (drivers/net/ethernet/aurora). I've decided not to support ethernet pins, at least for the first iteration of the driver. I've put together a first draft, would you mind taking a look at it? I haven't documented the DT binding yet, though. Regards, Yves. From: Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> Date: Tue, 25 Apr 2017 14:06:46 +0200 Subject: [PATCH] drivers: pinctrl: Add a pinctrl for mach-tango Signed-off-by: Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> --- arch/arm/boot/dts/tango4-common.dtsi | 98 ++++++++ arch/arm/configs/tango4_defconfig | 2 + arch/arm/mach-tango/Kconfig | 1 + drivers/pinctrl/Kconfig | 7 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-tango.c | 421 +++++++++++++++++++++++++++++++++++ 6 files changed, 530 insertions(+) create mode 100644 drivers/pinctrl/pinctrl-tango.c diff --git a/arch/arm/boot/dts/tango4-common.dtsi b/arch/arm/boot/dts/tango4-common.dtsi index a3e81ab..5e89af9 100644 --- a/arch/arm/boot/dts/tango4-common.dtsi +++ b/arch/arm/boot/dts/tango4-common.dtsi @@ -18,6 +18,104 @@ #address-cells = <1>; #size-cells = <1>; + pins { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + pinctrl@10500 { + reg = <0x10500 0x8>; + compatible = "sigma,smp8758-pinctrl"; + label = "sys"; + tango,pins = <0 16>; + #address-cells = <1>; + #size-cells = <1>; + + gpio { + gpio-controller; + #gpio-cells = <1>; + }; + }; + + pinctrl@15b8e0 { + compatible = "sigma,smp8758-pinctrl"; + label = "tdmux"; + reg = <0x15b8e0 0xc>; + tango,pins = <16 2>; + #address-cells = <1>; + #size-cells = <1>; + + gpio { + gpio-controller; + #gpio-cells = <1>; + }; + + alt_function { + function = "tdmux"; + groups = "tdmux"; + }; + }; + + pinctrl@6c130 { + compatible = "sigma,smp8758-pinctrl"; + label = "spi"; + reg = <0x6c130 0xc>; + tango,pins = <18 8>; + #address-cells = <1>; + #size-cells = <1>; + + gpio { + gpio-controller; + #gpio-cells = <1>; + }; + + alt_function { + function = "spi"; + groups = "spi"; + }; + }; + + pinctrl@6c230 { + compatible = "sigma,smp8758-pinctrl"; + label = "uart1"; + reg = <0x6c230 0xc>; + tango,pins = <26 7>; + #address-cells = <1>; + #size-cells = <1>; + + gpio { + gpio-controller; + #gpio-cells = <1>; + }; + + alt_function { + function = "uart1"; + groups = "uart1"; + }; + + }; + + pinctrl@6c358 { + compatible = "sigma,smp8758-pinctrl"; + label = "smcard"; + reg = <0x6c368 0xc>; + tango,pins = <71 8>; + #address-cells = <1>; + #size-cells = <1>; + + gpio { + gpio-controller; + #gpio-cells = <1>; + }; + + alt_function { + function = "smcard"; + groups = "smcard"; + }; + }; + }; + periph_clk: periph_clk { compatible = "fixed-factor-clock"; clocks = <&clkgen CPU_CLK>; diff --git a/arch/arm/configs/tango4_defconfig b/arch/arm/configs/tango4_defconfig index b26bb4e..c973758 100644 --- a/arch/arm/configs/tango4_defconfig +++ b/arch/arm/configs/tango4_defconfig @@ -68,7 +68,9 @@ CONFIG_SERIAL_OF_PLATFORM=y # CONFIG_HW_RANDOM is not set CONFIG_I2C=y CONFIG_I2C_XLR=y +CONFIG_PINCTRL_TANGO=m CONFIG_GPIOLIB=y +CONFIG_GPIO_SYSFS=y CONFIG_THERMAL=y CONFIG_CPU_THERMAL=y CONFIG_TANGO_THERMAL=y diff --git a/arch/arm/mach-tango/Kconfig b/arch/arm/mach-tango/Kconfig index ebe15b9..479c9aa 100644 --- a/arch/arm/mach-tango/Kconfig +++ b/arch/arm/mach-tango/Kconfig @@ -11,3 +11,4 @@ config ARCH_TANGO select HAVE_ARM_SCU select HAVE_ARM_TWD select TANGO_IRQ + select PINCTRL diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index 0e75d94..5648e76 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -179,6 +179,13 @@ config PINCTRL_ST select PINCONF select GPIOLIB_IRQCHIP +config PINCTRL_TANGO + tristate "Tango pin control driver" + depends on OF + select PINMUX + help + Say yes to activate the pinctrl/gpio driver for Tango chips. + config PINCTRL_TZ1090 bool "Toumaz Xenif TZ1090 pin control driver" depends on SOC_TZ1090 diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile index 11bad37..d5563e49 100644 --- a/drivers/pinctrl/Makefile +++ b/drivers/pinctrl/Makefile @@ -26,6 +26,7 @@ obj-$(CONFIG_PINCTRL_ROCKCHIP) += pinctrl-rockchip.o obj-$(CONFIG_PINCTRL_SINGLE) += pinctrl-single.o obj-$(CONFIG_PINCTRL_SIRF) += sirf/ obj-$(CONFIG_ARCH_TEGRA) += tegra/ +obj-$(CONFIG_PINCTRL_TANGO) += pinctrl-tango.o obj-$(CONFIG_PINCTRL_TZ1090) += pinctrl-tz1090.o obj-$(CONFIG_PINCTRL_TZ1090_PDC) += pinctrl-tz1090-pdc.o obj-$(CONFIG_PINCTRL_U300) += pinctrl-u300.o diff --git a/drivers/pinctrl/pinctrl-tango.c b/drivers/pinctrl/pinctrl-tango.c new file mode 100644 index 0000000..1112047 --- /dev/null +++ b/drivers/pinctrl/pinctrl-tango.c @@ -0,0 +1,421 @@ +/* + * Copyright (C) 2017 Sigma Designs + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + */ +#include <linux/bitops.h> +#include <linux/gpio/driver.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/pinctrl/pinctrl.h> +#include <linux/pinctrl/pinmux.h> +#include <linux/platform_device.h> + +/* Pin regs are laid out this way: + * ______________________________ + * | bitwise mask | bitwise val | + * 32 16 0 + * + * To set bit 1, one must write (1 << (16+1) | 1). + * + * Because of this layout, there are no more than 16 pins in a reg. + */ + +/* + * Offsets from `addr' in `struct tango_pinctrl'. + * The `mod' register isn't there for pins dedicated to GPIO. + */ +#define GPIO_OFF_DIR 0x0 +#define GPIO_OFF_VAL 0x4 +#define GPIO_OFF_MOD 0x8 + +struct tango_pinctrl { + const char *label; + u8 id; + u8 len; + bool dedicated; + void __iomem *addr; + + struct pinctrl_desc pd; + struct pinctrl_gpio_range gr; +}; + +static int pinreg_get_bit(void __iomem *addr, unsigned offset) +{ + unsigned long val = readl(addr); + + /* TODO remove me later */ +#if IS_ENABLED(CONFIG_DEBUG) + BUG_ON(val & GENMASK(31, 16)); +#endif + + return test_bit(offset, &val); +} + +static void pinreg_set_bit(void __iomem *addr, unsigned offset, int value) +{ + unsigned long val = 0; + + /* Enable change by writing upper half-word too. */ + set_bit(offset + 16, &val); + if (value) + set_bit(offset, &val); + + writel(val, addr); + + /* TODO remove me later */ +#if IS_ENABLED(CONFIG_DEBUG) + BUG_ON(pinreg_get_bit(addr, offset) != value); +#endif +} + +static void pinreg_set_multiple(void __iomem *addr, u16 mask, u16 bits) +{ + unsigned long val = (mask << 16) | bits; + + writel(val, addr); +} + +static int gpio_get(struct gpio_chip *gc, unsigned offset) +{ + struct tango_pinctrl *data = gpiochip_get_data(gc); + + if (!data) + return -EINVAL; + + return pinreg_get_bit(data->addr + GPIO_OFF_VAL, offset); +} + +static void gpio_set(struct gpio_chip *gc, unsigned offset, int value) +{ + struct tango_pinctrl *data = gpiochip_get_data(gc); + + if (!data) + return; + + pinreg_set_bit(data->addr + GPIO_OFF_VAL, offset, value); +} + +static int gpio_get_direction(struct gpio_chip *gc, unsigned offset) +{ + struct tango_pinctrl *data = gpiochip_get_data(gc); + + if (!data) + return -EINVAL; + + /* Bit set means output. */ + return !pinreg_get_bit(data->addr + GPIO_OFF_DIR, offset); +} + +static int gpio_direction_input(struct gpio_chip *gc, unsigned offset) +{ + struct tango_pinctrl *data = gpiochip_get_data(gc); + + if (!data) + return -EINVAL; + + /* Bit set means output. */ + pinreg_set_bit(data->addr + GPIO_OFF_DIR, offset, 0); + + return 0; +} + +static int gpio_direction_output(struct gpio_chip *gc, unsigned offset, int value) +{ + struct tango_pinctrl *data = gpiochip_get_data(gc); + + if (!data) + return -EINVAL; + + /* The output value can be changed before the direction is. */ + pinreg_set_bit(data->addr + GPIO_OFF_VAL, offset, value); + + /* Bit set means output. */ + pinreg_set_bit(data->addr + GPIO_OFF_DIR, offset, 1); + + return 0; +} + +static void gpio_set_multiple(struct gpio_chip *gc, + unsigned long *mask, unsigned long *bits) +{ + u16 _mask = (u16) *mask; + u16 _bits = (u16) *bits; + struct tango_pinctrl *data = gpiochip_get_data(gc); + if (!data) + return; + + pinreg_set_multiple(data->addr + GPIO_OFF_VAL, _mask, _bits); +} + +static const unsigned int all_pins[] = { + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, + 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, + 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, + 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, + 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, + 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, + 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, + 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, + 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, + 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, +}; + +static int get_group_count(struct pinctrl_dev *pctldev) +{ + struct tango_pinctrl *data; + + data = pinctrl_dev_get_drvdata(pctldev); + if (!data) + return -EINVAL; + + return data->dedicated ? 0 : 1; +} + +static const char *get_group_name(struct pinctrl_dev *pctldev, unsigned selector) +{ + struct tango_pinctrl *data; + + data = pinctrl_dev_get_drvdata(pctldev); + if (!data) + return ERR_PTR(-EINVAL); + + return data->label; +} + +static int get_group_pins(struct pinctrl_dev *pctldev, unsigned selector, + const unsigned **pins, unsigned *num_pins) +{ + struct tango_pinctrl *data; + + data = pinctrl_dev_get_drvdata(pctldev); + if (!data) + return -EINVAL; + + *pins = all_pins + data->id; + *num_pins = data->len; + + return 0; +} + +static const struct pinctrl_ops pctlops = { + .get_groups_count = get_group_count, + .get_group_name = get_group_name, + .get_group_pins = get_group_pins, +}; + +int get_function_groups(struct pinctrl_dev *pctldev, unsigned selector, + const char * const **groups, unsigned *num_groups) +{ + struct tango_pinctrl *data; + + data = pinctrl_dev_get_drvdata(pctldev); + if (!data) + return -EINVAL; + + *groups = &data->label; + *num_groups = 1; + + return 0; +} + +int set_mux(struct pinctrl_dev *pctldev, unsigned func_selector, + unsigned group_selector) +{ + /* With only one possible alt function, muxing is only + * necessary for GPIO request/free. + */ + return 0; +} + +int gpio_request_enable(struct pinctrl_dev *pctldev, + struct pinctrl_gpio_range *range, unsigned offset) +{ + struct tango_pinctrl *data; + + data = pinctrl_dev_get_drvdata(pctldev); + if (!data) + return -EINVAL; + + if (offset >= data->len) + return -EINVAL; + + if (!data->dedicated) + pinreg_set_bit(data->addr + GPIO_OFF_MOD, offset, 1); + + return 0; +} + +void gpio_disable_free(struct pinctrl_dev *pctldev, + struct pinctrl_gpio_range *range, unsigned offset) +{ + struct tango_pinctrl *data; + + data = pinctrl_dev_get_drvdata(pctldev); + if (!data) + return; + + if (offset >= data->len) + return; + + if (!data->dedicated) + pinreg_set_bit(data->addr + GPIO_OFF_MOD, offset, 0); +} + +const static struct pinmux_ops pmxops = { + .get_functions_count = get_group_count, + .get_function_name = get_group_name, + .get_function_groups = get_function_groups, + .set_mux = set_mux, + .gpio_request_enable = gpio_request_enable, + .gpio_disable_free = gpio_disable_free, + .strict = true, +}; + +static int get_node_info(struct device_node *np, struct tango_pinctrl *data) +{ + /* Extract info from the pinctrl node: + * - the human-friendly name; + * - the base num and the num of pins; + * - whether pins are dedicated to GPIO (no alt state). + */ + int ret; + u32 tango_pins[2]; + + ret = of_property_read_string(np, "label", &data->label); + if (ret) + return ret; + ret = of_property_read_u32_array(np, "tango,pins", tango_pins, 2); + if (ret) + return ret; + data->id = tango_pins[0]; + data->len = tango_pins[1]; + data->dedicated = !of_get_property(np, "alt_function", NULL); + + return 0; +} + +static int init_pd(struct device *dev, struct tango_pinctrl *data) +{ + int i; + struct pinctrl_pin_desc *pdesc; + + pdesc = devm_kzalloc(dev, data->len * sizeof(*pdesc), GFP_KERNEL); + if (!pdesc) + return -ENOMEM; + for (i = 0; i < data->len; i++) + pdesc[i].number = data->id + i; + + data->pd.name = data->label; + data->pd.pins = pdesc; + data->pd.npins = data->len; + data->pd.pctlops = &pctlops; + data->pd.pmxops = &pmxops; + data->pd.owner = THIS_MODULE; + + return 0; +} + +static int init_gr(struct device *dev, struct tango_pinctrl *data) +{ + struct gpio_chip *gc; + + gc = devm_kzalloc(dev, sizeof(*gc), GFP_KERNEL); + if (!gc) + return -ENOMEM; + + data->gr.name = data->label; + data->gr.id = data->id; + data->gr.base = data->id; + data->gr.pin_base = data->id; + data->gr.npins = data->len; + data->gr.gc = gc; + + gc->label = data->label; + gc->parent = dev; + gc->owner = THIS_MODULE; + gc->get_direction = gpio_get_direction; + gc->direction_input = gpio_direction_input; + gc->direction_output = gpio_direction_output; + gc->get = gpio_get; + gc->set = gpio_set; + gc->set_multiple = gpio_set_multiple; + gc->base = data->id; + gc->ngpio = data->len; + + return devm_gpiochip_add_data(dev, gc, data); +} + +static int tango_pinctrl_probe(struct platform_device *pdev) +{ + int ret; + struct resource *res; + struct tango_pinctrl *data; + struct pinctrl_dev *pindev; + struct device_node *np = pdev->dev.of_node; + + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + platform_set_drvdata(pdev, data); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -EINVAL; + data->addr = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(data->addr)) + return PTR_ERR(data->addr); + + ret = get_node_info(np, data); + if (ret) + return ret; + dev_dbg(&pdev->dev, "%8s id=%d len=%d\n", + data->label, data->id, data->len); + + ret = init_pd(&pdev->dev, data); + if (ret) + return ret; + + pindev = devm_pinctrl_register(&pdev->dev, &data->pd, data); + if (IS_ERR(pindev)) + return PTR_ERR(pindev); + + ret = init_gr(&pdev->dev, data); + if (ret) + return ret; + pinctrl_add_gpio_range(pindev, &data->gr); + + return 0; +} + +static const struct of_device_id tango_pinctrl_of_match[] = { + { .compatible = "sigma,smp8758-pinctrl", }, + {}, +}; +MODULE_DEVICE_TABLE(of, tango_pinctrl_of_match); + +static struct platform_driver tango_pinctrl_driver = { + .driver = { + .name = "pinctrl-tango", + .of_match_table = of_match_ptr(tango_pinctrl_of_match), + }, + .probe = tango_pinctrl_probe, +}; + +static int __init tango_pinctrl_init(void) +{ + return platform_driver_register(&tango_pinctrl_driver); +} +module_init(tango_pinctrl_init); + +static void __exit tango_pinctrl_exit(void) +{ + platform_driver_unregister(&tango_pinctrl_driver); +} +module_exit(tango_pinctrl_exit); + +MODULE_AUTHOR("Sigma Designs"); +MODULE_DESCRIPTION("Tango pinctrl driver"); +MODULE_LICENSE("GPL"); -- 2.10.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-04-26 15:47 ` Yves Lefloch @ 2017-05-07 10:26 ` Linus Walleij 2017-05-09 10:49 ` Yves Lefloch 0 siblings, 1 reply; 17+ messages in thread From: Linus Walleij @ 2017-05-07 10:26 UTC (permalink / raw) To: Yves Lefloch; +Cc: linux-gpio, Alexandre Courbot, Mason, Thibaud Cornic On Wed, Apr 26, 2017 at 5:47 PM, Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> wrote: > I've put together a first draft, would you mind taking a look at it? > I haven't documented the DT binding yet, though. OK > + pins { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + pinctrl@10500 { > + reg = <0x10500 0x8>; > + compatible = "sigma,smp8758-pinctrl"; > + label = "sys"; > + tango,pins = <0 16>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + gpio { > + gpio-controller; > + #gpio-cells = <1>; > + }; > + }; > + > + pinctrl@15b8e0 { > + compatible = "sigma,smp8758-pinctrl"; > + label = "tdmux"; > + reg = <0x15b8e0 0xc>; > + tango,pins = <16 2>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + gpio { > + gpio-controller; > + #gpio-cells = <1>; > + }; > + > + alt_function { > + function = "tdmux"; > + groups = "tdmux"; > + }; > + }; > + > + pinctrl@6c130 { > + compatible = "sigma,smp8758-pinctrl"; > + label = "spi"; > + reg = <0x6c130 0xc>; > + tango,pins = <18 8>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + gpio { > + gpio-controller; > + #gpio-cells = <1>; > + }; > + > + alt_function { > + function = "spi"; > + groups = "spi"; > + }; > + }; > + > + pinctrl@6c230 { > + compatible = "sigma,smp8758-pinctrl"; > + label = "uart1"; > + reg = <0x6c230 0xc>; > + tango,pins = <26 7>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + gpio { > + gpio-controller; > + #gpio-cells = <1>; > + }; > + > + alt_function { > + function = "uart1"; > + groups = "uart1"; > + }; > + > + }; > + > + pinctrl@6c358 { > + compatible = "sigma,smp8758-pinctrl"; > + label = "smcard"; > + reg = <0x6c368 0xc>; > + tango,pins = <71 8>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + gpio { > + gpio-controller; > + #gpio-cells = <1>; > + }; > + > + alt_function { > + function = "smcard"; > + groups = "smcard"; > + }; > + }; > + }; Does all of these really have gpio controller functionality? You do not have to compulsively add that. Just make it a pin controller. But I was not meaning you should collect them all under a "pins" node like this, but inside each peripheral. So for example: mmc@80114000 { compatible = "mmc-sd-card"; reg = <0x80114000 0x1000>; interrupts = <....>; pinctrl { compatible = "sigma,smp8758-pinctrl"; label = "smcard"; (....) }; }; So it lives inside the node of the peripheral. It can then access the register range by simply looking at the parent, and maybe when making the Linux driver, the parent simply calls a library function to set up the pin controller for itself providing the child node as reference. (Child nodes are not automatically spawning devices, you have to do that with code.) Yours, Linus Walleij ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-05-07 10:26 ` Linus Walleij @ 2017-05-09 10:49 ` Yves Lefloch 2017-05-11 15:22 ` Linus Walleij 0 siblings, 1 reply; 17+ messages in thread From: Yves Lefloch @ 2017-05-09 10:49 UTC (permalink / raw) To: Linus Walleij; +Cc: linux-gpio, Alexandre Courbot, Mason, Thibaud Cornic On 05/07/2017 12:26 PM, Linus Walleij wrote: > Does all of these really have gpio controller functionality? > You do not have to compulsively add that. Just make it > a pin controller. Most of them have pin controller functionalities, because the pins can be muxed between GPIO and another function. Only the first group is GPIO only (dedicated pins). But apart from the choosing of the current function, the other properties (direction, value) are handled the same way. Regarding that, I was wondering what's the recommended approach to mutualize code: 1. Having a driver under drivers/gpio and another on drivers/pinctrl which depends on the first one. 2. Having both drivers in the same file under drivers/pinctrl. 3. Having only one pinctrl driver. For now I've taken approach 3, but isn't it strange to have pins dedicated to GPIO like this first group I mentioned controlled for a pinctrl driver? What is your opinion on this? > But I was not meaning you should collect them all under a > "pins" node like this, but inside each peripheral. OK, I will do that. Although not all alternate functions currently have a driver, nor a node in the device tree. I guess the best way to handle that is to declare the pins as GPIO only and not to worry about the alternate function; which brings me back to the question above. Do they need a separate GPIO driver? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-05-09 10:49 ` Yves Lefloch @ 2017-05-11 15:22 ` Linus Walleij 2017-05-12 15:31 ` Yves Lefloch 0 siblings, 1 reply; 17+ messages in thread From: Linus Walleij @ 2017-05-11 15:22 UTC (permalink / raw) To: Yves Lefloch; +Cc: linux-gpio, Alexandre Courbot, Mason, Thibaud Cornic On Tue, May 9, 2017 at 12:49 PM, Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> wrote: > I was wondering what's the recommended approach to > mutualize code: > 1. Having a driver under drivers/gpio and another on drivers/pinctrl > which depends on the first one. > 2. Having both drivers in the same file under drivers/pinctrl. > 3. Having only one pinctrl driver. All approaches exist. I have no strong opinion, but when possible try to make separation of concerns. > For now I've taken approach 3, but isn't it strange to have pins > dedicated to GPIO like this first group I mentioned controlled > for a pinctrl driver? > What is your opinion on this? Just make sure your driver also provides a gpio_chip and write good looking code. >> But I was not meaning you should collect them all under a >> "pins" node like this, but inside each peripheral. > > OK, I will do that. Although not all alternate functions currently > have a driver, nor a node in the device tree. I guess the best way > to handle that is to declare the pins as GPIO only and not to worry > about the alternate function; which brings me back to the question > above. Do they need a separate GPIO driver? Implement everything you have a data sheet for even if you do not use it right now. It is good to have. Have you read the section "GPIO mode pitfalls" in the documentation? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/pinctrl.txt It is very important that you know for sure whether it is actual GPIO or just named that way by some hardware engineer. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-05-11 15:22 ` Linus Walleij @ 2017-05-12 15:31 ` Yves Lefloch 2017-05-22 15:12 ` Linus Walleij 0 siblings, 1 reply; 17+ messages in thread From: Yves Lefloch @ 2017-05-12 15:31 UTC (permalink / raw) To: Linus Walleij; +Cc: linux-gpio, Alexandre Courbot, Mason, Thibaud Cornic On 05/11/2017 05:22 PM, Linus Walleij wrote: > Just make sure your driver also provides a gpio_chip and > write good looking code. OK, IMHO the code I attached earlier would do that. > Implement everything you have a data sheet for even if you > do not use it right now. It is good to have. Do you mean that I should submit a driver for every single device before I can declare its pins in the DT? That will surely take quite a long time, and I'm required to get GPIO functionality before that. Furthermore, even if I had a driver for a device whose pins can also be GPIOs, say smartcard. If the driver is built as a module, and has not been yet loaded in, then, since you want the pinctrl node has a child node, no gpiochip struct could have been registered, and the pins can't be used as GPIOs. Does this mean that to use the smartcard's pin as GPIOs, the smartcard driver must absolutely be loaded, even though smartcards are never used? > Have you read the section "GPIO mode pitfalls" in the documentation? > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/pinctrl.txt > > It is very important that you know for sure whether it is actual > GPIO or just named that way by some hardware engineer. I did read the documentation, and I'm pretty sure those pins are all actual GPIOs: once a pin's mode flag has been cleared, it can be arbitrarily set high or low, or listened to by kernel code. Regards, Yves. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-05-12 15:31 ` Yves Lefloch @ 2017-05-22 15:12 ` Linus Walleij 2017-05-24 9:41 ` Thibaud Cornic 0 siblings, 1 reply; 17+ messages in thread From: Linus Walleij @ 2017-05-22 15:12 UTC (permalink / raw) To: Yves Lefloch; +Cc: linux-gpio, Alexandre Courbot, Mason, Thibaud Cornic On Fri, May 12, 2017 at 5:31 PM, Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> wrote: > On 05/11/2017 05:22 PM, Linus Walleij wrote: >> Implement everything you have a data sheet for even if you >> do not use it right now. It is good to have. > > Do you mean that I should submit a driver for every single device > before I can declare its pins in the DT? That will surely take > quite a long time, and I'm required to get GPIO functionality before > that. I mean that when you write a GPIO driver, i.e. ONE driver for ONE GPIO controlling hardware, you should implement all the functionality for THAT GPIO block. > Furthermore, even if I had a driver for a device whose pins can also > be GPIOs, say smartcard. If the driver is built as a module, and has > not been yet loaded in, then, since you want the pinctrl node has a > child node, no gpiochip struct could have been registered, and the pins > can't be used as GPIOs. > Does this mean that to use the smartcard's pin as GPIOs, the smartcard > driver must absolutely be loaded, even though smartcards are never used? I do not understand this remark. If the GPIOs are provided by the smartcard IP block inside your chip, yes then it makes sense to spawn the GPIO driver as part of the smartcard driver. Usually the GPIO hardware is a separate IP block from any others (such as smartcard) so this situation seldom occurs. Is you system such that there is an IO range for something like a smart card, and then that IO range also contains registers for reusing some or all pins as GPIO? Then yes, we usually write a driver for this whole IP block and let it expose a gpiochip as part of its main function. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-05-22 15:12 ` Linus Walleij @ 2017-05-24 9:41 ` Thibaud Cornic 2017-05-29 10:02 ` Linus Walleij 0 siblings, 1 reply; 17+ messages in thread From: Thibaud Cornic @ 2017-05-24 9:41 UTC (permalink / raw) To: Linus Walleij, Yves Lefloch; +Cc: linux-gpio, Alexandre Courbot, Mason On 22/05/2017 17:12, Linus Walleij wrote: > On Fri, May 12, 2017 at 5:31 PM, Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> wrote: >> On 05/11/2017 05:22 PM, Linus Walleij wrote: >>> Implement everything you have a data sheet for even if you do not use it right now. It is good to have. >> Do you mean that I should submit a driver for every single device before I can declare its pins in the DT? That will surely take quite a long time, and I'm required to get GPIO functionality before that. > I mean that when you write a GPIO driver, i.e. ONE driver for ONE GPIO controlling hardware, you should implement all the functionality for THAT GPIO block. Right now, Yves works on the GPIOs, and he'd like to be able to support all the GPIOs of the chip, even those which are muxed with the UART/smart card/other IPs pins. For example, we have a UART2 IP, which pins can be used as GPIOs or for the UART functionality. The registers which toggle the GPIO/UART "mode" of these pins are in the address range of the UART block. Does your answer mean that you won't allow any other code than a UART driver to touch those registers? Which would mean that to be able to use those pins as GPIOs, Yves must first submit the UART driver and DT node, then modify it to somehow expose a GPIO functionality? >> Furthermore, even if I had a driver for a device whose pins can also be GPIOs, say smartcard. If the driver is built as a module, and has not been yet loaded in, then, since you want the pinctrl node has a child node, no gpiochip struct could have been registered, and the pins can't be used as GPIOs. Does this mean that to use the smartcard's pin as GPIOs, the smartcard driver must absolutely be loaded, even though smartcards are never used? > I do not understand this remark. If the GPIOs are provided by the smartcard IP block inside your chip, yes then it makes sense to spawn the GPIO driver as part of the smartcard driver. Let's talk about UART instead (as there's no real use case for a dynamic switch of pin usage on the board between the smart card and something else). The remark is: when the UART driver is loaded, it will start using the pins in the UART mode. And this driver is not designed to serve GPIO requests. It seems like a big modification to add this (like a wrapper completely disabling the function for which that driver has been written). Do we need to do this for all the drivers which control pins which can be used as GPIOs? I had imagined that the pin controller driver would be the one writing in the "pin mode" registers, wherever they might be located, and we would add "pin request" calls in the other drivers (UART, smart card) which would tell the pin controller to put those pins in the UART/smart card mode, and not allow gpio requests to them. > Usually the GPIO hardware is a separate IP block from any others (such as smartcard) so this situation seldom occurs. Really? I imagine we're not the only company adding muxes to use certain IP pins as GPIOs. And those muxes a necessarily placed in those IP blocks, which (depending on the interconnect) will force the mux register address to be in the IP address range. > Is you system such that there is an IO range for something like a smart card, and then that IO range also contains registers for reusing some or all pins as GPIO? Yes exactly. It contains the register to reuse the smart card pins as GPIOs. And let's say the ethernet IO range contains the regs to reuse the ethernet pins. > Then yes, we usually write a driver for this whole IP block and let it expose a gpiochip as part of its main function. On top of my other remarks above, what if the driver is completely standard? Regards, Thibaud Cornic ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-05-24 9:41 ` Thibaud Cornic @ 2017-05-29 10:02 ` Linus Walleij 2017-05-30 13:59 ` Yves Lefloch 0 siblings, 1 reply; 17+ messages in thread From: Linus Walleij @ 2017-05-29 10:02 UTC (permalink / raw) To: Thibaud Cornic; +Cc: Yves Lefloch, linux-gpio, Alexandre Courbot, Mason On Wed, May 24, 2017 at 11:41 AM, Thibaud Cornic <thibaud_cornic@sigmadesigns.com> wrote: > Right now, Yves works on the GPIOs, and he'd like to be able to support all the GPIOs of the chip, even those > which are muxed with the UART/smart card/other IPs pins. > For example, we have a UART2 IP, which pins can be used as GPIOs or for the UART functionality. > The registers which toggle the GPIO/UART "mode" of these pins are in the address range of the UART block. OK I see. > Does your answer mean that you won't allow any other code than a UART driver to touch those registers? No. Your answer seems to reflect an organizational problem though: https://en.wikipedia.org/wiki/Stovepipe_(organisation) > Which would mean that to be able to use those pins as GPIOs, Yves must first submit the UART driver and DT node, > then modify it to somehow expose a GPIO functionality? What is needed is first a proper DT represenation and second a driver. I think this: uart { compatible = "..."; ... pinctrl-gpio { compatible = "..."; ... }; }; Is going to be the pattern you want to solidify first. Then you can think about how to engineer the driver(s). >>> Furthermore, even if I had a driver for a device whose pins can also be GPIOs, say smartcard. >>> If the driver is built as a module, and has not been yet loaded in, then, since you want the pinctrl >>> node has a child node, no gpiochip struct could have been registered, and the pins can't be used >>> as GPIOs. Does this mean that to use the smartcard's pin as GPIOs, the smartcard driver must >>> absolutely be loaded, even though smartcards are never used? >> I do not understand this remark. If the GPIOs are provided by the smartcard IP block inside your >> chip, yes then it makes sense to spawn the GPIO driver as part of the smartcard driver. > Let's talk about UART instead (as there's no real use case for a dynamic switch of pin usage on the board between > the smart card and something else). > The remark is: when the UART driver is loaded, it will start using the pins in the UART mode. And this driver is not > designed to serve GPIO requests. It seems like a big modification to add this (like a wrapper completely disabling the > function for which that driver has been written). > Do we need to do this for all the drivers which control pins which can be used as GPIOs? There are tons of UART drivers out there asking separate pin control drivers to provide them the pins they use. What you need is to abstract out the pin control portions and let the UART driver grab these. Linux already grabs the "default" pin control setting for any driver right before it calls probe() on the driver, see drivers/base/pinctrl.c The pin control resources need to be created from an orthogonal driver, whether the addresses used by this driver are in the same range as the UART is not important, it however seems that it needs to be instantiated before the UART driver and they can hardly be one and the same. This is a Linux shortcoming augmented by deferred probe and other mechanisms such as initcall reordering (shudder). > I had imagined that the pin controller driver would be the one writing in the "pin mode" registers, wherever they might > be located, That is one way to do it. But another way to do it is to instantiate one pin controller per pin mode register range. Do whatever you think becomes most elegant. > and we would add "pin request" calls in the other drivers (UART, smart card) which would tell the pin > controller to put those pins in the UART/smart card mode, and not allow gpio requests to them. The "default" mode will be requested automatically. If you need more pin modes, you need to request them using explicit code. >> Usually the GPIO hardware is a separate IP block from any others (such as smartcard) >> so this situation seldom occurs. > > Really? I imagine we're not the only company adding muxes to use certain IP pins as GPIOs. That is usually controlled by a register range specifically dedicated to pins and GPIOs, and not intermingled with the register range for the other IP as seems to be your case. So they see the "pad ring" as a device. Essentially it is usually constructed by stacking together control lines to the I/O cells into a register range just for that purpose. Much like so: https://dflund.se/~triad/papers/pincontrol.pdf > And those muxes a necessarily placed in those IP blocks, which (depending on the interconnect) > will force the mux register address to be in the IP address range. I've heard about this before but noone ever submitted a driver for that. (I'm sorry you are the first, being first always sucks.) >> Is you system such that there is an IO range for something like a smart card, and >> then that IO range also contains registers for reusing some or all pins as GPIO? > > Yes exactly. It contains the register to reuse the smart card pins as GPIOs. > And let's say the ethernet IO range contains the regs to reuse the ethernet pins. Interesting. That is a first, or a second I heard, or something. >> Then yes, we usually write a driver for this whole IP block and let it expose a >> gpiochip as part of its main function. > > On top of my other remarks above, what if the driver is completely standard? Better to have the pin control driver separate. Sorry if I was wrong about that. But I don't know whether to: (A) Try to collect the address ranges from all over the place into one driver. (B) Try to create one instance of a pin controller per IP I'd like to see some code in the direction you choose I guess. Either should work, I just don't know how to do (A) in an elegant manner, nor how to instantiate (B) in a nice way, so some challenges there. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-05-29 10:02 ` Linus Walleij @ 2017-05-30 13:59 ` Yves Lefloch 2017-05-31 0:24 ` Linus Walleij 0 siblings, 1 reply; 17+ messages in thread From: Yves Lefloch @ 2017-05-30 13:59 UTC (permalink / raw) To: Linus Walleij, Thibaud Cornic; +Cc: linux-gpio, Alexandre Courbot, Mason On 05/29/2017 12:02 PM, Linus Walleij wrote: > But I don't know whether to: > > (A) Try to collect the address ranges from all over the place into one driver. > > (B) Try to create one instance of a pin controller per IP > > I'd like to see some code in the direction you choose I guess. Either should > work, I just don't know how to do (A) in an elegant manner, nor how to > instantiate (B) in a nice way, so some challenges there. I've put together a draft to try and solve this problem using approach (A). Each IP node gets a GPIO child node, since this is what the hardware looks like, and they are all tied together using phandles to a central pin controller node. The draft only demonstrates the solution for the pins dedicated to GPIO, namely `gpio_sys', and for the smartcard pins, namely `gpio_smcard'. As before, bindings are not yet commented. BTW, I'm sure there is a far better way than this `all_pins' array, let me know if you have feedback on that. Regards, Yves. >From f4e7e9314a3201a3a2ca00ad2381114af4ff6fd9 Mon Sep 17 00:00:00 2001 From: Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> Date: Tue, 25 Apr 2017 14:06:46 +0200 Subject: [PATCH] drivers: pinctrl: Add a pinctrl for mach-tango Change-Id: Id5b5872b2a7d116829ddc75577f26115eb6e1c98 Signed-off-by: Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> --- arch/arm/boot/dts/tango4-common.dtsi | 28 ++ arch/arm/configs/tango4_defconfig | 2 + arch/arm/mach-tango/Kconfig | 1 + drivers/pinctrl/Kconfig | 7 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-tango.c | 566 +++++++++++++++++++++++++++++++++++ 6 files changed, 605 insertions(+) create mode 100644 drivers/pinctrl/pinctrl-tango.c diff --git a/arch/arm/boot/dts/tango4-common.dtsi b/arch/arm/boot/dts/tango4-common.dtsi index a3e81ab..671f9fc 100644 --- a/arch/arm/boot/dts/tango4-common.dtsi +++ b/arch/arm/boot/dts/tango4-common.dtsi @@ -18,6 +18,34 @@ #address-cells = <1>; #size-cells = <1>; + pinctrl { + compatible = "sigma,smp8758-pinctrl"; + groups = <&gpio_sys>, <&gpio_smcard>; + }; + + gpio_sys: gpio@10500 { + reg = <0x10500 0x8>; + label = "sys"; + ngpios = <16>; + tango,mux = "none"; + gpio-controller; + #gpio-cells = <1>; + }; + + smcard: smcard@6c300 { + /* Driver in the works. */ + reg = <0x6c300 0x6c>; + + gpio_smcard: gpio@6c358 { + reg = <0x6c358 0xc>; + label = "smcard"; + ngpios = <8>; + tango,mux = "pin"; + gpio-controller; + #gpio-cells = <1>; + }; + }; + periph_clk: periph_clk { compatible = "fixed-factor-clock"; clocks = <&clkgen CPU_CLK>; diff --git a/arch/arm/configs/tango4_defconfig b/arch/arm/configs/tango4_defconfig index b26bb4e..b58b01b 100644 --- a/arch/arm/configs/tango4_defconfig +++ b/arch/arm/configs/tango4_defconfig @@ -68,7 +68,9 @@ CONFIG_SERIAL_OF_PLATFORM=y # CONFIG_HW_RANDOM is not set CONFIG_I2C=y CONFIG_I2C_XLR=y +CONFIG_PINCTRL_TANGO=y CONFIG_GPIOLIB=y +CONFIG_GPIO_SYSFS=y CONFIG_THERMAL=y CONFIG_CPU_THERMAL=y CONFIG_TANGO_THERMAL=y diff --git a/arch/arm/mach-tango/Kconfig b/arch/arm/mach-tango/Kconfig index ebe15b9..479c9aa 100644 --- a/arch/arm/mach-tango/Kconfig +++ b/arch/arm/mach-tango/Kconfig @@ -11,3 +11,4 @@ config ARCH_TANGO select HAVE_ARM_SCU select HAVE_ARM_TWD select TANGO_IRQ + select PINCTRL diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index 0e75d94..5648e76 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -179,6 +179,13 @@ config PINCTRL_ST select PINCONF select GPIOLIB_IRQCHIP +config PINCTRL_TANGO + tristate "Tango pin control driver" + depends on OF + select PINMUX + help + Say yes to activate the pinctrl/gpio driver for Tango chips. + config PINCTRL_TZ1090 bool "Toumaz Xenif TZ1090 pin control driver" depends on SOC_TZ1090 diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile index 11bad37..d5563e49 100644 --- a/drivers/pinctrl/Makefile +++ b/drivers/pinctrl/Makefile @@ -26,6 +26,7 @@ obj-$(CONFIG_PINCTRL_ROCKCHIP) += pinctrl-rockchip.o obj-$(CONFIG_PINCTRL_SINGLE) += pinctrl-single.o obj-$(CONFIG_PINCTRL_SIRF) += sirf/ obj-$(CONFIG_ARCH_TEGRA) += tegra/ +obj-$(CONFIG_PINCTRL_TANGO) += pinctrl-tango.o obj-$(CONFIG_PINCTRL_TZ1090) += pinctrl-tz1090.o obj-$(CONFIG_PINCTRL_TZ1090_PDC) += pinctrl-tz1090-pdc.o obj-$(CONFIG_PINCTRL_U300) += pinctrl-u300.o diff --git a/drivers/pinctrl/pinctrl-tango.c b/drivers/pinctrl/pinctrl-tango.c new file mode 100644 index 0000000..33b2ad9 --- /dev/null +++ b/drivers/pinctrl/pinctrl-tango.c @@ -0,0 +1,566 @@ +/* + * Copyright (C) 2017 Sigma Designs + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + */ +#include <linux/bitops.h> +#include <linux/gpio/driver.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/pinctrl/pinctrl.h> +#include <linux/pinctrl/pinmux.h> +#include <linux/platform_device.h> + +/* Pin regs are laid out this way: + * ______________________________ + * | bitwise mask | bitwise val | + * 32 16 0 + * + * To set bit n, one must write (1 << (16+n) | n) to the reg. + * Consequently, one reg can't handle more than 16 pins, so n < 16. + * + * There are 3 kind of registers that follow this layout: value registers, + * direction registers, and gpio-mode registers. + * The latter kind allows muxing between the primary function (eg UART), which + * is the pin's direction and value entirely controlled by hardware, and the + * "GPIO mode", which is the pin's direction and value entirely controlled by + * software. This gpio-mode register only exists for some GPIO groups. + * + * Additionally, for some pin groups, there is no gpio-mode register that muxes + * with the granularity of a pin, but rather with the granularity of the whole + * group. In that case, such mux register does *not* follow the layout above. + * + * Finally, it is possible for some groups to have neither type of muxing. In + * that case, the pins are dedicated GPIOs. + * + * Registers are organized this way for each group: + * group_mux? (direction value pin_mux?)+ + */ + +enum mux_type { + MUX_NONE, + MUX_PIN, + MUX_GROUP, +}; + +/* Which magic values to write in the mux group registers. */ +#define MUX_GROUP_GPIO 0x6 +#define MUX_GROUP_ALTF 0x1 + +/* Conversion table for char* <-> enum mux_type */ +static const struct { + const char *str; + enum mux_type type; +} mux_types[] = { + { .str = "none", .type = MUX_NONE, }, + { .str = "pin", .type = MUX_PIN, }, + { .str = "group", .type = MUX_GROUP, }, +}; + +/* Pinctrl device data */ +struct pingroup { + struct list_head list; + const char *label; + unsigned base; + unsigned ngpio; + size_t sz; + phys_addr_t pa; + void __iomem *va; + enum mux_type mux_type; +}; + +/* Platform device data */ +struct pinctrl_data { + struct list_head group_list; +}; + +#define __reg_offset_from_pin(pg, pin) ( \ + ((pg)->mux_type == MUX_GROUP ? sizeof(u32) : 0) + \ + ((pg)->mux_type == MUX_PIN ? 3 : 2) * sizeof(u32) * ((pin)/16) \ + ) +#define group_mux_reg_va(pg) ((pg)->va) +#define dir_reg_va(pg, pin) (group_mux_reg_va(pg) + __reg_offset_from_pin(pg, pin)) +#define val_reg_va(pg, pin) (dir_reg_va(pg, pin) + sizeof(u32)) +#define pin_mux_reg_va(pg, pin) (val_reg_va(pg, pin) + sizeof(u32)) + +static int pinreg_get_bit(void __iomem *addr, unsigned offset) +{ + unsigned long val = readl(addr); + + return test_bit(offset, &val); +} + +static void pinreg_set_bit(void __iomem *addr, unsigned offset, int value) +{ + unsigned long val = 0; + + /* Enable change by writing upper half-word too. */ + set_bit(offset + 16, &val); + if (value) + set_bit(offset, &val); + + writel(val, addr); +} + +static void pinreg_set_multiple(void __iomem *addr, u16 mask, u16 bits) +{ + unsigned long val = (mask << 16) | bits; + + writel(val, addr); +} + +static int gpio_get(struct gpio_chip *gc, unsigned offset) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + return pinreg_get_bit(val_reg_va(group, offset), offset % 16); +} + +static void gpio_set(struct gpio_chip *gc, unsigned offset, int value) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return; + + pinreg_set_bit(val_reg_va(group, offset), offset % 16, value); +} + +static int gpio_get_direction(struct gpio_chip *gc, unsigned offset) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + /* Bit set means output. */ + return !pinreg_get_bit(dir_reg_va(group, offset), offset % 16); +} + +static int gpio_direction_input(struct gpio_chip *gc, unsigned offset) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + /* Bit set means output. */ + pinreg_set_bit(dir_reg_va(group, offset), offset % 16, 0); + + return 0; +} + +static int gpio_direction_output(struct gpio_chip *gc, unsigned offset, int value) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + /* The output value can be changed before the direction is. */ + pinreg_set_bit(val_reg_va(group, offset), offset % 16, value); + + /* Bit set means output. */ + pinreg_set_bit(dir_reg_va(group, offset), offset % 16, 1); + + return 0; +} + +static void gpio_set_multiple(struct gpio_chip *gc, + unsigned long *mask, unsigned long *bits) +{ + unsigned i, j; + unsigned num_of_ulongs; + struct pingroup *group = gpiochip_get_data(gc); + + if (!group) + return; + + num_of_ulongs = (group->ngpio / 32) + 1; + for (i = 0; i < num_of_ulongs; i++) + for (j = 0; j < 1; j++) { + u16 _mask = (u16)(*(mask + i) >> (16 * j)); + u16 _bits = (u16)(*(bits + i) >> (16 * j)); + pinreg_set_multiple(val_reg_va(group, 2*i + j), _mask, _bits); + } +} + +static const unsigned int all_pins[] = { + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, + 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, + 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, + 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, + 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, + 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, + 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, + 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, + 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, + 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, +}; + +static int get_group_count(struct pinctrl_dev *pctldev) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group) + return -EINVAL; + + return group->mux_type == MUX_NONE ? 0 : 1; +} + +static const char *get_group_name(struct pinctrl_dev *pctldev, unsigned selector) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group) + return ERR_PTR(-EINVAL); + + return group->label; +} + +static int get_group_pins(struct pinctrl_dev *pctldev, unsigned selector, + const unsigned **pins, unsigned *num_pins) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group) + return -EINVAL; + + *pins = all_pins + group->base; + *num_pins = group->ngpio; + + return 0; +} + +static const struct pinctrl_ops pctlops = { + .get_groups_count = get_group_count, + .get_group_name = get_group_name, + .get_group_pins = get_group_pins, +}; + +int get_function_groups(struct pinctrl_dev *pctldev, unsigned selector, + const char * const **groups, unsigned *num_groups) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group) + return -EINVAL; + + if (group->mux_type == MUX_NONE) { + *groups = NULL; + *num_groups = 0; + } else { + *groups = &group->label; + *num_groups = 1; + } + + return 0; +} + +int set_mux(struct pinctrl_dev *pctldev, unsigned func_selector, + unsigned group_selector) +{ + /* With only one possible alt function, muxing is only + * necessary for GPIO request/free. + */ + return 0; +} + +int gpio_request_enable(struct pinctrl_dev *pctldev, + struct pinctrl_gpio_range *range, unsigned offset) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + switch (group->mux_type) { + case MUX_NONE: + break; + case MUX_PIN: + pinreg_set_bit(pin_mux_reg_va(group, offset), offset % 16, 1); + break; + case MUX_GROUP: + writel(MUX_GROUP_GPIO, group_mux_reg_va(group)); + break; + default: + return -EINVAL; + } + + return 0; +} + +void gpio_disable_free(struct pinctrl_dev *pctldev, + struct pinctrl_gpio_range *range, unsigned offset) +{ + struct pingroup *group; + + group = pinctrl_dev_get_drvdata(pctldev); + + if (!group || offset >= group->ngpio) + return; + + switch (group->mux_type) { + case MUX_NONE: + break; + case MUX_PIN: + pinreg_set_bit(pin_mux_reg_va(group, offset), offset % 16, 0); + break; + case MUX_GROUP: + writel(MUX_GROUP_ALTF, group_mux_reg_va(group)); + break; + } +} + +const static struct pinmux_ops pmxops = { + .get_functions_count = get_group_count, + .get_function_name = get_group_name, + .get_function_groups = get_function_groups, + .set_mux = set_mux, + .gpio_request_enable = gpio_request_enable, + .gpio_disable_free = gpio_disable_free, + .strict = true, +}; + +static int get_mux_type_from_str(const char *str, enum mux_type *type) +{ + unsigned i; + for (i = 0; i < ARRAY_SIZE(mux_types); i++) + if (!strcmp(mux_types[i].str, str)) { + *type = mux_types[i].type; + return 0; + } + return -EINVAL; +} + +static int get_node_info(struct device_node *np, struct pingroup *group) +{ + /* Extract info from the pinctrl node: + * - the label; + * - the number of pins; + * - the address and size of the registers; + * - the mux type. + */ + int ret; + u32 ngpio; + u32 reg[2]; + const char *label; + const char *tango_mux; + + ret = of_property_read_u32(np, "ngpios", &ngpio); + if (ret) + return ret; + ret = of_property_read_u32_array(np, "reg", reg, 2); + if (ret) + return ret; + ret = of_property_read_string(np, "label", &label); + if (ret) + return ret; + ret = of_property_read_string(np, "tango,mux", &tango_mux); + if (ret) + return ret; + + ret = get_mux_type_from_str(tango_mux, &group->mux_type); + if (ret) + return ret; + group->label = label; + group->ngpio = ngpio; + group->pa = reg[0]; + group->sz = reg[1]; + + return 0; +} + +static int register_group_gpios(struct device *dev, + struct pingroup *group, + struct pinctrl_dev *pindev) +{ + int ret; + struct gpio_chip *gchip; + struct pinctrl_gpio_range *prange; + + gchip = devm_kzalloc(dev, sizeof(*gchip), GFP_KERNEL); + if (!gchip) + return -ENOMEM; + prange = devm_kzalloc(dev, sizeof(*prange), GFP_KERNEL); + if (!prange) + return -ENOMEM; + + gchip->label = group->label; + gchip->parent = dev; + gchip->owner = THIS_MODULE; + gchip->get_direction = gpio_get_direction; + gchip->direction_input = gpio_direction_input; + gchip->direction_output = gpio_direction_output; + gchip->get = gpio_get; + gchip->set = gpio_set; + gchip->set_multiple = gpio_set_multiple; + gchip->base = group->base; + gchip->ngpio = group->ngpio; + ret = devm_gpiochip_add_data(dev, gchip, group); + if (ret) + return ret; + + prange->name = group->label; + prange->id = group->base; + prange->base = group->base; + prange->pin_base = group->base; + prange->npins = group->ngpio; + prange->gc = gchip; + pinctrl_add_gpio_range(pindev, prange); + + return 0; +} + +static int register_group_pins(struct device *dev, + struct pingroup *group) +{ + unsigned i; + struct pinctrl_desc *pdesc; + struct pinctrl_pin_desc *ppdesc; + struct pinctrl_dev *pindev; + + pdesc = devm_kzalloc(dev, sizeof(*pdesc), GFP_KERNEL); + if (!pdesc) + return -ENOMEM; + ppdesc = devm_kzalloc(dev, group->ngpio * sizeof(*ppdesc), GFP_KERNEL); + if (!ppdesc) + return -ENOMEM; + + for (i = 0; i < group->ngpio; i++) + ppdesc[i].number = group->base + i; + + pdesc->name = group->label; + pdesc->pins = ppdesc; + pdesc->npins = group->ngpio; + pdesc->pctlops = &pctlops; + pdesc->pmxops = &pmxops; + pdesc->owner = THIS_MODULE; + + pindev = devm_pinctrl_register(dev, pdesc, group); + if (IS_ERR(pindev)) + return PTR_ERR(pindev); + + return register_group_gpios(dev, group, pindev); +} + +static int register_group(struct platform_device *pdev, struct device_node *np) +{ + int ret; + struct pingroup *group; + struct pinctrl_data *pdata; + + group = devm_kzalloc(&pdev->dev, sizeof(*group), GFP_KERNEL); + if (!group) + return -ENOMEM; + + /* Extract info from the device node. */ + ret = get_node_info(np, group); + if (ret) + return ret; + + /* Get the base number of this group by looking at the previous one. */ + pdata = platform_get_drvdata(pdev); + if (!pdata) + return -EINVAL; + if (list_empty(&pdata->group_list)) { + group->base = 0; + } else { + struct pingroup *prev; + prev = list_last_entry(&pdata->group_list, typeof(*prev), list); + group->base = prev->base + prev->ngpio; + } + + /* Ioremap the group registers. */ + group->va = devm_ioremap_nocache(&pdev->dev, group->pa, group->sz); + if (!group->va) + return -ENOMEM; + + /* Register the group pins. */ + ret = register_group_pins(&pdev->dev, group); + if (ret) + return ret; + + /* Add this new group to the list. */ + list_add_tail(&group->list, &pdata->group_list); + + dev_dbg(&pdev->dev, "%8s base=%d ngpio=%d\n", + group->label, group->base, group->ngpio); + return 0; +} + +static int tango_pinctrl_probe(struct platform_device *pdev) +{ + unsigned i = 0; + unsigned groups_num; + struct pinctrl_data *pdata; + struct device_node *np = pdev->dev.of_node; + + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return -ENOMEM; + INIT_LIST_HEAD(&pdata->group_list); + platform_set_drvdata(pdev, pdata); + + groups_num = of_count_phandle_with_args(np, "groups", NULL); + if (!groups_num) + return -EINVAL; + + for (i = 0; i < groups_num; i++) { + int ret; + struct of_phandle_args args; + ret = of_parse_phandle_with_args(np, "groups", NULL, i, &args); + if (ret) + return ret; + ret = register_group(pdev, args.np); + if (ret) + return ret; + } + + return 0; +} + +static const struct of_device_id tango_pinctrl_of_match[] = { + { .compatible = "sigma,smp8758-pinctrl", }, + {}, +}; +MODULE_DEVICE_TABLE(of, tango_pinctrl_of_match); + +static struct platform_driver tango_pinctrl_driver = { + .driver = { + .name = "pinctrl-tango", + .of_match_table = of_match_ptr(tango_pinctrl_of_match), + }, + .probe = tango_pinctrl_probe, +}; + +static int __init tango_pinctrl_init(void) +{ + return platform_driver_register(&tango_pinctrl_driver); +} +module_init(tango_pinctrl_init); + +static void __exit tango_pinctrl_exit(void) +{ + platform_driver_unregister(&tango_pinctrl_driver); +} +module_exit(tango_pinctrl_exit); + +MODULE_AUTHOR("Sigma Designs"); +MODULE_DESCRIPTION("Tango pinctrl driver"); +MODULE_LICENSE("GPL"); -- 2.10.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-05-30 13:59 ` Yves Lefloch @ 2017-05-31 0:24 ` Linus Walleij 2017-05-31 16:36 ` Yves Lefloch 0 siblings, 1 reply; 17+ messages in thread From: Linus Walleij @ 2017-05-31 0:24 UTC (permalink / raw) To: Yves Lefloch; +Cc: Thibaud Cornic, linux-gpio, Alexandre Courbot, Mason On Tue, May 30, 2017 at 3:59 PM, Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> wrote: > I've put together a draft to try and solve this problem using approach (A). > Each IP node gets a GPIO child node, since this is what the hardware looks like, > and they are all tied together using phandles to a central pin controller node. I like the looks of this! > The draft only demonstrates the solution for the pins dedicated to GPIO, > namely `gpio_sys', and for the smartcard pins, namely `gpio_smcard'. It is elegant and it will scale. > As before, bindings are not yet commented. > > BTW, I'm sure there is a far better way than this `all_pins' array, let me know > if you have feedback on that. So that is this: > +static const unsigned int all_pins[] = { > + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, > + 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, > + 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, > + 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, > + 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, > + 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, > + 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, > + 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, > + 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, > + 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, > +}; That looks a bit like what we usually put in pin descriptors. (...) This: > + struct pinctrl_gpio_range *prange; > + (...) > + prange->name = group->label; > + prange->id = group->base; > + prange->base = group->base; > + prange->pin_base = group->base; > + prange->npins = group->ngpio; > + prange->gc = gchip; > + pinctrl_add_gpio_range(pindev, prange); Can't you just add that to the device tree using the gpio-ranges property? It is handled in the core and should "just work". The documentation for that is a bit terse in Documentation/devicetree/bindings/gpio/gpio.txt but if you just grep for gpio-ranges you will get the hang of it. It is handled from the gpiochip side which is advisible, even if you hardcode it in the driver. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-05-31 0:24 ` Linus Walleij @ 2017-05-31 16:36 ` Yves Lefloch 2017-06-09 8:14 ` Linus Walleij 0 siblings, 1 reply; 17+ messages in thread From: Yves Lefloch @ 2017-05-31 16:36 UTC (permalink / raw) To: Linus Walleij; +Cc: Thibaud Cornic, linux-gpio, Alexandre Courbot, Mason On 05/31/2017 02:24 AM, Linus Walleij wrote: >> BTW, I'm sure there is a far better way than this `all_pins' array, let me know >> if you have feedback on that. > > So that is this: > >> +static const unsigned int all_pins[] = { >> + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, >> + 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, >> + 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, >> + 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, >> + 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, >> + 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, >> + 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, >> + 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, >> + 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, >> + 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, >> +}; > > That looks a bit like what we usually put in pin descriptors. This array is here because of my implementation of get_group_pins() in `struct pinctrl_ops'. It seems the function is expecting such an array. I mean I probably could have done something like: static const unsigned int smcard_pins[] = { 35, 36, 37, ... }; and returned `smcard_pins' instead of `&all_pins[35]' But the thing is, this `35' is not a constant number because it depends on the number of pins that got registered before the smartcard's own. I know it should reflect the number of the pin on my board's spec, and it will once all pin groups are put in the device tree in the right order. That being said, we might want to add a `base' property to the GPIO nodes which are parsed by the pinctrl driver to make sure this `35' never moves. > This: > >> + struct pinctrl_gpio_range *prange; >> + > (...) >> + prange->name = group->label; >> + prange->id = group->base; >> + prange->base = group->base; >> + prange->pin_base = group->base; >> + prange->npins = group->ngpio; >> + prange->gc = gchip; >> + pinctrl_add_gpio_range(pindev, prange); > > Can't you just add that to the device tree using the gpio-ranges > property? It is handled in the core and should "just work". > > The documentation for that is a bit terse in > Documentation/devicetree/bindings/gpio/gpio.txt > but if you just grep for gpio-ranges you will get the hang of it. > > It is handled from the gpiochip side which is advisible, even if > you hardcode it in the driver. OK got it. I missed the deprecation warning in the doc, my bad. I've updated the patch to reflect this change, and got rid of the `label' property in the GPIO nodes to use `gpio-ranges-group-names' instead. I tried to carry on and add the other IPs' pins, and I've got a question regarding pin requesting. See, adding the smartcard's pins was easy because we don't have a driver for it yet, so the pins are just straightforward GPIOs, which the user-space can request using /sys/class/gpio/export. But that's not the case for the ethernet device, whose pins are already in use, although not requested explicitly by its driver (drivers/net/ethernet/aurora/nb8800.c). Which means user-space can also request/set them within the sysfs, and that is wrong. So I wondered: - Is there a mechanism in the device tree to automatically request some pins for a driver once it gets loaded? I have looked at the `stateN_nodeM' thing in the documentation, but that doesn't seem to do the job without changes in the ethernet driver's code. - Should the pins be declared at all if we know they are not going to be used for anything else than, for instance, ethernet? What about some removable device then, for instance UART as Thibaud pointed out previously? Thanks and regards, Yves. >From 0c0d9bd5e55203b4a450e44097e1e50fa1d525aa Mon Sep 17 00:00:00 2001 From: Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> Date: Tue, 25 Apr 2017 14:06:46 +0200 Subject: [PATCH] drivers: pinctrl: Add a pinctrl for mach-tango Change-Id: Id5b5872b2a7d116829ddc75577f26115eb6e1c98 Signed-off-by: Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> --- arch/arm/boot/dts/tango4-common.dtsi | 46 +++ arch/arm/configs/tango4_defconfig | 2 + arch/arm/mach-tango/Kconfig | 1 + drivers/pinctrl/Kconfig | 7 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-tango.c | 552 +++++++++++++++++++++++++++++++++++ 6 files changed, 609 insertions(+) create mode 100644 drivers/pinctrl/pinctrl-tango.c diff --git a/arch/arm/boot/dts/tango4-common.dtsi b/arch/arm/boot/dts/tango4-common.dtsi index a3e81ab..4eee27d 100644 --- a/arch/arm/boot/dts/tango4-common.dtsi +++ b/arch/arm/boot/dts/tango4-common.dtsi @@ -18,6 +18,42 @@ #address-cells = <1>; #size-cells = <1>; + pinctrl: pinctrl { + compatible = "sigma,smp8758-pinctrl"; + groups = <&gpio_sys>, <&gpio_eth0>, <&gpio_smcard>; + + pin_state_eth0: state_eth0 { + function = "eth0"; + groups = "eth0"; + }; + }; + + gpio_sys: gpio@10500 { + reg = <0x10500 0x8>; + ngpios = <16>; + tango,mux = "none"; + gpio-controller; + #gpio-cells = <1>; + gpio-ranges = <&pinctrl 0 0 0>; + gpio-ranges-group-names = "sys"; + }; + + smcard: smcard@6c300 { + /* Driver in the works. */ + reg = <0x6c300 0x6c>; + #address-cells = <1>; + #size-cells = <1>; + + gpio_smcard: gpio@6c358 { + reg = <0x6c358 0xc>; + ngpios = <8>; + tango,mux = "pin"; + gpio-controller; + #gpio-cells = <1>; + gpio-ranges-group-names = "smcard"; + }; + }; + periph_clk: periph_clk { compatible = "fixed-factor-clock"; clocks = <&clkgen CPU_CLK>; @@ -151,6 +187,16 @@ reg = <0x26000 0x800>; interrupts = <38 IRQ_TYPE_LEVEL_HIGH>; clocks = <&clkgen SYS_CLK>; + pinctrl-0 = <&pin_state_eth0>; + + gpio_eth0: gpio@26404 { + reg = <0x26404 0x10>; + ngpios = <19>; + tango,mux = "group"; + gpio-controller; + #gpio-cells = <1>; + gpio-ranges-group-names = "eth0"; + }; }; dma: dma@290a0 { diff --git a/arch/arm/configs/tango4_defconfig b/arch/arm/configs/tango4_defconfig index b26bb4e..b58b01b 100644 --- a/arch/arm/configs/tango4_defconfig +++ b/arch/arm/configs/tango4_defconfig @@ -68,7 +68,9 @@ CONFIG_SERIAL_OF_PLATFORM=y # CONFIG_HW_RANDOM is not set CONFIG_I2C=y CONFIG_I2C_XLR=y +CONFIG_PINCTRL_TANGO=y CONFIG_GPIOLIB=y +CONFIG_GPIO_SYSFS=y CONFIG_THERMAL=y CONFIG_CPU_THERMAL=y CONFIG_TANGO_THERMAL=y diff --git a/arch/arm/mach-tango/Kconfig b/arch/arm/mach-tango/Kconfig index ebe15b9..479c9aa 100644 --- a/arch/arm/mach-tango/Kconfig +++ b/arch/arm/mach-tango/Kconfig @@ -11,3 +11,4 @@ config ARCH_TANGO select HAVE_ARM_SCU select HAVE_ARM_TWD select TANGO_IRQ + select PINCTRL diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index 0e75d94..5648e76 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -179,6 +179,13 @@ config PINCTRL_ST select PINCONF select GPIOLIB_IRQCHIP +config PINCTRL_TANGO + tristate "Tango pin control driver" + depends on OF + select PINMUX + help + Say yes to activate the pinctrl/gpio driver for Tango chips. + config PINCTRL_TZ1090 bool "Toumaz Xenif TZ1090 pin control driver" depends on SOC_TZ1090 diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile index 11bad37..d5563e49 100644 --- a/drivers/pinctrl/Makefile +++ b/drivers/pinctrl/Makefile @@ -26,6 +26,7 @@ obj-$(CONFIG_PINCTRL_ROCKCHIP) += pinctrl-rockchip.o obj-$(CONFIG_PINCTRL_SINGLE) += pinctrl-single.o obj-$(CONFIG_PINCTRL_SIRF) += sirf/ obj-$(CONFIG_ARCH_TEGRA) += tegra/ +obj-$(CONFIG_PINCTRL_TANGO) += pinctrl-tango.o obj-$(CONFIG_PINCTRL_TZ1090) += pinctrl-tz1090.o obj-$(CONFIG_PINCTRL_TZ1090_PDC) += pinctrl-tz1090-pdc.o obj-$(CONFIG_PINCTRL_U300) += pinctrl-u300.o diff --git a/drivers/pinctrl/pinctrl-tango.c b/drivers/pinctrl/pinctrl-tango.c new file mode 100644 index 0000000..0a50238 --- /dev/null +++ b/drivers/pinctrl/pinctrl-tango.c @@ -0,0 +1,552 @@ +/* + * Copyright (C) 2017 Sigma Designs + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + */ +#include <linux/bitops.h> +#include <linux/gpio/driver.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/pinctrl/pinctrl.h> +#include <linux/pinctrl/pinmux.h> +#include <linux/platform_device.h> + +/* Pin regs are laid out this way: + * ______________________________ + * | bitwise mask | bitwise val | + * 32 16 0 + * + * To set bit n, one must write (1 << (16+n) | n) to the reg. + * Consequently, one reg can't handle more than 16 pins, so n < 16. + * + * There are 3 kind of registers that follow this layout: value registers, + * direction registers, and gpio-mode registers. + * The latter kind allows muxing between the primary function (eg UART), which + * is the pin's direction and value entirely controlled by hardware, and the + * "GPIO mode", which is the pin's direction and value entirely controlled by + * software. This gpio-mode register only exists for some GPIO groups. + * + * Additionally, for some pin groups, there is no gpio-mode register that muxes + * with the granularity of a pin, but rather with the granularity of the whole + * group. In that case, such mux register does *not* follow the layout above. + * + * Finally, it is possible for some groups to have neither type of muxing. In + * that case, the pins are dedicated GPIOs. + * + * Registers are organized this way for each group: + * group_mux? (direction value pin_mux?)+ + */ + +enum mux_type { + MUX_NONE, + MUX_PIN, + MUX_GROUP, +}; + +/* Which magic values to write in the mux group registers. */ +#define MUX_GROUP_GPIO 0x6 +#define MUX_GROUP_ALTF 0x1 + +/* Conversion table for char* <-> enum mux_type */ +static const struct { + const char *str; + enum mux_type type; +} mux_types[] = { + { .str = "none", .type = MUX_NONE, }, + { .str = "pin", .type = MUX_PIN, }, + { .str = "group", .type = MUX_GROUP, }, +}; + +/* Pinctrl device data */ +struct pingroup { + struct list_head list; + const char *label; + unsigned base; + unsigned ngpio; + size_t sz; + phys_addr_t pa; + void __iomem *va; + enum mux_type mux_type; +}; + +/* Platform device data */ +struct pinctrl_data { + struct list_head group_list; +}; + +#define __reg_offset_from_pin(pg, pin) ( \ + ((pg)->mux_type == MUX_GROUP ? sizeof(u32) : 0) + \ + ((pg)->mux_type == MUX_PIN ? 3 : 2) * sizeof(u32) * ((pin)/16) \ + ) +#define group_mux_reg_va(pg) ((pg)->va) +#define dir_reg_va(pg, pin) (group_mux_reg_va(pg) + __reg_offset_from_pin(pg, pin)) +#define val_reg_va(pg, pin) (dir_reg_va(pg, pin) + sizeof(u32)) +#define pin_mux_reg_va(pg, pin) (val_reg_va(pg, pin) + sizeof(u32)) + +static int pinreg_get_bit(void __iomem *addr, unsigned offset) +{ + unsigned long val = readl(addr); + + /* TODO remove me later */ +#if IS_ENABLED(CONFIG_DEBUG) + BUG_ON(val & GENMASK(31, 16)); +#endif + + return test_bit(offset, &val); +} + +static void pinreg_set_bit(void __iomem *addr, unsigned offset, int value) +{ + unsigned long val = 0; + + /* Enable change by writing upper half-word too. */ + set_bit(offset + 16, &val); + if (value) + set_bit(offset, &val); + + writel(val, addr); + + /* TODO remove me later */ +#if IS_ENABLED(CONFIG_DEBUG) + BUG_ON(pinreg_get_bit(addr, offset) != value); +#endif +} + +static void pinreg_set_multiple(void __iomem *addr, u16 mask, u16 bits) +{ + unsigned long val = (mask << 16) | bits; + + writel(val, addr); +} + +static int gpio_get(struct gpio_chip *gc, unsigned offset) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + return pinreg_get_bit(val_reg_va(group, offset), offset % 16); +} + +static void gpio_set(struct gpio_chip *gc, unsigned offset, int value) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return; + + pinreg_set_bit(val_reg_va(group, offset), offset % 16, value); +} + +static int gpio_get_direction(struct gpio_chip *gc, unsigned offset) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + /* Bit set means output. */ + return !pinreg_get_bit(dir_reg_va(group, offset), offset % 16); +} + +static int gpio_direction_input(struct gpio_chip *gc, unsigned offset) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + /* Bit set means output. */ + pinreg_set_bit(dir_reg_va(group, offset), offset % 16, 0); + + return 0; +} + +static int gpio_direction_output(struct gpio_chip *gc, unsigned offset, int value) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + /* The output value can be changed before the direction is. */ + pinreg_set_bit(val_reg_va(group, offset), offset % 16, value); + + /* Bit set means output. */ + pinreg_set_bit(dir_reg_va(group, offset), offset % 16, 1); + + return 0; +} + +static void gpio_set_multiple(struct gpio_chip *gc, + unsigned long *mask, unsigned long *bits) +{ + unsigned i, j; + unsigned num_of_ulongs; + struct pingroup *group = gpiochip_get_data(gc); + + if (!group) + return; + + num_of_ulongs = (group->ngpio / 32) + 1; + for (i = 0; i < num_of_ulongs; i++) + for (j = 0; j < 1; j++) { + u16 _mask = (u16)(*(mask + i) >> (16 * j)); + u16 _bits = (u16)(*(bits + i) >> (16 * j)); + pinreg_set_multiple(val_reg_va(group, 2*i + j), _mask, _bits); + } +} + +static const unsigned int all_pins[] = { + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, + 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, + 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, + 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, + 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, + 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, + 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, + 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, + 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, + 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, +}; + +static int get_group_count(struct pinctrl_dev *pctldev) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group) + return -EINVAL; + + return group->mux_type == MUX_NONE ? 0 : 1; +} + +static const char *get_group_name(struct pinctrl_dev *pctldev, unsigned selector) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group) + return ERR_PTR(-EINVAL); + + return group->label; +} + +static int get_group_pins(struct pinctrl_dev *pctldev, unsigned selector, + const unsigned **pins, unsigned *num_pins) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group) + return -EINVAL; + + *pins = all_pins + group->base; + *num_pins = group->ngpio; + + return 0; +} + +static const struct pinctrl_ops pctlops = { + .get_groups_count = get_group_count, + .get_group_name = get_group_name, + .get_group_pins = get_group_pins, +}; + +int get_function_groups(struct pinctrl_dev *pctldev, unsigned selector, + const char * const **groups, unsigned *num_groups) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group) + return -EINVAL; + + if (group->mux_type == MUX_NONE) { + *groups = NULL; + *num_groups = 0; + } else { + *groups = &group->label; + *num_groups = 1; + } + + return 0; +} + +int set_mux(struct pinctrl_dev *pctldev, unsigned func_selector, + unsigned group_selector) +{ + /* With only one possible alt function, muxing is only + * necessary for GPIO request/free. + */ + return 0; +} + +int gpio_request_enable(struct pinctrl_dev *pctldev, + struct pinctrl_gpio_range *range, unsigned offset) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + switch (group->mux_type) { + case MUX_NONE: + break; + case MUX_PIN: + pinreg_set_bit(pin_mux_reg_va(group, offset), offset % 16, 1); + break; + case MUX_GROUP: + writel(MUX_GROUP_GPIO, group_mux_reg_va(group)); + break; + default: + return -EINVAL; + } + + return 0; +} + +void gpio_disable_free(struct pinctrl_dev *pctldev, + struct pinctrl_gpio_range *range, unsigned offset) +{ + struct pingroup *group; + + group = pinctrl_dev_get_drvdata(pctldev); + + if (!group || offset >= group->ngpio) + return; + + switch (group->mux_type) { + case MUX_NONE: + break; + case MUX_PIN: + pinreg_set_bit(pin_mux_reg_va(group, offset), offset % 16, 0); + break; + case MUX_GROUP: + writel(MUX_GROUP_ALTF, group_mux_reg_va(group)); + break; + } +} + +const static struct pinmux_ops pmxops = { + .get_functions_count = get_group_count, + .get_function_name = get_group_name, + .get_function_groups = get_function_groups, + .set_mux = set_mux, + .gpio_request_enable = gpio_request_enable, + .gpio_disable_free = gpio_disable_free, + .strict = true, +}; + +static int get_mux_type_from_str(const char *str, enum mux_type *type) +{ + unsigned i; + for (i = 0; i < ARRAY_SIZE(mux_types); i++) + if (!strcmp(mux_types[i].str, str)) { + *type = mux_types[i].type; + return 0; + } + return -EINVAL; +} + +static int get_node_info(struct device_node *np, struct pingroup *group) +{ + /* Extract info from the pinctrl node: + * - the label; + * - the number of pins; + * - the address and size of the registers; + * - the mux type. + */ + int ret; + u32 ngpio; + u32 reg[2]; + const char *label; + const char *tango_mux; + + ret = of_property_read_u32(np, "ngpios", &ngpio); + if (ret) + return ret; + ret = of_property_read_u32_array(np, "reg", reg, 2); + if (ret) + return ret; + ret = of_property_read_string(np, "gpio-ranges-group-names", &label); + if (ret) + return ret; + ret = of_property_read_string(np, "tango,mux", &tango_mux); + if (ret) + return ret; + + ret = get_mux_type_from_str(tango_mux, &group->mux_type); + if (ret) + return ret; + group->label = label; + group->ngpio = ngpio; + group->pa = reg[0]; + group->sz = reg[1]; + + return 0; +} + +static int register_group_gpios(struct device *dev, struct pingroup *group) +{ + struct gpio_chip *gchip; + + gchip = devm_kzalloc(dev, sizeof(*gchip), GFP_KERNEL); + if (!gchip) + return -ENOMEM; + + gchip->label = group->label; + gchip->parent = dev; + gchip->owner = THIS_MODULE; + gchip->get_direction = gpio_get_direction; + gchip->direction_input = gpio_direction_input; + gchip->direction_output = gpio_direction_output; + gchip->get = gpio_get; + gchip->set = gpio_set; + gchip->set_multiple = gpio_set_multiple; + gchip->base = group->base; + gchip->ngpio = group->ngpio; + + return devm_gpiochip_add_data(dev, gchip, group); +} + +static int register_group_pins(struct device *dev, struct pingroup *group) +{ + unsigned i; + struct pinctrl_desc *pdesc; + struct pinctrl_pin_desc *ppdesc; + struct pinctrl_dev *pindev; + + pdesc = devm_kzalloc(dev, sizeof(*pdesc), GFP_KERNEL); + if (!pdesc) + return -ENOMEM; + ppdesc = devm_kzalloc(dev, group->ngpio * sizeof(*ppdesc), GFP_KERNEL); + if (!ppdesc) + return -ENOMEM; + + for (i = 0; i < group->ngpio; i++) + ppdesc[i].number = group->base + i; + + pdesc->name = group->label; + pdesc->pins = ppdesc; + pdesc->npins = group->ngpio; + pdesc->pctlops = &pctlops; + pdesc->pmxops = &pmxops; + pdesc->owner = THIS_MODULE; + + pindev = devm_pinctrl_register(dev, pdesc, group); + if (IS_ERR(pindev)) + return PTR_ERR(pindev); + + return 0; +} + +static int register_group(struct platform_device *pdev, struct device_node *np) +{ + int ret; + struct pingroup *group; + struct pinctrl_data *pdata; + + group = devm_kzalloc(&pdev->dev, sizeof(*group), GFP_KERNEL); + if (!group) + return -ENOMEM; + + /* Extract info from the device node. */ + ret = get_node_info(np, group); + if (ret) + return ret; + + /* Get the base number of this group by looking at the previous one. */ + pdata = platform_get_drvdata(pdev); + if (!pdata) + return -EINVAL; + if (list_empty(&pdata->group_list)) { + group->base = 0; + } else { + struct pingroup *prev; + prev = list_last_entry(&pdata->group_list, typeof(*prev), list); + group->base = prev->base + prev->ngpio; + } + + /* Ioremap the group registers. */ + group->va = devm_ioremap_nocache(&pdev->dev, group->pa, group->sz); + if (!group->va) + return -ENOMEM; + + /* Register the group pins. */ + ret = register_group_pins(&pdev->dev, group); + if (ret) + return ret; + + /* Register the group GPIOs. */ + ret = register_group_gpios(&pdev->dev, group); + if (ret) + return ret; + + /* Add this new group to the list. */ + list_add_tail(&group->list, &pdata->group_list); + + dev_dbg(&pdev->dev, "%-8s base=%-2d ngpio=%-2d OK\n", + group->label, group->base, group->ngpio); + return 0; +} + +static int tango_pinctrl_probe(struct platform_device *pdev) +{ + unsigned i = 0; + unsigned groups_num; + struct pinctrl_data *pdata; + struct device_node *np = pdev->dev.of_node; + + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return -ENOMEM; + INIT_LIST_HEAD(&pdata->group_list); + platform_set_drvdata(pdev, pdata); + + groups_num = of_count_phandle_with_args(np, "groups", NULL); + if (!groups_num) + return -EINVAL; + + for (i = 0; i < groups_num; i++) { + int ret; + struct of_phandle_args args; + ret = of_parse_phandle_with_args(np, "groups", NULL, i, &args); + if (ret) + return ret; + ret = register_group(pdev, args.np); + if (ret) + return ret; + } + + return 0; +} + +static const struct of_device_id tango_pinctrl_of_match[] = { + { .compatible = "sigma,smp8758-pinctrl", }, + {}, +}; +MODULE_DEVICE_TABLE(of, tango_pinctrl_of_match); + +static struct platform_driver tango_pinctrl_driver = { + .driver = { + .name = "pinctrl-tango", + .of_match_table = of_match_ptr(tango_pinctrl_of_match), + }, + .probe = tango_pinctrl_probe, +}; + +static int __init tango_pinctrl_init(void) +{ + return platform_driver_register(&tango_pinctrl_driver); +} +module_init(tango_pinctrl_init); + +static void __exit tango_pinctrl_exit(void) +{ + platform_driver_unregister(&tango_pinctrl_driver); +} +module_exit(tango_pinctrl_exit); + +MODULE_AUTHOR("Sigma Designs"); +MODULE_DESCRIPTION("Tango pinctrl driver"); +MODULE_LICENSE("GPL"); -- 2.10.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-05-31 16:36 ` Yves Lefloch @ 2017-06-09 8:14 ` Linus Walleij 2017-06-14 13:51 ` Yves Lefloch 0 siblings, 1 reply; 17+ messages in thread From: Linus Walleij @ 2017-06-09 8:14 UTC (permalink / raw) To: Yves Lefloch; +Cc: Thibaud Cornic, linux-gpio, Alexandre Courbot, Mason On Wed, May 31, 2017 at 6:36 PM, Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> wrote: > I tried to carry on and add the other IPs' pins, and I've got a question > regarding pin requesting. > See, adding the smartcard's pins was easy because we don't have a driver for > it yet, so the pins are just straightforward GPIOs, which the user-space can > request using /sys/class/gpio/export. Please do not use sysfs for userspace tests, familiarize yourself with tools/gpio in the kernel and use these for testing using the ioctl()s. > But that's not the case for the ethernet device, whose pins are already in > use, although not requested explicitly by its driver > (drivers/net/ethernet/aurora/nb8800.c). Which means user-space can also > request/set them within the sysfs, and that is wrong. It is common that a device allows simultaneous use of a pin for GPIO and a certain device. (E.g. so that GPIO can "spy" on the pin or similar.) If on a certain system, this is not allowed, one shall set .strict = true on the struct pinmux_ops, see include/linux/pinctrl/pinmux.h > - Is there a mechanism in the device tree to automatically request some pins > for a driver once it gets loaded? I have looked at the `stateN_nodeM' thing > in the documentation, but that doesn't seem to do the job without changes in > the ethernet driver's code. Yes those are called pin control hogs. Then the pins get associated with the pin controller itself. Try first to associate the pins with the device actually using them if possible. > - Should the pins be declared at all if we know they are not going to be used > for anything else than, for instance, ethernet? What about some removable > device then, for instance UART as Thibaud pointed out previously? I guess it depends on how much control of things you need in your system. Also you might not need it now, but later on it turns out that in order to properly deal with things like suspend/resume, you all of a sudden need to switch pin control states when going to sleep and then it is nice if the driver can handle these pins. When in doubt, implement a driver for everything you have hardware registers for, anything that is software controlled, I guess. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Right amount of info in the DT 2017-06-09 8:14 ` Linus Walleij @ 2017-06-14 13:51 ` Yves Lefloch 0 siblings, 0 replies; 17+ messages in thread From: Yves Lefloch @ 2017-06-14 13:51 UTC (permalink / raw) To: Linus Walleij; +Cc: Thibaud Cornic, linux-gpio, Alexandre Courbot, Mason On 06/09/2017 10:14 AM, Linus Walleij wrote: > Please do not use sysfs for userspace tests, familiarize yourself with > tools/gpio in the kernel and use these for testing using the ioctl()s. OK I will. What is the reasoning behind using ioctl()s instead of sysfs? >> But that's not the case for the ethernet device, whose pins are already in >> use, although not requested explicitly by its driver >> (drivers/net/ethernet/aurora/nb8800.c). Which means user-space can also >> request/set them within the sysfs, and that is wrong. > > It is common that a device allows simultaneous use of a pin for > GPIO and a certain device. (E.g. so that GPIO can "spy" on the pin > or similar.) In my case, this is not allowed, writing the registers won't do anything. > If on a certain system, this is not allowed, one shall set > .strict = true on the struct pinmux_ops, see > include/linux/pinctrl/pinmux.h Yes I was already using this property, but it didn't give me the exclusivity. I think that this was because I never made the call to pinctrl_request_gpio(). Looking at "drivers/pinctrl/meson/pinctrl-meson.c", I tried to add a call to that function in the gpiochip.request() op. And then something strange happened: I'm only able to export to user-space the GPIOs from the `gpio_sys' group, but not from the `gpio_smcard' group (see DT below). Here's how I'm seeing that (yes I know I should switch to tools/gpio!): # ls /sys/class/gpio export gpiochip0 gpiochip71 unexport # echo 0 > /sys/class/gpio/export [ 543.733511] pinctrl-tango pinctrl: request pin 0 (PIN0) for sys:0 [ 543.739696] device: 'gpio0': device_add # echo 71 > /sys/class/gpio/export [ 626.460176] pinctrl-tango pinctrl: pin 71 is not registered so it cannot be requested [ 626.468092] pinctrl-tango pinctrl: pin-71 (smcard:71) status -22 [ 626.474148] gpio-71 (?): gpiod_request: status -22 [ 626.479121] export_store: status -22 bash: echo: write error: Invalid argument Eventually, I managed to find the root cause of this issue: For every "group" in the pinctrl node of my DT, there is a different pinctrl_dev, because I make for every one of them a call to `devm_pinctrl_register'. When the gpiolib parses the gpio-controller nodes of the DT and creates the GPIO ranges with gpiochip_add_pin_range(), it tries to retrieve a pinctrl_dev with of_pinctrl_get(), and attaches the GPIO range to it. But the thing is, of_pinctrl_get() just returns the first pinctrl_dev that corresponds to the pinctrl node, and in my case, that's always the "sys" one. So instead of having: - pinctrl: - pinctrl_dev: sys - gpio_range: sys - pinctrl_dev: smcard - gpio_range: smcard I have: - pinctrl: - pinctrl_dev: sys - gpio_range: sys - gpio_range: smcard - pinctrl_dev: smcard However, pinctrl_request_gpio() which, as I said, is now called by my code, calls pinctrl_get_device_gpio_range() and therefore does check the GPIO range of the pinctrl_dev before returning it. When exporting GPIO-0, the "sys" GPIO range does exist in the "sys" pinctrl_dev, so that works. But when exporting GPIO-71, the "smcard" GPIO range does not for the "smcard" pinctrl_dev, hence the error. I think of_pinctrl_get() should check the pin range as well, do you concur? Best regards, Yves. >From 62cfce78da5bc3d4a8d8870bad8396747f7b5806 Mon Sep 17 00:00:00 2001 From: Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> Date: Tue, 25 Apr 2017 14:06:46 +0200 Subject: [PATCH] drivers: pinctrl: Add a pinctrl for mach-tango Change-Id: Id5b5872b2a7d116829ddc75577f26115eb6e1c98 Signed-off-by: Yves Lefloch <YvesMarie_Lefloch@sigmadesigns.com> --- arch/arm/boot/dts/tango4-common.dtsi | 31 ++ arch/arm/configs/tango4_defconfig | 2 + arch/arm/mach-tango/Kconfig | 1 + drivers/pinctrl/Kconfig | 7 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/pinctrl-tango.c | 559 +++++++++++++++++++++++++++++++++++ 6 files changed, 601 insertions(+) create mode 100644 drivers/pinctrl/pinctrl-tango.c diff --git a/arch/arm/boot/dts/tango4-common.dtsi b/arch/arm/boot/dts/tango4-common.dtsi index a3e81ab..7cf698a 100644 --- a/arch/arm/boot/dts/tango4-common.dtsi +++ b/arch/arm/boot/dts/tango4-common.dtsi @@ -18,6 +18,37 @@ #address-cells = <1>; #size-cells = <1>; + pinctrl: pinctrl { + compatible = "sigma,smp8758-pinctrl"; + groups = <&gpio_sys>, <&gpio_smcard>; + }; + + gpio_sys: gpio@10500 { + reg = <0x10500 0x8>; + tango,mux = "none"; + tango,label = "sys"; + gpio-controller; + #gpio-cells = <1>; + gpio-ranges = <&pinctrl 0 0 16>; + }; + + smcard: smcard@6c300 { + /* Driver in the works. */ + reg = <0x6c300 0x6c>; + #address-cells = <1>; + #size-cells = <1>; + + gpio_smcard: gpio@6c358 { + reg = <0x6c358 0xc>; + ngpios = <8>; + tango,mux = "pin"; + tango,label = "smcard"; + gpio-controller; + #gpio-cells = <1>; + gpio-ranges = <&pinctrl 0 71 8>; + }; + }; + periph_clk: periph_clk { compatible = "fixed-factor-clock"; clocks = <&clkgen CPU_CLK>; diff --git a/arch/arm/configs/tango4_defconfig b/arch/arm/configs/tango4_defconfig index b26bb4e..b58b01b 100644 --- a/arch/arm/configs/tango4_defconfig +++ b/arch/arm/configs/tango4_defconfig @@ -68,7 +68,9 @@ CONFIG_SERIAL_OF_PLATFORM=y # CONFIG_HW_RANDOM is not set CONFIG_I2C=y CONFIG_I2C_XLR=y +CONFIG_PINCTRL_TANGO=y CONFIG_GPIOLIB=y +CONFIG_GPIO_SYSFS=y CONFIG_THERMAL=y CONFIG_CPU_THERMAL=y CONFIG_TANGO_THERMAL=y diff --git a/arch/arm/mach-tango/Kconfig b/arch/arm/mach-tango/Kconfig index ebe15b9..479c9aa 100644 --- a/arch/arm/mach-tango/Kconfig +++ b/arch/arm/mach-tango/Kconfig @@ -11,3 +11,4 @@ config ARCH_TANGO select HAVE_ARM_SCU select HAVE_ARM_TWD select TANGO_IRQ + select PINCTRL diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index 0e75d94..5648e76 100644 --- a/drivers/pinctrl/Kconfig +++ b/drivers/pinctrl/Kconfig @@ -179,6 +179,13 @@ config PINCTRL_ST select PINCONF select GPIOLIB_IRQCHIP +config PINCTRL_TANGO + tristate "Tango pin control driver" + depends on OF + select PINMUX + help + Say yes to activate the pinctrl/gpio driver for Tango chips. + config PINCTRL_TZ1090 bool "Toumaz Xenif TZ1090 pin control driver" depends on SOC_TZ1090 diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile index 11bad37..d5563e49 100644 --- a/drivers/pinctrl/Makefile +++ b/drivers/pinctrl/Makefile @@ -26,6 +26,7 @@ obj-$(CONFIG_PINCTRL_ROCKCHIP) += pinctrl-rockchip.o obj-$(CONFIG_PINCTRL_SINGLE) += pinctrl-single.o obj-$(CONFIG_PINCTRL_SIRF) += sirf/ obj-$(CONFIG_ARCH_TEGRA) += tegra/ +obj-$(CONFIG_PINCTRL_TANGO) += pinctrl-tango.o obj-$(CONFIG_PINCTRL_TZ1090) += pinctrl-tz1090.o obj-$(CONFIG_PINCTRL_TZ1090_PDC) += pinctrl-tz1090-pdc.o obj-$(CONFIG_PINCTRL_U300) += pinctrl-u300.o diff --git a/drivers/pinctrl/pinctrl-tango.c b/drivers/pinctrl/pinctrl-tango.c new file mode 100644 index 0000000..3170d8f --- /dev/null +++ b/drivers/pinctrl/pinctrl-tango.c @@ -0,0 +1,559 @@ +/* + * Copyright (C) 2017 Sigma Designs + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + */ +#include <linux/bitops.h> +#include <linux/gpio/driver.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/pinctrl/pinctrl.h> +#include <linux/pinctrl/pinmux.h> +#include <linux/platform_device.h> + +/* Pin regs are laid out this way: + * ______________________________ + * | bitwise mask | bitwise val | + * 32 16 0 + * + * To set bit n, one must write (1 << (16+n) | n) to the reg. + * Consequently, one reg can't handle more than 16 pins, so n < 16. + * + * There are 3 kind of registers that follow this layout: value registers, + * direction registers, and gpio-mode registers. + * The latter kind allows muxing between the primary function (eg UART), which + * is the pin's direction and value entirely controlled by hardware, and the + * "GPIO mode", which is the pin's direction and value entirely controlled by + * software. This gpio-mode register only exists for some GPIO groups. + * + * Additionally, for some pin groups, there is no gpio-mode register that muxes + * with the granularity of a pin, but rather with the granularity of the whole + * group. In that case, such mux register does *not* follow the layout above. + * + * Finally, it is possible for some groups to have neither type of muxing. In + * that case, the pins are dedicated GPIOs. + * + * Registers are organized this way for each group: + * group_mux? (direction value pin_mux?)+ + */ + +enum mux_type { + MUX_NONE, + MUX_PIN, + MUX_GROUP, +}; + +/* Which magic values to write in the mux group registers. */ +#define MUX_GROUP_GPIO 0x6 +#define MUX_GROUP_ALTF 0x1 + +/* Conversion table for char* <-> enum mux_type */ +static const struct { + const char *str; + enum mux_type type; +} mux_types[] = { + { .str = "none", .type = MUX_NONE, }, + { .str = "pin", .type = MUX_PIN, }, + { .str = "group", .type = MUX_GROUP, }, +}; + +/* Pinctrl device data */ +struct pingroup { + struct list_head list; + struct device_node *np; + const char *label; + unsigned base; + unsigned ngpio; + size_t sz; + phys_addr_t pa; + void __iomem *va; + enum mux_type mux_type; +}; + +/* Platform device data */ +struct pinctrl_data { + struct list_head group_list; +}; + +#define __reg_offset_from_pin(pg, pin) ( \ + ((pg)->mux_type == MUX_GROUP ? sizeof(u32) : 0) + \ + ((pg)->mux_type == MUX_PIN ? 3 : 2) * sizeof(u32) * ((pin)/16) \ + ) +#define group_mux_reg_va(pg) ((pg)->va) +#define dir_reg_va(pg, pin) (group_mux_reg_va(pg) + __reg_offset_from_pin(pg, pin)) +#define val_reg_va(pg, pin) (dir_reg_va(pg, pin) + sizeof(u32)) +#define pin_mux_reg_va(pg, pin) (val_reg_va(pg, pin) + sizeof(u32)) + +static int pinreg_get_bit(void __iomem *addr, unsigned offset) +{ + unsigned long val = readl(addr); + + /* TODO remove me later */ +#if IS_ENABLED(CONFIG_DEBUG) + BUG_ON(val & GENMASK(31, 16)); +#endif + + return test_bit(offset, &val); +} + +static void pinreg_set_bit(void __iomem *addr, unsigned offset, int value) +{ + unsigned long val = 0; + + /* Enable change by writing upper half-word too. */ + set_bit(offset + 16, &val); + if (value) + set_bit(offset, &val); + + writel(val, addr); + + /* TODO remove me later */ +#if IS_ENABLED(CONFIG_DEBUG) + BUG_ON(pinreg_get_bit(addr, offset) != value); +#endif +} + +static void pinreg_set_multiple(void __iomem *addr, u16 mask, u16 bits) +{ + unsigned long val = (mask << 16) | bits; + + writel(val, addr); +} + +static int gpio_request(struct gpio_chip *gc, unsigned offset) +{ + return pinctrl_request_gpio(gc->base + offset); +} + +static void gpio_free(struct gpio_chip *gc, unsigned offset) +{ + pinctrl_free_gpio(gc->base + offset); +} + +static int gpio_get(struct gpio_chip *gc, unsigned offset) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + return pinreg_get_bit(val_reg_va(group, offset), offset % 16); +} + +static void gpio_set(struct gpio_chip *gc, unsigned offset, int value) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return; + + pinreg_set_bit(val_reg_va(group, offset), offset % 16, value); +} + +static int gpio_get_direction(struct gpio_chip *gc, unsigned offset) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + /* Bit set means output. */ + return !pinreg_get_bit(dir_reg_va(group, offset), offset % 16); +} + +static int gpio_direction_input(struct gpio_chip *gc, unsigned offset) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + /* Bit set means output. */ + pinreg_set_bit(dir_reg_va(group, offset), offset % 16, 0); + + return 0; +} + +static int gpio_direction_output(struct gpio_chip *gc, unsigned offset, int value) +{ + struct pingroup *group = gpiochip_get_data(gc); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + /* The output value can be changed before the direction is. */ + pinreg_set_bit(val_reg_va(group, offset), offset % 16, value); + + /* Bit set means output. */ + pinreg_set_bit(dir_reg_va(group, offset), offset % 16, 1); + + return 0; +} + +static void gpio_set_multiple(struct gpio_chip *gc, + unsigned long *mask, unsigned long *bits) +{ + unsigned i, j; + unsigned num_of_ulongs; + struct pingroup *group = gpiochip_get_data(gc); + + if (!group) + return; + + num_of_ulongs = (group->ngpio / 32) + 1; + for (i = 0; i < num_of_ulongs; i++) + for (j = 0; j < 1; j++) { + u16 _mask = (u16)(*(mask + i) >> (16 * j)); + u16 _bits = (u16)(*(bits + i) >> (16 * j)); + pinreg_set_multiple(val_reg_va(group, 2*i + j), _mask, _bits); + } +} + +static const unsigned int all_pins[] = { + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, + 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, + 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, + 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, + 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, + 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, + 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, + 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, + 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, + 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, +}; + +static int get_group_count(struct pinctrl_dev *pctldev) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group) + return -EINVAL; + + return group->mux_type == MUX_NONE ? 0 : 1; +} + +static const char *get_group_name(struct pinctrl_dev *pctldev, unsigned selector) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group) + return ERR_PTR(-EINVAL); + + return group->label; +} + +static int get_group_pins(struct pinctrl_dev *pctldev, unsigned selector, + const unsigned **pins, unsigned *num_pins) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group) + return -EINVAL; + + *pins = all_pins + group->base; + *num_pins = group->ngpio; + + return 0; +} + +static const struct pinctrl_ops pctlops = { + .get_groups_count = get_group_count, + .get_group_name = get_group_name, + .get_group_pins = get_group_pins, +}; + +int get_function_groups(struct pinctrl_dev *pctldev, unsigned selector, + const char * const **groups, unsigned *num_groups) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group) + return -EINVAL; + + if (group->mux_type == MUX_NONE) { + *groups = NULL; + *num_groups = 0; + } else { + *groups = &group->label; + *num_groups = 1; + } + + return 0; +} + +int set_mux(struct pinctrl_dev *pctldev, unsigned func_selector, + unsigned group_selector) +{ + /* With only one possible alt function, muxing is only + * necessary for GPIO request/free. + */ + return 0; +} + +int gpio_request_enable(struct pinctrl_dev *pctldev, + struct pinctrl_gpio_range *range, unsigned offset) +{ + struct pingroup *group = pinctrl_dev_get_drvdata(pctldev); + + if (!group || offset >= group->ngpio) + return -EINVAL; + + switch (group->mux_type) { + case MUX_NONE: + break; + case MUX_PIN: + pinreg_set_bit(pin_mux_reg_va(group, offset), offset % 16, 1); + break; + case MUX_GROUP: + writel(MUX_GROUP_GPIO, group_mux_reg_va(group)); + break; + default: + return -EINVAL; + } + + return 0; +} + +void gpio_disable_free(struct pinctrl_dev *pctldev, + struct pinctrl_gpio_range *range, unsigned offset) +{ + struct pingroup *group; + + group = pinctrl_dev_get_drvdata(pctldev); + + if (!group || offset >= group->ngpio) + return; + + switch (group->mux_type) { + case MUX_NONE: + break; + case MUX_PIN: + pinreg_set_bit(pin_mux_reg_va(group, offset), offset % 16, 0); + break; + case MUX_GROUP: + writel(MUX_GROUP_ALTF, group_mux_reg_va(group)); + break; + } +} + +const static struct pinmux_ops pmxops = { + .get_functions_count = get_group_count, + .get_function_name = get_group_name, + .get_function_groups = get_function_groups, + .set_mux = set_mux, + .gpio_request_enable = gpio_request_enable, + .gpio_disable_free = gpio_disable_free, + .strict = true, +}; + +static int get_mux_type_from_str(const char *str, enum mux_type *type) +{ + unsigned i; + for (i = 0; i < ARRAY_SIZE(mux_types); i++) + if (!strcmp(mux_types[i].str, str)) { + *type = mux_types[i].type; + return 0; + } + return -EINVAL; +} + +static int get_node_info(struct device_node *np, struct pingroup *group) +{ + /* Extract info from the pinctrl node: + * - the label; + * - the number of pins; + * - the address and size of the registers; + * - the mux type. + */ + int ret; + u32 reg[2]; + const char *label; + const char *tango_mux; + struct of_phandle_args ranges; + + ret = of_property_read_string(np, "tango,mux", &tango_mux); + if (ret) + return ret; + ret = of_property_read_string(np, "tango,label", &label); + if (ret) + return ret; + ret = of_property_read_u32_array(np, "reg", reg, 2); + if (ret) + return ret; + ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", + 3, 0, &ranges); + if (ret) + return ret; + + ret = get_mux_type_from_str(tango_mux, &group->mux_type); + if (ret) + return ret; + group->label = label; + group->pa = reg[0]; + group->sz = reg[1]; + group->base = ranges.args[1]; + group->ngpio = ranges.args[2]; + + group->np = np; + + return 0; +} + +static int register_group_gpios(struct device *dev, struct pingroup *group) +{ + struct gpio_chip *gchip; + + gchip = devm_kzalloc(dev, sizeof(*gchip), GFP_KERNEL); + if (!gchip) + return -ENOMEM; + + gchip->label = group->label; + gchip->parent = dev; + gchip->owner = THIS_MODULE; + gchip->request = gpio_request; + gchip->free = gpio_free; + gchip->get_direction = gpio_get_direction; + gchip->direction_input = gpio_direction_input; + gchip->direction_output = gpio_direction_output; + gchip->get = gpio_get; + gchip->set = gpio_set; + gchip->set_multiple = gpio_set_multiple; + gchip->base = group->base; + gchip->ngpio = group->ngpio; + gchip->of_node = group->np; + + return devm_gpiochip_add_data(dev, gchip, group); +} + +static int register_group_pins(struct device *dev, struct pingroup *group) +{ + unsigned i; + struct pinctrl_desc *pdesc; + struct pinctrl_pin_desc *ppdesc; + struct pinctrl_dev *pindev; + + pdesc = devm_kzalloc(dev, sizeof(*pdesc), GFP_KERNEL); + if (!pdesc) + return -ENOMEM; + ppdesc = devm_kzalloc(dev, group->ngpio * sizeof(*ppdesc), GFP_KERNEL); + if (!ppdesc) + return -ENOMEM; + + for (i = 0; i < group->ngpio; i++) + ppdesc[i].number = group->base + i; + + pdesc->name = group->label; + pdesc->pins = ppdesc; + pdesc->npins = group->ngpio; + pdesc->pctlops = &pctlops; + pdesc->pmxops = &pmxops; + pdesc->owner = THIS_MODULE; + + pindev = devm_pinctrl_register(dev, pdesc, group); + if (IS_ERR(pindev)) + return PTR_ERR(pindev); + + return 0; +} + +static struct pingroup *create_group(struct device *dev, struct device_node *np) +{ + int ret; + struct pingroup *group; + + group = devm_kzalloc(dev, sizeof(*group), GFP_KERNEL); + if (!group) + return ERR_PTR(-ENOMEM); + + /* Extract info from the device node. */ + ret = get_node_info(np, group); + if (ret) + return ERR_PTR(ret); + + /* Ioremap the group registers. */ + group->va = devm_ioremap_nocache(dev, group->pa, group->sz); + if (!group->va) + return ERR_PTR(-ENOMEM); + + /* Register the group pins. */ + ret = register_group_pins(dev, group); + if (ret) + return ERR_PTR(ret); + + /* Register the group GPIOs. */ + ret = register_group_gpios(dev, group); + if (ret) + return ERR_PTR(ret); + + dev_dbg(dev, "%-8s base=%-2d ngpio=%-2d OK\n", + group->label, group->base, group->ngpio); + return group; +} + +static int tango_pinctrl_probe(struct platform_device *pdev) +{ + unsigned i = 0; + unsigned groups_num; + struct pinctrl_data *pdata; + struct device_node *np = pdev->dev.of_node; + + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return -ENOMEM; + INIT_LIST_HEAD(&pdata->group_list); + platform_set_drvdata(pdev, pdata); + + groups_num = of_count_phandle_with_args(np, "groups", NULL); + if (!groups_num) + return -EINVAL; + + for (i = 0; i < groups_num; i++) { + int ret; + struct pingroup *group; + struct of_phandle_args args; + + ret = of_parse_phandle_with_args(np, "groups", NULL, i, &args); + if (ret) + return ret; + + group = create_group(&pdev->dev, args.np); + if (IS_ERR(group)) + return PTR_ERR(group); + else + list_add_tail(&group->list, &pdata->group_list); + } + + return 0; +} + +static const struct of_device_id tango_pinctrl_of_match[] = { + { .compatible = "sigma,smp8758-pinctrl", }, + {}, +}; +MODULE_DEVICE_TABLE(of, tango_pinctrl_of_match); + +static struct platform_driver tango_pinctrl_driver = { + .driver = { + .name = "pinctrl-tango", + .of_match_table = of_match_ptr(tango_pinctrl_of_match), + }, + .probe = tango_pinctrl_probe, +}; + +static int __init tango_pinctrl_init(void) +{ + return platform_driver_register(&tango_pinctrl_driver); +} +arch_initcall(tango_pinctrl_init); + +static void __exit tango_pinctrl_exit(void) +{ + platform_driver_unregister(&tango_pinctrl_driver); +} +module_exit(tango_pinctrl_exit); + +MODULE_AUTHOR("Sigma Designs"); +MODULE_DESCRIPTION("Tango pinctrl driver"); +MODULE_LICENSE("GPL"); -- 2.10.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-06-14 13:51 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-20 9:54 Right amount of info in the DT Yves Lefloch 2017-04-24 8:23 ` Linus Walleij 2017-04-24 13:03 ` Yves Lefloch 2017-04-24 16:45 ` Linus Walleij 2017-04-26 15:47 ` Yves Lefloch 2017-05-07 10:26 ` Linus Walleij 2017-05-09 10:49 ` Yves Lefloch 2017-05-11 15:22 ` Linus Walleij 2017-05-12 15:31 ` Yves Lefloch 2017-05-22 15:12 ` Linus Walleij 2017-05-24 9:41 ` Thibaud Cornic 2017-05-29 10:02 ` Linus Walleij 2017-05-30 13:59 ` Yves Lefloch 2017-05-31 0:24 ` Linus Walleij 2017-05-31 16:36 ` Yves Lefloch 2017-06-09 8:14 ` Linus Walleij 2017-06-14 13:51 ` Yves Lefloch
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.