From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::243; helo=mail-io0-x243.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x243.google.com (mail-io0-x243.google.com [IPv6:2607:f8b0:4001:c06::243]) (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 AD78C2117A295 for ; Tue, 5 Jun 2018 05:23:49 -0700 (PDT) Received: by mail-io0-x243.google.com with SMTP id l25-v6so3043172ioh.12 for ; Tue, 05 Jun 2018 05:23:49 -0700 (PDT) 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=mddiBkNkjqqJ+27zUEGNdkzD07fA0jtHIafnAjdLh4A=; b=fzo9cqqaUyQS8cTF/T6d5JIasdJ8LWVUwAtOkHdX9Hgs6+rNCcgXqr8xjvQTx2qr0A awDzy2cJ6g8lf15Tm/eOeoAeKMURAjMF8hYaYv5kLL2Bc7DlptRU3dUYMtn+1r7Y92Hb SdWE6a9T+OtSON55B1dwrzTaHnhuHNv9g09dM= 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=mddiBkNkjqqJ+27zUEGNdkzD07fA0jtHIafnAjdLh4A=; b=Vgf3iLuTU7du6e2LKfMDUMqGrBiYDPTK7c0SWG1vTwQSYeOhs5GN/VPX+fY3Xp0/Og PL01w11kUiRSvnffvaNXuyn2BobCJsjI1lBAsiU85iQQeX0MTEckWhtvpu8lFLWR5kCu GnKMKmQYNG91RuEXSIOX1wVWTXd7gIF2q9pOlehmR5/Ev3XtjSSXABfQVieJqQlvL/Fd PSkj54MZpSAYTWmKXk8gNpplBiKQ30qYAN96pWiBfOS5pve84L+XBSXyhTgfw+3+miDE GjPea/FLI/H2uJbWKBNckk5MRyZYnx0odYs705BWe1Q3owPH0lcrqgScARhAUbVvfBI3 MQ2Q== X-Gm-Message-State: APt69E1PruUTTOKyC9+XXziz5AW55XzMtuj3mD4VJN3zyO7Sr5ykqXcz CZ2ir9I8J+5nardg79nrsMPE0y2e97fi/2SqQPPAGuH9JYY= X-Google-Smtp-Source: ADUXVKJeRHQqQv0EAocJnXWNLlrqo3ldLv7CWYRqubYaxobrqOTOn3+pOXu34j8Ocl4NPb7cmhYIILd5dg8/M6uFd6c= X-Received: by 2002:a6b:6709:: with SMTP id b9-v6mr24595503ioc.170.1528201428667; Tue, 05 Jun 2018 05:23:48 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bb86:0:0:0:0:0 with HTTP; Tue, 5 Jun 2018 05:23:48 -0700 (PDT) In-Reply-To: <1528155339-5050-1-git-send-email-udit.kumar@nxp.com> References: <1528155339-5050-1-git-send-email-udit.kumar@nxp.com> From: Ard Biesheuvel Date: Tue, 5 Jun 2018 14:23:48 +0200 Message-ID: To: Udit Kumar Cc: "edk2-devel@lists.01.org" , Leif Lindholm Subject: Re: [PATCH 1/2] ArmPlatformPkg: PL011 Dynamic clock freq Support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Jun 2018 12:23:49 -0000 Content-Type: text/plain; charset="UTF-8" On 5 June 2018 at 01:35, Udit Kumar wrote: > Some platform support dynamic clocking, Which is controlled > by some jumper setting or hardware registers. > Result of that PCD PL011UartClkInHz needs to be updated for > frequency change. > This patch implements support for dynamic frequency for > PL011 uart. > This patch implement NULL lib for such platform where > Pcd clock frequency to PL011 can change > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Udit Kumar I'm not crazy about this. How exactly are you reading the frequency in your case? > --- > .../Include/Library/ArmPlatformClockLib.h | 32 ++++++++++++++++++++ > .../ArmPlatformClockLib.inf | 33 ++++++++++++++++++++ > .../ArmPlatformClockLibNull.c | 35 ++++++++++++++++++++++ > 3 files changed, 100 insertions(+) > create mode 100644 ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h > create mode 100644 ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf > create mode 100644 ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c > ArmPlatformClockLib doesn't make sense, since this is specific to PL011, no? > diff --git a/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h b/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h > new file mode 100644 > index 0000000..f9d1425 > --- /dev/null > +++ b/ArmPlatformPkg/Include/Library/ArmPlatformClockLib.h Please add new library classes to the package .dec file > @@ -0,0 +1,32 @@ > +/** @file > +* > +* Copyright 2018 NXP > +* > +* This program and the accompanying materials > +* are licensed and made available under the terms and conditions of the BSD License > +* which accompanies this distribution. The full text of the license may be found at > +* http://opensource.org/licenses/bsd-license.php > +* > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > +* > +**/ > + > +#ifndef _ARMPLATFORMCLOCKLIB_H_ > +#define _ARMPLATFORMCLOCKLIB_H_ > + > + > +/** > + Return frequency of PL011. > + > + If this function return 0 then fixed value in Pcd will be used > + > + @return Return frequency of PL011 > + > +**/ > +UINT32 > +ArmPlatformGetPL011ClockFreq ( > + VOID > + ); > + > +#endif > diff --git a/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf > new file mode 100644 > index 0000000..b708ad3 > --- /dev/null > +++ b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLib.inf > @@ -0,0 +1,33 @@ > +#/* @file > +# Copyright 2018 NXP > +# > +# This program and the accompanying materials > +# are licensed and made available under the terms and conditions of the BSD License > +# which accompanies this distribution. The full text of the license may be found at > +# http://opensource.org/licenses/bsd-license.php > +# > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > +# > +#*/ > + > +[Defines] > + INF_VERSION = 0x0001000A > + BASE_NAME = ArmPlatformClockLibNull > + FILE_GUID = af8fef24-afbb-472a-b8b7-13101a79703c > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = ArmPlatformClockLib > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + ArmPkg/ArmPkg.dec > + ArmPlatformPkg/ArmPlatformPkg.dec > + > +[LibraryClasses] > + ArmLib > + DebugLib > + > +[Sources.common] > + ArmPlatformClockLibNull.c > diff --git a/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c > new file mode 100644 > index 0000000..28eaa63 > --- /dev/null > +++ b/ArmPlatformPkg/Library/ArmPlatformClockLibNull/ArmPlatformClockLibNull.c > @@ -0,0 +1,35 @@ > +/** @file > +* > +* Copyright 2018 NXP > +* > +* This program and the accompanying materials > +* are licensed and made available under the terms and conditions of the BSD License > +* which accompanies this distribution. The full text of the license may be found at > +* http://opensource.org/licenses/bsd-license.php > +* > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > +* > +**/ > + > +#include > + > + > + > +/** > + Return clock in for PL011 Uart IP. > +**/ > +UINT32 > +ArmPlatformGetPL011ClockFreq ( > + VOID > + ) > +{ > + // Implement platform specific code > + // and return PL011 freq > + > + // Some platform supports dynamic clocking, > + // This function needs to be implemented on platforms which supports > + // dynamic clocking to avoid re-building of UEFI firmware for PL011 > + // clock change > + return 0; Why not return the PCD value here? That way, you can also get rid of the horrid conditional expression in the next patch. > +} > -- > 1.9.1 >