Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
Hi Jan, On Tue, 20 Jul 2021 at 06:58, Jan Kiszka wrote: > > On 14.07.21 16:15, Simon Glass wrote: > > Hi Jan, > > > > On Wed, 14 Jul 2021 at 03:53, Jan Kiszka wrote: > >> > >> On 05.07.21 17:29, Simon Glass wrote: > >>> Hi Jan, > >>> > >>> On Sun, 27 Jun 2021 at 23:40, Jan Kiszka wrote: > > On 27.06.21 20:18, Simon Glass wrote: > > Hi Jan, > > > > On Sun, 27 Jun 2021 at 12:01, Jan Kiszka wrote: > >> > >> On 26.06.21 20:29, Simon Glass wrote: > >>> Hi, > >>> > >>> On Fri, 11 Jun 2021 at 08:08, Tom Rini 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 > >> > >> 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 > >> --- > >> 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
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
On 20.07.21 14:57, Jan Kiszka wrote: > On 14.07.21 16:15, Simon Glass wrote: >> Hi Jan, >> >> On Wed, 14 Jul 2021 at 03:53, Jan Kiszka wrote: >>> >>> On 05.07.21 17:29, Simon Glass wrote: Hi Jan, On Sun, 27 Jun 2021 at 23:40, Jan Kiszka wrote: > > On 27.06.21 20:18, Simon Glass wrote: >> Hi Jan, >> >> On Sun, 27 Jun 2021 at 12:01, Jan Kiszka wrote: >>> >>> On 26.06.21 20:29, Simon Glass wrote: Hi, On Fri, 11 Jun 2021 at 08:08, Tom Rini 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 >>> >>> 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 >>> --- >>> 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
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
On 14.07.21 16:15, Simon Glass wrote: > Hi Jan, > > On Wed, 14 Jul 2021 at 03:53, Jan Kiszka wrote: >> >> On 05.07.21 17:29, Simon Glass wrote: >>> Hi Jan, >>> >>> On Sun, 27 Jun 2021 at 23:40, Jan Kiszka wrote: On 27.06.21 20:18, Simon Glass wrote: > Hi Jan, > > On Sun, 27 Jun 2021 at 12:01, Jan Kiszka wrote: >> >> On 26.06.21 20:29, Simon Glass wrote: >>> Hi, >>> >>> On Fri, 11 Jun 2021 at 08:08, Tom Rini 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 >> >> 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 >> --- >> 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
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
Hi Jan, On Wed, 14 Jul 2021 at 03:53, Jan Kiszka wrote: > > On 05.07.21 17:29, Simon Glass wrote: > > Hi Jan, > > > > On Sun, 27 Jun 2021 at 23:40, Jan Kiszka wrote: > >> > >> On 27.06.21 20:18, Simon Glass wrote: > >>> Hi Jan, > >>> > >>> On Sun, 27 Jun 2021 at 12:01, Jan Kiszka wrote: > > On 26.06.21 20:29, Simon Glass wrote: > > Hi, > > > > On Fri, 11 Jun 2021 at 08:08, Tom Rini 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 > > 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 > --- > 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 > >>
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
On 05.07.21 17:29, Simon Glass wrote: > Hi Jan, > > On Sun, 27 Jun 2021 at 23:40, Jan Kiszka wrote: >> >> On 27.06.21 20:18, Simon Glass wrote: >>> Hi Jan, >>> >>> On Sun, 27 Jun 2021 at 12:01, Jan Kiszka wrote: On 26.06.21 20:29, Simon Glass wrote: > Hi, > > On Fri, 11 Jun 2021 at 08:08, Tom Rini 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 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 --- 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
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
Hi Jan, On Sun, 27 Jun 2021 at 23:40, Jan Kiszka wrote: > > On 27.06.21 20:18, Simon Glass wrote: > > Hi Jan, > > > > On Sun, 27 Jun 2021 at 12:01, Jan Kiszka wrote: > >> > >> On 26.06.21 20:29, Simon Glass wrote: > >>> Hi, > >>> > >>> On Fri, 11 Jun 2021 at 08:08, Tom Rini 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 > >> > >> 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 > >> --- > >> 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
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
On 27.06.21 20:18, Simon Glass wrote: > Hi Jan, > > On Sun, 27 Jun 2021 at 12:01, Jan Kiszka wrote: >> >> On 26.06.21 20:29, Simon Glass wrote: >>> Hi, >>> >>> On Fri, 11 Jun 2021 at 08:08, Tom Rini 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 >> >> 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 >> --- >> 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. > > Do you mean 'properly hashed' and 'easy access', or something else? > What can binman not do? Binman itself can stick things into binary
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
Hi Tom, On Sun, 27 Jun 2021 at 13:34, Tom Rini wrote: > > On Sun, Jun 27, 2021 at 12:18:09PM -0600, Simon Glass wrote: > > Hi Jan, > > > > On Sun, 27 Jun 2021 at 12:01, Jan Kiszka wrote: > > > > > > On 26.06.21 20:29, Simon Glass wrote: > > > > Hi, > > > > > > > > On Fri, 11 Jun 2021 at 08:08, Tom Rini 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 > > > > > > 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 > > > --- > > > 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
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
On Sun, Jun 27, 2021 at 12:18:09PM -0600, Simon Glass wrote: > Hi Jan, > > On Sun, 27 Jun 2021 at 12:01, Jan Kiszka wrote: > > > > On 26.06.21 20:29, Simon Glass wrote: > > > Hi, > > > > > > On Fri, 11 Jun 2021 at 08:08, Tom Rini 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 > > > > 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 > > --- > > 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
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
Hi Jan, On Sun, 27 Jun 2021 at 12:01, Jan Kiszka wrote: > > On 26.06.21 20:29, Simon Glass wrote: > > Hi, > > > > On Fri, 11 Jun 2021 at 08:08, Tom Rini 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 > > 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 > --- > 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. Do you mean 'properly hashed' and 'easy access', or something else? What can binman not do? Regards, Simon
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
On 26.06.21 20:29, Simon Glass wrote: > Hi, > > On Fri, 11 Jun 2021 at 08:08, Tom Rini 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 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 --- 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
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
Hi, On Fri, 11 Jun 2021 at 08:08, Tom Rini 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 > > > > > > 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 > > > --- > > > 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. > > Reviewed-by: Tom Rini > > -- > Tom Regards, Simon
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
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 > > > > 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 > > --- > > 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. Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
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 > > 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 > --- > 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? Thanks and regards, Lokesh > > Thanks, > Jan >
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
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 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 --- 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. Thanks, Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
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 >>> >>> 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 >>> --- >>> 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 -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
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 > > > > 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 > > --- > > 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. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
On 07.06.21 12:03, Lokesh Vutla wrote: > +Tom, > > Hi Tom, > > On 02/06/21 3:07 pm, Jan Kiszka wrote: >> From: Jan Kiszka >> >> 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 >> --- >> 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? > >> + >> +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 5c7ef593fe..5016ee4708 100644 >> --- a/drivers/watchdog/Makefile >> +++ b/drivers/watchdog/Makefile >> @@ -33,7 +33,12 @@ obj-$(CONFIG_WDT_OCTEONTX) += octeontx_wdt.o >> obj-$(CONFIG_WDT_OMAP3) += omap_wdt.o >> obj-$(CONFIG_WDT_SBSA) += sbsa_gwdt.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 >> + > > [...snip..] > >> 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 00..78d99ff9f2 >> --- /dev/null >> +++ b/drivers/watchdog/rti_wdt_fw.S > > Isn't this usecase specific? IMHO, drivers might not be the right place. Did I > misunderstand something? > It's specific to this driver - on a subset of supported hardware platforms, namely AM65x SR1.0. Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux
Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
+Tom, Hi Tom, On 02/06/21 3:07 pm, Jan Kiszka wrote: > From: Jan Kiszka > > 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 > --- > 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? > + > +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 5c7ef593fe..5016ee4708 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -33,7 +33,12 @@ obj-$(CONFIG_WDT_OCTEONTX) += octeontx_wdt.o > obj-$(CONFIG_WDT_OMAP3) += omap_wdt.o > obj-$(CONFIG_WDT_SBSA) += sbsa_gwdt.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 > + [...snip..] > 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 00..78d99ff9f2 > --- /dev/null > +++ b/drivers/watchdog/rti_wdt_fw.S Isn't this usecase specific? IMHO, drivers might not be the right place. Did I misunderstand something? Thanks and regards, Lokesh > @@ -0,0 +1,20 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (c) Siemens AG, 2020 > + * > + * Authors: > + * Jan Kiszka > + */ > + > +.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 >
[PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
From: Jan Kiszka
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
---
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.
+
+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 5c7ef593fe..5016ee4708 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -33,7 +33,12 @@ obj-$(CONFIG_WDT_OCTEONTX) += octeontx_wdt.o
obj-$(CONFIG_WDT_OMAP3) += omap_wdt.o
obj-$(CONFIG_WDT_SBSA) += sbsa_gwdt.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
+
+ifeq ($(CONFIG_WDT_K3_RTI_LOAD_FW),y)
+$(obj)/rti_wdt_fw.o: $(shell readlink -f $(CONFIG_WDT_K3_RTI_FW_FILE)) FORCE
+endif
diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index 8335b20ae8..97daf40145 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -11,9 +11,11 @@
#include
#include
#include
+#include
#include
#include
#include
+#include
/* Timer register set definition */
#define RTIDWDCTRL 0x90
@@ -42,15 +44,69 @@ struct rti_wdt_priv {
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)
{
+#ifdef CONFIG_WDT_K3_RTI_LOAD_FW
+ struct udevice *rproc_dev;
+ int primary_core, ret;
+ u32 cluster_mode;
+ ofnode node;
+#endif
struct rti_wdt_priv *priv = dev_get_priv(dev);
u32 timer_margin;
- int ret;
if (readl(priv->regs + RTIDWDCTRL) == WDENABLE_KEY)
return -EBUSY;
+#ifdef CONFIG_WDT_K3_RTI_LOAD_FW
+ node = ofnode_by_compatible(ofnode_null(), "ti,am654-r5fss");
+ if (!ofnode_valid(node)) {
+ dt_error:
+ dev_err(dev, "No compatible firmware target processor found\n");
+ return -ENODEV;
+ }
+
+ ret = ofnode_read_u32(node, "ti,cluster-mode", _mode);
+ if (ret)
+ cluster_mode = 1;
+
+ node = ofnode_by_compatible(node, "ti,am654-r5f");
+ if (!ofnode_valid(node))
+ goto dt_error;
+
+ ret = uclass_get_device_by_ofnode(UCLASS_REMOTEPROC, node, _dev);
+ if (ret)
+ return ret;
+
+ primary_core = dev_seq(rproc_dev);
+
+ ret = rproc_dev_init(primary_core);
+ if (ret) {
+ fw_error:
+ dev_err(dev, "Failed to load watchdog firmware into remote
processor %d\n",
+ primary_core);
+ return ret;
+ }
+
+ if (cluster_mode == 1) {
+ ret = rproc_dev_init(primary_core + 1);
+ if (ret)
+ goto fw_error;
+ }
+
+ ret = rproc_load(primary_core, (ulong)rti_wdt_fw,
+rti_wdt_fw_size);
+ if (ret)
+ goto fw_error;
+
+ ret = rproc_start(primary_core);
+ if (ret)
+ goto fw_error;
+#endif
+
timer_margin = timeout_ms * priv->clk_khz / 1000;
timer_margin >>= WDT_PRELOAD_SHIFT;
if
