From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:4864:20::344; helo=mail-wm1-x344.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm1-x344.google.com (mail-wm1-x344.google.com [IPv6:2a00:1450:4864:20::344]) (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 E0E8321180F34 for ; Fri, 9 Nov 2018 08:29:23 -0800 (PST) Received: by mail-wm1-x344.google.com with SMTP id 124-v6so2610823wmw.0 for ; Fri, 09 Nov 2018 08:29:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=ev/euzFIFcLBYzb9RqH2Ta7q1cCLIkwr4NSUmMsYN20=; b=iAW5zqPDE7s2VtQx+YIBWkfXc6uMPVlb3IraOY/RljC6c5laOm9pIfeC1h+ecp/Wua EAGG9hyXwX5D6xZaDblxq2xx5OzITkV5Jxidcaxryjv/YEb9K5pB6qfPVnVBfOaBnjFo utfUuH12Vlm6MA7a4d79t8BT6+WSyAvQKAMyQ= 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:references :mime-version:content-disposition:in-reply-to:user-agent; bh=ev/euzFIFcLBYzb9RqH2Ta7q1cCLIkwr4NSUmMsYN20=; b=UbexG7Wmrq3vZF7Xyu6cdYWx6E7o6RdQjoVd5cprnuXBbitgLnDXwmShDzjOQ6brFv lOygYyh+2ghUtJ0fbJ/wxuoNoooImXbWn3ueHout++EXWCB4WalHAewwEoCJ/IPxVWEh c2qThJt5+b4ZrNjTOtegUDQBYXQD9HTxvVE8afcxeKy1qMZmYWK8BEmzhr7u9bqBRSe0 zoAFJgi2rrqYxKmAWJHp5P9DLvXDMDs9etFZ+iWCD+9KxBfYyrAn+tlxpjD1aGTg4t0s r3GmUIn7nppxam0F39IRqRAgmE6IyttiASMqlX3KcOIUjWU9ytKIwyq/H0uSeaH2OnDf LO3w== X-Gm-Message-State: AGRZ1gLoRUIALttUmRq6yhrNgdtq4W5AvfdJlp4Polh42e0J3wj5GIFd KzG+nwnJQqzn26L0Vv9vvhDhKA== X-Google-Smtp-Source: AJdET5eY0K9ZhySZaU7RgDR8I0rDAUvotoZeDqOatXt9XetrWwXQGa51J/cbD91yMehei0Uyb6H73A== X-Received: by 2002:a7b:c10e:: with SMTP id w14-v6mr66983wmi.20.1541780961990; Fri, 09 Nov 2018 08:29:21 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id y2-v6sm13702091wrh.53.2018.11.09.08.29.20 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 09 Nov 2018 08:29:20 -0800 (PST) Date: Fri, 9 Nov 2018 16:29:18 +0000 From: Leif Lindholm To: Chandni Cherukuri Cc: edk2-devel@lists.01.org, ard.beisheuvel@linaro.org Message-ID: <20181109162918.biighqlpmr3ueesw@bivouac.eciton.net> References: <1541410019-1781-1-git-send-email-chandni.cherukuri@arm.com> <1541410019-1781-2-git-send-email-chandni.cherukuri@arm.com> MIME-Version: 1.0 In-Reply-To: <1541410019-1781-2-git-send-email-chandni.cherukuri@arm.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH v2 edk2-platforms 1/5] Platform/ARM/Sgi: Adapt to changes in system-id DT node. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 09 Nov 2018 16:29:24 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Nov 05, 2018 at 02:56:55PM +0530, Chandni Cherukuri wrote: > The 'system-id' node of HW_CONFIG device tree has been updated to have > a new property 'config-id' to hold the platform configuration value. > Prior to this, configuration ID value was represented by the the upper > four bits of the 'platform ID' property value but it now has a seperate > property of its own in the 'system-id' node. So adapt to these changes > in the 'system-id' node. This gets a bit hairy semantically: - PlatformId used to be the contents of node "platform-id" - PlatformId is now the union of the lower 28 bits of node "platform-id" and the 4 bottom bits of new node "config-id", - The result is still called PlatformId. Is there some better way of describing this? Should the function name change? > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Chandni Cherukuri > --- > .../SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c | 19 ++++++++++++++++++- Can you ensure you generate patches in accordance with the instructions at https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-23 ? > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c > index 081d329..8b6c71b 100644 > --- a/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c > +++ b/Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c > @@ -42,6 +42,8 @@ GetSgiPlatformId ( Since this is changing what the return value of the function means, can the function description comment be updated as well? > CONST VOID *HwCfgDtBlob; > SGI_HW_CONFIG_INFO_PPI *HwConfigInfoPpi; > EFI_STATUS Status; > + UINT32 PlatformId; > + UINT32 ConfigId; > > Status = PeiServicesLocatePpi (&gHwConfigDtInfoPpiGuid, 0, NULL, > (VOID**)&HwConfigInfoPpi); > @@ -69,7 +71,22 @@ GetSgiPlatformId ( > return 0; > } > > - return fdt32_to_cpu (*Property); > + PlatformId = fdt32_to_cpu (*Property); > + > + Property = fdt_getprop (HwCfgDtBlob, Offset, "config-id", NULL); > + if (Property == NULL) { > + DEBUG ((DEBUG_ERROR, "Config Id is NULL\n")); > + return 0; > + } > + > + ConfigId = fdt32_to_cpu (*Property); > + > + // Concatenate the value of config ID into the platform ID value with > + // config ID occupying the upper 4 bits of platform ID. > + ConfigId = ConfigId & SGI_CONFIG_MASK; We are here silently discarding 28 bits of data that need to be set to 0 for the below operation to make sense. I think an ASSERT might be in order. I also note that we've neither looked at nor cleared the top four bits of PlatformId. > + PlatformId = PlatformId | (ConfigId << SGI_CONFIG_SHIFT); > + > + return PlatformId; I would be happier with just the return statement - reusing the PlatformId variable does not simplify anything. / Leif. > } > > /** > -- > 2.7.4 >