From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::243; helo=mail-it0-x243.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x243.google.com (mail-it0-x243.google.com [IPv6:2607:f8b0:4001:c0b::243]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id DC052210F16A2 for ; Mon, 18 Jun 2018 10:05:39 -0700 (PDT) Received: by mail-it0-x243.google.com with SMTP id j135-v6so13041706itj.1 for ; Mon, 18 Jun 2018 10:05:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=Lq7Izwq9nGdB/4o6I0x3GIZGOmZDXTng3c5fBQVY3Ek=; b=fFtXagtf2QjXa7Adyeat3Idpxl9+J0Vq/LCL65lBVQ21rRGziLMaptJtqNkYSPXHfU 9QmUux8/NsBKFcEthIsqoHX4skJ7Yx3vG82LxurqprkNlCRouyGf1YL6hddpc9r8pzgy 2fqLfNH33FYXoQtwJDiH89A/Qehncnz0WOPWY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Lq7Izwq9nGdB/4o6I0x3GIZGOmZDXTng3c5fBQVY3Ek=; b=ZNaVD7da+0fyERVIgSIyltbwr/0+NSMe1qLDP7uLKnDw5wrT29R6GxuWtbyd8VFXfA 03mrDi2EEua2E7dw4UdEw96UH5CnSbTZSGXwhyU8biv3kdjxhv82b/PPu8k/TBHyHZ/W ITAjgbocQOMv1QJ9bqPmESJPtqdzhDean8jT+g4HIEyL1nmp4kC28RYQeTsepTCeZwxv BcVSLcoSUau17hJ/hPsnItfrgtfgqgQjvfhTKS8a82U357YjuYWs7ISYAJSLuZznZof+ xixR9aiwoPw7PrhKM5nBYkJBoymcjqCTuJjAtptchnrFplGx/vHWtGpJaBDyC+v/jS0h vL5g== X-Gm-Message-State: APt69E3sdjwwQeY9p61oFhH2mMqFCmgIrT788xm3KXcttccUpAY47sGR ipWvuQzyNewkI7pCwIKjeLHQJRwLCC4GqnBPODOXbA== X-Google-Smtp-Source: ADUXVKIFBZZ0pWa12R/0ndbUQxaDclEOqlwduG71VwHoRkYmvK7lzG3vPP0kovwDec3TsVbef2fzWMu0g91GuCGalIw= X-Received: by 2002:a02:6001:: with SMTP id i1-v6mr10848264jac.5.1529341538743; Mon, 18 Jun 2018 10:05:38 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bbc7:0:0:0:0:0 with HTTP; Mon, 18 Jun 2018 10:05:38 -0700 (PDT) In-Reply-To: <20180615141020.9428-3-girish.pathak@arm.com> References: <20180615141020.9428-1-girish.pathak@arm.com> <20180615141020.9428-3-girish.pathak@arm.com> From: Ard Biesheuvel Date: Mon, 18 Jun 2018 19:05:38 +0200 Message-ID: To: Girish Pathak Cc: "edk2-devel@lists.01.org" , Leif Lindholm , Matteo Carlini , Stephanie Hughes-Fitt , nd Subject: Re: [PATCH v1 2/2] ArmPkg/ArmScmiDxe: Dynamically allocate buffer for protocol ids X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Jun 2018 17:05:40 -0000 Content-Type: text/plain; charset="UTF-8" On 15 June 2018 at 16:10, Girish Pathak wrote: > Dynamically allocate the buffer to receive the SCMI protocol list. > This makes MAX_PROTOCOLS redundant, so it is removed. > It also fixes one minor code alignment issue and removes an unused > macro PROTOCOL_MASK. > > Change-Id: Idc5880d4fb7b5c674f5fb7dce1198a7cad0303a7 Can you please get rid of these change IDs? They have no meaning in the context of the upstream repo. > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Girish Pathak > --- > ArmPkg/Drivers/ArmScmiDxe/Scmi.c | 5 ---- > ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.c | 27 +++++++++++++++----- > ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.h | 1 - > 3 files changed, 21 insertions(+), 12 deletions(-) > > diff --git a/ArmPkg/Drivers/ArmScmiDxe/Scmi.c b/ArmPkg/Drivers/ArmScmiDxe/Scmi.c > index 1e279f69cf615428dbb6477b8ac7de3258628df3..d247d3a932fe9f197460a95e9afa88681742e4b4 100644 > --- a/ArmPkg/Drivers/ArmScmiDxe/Scmi.c > +++ b/ArmPkg/Drivers/ArmScmiDxe/Scmi.c > @@ -22,11 +22,6 @@ > > #include "ScmiPrivate.h" > > -// SCMI Specification 1.0 > -#define MAX_PROTOCOLS 6 > - > -#define PROTOCOL_MASK 0xF > - > // Arbitrary timeout value 20ms. > #define RESPONSE_TIMEOUT 20000 > > diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.c b/ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.c > index cc68cbc922fcc06bff8f7e0aa8b6bf64a9932874..e7b9feca5e1b565fc031385bfe2f2dc0ca53aab0 100644 > --- a/ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.c > +++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.c > @@ -63,8 +63,8 @@ ArmScmiDxeEntryPoint ( > UINT32 Index; > UINT32 NumProtocols; > UINT32 ProtocolIndex = 0; > - UINT8 SupportedList[MAX_PROTOCOLS]; > - UINT32 SupportedListSize = sizeof (SupportedList); Ah excellent. Forget what I said about this line in the previous message. > + UINT8 *SupportedList; > + UINT32 SupportedListSize; > > // Every SCMI implementation must implement the base protocol. > ASSERT (Protocols[ProtocolIndex].Id == SCMI_PROTOCOL_ID_BASE); > @@ -108,13 +108,26 @@ ArmScmiDxeEntryPoint ( > > ASSERT (NumProtocols != 0); > > + SupportedListSize = (NumProtocols * sizeof (*SupportedList)); > + > + Status = gBS->AllocatePool ( > + EfiBootServicesData, > + SupportedListSize, > + (VOID**)&SupportedList > + ); > + if (EFI_ERROR (Status)) { > + ASSERT (FALSE); > + return Status; > + } > + > // Get the list of protocols supported by SCP firmware on the platform. > Status = BaseProtocol->DiscoverListProtocols ( > - BaseProtocol, > - &SupportedListSize, > - SupportedList > - ); > + BaseProtocol, > + &SupportedListSize, > + SupportedList > + ); > if (EFI_ERROR (Status)) { > + gBS->FreePool (SupportedList); > ASSERT (FALSE); > return Status; > } > @@ -133,5 +146,7 @@ ArmScmiDxeEntryPoint ( > } > } > > + gBS->FreePool (SupportedList); > + > return EFI_SUCCESS; > } > diff --git a/ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.h b/ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.h > index 5815e1e78074067760b6f618e248526ee25e59c8..b50a52a01d47efbbccec8c97bf44041c47ff8b38 100644 > --- a/ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.h > +++ b/ArmPkg/Drivers/ArmScmiDxe/ScmiDxe.h > @@ -19,7 +19,6 @@ > > #include "ScmiPrivate.h" > > -#define MAX_PROTOCOLS 6 > #define MAX_VENDOR_LEN SCMI_MAX_STR_LEN > > /** Pointer to protocol initialization function. > -- > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' > >