From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x22c.google.com (mail-io0-x22c.google.com [IPv6:2607:f8b0:4001:c06::22c]) (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 CCC3581EA9 for ; Fri, 20 Jan 2017 06:35:46 -0800 (PST) Received: by mail-io0-x22c.google.com with SMTP id l66so62619325ioi.1 for ; Fri, 20 Jan 2017 06:35:46 -0800 (PST) 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=zA7DHpNG8O8eHSJOVFOmtlbQVEpnKb3z0CQFNj59v3E=; b=Yph2TpOhLRUuK2jfPuIo3OVtUvLGfhAoEShjtnYpOBzFuK6+jbm9kGvLpWM1UNwHdK uGu65A2LUIBh8eD8wY455/C1ZswvlSZhfMnFCjFy5gl16gtx7cok/0QZ7/9bxAorR6KL Xky5qABEc5pZH5XHdk7bRUTYritM+Ff18SwJE= 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=zA7DHpNG8O8eHSJOVFOmtlbQVEpnKb3z0CQFNj59v3E=; b=oEqETtnguhxpUfbkE3fXMHz9SAw4HDLr/+Qzv6gfar224m30Fl+bEfAAbdfaOjXwfE IZVevFCnPtevAqxEx2nCPSq7gCslb4wBMkid4ZdHQWJT7rg+Cmx77O6hXjCCIzoRTaxk U8Rwlc5JZH80B11pb/Qu3oC7GweK9uCipDUYDDpnWX4WrSdFWLYVegRF3vYapfVoUkXL t70zjIZaYBQpwifFGY5j/QXd3F14pihTUiVSXrDHJusNAfob9VKwBJCfJZ84rOfUxwv7 DvzkVtRpczVLrmDZa+RtPpE+ye6OIaY8/lux6artG4YTM3Ha53hUNkMkmhH7BBmsP4LW V4hQ== X-Gm-Message-State: AIkVDXK7VuqNMx2D9IBe1SmW5FQY8+R5qTd+/SvfgoFOBNlXBIG57Oszhbs/w0baDdi/BKQ4zTY8ZGYCLS+rbF4I X-Received: by 10.107.18.230 with SMTP id 99mr12657352ios.45.1484922945925; Fri, 20 Jan 2017 06:35:45 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.144.135 with HTTP; Fri, 20 Jan 2017 06:35:45 -0800 (PST) In-Reply-To: References: <1484913996-5062-1-git-send-email-ard.biesheuvel@linaro.org> From: Ard Biesheuvel Date: Fri, 20 Jan 2017 14:35:45 +0000 Message-ID: To: Ryan Harkin Cc: "edk2-devel@lists.01.org" , Leif Lindholm Subject: Re: [PATCH] ArmPkg/ArmLib: remove indirection layer from timer register accessors X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Jan 2017 14:35:47 -0000 Content-Type: text/plain; charset=UTF-8 On 20 January 2017 at 14:32, Ryan Harkin wrote: > Hi Ard, > > On 20 January 2017 at 12:06, Ard Biesheuvel wrote: >> The generic timer support libraries call the actual system register >> accessor function via a single pair of functions ArmArchTimerReadReg() >> and ArmArchTimerWriteReg(), which take an enum to argument to identify >> the register, and return output values by pointer reference. >> >> Since these functions are never called with a non-immediate argument, >> we can simply replace each invocation with the underlying system register >> accessor instead. This is mostly functionally equivalent, with the >> exception of the bounds check for the enum (which is pointless given the >> fact that we never pass a variable), the check for the presence of the >> architected timer (which only makes sense for ARMv7, but is highly unlikely >> to vary between platforms that are similar enough to run the same firmware >> image), and a check for enum values that refer to the HYP view of the timer, >> which we never referred to anywhere in the code in the first place. >> >> So get rid of the middle man, and update the ArmGenericTimerPhyCounterLib >> and ArmGenericTimerVirtCounterLib implementations to call the system >> register accessors directly. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel > > Are there any other patches needed to get this working? > > I've just applied it to the head of EDK2 [1] and I get this error when > building for TC2, FVP and Juno: > > Building ... /linaro/platforms/uefi/edk2/ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf > [AARCH64] > /linaro/platforms/uefi/edk2/ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.c: > In function 'ArmGenericTimerSetTimerFreq': > /linaro/platforms/uefi/edk2/ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.c:51:19: > error: 'CntFrq' undeclared (first use in this function) > ArmWriteCntFrq (CntFrq); > ^ > /linaro/platforms/uefi/edk2/ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.c:51:19: > note: each undeclared identifier is reported only once for each > function it appears in > GNUmakefile:304: recipe for target > '/linaro/platforms/uefi/edk2/Build/ArmVExpress-FVP-AArch64/DEBUG_GCC5/AARCH64/ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib/OUTPUT/ArmGenericTimerPhyCounterLib.obj' > failed > make: *** [/linaro/platforms/uefi/edk2/Build/ArmVExpress-FVP-AArch64/DEBUG_GCC5/AARCH64/ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib/OUTPUT/ArmGenericTimerPhyCounterLib.obj] > Error 1 > Oops! Poor testing on my part: that should be FreqInHz not CntFrq