Hi Sebastian,
On Thu, 13 Aug 2020 at 02:28, Sebastian Reichel sebastian.reichel@collabora.com wrote:
Add GPIO poweroff driver, which is based on the Linux driver and uses the same DT binding.
Signed-off-by: Sebastian Reichel sebastian.reichel@collabora.com
drivers/sysreset/Kconfig | 6 +++ drivers/sysreset/Makefile | 1 + drivers/sysreset/poweroff_gpio.c | 84 ++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 drivers/sysreset/poweroff_gpio.c
Reviewed-by: Simon Glass sjg@chromium.org
nits below
diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig index 6ebc90e1d33b..82af8db47f65 100644 --- a/drivers/sysreset/Kconfig +++ b/drivers/sysreset/Kconfig @@ -43,6 +43,12 @@ config SYSRESET_CMD_POWEROFF
endif
+config POWEROFF_GPIO
bool "Enable support for GPIO poweroff driver"select DM_GPIOhelpPoweroff support via GPIO pin connected reset logic.
Does this mean GPIO-pin-connected reset logic? Can you please add the hyphens or rephrase to clarify the meaning?
Also could use a bit more detail.
config SYSRESET_GPIO bool "Enable support for GPIO reset driver" select DM_GPIO diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile index df2293b8489d..90a9b26abef4 100644 --- a/drivers/sysreset/Makefile +++ b/drivers/sysreset/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_ARCH_ASPEED) += sysreset_ast.o obj-$(CONFIG_ARCH_ROCKCHIP) += sysreset_rockchip.o obj-$(CONFIG_ARCH_STI) += sysreset_sti.o obj-$(CONFIG_SANDBOX) += sysreset_sandbox.o +obj-$(CONFIG_POWEROFF_GPIO) += poweroff_gpio.o obj-$(CONFIG_SYSRESET_GPIO) += sysreset_gpio.o obj-$(CONFIG_SYSRESET_MPC83XX) += sysreset_mpc83xx.o obj-$(CONFIG_SYSRESET_MICROBLAZE) += sysreset_microblaze.o diff --git a/drivers/sysreset/poweroff_gpio.c b/drivers/sysreset/poweroff_gpio.c new file mode 100644 index 000000000000..0eb2426d7574 --- /dev/null +++ b/drivers/sysreset/poweroff_gpio.c @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Toggles a GPIO pin to power down a device
- Created using the Linux driver as reference, which
- has been written by:
- Jamie Lentin jm@lentin.co.uk
- Andrew Lunn andrew@lunn.ch
- Copyright (C) 2012 Jamie Lentin
- */
+#include <common.h> +#include <asm/gpio.h>
Check order.
+#include <dm.h> +#include <errno.h> +#include <linux/delay.h> +#include <log.h> +#include <sysreset.h>
+struct poweroff_gpio_info {
struct gpio_desc gpio;u32 active_delay;u32 inactive_delay;u32 timeout;
timeout_ms so units are clear?
+};
+static int poweroff_gpio_request(struct udevice *dev, enum sysreset_t type) +{
struct poweroff_gpio_info *priv = dev_get_priv(dev);if (type != SYSRESET_POWER_OFF)return -ENOSYS;debug("GPIO poweroff\n");/* drive it active, also inactive->active edge */dm_gpio_set_value(&priv->gpio, 1);
Should check return values in this function
mdelay(priv->active_delay);/* drive inactive, also active->inactive edge */dm_gpio_set_value(&priv->gpio, 0);mdelay(priv->inactive_delay);/* drive it active, also inactive->active edge */dm_gpio_set_value(&priv->gpio, 1);/* give it some time */mdelay(priv->timeout);return -ETIMEDOUT;
-EINPROGRESS since it may still happen later, right?
+}
+static int poweroff_gpio_probe(struct udevice *dev) +{
struct poweroff_gpio_info *priv = dev_get_priv(dev);int flags;flags = dev_read_bool(dev, "input") ? GPIOD_IS_IN : GPIOD_IS_OUT;priv->active_delay = dev_read_u32_default(dev, "active-delay-ms", 100);priv->inactive_delay = dev_read_u32_default(dev, "inactive-delay-ms", 100);priv->timeout = dev_read_u32_default(dev, "timeout-ms", 3000);return gpio_request_by_name(dev, "gpios", 0, &priv->gpio, flags);+}
+static struct sysreset_ops poweroff_gpio_ops = {
.request = poweroff_gpio_request,+};
+static const struct udevice_id poweroff_gpio_ids[] = {
{ .compatible = "gpio-poweroff", },{},+};
+U_BOOT_DRIVER(poweroff_gpio) = {
.name = "poweroff-gpio",.id = UCLASS_SYSRESET,.ops = &poweroff_gpio_ops,.probe = poweroff_gpio_probe,.priv_auto_alloc_size = sizeof(struct poweroff_gpio_info),.of_match = poweroff_gpio_ids,+};
2.28.0
Regards, Simon