From: sr@denx•de (Stefan Roese)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 4/4] ARM: sunxi: Add sunxi restart function via onchip watchdog
Date: Mon, 19 Nov 2012 17:13:27 +0100 [thread overview]
Message-ID: <50AA5AA7.9070705@denx.de> (raw)
In-Reply-To: <50AA53A0.2040404@free-electrons.com>
On 11/19/2012 04:43 PM, Maxime Ripard wrote:
> While I've taken the three other patches, I'm more concerned about this one.
>
> On a general basis, I would rather see this code into the machine source
> file, since it doesn't directly relate to the timer driver. If at some
> point someone wants to write a proper watchdog driver alongside with the
> timer driver, I'll be fine with moving the code there, but for now,
> let's keep it simple.
I thought about that too. But it would either a) result in code
duplication (finding and mapping this timer device node) or b) we would
have to make this "timer_base" an ugly global variable so that we can
reference it from the platform code. That's why I placed it the timer
source.
<snip>
>> +void sunxi_restart(char mode, const char *cmd)
>> +{
>> + /* Use watchdog to reset system */
>> +
>> + /* Enable timer and set reset bit */
>> + writel(3, timer_base + WATCH_DOG_MODE_REG);
>> + writel(0xa57 << 1 | 1, timer_base + WATCH_DOG_CTRL_REG);
>> +
>> + while(1)
>> + ;
>> +}
>
> Here, your code looks way more obscure and "complicated" than the one
> from linux-sunxi found here:
> https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/arch/arm/mach-sun4i/core.c#L289
>
> Why is that?
"Way more obscure" sounds a bit exaggerated for 3 lines of code. ;)
The delays have been removed as they are not necessary. So its even
"cleaner" than the code that you referenced. And the code at linux-sunxi
also has incorrect register definitions (CTRL_REG is not 0x94 but 0x90).
The main difference is the added write to the real CTRL_REG (0x90) to
rearm the wdog. This has been suggested by Henrik Norstr?m, who has done
most of the U-Boot work (preparing for mainlining as well).
So since Arnd seem to be fine with this patch, I suggest to leave it as
is for now.
Thanks,
Stefan
next prev parent reply other threads:[~2012-11-19 16:13 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-19 11:09 [PATCH 1/4] ARM: sunxi: Restructure sunxi dts/dtsi files Stefan Roese
2012-11-19 11:09 ` [PATCH 2/4] ARM: sunxi: Add earlyprintk support for UART0 (sun4i) Stefan Roese
2012-11-19 15:22 ` Maxime Ripard
2012-11-19 15:53 ` Arnd Bergmann
2012-11-19 11:09 ` [PATCH 3/4] ARM: sunxi: Add sun4i and cubieboard support Stefan Roese
2012-11-19 15:22 ` Maxime Ripard
2012-11-19 15:53 ` Arnd Bergmann
2012-11-19 11:09 ` [PATCH 4/4] ARM: sunxi: Add sunxi restart function via onchip watchdog Stefan Roese
2012-11-19 15:43 ` Maxime Ripard
2012-11-19 16:13 ` Stefan Roese [this message]
2012-11-19 15:55 ` Arnd Bergmann
2012-11-19 15:22 ` [PATCH 1/4] ARM: sunxi: Restructure sunxi dts/dtsi files Maxime Ripard
2012-11-19 15:52 ` Arnd Bergmann
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=50AA5AA7.9070705@denx.de \
--to=sr@denx$(echo .)de \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox