From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x22c.google.com (mail-it0-x22c.google.com [IPv6:2607:f8b0:4001:c0b::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 9EB351A1F53 for ; Sat, 24 Sep 2016 05:05:17 -0700 (PDT) Received: by mail-it0-x22c.google.com with SMTP id j69so7840894itb.0 for ; Sat, 24 Sep 2016 05:05:17 -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=9NAQTzbSFguSffurGdFhRSmSBE5VtcH8U2e8UWGURps=; b=fsCCHhAJFZtiwp+Ho+3vOsEx9HQ5++gdqFqUMgzsabv3yjdepCjRWRdmDGvzlh3b4Q rmtyabMcJp8kAkOJQIt6Z+7Sh99ONpFCZeYQzDHP2uqowbOjiWvP5tf2B1wk4CpP23lF SFErKnwqR+4K3InGRCXsaw2CGz+xxym+eFOc8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=9NAQTzbSFguSffurGdFhRSmSBE5VtcH8U2e8UWGURps=; b=UB4ZUOLeY+LDidH9GJR8eVBhKiFkZ2nKeUYUGyErUGBHsjSqb8tT/lVxh5Gw55TqPu BLAQC+qPkVmqQfg62IK1a9mI813e+Q3rOXMkLzyywqO3IKCaIHnA5YJpgCmdA638aFUH UGTbM3oVk9iRVndTpuscr+VhO3f9K+XAkW0pPc6ya5+XywOrDIlH4NbHZu54c867QD/B Yq66jbmALT9IWIxuJq0MitkMNtu+H2OKUbbhCHSjl7qWND6YbMrppF3KTxrqd9RaA1fF TTGkp6VWShrYuU62iAYGhLa6BphkRas404ypKCPSMIwFoupyXDv8Mx+G7+RL6WwNI8jE Y5xg== X-Gm-Message-State: AA6/9RnR87B+NSKEEoTAA643iHd3McEDaI4hr+GJ8CefH/RhM8yEkYPWqiifmv4OF8O6wzz9d8xlncuOgNtMnEZW X-Received: by 10.36.216.138 with SMTP id b132mr8634845itg.38.1474718716911; Sat, 24 Sep 2016 05:05:16 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.204.195 with HTTP; Sat, 24 Sep 2016 05:05:16 -0700 (PDT) In-Reply-To: References: <1474583581-41663-1-git-send-email-daniil.egranov@arm.com> <1474583581-41663-2-git-send-email-daniil.egranov@arm.com> From: Ard Biesheuvel Date: Sat, 24 Sep 2016 13:05:16 +0100 Message-ID: To: Daniil Egranov Cc: "edk2-devel@lists.01.org" , Leif Lindholm Subject: Re: [PATCH 1/2] ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe: Remove calls to ArmDmaLib 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: Sat, 24 Sep 2016 12:05:17 -0000 Content-Type: text/plain; charset=UTF-8 On 23 September 2016 at 22:47, Daniil Egranov wrote: > > > On 09/23/2016 11:21 AM, Ard Biesheuvel wrote: >> >> On 23 September 2016 at 17:20, Daniil Egranov >> wrote: >>> >>> Hi Ard, >>> >>> >>> On 09/23/2016 02:57 AM, Ard Biesheuvel wrote: >>>> >>>> Hi Daniil, >>>> >>>> On 22 September 2016 at 23:33, Daniil Egranov >>>> wrote: >>>>> >>>>> The PCI on Juno is DMA coherent, which means it should not be >>>>> using ArmDmaLib for PCI DMA. >>>>> >>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>> Signed-off-by: Daniil Egranov >>>>> --- >>>>> >>>>> ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciHostBridgeDxe.inf >>>>> | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git >>>>> >>>>> a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciHostBridgeDxe.inf >>>>> >>>>> b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciHostBridgeDxe.inf >>>>> index de28c80..597154c 100644 >>>>> --- >>>>> >>>>> a/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciHostBridgeDxe.inf >>>>> +++ >>>>> >>>>> b/ArmPlatformPkg/ArmJunoPkg/Drivers/PciHostBridgeDxe/PciHostBridgeDxe.inf >>>>> @@ -36,7 +36,7 @@ >>>>> MemoryAllocationLib >>>>> DxeServicesTableLib >>>>> CacheMaintenanceLib >>>>> - DmaLib >>>>> + NullDmaLib >>>>> >>>> This is wrong. The module .inf lists library *classes* and the >>>> platform .dsc decides how each class maps onto an implementation (aka >>>> library resolution) >>> >>> Agree. However, this is platform specific module and as i understand, the >>> behavior of it will not change so, I think, having NullDmaLib here will >>> be >>> appropriate as well. We can move it to .dsc file, if it fits better to >>> the >>> platform description/module dependencies structure. >>> >> What you could do is remove the DmaLib dependency altogether, and just >> implement PciIo->Map and PciIo->Unmap directly, and always return the >> host address. > > This implementation will be almost the same as NullDmaLib one :). If you > think it will be better structure-wise to remap it in .dsc file, let's go > this path and disregard this patch. > The [LibraryClasses] section expects library classes, and NullDmaLib is not a library class. So putting this here is simply wrong. This is not a matter of taste. Also, another platform could potentially use the same PCIe IP and integrate it in a non-coherent fashion, so it is not a property of the device if it is coherent or not. So forget about my suggestion to drop the DmaLib dependency. Just add NullDmaLib to the .dsc, but for this driver only.