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=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 5BDE0C4338F for ; Tue, 24 Aug 2021 04:38:59 +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 4B3AA60C51 for ; Tue, 24 Aug 2021 04:38:57 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 4B3AA60C51 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 5488882D5B; Tue, 24 Aug 2021 06:38:55 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org 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=linaro.org header.i=@linaro.org header.b="v55Sr9+c"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id DC85482C03; Tue, 24 Aug 2021 06:38:53 +0200 (CEST) Received: from mail-pf1-x433.google.com (mail-pf1-x433.google.com [IPv6:2607:f8b0:4864:20::433]) (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 1E4D182C03 for ; Tue, 24 Aug 2021 06:38:49 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=takahiro.akashi@linaro.org Received: by mail-pf1-x433.google.com with SMTP id r9so1163681pfh.6 for ; Mon, 23 Aug 2021 21:38:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=iMaLoSXD/v1OgZGME8M09F135WhOSyXEJ90onylBpFw=; b=v55Sr9+cEuDzy575vDjNu0keXQU64lCVSj/b0nXvlHbsRwzdw0Jo6thF4uBwBbnWad FrdEsXguqXu2fashvbYPtFoWW8t28ETwATUs9CiKv5r0bWkT9jjYGijpxL2XJp0Wd56j uUyUzaSTJQtfbEsr8g5FiMzHNSSGl3OJF0DR0yydrHAOYkWr52ksVvJsN7xtYGIBC/s/ /vKVQ4z7xzvzRoE++NmglgGPJUQE5bFMRLjj7JzpCcV2NsMy2JSXPA/7mam6bBaTbzX3 qR5jIJu/Ni7QwtAtOs8gAE4ktrKhYrXlDIsTyhw4nbXSW2It21+0DAvbA0OXRTwNi15g 62GA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:subject:message-id:mail-followup-to :references:mime-version:content-disposition:in-reply-to; bh=iMaLoSXD/v1OgZGME8M09F135WhOSyXEJ90onylBpFw=; b=SAdXDPjNnJcGy0Rs9pio5HK1jCGgrWhZ8iWWbXq6mQVuZNLSXaH7AWnwQnvAnrQcBo Y63454lO38D8tyM+t7gCCJFwwxCfIbs7ege5+jq9fdQj576HJwbCmE5RbOM3lNljv2vU vaJFIyKSPU2jCi6SMvH18WtWNm/sbZzo82XYhUAYZFaubrHQp7xYEs/S5SnMP4nFDO0C XVb+BfNGlSX0F/ntLijMSW9WSyQap07Z0ZLgZoo3ZBGzmdleHi2lEvkALNcJPUU1DaVr h3Tgpjo2U9i+sK0jyRLXKtbRnUOtLm+MrskTU7DNIh/EOJpm099vt9xop7hlKntJglmm CSLA== X-Gm-Message-State: AOAM533AE/i/Kw7qdaMqYQyH2DgaGkgMoZxkY0flWMWyzBPmcLepjykE xUC2JoIDErj7dRewzogacCnXEA== X-Google-Smtp-Source: ABdhPJxxWDEIXFtHI1SBvj0n46aqQtCMeUNuXnYX4xmpEkBUA+Qcd1Cgt7OxYuTFGUsNFNZGpArhFw== X-Received: by 2002:a65:5342:: with SMTP id w2mr35073924pgr.333.1629779927075; Mon, 23 Aug 2021 21:38:47 -0700 (PDT) Received: from laputa (pdb6272e8.tkyea130.ap.so-net.ne.jp. [219.98.114.232]) by smtp.gmail.com with ESMTPSA id o73sm7011573pfg.175.2021.08.23.21.38.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Aug 2021 21:38:46 -0700 (PDT) Date: Tue, 24 Aug 2021 13:38:06 +0900 From: AKASHI Takahiro To: Heinrich Schuchardt , Heinrich Schuchardt , u-boot@lists.denx.de, Alexander Graf , Bin Meng , Simon Glass Subject: Re: [PATCH 3/5] efi_loader: use correct type for AllocatePages, AllocatePool Message-ID: <20210824043806.GA47139@laputa> Mail-Followup-To: AKASHI Takahiro , Heinrich Schuchardt , Heinrich Schuchardt , u-boot@lists.denx.de, Alexander Graf , Bin Meng , Simon Glass References: <20210817160225.222760-1-heinrich.schuchardt@canonical.com> <20210817160225.222760-4-heinrich.schuchardt@canonical.com> <20210818014528.GB39588@laputa> <20210818052340.GD39588@laputa> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210818052340.GD39588@laputa> 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 Heinrich, On Wed, Aug 18, 2021 at 02:23:40PM +0900, AKASHI Takahiro wrote: > On Wed, Aug 18, 2021 at 06:50:40AM +0200, Heinrich Schuchardt wrote: > > Am 18. August 2021 03:45:28 MESZ schrieb AKASHI Takahiro : > > >Heinrich, > > > > > >On Tue, Aug 17, 2021 at 06:02:23PM +0200, Heinrich Schuchardt wrote: > > >> Use enum efi_memory_type and enum_allocate_type in the definitions of the > > >> efi_allocate_pages(), efi_allocate_pool(). > > >> > > >> In the external UEFI API leave the type as int as the UEFI specification > > >> explicitly requires that enums use a 32bit type. > > >> > > >> Signed-off-by: Heinrich Schuchardt > > >> --- > > >> include/efi_loader.h | 9 +++++---- > > >> lib/efi_loader/efi_memory.c | 5 +++-- > > >> 2 files changed, 8 insertions(+), 6 deletions(-) > > >> > > >> diff --git a/include/efi_loader.h b/include/efi_loader.h > > >> index 32cb8d0f1e..c440962fe5 100644 > > >> --- a/include/efi_loader.h > > >> +++ b/include/efi_loader.h > > >> @@ -676,13 +676,14 @@ struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid); > > >> /* Generic EFI memory allocator, call this to get memory */ > > >> void *efi_alloc(uint64_t len, int memory_type); > > >> /* More specific EFI memory allocator, called by EFI payloads */ > > >> -efi_status_t efi_allocate_pages(int type, int memory_type, efi_uintn_t pages, > > >> - uint64_t *memory); > > >> +efi_status_t efi_allocate_pages(enum efi_allocate_type type, > > >> + enum efi_memory_type memory_type, > > >> + efi_uintn_t pages, uint64_t *memory); > > >> /* EFI memory free function. */ > > >> efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages); > > >> /* EFI memory allocator for small allocations */ > > >> -efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, > > >> - void **buffer); > > >> +efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, > > >> + efi_uintn_t size, void **buffer); > > >> /* EFI pool memory free function. */ > > >> efi_status_t efi_free_pool(void *buffer); > > >> /* Returns the EFI memory map */ > > >> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > > >> index be2f655dff..f4acbee4f9 100644 > > >> --- a/lib/efi_loader/efi_memory.c > > >> +++ b/lib/efi_loader/efi_memory.c > > >> @@ -454,7 +454,8 @@ static uint64_t efi_find_free_memory(uint64_t len, uint64_t max_addr) > > >> * @memory allocated memory > > >> * @return status code > > >> */ > > >> -efi_status_t efi_allocate_pages(int type, int memory_type, > > >> +efi_status_t efi_allocate_pages(enum efi_allocate_type type, > > >> + enum efi_memory_type memory_type, > > >> efi_uintn_t pages, uint64_t *memory) > > >> { > > >> u64 len = pages << EFI_PAGE_SHIFT; > > >> @@ -556,7 +557,7 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) > > >> * @buffer: allocated memory > > >> * Return: status code > > >> */ > > >> -efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer) > > >> +efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size, void **buffer) > > > > > >Given the purpose of this patch series, I think that the second argument > > >of this function should be renamed from "pool_type" to "memory_type" > > >which is also used in efi_allocate_pages() to avoid any confusion. > > >(and the description for @pool_type as well) > > > > pool_type is the name used by the UEFI specification. > > So what? > > (for efi_allocate_pages()) > !/* > ! * Allocate memory pages. > ! * > ! * @type type of allocation to be performed > ! * @memory_type usage type of the allocated memory > > !/** > ! * efi_allocate_pool - allocate memory from pool > ! * > ! * @pool_type: type of the pool from which memory is to be allocated > > You give the same type of arguments different names and explanation. > I think that could be confusing. > It is worth noting that efi_allocate_pool() directly passes > the "pool_type" variable to efi_allocate_pages(). Did you ignore my comment? -Takahiro Akashi > -Takahiro Akashi > > > Best regards > > > > Heinrich > > > > > > > >Otherwise, it looks good. > > > > > >-Takahiro Akashi > > > > > > > > >> { > > >> efi_status_t r; > > >> u64 addr; > > >> -- > > >> 2.30.2 > > >> > >