Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1284084 > unrolled thread

Add support for monitoring gpio switches

Started byMartyn Welch <martyn.welch@collabora.co.uk>
First post2015-12-04 18:40 +0100
Last post2015-12-04 18:40 +0100
Articles 13 — 6 participants

Back to article view | Back to linux.kernel


Contents

  Add support for monitoring gpio switches Martyn Welch <martyn.welch@collabora.co.uk> - 2015-12-04 18:40 +0100
    [PATCH 2/3] Add support for monitoring gpio switches Martyn Welch <martyn.welch@collabora.co.uk> - 2015-12-04 18:40 +0100
      [PATCH] fix noderef.cocci warnings kbuild test robot <lkp@intel.com> - 2015-12-04 19:20 +0100
      Re: [PATCH 2/3] Add support for monitoring gpio switches kbuild test robot <lkp@intel.com> - 2015-12-04 19:20 +0100
      Re: [PATCH 2/3] Add support for monitoring gpio switches Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-12-04 20:00 +0100
        Re: [PATCH 2/3] Add support for monitoring gpio switches Martyn Welch <martyn.welch@collabora.co.uk> - 2015-12-05 11:50 +0100
      Re: [PATCH 2/3] Add support for monitoring gpio switches Linus Walleij <linus.walleij@linaro.org> - 2015-12-11 10:10 +0100
    [PATCH 1/3] Device tree binding documentation for gpio-switch Martyn Welch <martyn.welch@collabora.co.uk> - 2015-12-04 18:40 +0100
      Re: [PATCH 1/3] Device tree binding documentation for gpio-switch Rob Herring <robh@kernel.org> - 2015-12-07 18:40 +0100
        Re: [PATCH 1/3] Device tree binding documentation for gpio-switch Martyn Welch <martyn.welch@collabora.co.uk> - 2015-12-07 22:20 +0100
      Re: [PATCH 1/3] Device tree binding documentation for gpio-switch Linus Walleij <linus.walleij@linaro.org> - 2015-12-11 13:50 +0100
        Re: [PATCH 1/3] Device tree binding documentation for gpio-switch Rob Herring <robh+dt@kernel.org> - 2015-12-11 15:10 +0100
    [PATCH 3/3] ARM: dts: Addition of binding for gpio switches on peach-pi Martyn Welch <martyn.welch@collabora.co.uk> - 2015-12-04 18:40 +0100

#1284084 — Add support for monitoring gpio switches

FromMartyn Welch <martyn.welch@collabora.co.uk>
Date2015-12-04 18:40 +0100
SubjectAdd support for monitoring gpio switches
Message-ID<qC2uT-5Kw-37@gated-at.bofh.it>
This driver was written to expose a read only interface to a number of
gpios on Chromebooks. These gpios are attached to signals which cause the
firmware on Chromebooks to enter alternative modes of operation and/or
control other device characteristics (such as write protection on flash
devices). It was originally posted as "Add support for monitoring Chrome
OS firmware signals". A request was made to make it more generic.

In addition this patch series provides the required bindings for this to
the peach-pi Chromebook.

This is a new binding, but the driver is based (now some what loosely) on
functionality in the kernel shipped on Chromebooks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1284092 — [PATCH 2/3] Add support for monitoring gpio switches

FromMartyn Welch <martyn.welch@collabora.co.uk>
Date2015-12-04 18:40 +0100
Subject[PATCH 2/3] Add support for monitoring gpio switches
Message-ID<qC2uU-5Kw-63@gated-at.bofh.it>
In reply to#1284084
Select Chromebooks have gpio attached to switches used to cause the
firmware to enter alternative modes of operation and/or control other
device characteristics (such as write protection on flash devices). This
patch adds a driver that exposes a read-only interface to allow these
signals to be read from user space.

This functionality has been generalised to provide support for any device
with device tree support which needs to identify a gpio as being used for a
specific task.

Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
 drivers/misc/Kconfig       |  11 ++++
 drivers/misc/Makefile      |   1 +
 drivers/misc/gpio-switch.c | 160 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 172 insertions(+)
 create mode 100644 drivers/misc/gpio-switch.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 22892c7..d24367c 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -525,6 +525,17 @@ config VEXPRESS_SYSCFG
 	  bus. System Configuration interface is one of the possible means
 	  of generating transactions on this bus.
 
+config GPIO_SWITCH
+	tristate "GPIO Switch driver"
+	depends on GPIO_SYSFS
+	---help---
+	 Some devices have gpio attached to dedicated switches, an example of
+	 this are chromebooks (where connection to some switches for predefined
+	 purposes are provided on the generic development card). This driver
+	 provides the ability to create consistently named sysfs entries to the
+	 functionally equivalent signals across a range of devices. This
+	 functionality currently requires a device which supports device tree.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 537d7f3..7a7e11a 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -56,3 +56,4 @@ obj-$(CONFIG_GENWQE)		+= genwqe/
 obj-$(CONFIG_ECHO)		+= echo/
 obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
+obj-$(CONFIG_GPIO_SWITCH)	+= gpio-switch.o
diff --git a/drivers/misc/gpio-switch.c b/drivers/misc/gpio-switch.c
new file mode 100644
index 0000000..1f381d6
--- /dev/null
+++ b/drivers/misc/gpio-switch.c
@@ -0,0 +1,160 @@
+/*
+ * Copyright (C) 2015 Collabora Ltd.
+ *
+ * based on vendor driver,
+ *
+ * Copyright (C) 2011 The Chromium OS Authors
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/bcd.h>
+#include <linux/gpio.h>
+#include <linux/notifier.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+struct gpio_switch_gpio_info {
+	int gpio;
+	const char *link;
+};
+
+static int dt_gpio_init(struct platform_device *pdev, struct device_node *child,
+			struct gpio_switch_gpio_info *gpio)
+{
+	int err;
+	enum of_gpio_flags of_flags;
+	unsigned long flags = GPIOF_DIR_IN | GPIOF_EXPORT;
+	const char *name;
+
+	err = of_property_read_string(child, "label", &name);
+	if (err)
+		return err;
+
+	gpio->gpio = of_get_named_gpio_flags(child, "gpios", 0, &of_flags);
+	if (!gpio_is_valid(gpio->gpio)) {
+		err = -EINVAL;
+		goto err_prop;
+	}
+
+	if (of_flags & OF_GPIO_ACTIVE_LOW)
+		flags |= GPIOF_ACTIVE_LOW;
+
+	if (!of_property_read_bool(child, "read-only"))
+		flags |= GPIOF_EXPORT_CHANGEABLE;
+
+	err = gpio_request_one(gpio->gpio, flags, name);
+	if (err)
+		goto err_prop;
+
+	err = gpio_export_link(&pdev->dev, name, gpio->gpio);
+	if (err)
+		goto err_gpio;
+
+	gpio->link = name;
+
+	return 0;
+
+err_gpio:
+	gpio_free(gpio->gpio);
+err_prop:
+	of_node_put(child);
+
+	return err;
+}
+
+static void gpio_switch_rem(struct device *dev,
+			    struct gpio_switch_gpio_info *gpio)
+{
+	sysfs_remove_link(&dev->kobj, gpio->link);
+
+	gpio_unexport(gpio->gpio);
+
+	gpio_free(gpio->gpio);
+}
+
+static int gpio_switch_probe(struct platform_device *pdev)
+{
+	struct gpio_switch_gpio_info *gpios;
+	struct device_node *child;
+	struct device_node *np = pdev->dev.of_node;
+	int ret;
+	int i;
+
+	i = of_get_child_count(np);
+	if (i < 1)
+		return i;
+
+	gpios = devm_kmalloc(&pdev->dev, sizeof(gpios) * i, GFP_KERNEL);
+	if (!gpios)
+		return -ENOMEM;
+
+	i = 0;
+
+	for_each_child_of_node(np, child) {
+		ret = dt_gpio_init(pdev, child, &gpios[i]);
+		if (ret) {
+			dev_err(&pdev->dev, "Failed to init child node %d.\n",
+				i);
+			goto err;
+		}
+
+		i++;
+	}
+
+	platform_set_drvdata(pdev, gpios);
+
+	return 0;
+
+err:
+	while (i > 0) {
+		i--;
+		gpio_switch_rem(&pdev->dev, &gpios[i]);
+	}
+
+	return ret;
+}
+
+static int gpio_switch_remove(struct platform_device *pdev)
+{
+	struct gpio_switch_gpio_info *gpios = platform_get_drvdata(pdev);
+	struct device_node *np = pdev->dev.of_node;
+	int i;
+
+	i = of_get_child_count(np);
+
+	while (i > 0) {
+		i--;
+		gpio_switch_rem(&pdev->dev, &gpios[i]);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id gpio_switch_of_match[] = {
+	{ .compatible = "gpio-switch" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, gpio_switch_of_match);
+
+static struct platform_driver gpio_switch_driver = {
+	.probe = gpio_switch_probe,
+	.remove = gpio_switch_remove,
+	.driver = {
+		.name = "gpio_switch",
+		.of_match_table = gpio_switch_of_match,
+	},
+};
+module_platform_driver(gpio_switch_driver);
+
+MODULE_LICENSE("GPL");
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1284135 — [PATCH] fix noderef.cocci warnings

Fromkbuild test robot <lkp@intel.com>
Date2015-12-04 19:20 +0100
Subject[PATCH] fix noderef.cocci warnings
Message-ID<qC37z-6ec-11@gated-at.bofh.it>
In reply to#1284092
drivers/misc/gpio-switch.c:98:34-40: ERROR: application of sizeof to pointer

 sizeof when applied to a pointer typed expression gives the size of
 the pointer

Generated by: scripts/coccinelle/misc/noderef.cocci

CC: Martyn Welch <martyn.welch@collabora.co.uk>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 gpio-switch.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/misc/gpio-switch.c
+++ b/drivers/misc/gpio-switch.c
@@ -95,7 +95,7 @@ static int gpio_switch_probe(struct plat
 	if (i < 1)
 		return i;
 
-	gpios = devm_kmalloc(&pdev->dev, sizeof(gpios) * i, GFP_KERNEL);
+	gpios = devm_kmalloc(&pdev->dev, sizeof(*gpios) * i, GFP_KERNEL);
 	if (!gpios)
 		return -ENOMEM;
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1284141 — Re: [PATCH 2/3] Add support for monitoring gpio switches

Fromkbuild test robot <lkp@intel.com>
Date2015-12-04 19:20 +0100
SubjectRe: [PATCH 2/3] Add support for monitoring gpio switches
Message-ID<qC37z-6ec-13@gated-at.bofh.it>
In reply to#1284092
Hi Martyn,

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on v4.4-rc3 next-20151203]

url:    https://github.com/0day-ci/linux/commits/Martyn-Welch/Device-tree-binding-documentation-for-gpio-switch/20151205-014105


coccinelle warnings: (new ones prefixed by >>)

>> drivers/misc/gpio-switch.c:98:34-40: ERROR: application of sizeof to pointer

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1284159 — Re: [PATCH 2/3] Add support for monitoring gpio switches

FromGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Date2015-12-04 20:00 +0100
SubjectRe: [PATCH 2/3] Add support for monitoring gpio switches
Message-ID<qC3Ki-6rY-5@gated-at.bofh.it>
In reply to#1284092
On Fri, Dec 04, 2015 at 05:31:14PM +0000, Martyn Welch wrote:
> Select Chromebooks have gpio attached to switches used to cause the
> firmware to enter alternative modes of operation and/or control other
> device characteristics (such as write protection on flash devices). This
> patch adds a driver that exposes a read-only interface to allow these
> signals to be read from user space.
> 
> This functionality has been generalised to provide support for any device
> with device tree support which needs to identify a gpio as being used for a
> specific task.
> 
> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
> ---
>  drivers/misc/Kconfig       |  11 ++++
>  drivers/misc/Makefile      |   1 +
>  drivers/misc/gpio-switch.c | 160 +++++++++++++++++++++++++++++++++++++++++++++

Why isn't this in drivers/gpio/ ?

why make it a misc driver?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1284521 — Re: [PATCH 2/3] Add support for monitoring gpio switches

FromMartyn Welch <martyn.welch@collabora.co.uk>
Date2015-12-05 11:50 +0100
SubjectRe: [PATCH 2/3] Add support for monitoring gpio switches
Message-ID<qCizE-8mb-11@gated-at.bofh.it>
In reply to#1284159

On 04/12/15 18:57, Greg Kroah-Hartman wrote:
> On Fri, Dec 04, 2015 at 05:31:14PM +0000, Martyn Welch wrote:
>> Select Chromebooks have gpio attached to switches used to cause the
>> firmware to enter alternative modes of operation and/or control other
>> device characteristics (such as write protection on flash devices). This
>> patch adds a driver that exposes a read-only interface to allow these
>> signals to be read from user space.
>>
>> This functionality has been generalised to provide support for any device
>> with device tree support which needs to identify a gpio as being used for a
>> specific task.
>>
>> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
>> ---
>>   drivers/misc/Kconfig       |  11 ++++
>>   drivers/misc/Makefile      |   1 +
>>   drivers/misc/gpio-switch.c | 160 +++++++++++++++++++++++++++++++++++++++++++++
>
> Why isn't this in drivers/gpio/ ?
>
> why make it a misc driver?
>

I thought all the drivers in /drivers/gpio were gpio drivers, rather 
than users of the gpio framework. Is that not the case?

Happy to move it if the consensus is that that's the correct place to 
put it.

Martyn

> thanks,
>
> greg k-h
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1289343 — Re: [PATCH 2/3] Add support for monitoring gpio switches

FromLinus Walleij <linus.walleij@linaro.org>
Date2015-12-11 10:10 +0100
SubjectRe: [PATCH 2/3] Add support for monitoring gpio switches
Message-ID<qErS9-29T-21@gated-at.bofh.it>
In reply to#1284092
On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch
<martyn.welch@collabora.co.uk> wrote:

> Select Chromebooks have gpio attached to switches used to cause the
> firmware to enter alternative modes of operation and/or control other
> device characteristics (such as write protection on flash devices). This
> patch adds a driver that exposes a read-only interface to allow these
> signals to be read from user space.
>
> This functionality has been generalised to provide support for any device
> with device tree support which needs to identify a gpio as being used for a
> specific task.
>
> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>

If you want to do this thing, also propose a device tree binding document
for "gpio-switch".

But first (from Documentation/gpio/drivers-on-gpio.txt):

- gpio-keys: drivers/input/keyboard/gpio_keys.c is used when your GPIO line
  can generate interrupts in response to a key press. Also supports debounce.

- gpio-keys-polled: drivers/input/keyboard/gpio_keys_polled.c is used when your
  GPIO line cannot generate interrupts, so it needs to be periodically polled
  by a timer.

- extcon-gpio: drivers/extcon/extcon-gpio.c is used when you need to read an
  external connector status, such as a headset line for an audio driver or an
  HDMI connector. It will provide a better userspace sysfs interface than GPIO.

So you mean none of these apply for this case?

Second: what you want to do is export a number of GPIOs with certain names
to userspace. This is something very generic and should be implemented
as such, not as something Chromebook-specific.

Patches like that has however already been suggested, and I have NACKed
them because the GPIO sysfs ABI is insane, and that is why I am refactoring
the world to create a proper chardev ABI for GPIO instead. See:
http://marc.info/?l=linux-gpio&m=144550276512673&w=2

So for the moment, NACK on this, please participate in creating the
*right* ABI for GPIO instead of trying to shoehorn stuff into the dying
sysfs ABI.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1284097 — [PATCH 1/3] Device tree binding documentation for gpio-switch

FromMartyn Welch <martyn.welch@collabora.co.uk>
Date2015-12-04 18:40 +0100
Subject[PATCH 1/3] Device tree binding documentation for gpio-switch
Message-ID<qC2uV-5Kw-73@gated-at.bofh.it>
In reply to#1284084
This patch adds documentation for the gpio-switch binding. This binding
provides a mechanism to bind named links to gpio, with the primary
purpose of enabling standardised access to switches that might be standard
across a group of devices but implemented differently on each device.

Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
 .../devicetree/bindings/misc/gpio-switch.txt       | 47 ++++++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/gpio-switch.txt

diff --git a/Documentation/devicetree/bindings/misc/gpio-switch.txt b/Documentation/devicetree/bindings/misc/gpio-switch.txt
new file mode 100644
index 0000000..13528bd
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/gpio-switch.txt
@@ -0,0 +1,47 @@
+Device-Tree bindings for gpio attached switches.
+
+This provides a mechanism to provide a named link to specified gpios. This can
+be useful in instances such as when theres a need to monitor a switch, which is
+common across a family of devices, but attached to different gpios and even
+implemented in different ways on differnet devices.
+
+Required properties:
+	- compatible = "gpio-switch";
+
+Each signal is represented as a sub-node of "gpio-switch". The naming of
+sub-nodes is arbitrary.
+
+Required sub-node properties:
+
+	- label: Name to be given to gpio switch.
+	- gpios: OF device-tree gpio specification.
+
+Optional sub-node properties:
+
+	- read-only: Boolean flag to mark the gpio as read-only, i.e. the line
+	  should not be driven by the host.
+
+Example nodes:
+
+        gpio-switch {
+                compatible = "gpio-switch";
+
+                write-protect {
+                        label = "write-protect";
+                        gpios = <&gpx3 0 GPIO_ACTIVE_LOW>;
+                        read-only;
+                };
+
+                developer-switch {
+                        label = "developer-switch";
+                        gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>;
+                        read-only;
+                };
+
+                recovery-switch {
+                        label = "recovery-switch";
+                        gpios = <&gpx0 7 GPIO_ACTIVE_LOW>;
+                        read-only;
+                };
+        };
+
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1285823 — Re: [PATCH 1/3] Device tree binding documentation for gpio-switch

FromRob Herring <robh@kernel.org>
Date2015-12-07 18:40 +0100
SubjectRe: [PATCH 1/3] Device tree binding documentation for gpio-switch
Message-ID<qD7Vw-7B3-13@gated-at.bofh.it>
In reply to#1284097
+Linus W

On Fri, Dec 04, 2015 at 05:31:13PM +0000, Martyn Welch wrote:
> This patch adds documentation for the gpio-switch binding. This binding
> provides a mechanism to bind named links to gpio, with the primary
> purpose of enabling standardised access to switches that might be standard
> across a group of devices but implemented differently on each device.

This is good and what I suggested, but it now makes me wonder if switch 
is generic enough. This boils down to needing to expose single gpio 
lines to userspace with a defined function/use. IIRC, there's been some 
discussion about this before along with improving the userspace 
interface for GPIO in general. So I'd like to get Linus' thoughts on 
this.


> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
> ---
>  .../devicetree/bindings/misc/gpio-switch.txt       | 47 ++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/gpio-switch.txt
> 
> diff --git a/Documentation/devicetree/bindings/misc/gpio-switch.txt b/Documentation/devicetree/bindings/misc/gpio-switch.txt
> new file mode 100644
> index 0000000..13528bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/gpio-switch.txt
> @@ -0,0 +1,47 @@
> +Device-Tree bindings for gpio attached switches.
> +
> +This provides a mechanism to provide a named link to specified gpios. This can
> +be useful in instances such as when theres a need to monitor a switch, which is
> +common across a family of devices, but attached to different gpios and even
> +implemented in different ways on differnet devices.
> +
> +Required properties:
> +	- compatible = "gpio-switch";
> +
> +Each signal is represented as a sub-node of "gpio-switch". The naming of
> +sub-nodes is arbitrary.
> +
> +Required sub-node properties:
> +
> +	- label: Name to be given to gpio switch.
> +	- gpios: OF device-tree gpio specification.
> +
> +Optional sub-node properties:
> +
> +	- read-only: Boolean flag to mark the gpio as read-only, i.e. the line
> +	  should not be driven by the host.

In terms a a switch use, allowing driving it would be an override of the 
switch. Is that the idea here?

> +
> +Example nodes:
> +
> +        gpio-switch {
> +                compatible = "gpio-switch";

Both from a binding and driver perspective, there is no point in 
grouping these. Each node can simply have this compatible string.

> +
> +                write-protect {
> +                        label = "write-protect";
> +                        gpios = <&gpx3 0 GPIO_ACTIVE_LOW>;
> +                        read-only;
> +                };
> +
> +                developer-switch {
> +                        label = "developer-switch";
> +                        gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>;
> +                        read-only;
> +                };
> +
> +                recovery-switch {
> +                        label = "recovery-switch";
> +                        gpios = <&gpx0 7 GPIO_ACTIVE_LOW>;
> +                        read-only;
> +                };
> +        };
> +
> -- 
> 2.1.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1285934 — Re: [PATCH 1/3] Device tree binding documentation for gpio-switch

FromMartyn Welch <martyn.welch@collabora.co.uk>
Date2015-12-07 22:20 +0100
SubjectRe: [PATCH 1/3] Device tree binding documentation for gpio-switch
Message-ID<qDbmp-1sP-13@gated-at.bofh.it>
In reply to#1285823

On 07/12/15 17:37, Rob Herring wrote:
> +Linus W
>
> On Fri, Dec 04, 2015 at 05:31:13PM +0000, Martyn Welch wrote:
>> This patch adds documentation for the gpio-switch binding. This binding
>> provides a mechanism to bind named links to gpio, with the primary
>> purpose of enabling standardised access to switches that might be standard
>> across a group of devices but implemented differently on each device.
>
> This is good and what I suggested, but it now makes me wonder if switch
> is generic enough. This boils down to needing to expose single gpio
> lines to userspace with a defined function/use. IIRC, there's been some
> discussion about this before along with improving the userspace
> interface for GPIO in general. So I'd like to get Linus' thoughts on
> this.
>

No problem. Rename gpio-signal?

>
>> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
>> ---
>>   .../devicetree/bindings/misc/gpio-switch.txt       | 47 ++++++++++++++++++++++
>>   1 file changed, 47 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/misc/gpio-switch.txt
>>
>> diff --git a/Documentation/devicetree/bindings/misc/gpio-switch.txt b/Documentation/devicetree/bindings/misc/gpio-switch.txt
>> new file mode 100644
>> index 0000000..13528bd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/gpio-switch.txt
>> @@ -0,0 +1,47 @@
>> +Device-Tree bindings for gpio attached switches.
>> +
>> +This provides a mechanism to provide a named link to specified gpios. This can
>> +be useful in instances such as when theres a need to monitor a switch, which is
>> +common across a family of devices, but attached to different gpios and even
>> +implemented in different ways on differnet devices.
>> +
>> +Required properties:
>> +	- compatible = "gpio-switch";
>> +
>> +Each signal is represented as a sub-node of "gpio-switch". The naming of
>> +sub-nodes is arbitrary.
>> +
>> +Required sub-node properties:
>> +
>> +	- label: Name to be given to gpio switch.
>> +	- gpios: OF device-tree gpio specification.
>> +
>> +Optional sub-node properties:
>> +
>> +	- read-only: Boolean flag to mark the gpio as read-only, i.e. the line
>> +	  should not be driven by the host.
>
> In terms a a switch use, allowing driving it would be an override of the
> switch. Is that the idea here?
>

Yeah - since it had become a lot more generic and a lot of 
switches/signals would probably be implemented with a pull-up resistor 
of something like that, it seemed to make sense to allow them to be 
driven as well.

>> +
>> +Example nodes:
>> +
>> +        gpio-switch {
>> +                compatible = "gpio-switch";
>
> Both from a binding and driver perspective, there is no point in
> grouping these. Each node can simply have this compatible string.
>

True. I did it this way as this is how gpio-keys is implemented. OK, 
that has one optional parameter (autorepeat) for the block and this has 
none. Though I can also see that these probably have less in common than 
the individual lines used in gpio-keys.

>> +
>> +                write-protect {
>> +                        label = "write-protect";
>> +                        gpios = <&gpx3 0 GPIO_ACTIVE_LOW>;
>> +                        read-only;
>> +                };
>> +
>> +                developer-switch {
>> +                        label = "developer-switch";
>> +                        gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>;
>> +                        read-only;
>> +                };
>> +
>> +                recovery-switch {
>> +                        label = "recovery-switch";
>> +                        gpios = <&gpx0 7 GPIO_ACTIVE_LOW>;
>> +                        read-only;
>> +                };
>> +        };
>> +
>> --
>> 2.1.4
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1289530 — Re: [PATCH 1/3] Device tree binding documentation for gpio-switch

FromLinus Walleij <linus.walleij@linaro.org>
Date2015-12-11 13:50 +0100
SubjectRe: [PATCH 1/3] Device tree binding documentation for gpio-switch
Message-ID<qEvj4-4o4-15@gated-at.bofh.it>
In reply to#1284097
On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch
<martyn.welch@collabora.co.uk> wrote:

> This patch adds documentation for the gpio-switch binding. This binding
> provides a mechanism to bind named links to gpio, with the primary
> purpose of enabling standardised access to switches that might be standard
> across a group of devices but implemented differently on each device.
>
> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>

As mentioned in the comment to the second patch, this solves the
following generic problem:

Expose a GPIO line to userspace using a specific name

That means basically naming GPIO lines and marking them as
"not used by the operating system".

This is something that has been proposed before, and postponed
because the kernel lacks the right infrastructure.

Markus Pargmann also did a series that add initial values to
hogs, which is the inverse usecase of this, where you want to
*output* something by default, then maybe also make it available
to userspace.

So what we need to see here is a patch series that does all of these
things:

- Name lines

- Sets them to initial values

- Mark them as read-only

- Mark them as "not used by the operating system" so that they
  can be default-exported to userspace.

Making *USE* of this naming etc inside the Linux kernel

> +        gpio-switch {
> +                compatible = "gpio-switch";
> +
> +                write-protect {
> +                        label = "write-protect";
> +                        gpios = <&gpx3 0 GPIO_ACTIVE_LOW>;
> +                        read-only;
> +                };

This should not need new structures and nodes like this. It should
be part of Documentation/devicetree/bindings/gpio/gpio.txt
and put directly in the gpiochip node.

Maybe as an extension of the existing hogs, but that has already
been tried.

While we can agree on a device tree binding, the kernel still needs
major refactoring to actually expose named GPIOs to userspace,
and that should be done using the new chardev, not with sysfs
links.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1289600 — Re: [PATCH 1/3] Device tree binding documentation for gpio-switch

FromRob Herring <robh+dt@kernel.org>
Date2015-12-11 15:10 +0100
SubjectRe: [PATCH 1/3] Device tree binding documentation for gpio-switch
Message-ID<qEwyu-5wq-23@gated-at.bofh.it>
In reply to#1289530
On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch
> <martyn.welch@collabora.co.uk> wrote:
>
>> This patch adds documentation for the gpio-switch binding. This binding
>> provides a mechanism to bind named links to gpio, with the primary
>> purpose of enabling standardised access to switches that might be standard
>> across a group of devices but implemented differently on each device.
>>
>> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
>
> As mentioned in the comment to the second patch, this solves the
> following generic problem:
>
> Expose a GPIO line to userspace using a specific name
>
> That means basically naming GPIO lines and marking them as
> "not used by the operating system".
>
> This is something that has been proposed before, and postponed
> because the kernel lacks the right infrastructure.

That doesn't necessarily mean we can't define a binding.

> Markus Pargmann also did a series that add initial values to
> hogs, which is the inverse usecase of this, where you want to
> *output* something by default, then maybe also make it available
> to userspace.
>
> So what we need to see here is a patch series that does all of these
> things:
>
> - Name lines
>
> - Sets them to initial values
>
> - Mark them as read-only
>
> - Mark them as "not used by the operating system" so that they
>   can be default-exported to userspace.

No! This should not be a DT property.

Whether I want to control a GPIO in the kernel or userspace is not
known and can change over time. It could simply depend on kernel
config. There is also the case that a GPIO has no connection or kernel
driver until some time later when a DT overlay for an expansion board
is applied.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1284100 — [PATCH 3/3] ARM: dts: Addition of binding for gpio switches on peach-pi

FromMartyn Welch <martyn.welch@collabora.co.uk>
Date2015-12-04 18:40 +0100
Subject[PATCH 3/3] ARM: dts: Addition of binding for gpio switches on peach-pi
Message-ID<qC2uV-5Kw-81@gated-at.bofh.it>
In reply to#1284084
The peach pi has a GPIO connected to the firmware write protect, developer
mode and recovery mode lines (which are primarily controlled via external
switches on developer test board). This patch adds the required nodes to
the device tree to configure the pinmuxing and allow these to be read from
user space.

Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
 arch/arm/boot/dts/exynos5800-peach-pi.dts | 46 +++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index 49a4f43..2937372 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -53,6 +53,31 @@
 		};
 	};
 
+	gpio-switch {
+		compatible = "gpio-switch";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&wp_gpio &dev_mode &rec_mode>;
+
+		write-protect {
+			label = "write-protect";
+			gpios = <&gpx3 0 GPIO_ACTIVE_LOW>;
+			read-only;
+		};
+
+		developer-switch {
+			label = "developer-switch";
+			gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>;
+			read-only;
+		};
+
+		recovery-switch {
+			label = "recovery-switch";
+			gpios = <&gpx0 7 GPIO_ACTIVE_LOW>;
+			read-only;
+		};
+	};
+
 	gpio-keys {
 		compatible = "gpio-keys";
 
@@ -731,6 +756,13 @@
 		samsung,pin-val = <0>;
 	};
 
+	rec_mode: rec-mode {
+		samsung,pins = "gpx0-7";
+		samsung,pin-function = <0>;
+		samsung,pin-pud = <0>;
+		samsung,pin-drv = <0>;
+	};
+
 	tpm_irq: tpm-irq {
 		samsung,pins = "gpx1-0";
 		samsung,pin-function = <0>;
@@ -752,6 +784,13 @@
 		samsung,pin-drv = <0>;
 	};
 
+	dev_mode: dev-mode {
+		samsung,pins = "gpx1-3";
+		samsung,pin-function = <0>;
+		samsung,pin-pud = <3>;
+		samsung,pin-drv = <0>;
+	};
+
 	ec_irq: ec-irq {
 		samsung,pins = "gpx1-5";
 		samsung,pin-function = <0>;
@@ -773,6 +812,13 @@
 		samsung,pin-drv = <0>;
 	};
 
+	wp_gpio: wp_gpio {
+		samsung,pins = "gpx3-0";
+		samsung,pin-function = <0>;
+		samsung,pin-pud = <0>;
+		samsung,pin-drv = <0>;
+	};
+
 	max77802_irq: max77802-irq {
 		samsung,pins = "gpx3-1";
 		samsung,pin-function = <0>;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web