All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Maxime Ripard <maxime.ripard@free-electrons.com>,
	Arnd Bergmann <arnd@arndb.de>,
	wim@iguana.be, dbaryshkov@gmail.com, dwmw2@infradead.org
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org,
	linux-sunxi@googlegroups.com
Subject: Re: [PATCH 2/5] power: reset: Add Allwinner A31 reset code
Date: Sat, 26 Apr 2014 08:32:27 -0700	[thread overview]
Message-ID: <535BD18B.2070902@roeck-us.net> (raw)
In-Reply-To: <1398265476-29373-3-git-send-email-maxime.ripard@free-electrons.com>

On 04/23/2014 08:04 AM, Maxime Ripard wrote:
> That code used to be in the machine code, but it's more fit here with other
> restart hooks.
>
> That will allow to cleanup the machine directory, while waiting for a proper
> watchdog driver for the A31.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

I am a bit lost here. Why is this a separate driver, accessing watchdog registers,
while the other reset functions are being moved into the watchdog code ?

Any chance to handle all platforms the same ? Seems to me that would be less messy.

Guenter

> ---
>   drivers/power/reset/Kconfig        |  7 ++++
>   drivers/power/reset/Makefile       |  1 +
>   drivers/power/reset/sun6i-reboot.c | 85 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 93 insertions(+)
>   create mode 100644 drivers/power/reset/sun6i-reboot.c
>
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> index fa0e4e057b99..67aeb6ec08f9 100644
> --- a/drivers/power/reset/Kconfig
> +++ b/drivers/power/reset/Kconfig
> @@ -43,6 +43,13 @@ config POWER_RESET_RESTART
>   	  Instead they restart, and u-boot holds the SoC until the
>   	  user presses a key. u-boot then boots into Linux.
>
> +config POWER_RESET_SUN6I
> +	bool "Allwinner A31 SoC reset driver"
> +	depends on ARCH_SUNXI
> +	depends on POWER_RESET
> +	help
> +	  Reboot support for the Allwinner A31 SoCs.
> +
>   config POWER_RESET_VEXPRESS
>   	bool "ARM Versatile Express power-off and reset driver"
>   	depends on ARM || ARM64
> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> index a5b4a77d1a41..950fdc011c7a 100644
> --- a/drivers/power/reset/Makefile
> +++ b/drivers/power/reset/Makefile
> @@ -3,5 +3,6 @@ obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
>   obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
>   obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
>   obj-$(CONFIG_POWER_RESET_RESTART) += restart-poweroff.o
> +obj-$(CONFIG_POWER_RESET_SUN6I) += sun6i-reboot.o
>   obj-$(CONFIG_POWER_RESET_VEXPRESS) += vexpress-poweroff.o
>   obj-$(CONFIG_POWER_RESET_XGENE) += xgene-reboot.o
> diff --git a/drivers/power/reset/sun6i-reboot.c b/drivers/power/reset/sun6i-reboot.c
> new file mode 100644
> index 000000000000..af2cd7ff2fe8
> --- /dev/null
> +++ b/drivers/power/reset/sun6i-reboot.c
> @@ -0,0 +1,85 @@
> +/*
> + * Allwinner A31 SoCs reset code
> + *
> + * Copyright (C) 2012-2014 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +
> +#include <asm/system_misc.h>
> +
> +#define SUN6I_WATCHDOG1_IRQ_REG		0x00
> +#define SUN6I_WATCHDOG1_CTRL_REG	0x10
> +#define SUN6I_WATCHDOG1_CTRL_RESTART		BIT(0)
> +#define SUN6I_WATCHDOG1_CONFIG_REG	0x14
> +#define SUN6I_WATCHDOG1_CONFIG_RESTART		BIT(0)
> +#define SUN6I_WATCHDOG1_CONFIG_IRQ		BIT(1)
> +#define SUN6I_WATCHDOG1_MODE_REG	0x18
> +#define SUN6I_WATCHDOG1_MODE_ENABLE		BIT(0)
> +
> +static void __iomem *wdt_base;
> +
> +static void sun6i_wdt_restart(enum reboot_mode mode, const char *cmd)
> +{
> +	if (!wdt_base)
> +		return;
> +
> +	/* Disable interrupts */
> +	writel(0, wdt_base + SUN6I_WATCHDOG1_IRQ_REG);
> +
> +	/* We want to disable the IRQ and just reset the whole system */
> +	writel(SUN6I_WATCHDOG1_CONFIG_RESTART,
> +		wdt_base + SUN6I_WATCHDOG1_CONFIG_REG);
> +
> +	/* Enable timer. The default and lowest interval value is 0.5s */
> +	writel(SUN6I_WATCHDOG1_MODE_ENABLE,
> +		wdt_base + SUN6I_WATCHDOG1_MODE_REG);
> +
> +	/* Restart the watchdog. */
> +	writel(SUN6I_WATCHDOG1_CTRL_RESTART,
> +		wdt_base + SUN6I_WATCHDOG1_CTRL_REG);
> +
> +	while (1) {
> +		mdelay(5);
> +		writel(SUN6I_WATCHDOG1_MODE_ENABLE,
> +			wdt_base + SUN6I_WATCHDOG1_MODE_REG);
> +	}
> +}
> +
> +static int sun6i_reboot_probe(struct platform_device *pdev)
> +{
> +	wdt_base = of_iomap(pdev->dev.of_node, 0);
> +	if (!wdt_base) {
> +		WARN(1, "failed to map watchdog base address");
> +		return -ENODEV;
> +	}
> +
> +	arm_pm_restart = sun6i_wdt_restart;
> +
> +	return 0;
> +}
> +
> +static struct of_device_id sun6i_reboot_of_match[] = {
> +	{ .compatible = "allwinner,sun6i-a31-wdt" },
> +	{}
> +};
> +
> +static struct platform_driver sun6i_reboot_driver = {
> +	.probe = sun6i_reboot_probe,
> +	.driver = {
> +		.name = "sun6i-reboot",
> +		.of_match_table = sun6i_reboot_of_match,
> +	},
> +};
> +module_platform_driver(sun6i_reboot_driver);
>


