From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x229.google.com (mail-it0-x229.google.com [IPv6:2607:f8b0:4001:c0b::229]) (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 E556581ED9 for ; Tue, 15 Nov 2016 04:02:34 -0800 (PST) Received: by mail-it0-x229.google.com with SMTP id q124so188764377itd.1 for ; Tue, 15 Nov 2016 04:02:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=a4uKPrKJUePY3mxA9CY+rj4cHj3+nr9pxW5B3QogWA4=; b=JtS7nX6gcJ67glbEGShuYf0IzGIrxxjIwTjk3QAvAtWAWCdi0n/PmSrA9PzQXQMRiB rqxWXwVh4GP2ira4dKAH2O6qoTF9FsXESkiT87K3vTwoaUzLbb5jIddzMeXaeix4zU4c G8nT+JRJiB7JeEO3HTlAp1E4TFJ8C5YeuVdBa6lC4SakzJT3ppz/AqsnKw9Z35Rc8fKE BN914t0CNxVr+ev4yJfdVOCeHyJ7lWRwR7tqk7iz061AiNaOrpVlIQdv3zqEug+VEIkq xrOAS/6n1dmCn8kLWN9U2LQvaj8XMKNt1pecLr5pYuNdoMtxJKWvjAXC1F6q8mv1hLb+ sRSg== 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=a4uKPrKJUePY3mxA9CY+rj4cHj3+nr9pxW5B3QogWA4=; b=APCkRu6FvcaQT4VcIU2uPSdB4C0gx+ZwkHM/lUhTU9151AMxJvsKbVW8d1Mm7SYZws 6OI7CBqYRatS+zSmueYaiFTxSSHaJM6U/FLRBSnl2KfV9xaHe634z5sdj9H+tXkTBw5I f9BeYvTbQi1lr17h24vqQvqVUoaIHKvkIcEIZpQ/ywxTPnftd52kpzDxJKe2O7YfHyBX 29WvAj6+68iehrDzrI045VQkUmax1aWjwaldKCjJ2bsr7JBvRV2oIFGnpRZlbRU7frP9 o+4FtLU+c1kmUheer0KZTQCclKXvcgAp3Pl3NGEXQxWW4fbVugdtReFZl1DslZdiuzdZ K8gA== X-Gm-Message-State: ABUngvcVgi5LGFFFQqF760ITVZ43gEMJ1+zkLDB3wfsP/+3d2GYscyGPPkFNNFTLge/IZjAnIhlJon4qm3WubQ== X-Received: by 10.36.225.75 with SMTP id n72mr2452797ith.13.1479211358058; Tue, 15 Nov 2016 04:02:38 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.25.140 with HTTP; Tue, 15 Nov 2016 04:02:37 -0800 (PST) In-Reply-To: References: <1478173239-22270-1-git-send-email-ard.biesheuvel@linaro.org> From: Marcin Wojtas Date: Tue, 15 Nov 2016 13:02:37 +0100 Message-ID: To: Ard Biesheuvel Cc: edk2-devel-01 , Leif Lindholm , "Kinney, Michael D" , "afish@apple.com" , "Tian, Feng" , "Zeng, Star" Subject: Re: [PATCH v2 0/5] MdeModulePkg: add support for non-discoverable devices 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: Tue, 15 Nov 2016 12:02:35 -0000 Content-Type: text/plain; charset=UTF-8 Hi Ard, 2016-11-15 12:23 GMT+01:00 Ard Biesheuvel : > On 14 November 2016 at 20:52, Marcin Wojtas wrote: >> Hi Ard, >> >> I tested your solution on Marvell Armada 70x0, whose upstream process >> to OpenPlatformPkg is being finalized now. So far I was able to check >> generic XHCI IP (MdeModulePkg/Bus/Pci/XhciDxe) and custom SD/MMC >> driver - they both work fine with following modification in your code: >> >> --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c >> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c >> @@ -942,7 +942,7 @@ InitializePciIoProtocol ( >> Dev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB; >> Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL; >> Dev->BarIndex = 0; >> - Dev->BarSize = SIZE_2KB; >> + Dev->BarSize = SIZE_16KB; >> break; >> >> case NonDiscoverableDeviceTypeAhci: >> @@ -958,7 +958,7 @@ InitializePciIoProtocol ( >> Dev->ConfigSpace.Hdr.ClassCode[1] = PCI_SUBCLASS_SD_HOST_CONTROLLER; >> Dev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SYSTEM_PERIPHERAL; >> Dev->BarIndex = 0; >> - Dev->BarSize = 0x100; >> + Dev->BarSize = 0x300; >> break; >> >> Any value lower than SIZE_16KB of BarSize resulted in XhcReadExtCapReg >> function failing due to reaching out of declared space. So happened >> with custom SD/MMC driver. Is it a problem to extend the space? >> > > No, not at all. It was just a ballpark figure derived from a quick > glance at the PCI drivers. Also Ahci should be extended to SIZE_8KB - according to the spec, single controller can comprise up to 32 ports, which gives possible 0x1200 size. > >> Did you consider extending RegisterNonDiscoverableDevice() function >> with possible custom BarSize value? I think enabling the users >> overriding the default value could be possible. What do you think? >> > > Well, it depends on whether the PCI drivers will actually access that > space. The idea is to make the non-discoverable device description as > simple as possible. > If extending the size is not a problem, I'm also for leaving it as-is. >> I also reviewed the code during debug and have no objections. >> > > Thanks! > > Note to reviewers: Marcin's patch series to add AHCI, XHCI and SDHCI > support went from > > 10 files changed, 1583 insertions(+), 3 deletions(-) > > to > > 7 files changed, 271 insertions(+) > > which sounds like an improvement to me. > Right, it's way more simple, without code duplication. Best regards, Marcin