From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 64A8B81F20 for ; Wed, 16 Nov 2016 18:54:03 -0800 (PST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP; 16 Nov 2016 18:53:59 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,651,1473145200"; d="scan'208";a="1060472715" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga001.jf.intel.com with ESMTP; 16 Nov 2016 18:53:58 -0800 Received: from fmsmsx118.amr.corp.intel.com (10.18.116.18) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 16 Nov 2016 18:53:58 -0800 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by fmsmsx118.amr.corp.intel.com (10.18.116.18) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 16 Nov 2016 18:53:58 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.142]) by shsmsx102.ccr.corp.intel.com ([169.254.2.239]) with mapi id 14.03.0248.002; Thu, 17 Nov 2016 10:53:54 +0800 From: "Ni, Ruiyu" To: Leif Lindholm , Ard Biesheuvel CC: "edk2-devel@lists.01.org" , "afish@apple.com" , "Gao, Liming" , "Kinney, Michael D" Thread-Topic: [edk2] [PATCH v3 1/5] MdeModulePkg: introduce non-discoverable device protocol Thread-Index: AQHSQCraEwMvOFFyHUuvZ27kFPDix6DbXOsAgAEaWfA= Date: Thu, 17 Nov 2016 02:53:54 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D58E7C6D4@SHSMSX104.ccr.corp.intel.com> References: <1479315571-14953-1-git-send-email-ard.biesheuvel@linaro.org> <1479315571-14953-2-git-send-email-ard.biesheuvel@linaro.org> <20161116174848.GC27644@bivouac.eciton.net> In-Reply-To: <20161116174848.GC27644@bivouac.eciton.net> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v3 1/5] MdeModulePkg: introduce non-discoverable device protocol 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: Thu, 17 Nov 2016 02:54:03 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Ard, I have two comments in below. Thanks/Ray > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Leif Lindholm > Sent: Thursday, November 17, 2016 1:49 AM > To: Ard Biesheuvel > Cc: Ni, Ruiyu ; edk2-devel@lists.01.org; > afish@apple.com; Gao, Liming ; Kinney, Michael D > > Subject: Re: [edk2] [PATCH v3 1/5] MdeModulePkg: introduce non- > discoverable device protocol >=20 > On Wed, Nov 16, 2016 at 04:59:27PM +0000, Ard Biesheuvel wrote: > > Introduce a protocol that can be exposed by a platform for devices > > that are not discoverable, usually because they are wired straight to > > the memory bus rather than to an enumerable bus like PCI or USB. > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Ard Biesheuvel > > --- > > MdeModulePkg/Include/Protocol/NonDiscoverableDevice.h | 90 > ++++++++++++++++++++ > > MdeModulePkg/MdeModulePkg.dec | 3 + > > 2 files changed, 93 insertions(+) > > > > diff --git a/MdeModulePkg/Include/Protocol/NonDiscoverableDevice.h > > b/MdeModulePkg/Include/Protocol/NonDiscoverableDevice.h > > new file mode 100644 > > index 000000000000..47ed841b407b > > --- /dev/null > > +++ b/MdeModulePkg/Include/Protocol/NonDiscoverableDevice.h > > @@ -0,0 +1,90 @@ > > +/** @file > > + Protocol to describe devices that are not on a discoverable bus > > + > > + Copyright (c) 2016, Linaro, Ltd. 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. > > + > > +**/ > > + > > +#ifndef __NON_DISCOVERABLE_DEVICE_H__ #define > > +__NON_DISCOVERABLE_DEVICE_H__ > > + > > +#include > > + > > +#define EDKII_NON_DISCOVERABLE_DEVICE_PROTOCOL_GUID \ > > + { 0x0d51905b, 0xb77e, 0x452a, {0xa2, 0xc0, 0xec, 0xa0, 0xcc, 0x8d, > > +0x51, 0x4a } } 1. Can you add "PCI" keyword into the protocol name? e.g.: EDKII_NON_DISCOVERABLE_PCI_DEVICE_PROTOCOL_GUID > > + > > +// > > +// Protocol interface structure > > +// > > +typedef struct _NON_DISCOVERABLE_DEVICE > NON_DISCOVERABLE_DEVICE; > > + > > +// > > +// Data Types > > +// > > +typedef enum { > > + NonDiscoverableDeviceTypeAmba, > > + NonDiscoverableDeviceTypeOhci, > > + NonDiscoverableDeviceTypeUhci, > > + NonDiscoverableDeviceTypeEhci, > > + NonDiscoverableDeviceTypeXhci, > > + NonDiscoverableDeviceTypeAhci, > > + NonDiscoverableDeviceTypeSdhci, > > + NonDiscoverableDeviceTypeUfs, > > + NonDiscoverableDeviceTypeNvme, >=20 > Just one OCD comment/question left: > Can we keep these sorted alphabetically? > (Also in switch statements in later patches?) >=20 > Other than that, I'm (very) happy with this series. >=20 > / > Leif >=20 > > + NonDiscoverableDeviceTypeMax, > > +} NON_DISCOVERABLE_DEVICE_TYPE; > > + > > +typedef enum { > > + NonDiscoverableDeviceDmaTypeCoherent, > > + NonDiscoverableDeviceDmaTypeNonCoherent, > > + NonDiscoverableDeviceDmaTypeMax, > > +} NON_DISCOVERABLE_DEVICE_DMA_TYPE; > > + > > +// > > +// Function Prototypes > > +// > > + > > +/** > > + Perform device specific initialization before the device is started > > + > > + @param This The non-discoverable device protocol pointer > > + > > + @retval EFI_SUCCESS Initialization successful, the device may be u= sed > > + @retval Other Initialization failed, device should not be st= arted > > +**/ > > +typedef > > +EFI_STATUS > > +(EFIAPI *NON_DISCOVERABLE_DEVICE_INIT) ( > > + IN NON_DISCOVERABLE_DEVICE *This > > + ); > > + > > +struct _NON_DISCOVERABLE_DEVICE { > > + // > > + // The type of device > > + // > > + NON_DISCOVERABLE_DEVICE_TYPE Type; 2. Can you use PCI class code to replace the enum type here? e.g.: UINT8 Class; UINT8 SubClass; UINT8 Programming Interface; The enum type can be defined in the helper library. In this way, we make the protocol definition stable enough. > > + // > > + // Whether this device is DMA coherent > > + // > > + NON_DISCOVERABLE_DEVICE_DMA_TYPE DmaType; > > + // > > + // Initialization function for the device > > + // > > + NON_DISCOVERABLE_DEVICE_INIT Initialize; > > + // > > + // The MMIO and I/O regions owned by the device > > + // > > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Resources; > > +}; > > + > > +extern EFI_GUID gEdkiiNonDiscoverableDeviceProtocolGuid; > > + > > +#endif > > diff --git a/MdeModulePkg/MdeModulePkg.dec > > b/MdeModulePkg/MdeModulePkg.dec index 74b870051c67..6b956fc80c93 > > 100644 > > --- a/MdeModulePkg/MdeModulePkg.dec > > +++ b/MdeModulePkg/MdeModulePkg.dec > > @@ -505,6 +505,9 @@ [Protocols] > > # Include/Protocol/Ps2Policy.h > > gEfiPs2PolicyProtocolGuid =3D { 0x4DF19259, 0xDC71, 0x4D46, { 0xBE, > > 0xF1, 0x35, 0x7B, 0xB5, 0x78, 0xC4, 0x18 } } > > > > + ## Include/Protocol/NonDiscoverableDevice.h > > + gEdkiiNonDiscoverableDeviceProtocolGuid =3D { 0x0d51905b, 0xb77e, > > + 0x452a, {0xa2, 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } } > > + > > # > > # [Error.gEfiMdeModulePkgTokenSpaceGuid] > > # 0x80000001 | Invalid value provided. > > -- > > 2.7.4 > > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel