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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 7A023C25B08 for ; Tue, 23 Aug 2022 03:43:03 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id DE74D80F4E; Tue, 23 Aug 2022 05:43:00 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="QfamGog+"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id A7C3083EF1; Tue, 23 Aug 2022 05:42:59 +0200 (CEST) Received: from mail-qk1-x734.google.com (mail-qk1-x734.google.com [IPv6:2607:f8b0:4864:20::734]) (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 0C8B280F4E for ; Tue, 23 Aug 2022 05:42:57 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=seanga2@gmail.com Received: by mail-qk1-x734.google.com with SMTP id c9so8280855qkk.6 for ; Mon, 22 Aug 2022 20:42:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject:from:to:cc; bh=wJLmMdHT44iTJMStFc7qM7vs5M2XH9m+jCLd9exlp5E=; b=QfamGog+32zx4YC8S4dYzQqG8Xpxc1RAmL7GzwfYyIJFf4O9p1M5oKnGiJNh0lhbAW 9Szq3kX+L1h3/IH/FKh0ea5yeTNH5WNfV4BcKRBkZNjG7oV2jxPOVVk+CkqzkXk0nxl9 Y3Sn0bMM9y3+xp69/y3y3Qqoz0SFElugrC0IYOYc7ymI1s2Lu+EwqDxWyZ0PrBS+menn +4Mb5H3k3/03xQDYPBo2AozZMzH2i4YXjSz7F69Z7XQwF4y2VayPrRKZDzFNKNWhvyQB qiP5wmVw7jhi29Cl7c1A/OwcPKwgUaWV8QPSqwiYLwoiGvqnIiC+8zT7XOa7fkhyCLBW vuig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :x-gm-message-state:from:to:cc; bh=wJLmMdHT44iTJMStFc7qM7vs5M2XH9m+jCLd9exlp5E=; b=3Mg2MUdwAbU4yOlYArko753k2GYn0MKnSGGsGcU9U4QWmecY0jMZn/A/wFWFHhPvzI q80XTapj085PZ02pfRAOjLmEdAdcIctE6rB117KTjQRzIT/tCpoX1VTT+1KyhV807j+V cSuHGFfvEmlg5Ho3BlEqcfRTRB3L7dWxyDAuSX7KOGL6jjgOLHd48vm17eQMjOACAd2Y UXWJv/rkSmUuL/uW4ws5w2/sFFo29GqyggA5HySm2IZlmAQ26QEyBOZ4MRbcxH5fansS +ifv8dwrdPTKqpdix5GwMAPTI4FKktCtSPAFiQLI3qN+WF4EJaMrUqd3OuOx4Sv5XCR4 Z5hA== X-Gm-Message-State: ACgBeo0j44h5h1pPGj5RV3tLaDw2eB+cBO/4zVCnP50dbfOthDDi+dec 0tWyEiOO7RXNOJWIyFxscVs= X-Google-Smtp-Source: AA6agR771PT/sxeQTCktizWIUbvZvIhmkmC8++6e2Z0ENBmRxEK9XvGqtsI1S91KMnqHGWMiIhnQbw== X-Received: by 2002:a05:620a:4155:b0:6bb:2061:1167 with SMTP id k21-20020a05620a415500b006bb20611167mr14743925qko.623.1661226175529; Mon, 22 Aug 2022 20:42:55 -0700 (PDT) Received: from [192.168.1.201] (pool-173-73-95-180.washdc.fios.verizon.net. [173.73.95.180]) by smtp.gmail.com with ESMTPSA id s11-20020a05620a0bcb00b006bb9125363fsm12503544qki.121.2022.08.22.20.42.54 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 22 Aug 2022 20:42:55 -0700 (PDT) Subject: Re: [RFC PATCH v2 9/9] tools: spkgimage: add Renesas SPKG format To: Ralph Siemsen Cc: andre.przywara@arm.com, heiko.thiery@gmail.com, pali@kernel.org, samuel@sholland.org, sjg@chromium.org, sr@denx.de, takahiro.akashi@linaro.org, u-boot@lists.denx.de, sean.anderson@seco.com References: <20220809125959.217333-10-ralph.siemsen@linaro.org> <20220812170325.485707-1-ralph.siemsen@linaro.org> <7e4a3833-bc51-9b2c-aa0a-4af017e1fb1b@gmail.com> From: Sean Anderson Message-ID: <0e426d60-e6a4-4f3f-7a55-8c3cf9d90e08@gmail.com> Date: Mon, 22 Aug 2022 23:42:54 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.6 at phobos.denx.de X-Virus-Status: Clean On 8/16/22 10:33 AM, Ralph Siemsen wrote: > Hi Sean, > > I've implemented most of the suggestions. I will post an updated > series, since it seems that sending v2 of just one patch has confused > patchwork. > > However so as not to entirely remove confusion, the updated series > will be v3, since I already used v2 for the one patch. :-P > > On Sat, Aug 13, 2022 at 9:45 PM Ralph Siemsen wrote: >>> >>> I wonder if you could just fill in the header directly. This is >>> for a userspace tool, and this struct will be created at most >>> once. It's OK to use 10 bytes :) >> >> I could fill the header directly, but I figured it would be cleaner to >> keep the config file parsing separate from header generation. > > Does it seem reasonable to keep these structures separated? Yes, that is fine. >>>> +static int spkgimage_verify_header(unsigned char *ptr, int size, >>>> + struct image_tool_params *param) >>>> +{ >>>> + struct spkg_file *file = (struct spkg_file *)ptr; >>>> + struct spkg_hdr *header = (struct spkg_hdr *)ptr; >>>> + char signature[4] = SPKG_HEADER_SIGNATURE; >>> >>> If this naming does not come from documentation, I would suggest >>> something like SPKG_HEADER_MAGIC, since this is not a signature, >>> or even a CRC. >> >> The name does in fact come from the RZ/N1 documentation. However I >> agree that SPKG_HEADER_MAGIC would better reflect what these bytes >> actually are. > > Upon checking the documentation, it turns out they use the term > "marker" rather than "signature" for these bytes. So I have switched > the code to match. That sounds good. >>>> +static int spkgimage_check_image_types(uint8_t type) >>>> +{ >>>> + return type == IH_TYPE_RENESAS_SPKG ? 0 : 1; >>> >>> This function is not necessary if you only support one type. >> >> Without this function, mkimage kept telling me that my format >> (spkgimage) was not supported, and none of my callbacks got invoked. >> It only complained when trying to generate a header. When listing the >> supported formats, spkgimage showed up correctly. >> >> I'll take another look on Monday, maybe I missed something obvious. > > I have re-checked this: > - without the function, mkimage complains that spkgimage is unknown > - with a function that unconditionally returns 0, it works fine > > If it really is meant to work without the function, then a bug must > have crept in elsewhere... Huh. I did a quick grep so maybe I missed something. IMO this *should* work without a function, because we have tons of drivers which just have an equality check. In any case, you can just do return type == IH_TYPE_RENESAS_SPKG ? 0 : -EINVAL; --Sean