From: stefan@agner.ch (Stefan Agner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
Date: Fri, 28 Nov 2014 23:09:09 +0100 [thread overview]
Message-ID: <6341b31998ff843d236bc2a3e34537b0@agner.ch> (raw)
In-Reply-To: <2716009.DOfHfaSPkY@wuerfel>
On 2014-11-28 22:24, Arnd Bergmann wrote:
> On Friday 28 November 2014 22:02:01 Stefan Agner wrote:
>> On 2014-11-28 17:49, Arnd Bergmann wrote:
>> > On Friday 28 November 2014 17:43:35 Stefan Agner wrote:
>> >> Support Vybrid SoC's system reset controller (SRC). Currently we
>> >> don't register a reset controller but only support the imx_cpu_jump
>> >> and imx_cpu_arg functions.
>> >>
>> >> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> >
>> > I think this should be a platform driver in drivers/power/reset.
>>
>> Yeah, I thought that too, see my cover letter. The problem is, in that
>> module are also some register which are of interest when implementing
>> suspend/resume support (see cover letter too). However, we could also
>> just make a dt entry for that reset register only, and create another dt
>> entry for the other registers.
>
> Don't make a node with just one register, in this case, a syscon device
> would be best.
Syscon seems like a match here. Was not aware of that, thx!
>
>> > If the SRC is also capable of resetting individual blocks instead of just
>> > the entire machine, it would be a reset driver in drivers/reset instead.
>>
>> Beside the system reset, there is only a mask functionality for the
>> watchdogs (there are two watchdogs, one for Cortex-A5 and one for the
>> M4). This makes the SRC module in the Vybrid a bit different then what
>> is available on other i.MX SoC's...
>
> If you already have the watchdog registers in there and want to have
> a watchdog driver too, the easiest way would be to register the reboot
> handler from the watchdog driver.
Hm, not sure we speak about the same here. The SRC module has two
(multi-)bit fields to mask the watchdog reset event for each watchdog.
Beside that, there are two full watchdog register maps, which are in
different areas. There is already a driver for this watchdogs. I'm not
sure what the idea behind this is exactly, I guess it would easily allow
to (temporary) mask the other CPU's watchdog. However, I don't think we
need that functionality, so I don't care about that right now.
There is also a restart handler in the watchdog driver, but I prefer to
use the reset capabilities of the SRC since it has immediate effect.
Lets get to the big picture again: I could register the whole SRC
register map as a syscon device and then access the registers from my
suspend/resume implementation later on. And similar in the restart
driver, I would use syscon_regmap_lookup_by_compatible to check if it
contains the vf610-src compatible string and register the restart
driver/handler if available.
--
Stefan
WARNING: multiple messages have this Message-ID (diff)
From: Stefan Agner <stefan@agner.ch>
To: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arm-kernel@lists.infradead.org, shawn.guo@linaro.org,
kernel@pengutronix.de, linux@roeck-us.net,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
Date: Fri, 28 Nov 2014 23:09:09 +0100 [thread overview]
Message-ID: <6341b31998ff843d236bc2a3e34537b0@agner.ch> (raw)
In-Reply-To: <2716009.DOfHfaSPkY@wuerfel>
On 2014-11-28 22:24, Arnd Bergmann wrote:
> On Friday 28 November 2014 22:02:01 Stefan Agner wrote:
>> On 2014-11-28 17:49, Arnd Bergmann wrote:
>> > On Friday 28 November 2014 17:43:35 Stefan Agner wrote:
>> >> Support Vybrid SoC's system reset controller (SRC). Currently we
>> >> don't register a reset controller but only support the imx_cpu_jump
>> >> and imx_cpu_arg functions.
>> >>
>> >> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> >
>> > I think this should be a platform driver in drivers/power/reset.
>>
>> Yeah, I thought that too, see my cover letter. The problem is, in that
>> module are also some register which are of interest when implementing
>> suspend/resume support (see cover letter too). However, we could also
>> just make a dt entry for that reset register only, and create another dt
>> entry for the other registers.
>
> Don't make a node with just one register, in this case, a syscon device
> would be best.
Syscon seems like a match here. Was not aware of that, thx!
>
>> > If the SRC is also capable of resetting individual blocks instead of just
>> > the entire machine, it would be a reset driver in drivers/reset instead.
>>
>> Beside the system reset, there is only a mask functionality for the
>> watchdogs (there are two watchdogs, one for Cortex-A5 and one for the
>> M4). This makes the SRC module in the Vybrid a bit different then what
>> is available on other i.MX SoC's...
>
> If you already have the watchdog registers in there and want to have
> a watchdog driver too, the easiest way would be to register the reboot
> handler from the watchdog driver.
Hm, not sure we speak about the same here. The SRC module has two
(multi-)bit fields to mask the watchdog reset event for each watchdog.
Beside that, there are two full watchdog register maps, which are in
different areas. There is already a driver for this watchdogs. I'm not
sure what the idea behind this is exactly, I guess it would easily allow
to (temporary) mask the other CPU's watchdog. However, I don't think we
need that functionality, so I don't care about that right now.
There is also a restart handler in the watchdog driver, but I prefer to
use the reset capabilities of the SRC since it has immediate effect.
Lets get to the big picture again: I could register the whole SRC
register map as a syscon device and then access the registers from my
suspend/resume implementation later on. And similar in the restart
driver, I would use syscon_regmap_lookup_by_compatible to check if it
contains the vf610-src compatible string and register the restart
driver/handler if available.
--
Stefan
next prev parent reply other threads:[~2014-11-28 22:09 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-28 16:43 [PATCH 0/2] ARM: imx: src: vf610 system reset controller Stefan Agner
2014-11-28 16:43 ` Stefan Agner
2014-11-28 16:43 ` [PATCH 1/2] ARM: dts: vf610: add system reset controller (SRC) Stefan Agner
2014-11-28 16:43 ` Stefan Agner
2014-11-28 16:43 ` [PATCH 2/2] ARM: imx: src: support vf610 system reset controller Stefan Agner
2014-11-28 16:43 ` Stefan Agner
2014-11-28 16:49 ` Arnd Bergmann
2014-11-28 16:49 ` Arnd Bergmann
2014-11-28 21:02 ` Stefan Agner
2014-11-28 21:02 ` Stefan Agner
2014-11-28 21:24 ` Arnd Bergmann
2014-11-28 21:24 ` Arnd Bergmann
2014-11-28 22:09 ` Stefan Agner [this message]
2014-11-28 22:09 ` Stefan Agner
2014-11-28 22:22 ` Arnd Bergmann
2014-11-28 22:22 ` Arnd Bergmann
2014-11-28 23:00 ` Stefan Agner
2014-11-28 23:00 ` Stefan Agner
2014-11-28 23:10 ` Guenter Roeck
2014-11-28 23:10 ` Guenter Roeck
2014-11-29 0:15 ` Stefan Agner
2014-11-29 0:15 ` Stefan Agner
2014-11-30 11:54 ` Arnd Bergmann
2014-11-30 11:54 ` Arnd Bergmann
2014-11-30 20:02 ` Stefan Agner
2014-11-30 20:02 ` Stefan Agner
2014-11-28 17:57 ` Guenter Roeck
2014-11-28 17:57 ` Guenter Roeck
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=6341b31998ff843d236bc2a3e34537b0@agner.ch \
--to=stefan@agner.ch \
--cc=linux-arm-kernel@lists.infradead.org \
/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.