From: Guenter Roeck <linux@roeck-us.net>
To: Jonas Jensen <jonas.jensen@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"arm@kernel.org" <arm@kernel.org>,
Russell King - ARM Linux <linux@arm.linux.org.uk>,
Olof Johansson <olof@lixom.net>
Subject: Re: [PATCH v4 1/2] ARM: mach-moxart: add MOXA ART SoC platform files
Date: Sat, 14 Dec 2013 07:50:25 -0800 [thread overview]
Message-ID: <52AC7E41.7050201@roeck-us.net> (raw)
In-Reply-To: <CACmBeS2_WZj6_HJo0N50TBwneNVmeB9TyXbHRLm9CJFGNN+HtQ@mail.gmail.com>
On 12/14/2013 12:32 AM, Jonas Jensen wrote:
> On 13 December 2013 20:07, Guenter Roeck <linux@roeck-us.net> wrote:
>>> I got the impression from Guenter Roeck's review, that it doesn't belong there,
>>> maybe I was too quick to remove it?
>>>
>> You'd have to answer the questions I raised in my review if you want it in
>> there.
>
> I didn't see a solution at the time. Now I'm thinking arm_pm_restart
> can be set later in probe, preventing a crash on load failure, and
> maybe it can be unset on unload.
>
> Would you accept a patch for this?
>
The above would at least avoid the crash, though I would not understand
the point of having an unloadable restart handler. Forcing the watchdog
driver into the kernel just because you want the restart handler in it
would seem odd. And if the restart handler is optional, why have it
in the first place ? I also don't follow Arnd's logic of wanting to have
the code in the watchdog driver just because it happens to use a register
that it needs.
Conceptually it might be cleaner to write a separate driver, for example
in drivers/power/restart, than plugging the functionality into
the watchdog driver, at least if you don't want it in architecture
or platform code. The xgene restart driver is a good example.
Anyway, it is not up to me to accept the code, that is up to Wim.
My rejection was primarily due to technical flaws, which can be addressed.
For the logical reasoning you'll have to convince Wim.
Guenter
next prev parent reply other threads:[~2013-12-14 15:50 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-13 14:33 [PATCH v4 0/2] ARM: mach-moxart: add MOXA ART SoC support Jonas Jensen
2013-12-13 14:33 ` [PATCH v4 1/2] ARM: mach-moxart: add MOXA ART SoC platform files Jonas Jensen
2013-12-13 16:17 ` Arnd Bergmann
2013-12-13 17:23 ` Jonas Jensen
2013-12-13 19:07 ` Guenter Roeck
2013-12-14 4:01 ` Arnd Bergmann
2013-12-14 8:32 ` Jonas Jensen
2013-12-14 15:50 ` Guenter Roeck [this message]
2013-12-14 16:26 ` Jonas Jensen
2013-12-14 18:50 ` Arnd Bergmann
2013-12-14 20:14 ` Guenter Roeck
2013-12-15 0:21 ` Arnd Bergmann
2013-12-22 19:48 ` Olof Johansson
2013-12-13 14:33 ` [PATCH v4 2/2] ARM: mach-moxart: add MOXA ART SoC device tree files Jonas Jensen
2013-12-13 16:17 ` Arnd Bergmann
2013-12-15 4:27 ` Peter Crosthwaite
2013-12-16 20:05 ` Jonas Jensen
2013-12-16 23:53 ` Peter Crosthwaite
2013-12-17 0:09 ` Sören Brinkmann
2013-12-22 19:49 ` Olof Johansson
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=52AC7E41.7050201@roeck-us.net \
--to=linux@roeck-us.net \
--cc=arm@kernel.org \
--cc=arnd@arndb.de \
--cc=jonas.jensen@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=olof@lixom.net \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox