From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x231.google.com (mail-it0-x231.google.com [IPv6:2607:f8b0:4001:c0b::231]) (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 08ADA81E9E for ; Fri, 20 Jan 2017 07:51:22 -0800 (PST) Received: by mail-it0-x231.google.com with SMTP id c7so23483798itd.1 for ; Fri, 20 Jan 2017 07:51:22 -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=2bOif1er6TCycL47JTaAUmApWxOCjhXitD9F7ssPBaA=; b=BPf5y9o+j4YZeddpmISu0Gq4kaNUjqyqdu0xAw61nRn0JTaAUPVVybrmURyyDUGGh9 AwPPpPmIhwLK5vSNgDc/W4mPTNp5GfjVFTLwVcnY6hftgbQd8ELD05FEvHr0OMgatota UQlktpFF3WUL5IVyzUUP2M+Qh/jpWAngUO7zo= 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=2bOif1er6TCycL47JTaAUmApWxOCjhXitD9F7ssPBaA=; b=BjXjKB0gtk0Pkuf5XIwS8a+GrRbmUTUlJVyJLtcCn66rhW0xV8ghGlY4jzQLmJ3B2e 6DfJOr9vr7exG7GiN+IsO91aboEgDZOrur5sOE2UbihMM11ri/gbHS/UQAB1qJbVBfDF 8FIpLKMJx+/hWcfibdzCiBV4oq0xPZfDUIxc7xEY37qSQ9pvtEHd1YxkSoGLGnF+aXM9 DsjSJ+IUYBV864/aR32zFkuG2chOlRL3Uu/EW0OHSoXs0X0iaCiDcVUH6nJQ4MUviIm0 thC7DIt6uYb9Npn2VBMPZwIaZhOPBj4DozphLhZWzjRwV2RTGgPYtpOOcyGGhO+gccbw PNjw== X-Gm-Message-State: AIkVDXIkJ8wuBeea6IbQhq/hWrbtoV8LFlGl/+0vzmEcu/2A0VksVfACG7GVTUDB4D4bdoglNNbYRzpv1kWlmPMv X-Received: by 10.36.74.67 with SMTP id k64mr3920873itb.37.1484927481140; Fri, 20 Jan 2017 07:51:21 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.144.135 with HTTP; Fri, 20 Jan 2017 07:51:20 -0800 (PST) In-Reply-To: References: <1484913996-5062-1-git-send-email-ard.biesheuvel@linaro.org> From: Ard Biesheuvel Date: Fri, 20 Jan 2017 15:51:20 +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 15:51:22 -0000 Content-Type: text/plain; charset=UTF-8 On 20 January 2017 at 15:50, Ryan Harkin wrote: > 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 Splendid! Thanks