[RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Houcheng Lin]

The Problem
-----------
The reset signal on a hardware board is send either:
    - during machine initialization
    - during bus master's initialization

In some hardware design, devices on bus need a non-standard and extra reset
signal after bus is initialied. Most reason is to wake up device from hanging
state.

The board spefic reset code can not be put into machine init code, as it is
too early. This code can not also be put onto chip's driver, as it is board
specific and not suitable for a common chip driver.

Defer Reset Object
------------------
The defer reset object is to resolve this issue, developer can put a defer-
reset device on the board's dts file and enable DEFER RESET OBJECT CONFIG.
During driver init-calls, a defer-reset object is created and issue reset signal
after the enclosing device is initialized.

This eliminate the need to rewrite a driver module with only one purpose: sending
a board specific reset. This also allow mainstream kernel to support many boards
that modify the common drivers to send board specific reset. Configuring defer-reset
device in dts leave the board specific reset rules on board level and simple to
maintain.

Example dts File
----------------
Example 1:
    defer_reset_vbus {
        compatible = "defer-reset";
        reset-gpios = <&gpx3 5 GPIO_ACTIVE_LOW>;
        duration = <5>;
    };

Example 2:
    defer_reset_vbus {
        compatible = "defer-reset";
        reset-gpios = <&gpx3 5 GPIO_ACTIVE_HIGH>;
        duration = <0>;
    };

Block Diagram of dts File
-------------------------
    +-------------------------------------+
    | usb-ehci-chip@1211000               |
    |   +-------------------------+       |
    |   | defer-reset(gpx3)       |       |
    |   +-------------------------+       |
    +-------------------------------------+

Signed-off-by: Houcheng Lin <houcheng@gmail.com>
---
 .../devicetree/bindings/reset/gpio-defer-reset.txt |  30 ++++
 drivers/reset/Kconfig                              |   8 +
 drivers/reset/Makefile                             |   1 +
 drivers/reset/gpio-defer-reset.c                   | 180 +++++++++++++++++++++
 4 files changed, 219 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/gpio-defer-reset.txt
 create mode 100644 drivers/reset/gpio-defer-reset.c

diff --git a/Documentation/devicetree/bindings/reset/gpio-defer-reset.txt b/Documentation/devicetree/bindings/reset/gpio-defer-reset.txt
new file mode 100644
index 0000000..2ef416e
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/gpio-defer-reset.txt
@@ -0,0 +1,30 @@
+GPIO defer reset binding
+
+Put a defer-reset device in a device node and enable DEFER RESET OBJECT CONFIG.
+During driver init-calls, a defer-reset object will be created and issue reset
+signal after the enclosing device node's initialization complete.
+
+Required properties:
+- compatible:
+  - "defer-reset" for creating defer reset object
+- reset-gpio: specify gpio pin to send reset signal, GPIO_ACTIVE_LOW indicates
+              the reset signal is low and would revert line back to high.
+              It can be an array.
+- duration: specify reset signal duration in ms, 0 indicates hold the reset
+            signal forever.
+            It can be an array.
+
+Example 1:
+	defer_reset_vbus {
+		compatible = "defer-reset";
+		reset-gpios = <&gpx3 5 GPIO_ACTIVE_LOW>;
+		duration = <5>;
+	};
+
+Example 2:
+	defer_reset_vbus {
+		compatible = "defer-reset";
+		reset-gpios = <&gpx3 5 GPIO_ACTIVE_HIGH>;
+		duration = <0>;
+	};
+
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 0615f50..d53eb26 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -12,4 +12,12 @@ menuconfig RESET_CONTROLLER
 
 	  If unsure, say no.
 
+config GPIO_DEFER_RESET
+	bool "Defer reset driver via gpio"
+	depends on OF_GPIO
+	help
+	  Enable defer reset drvier
+	  The reset signal would be issued after a device on USB or PCI bus is initialied.
+	  The dependency of reset signal and device can be specified in board's dts file.
+
 source "drivers/reset/sti/Kconfig"
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 4f60caf..a3fdfef 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_RESET_CONTROLLER) += core.o
 obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_ARCH_STI) += sti/
