[PATCH v2 2/3] watchdog: rti_wdt: Add support for loading firmware

[Jan Kiszka]

On 23.06.20 14:37, Jan Kiszka wrote:
> On 23.06.20 13:50, Lokesh Vutla wrote:
>>
>>
>> On 23/06/20 4:45 pm, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka at siemens.com>
>>>
>>> To avoid the need of extra boot scripting on AM65x for loading a
>>> watchdog firmware, add the required rproc init and loading logic for the
>>> first R5F core to the watchdog start handler. The firmware itself is
>>> embedded into U-Boot binary.
>>>
>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka at siemens.com>
>>> ---
>>>   drivers/watchdog/Kconfig      | 20 ++++++++++++++++++++
>>>   drivers/watchdog/Makefile     |  3 +++
>>>   drivers/watchdog/rti_wdt.c    | 24 ++++++++++++++++++++++++
>>>   drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++++++++++
>>>   4 files changed, 67 insertions(+)
>>>   create mode 100644 drivers/watchdog/rti_wdt_fw.S
>>>
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>> index ee99bd2af1..fd6ab9a85b 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>> @@ -162,6 +162,26 @@ config WDT_K3_RTI
>>>         Say Y here if you want to include support for the K3 watchdog
>>>         timer (RTI module) available in the K3 generation of processors.
>>> +if WDT_K3_RTI
>>> +
>>> +config WDT_K3_RTI_LOAD_FW
>>> +    bool "Load watchdog firmware"
>>> +    depends on REMOTEPROC
>>> +    help
>>> +      Automatically load the specified firmware image into the MCU R5F
>>> +      core 0. On the AM65x, this firmware is supposed to handle the 
>>> expiry
>>> +      of the watchdog timer, typically by resetting the system.
>>> +
>>> +config WDT_K3_RTI_FW_FILE
>>> +    string "Watchdog firmware image file"
>>> +    default "k3-rti-wdt.fw"
>>> +    depends on WDT_K3_RTI_LOAD_FW
>>> +    help
>>> +      Firmware image to be embedded into U-Boot and loaded on watchdog
>>> +      start.
>>> +
>>> +endif
>>> +
>>>   config WDT_SANDBOX
>>>       bool "Enable Watchdog Timer support for Sandbox"
>>>       depends on SANDBOX && WDT
>>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>>> index 16bdbf4970..bf58684875 100644
>>> --- a/drivers/watchdog/Makefile
>>> +++ b/drivers/watchdog/Makefile
>>> @@ -28,7 +28,10 @@ obj-$(CONFIG_WDT_MT7621) += mt7621_wdt.o
>>>   obj-$(CONFIG_WDT_MTK) += mtk_wdt.o
>>>   obj-$(CONFIG_WDT_OMAP3) += omap_wdt.o
>>>   obj-$(CONFIG_WDT_K3_RTI) += rti_wdt.o
>>> +obj-$(CONFIG_WDT_K3_RTI_LOAD_FW) += rti_wdt_fw.o
>>>   obj-$(CONFIG_WDT_SP805) += sp805_wdt.o
>>>   obj-$(CONFIG_WDT_STM32MP) += stm32mp_wdt.o
>>>   obj-$(CONFIG_WDT_TANGIER) += tangier_wdt.o
>>>   obj-$(CONFIG_WDT_XILINX) += xilinx_wwdt.o
>>> +
>>> +$(obj)/rti_wdt_fw.o: $(shell readlink -f 
>>> $(CONFIG_WDT_K3_RTI_FW_FILE)) FORCE
>>> diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
>>> index ebe29c7409..38e82a6b6b 100644
>>> --- a/drivers/watchdog/rti_wdt.c
>>> +++ b/drivers/watchdog/rti_wdt.c
>>> @@ -14,6 +14,7 @@
>>>   #include <power-domain.h>
>>>   #include <wdt.h>
>>>   #include <asm/io.h>
>>> +#include <remoteproc.h>
>>>   /* Timer register set definition */
>>>   #define RTIDWDCTRL        0x90
>>> @@ -37,11 +38,16 @@
>>>   #define WDT_PRELOAD_MAX        0xfff
>>> +#define RTI_PROC_ID        0
>>
>> Can we get the rproc id from DT?
> 
> You mean, resolve "mcu_r5fss0_core0" to the ID? Doable.
> 

I'm now probing for the first instance of "ti,am654-r5f" compatible. 
That excludes usage for the J721E for now, but that one is fine without 
firmware - provided there is way to prevent power-down for RTI watchdog 
otherwise...

>>
>>> +
>>>   struct rti_wdt_priv {
>>>       phys_addr_t regs;
>>>       unsigned int clk_khz;
>>>   };
>>> +extern const u32 rti_wdt_fw[];
>>> +extern const int rti_wdt_fw_size;
>>
>> FIT is the preferred way of packing images in U-Boot. Can you try 
>> using it?
> 
> How does that work? Some example for me?
> 

If you happen to refer to fs-loader: That does not target OSPI, our 
primary use case.

> What benefit would that bring? There are other users of this pattern, 
> e.g. board/xilinx/zynqmp/pm_cfg_obj.S.
> 

The only benefit of an alternative loading mechanism seems to be 
handling of larger images from different sources. But that's what I 
would tackle via boot scripting and, thus, without this feature. If you 
only have a few hundred bytes to embed, like for k3-rti-wdt, you quickly 
have a lot of overhead with other approaches.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux