diff mbox

gpio: Driver for SYSCON-based GPIOs

Message ID 1386689216-5139-1-git-send-email-shc_work@mail.ru
State Superseded, archived
Headers show

Commit Message

Alexander Shiyan Dec. 10, 2013, 3:26 p.m. UTC
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

Comments

Linus Walleij Dec. 12, 2013, 6:52 p.m. UTC | #1
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
Linus Walleij Dec. 12, 2013, 9:35 p.m. UTC | #2
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
Alexander Shiyan Dec. 13, 2013, 4:15 a.m. UTC | #3
> >> 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 mbox

Patch

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");