Message ID | 1386689216-5139-1-git-send-email-shc_work@mail.ru |
---|---|
State | Superseded, archived |
Headers | show |
On Tue, Dec 10, 2013 at 4:26 PM, Alexander Shiyan <shc_work@mail.ru> wrote: > SYSCON driver was designed for using memory areas (registers) > that are used in several subsystems. There are systems (CPUs) > which use bits in one register for various purposes and thus > should be handled by various kernel subsystems. This driver > allows you to use the individual SYSCON bits as GPIOs. > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru> (...) > --- > +Required properties: > +- compatible: Should be "gpio-syscon". > +- syscon: Phandle to syscon device node. > +- gpio-controller: Marks the device node as a gpio controller. > +- #gpio-cells: Should be two. The first cell is the pin number and > + the second cell is used to specify the gpio polarity: > + 0 = Active high, > + 1 = Active low. > +- bit-offset: Offset to the first bit used by driver. > +- bit-count: Number of bits used. > + > +Example: > + sysgpio: sysgpio { > + compatible = "gpio-syscon"; > + syscon = <&syscon>; > + gpio-controller; > + #gpio-cells = <2>; > + bit-offset = <4>; > + bit-count = <1>; I take it that bit-offset can be say 1024 and bit count 32 for a 32-bit register at syscon_base + 0x80? I have a real big problem with encoding register offsets into the device tree as I have stated before, it is starting to get real hard to debug this, like: "hmmm where did this go wrong?" "in the byte offset from the base stored in this other driver plus ... 1024? no wait, that is stored it bit notation so I need to divide by 8 ... hm which was it now ... and where do I have all these figures in my ICE debugger?" The code is not helpful anymore, you have to go in and reverse-engineer this and bring up a calculator to figure out what is going on. I sort of like the syscon driver but I don't like the idea of encoding register offsets into the device tree, even less do I like the idea of encoding it in bit notation. I prefer that you put a header with all the offsets into <linux/mfd/syscon-foo.h> and define all the offsets there, then to reuse this driver with several GPIO controllers all doing this GPIO-in-syscon business, to #include <linux/mfd/syscon-bar.h> etc and use the compatible string to figure out which file to pick the offsets from. I know I may be alone in thinking this so if other smarter people think this register-offset-in-DT is a real good idea I guess I need to rest my case... anyway I'll need the ACK of the DT bindings maintainers to proceed and I'd even like two of them to vote me down on this if I shall apply this to the GPIO tree. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Dec 12, 2013 at 8:20 PM, Alexander Shiyan <shc_work@mail.ru> wrote: >> On Tue, Dec 10, 2013 at 4:26 PM, Alexander Shiyan <shc_work@mail.ru> wrote: >> I take it that bit-offset can be say 1024 and bit count >> 32 for a 32-bit register at syscon_base + 0x80? > > This can be represented in DT as (0x80 << 4). That's better of course but you still have to read both the device tree and the driver and correlate to figure out what's going on. >> I sort of like the syscon driver but I don't like the idea >> of encoding register offsets into the device tree, even >> less do I like the idea of encoding it in bit notation. > > I also have no idea how to present it differently. > Can try to specify the byte offset through the "reg" property? > The "bit-offset" will remain, but will point offset from the "reg" byte. This does not help. I don't want the offset inside the device tree at all. >> I prefer that you put a header with all the offsets >> into <linux/mfd/syscon-foo.h> and define all the >> offsets there, then to reuse this driver with several >> GPIO controllers all doing this GPIO-in-syscon >> business, to #include <linux/mfd/syscon-bar.h> >> etc and use the compatible string to figure out >> which file to pick the offsets from. > > No headers is needed. Driver will be used in DTS only. I am claiming it is needed because I don't want it to be inside the DTS, I want it to be in the kernel. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> >> On Tue, Dec 10, 2013 at 4:26 PM, Alexander Shiyan <shc_work@mail.ru> wrote: > > >> I take it that bit-offset can be say 1024 and bit count > >> 32 for a 32-bit register at syscon_base + 0x80? > > > > This can be represented in DT as (0x80 << 4). > > That's better of course but you still have to read both the > device tree and the driver and correlate to figure out > what's going on. > > >> I sort of like the syscon driver but I don't like the idea > >> of encoding register offsets into the device tree, even > >> less do I like the idea of encoding it in bit notation. > > > > I also have no idea how to present it differently. > > Can try to specify the byte offset through the "reg" property? > > The "bit-offset" will remain, but will point offset from the "reg" byte. > > This does not help. I don't want the offset inside the > device tree at all. OK, let me show next version in a week or so. ---
diff --git a/Documentation/devicetree/bindings/gpio/gpio-syscon.txt b/Documentation/devicetree/bindings/gpio/gpio-syscon.txt new file mode 100644 index 0000000..d8ecfe2 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-syscon.txt @@ -0,0 +1,22 @@ +* SYSCON-based GPIO driver + +Required properties: +- compatible: Should be "gpio-syscon". +- syscon: Phandle to syscon device node. +- gpio-controller: Marks the device node as a gpio controller. +- #gpio-cells: Should be two. The first cell is the pin number and + the second cell is used to specify the gpio polarity: + 0 = Active high, + 1 = Active low. +- bit-offset: Offset to the first bit used by driver. +- bit-count: Number of bits used. + +Example: + sysgpio: sysgpio { + compatible = "gpio-syscon"; + syscon = <&syscon>; + gpio-controller; + #gpio-cells = <2>; + bit-offset = <4>; + bit-count = <1>; + }; diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 365ea61..5eee6e7 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -259,6 +259,12 @@ config GPIO_STA2X11 Say yes here to support the STA2x11/ConneXt GPIO device. The GPIO module has 128 GPIO pins with alternate functions. +config GPIO_SYSCON + tristate "GPIO based on SYSCON" + depends on MFD_SYSCON && OF + help + Say yes here to support GPIO functionality though SYSCON driver. + config GPIO_TS5500 tristate "TS-5500 DIO blocks and compatibles" help diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 95d7359..130bc76 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -75,6 +75,7 @@ obj-$(CONFIG_GPIO_STA2X11) += gpio-sta2x11.o obj-$(CONFIG_GPIO_STMPE) += gpio-stmpe.o obj-$(CONFIG_GPIO_STP_XWAY) += gpio-stp-xway.o obj-$(CONFIG_GPIO_SX150X) += gpio-sx150x.o +obj-$(CONFIG_GPIO_SYSCON) += gpio-syscon.o obj-$(CONFIG_GPIO_TB10X) += gpio-tb10x.o obj-$(CONFIG_GPIO_TC3589X) += gpio-tc3589x.o obj-$(CONFIG_ARCH_TEGRA) += gpio-tegra.o diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c new file mode 100644 index 0000000..2d02cc1 --- /dev/null +++ b/drivers/gpio/gpio-syscon.c @@ -0,0 +1,159 @@ +/* + * SYSCON GPIO driver + * + * Copyright (C) 2013 Alexander Shiyan <shc_work@mail.ru> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include <linux/err.h> +#include <linux/gpio.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/mfd/syscon.h> + +#define GPIO_SYSCON_INPUT (1 << 0) +#define GPIO_SYSCON_OUTPUT (1 << 1) + +/* SYSCON driver is designed to use 32-bit wide registers */ +#define SYSCON_REG_SIZE (4) +#define SYSCON_REG_BITS (SYSCON_REG_SIZE * 8) + +struct syscon_gpio_priv { + struct gpio_chip chip; + struct regmap *syscon; + unsigned int bit_offset; +}; + +static inline struct syscon_gpio_priv *to_syscon_gpio(struct gpio_chip *chip) +{ + return container_of(chip, struct syscon_gpio_priv, chip); +} + +static int syscon_gpio_get(struct gpio_chip *chip, unsigned offset) +{ + struct syscon_gpio_priv *priv = to_syscon_gpio(chip); + unsigned int val, offs = priv->bit_offset + offset; + int ret; + + ret = regmap_read(priv->syscon, + (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE, &val); + if (ret < 0) + return ret; + + return !!(val & (offs % SYSCON_REG_BITS)); +} + +static void syscon_gpio_set(struct gpio_chip *chip, unsigned offset, int val) +{ + struct syscon_gpio_priv *priv = to_syscon_gpio(chip); + unsigned int offs = priv->bit_offset + offset; + + regmap_update_bits(priv->syscon, + (offs / SYSCON_REG_BITS) * SYSCON_REG_SIZE, + offs % SYSCON_REG_BITS, + val ? (offs % SYSCON_REG_BITS) : 0); +} + +static int syscon_gpio_dir_in(struct gpio_chip *chip, unsigned offset) +{ + return 0; +} + +static int syscon_gpio_dir_out(struct gpio_chip *chip, unsigned offset, int val) +{ + syscon_gpio_set(chip, offset, val); + + return 0; +} + +static const struct of_device_id syscon_gpio_ids[] = { + { + .compatible = "gpio-syscon-input", + .data = (void *)GPIO_SYSCON_INPUT, + }, + { + .compatible = "gpio-syscon-output", + .data = (void *)GPIO_SYSCON_OUTPUT, + }, + { } +}; +MODULE_DEVICE_TABLE(of, syscon_gpio_ids); + +static int syscon_gpio_probe(struct platform_device *pdev) +{ + const struct of_device_id *of_id = + of_match_device(syscon_gpio_ids, &pdev->dev); + struct device_node *np = pdev->dev.of_node; + struct syscon_gpio_priv *priv; + unsigned int flags; + u32 ngpio; + int ret; + + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->syscon = syscon_regmap_lookup_by_phandle(np, "syscon"); + if (IS_ERR(priv->syscon)) + return PTR_ERR(priv->syscon); + + ret = of_property_read_u32(np, "bit-offset", &priv->bit_offset); + if (ret) + return ret; + + ret = of_property_read_u32(np, "bit-count", &ngpio); + if (ret) + return ret; + + /* Sanity check */ + if (ngpio < 1 || ngpio > (u16)-1) + return -EINVAL; + + flags = (unsigned int)of_id->data; + + priv->chip.owner = THIS_MODULE; + priv->chip.label = dev_name(&pdev->dev); + priv->chip.base = -1; + priv->chip.ngpio = ngpio; + if (flags & GPIO_SYSCON_INPUT) { + priv->chip.get = syscon_gpio_get; + priv->chip.direction_input = syscon_gpio_dir_in; + } + if (flags & GPIO_SYSCON_OUTPUT) { + priv->chip.set = syscon_gpio_set; + priv->chip.direction_output = syscon_gpio_dir_out; + } + + platform_set_drvdata(pdev, priv); + + return gpiochip_add(&priv->chip); +} + +static int syscon_gpio_remove(struct platform_device *pdev) +{ + struct syscon_gpio_priv *priv = platform_get_drvdata(pdev); + + return gpiochip_remove(&priv->chip); +} + +static struct platform_driver syscon_gpio_driver = { + .driver = { + .name = "gpio-syscon", + .owner = THIS_MODULE, + .of_match_table = syscon_gpio_ids, + }, + .probe = syscon_gpio_probe, + .remove = syscon_gpio_remove, +}; +module_platform_driver(syscon_gpio_driver); + +MODULE_AUTHOR("Alexander Shiyan <shc_work@mail.ru>"); +MODULE_DESCRIPTION("SYSCON GPIO driver"); +MODULE_LICENSE("GPL");
SYSCON driver was designed for using memory areas (registers) that are used in several subsystems. There are systems (CPUs) which use bits in one register for various purposes and thus should be handled by various kernel subsystems. This driver allows you to use the individual SYSCON bits as GPIOs. Signed-off-by: Alexander Shiyan <shc_work@mail.ru> --- .../devicetree/bindings/gpio/gpio-syscon.txt | 22 +++ drivers/gpio/Kconfig | 6 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-syscon.c | 159 +++++++++++++++++++++ 4 files changed, 188 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-syscon.txt create mode 100644 drivers/gpio/gpio-syscon.c