reset: Add driver for gpio-controlled reset pins
Subsystems (2)
Philipp Zabel
maintainerPATCH v7 1/1 reset: Add driver for gpio-controlled reset pins
This driver implements a reset controller device that toggle a gpio connected to a reset pin of a peripheral IC. The delay between assertion and de-assertion of the reset signal can be configured via device tree.
---
Changes since v6:
- Switched to a single gpio per gpio-reset device, as Olof suggested. This should make the device tree binding more straightforward.
- Changed reset delay property to reset-delay-us.
--- .../devicetree/bindings/reset/gpio-reset.txt | 35 +++++ drivers/reset/Kconfig | 11 ++ drivers/reset/Makefile | 1 + drivers/reset/gpio-reset.c | 171 +++++++++++++++++++++ 4 files changed, 218 insertions(+) create mode 100644 Documentation/devicetree/bindings/reset/gpio-reset.txt create mode 100644 drivers/reset/gpio-reset.c
Documentation/devicetree/bindings/reset/gpio-reset.txt new file +35 -0
@@ -0,0 +1,35 @@ +GPIO reset controller +===================== + +A GPIO reset controller controls a single GPIO that is connected to the reset +pin of a peripheral IC. Please also refer to reset.txt in this directory for +common reset controller binding usage. + +Required properties: +- compatible: Should be "gpio-reset" +- reset-gpios: A gpio used as reset line. The gpio specifier for this property + depends on the gpio controller that provides the gpio. +- #reset-cells: 0, see below + +Optional properties: +- reset-delay-us: delay in microseconds. The gpio reset line will be asserted for + this duration to reset. +- initially-in-reset: boolean. If not set, the initial state should be a + deasserted reset line. If this property exists, the + reset line should be kept in reset. + +example: + +sii902x_reset: gpio-reset { + compatible = "gpio-reset"; + reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; + reset-delay-us = <10000>; + initially-in-reset; + #reset-cells = <0>; +}; + +/* Device with nRESET pin connected to GPIO5_0 */ +sii902x at 39 { + /* ... */ + resets = <&sii902x_reset>; /* active-low GPIO5_0, 10 ms delay */ +};
drivers/reset/Kconfig +11 -0
@@ -11,3 +11,14 @@ menuconfig RESET_CONTROLLER via GPIOs or SoC-internal reset controller modules. If unsure, say no. + +if RESET_CONTROLLER + +config RESET_GPIO + tristate "GPIO reset controller support" + depends on GPIOLIB && OF + help + This driver provides support for reset lines that are controlled + directly by GPIOs. + +endif
drivers/reset/Makefile +1 -0
@@ -1 +1,2 @@ obj-$(CONFIG_RESET_CONTROLLER) += core.o +obj-$(CONFIG_RESET_GPIO) += gpio-reset.o
drivers/reset/gpio-reset.c new file +169 -0
169 lines not on this page · raw message
Signature · 1 line
1.8.2.rc2
PATCH v7 1/1 reset: Add driver for gpio-controlled reset pins
drivers/reset/gpio-reset.c
Quoted 7 lines · Philipp Zabel
This driver implements a reset controller device that toggle a gpio connected to a reset pin of a peripheral IC. The delay between assertion and de-assertion of the reset signal can be configured via device tree.
drivers/reset/gpio-reset.c +1
-0+static void __gpio_reset_set (struct reset_controller_dev *rcdev,int asserted)
Nit: Technically I think __ is a reserved name-space, so you shouldn't prefix "user-level" (rather than libc/similar) symbols with it, but it's not a big deal.
drivers/reset/gpio-reset.c line 98
Quoted 4 lines
drivers/reset/gpio-reset.c +3
-0+static int gpio_reset_probe(struct platform_device *pdev) + if (of_gpio_named_count(np, "reset-gpios") != 1) + return -EINVAL;
Should that error-path (and the path for of_property_read_u32() failure) include a dev_err() like some already do, so it's obvious what the failure is?
drivers/reset/gpio-reset.c line 134
Quoted 3 lines
drivers/reset/gpio-reset.c +3
-0+reset_controller_register (&drvdata->rcdev); + +platform_set_drvdata (pdev, drvdata);
It might be better to set the drvdata right after it's been allocated. It perhaps doesn't matter much here, but if the reset controller core ever starts calling into the registered device inside the call to reset_controller_register(), then there might be a problem.
Aside from those minor issues,
Philipp Zabel
maintainerPATCH v7 1/1 reset: Add driver for gpio-controlled reset pins
Hi Stephen,
Am Mittwoch, den 29.05.2013, 17:19 -0600 schrieb Stephen Warren:
Quoted 5 lines · Philipp Zabel
Quoted 3 lines
This driver implements a reset controller device that toggle a gpio connected to a reset pin of a peripheral IC. The delay between assertion and de-assertion of the reset signal can be configured via device tree.
Quoted 7 lines
drivers/reset/gpio-reset.c
Quoted 3 lines
drivers/reset/gpio-reset.c +1
-0+static void __gpio_reset_set (struct reset_controller_dev *rcdev,int asserted)Nit: Technically I think __ is a reserved name-space, so you shouldn't prefix "user-level" (rather than libc/similar) symbols with it, but it's not a big deal.
I'll s/__//.
Quoted 8 lines
drivers/reset/gpio-reset.c line 98
Quoted 4 lines
drivers/reset/gpio-reset.c +3
-0+static int gpio_reset_probe(struct platform_device *pdev) + if (of_gpio_named_count(np, "reset-gpios") != 1) + return -EINVAL;Should that error-path (and the path for
of_property_read_u32()failure) include adev_err()like some already do, so it's obvious what the failure is?
For reset-gpios, yes. reset-delay-us is marked as optional in the bindings.
Quoted 8 lines
drivers/reset/gpio-reset.c line 134
Quoted 3 lines
drivers/reset/gpio-reset.c +3
-0+reset_controller_register (&drvdata->rcdev); + +platform_set_drvdata (pdev, drvdata);It might be better to set the drvdata right after it's been allocated. It perhaps doesn't matter much here, but if the reset controller core ever starts calling into the registered device inside the call to
reset_controller_register(), then there might be a problem.
Alright. I'll move ist above the reset_controller_register() call.
Quoted 2 lines
Aside from those minor issues,
Reviewed-by Stephen Warren <[email protected]>
thanks
Philipp
PATCH v7 1/1 reset: Add driver for gpio-controlled reset pins
In reply to Stephen WarrenQuoted 5 lines · Stephen Warren
Quoted 3 lines · Philipp Zabel
This driver implements a reset controller device that toggle a gpio connected to a reset pin of a peripheral IC. The delay between assertion and de-assertion of the reset signal can be configured via device tree.
Quoted 7 lines
drivers/reset/gpio-reset.c
Quoted 3 lines
drivers/reset/gpio-reset.c +1
-0+static void __gpio_reset_set (struct reset_controller_dev *rcdev,int asserted)Nit: Technically I think __ is a reserved name-space, so you shouldn't prefix "user-level" (rather than libc/similar) symbols with it, but it's not a big deal.
Well.. this is kernel :-). We don't have libc around... and this standard is normally ignored here. __ is normally used as a prefix for "simpler" function.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
PATCH v7 1/1 reset: Add driver for gpio-controlled reset pins
Quoted 10 lines · Pavel Machek
Quoted 9 lines · Stephen Warren
Quoted 7 lines · Philipp Zabel
This driver implements a reset controller device that toggle a gpio connected to a reset pin of a peripheral IC. The delay between assertion and de-assertion of the reset signal can be configured via device tree.
drivers/reset/gpio-reset.c +1
-0+static void __gpio_reset_set (struct reset_controller_dev *rcdev,int asserted)
Quoted 7 lines
Quoted 3 lines
Nit: Technically I think __ is a reserved name-space, so you shouldn't prefix "user-level" (rather than libc/similar) symbols with it, but it's not a big deal.
Well.. this is kernel :-). We don't have libc around... and this standard is normally ignored here. __ is normally used as a prefix for "simpler" function.
We're not really consistent in the kernel, but I agree the most common use is where you have a wrapper calling the __foo function and normal drivers are supposed to call foo instead. My reading is usually that by calling __foo, the caller says "I know exactly what I'm doing so I can optimize this".
Arnd