diff mbox series

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

Message ID a42ab2dde9a588a8efc186474becd837f2e470c5.1592910910.git.jan.kiszka@siemens.com
State New
Headers show
Series watchdog: K3: Add RTI watchdog support | expand

Commit Message

Jan Kiszka June 23, 2020, 11:15 a.m. UTC
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

Comments

Lokesh Vutla June 23, 2020, 11:50 a.m. UTC | #1
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?

> +
>  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?

Thanks and regards,
Lokesh
Jan Kiszka June 23, 2020, 12:37 p.m. UTC | #2
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.

> 
>> +
>>   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?

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

Jan
Jan Kiszka June 23, 2020, 2:41 p.m. UTC | #3
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
Lokesh Vutla June 29, 2020, 2:26 a.m. UTC | #4
+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.

> 
>>>
>>>> +
>>>> ? 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?

>>
> 
> 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.

hmm.. Does the build break if firmware is not present?

Thanks and regards,
Lokesh

> 
> Jan
>
Jan Kiszka June 29, 2020, 4:50 a.m. UTC | #5
On 29.06.20 04:26, 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.
> 
>>
>>>>
>>>>> +
>>>>>  ? 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.

In case of our board, that image is not processed by U-Boot (only 
generated). Also, processing it from the R5 loader would imply putting 
remoteproc support into that level, duplicating what we have in A53 U-Boot.

> 
>>
>>> 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?
> 
>>>
>>
>> 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.
> 
> hmm.. Does the build break if firmware is not present?

Yes, if you configure to include the firmware but specify an invalid 
path, the build breaks.

Jan
Lokesh Vutla June 29, 2020, 4:54 a.m. UTC | #6
On 29/06/20 10:20 am, Jan Kiszka wrote:
> On 29.06.20 04:26, 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.
>>
>>>
>>>>>
>>>>>> +
>>>>>> ?? 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.
> 
> In case of our board, that image is not processed by U-Boot (only generated).
> Also, processing it from the R5 loader would imply putting remoteproc support
> into that level, duplicating what we have in A53 U-Boot.
> 
>>
>>>
>>>> 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?
>>
>>>>
>>>
>>> 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.
>>
>> hmm.. Does the build break if firmware is not present?
> 
> Yes, if you configure to include the firmware but specify an invalid path, the
> build breaks.

So, how do we handle the travis CI/gitlab build without this firmware. I am not
sure how xilinx is handling this. It should be the same case in the above
scenario you pointed out.

Thanks and regards,
Lokesh
Jan Kiszka June 29, 2020, 7:44 a.m. UTC | #7
On 29.06.20 06:54, Lokesh Vutla wrote:
> 
> 
> On 29/06/20 10:20 am, Jan Kiszka wrote:
>> On 29.06.20 04:26, 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.
>>>
>>>>
>>>>>>
>>>>>>> +
>>>>>>>  ?? 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.
>>
>> In case of our board, that image is not processed by U-Boot (only generated).
>> Also, processing it from the R5 loader would imply putting remoteproc support
>> into that level, duplicating what we have in A53 U-Boot.
>>
>>>
>>>>
>>>>> 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?
>>>
>>>>>
>>>>
>>>> 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.
>>>
>>> hmm.. Does the build break if firmware is not present?
>>
>> Yes, if you configure to include the firmware but specify an invalid path, the
>> build breaks.
> 
> So, how do we handle the travis CI/gitlab build without this firmware. I am not
> sure how xilinx is handling this. It should be the same case in the above
> scenario you pointed out.

You mean some all-yes-config? We could create a dummy image prior to 
running that build.

The Xilinx has its logic disabled when the file name is empty, which is 
the default. The linker should kick unused bits out then. I can try that 
as well.

Jan
Tom Rini June 29, 2020, 12:37 p.m. UTC | #8
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!
Jan Kiszka June 29, 2020, 6:44 p.m. UTC | #9
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
Lokesh Vutla July 21, 2020, 5:27 a.m. UTC | #10
Hi Jan,

On 20/07/20 3:06 pm, Jan Kiszka wrote:
> 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@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@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.


If you want remotecore to be loaded early, then my preference would be to pack
Firmware inside FIT image(along with A72 SPL, ATF or A72 U-Boot) as loadable and
register a handle to load and start the rproc in SPL. If it can be delayed, I
prefer to do at U-Boot prompt.


However, the driver can be merged separately. If you can re post the driver part
I can pull it in soon.

Thanks and regards,
Lokesh
Jan Kiszka July 21, 2020, 6:26 a.m. UTC | #11
On 21.07.20 07:27, Lokesh Vutla wrote:
> Hi Jan,

> 

> On 20/07/20 3:06 pm, Jan Kiszka wrote:

>> 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@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@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.

> 

> If you want remotecore to be loaded early, then my preference would be to pack

> Firmware inside FIT image(along with A72 SPL, ATF or A72 U-Boot) as loadable and

> register a handle to load and start the rproc in SPL. If it can be delayed, I

> prefer to do at U-Boot prompt.

> 


So far, we do not need it in SPL as we have no procedure to update 
U-Boot proper safely. Moving things to SPL would imply carrying the 
remoteproc infrastructure twice.

For loading from the prompt, we are still lacking a handy way to bundle 
the firmware with the integrity-checked U-Boot proper. Yes, technically 
I could push those 676 bytes into a fitimage, sign it, reserve a block 
in SPI flash or some other storage, load it from there and validate it. 
But that's overkill IMHO.

> 

> However, the driver can be merged separately. If you can re post the driver part

> I can pull it in soon.


You could cherry-pick patch 1 and 3 for that already, they still apply.

Thanks,
Jan

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

Patch

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
+
 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;
+
 static int rti_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
 {
 	struct rti_wdt_priv *priv = dev_get_priv(dev);
@@ -51,6 +57,24 @@  static int rti_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
 	if (readl(priv->regs + RTIDWDCTRL) == WDENABLE_KEY)
 		return -EBUSY;
 
+#ifdef CONFIG_WDT_K3_RTI_LOAD_FW
+	ret = rproc_dev_init(RTI_PROC_ID);
+	if (ret) {
+	    fw_error:
+		dev_err(dev, "Failed to load watchdog firmware into remote processor %d\n",
+			RTI_PROC_ID);
+		return ret;
+	}
+
+	ret = rproc_load(RTI_PROC_ID, (ulong)rti_wdt_fw, rti_wdt_fw_size);
+	if (ret)
+		goto fw_error;
+
+	ret = rproc_start(RTI_PROC_ID);
+	if (ret)
+		goto fw_error;
+#endif
+
 	timer_margin = timeout_ms * priv->clk_khz / 1000;
 	timer_margin >>= WDT_PRELOAD_SHIFT;
 	if (timer_margin > WDT_PRELOAD_MAX)
diff --git a/drivers/watchdog/rti_wdt_fw.S b/drivers/watchdog/rti_wdt_fw.S
new file mode 100644
index 0000000000..78d99ff9f2
--- /dev/null
+++ b/drivers/watchdog/rti_wdt_fw.S
@@ -0,0 +1,20 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) Siemens AG, 2020
+ *
+ * Authors:
+ *   Jan Kiszka <jan.kiszka at siemens.com>
+ */
+
+.section .rodata
+
+.global rti_wdt_fw
+.global rti_wdt_fw_size
+
+rti_wdt_fw:
+.align 4
+.incbin CONFIG_WDT_K3_RTI_FW_FILE
+rti_wdt_fw_end:
+
+rti_wdt_fw_size:
+.int rti_wdt_fw_end - rti_wdt_fw