From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-21.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B95C9C48BC2 for ; Sun, 27 Jun 2021 18:18:30 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id DC905619B1 for ; Sun, 27 Jun 2021 18:18:29 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DC905619B1 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7E815829E4; Sun, 27 Jun 2021 20:18:27 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="f3epUcid"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 8929382C27; Sun, 27 Jun 2021 20:18:25 +0200 (CEST) Received: from mail-wm1-x336.google.com (mail-wm1-x336.google.com [IPv6:2a00:1450:4864:20::336]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id F0C6B829AE for ; Sun, 27 Jun 2021 20:18:21 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-wm1-x336.google.com with SMTP id u20so4533011wmq.4 for ; Sun, 27 Jun 2021 11:18:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=jOCPtiKBzjPp8cPZeIzNYAyFpTejm8GFd4mXSDYR2YQ=; b=f3epUcidX9vA5oabzun4tSAufWTUlvbERkvbIeVYrx8EUZCkZOrprUw9WF9Db/PBfF oxFoq/f80OZZeIW79VuBwUW/QC7yQwPU/LMCogC3TAQt65GOv1l/xcOBUKGQr3/sR/ve /C4xVT+fQqIvFuPUk2U+zTNZ9njZwwMcPNYiw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=jOCPtiKBzjPp8cPZeIzNYAyFpTejm8GFd4mXSDYR2YQ=; b=tdCtGlvXyKoqcUjdu9zlpIkfgmdQgLc46qMjngQ5CUpslfq2wT3WJZycB1A+y6lZfX Pj2tQntwkb4USS3tbfz3f+x+aYu/94rBrVKoB9ZO0IVu4VjZtMxCjnXPh0AYX3MgKTLr rchWYp51PjU3SulbfAYK12+qTxrrDi1ORgeH/KTAI8lY210Cwv4XO7jMYHmMMQ+tn5A5 sKM5e1CDCPKWKS4hns0icuD5/9Xm2DEYZ0zuvjAAr6IOm4A/sraCUpffEG89K5xInoN4 kxmxGtwUTSuyNDKp4KowBxSZNk5QRBsNHDG32Zmuq0EcVtt+L+BZwk9anXJAN8LZ+m8f HTYA== X-Gm-Message-State: AOAM530EeTl24S1GYJvNre1ZYWtgOF63Ko1bqBtZpO7MsGHWRvGs44ci Pdfqv1xSd6bhGHRGs9/bVntnoXkUd8PTNJzoIaTy+A== X-Google-Smtp-Source: ABdhPJx4/xG27pgZqgV+TYYvFkAvIgUCNNXsvcWqpRHW5hXNEic2Xc+bWsYHZ+nAXZnZhraWwuSUPTUqD5z5sFuL6fs= X-Received: by 2002:a1c:28a:: with SMTP id 132mr21983855wmc.120.1624817901233; Sun, 27 Jun 2021 11:18:21 -0700 (PDT) MIME-Version: 1.0 References: <88d7d3e323c27417d7109b8a92bf53a08ad77654.1622626660.git.jan.kiszka@siemens.com> <96039724-9a5f-dbb8-d46b-b268a0d9a8c2@ti.com> <20210607114007.GD9516@bill-the-cat> <20210611140831.GU9516@bill-the-cat> <54a75091-0499-8902-b7f5-b1f75ae0091c@siemens.com> In-Reply-To: <54a75091-0499-8902-b7f5-b1f75ae0091c@siemens.com> From: Simon Glass Date: Sun, 27 Jun 2021 12:18:09 -0600 Message-ID: Subject: Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware To: Jan Kiszka Cc: Tom Rini , Lokesh Vutla , U-Boot Mailing List , Le Jin , Bao Cheng Su , Nian Gao , Chao Zeng Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean 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