From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-x235.google.com (mail-lf0-x235.google.com [IPv6:2a00:1450:4010:c07::235]) (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 BE70A81E9E for ; Fri, 20 Jan 2017 07:50:45 -0800 (PST) Received: by mail-lf0-x235.google.com with SMTP id n124so60677650lfd.2 for ; Fri, 20 Jan 2017 07:50:45 -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=69hSLcAA/HKqRKxsyHCmIg+xDFYjlOvToySPxppI9Mw=; b=hq0Irm1Es+Cb8u7Gk6Brb+1UUVYJ78AqhJfd6vjCdxj5luio/IpEBtKVbg+CWI4ybV akIWWSyJu9bq4qzlj96lZgI+eHdtvIAwCC3hHe7zVTtXz/c0q+tbCwYY6MTn8OzVoAaY 6Zf5J7TyEAZaZTAVykmQh9fhR2c2LEHbLEILk= 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=69hSLcAA/HKqRKxsyHCmIg+xDFYjlOvToySPxppI9Mw=; b=qSrd6VlyXLmbOkKMrOzG2vcmcyb0hD2dNPCO/4zFWujXXH7bSgbS1VMxQhOq9GoECH YYPFIIwPm5r1HDt6NsqN2//RXwNVE6fi8iMBqLP4SfGUdGjzAt0mzZSBN9J2FKhkrVko Nt2iSbKUNrStEFmLn4erHR1RChLDn1HL+M94U2SNSggmKdICLNdnOeeEM+6vbRE/z//m hE4G03E+hWdzgNq1JsfVQr5aVV6r8tx8hXtSWtAncDQvlgTcP2focpiz9VR1+5l53WIT wpeOWcUsxwn/fV/e/4wvlYDacuZfq6ur70VzkIXMxzPFCHccyfR7pWli3AU/ybaj8z1p yoFg== X-Gm-Message-State: AIkVDXL4EU1cFmMZgMKvg3JfR+tju/4ngSEpMQ1O4j9pMW9l2ssw9A8lBx+Ju9PI3S5dhCo9GzVIwtwBF55hIE/p X-Received: by 10.25.10.6 with SMTP id 6mr5481007lfk.88.1484927444027; Fri, 20 Jan 2017 07:50:44 -0800 (PST) MIME-Version: 1.0 Received: by 10.25.207.72 with HTTP; Fri, 20 Jan 2017 07:50:43 -0800 (PST) In-Reply-To: References: <1484913996-5062-1-git-send-email-ard.biesheuvel@linaro.org> From: Ryan Harkin Date: Fri, 20 Jan 2017 15:50:43 +0000 Message-ID: To: Ard Biesheuvel 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 15:50:46 -0000 Content-Type: text/plain; charset=UTF-8 On 20 January 2017 at 14:35, Ard Biesheuvel wrote: > 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 Naughty! ;-) Okay, so with that change, it works on TC2, FVP Foundation & AEMv8 and Juno R0/R1/R2: Tested-by: Ryan Harkin