On 26.06.21 20:29, Simon Glass wrote: > Hi, > > On Fri, 11 Jun 2021 at 08:08, Tom Rini <tr...@konsulko.com> wrote: >> >> On Fri, Jun 11, 2021 at 07:14:21PM +0530, Lokesh Vutla wrote: >>> Hi Tom, >>> >>> On 09/06/21 6:47 pm, Jan Kiszka wrote: >>>> On 07.06.21 13:44, Jan Kiszka wrote: >>>>> On 07.06.21 13:40, Tom Rini wrote: >>>>>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote: >>>>>>> +Tom, >>>>>>> >>>>>>> Hi Tom, >>>>>>> >>>>>>> On 02/06/21 3:07 pm, Jan Kiszka wrote: >>>>>>>> From: Jan Kiszka <jan.kis...@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. In case the R5F cluster >>>>>>>> is >>>>>>>> in lock-step mode, also initialize the second core. The firmware itself >>>>>>>> is embedded into U-Boot binary to ease access to it and ensure it is >>>>>>>> properly hashed in case of secure boot. >>>>>>>> >>>>>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt. >>>>>>>> >>>>>>>> Signed-off-by: Jan Kiszka <jan.kis...@siemens.com> >>>>>>>> --- >>>>>>>> drivers/watchdog/Kconfig | 20 ++++++++++++ >>>>>>>> drivers/watchdog/Makefile | 5 +++ >>>>>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++- >>>>>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++ >>>>>>>> 4 files changed, 102 insertions(+), 1 deletion(-) >>>>>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S >>>>>>>> >>>>>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig >>>>>>>> index f0ff2612a6..1a1fddfe9f 100644 >>>>>>>> --- a/drivers/watchdog/Kconfig >>>>>>>> +++ b/drivers/watchdog/Kconfig >>>>>>>> @@ -209,6 +209,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. >>>>>>> >>>>>>> I need your input on this proach. Is it okay to include the linker file >>>>>>> unders >>>>>>> drivers? >>>>>> >>>>>> Maybe? I suppose the first thing that springs to mind is why aren't we >>>>>> using binman and including this blob (which I happily see is GPLv2) >>>>>> similar to how we do things with x86 for one example. >>>>>> >>>>> >>>>> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html >>>>> >>>>> Jan >>>>> >>>> >>>> Did this help to answer open questions? Otherwise, please let me know. >>>> >>>> I'd also like to avoid that his patch alone blocks 1-3 of the series >>>> needless - but I would also not mind getting everything in at once. >>> >>> Can you provide your reviewed-by if you are okay with this approach? >> >> I was kind of hoping Simon would chime in here on binman usage. So, >> re-re-reading the above URL, yes, fsloader wouldn't be the right choice >> for watchdog firmware. But I think binman_entry_find() and related >> could work, in general, for this case of "need firmware blob embedded in >> to image". That said, this isn't just any firmware blob, it's the >> watchdog firmware. The less reliance on other things the safer it is. >> That means this would be an exception to the general firmware blob >> loading rule and yeah, OK, we can do it this way. Sorry for the delay. > > Yes I've been a little tied up recently. But I think this should use > binman. We really don't want to be building binary firmware into > U-Boot itself! > > Also Tom says, see x86 for a load of binaries of different types and > how they are accessed at runttime. This is what binman is for. >
Please take the time and study my arguments. I'm open for better proposals, but they need to be concrete and addressing my points. Thanks, Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux
