From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by mx.groups.io with SMTP id smtpd.web12.616.1583365099131279627 for ; Wed, 04 Mar 2020 15:38:19 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=iIcwMVyX; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.128.68, mailfrom: pete@akeo.ie) Received: by mail-wm1-f68.google.com with SMTP id g134so4212846wme.3 for ; Wed, 04 Mar 2020 15:38:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=wNKA1cOuTsdKbSeyoGZrl99qAAVAFYt86O2WP2qpCCo=; b=iIcwMVyXH6avhqFE+fr4OCHaKzuQqFahZMLCJBGfYluJzlLy3zmcaixf4Jtks04f26 KZJ526ZHxqUPfTX09G+UYc+yLJma5uhu8JPhv38CcalutDoXVO+I914dUI0c6GblZrve i/8/IkBPXhGcgCJ8AwoDNz9OCWYdGls0p537j9EYdO790QragBmuZHevRYmwI7ZykGDl w9n4k0aHXyj2N2VM5YgOcP3s5R8iuk8MVynbjUiu3y8aHkw2P3quBgzwcqCMvkIQZHkP 4DzGiD5ri9eFiLgEcmbqc7ji5XroXmCRcvXIpe1W7uGcimJz5tf4DAD7MVQvGtowZjN9 c5Zw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=wNKA1cOuTsdKbSeyoGZrl99qAAVAFYt86O2WP2qpCCo=; b=ehsBcjTSyYL+b8roGdQW9F0oT2lzG9/xWK+rJEee4ytyvFP+IyCdOc3Mo6IVk41jcc yO1lHXylnVixGGewywJqYKlsmkeHJEBWmCgaWViKBq3Q2RJqsFDCfd0czeS3Hy3HkXg/ 0rLAILidPYYTJtyiSDbSsNLfROH805h9r/OoXpEVLtRSNm1323UjBPG3WAgfOK4TuCKw FjUS5BIC3tXbS+nFF+/PFdUAs9YTzWZFsfQHNYjcPYUYjWbG844SegdODNvCRhDaEtI1 31v9JeLMSMKMMDX4dQldMuI52SQ81V3kvNdk1O/QvAO3BCbO+GPa590Sr3a/GF80CGwn 3GXw== X-Gm-Message-State: ANhLgQ0QrnIWr55qHyP9vTW12NX7iFUkxbw6z/heY22KZOmKjvbf+7gF ngPcs6E6lQHkkjJxJzqjZTl7IQ== X-Google-Smtp-Source: ADFU+vsuJCnjesJC2MTyxJYfnbdD+Q6J3o0a1O2nHh+aD5ZLqSyGmVvdA8lkoNWCwvThpDZAlmilrg== X-Received: by 2002:a1c:ba42:: with SMTP id k63mr1449635wmf.71.1583365097718; Wed, 04 Mar 2020 15:38:17 -0800 (PST) Return-Path: Received: from [10.0.0.122] ([84.203.49.209]) by smtp.googlemail.com with ESMTPSA id c9sm7833768wmc.3.2020.03.04.15.38.16 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 04 Mar 2020 15:38:16 -0800 (PST) Subject: Re: [edk2-platforms][PATCH 1/2] Platform/RaspberryPi/Drivers/ConfigDxe: fix bug in 3GB RAM logic To: Andrei Warkentin , "devel@edk2.groups.io" Cc: "ard.biesheuvel@linaro.org" , "leif@nuviainc.com" , "philmd@redhat.com" References: <20200304223056.116868-1-awarkentin@vmware.com> <20200304223056.116868-2-awarkentin@vmware.com> From: "Pete Batard" Message-ID: <3849ba68-378d-88ab-dd70-9ab19bf35853@akeo.ie> Date: Wed, 4 Mar 2020 23:38:15 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: <20200304223056.116868-2-awarkentin@vmware.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit On 2020.03.04 22:31, Andrei Warkentin wrote: > The original change*** used positive logic (PcdPi4GBEnabled), while the > upstreamed change uses negative logic (PcdRamLimitTo3GB), which > requires an additional condition, or it blows up on 1GiB and 2GiB boards. Good catch! > Tested on 2GB and 4GB boards (with limiting and without) > > *** https://github.com/pftf/edk2-platforms/\ > commit/968451beb7c9302517098abf72f7e42b57a0e024 > > Signed-off-by: Andrei Warkentin > --- > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > index ccac8daa..5fca3c7a 100644 > --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > @@ -288,7 +288,8 @@ ApplyVariables ( > DEBUG ((DEBUG_INFO, "Current CPU speed is %uHz\n", Rate)); > } > > - if (mModelFamily >= 4 && PcdGet32 (PcdRamLimitTo3GB) == 0) { > + if (mModelFamily >= 4 && PcdGet32 (PcdRamMoreThan3GB) && Just a super minor nitpick, but if we want to be consistent with the next line, where we use the comparison to the integer value as the boolean, instead of using the result of the PcdGet operation itself, then the condition above should be: if (mModelFamily >= 4 && PcdGet32 (PcdRamMoreThan3GB) != 0 && Since it doesn't matter either way, I'm 100% fine with the patch as it stands. It's just that I remember trying to go for an explicit integer comparison for code clarity, on account that we went for integer PCDs rather than boolean ones... > + PcdGet32 (PcdRamLimitTo3GB) == 0) { > /* > * Similar to how we compute the > 3 GB RAM segment's size in PlatformLib/ > * RaspberryPiMem.c, with some overlap protection for the Bcm2xxx register > Reviewed-by: Pete Batard