Re: [PATCH v2 0/2] Add a watchdog driver that uses ARM Secure Monitor Calls.

From: Julius Werner <jwerner@chromium.org>
To: Xingyu Chen <xingyu.chen@amlogic.com>
Cc: Evan Benn <evanbenn@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Julius Werner <jwerner@chromium.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Anson Huang <Anson.Huang@nxp.com>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	"David S. Miller" <davem@davemloft.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Leonard Crestez <leonard.crestez@nxp.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
	Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
	Olof Johansson <olof@lixom.net>, Rob Herring <robh+dt@kernel.org>,
	Rob Herring <robh@kernel.org>, Shawn Guo <shawnguo@kernel.org>,
	Valentin Schneider <valentin.schneider@arm.com>,
	Vinod Koul <vkoul@kernel.org>, Will Deacon <will@kernel.org>,
	Wim Van Sebroeck <wim@linux-watchdog.org>,
	devicetree@vger.kernel.org,
	"moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
	<linux-arm-kernel@lists.infradead.org>,
	"moderated list:ARM/Mediatek SoC support" 
	<linux-mediatek@lists.infradead.org>,
	LINUX-WATCHDOG <linux-watchdog@vger.kernel.org>,
	Jianxin Pan <jianxin.pan@amlogic.com>,
	Yonghui Yu <yonghui.yu@amlogic.com>,
	"open list:ARM/Amlogic Meson..."
	<linux-amlogic@lists.infradead.org>
Subject: Re: [PATCH v2 0/2] Add a watchdog driver that uses ARM Secure Monitor Calls.
Date: Wed, 15 Apr 2020 15:29:29 -0700	[thread overview]
Message-ID: <CAODwPW9RSB37+4EJ2QXAwz=ShFB23L1GKC2mLYE5L5JuQR2tPw@mail.gmail.com> (raw)
In-Reply-To: <243e107c-35c1-2d14-5285-c9e13744963c@amlogic.com>

> In addition, It looks more reasonable to use the "msec" as the unit of
> timeout parameter for the ATF fw interface with SMCWD_SET_TIMEOUT:
>
> - The fw interface will compatible with the uboot generic watchdog
> interface at [0], and there is no need to convert timeout from msec
> to sec.

I think we're trying hard to keep this compatible to a Trusted
Firmware counterpart that we have already shipped, so we would prefer
to keep it at seconds if possible. That's what the Linux watchdog core
uses as well after all, so it just seems natural. I don't really see
how what U-Boot does would have anything to do with this.

> - Some vendor's watchdog may be not support the "wdt_trigger_reset"
> reset operation, but they can use the method below to reset the system
> by the watchdog right now.
>
> watchdog_set_time(1);  //1ms
> watchdog_enable();

They can still do that but they should do that on the Trusted Firmware
side. Emulating a missing reset functionality should be handled by the
hardware abstraction layer (in this case Trusted Firmware), not at the
Linux API level. So Linux would still send a PSCI_SYSTEM_RESET SMC,
but then Trusted Firmware can choose to implement that by setting the
watchdog to the smallest possible timeout (which it can because it's
accessing it directly, not through this SMC interface) and letting it
expire.