WARNING: multiple messages have this Message-ID (diff)
From: linux@roeck-us.net (Guenter Roeck)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/5] power: reset: Add Allwinner A31 reset code
Date: Sat, 26 Apr 2014 08:32:27 -0700	[thread overview]
Message-ID: <535BD18B.2070902@roeck-us.net> (raw)
In-Reply-To: <1398265476-29373-3-git-send-email-maxime.ripard@free-electrons.com>

On 04/23/2014 08:04 AM, Maxime Ripard wrote:
> That code used to be in the machine code, but it's more fit here with other
> restart hooks.
>
> That will allow to cleanup the machine directory, while waiting for a proper
> watchdog driver for the A31.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

I am a bit lost here. Why is this a separate driver, accessing watchdog registers,
while the other reset functions are being moved into the watchdog code ?

Any chance to handle all platforms the same ? Seems to me that would be less messy.

Guenter

> ---
>   drivers/power/reset/Kconfig        |  7 ++++
>   drivers/power/reset/Makefile       |  1 +
>   drivers/power/reset/sun6i-reboot.c | 85 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 93 insertions(+)
>   create mode 100644 drivers/power/reset/sun6i-reboot.c
>
> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
> index fa0e4e057b99..67aeb6ec08f9 100644
> --- a/drivers/power/reset/Kconfig
> +++ b/drivers/power/reset/Kconfig
> @@ -43,6 +43,13 @@ config POWER_RESET_RESTART
>   	  Instead they restart, and u-boot holds the SoC until the
>   	  user presses a key. u-boot then boots into Linux.
>
> +config POWER_RESET_SUN6I
> +	bool "Allwinner A31 SoC reset driver"
> +	depends on ARCH_SUNXI
> +	depends on POWER_RESET
> +	help
> +	  Reboot support for the Allwinner A31 SoCs.
> +
>   config POWER_RESET_VEXPRESS
>   	bool "ARM Versatile Express power-off and reset driver"
>   	depends on ARM || ARM64
> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
> index a5b4a77d1a41..950fdc011c7a 100644
> --- a/drivers/power/reset/Makefile
> +++ b/drivers/power/reset/Makefile
> @@ -3,5 +3,6 @@ obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
>   obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
>   obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
>   obj-$(CONFIG_POWER_RESET_RESTART) += restart-poweroff.o
> +obj-$(CONFIG_POWER_RESET_SUN6I) += sun6i-reboot.o
>   obj-$(CONFIG_POWER_RESET_VEXPRESS) += vexpress-poweroff.o
>   obj-$(CONFIG_POWER_RESET_XGENE) += xgene-reboot.o
> diff --git a/drivers/power/reset/sun6i-reboot.c b/drivers/power/reset/sun6i-reboot.c
> new file mode 100644
> index 000000000000..af2cd7ff2fe8
> --- /dev/null
> +++ b/drivers/power/reset/sun6i-reboot.c
> @@ -0,0 +1,85 @@
> +/*
> + * Allwinner A31 SoCs reset code
> + *
> + * Copyright (C) 2012-2014 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +
> +#include <asm/system_misc.h>
> +
> +#define SUN6I_WATCHDOG1_IRQ_REG		0x00
> +#define SUN6I_WATCHDOG1_CTRL_REG	0x10
> +#define SUN6I_WATCHDOG1_CTRL_RESTART		BIT(0)
> +#define SUN6I_WATCHDOG1_CONFIG_REG	0x14
> +#define SUN6I_WATCHDOG1_CONFIG_RESTART		BIT(0)
> +#define SUN6I_WATCHDOG1_CONFIG_IRQ		BIT(1)
> +#define SUN6I_WATCHDOG1_MODE_REG	0x18
> +#define SUN6I_WATCHDOG1_MODE_ENABLE		BIT(0)
> +
> +static void __iomem *wdt_base;
> +
> +static void sun6i_wdt_restart(enum reboot_mode mode, const char *cmd)
> +{
> +	if (!wdt_base)
> +		return;
> +
> +	/* Disable interrupts */
> +	writel(0, wdt_base + SUN6I_WATCHDOG1_IRQ_REG);
> +
> +	/* We want to disable the IRQ and just reset the whole system */
> +	writel(SUN6I_WATCHDOG1_CONFIG_RESTART,
> +		wdt_base + SUN6I_WATCHDOG1_CONFIG_REG);
> +
> +	/* Enable timer. The default and lowest interval value is 0.5s */
> +	writel(SUN6I_WATCHDOG1_MODE_ENABLE,
> +		wdt_base + SUN6I_WATCHDOG1_MODE_REG);
> +
> +	/* Restart the watchdog. */
> +	writel(SUN6I_WATCHDOG1_CTRL_RESTART,
> +		wdt_base + SUN6I_WATCHDOG1_CTRL_REG);
> +
> +	while (1) {
> +		mdelay(5);
> +		writel(SUN6I_WATCHDOG1_MODE_ENABLE,
> +			wdt_base + SUN6I_WATCHDOG1_MODE_REG);
> +	}
> +}
> +
> +static int sun6i_reboot_probe(struct platform_device *pdev)
> +{
> +	wdt_base = of_iomap(pdev->dev.of_node, 0);
> +	if (!wdt_base) {
> +		WARN(1, "failed to map watchdog base address");
> +		return -ENODEV;
> +	}
> +
> +	arm_pm_restart = sun6i_wdt_restart;
> +
> +	return 0;
> +}
> +
> +static struct of_device_id sun6i_reboot_of_match[] = {
> +	{ .compatible = "allwinner,sun6i-a31-wdt" },
> +	{}
> +};
> +
> +static struct platform_driver sun6i_reboot_driver = {
> +	.probe = sun6i_reboot_probe,
> +	.driver = {
> +		.name = "sun6i-reboot",
> +		.of_match_table = sun6i_reboot_of_match,
> +	},
> +};
> +module_platform_driver(sun6i_reboot_driver);
>

  parent reply	other threads:[~2014-04-26 15:32 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-23 15:04 [PATCH 0/5] ARM: sunxi: Machine code cleanup Maxime Ripard
2014-04-23 15:04 ` Maxime Ripard
2014-04-23 15:04 ` [PATCH 1/5] wdt: sunxi: Move restart code to the watchdog driver Maxime Ripard
2014-04-23 15:04   ` Maxime Ripard
2014-04-23 15:57   ` Arnd Bergmann
2014-04-23 15:57     ` Arnd Bergmann
2014-04-26 15:31   ` Guenter Roeck
2014-04-26 15:31     ` Guenter Roeck
2014-04-28 23:07     ` Maxime Ripard
2014-04-28 23:07       ` Maxime Ripard
2014-04-23 15:04 ` [PATCH 2/5] power: reset: Add Allwinner A31 reset code Maxime Ripard
2014-04-23 15:04   ` Maxime Ripard
2014-04-23 15:58   ` Arnd Bergmann
2014-04-23 15:58     ` Arnd Bergmann
2014-04-26 15:32   ` Guenter Roeck [this message]
2014-04-26 15:32     ` Guenter Roeck
2014-04-28 23:11     ` Maxime Ripard
2014-04-28 23:11       ` Maxime Ripard
2014-04-23 15:04 ` [PATCH 3/5] ARM: sunxi: Select restart drivers Maxime Ripard
2014-04-23 15:04   ` Maxime Ripard
2014-04-23 16:00   ` Arnd Bergmann
2014-04-23 16:00     ` Arnd Bergmann
2014-04-24 12:57     ` Maxime Ripard
2014-04-24 12:57       ` Maxime Ripard
2014-04-23 15:04 ` [PATCH 4/5] ARM: sunxi: Remove reset code from the platform Maxime Ripard
2014-04-23 15:04   ` Maxime Ripard
2014-04-23 16:00   ` Arnd Bergmann
2014-04-23 16:00     ` Arnd Bergmann
2014-04-23 15:04 ` [PATCH 5/5] ARM: sunxi: Remove sun4i and sun7i machine definitions Maxime Ripard
2014-04-23 15:04   ` Maxime Ripard
2014-04-23 16:02   ` Arnd Bergmann
2014-04-23 16:02     ` Arnd Bergmann
2014-04-24 12:59     ` Maxime Ripard
2014-04-24 12:59       ` Maxime Ripard
2014-04-29  1:08     ` Olof Johansson
2014-04-29  1:08       ` Olof Johansson
2014-04-30 18:31       ` Maxime Ripard
2014-04-30 18:31         ` Maxime Ripard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=535BD18B.2070902@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=arnd@arndb.de \
    --cc=dbaryshkov@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sunxi@googlegroups.com \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=maxime.ripard@free-electrons.com \
    --cc=wim@iguana.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.