+obj-$(CONFIG_GPIO_DEFER_RESET)	+= gpio-defer-reset.o
diff --git a/drivers/reset/gpio-defer-reset.c b/drivers/reset/gpio-defer-reset.c
new file mode 100644
index 0000000..e75ec14
--- /dev/null
+++ b/drivers/reset/gpio-defer-reset.c
@@ -0,0 +1,180 @@
+/*
+ * GPIO Defer Reset Driver
+ *
+ * Copyright (C) 2014 Houcheng Lin
+ * Author: Houcheng Lin <houcheng@gmail.com>
+ *
+ * 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/module.h>
+#include <linux/gpio/consumer.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+
+#define DRIVER_NAME "defer-reset"
+#define DRIVER_DESC "GPIO defer reset driver"
+#define GDR_MAX_DEV 32
+
+
+static DEFINE_MUTEX(deferred_reset_mutex);
+static LIST_HEAD(deferred_reset_list);
+
+struct defer_reset_private {
+	struct list_head lnode;
+	struct device_node *node;  /* defer_reset device */
+};
+
+static void __exit gdr_cleanup(void);
+
+
+/*
+ * Try to force active_low if board DTS does not configure the
+ * GPIO_ACTIVE_LOW properties into gpio chip.
+ */
+static void gdr_issue_reset_gpio(struct gpio_desc *gpiod, int val,
+	enum of_gpio_flags  flags)
+{
+	if (flags == OF_GPIO_ACTIVE_LOW && !gpiod_is_active_low(gpiod))
+		gpiod_set_value(gpiod, !val);
+	else
+		gpiod_set_value(gpiod, val);
+}
+
+
+static int gdr_issue_reset_line(
+	struct device_node *of_node, int index, int val)
+{
+	enum of_gpio_flags flags = 0;
+	struct gpio_desc *gpiod;
+	gpiod = of_get_named_gpiod_flags(of_node, "reset-gpios", index, &flags);
+	if (IS_ERR(gpiod))
+		return -PTR_ERR(gpiod);
+	gdr_issue_reset_gpio(gpiod, val, flags);
+	return 0;
+}
+
+/**
+ *  @pdev: deferred reset object's pdev
+ *  @of_node: deferred reset object's OF node
+ */
+static int gdr_issue_reset(
+	struct platform_device *pdev, struct device_node *of_node)
+{
+	int i;
+	int count = of_gpio_named_count(of_node, "reset-gpios");
+	u32 duration = 5;
+
+	if (count > GDR_MAX_DEV) {
+		dev_err(&pdev->dev, "too large reset array!\n");
+		return -EINVAL;
+	}
+	dev_info(&pdev->dev, "send reset signal for device [%s]\n",
+		of_node->parent->name);
+	/* setup parameters */
+	of_property_read_u32(of_node, "duration", &duration);
+	for (i = 0; i < count; i++) {
+		int ret = gdr_issue_reset_line(of_node, i, 1);
+		if (ret < 0)
+			dev_err(&pdev->dev, "error get gpiod:%d\n", ret);
+	}
+	/** hold reset signal forever if duration 0 */
+	if (duration == 0)
+		return 0;
+	mdelay(duration);
+	for (i = 0; i < count; i++) {
+		int ret = gdr_issue_reset_line(of_node, i, 0);
+		if (ret < 0)
+			dev_err(&pdev->dev, "error get gpiod:%d\n", ret);
+	}
+	return 0;
+}
+
+/*
+ * The pdev parameter is provided by register routine,
+ * platform_device_register_simple()
+ */
+static int gdr_probe(struct platform_device *pdev_gdr)
+{
+	struct list_head *pos, *n;
+
+	pr_debug("gpio defer reset probe\n");
+
+	mutex_lock(&deferred_reset_mutex);
+	list_for_each_safe(pos, n, &deferred_reset_list) {
+		struct defer_reset_private *pdata;
+		struct platform_device *pdev;
+		pdata = list_entry(pos, struct defer_reset_private, lnode);
+		pdev = of_find_device_by_node(pdata->node->parent);
+		if (pdev != NULL && pdev->dev.driver != NULL) {
+			gdr_issue_reset(pdev_gdr, pdata->node);
+			list_del(pos);
+			kfree(pdata);
+		}
+	}
+	mutex_unlock(&deferred_reset_mutex);
+	list_for_each(pos, &deferred_reset_list) {
+		return -EPROBE_DEFER;
+	}
+	dev_err(&pdev_gdr->dev, "return %d to release resources\n", -EUCLEAN);
+	return -EUCLEAN;
+}
+
+
+
+#ifdef CONFIG_OF
+static const struct of_device_id gdr_match[] = {
+	{ .compatible = "defer-reset" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, gdr_match);
+#endif
+
+static struct platform_driver gdr_driver = {
+	.probe      = gdr_probe,
+	.driver = {
+		.name   = DRIVER_NAME,
+		.owner  = THIS_MODULE,
+		.of_match_table = of_match_ptr(gdr_match),
+	}
+};
+
+
+static int __init gdr_init(void)
+{
+	struct device_node *node;
+	pr_info("defer-reset object initialied.\n");
+	platform_device_register_simple("defer-reset", -1, NULL, 0);
+	mutex_lock(&deferred_reset_mutex);
+	for_each_compatible_node(node, NULL, "defer-reset") {
+		struct defer_reset_private *pdata = kmalloc(
+			sizeof(struct defer_reset_private), GFP_KERNEL);
+		pdata->node = node;
+		list_add_tail(&pdata->lnode, &deferred_reset_list);
+	}
+	mutex_unlock(&deferred_reset_mutex);
+	return platform_driver_register(&gdr_driver);
+}
+
+module_init(gdr_init);
+
+static void __exit gdr_cleanup(void)
+{
+	pr_info("defer-reset cleanup.\n");
+	platform_driver_unregister(&gdr_driver);
+}
+
+module_exit(gdr_cleanup);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_ALIAS("platform:defer-reset");
+MODULE_AUTHOR("Houcheng Lin");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2746 bytes --]

On Wed, Jun 18, 2014 at 09:37:53PM +0800, Houcheng Lin wrote:
> The Problem
> -----------
> The reset signal on a hardware board is send either:
>     - during machine initialization
>     - during bus master's initialization
> 
> In some hardware design, devices on bus need a non-standard and extra reset
> signal after bus is initialied. Most reason is to wake up device from hanging
> state.
> 
> The board spefic reset code can not be put into machine init code, as it is
> too early. This code can not also be put onto chip's driver, as it is board
> specific and not suitable for a common chip driver.

I don't see why not. I don't get the difference with an optional
regulator, or an optional GPIO, or whatever. And there's already some
support for such a case, with reset_control_get_optional.

> 
> Defer Reset Object
> ------------------
> The defer reset object is to resolve this issue, developer can put a defer-
> reset device on the board's dts file and enable DEFER RESET OBJECT CONFIG.
> During driver init-calls, a defer-reset object is created and issue reset signal
> after the enclosing device is initialized.
> 
> This eliminate the need to rewrite a driver module with only one purpose: sending
> a board specific reset. This also allow mainstream kernel to support many boards
> that modify the common drivers to send board specific reset. Configuring defer-reset
> device in dts leave the board specific reset rules on board level and simple to
> maintain.
> 
> Example dts File
> ----------------
> Example 1:
>     defer_reset_vbus {
>         compatible = "defer-reset";
>         reset-gpios = <&gpx3 5 GPIO_ACTIVE_LOW>;

This doesn't follow the generic reset bindings.

>         duration = <5>;
>     };
> 
> Example 2:
>     defer_reset_vbus {
>         compatible = "defer-reset";
>         reset-gpios = <&gpx3 5 GPIO_ACTIVE_HIGH>;
>         duration = <0>;
>     };
> 
> Block Diagram of dts File
> -------------------------
>     +-------------------------------------+
>     | usb-ehci-chip@1211000               |
>     |   +-------------------------+       |
>     |   | defer-reset(gpx3)       |       |
>     |   +-------------------------+       |
>     +-------------------------------------+

And this goes against the current way of doing things, with the resets
property holding a phandle.

Why not just having something like:

usb {
    compatible = "whatever";
    resets = <&rst_gpio <duration>>;
}

Which uses the generic bindings, and doesn't require anything fancy
either in the driver or the DT.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Linus Walleij]

On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:

(...)
> +static DEFINE_MUTEX(deferred_reset_mutex);
> +static LIST_HEAD(deferred_reset_list);
> +
> +struct defer_reset_private {
> +       struct list_head lnode;
> +       struct device_node *node;  /* defer_reset device */

Isn't it better if you get the gpio descriptor(s) in probe() and store that
inside defer_reset_private instead of storing the device_node?

struct gpio_desc *gpiod;

(Then you need not include <linux/of_gpio.h>)

> +};


> +static int gdr_issue_reset_line(
> +       struct device_node *of_node, int index, int val)
> +{
> +       enum of_gpio_flags flags = 0;
> +       struct gpio_desc *gpiod;
> +       gpiod = of_get_named_gpiod_flags(of_node, "reset-gpios", index, &flags);
> +       if (IS_ERR(gpiod))
> +               return -PTR_ERR(gpiod);
> +       gdr_issue_reset_gpio(gpiod, val, flags);

So here just use the gpiod from the private struct instead
of looking it up in the last minute.

> +static int gdr_probe(struct platform_device *pdev_gdr)
> +{
> +       struct list_head *pos, *n;
> +
> +       pr_debug("gpio defer reset probe\n");
> +
> +       mutex_lock(&deferred_reset_mutex);
> +       list_for_each_safe(pos, n, &deferred_reset_list) {
> +               struct defer_reset_private *pdata;
> +               struct platform_device *pdev;
> +               pdata = list_entry(pos, struct defer_reset_private, lnode);
> +               pdev = of_find_device_by_node(pdata->node->parent);

So in this iteration I would get the gpio descriptor for each one using
something like:

pdata->gpiod = gpiod_get_index(&pdev->dev, ...);

Yours,
Linus Walleij

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Linus Walleij]

On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:

> The Problem
> -----------
> The reset signal on a hardware board is send either:
>     - during machine initialization
>     - during bus master's initialization

I just thought about this a bit, since there isn't already a generic GPIO
reset driver, just call this drivers/reset/reset-gpio.c and make the
ability to deferral just a configuration detail of the GPIO reset driver.

Yours,
Linus Walleij

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Linus Walleij]

On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:

> The Problem
> -----------
> The reset signal on a hardware board is send either:
>     - during machine initialization
>     - during bus master's initialization

I just thought about this a bit, since there isn't already a generic GPIO
reset driver, just call this drivers/reset/reset-gpio.c and make the
ability to deferral just a configuration detail of the GPIO reset driver.

Yours,
Linus Walleij

[RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Linus Walleij]

On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:

> The Problem
> -----------
> The reset signal on a hardware board is send either:
>     - during machine initialization
>     - during bus master's initialization

I just thought about this a bit, since there isn't already a generic GPIO
reset driver, just call this drivers/reset/reset-gpio.c and make the
ability to deferral just a configuration detail of the GPIO reset driver.

Yours,
Linus Walleij

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]

On Tue, Jul 08, 2014 at 09:52:03AM +0200, Linus Walleij wrote:
> On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:
> 
> > The Problem
> > -----------
> > The reset signal on a hardware board is send either:
> >     - during machine initialization
> >     - during bus master's initialization
> 
> I just thought about this a bit, since there isn't already a generic GPIO
> reset driver, just call this drivers/reset/reset-gpio.c and make the
> ability to deferral just a configuration detail of the GPIO reset driver.

Philipp has been working on one for quite some time. See
http://www.spinics.net/lists/arm-kernel/msg321927.html

However, it seems to progress slowly, and we don't seem to be able to
reach a consensus here.

If you ask me, having to set a few extra properties like this just
advocates for a regular reset driver and DT node for the reset GPIO,
but I'm pretty sure Philipp will feel otherwise :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1083 bytes --]

On Tue, Jul 08, 2014 at 09:52:03AM +0200, Linus Walleij wrote:
> On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:
> 
> > The Problem
> > -----------
> > The reset signal on a hardware board is send either:
> >     - during machine initialization
> >     - during bus master's initialization
> 
> I just thought about this a bit, since there isn't already a generic GPIO
> reset driver, just call this drivers/reset/reset-gpio.c and make the
> ability to deferral just a configuration detail of the GPIO reset driver.

Philipp has been working on one for quite some time. See
http://www.spinics.net/lists/arm-kernel/msg321927.html

However, it seems to progress slowly, and we don't seem to be able to
reach a consensus here.

If you ask me, having to set a few extra properties like this just
advocates for a regular reset driver and DT node for the reset GPIO,
but I'm pretty sure Philipp will feel otherwise :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Maxime Ripard]

On Tue, Jul 08, 2014 at 09:52:03AM +0200, Linus Walleij wrote:
> On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:
> 
> > The Problem
> > -----------
> > The reset signal on a hardware board is send either:
> >     - during machine initialization
> >     - during bus master's initialization
> 
> I just thought about this a bit, since there isn't already a generic GPIO
> reset driver, just call this drivers/reset/reset-gpio.c and make the
> ability to deferral just a configuration detail of the GPIO reset driver.

Philipp has been working on one for quite some time. See
http://www.spinics.net/lists/arm-kernel/msg321927.html

However, it seems to progress slowly, and we don't seem to be able to
reach a consensus here.

If you ask me, having to set a few extra properties like this just
advocates for a regular reset driver and DT node for the reset GPIO,
but I'm pretty sure Philipp will feel otherwise :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140708/0509ca19/attachment.sig>

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Linus Walleij]

On Tue, Jul 8, 2014 at 10:05 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Tue, Jul 08, 2014 at 09:52:03AM +0200, Linus Walleij wrote:
>> On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:
>>
>> > The Problem
>> > -----------
>> > The reset signal on a hardware board is send either:
>> >     - during machine initialization
>> >     - during bus master's initialization
>>
>> I just thought about this a bit, since there isn't already a generic GPIO
>> reset driver, just call this drivers/reset/reset-gpio.c and make the
>> ability to deferral just a configuration detail of the GPIO reset driver.
>
> Philipp has been working on one for quite some time. See
> http://www.spinics.net/lists/arm-kernel/msg321927.html
>
> However, it seems to progress slowly, and we don't seem to be able to
> reach a consensus here.
>
> If you ask me, having to set a few extra properties like this just
> advocates for a regular reset driver and DT node for the reset GPIO,
> but I'm pretty sure Philipp will feel otherwise :)

Hm haha yeah let's fight it out :-)

This is not my subsystem so I'm getting some popcorn.

Yours,
Linus Walleij

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Linus Walleij]

On Tue, Jul 8, 2014 at 10:05 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Tue, Jul 08, 2014 at 09:52:03AM +0200, Linus Walleij wrote:
>> On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:
>>
>> > The Problem
>> > -----------
>> > The reset signal on a hardware board is send either:
>> >     - during machine initialization
>> >     - during bus master's initialization
>>
>> I just thought about this a bit, since there isn't already a generic GPIO
>> reset driver, just call this drivers/reset/reset-gpio.c and make the
>> ability to deferral just a configuration detail of the GPIO reset driver.
>
> Philipp has been working on one for quite some time. See
> http://www.spinics.net/lists/arm-kernel/msg321927.html
>
> However, it seems to progress slowly, and we don't seem to be able to
> reach a consensus here.
>
> If you ask me, having to set a few extra properties like this just
> advocates for a regular reset driver and DT node for the reset GPIO,
> but I'm pretty sure Philipp will feel otherwise :)

Hm haha yeah let's fight it out :-)

This is not my subsystem so I'm getting some popcorn.

Yours,
Linus Walleij

[RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Linus Walleij]

On Tue, Jul 8, 2014 at 10:05 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Tue, Jul 08, 2014 at 09:52:03AM +0200, Linus Walleij wrote:
>> On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:
>>
>> > The Problem
>> > -----------
>> > The reset signal on a hardware board is send either:
>> >     - during machine initialization
>> >     - during bus master's initialization
>>
>> I just thought about this a bit, since there isn't already a generic GPIO
>> reset driver, just call this drivers/reset/reset-gpio.c and make the
>> ability to deferral just a configuration detail of the GPIO reset driver.
>
> Philipp has been working on one for quite some time. See
> http://www.spinics.net/lists/arm-kernel/msg321927.html
>
> However, it seems to progress slowly, and we don't seem to be able to
> reach a consensus here.
>
> If you ask me, having to set a few extra properties like this just
> advocates for a regular reset driver and DT node for the reset GPIO,
> but I'm pretty sure Philipp will feel otherwise :)

Hm haha yeah let's fight it out :-)

This is not my subsystem so I'm getting some popcorn.

Yours,
Linus Walleij

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Houcheng Lin]

[-- Attachment #1: Type: text/plain, Size: 2226 bytes --]

Hi Linus,
Thank you a lot for reviewing.
All gpiod descriptors could be enumerated during the driver loading and
stored into
private data structure. So the device_node is not needed stored in private
data structure.
Then use gpio descriptor to check status in the last minutes. I'll modify
these on next patch
version.

Best regards,
Houcheng Lin


2014-07-07 20:38 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>:

> On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:
>
> (...)
> > +static DEFINE_MUTEX(deferred_reset_mutex);
> > +static LIST_HEAD(deferred_reset_list);
> > +
> > +struct defer_reset_private {
> > +       struct list_head lnode;
> > +       struct device_node *node;  /* defer_reset device */
>
> Isn't it better if you get the gpio descriptor(s) in probe() and store that
> inside defer_reset_private instead of storing the device_node?
>
> struct gpio_desc *gpiod;
>
> (Then you need not include <linux/of_gpio.h>)
>
> > +};
>
>
> > +static int gdr_issue_reset_line(
> > +       struct device_node *of_node, int index, int val)
> > +{
> > +       enum of_gpio_flags flags = 0;
> > +       struct gpio_desc *gpiod;
> > +       gpiod = of_get_named_gpiod_flags(of_node, "reset-gpios", index,
> &flags);
> > +       if (IS_ERR(gpiod))
> > +               return -PTR_ERR(gpiod);
> > +       gdr_issue_reset_gpio(gpiod, val, flags);
>
> So here just use the gpiod from the private struct instead
> of looking it up in the last minute.
>
> > +static int gdr_probe(struct platform_device *pdev_gdr)
> > +{
> > +       struct list_head *pos, *n;
> > +
> > +       pr_debug("gpio defer reset probe\n");
> > +
> > +       mutex_lock(&deferred_reset_mutex);
> > +       list_for_each_safe(pos, n, &deferred_reset_list) {
> > +               struct defer_reset_private *pdata;
> > +               struct platform_device *pdev;
> > +               pdata = list_entry(pos, struct defer_reset_private,
> lnode);
> > +               pdev = of_find_device_by_node(pdata->node->parent);
>
> So in this iteration I would get the gpio descriptor for each one using
> something like:
>
> pdata->gpiod = gpiod_get_index(&pdev->dev, ...);
>
> Yours,
> Linus Walleij
>



-- 
Best regards,
Houcheng Lin

[-- Attachment #2: Type: text/html, Size: 3193 bytes --]

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Philipp Zabel]

Am Dienstag, den 08.07.2014, 11:38 +0200 schrieb Linus Walleij:
> On Tue, Jul 8, 2014 at 10:05 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Tue, Jul 08, 2014 at 09:52:03AM +0200, Linus Walleij wrote:
> >> On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:
> >>
> >> > The Problem
> >> > -----------
> >> > The reset signal on a hardware board is send either:
> >> >     - during machine initialization
> >> >     - during bus master's initialization
> >>
> >> I just thought about this a bit, since there isn't already a generic GPIO
> >> reset driver, just call this drivers/reset/reset-gpio.c and make the
> >> ability to deferral just a configuration detail of the GPIO reset driver.
> >
> > Philipp has been working on one for quite some time. See
> > http://www.spinics.net/lists/arm-kernel/msg321927.html
> >
> > However, it seems to progress slowly, and we don't seem to be able to
> > reach a consensus here.

Mostly because Maxime and I seem to have a completely different opinion
and nobody else argued one way or the other.

> > If you ask me, having to set a few extra properties like this just
> > advocates for a regular reset driver and DT node for the reset GPIO,
> > but I'm pretty sure Philipp will feel otherwise :)
> 
> Hm haha yeah let's fight it out :-)
> 
> This is not my subsystem so I'm getting some popcorn.

Sorry about missing this earlier, I hope the popcorn is not stale.

I think a reset that needs to be released before a fixed device appears
on the bus, should be handled by the bus driver. The reset framework
could be made to help with that, but I don't think a separate entity
that scans the whole device tree itself is the right solution.

regards
Philipp

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Philipp Zabel]

Am Dienstag, den 08.07.2014, 11:38 +0200 schrieb Linus Walleij:
> On Tue, Jul 8, 2014 at 10:05 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Tue, Jul 08, 2014 at 09:52:03AM +0200, Linus Walleij wrote:
> >> On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:
> >>
> >> > The Problem
> >> > -----------
> >> > The reset signal on a hardware board is send either:
> >> >     - during machine initialization
> >> >     - during bus master's initialization
> >>
> >> I just thought about this a bit, since there isn't already a generic GPIO
> >> reset driver, just call this drivers/reset/reset-gpio.c and make the
> >> ability to deferral just a configuration detail of the GPIO reset driver.
> >
> > Philipp has been working on one for quite some time. See
> > http://www.spinics.net/lists/arm-kernel/msg321927.html
> >
> > However, it seems to progress slowly, and we don't seem to be able to
> > reach a consensus here.

Mostly because Maxime and I seem to have a completely different opinion
and nobody else argued one way or the other.

> > If you ask me, having to set a few extra properties like this just
> > advocates for a regular reset driver and DT node for the reset GPIO,
> > but I'm pretty sure Philipp will feel otherwise :)
> 
> Hm haha yeah let's fight it out :-)
> 
> This is not my subsystem so I'm getting some popcorn.

Sorry about missing this earlier, I hope the popcorn is not stale.

I think a reset that needs to be released before a fixed device appears
on the bus, should be handled by the bus driver. The reset framework
could be made to help with that, but I don't think a separate entity
that scans the whole device tree itself is the right solution.

regards
Philipp

[RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Philipp Zabel]

Am Dienstag, den 08.07.2014, 11:38 +0200 schrieb Linus Walleij:
> On Tue, Jul 8, 2014 at 10:05 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Tue, Jul 08, 2014 at 09:52:03AM +0200, Linus Walleij wrote:
> >> On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:
> >>
> >> > The Problem
> >> > -----------
> >> > The reset signal on a hardware board is send either:
> >> >     - during machine initialization
> >> >     - during bus master's initialization
> >>
> >> I just thought about this a bit, since there isn't already a generic GPIO
> >> reset driver, just call this drivers/reset/reset-gpio.c and make the
> >> ability to deferral just a configuration detail of the GPIO reset driver.
> >
> > Philipp has been working on one for quite some time. See
> > http://www.spinics.net/lists/arm-kernel/msg321927.html
> >
> > However, it seems to progress slowly, and we don't seem to be able to
> > reach a consensus here.

Mostly because Maxime and I seem to have a completely different opinion
and nobody else argued one way or the other.

> > If you ask me, having to set a few extra properties like this just
> > advocates for a regular reset driver and DT node for the reset GPIO,
> > but I'm pretty sure Philipp will feel otherwise :)
> 
> Hm haha yeah let's fight it out :-)
> 
> This is not my subsystem so I'm getting some popcorn.

Sorry about missing this earlier, I hope the popcorn is not stale.

I think a reset that needs to be released before a fixed device appears
on the bus, should be handled by the bus driver. The reset framework
could be made to help with that, but I don't think a separate entity
that scans the whole device tree itself is the right solution.

regards
Philipp

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2157 bytes --]

On Fri, Aug 08, 2014 at 04:23:09PM +0200, Philipp Zabel wrote:
> Am Dienstag, den 08.07.2014, 11:38 +0200 schrieb Linus Walleij:
> > On Tue, Jul 8, 2014 at 10:05 AM, Maxime Ripard
> > <maxime.ripard@free-electrons.com> wrote:
> > > On Tue, Jul 08, 2014 at 09:52:03AM +0200, Linus Walleij wrote:
> > >> On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:
> > >>
> > >> > The Problem
> > >> > -----------
> > >> > The reset signal on a hardware board is send either:
> > >> >     - during machine initialization
> > >> >     - during bus master's initialization
> > >>
> > >> I just thought about this a bit, since there isn't already a generic GPIO
> > >> reset driver, just call this drivers/reset/reset-gpio.c and make the
> > >> ability to deferral just a configuration detail of the GPIO reset driver.
> > >
> > > Philipp has been working on one for quite some time. See
> > > http://www.spinics.net/lists/arm-kernel/msg321927.html
> > >
> > > However, it seems to progress slowly, and we don't seem to be able to
> > > reach a consensus here.
> 
> Mostly because Maxime and I seem to have a completely different opinion
> and nobody else argued one way or the other.

Yep, mostly because I don't see how a generic approach can work.

The existing reset-gpios property only provide the gpio to use, but
some informations are encoded in the driver, such as the reset
duration, or a reset sequence if any.

How do you plan on giving that information to your generic driver?

The only solution I can think of would be to add an extra property
that your code would parse. But then, you break the existing DT
bindings.

And if we're going to break those bindings, at least do it in a way
consistent with reset bindings.

Plus, your approach doesn't cover the weird corner cases such as:
  - reset-gpio
  - wlf,reset-gpios
  - phy-reset-gpios
  - snps,reset-gpio
  - the drivers that need several gpio and expect the reset one as a
    positional argument.
  - etc.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2157 bytes --]

On Fri, Aug 08, 2014 at 04:23:09PM +0200, Philipp Zabel wrote:
> Am Dienstag, den 08.07.2014, 11:38 +0200 schrieb Linus Walleij:
> > On Tue, Jul 8, 2014 at 10:05 AM, Maxime Ripard
> > <maxime.ripard@free-electrons.com> wrote:
> > > On Tue, Jul 08, 2014 at 09:52:03AM +0200, Linus Walleij wrote:
> > >> On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:
> > >>
> > >> > The Problem
> > >> > -----------
> > >> > The reset signal on a hardware board is send either:
> > >> >     - during machine initialization
> > >> >     - during bus master's initialization
> > >>
> > >> I just thought about this a bit, since there isn't already a generic GPIO
> > >> reset driver, just call this drivers/reset/reset-gpio.c and make the
> > >> ability to deferral just a configuration detail of the GPIO reset driver.
> > >
> > > Philipp has been working on one for quite some time. See
> > > http://www.spinics.net/lists/arm-kernel/msg321927.html
> > >
> > > However, it seems to progress slowly, and we don't seem to be able to
> > > reach a consensus here.
> 
> Mostly because Maxime and I seem to have a completely different opinion
> and nobody else argued one way or the other.

Yep, mostly because I don't see how a generic approach can work.

The existing reset-gpios property only provide the gpio to use, but
some informations are encoded in the driver, such as the reset
duration, or a reset sequence if any.

How do you plan on giving that information to your generic driver?

The only solution I can think of would be to add an extra property
that your code would parse. But then, you break the existing DT
bindings.

And if we're going to break those bindings, at least do it in a way
consistent with reset bindings.

Plus, your approach doesn't cover the weird corner cases such as:
  - reset-gpio
  - wlf,reset-gpios
  - phy-reset-gpios
  - snps,reset-gpio
  - the drivers that need several gpio and expect the reset one as a
    positional argument.
  - etc.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Maxime Ripard]

On Fri, Aug 08, 2014 at 04:23:09PM +0200, Philipp Zabel wrote:
> Am Dienstag, den 08.07.2014, 11:38 +0200 schrieb Linus Walleij:
> > On Tue, Jul 8, 2014 at 10:05 AM, Maxime Ripard
> > <maxime.ripard@free-electrons.com> wrote:
> > > On Tue, Jul 08, 2014 at 09:52:03AM +0200, Linus Walleij wrote:
> > >> On Wed, Jun 18, 2014 at 3:37 PM, Houcheng Lin <houcheng@gmail.com> wrote:
> > >>
> > >> > The Problem
> > >> > -----------
> > >> > The reset signal on a hardware board is send either:
> > >> >     - during machine initialization
> > >> >     - during bus master's initialization
> > >>
> > >> I just thought about this a bit, since there isn't already a generic GPIO
> > >> reset driver, just call this drivers/reset/reset-gpio.c and make the
> > >> ability to deferral just a configuration detail of the GPIO reset driver.
> > >
> > > Philipp has been working on one for quite some time. See
> > > http://www.spinics.net/lists/arm-kernel/msg321927.html
> > >
> > > However, it seems to progress slowly, and we don't seem to be able to
> > > reach a consensus here.
> 
> Mostly because Maxime and I seem to have a completely different opinion
> and nobody else argued one way or the other.

Yep, mostly because I don't see how a generic approach can work.

The existing reset-gpios property only provide the gpio to use, but
some informations are encoded in the driver, such as the reset
duration, or a reset sequence if any.

How do you plan on giving that information to your generic driver?

The only solution I can think of would be to add an extra property
that your code would parse. But then, you break the existing DT
bindings.

And if we're going to break those bindings, at least do it in a way
consistent with reset bindings.

Plus, your approach doesn't cover the weird corner cases such as:
  - reset-gpio
  - wlf,reset-gpios
  - phy-reset-gpios
  - snps,reset-gpio
  - the drivers that need several gpio and expect the reset one as a
    positional argument.
  - etc.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140811/8b38b659/attachment.sig>

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Philipp Zabel]

Hi Maxime,

Am Montag, den 11.08.2014, 19:33 +0200 schrieb Maxime Ripard:
> > Mostly because Maxime and I seem to have a completely different opinion
> > and nobody else argued one way or the other.
> 
> Yep, mostly because I don't see how a generic approach can work.
> 
> The existing reset-gpios property only provide the gpio to use, but
> some informations are encoded in the driver, such as the reset
> duration, or a reset sequence if any.

The driver should provide the duration. I'd really like to see an
example where sequencing is necessary.
I agree that as soon as things get significantly more complicated than
pulsing a single GPIO, the reset-gpios binding is too limited.
Still, I'm not happy to mandate a separate gpio reset device for each
reset line if most devices are simple enough for it to work without.
What about using reset-gpios for the majority of simple cases and have a
separate gpio-reset-sequencer driver when multiple GPIO resets have to
be timed?

> How do you plan on giving that information to your generic driver?
> 
> The only solution I can think of would be to add an extra property
> that your code would parse. But then, you break the existing DT
> bindings.
>
> And if we're going to break those bindings, at least do it in a way
> consistent with reset bindings.

For the backwards compatibility case, the driver already has to provide
the duration. I don't want to break the existing bindings at all.

> Plus, your approach doesn't cover the weird corner cases such as:
>   - reset-gpio
>   - wlf,reset-gpios
>   - phy-reset-gpios
>   - snps,reset-gpio
>   - the drivers that need several gpio and expect the reset one as a
>     positional argument.
>   - etc.

Those are just an issue of the implementation I posted earlier because
gpiod_get doesn't support custom names other than %s-gpios. This could
be extended and handled just as well if deemed necessary.

regards
Philipp

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Philipp Zabel]

Hi Maxime,

Am Montag, den 11.08.2014, 19:33 +0200 schrieb Maxime Ripard:
> > Mostly because Maxime and I seem to have a completely different opinion
> > and nobody else argued one way or the other.
> 
> Yep, mostly because I don't see how a generic approach can work.
> 
> The existing reset-gpios property only provide the gpio to use, but
> some informations are encoded in the driver, such as the reset
> duration, or a reset sequence if any.

The driver should provide the duration. I'd really like to see an
example where sequencing is necessary.
I agree that as soon as things get significantly more complicated than
pulsing a single GPIO, the reset-gpios binding is too limited.
Still, I'm not happy to mandate a separate gpio reset device for each
reset line if most devices are simple enough for it to work without.
What about using reset-gpios for the majority of simple cases and have a
separate gpio-reset-sequencer driver when multiple GPIO resets have to
be timed?

> How do you plan on giving that information to your generic driver?
> 
> The only solution I can think of would be to add an extra property
> that your code would parse. But then, you break the existing DT
> bindings.
>
> And if we're going to break those bindings, at least do it in a way
> consistent with reset bindings.

For the backwards compatibility case, the driver already has to provide
the duration. I don't want to break the existing bindings at all.

> Plus, your approach doesn't cover the weird corner cases such as:
>   - reset-gpio
>   - wlf,reset-gpios
>   - phy-reset-gpios
>   - snps,reset-gpio
>   - the drivers that need several gpio and expect the reset one as a
>     positional argument.
>   - etc.

Those are just an issue of the implementation I posted earlier because
gpiod_get doesn't support custom names other than %s-gpios. This could
be extended and handled just as well if deemed necessary.

regards
Philipp

[RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Philipp Zabel]

Hi Maxime,

Am Montag, den 11.08.2014, 19:33 +0200 schrieb Maxime Ripard:
> > Mostly because Maxime and I seem to have a completely different opinion
> > and nobody else argued one way or the other.
> 
> Yep, mostly because I don't see how a generic approach can work.
> 
> The existing reset-gpios property only provide the gpio to use, but
> some informations are encoded in the driver, such as the reset
> duration, or a reset sequence if any.

The driver should provide the duration. I'd really like to see an
example where sequencing is necessary.
I agree that as soon as things get significantly more complicated than
pulsing a single GPIO, the reset-gpios binding is too limited.
Still, I'm not happy to mandate a separate gpio reset device for each
reset line if most devices are simple enough for it to work without.
What about using reset-gpios for the majority of simple cases and have a
separate gpio-reset-sequencer driver when multiple GPIO resets have to
be timed?

> How do you plan on giving that information to your generic driver?
> 
> The only solution I can think of would be to add an extra property
> that your code would parse. But then, you break the existing DT
> bindings.
>
> And if we're going to break those bindings, at least do it in a way
> consistent with reset bindings.

For the backwards compatibility case, the driver already has to provide
the duration. I don't want to break the existing bindings at all.

> Plus, your approach doesn't cover the weird corner cases such as:
>   - reset-gpio
>   - wlf,reset-gpios
>   - phy-reset-gpios
>   - snps,reset-gpio
>   - the drivers that need several gpio and expect the reset one as a
>     positional argument.
>   - etc.

Those are just an issue of the implementation I posted earlier because
gpiod_get doesn't support custom names other than %s-gpios. This could
be extended and handled just as well if deemed necessary.

regards
Philipp

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1808 bytes --]

On Thu, Aug 14, 2014 at 11:36:38AM +0200, Philipp Zabel wrote:
> Hi Maxime,
> 
> Am Montag, den 11.08.2014, 19:33 +0200 schrieb Maxime Ripard:
> > > Mostly because Maxime and I seem to have a completely different opinion
> > > and nobody else argued one way or the other.
> > 
> > Yep, mostly because I don't see how a generic approach can work.
> > 
> > The existing reset-gpios property only provide the gpio to use, but
> > some informations are encoded in the driver, such as the reset
> > duration, or a reset sequence if any.
> 
> The driver should provide the duration.

How? This used to be in the code, and reset_control_reset doesn't take
such argument.

> I'd really like to see an example where sequencing is necessary.

Well, if you have several reset lines, the sequencing between each
might be important.

> I agree that as soon as things get significantly more complicated than
> pulsing a single GPIO, the reset-gpios binding is too limited.

While the generic reset bindings are perfect for that.

> Still, I'm not happy to mandate a separate gpio reset device for each
> reset line if most devices are simple enough for it to work without.

Well, it's pretty much already the case for other subsystems, such as
regulator.

I guess we can treat this as a legacy option, but allow the reset-gpio
code to be a full driver of its own, if we need more advanced use
cases?

> What about using reset-gpios for the majority of simple cases and have a
> separate gpio-reset-sequencer driver when multiple GPIO resets have to
> be timed?

I don't know. I feel like it should be in the driver itself, rather
than in a generic layer.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1808 bytes --]

On Thu, Aug 14, 2014 at 11:36:38AM +0200, Philipp Zabel wrote:
> Hi Maxime,
> 
> Am Montag, den 11.08.2014, 19:33 +0200 schrieb Maxime Ripard:
> > > Mostly because Maxime and I seem to have a completely different opinion
> > > and nobody else argued one way or the other.
> > 
> > Yep, mostly because I don't see how a generic approach can work.
> > 
> > The existing reset-gpios property only provide the gpio to use, but
> > some informations are encoded in the driver, such as the reset
> > duration, or a reset sequence if any.
> 
> The driver should provide the duration.

How? This used to be in the code, and reset_control_reset doesn't take
such argument.

> I'd really like to see an example where sequencing is necessary.

Well, if you have several reset lines, the sequencing between each
might be important.

> I agree that as soon as things get significantly more complicated than
> pulsing a single GPIO, the reset-gpios binding is too limited.

While the generic reset bindings are perfect for that.

> Still, I'm not happy to mandate a separate gpio reset device for each
> reset line if most devices are simple enough for it to work without.

Well, it's pretty much already the case for other subsystems, such as
regulator.

I guess we can treat this as a legacy option, but allow the reset-gpio
code to be a full driver of its own, if we need more advanced use
cases?

> What about using reset-gpios for the majority of simple cases and have a
> separate gpio-reset-sequencer driver when multiple GPIO resets have to
> be timed?

I don't know. I feel like it should be in the driver itself, rather
than in a generic layer.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[RFC PATCH v3] reset: Add a defer reset object to send board specific reset

[Maxime Ripard]

On Thu, Aug 14, 2014 at 11:36:38AM +0200, Philipp Zabel wrote:
> Hi Maxime,
> 
> Am Montag, den 11.08.2014, 19:33 +0200 schrieb Maxime Ripard:
> > > Mostly because Maxime and I seem to have a completely different opinion
> > > and nobody else argued one way or the other.
> > 
> > Yep, mostly because I don't see how a generic approach can work.
> > 
> > The existing reset-gpios property only provide the gpio to use, but
> > some informations are encoded in the driver, such as the reset
> > duration, or a reset sequence if any.
> 
> The driver should provide the duration.

How? This used to be in the code, and reset_control_reset doesn't take
such argument.

> I'd really like to see an example where sequencing is necessary.

Well, if you have several reset lines, the sequencing between each
might be important.

> I agree that as soon as things get significantly more complicated than
> pulsing a single GPIO, the reset-gpios binding is too limited.

While the generic reset bindings are perfect for that.

> Still, I'm not happy to mandate a separate gpio reset device for each
> reset line if most devices are simple enough for it to work without.

Well, it's pretty much already the case for other subsystems, such as
regulator.

I guess we can treat this as a legacy option, but allow the reset-gpio
code to be a full driver of its own, if we need more advanced use
cases?

> What about using reset-gpios for the majority of simple cases and have a
> separate gpio-reset-sequencer driver when multiple GPIO resets have to
> be timed?

I don't know. I feel like it should be in the driver itself, rather
than in a generic layer.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140814/36248234/attachment.sig>