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.8 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 91B25C4338F for ; Wed, 18 Aug 2021 05:23:56 +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 D7C2560F39 for ; Wed, 18 Aug 2021 05:23:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org D7C2560F39 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 81D1D82054; Wed, 18 Aug 2021 07:23:53 +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="L+Q5zYzF"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 607F382054; Wed, 18 Aug 2021 07:23:51 +0200 (CEST) Received: from mail-pf1-x42c.google.com (mail-pf1-x42c.google.com [IPv6:2607:f8b0:4864:20::42c]) (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 C8FA982C89 for ; Wed, 18 Aug 2021 07:23:46 +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-x42c.google.com with SMTP id m26so980611pff.3 for ; Tue, 17 Aug 2021 22:23:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=Qg8UU8mXux361B+NzAsLkQsN6csHkbvhd0l+yxWoIoU=; b=L+Q5zYzFo9pjZ++esWxeNBsP7XVagModNXo9UKgugdDmas7HdqmQUlpDauHEePw1Ef 6QqW2v670C7FBK5afDbvazWfSn/Io6fWCy8XKPBoXUOJzXtBXR339Svr8sGA0M5uF4JN KFjfJ+VhekmLtewSDZpSl8lrZ9WJ/bTWLMxpJvt3co0F15RvaQIxq5eF+JEd28yJYo2k +wxuSXKeL7nnHBQtjfqnaEYkAcpx6aiU2BbUWekzLm0Na2bN4dMfgvUgs7+KvEUHM5Yj SqzbMNXqkwii4fN5/ls92tsQA14YUP9K0t7yLt2g+EBFQ2EAVid/QZvnc/FhOEmk182J EewQ== 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:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=Qg8UU8mXux361B+NzAsLkQsN6csHkbvhd0l+yxWoIoU=; b=McxAIZ1khKhe3URFdSCBaVMla4S9dg+vOZcuFcNnFH3HAzO5b5rmT3hqbh8qW/otEn hzb5rH+SWuGlKh6hY9YWA2e6zZQh7o9XoCm+md6oVtbT9UW4zTkhZoDzcGKxZRh0n1Fi m4ZfeZUCqqrBHR5q6JD7kgV+cf5v6e/UZnhQh8U87llAOkuCtexpxi+2UdGT33qk6xVo GP8xTdiy4YJphcmE/5w7BSc1cvWE9L56SZxlT82XJNK8Aoja4WC74uuzHqIdvWZN3MxD JUCGxuRR0C0giakWorF5IIjDvZNI3HuF76ytUk9xFzLZgElB0SIYvUwoVQIxKHxNtW85 DRog== X-Gm-Message-State: AOAM530nuQ28q6dvGBNIILm/KarvPAkIlWEIsWHMlnuddRWycs53XaTv XC/BRtzlqaYvwEWCt0udIThABg== X-Google-Smtp-Source: ABdhPJxzbu0tuFItc7FmhFMq3schZVDlvT2NVm1QAq1Y1P2elPqm0WY41COiSRoQgK1AbwI3St6phQ== X-Received: by 2002:a63:1d23:: with SMTP id d35mr7071673pgd.357.1629264224782; Tue, 17 Aug 2021 22:23:44 -0700 (PDT) Received: from laputa (pdb6272e8.tkyea130.ap.so-net.ne.jp. [219.98.114.232]) by smtp.gmail.com with ESMTPSA id 23sm5121435pgk.89.2021.08.17.22.23.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 17 Aug 2021 22:23:44 -0700 (PDT) Date: Wed, 18 Aug 2021 14:23:40 +0900 From: AKASHI Takahiro To: Heinrich Schuchardt Cc: 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: <20210818052340.GD39588@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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 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(). -Takahiro Akashi > Best regards > > Heinrich > > > > >Otherwise, it looks good. > > > >-Takahiro Akashi > > > > > >> { > >> efi_status_t r; > >> u64 addr; > >> -- > >> 2.30.2 > >> >