From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c0c::241; helo=mail-wr0-x241.google.com; envelope-from=haojian.zhuang@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x241.google.com (mail-wr0-x241.google.com [IPv6:2a00:1450:400c:c0c::241]) (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 A568C21EBD1C0 for ; Thu, 3 May 2018 20:29:35 -0700 (PDT) Received: by mail-wr0-x241.google.com with SMTP id g21-v6so19539858wrb.8 for ; Thu, 03 May 2018 20:29:35 -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=eC+VMBNR0J58KhYnL1WgTy5c5k4vi9SLkIylV9cEOnY=; b=CLsibIKsh+vBS0tEKylEfoGpBc47jq4McPNioYGX084pqKrpu9cr7PMEu0gMwuYfDL IHTkFgG2JRAbOTfecfMmnQshxeD8EWHg90X5EHBgDfkM3Ek+MbWpRoHR8ZwfnbGi/UTu 8w+YL0OP4M/BhXlYoh8bZUWip2Sz8P44k7ZcI= 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=eC+VMBNR0J58KhYnL1WgTy5c5k4vi9SLkIylV9cEOnY=; b=rHcz1A9DAN7qNHt/ECyKgBjasnnNdEtBituzlQdVzc/CrI5HZiHnoGgTfS6lsXi/5d JLb02eMhkmwtOyy6pnmK6pwSQyvneZnA39NLCiXpp+qTuyD+CjO3jG/KH9p6A7P7qOOy jTkHDBX3Zin3/Reke8Yn/VIuiW9EP3JUl+Adnvm7qkrnUnA7w3KmmUAYIE2/rqa43Ea7 6wvIMnr6HAnL0b016bAgURFGiEN6OAhMA/VwvulDny5zE8UV5YwV5YfHcDQEPcGCdiBE VGFB9kOWg8BiiQhCTKvhzU2pBrPZg4Snbapo8xSTv4cEJ2Ode+5bqiIr+DM/yr5lutZl 7KgA== X-Gm-Message-State: ALQs6tBm5lFNa9FGxRp+UfXqo5TfgM5SEuwQQduGcq5NmV0GQP3fILfY NwgRnhgHrkFB0Mb7GxmxkToYGDzyXRjfy5tRwIlUeA== X-Google-Smtp-Source: AB8JxZrKJ85h95XPL8n5jf5+lq/+dmaHeuDxkummZuHa9S3UThfcPVZS/KBYZdYrrSf31XZDTMlBEIMNzNcEZeyglt0= X-Received: by 2002:adf:b00f:: with SMTP id f15-v6mr22341928wra.254.1525404574079; Thu, 03 May 2018 20:29:34 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.150.22 with HTTP; Thu, 3 May 2018 20:29:33 -0700 (PDT) In-Reply-To: <20180502151435.46zbhyux5vh45hh2@bivouac.eciton.net> References: <1520515790-29527-1-git-send-email-haojian.zhuang@linaro.org> <20180502151435.46zbhyux5vh45hh2@bivouac.eciton.net> From: Haojian Zhuang Date: Fri, 4 May 2018 11:29:33 +0800 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" , Ard Biesheuvel Subject: Re: [PATCH v2 edk-platforms 1/4] Platform/Hisilicon/HiKey960: add gpio platform driver 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: Fri, 04 May 2018 03:29:36 -0000 Content-Type: text/plain; charset="UTF-8" On 2 May 2018 at 23:14, Leif Lindholm wrote: > On Thu, Mar 08, 2018 at 09:29:50PM +0800, Haojian Zhuang wrote: >> Add gpio platform driver to enable GPIO in HiKey960 platform. >> >> Cc: Leif Lindholm >> Cc: Ard Biesheuvel >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Haojian Zhuang >> --- >> Platform/Hisilicon/HiKey960/HiKey960.dsc | 1 + >> Platform/Hisilicon/HiKey960/HiKey960.fdf | 1 + >> Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf | 35 +++++++++ >> Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.c | 77 ++++++++++++++++++++ >> 4 files changed, 114 insertions(+) >> >> diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc b/Platform/Hisilicon/HiKey960/HiKey960.dsc >> index 36f43956ab40..3da1b8556321 100644 >> --- a/Platform/Hisilicon/HiKey960/HiKey960.dsc >> +++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc >> @@ -179,6 +179,7 @@ [Components.common] >> # >> # GPIO >> # >> + Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf >> ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf >> >> # >> diff --git a/Platform/Hisilicon/HiKey960/HiKey960.fdf b/Platform/Hisilicon/HiKey960/HiKey960.fdf >> index 655032a36c53..162dbaaf2646 100644 >> --- a/Platform/Hisilicon/HiKey960/HiKey960.fdf >> +++ b/Platform/Hisilicon/HiKey960/HiKey960.fdf >> @@ -120,6 +120,7 @@ [FV.FvMain] >> # >> # GPIO >> # >> + INF Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf >> INF ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf >> >> # >> diff --git a/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf b/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf >> new file mode 100644 >> index 000000000000..a16213f02520 >> --- /dev/null >> +++ b/Platform/Hisilicon/HiKey960/HiKey960GpioDxe/HiKey960GpioDxe.inf >> @@ -0,0 +1,35 @@ >> +# >> +# Copyright (c) 2018, Linaro. All rights reserved. >> +# >> +# 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 = 0x00010019 > > Probably bump this to 0x0001001a by now. > OK > > Missing space before '='. > OK >> + { 0xe8a0b000, 0, 8 }, // GPIO0 > > It would not improve readability to request all of these live-coded > values to be replaced by #defines, but barring that, could you add a > comment header before the definition?: > > // { base address, gpio index, gpio count } > OK >> + { 0xe8a0c000, 8, 8 }, // GPIO1 >> + { 0xe8a0d000, 16, 8 }, // GPIO2 >> + { 0xe8a0e000, 24, 8 }, // GPIO3 >> + { 0xe8a0f000, 32, 8 }, // GPIO4 >> + { 0xe8a10000, 40, 8 }, // GPIO5 >> + { 0xe8a11000, 48, 8 }, // GPIO6 >> + { 0xe8a12000, 56, 8 }, // GPIO7 >> + { 0xe8a13000, 64, 8 }, // GPIO8 >> + { 0xe8a14000, 72, 8 }, // GPIO9 >> + { 0xe8a15000, 80, 8 }, // GPIO10 >> + { 0xe8a16000, 88, 8 }, // GPIO11 >> + { 0xe8a17000, 96, 8 }, // GPIO12 >> + { 0xe8a18000, 104, 8 }, // GPIO13 >> + { 0xe8a19000, 112, 8 }, // GPIO14 >> + { 0xe8a1a000, 120, 8 }, // GPIO15 >> + { 0xe8a1b000, 128, 8 }, // GPIO16 >> + { 0xe8a1c000, 136, 8 }, // GPIO17 >> + { 0xff3b4000, 144, 8 }, // GPIO18 >> + { 0xff3b5000, 152, 8 }, // GPIO19 > > I notice that these: >> + { 0xe8a1f000, 160, 8 }, // GPIO20 >> + { 0xe8a20000, 168, 8 }, // GPIO21 > are out of order, from their base address. > Are the names GPIO20/GPIO21 defined in the TRM? > Yes, it's mentioned in the TRM. (https://github.com/96boards/documentation/blob/master/consumer/hikey960/hardware-docs/HiKey960_SoC_Reference_Manual.pdf) >> + 232, 29, gGpioDevice > > And similarly > // { global gpio count, gpio controller count, GPIO_CONTROLLER } > OK Best Regards Haojian