linux-kernel - Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <547900D2.6050404@roeck-us.net>
Date:	Fri, 28 Nov 2014 15:10:10 -0800
From:	Guenter Roeck <linux@...ck-us.net>
To:	Stefan Agner <stefan@...er.ch>, Arnd Bergmann <arnd@...db.de>
CC:	linux-arm-kernel@...ts.infradead.org, shawn.guo@...aro.org,
	kernel@...gutronix.de, linux-kernel@...r.kernel.org, fkan@....com
Subject: Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller

On 11/28/2014 03:00 PM, Stefan Agner wrote:
> On 2014-11-28 23:22, Arnd Bergmann wrote:
>> On Friday 28 November 2014 23:09:09 Stefan Agner wrote:
>>> On 2014-11-28 22:24, Arnd Bergmann wrote:
>>>> On Friday 28 November 2014 22:02:01 Stefan Agner wrote:
>>>>
>>>>>> 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.
>>
>> Ok, I see, thanks for the clarification!
>>
>>> 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.
>>
>> Correct, and also in the watchdog driver, I guess.
>>
>> Instead of syscon_regmap_lookup_by_compatible, please use
>> syscon_regmap_lookup_by_phandle and put the link to the syscon
>> device into the device accessing it.
>>
>> Also, see if you can use or extend drivers/power/reset/syscon-reboot.c
>> for your use case.
>
> Nice, this allows to do the reset with almost no code. The only thing
> which might be a problem is the priority: I used 192 since 128 is
> already used by the watchdog driver. Maybe we can change that
> syscon-reboot is a bit above watchdogs by default, or maybe even a dt
> property? I will try to use that driver for v2. Thx!
>

A dt property might make more sense if the syscon driver is to be used
for multiple systems. Otherwise, no matter what you pick, it will be
wrong for some system.

Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/