On 2019-11-28 6:34 a.m., Harald Seiler wrote:
> Hello Claudius,
> 
> On Thu, 2019-11-28 at 13:06 +0100, Claudius Heine wrote:
>> Hi,
>>
>> currently the reset on the DHCOM i.MX6 board is brocken in u-boot.
>>
>> This patchset fixes that by integrating the sysreset and watchdog dm driver.
> 
> I think you should clarify that reset was broken by commit f2929d11a639
> ("watchdog: imx: Use immediate reset bits for expire_now") which changed
> reset to, by default, only assert the external reset [1].  DHCOM i.MX6
> needs the internal reset though, which previously was asserted as as
> well.  Maybe you can add a `Fixes:` line to one of your commits?

The behavior of the driver in the DM mode should match what the Linux
IMX watchdog driver is doing for both the watchdog timeout and reset
operations. The reset operation there explicitly uses either internal
reset or external reset, not both. For the actual watchdog timeout, they
both set the WDT bit or not depending on whether ext-reset is set, which
it seems would result in either internal+external reset or just internal
reset (it doesn't look like you can trigger only an external reset on
timeout).

> 
> Additionally, I am still unsure whether the current default behavior is
> correct.  I'd rather assert both external and internal reset, which is
> what the i.MX watchdog does on timeout as well (as long as WDT bit is
> set, which we do by default [2]).  There is also an inconsistency
> between the non-DM implementation (external by default) and DM
> implementation (internal by default).

The legacy non-DM implementation kept the settings for timeout the same
as they were before. For the reset, it appears that it used to do
internal+external reset whereas now it does external only, so it's
possible that might cause an issue on some boards. However, they should
really be switching to DM and setting the ext-reset attribute properly
depending on which reset they actually want, as that's needed to get
proper watchdog timeout/restart behavior in Linux as well. I doubt any
board actually needs both internal and external resets since then Linux
would be unable to reboot properly.

> 
>> regards,
>> Claudius
>>
>> Claudius Heine (4):
>>   ARM: dts: dh-imx6: add wdt-reboot node for sysreset driver
>>   ARM: imx6: DHCOM i.MX6 PDK: Enable sysreset driver
>>   ARM: imx6: DHCOM i.MX6 PDK: Enable wdt command
>>   ARM: imx6: DHCOM i.MX6 PDK: Use HW_WATCHDOG in SPL
>>
>>  arch/arm/dts/imx6qdl-dhcom-pdk2.dtsi | 5 +++++
>>  configs/dh_imx6_defconfig            | 3 +++
>>  include/configs/dh_imx6.h            | 5 +++++
>>  3 files changed, 13 insertions(+)
>>
> 
> [1]: 
> https://gitlab.denx.de/u-boot/u-boot/blob/master/drivers/watchdog/imx_watchdog.c#L45
> [2]: 
> https://gitlab.denx.de/u-boot/u-boot/blob/master/drivers/watchdog/imx_watchdog.c#L96
> 

-- 
Robert Hancock
Senior Software Developer
SED Systems, a division of Calian Ltd.
Email: hanc...@sedsystems.ca
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to