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

Mon Jul 20 11:36:11 CEST 2020

On 29.06.20 20:44, Jan Kiszka wrote:
> On 29.06.20 14:37, Tom Rini wrote:
>> On Mon, Jun 29, 2020 at 07:56:53AM +0530, Lokesh Vutla wrote:
>>> +Tom
>>>
>>> On 23/06/20 8:11 pm, Jan Kiszka wrote:
>>>> 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...
>>>
>>> I was more of thinking to have a DT property to reference R5-core. 
>>> But anyways,
>>> the property is not present in kernel. I am okay with this for now.
>>
>> Note that not all properties in a DT in Linux need to be used in Linux,
>> this is something I _think_ there is growing understanding of.
>>
>>>>>>> +
>>>>>>>    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.
>>>
>>> No. I was thinking to pack the image in FIT along with ATF and OPTEE 
>>> like in
>>> k3_fit_atf.sh and register a U_BOOT_FIT_LOADABLE_HANDLER for 
>>> installing the
>>> firmware.
>>>
>>>>
>>>>> What benefit would that bring? There are other users of this 
>>>>> pattern, e.g.
>>>>> board/xilinx/zynqmp/pm_cfg_obj.S.
>>>
>>> I didn't know U-Boot is accepting this. Tom, is this the preferred 
>>> way for
>>> including firmware images?
>>
>> Adding in Michal.  Why is this area doing what it's doing and not one of
>> the other generic ways to handle this problem?  Thanks!
>>
> 
> So, what are the recommended "generic ways" for these use cases?
> 
> Is there a way to stick own stuff into u-boot.itb and get that provided 
> to some handler of U-Boot proper? Seems unlikely. The pattern I found 
> pushes the firmware into the kernel fit image - which is a clear no-go 
> for the watchdog use case because it shall monitor the booting of that 
> very same image (software update case).
> 
> Jan
> 

Ping on this. I'd like to move forward with this series.

Jan

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