public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [patch 1/2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Add comments for functions
@ 2016-12-15  7:11 Dandan Bi
  2016-12-15  7:11 ` [patch 2/2] MdeModulePkg/NonDiscoverablePciDevice: Make variable definition follow rule Dandan Bi
  2016-12-15  7:45 ` [patch 1/2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Add comments for functions Ard Biesheuvel
  0 siblings, 2 replies; 7+ messages in thread
From: Dandan Bi @ 2016-12-15  7:11 UTC (permalink / raw)
  To: edk2-devel; +Cc: Ard Biesheuvel, Ruiyu Ni

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 .../NonDiscoverablePciDeviceDxe/ComponentName.c    |  47 ++++
 .../NonDiscoverablePciDeviceDxe.c                  |  50 +++-
 .../NonDiscoverablePciDeviceDxe.inf                |  18 +-
 .../NonDiscoverablePciDeviceIo.c                   | 298 ++++++++++++++++++++-
 .../NonDiscoverablePciDeviceIo.h                   |   6 +
 5 files changed, 405 insertions(+), 14 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c
index 6e51d00..0e6ebec 100644
--- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c
+++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c
@@ -28,10 +28,31 @@ EFI_UNICODE_STRING_TABLE mDriverNameTable[] = {
   { NULL,     NULL                   }
 };
 
 EFI_COMPONENT_NAME_PROTOCOL gComponentName;
 
+/**
+  Retrieves a Unicode string that is the user readable name of the UEFI Driver.
+
+  @param This           A pointer to the EFI_COMPONENT_NAME_PROTOCOL instance.
+  @param Language       A pointer to a three character ISO 639-2 language identifier.
+                        This is the language of the driver name that that the caller
+                        is requesting, and it must match one of the languages specified
+                        in SupportedLanguages.  The number of languages supported by a
+                        driver is up to the driver writer.
+  @param DriverName     A pointer to the Unicode string to return.  This Unicode string
+                        is the name of the driver specified by This in the language
+                        specified by Language.
+
+  @retval EFI_SUCCESS           The Unicode string for the Driver specified by This
+                                and the language specified by Language was returned
+                                in DriverName.
+  @retval EFI_INVALID_PARAMETER Language is NULL.
+  @retval EFI_INVALID_PARAMETER DriverName is NULL.
+  @retval EFI_UNSUPPORTED       The driver specified by This does not support the
+                                language specified by Language.
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 NonDiscoverablePciGetDriverName (
   IN  EFI_COMPONENT_NAME_PROTOCOL *This,
@@ -46,10 +67,36 @@ NonDiscoverablePciGetDriverName (
            DriverName,
            (BOOLEAN)(This == &gComponentName) // Iso639Language
            );
 }
 
+/**
+  Retrieves a Unicode string that is the user readable name of the controller
+  that is being managed by an UEFI Driver.
+
+  @param This                   A pointer to the EFI_COMPONENT_NAME_PROTOCOL instance.
+  @param DeviceHandle           The handle of a controller that the driver specified by
+                                This is managing.  This handle specifies the controller
+                                whose name is to be returned.
+  @param ChildHandle            The handle of the child controller to retrieve the name
+                                of.  This is an optional parameter that may be NULL.  It
+                                will be NULL for device drivers.  It will also be NULL
+                                for a bus drivers that wish to retrieve the name of the
+                                bus controller.  It will not be NULL for a bus driver
+                                that wishes to retrieve the name of a child controller.
+  @param Language               A pointer to a three character ISO 639-2 language
+                                identifier.  This is the language of the controller name
+                                that that the caller is requesting, and it must match one
+                                of the languages specified in SupportedLanguages.  The
+                                number of languages supported by a driver is up to the
+                                driver writer.
+  @param ControllerName         A pointer to the Unicode string to return.  This Unicode
+                                string is the name of the controller specified by
+                                ControllerHandle and ChildHandle in the language
+                                specified by Language from the point of view of the
+                                driver specified by This.
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 NonDiscoverablePciGetDeviceName (
   IN  EFI_COMPONENT_NAME_PROTOCOL *This,
diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
index ee765d7..a868ea2 100644
--- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
+++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
@@ -45,10 +45,23 @@ SupportedNonDiscoverableDevices [] = {
 //   - 5.1.3.4 OpenProtocol() and CloseProtocol()
 // - UEFI Spec 2.3.1 + Errata C
 //   -  6.3 Protocol Handler Services
 //
 
+/**
+  Supported function of Driver Binding protocol for this driver.
+  Test to see if this driver supports ControllerHandle.
+
+  @param This                   Protocol instance pointer.
+  @param DeviceHandle           Handle of device to test.
+  @param RemainingDevicePath    A pointer to the device path.
+                                it should be ignored by device driver.
+
+  @retval EFI_SUCCESS           This driver supports this device.
+  @retval other                 This driver does not support this device.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 NonDiscoverablePciDeviceSupported (
   IN EFI_DRIVER_BINDING_PROTOCOL *This,
@@ -106,10 +119,23 @@ CloseProtocol:
          This->DriverBindingHandle, DeviceHandle);
 
   return Status;
 }
 
+/**
+  This routine is called right after the .Supported() called and
+  Start this driver on ControllerHandle.
+
+  @param This                   Protocol instance pointer.
+  @param DeviceHandle           Handle of device to bind driver to.
+  @param RemainingDevicePath    A pointer to the device path.
+                                it should be ignored by device driver.
+
+  @retval EFI_SUCCESS           This driver is added to this device.
+  @retval other                 Some error occurs when binding this driver to this device.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 NonDiscoverablePciDeviceStart (
   IN EFI_DRIVER_BINDING_PROTOCOL *This,
@@ -156,11 +182,22 @@ FreeDev:
   FreePool (Dev);
 
   return Status;
 }
 
+/**
+  Stop this driver on ControllerHandle.
+
+  @param This               Protocol instance pointer.
+  @param DeviceHandle       Handle of device to stop driver on.
+  @param NumberOfChildren   Not used.
+  @param ChildHandleBuffer  Not used.
 
+  @retval EFI_SUCCESS   This driver is removed from this device.
+  @retval other         Some error occurs when removing this driver from this device.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 NonDiscoverablePciDeviceStop (
   IN EFI_DRIVER_BINDING_PROTOCOL *This,
@@ -212,13 +249,20 @@ STATIC EFI_DRIVER_BINDING_PROTOCOL gDriverBinding = {
   0x10, // Version, must be in [0x10 .. 0xFFFFFFEF] for IHV-developed drivers
   NULL,
   NULL
 };
 
-//
-// Entry point of this driver.
-//
+/**
+  Entry point of this driver.
+
+  @param  ImageHandle     Image handle this driver.
+  @param  SystemTable     Pointer to the System Table.
+
+  @retval EFI_SUCCESS     The entry point is executed successfully.
+  @retval other           Some error occurred when executing this entry point.
+
+**/
 EFI_STATUS
 EFIAPI
 NonDiscoverablePciDeviceDxeEntryPoint (
   IN EFI_HANDLE       ImageHandle,
   IN EFI_SYSTEM_TABLE *SystemTable
diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
index 996fe31..6d3f721 100644
--- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
+++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
@@ -1,6 +1,8 @@
 ## @file
+# PCI I/O driver for non-discoverable devices.
+#
 # Copyright (C) 2016, Linaro Ltd.
 #
 # 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
@@ -40,13 +42,13 @@ [LibraryClasses]
 [Protocols]
   gEfiPciIoProtocolGuid                         ## BY_START
   gEdkiiNonDiscoverableDeviceProtocolGuid       ## TO_START
 
 [Guids]
-  gEdkiiNonDiscoverableAhciDeviceGuid
-  gEdkiiNonDiscoverableEhciDeviceGuid
-  gEdkiiNonDiscoverableNvmeDeviceGuid
-  gEdkiiNonDiscoverableOhciDeviceGuid
-  gEdkiiNonDiscoverableSdhciDeviceGuid
-  gEdkiiNonDiscoverableUfsDeviceGuid
-  gEdkiiNonDiscoverableUhciDeviceGuid
-  gEdkiiNonDiscoverableXhciDeviceGuid
+  gEdkiiNonDiscoverableAhciDeviceGuid       ## CONSUMES ## GUID
+  gEdkiiNonDiscoverableEhciDeviceGuid       ## CONSUMES ## GUID
+  gEdkiiNonDiscoverableNvmeDeviceGuid       ## CONSUMES ## GUID
+  gEdkiiNonDiscoverableOhciDeviceGuid       ## CONSUMES ## GUID
+  gEdkiiNonDiscoverableSdhciDeviceGuid      ## CONSUMES ## GUID
+  gEdkiiNonDiscoverableUfsDeviceGuid        ## CONSUMES ## GUID
+  gEdkiiNonDiscoverableUhciDeviceGuid       ## CONSUMES ## GUID
+  gEdkiiNonDiscoverableXhciDeviceGuid       ## CONSUMES ## GUID
diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
index 82ee9d1..59b6076 100644
--- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
+++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
@@ -24,13 +24,18 @@ typedef struct {
   VOID                            *HostAddress;
   EFI_PCI_IO_PROTOCOL_OPERATION   Operation;
   UINTN                           NumberOfBytes;
 } NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO;
 
-//
-// Get the resource associated with BAR number 'BarIndex'.
-//
+/**
+  Get the resource associated with BAR number 'BarIndex'.
+
+  @param  Dev           Point to the NON_DISCOVERABLE_PCI_DEVICE instance.
+  @param  BarIndex      The BAR index of the standard PCI Configuration header to use as the
+                        base address for the memory operation to perform.
+  @param  Descriptor    Points to the address space descriptor
+**/
 STATIC
 EFI_STATUS
 GetBarResource (
   IN  NON_DISCOVERABLE_PCI_DEVICE         *Dev,
   IN  UINT8                               BarIndex,
@@ -57,10 +62,25 @@ GetBarResource (
     BarIndex -= 1;
   }
   return EFI_NOT_FOUND;
 }
 
+/**
+  Reads from the memory space of a PCI controller. Returns either when the polling exit criteria is
+  satisfied or after a defined duration.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Width                 Signifies the width of the memory or I/O operations.
+  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
+                                base address for the memory operation to perform.
+  @param  Offset                The offset within the selected BAR to start the memory operation.
+  @param  Mask                  Mask used for the polling criteria.
+  @param  Value                 The comparison value used for the polling exit criteria.
+  @param  Delay                 The number of 100 ns units to poll.
+  @param  Result                Pointer to the last value read from the memory location.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoPollMem (
   IN  EFI_PCI_IO_PROTOCOL         *This,
@@ -75,10 +95,25 @@ PciIoPollMem (
 {
   ASSERT (FALSE);
   return EFI_UNSUPPORTED;
 }
 
+/**
+  Reads from the memory space of a PCI controller. Returns either when the polling exit criteria is
+  satisfied or after a defined duration.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Width                 Signifies the width of the memory or I/O operations.
+  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
+                                base address for the memory operation to perform.
+  @param  Offset                The offset within the selected BAR to start the memory operation.
+  @param  Mask                  Mask used for the polling criteria.
+  @param  Value                 The comparison value used for the polling exit criteria.
+  @param  Delay                 The number of 100 ns units to poll.
+  @param  Result                Pointer to the last value read from the memory location.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoPollIo (
   IN  EFI_PCI_IO_PROTOCOL         *This,
@@ -93,10 +128,23 @@ PciIoPollIo (
 {
   ASSERT (FALSE);
   return EFI_UNSUPPORTED;
 }
 
+/**
+  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
+
+  @param  Width         Signifies the width of the memory or I/O operations.
+  @param  Count         The number of memory or I/O operations to perform.
+  @param  DstStride     The stride of the destination buffer.
+  @param  Dst           For read operations, the destination buffer to store the results. For write
+                        operations, the destination buffer to write data to.
+  @param  SrcStride     The stride of the source buffer.
+  @param  Src           For read operations, the source buffer to read data from. For write
+                        operations, the source buffer to write data from.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoMemRW (
   IN  EFI_PCI_IO_PROTOCOL_WIDTH   Width,
@@ -144,10 +192,30 @@ PciIoMemRW (
   }
 
   return EFI_SUCCESS;
 }
 
+/**
+  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Width                 Signifies the width of the memory or I/O operations.
+  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
+                                base address for the memory or I/O operation to perform.
+  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
+  @param  Count                 The number of memory or I/O operations to perform.
+  @param  Buffer                For read operations, the destination buffer to store the results. For write
+                                operations, the source buffer to write data from.
+
+  @retval EFI_SUCCESS           The data was read from or written to the PCI controller.
+  @retval EFI_UNSUPPORTED       BarIndex not valid for this PCI controller.
+  @retval EFI_UNSUPPORTED       The address range specified by Offset, Width, and Count is not
+                                valid for the PCI BAR specified by BarIndex.
+  @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a lack of resources.
+  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoMemRead (
   IN     EFI_PCI_IO_PROTOCOL          *This,
@@ -211,10 +279,30 @@ PciIoMemRead (
     break;
   }
   return EFI_INVALID_PARAMETER;
 }
 
+/**
+  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Width                 Signifies the width of the memory or I/O operations.
+  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
+                                base address for the memory or I/O operation to perform.
+  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
+  @param  Count                 The number of memory or I/O operations to perform.
+  @param  Buffer                For read operations, the destination buffer to store the results. For write
+                                operations, the source buffer to write data from.
+
+  @retval EFI_SUCCESS           The data was read from or written to the PCI controller.
+  @retval EFI_UNSUPPORTED       BarIndex not valid for this PCI controller.
+  @retval EFI_UNSUPPORTED       The address range specified by Offset, Width, and Count is not
+                                valid for the PCI BAR specified by BarIndex.
+  @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a lack of resources.
+  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoMemWrite (
   IN     EFI_PCI_IO_PROTOCOL          *This,
@@ -278,10 +366,23 @@ PciIoMemWrite (
     break;
   }
   return EFI_INVALID_PARAMETER;
 }
 
+/**
+  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Width                 Signifies the width of the memory or I/O operations.
+  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
+                                base address for the memory or I/O operation to perform.
+  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
+  @param  Count                 The number of memory or I/O operations to perform.
+  @param  Buffer                For read operations, the destination buffer to store the results. For write
+                                operations, the source buffer to write data from.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoIoRead (
   IN EFI_PCI_IO_PROTOCOL              *This,
@@ -294,10 +395,23 @@ PciIoIoRead (
 {
   ASSERT (FALSE);
   return EFI_UNSUPPORTED;
 }
 
+/**
+  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Width                 Signifies the width of the memory or I/O operations.
+  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
+                                base address for the memory or I/O operation to perform.
+  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
+  @param  Count                 The number of memory or I/O operations to perform.
+  @param  Buffer                For read operations, the destination buffer to store the results. For write
+                                operations, the source buffer to write data from.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoIoWrite (
   IN     EFI_PCI_IO_PROTOCOL          *This,
@@ -310,10 +424,21 @@ PciIoIoWrite (
 {
   ASSERT (FALSE);
   return EFI_UNSUPPORTED;
 }
 
+/**
+  Enable a PCI driver to access PCI config space.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Width                 Signifies the width of the memory or I/O operations.
+  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
+  @param  Count                 The number of memory or I/O operations to perform.
+  @param  Buffer                For read operations, the destination buffer to store the results. For write
+                                operations, the source buffer to write data from.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoPciRead (
   IN     EFI_PCI_IO_PROTOCOL        *This,
@@ -346,10 +471,21 @@ PciIoPciRead (
     Count -= Length >> ((UINTN)Width & 0x3);
   }
   return PciIoMemRW (Width, Count, 1, Buffer, 1, Address);
 }
 
+/**
+  Enable a PCI driver to access PCI config space.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Width                 Signifies the width of the memory or I/O operations.
+  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
+  @param  Count                 The number of memory or I/O operations to perform.
+  @param  Buffer                For read operations, the destination buffer to store the results. For write
+                                operations, the source buffer to write data from.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoPciWrite (
   IN EFI_PCI_IO_PROTOCOL              *This,
@@ -374,10 +510,28 @@ PciIoPciWrite (
   }
 
   return PciIoMemRW (Width, Count, 1, Address, 1, Buffer);
 }
 
+/**
+  Enables a PCI driver to copy one region of PCI memory space to another region of PCI
+  memory space.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Width                 Signifies the width of the memory operations.
+  @param  DestBarIndex          The BAR index in the standard PCI Configuration header to use as the
+                                base address for the memory operation to perform.
+  @param  DestOffset            The destination offset within the BAR specified by DestBarIndex to
+                                start the memory writes for the copy operation.
+  @param  SrcBarIndex           The BAR index in the standard PCI Configuration header to use as the
+                                base address for the memory operation to perform.
+  @param  SrcOffset             The source offset within the BAR specified by SrcBarIndex to start
+                                the memory reads for the copy operation.
+  @param  Count                 The number of memory operations to perform. Bytes moved is Width
+                                size * Count, starting at DestOffset and SrcOffset.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoCopyMem (
   IN EFI_PCI_IO_PROTOCOL              *This,
@@ -391,10 +545,29 @@ PciIoCopyMem (
 {
   ASSERT (FALSE);
   return EFI_UNSUPPORTED;
 }
 
+/**
+  Provides the PCI controller-specific addresses needed to access system memory.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Operation             Indicates if the bus master is going to read or write to system memory.
+  @param  HostAddress           The system memory address to map to the PCI controller.
+  @param  NumberOfBytes         On input the number of bytes to map. On output the number of bytes
+                                that were mapped.
+  @param  DeviceAddress         The resulting map address for the bus master PCI controller to use to
+                                access the hosts HostAddress.
+  @param  Mapping               A resulting value to pass to Unmap().
+
+  @retval EFI_SUCCESS           The range was mapped for the returned NumberOfBytes.
+  @retval EFI_UNSUPPORTED       The HostAddress cannot be mapped as a common buffer.
+  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+  @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a lack of resources.
+  @retval EFI_DEVICE_ERROR      The system hardware could not map the requested address.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 CoherentPciIoMap (
   IN     EFI_PCI_IO_PROTOCOL            *This,
@@ -457,10 +630,19 @@ CoherentPciIoMap (
     *Mapping = NULL;
   }
   return EFI_SUCCESS;
 }
 
+/**
+  Completes the Map() operation and releases any corresponding resources.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Mapping               The mapping value returned from Map().
+
+  @retval EFI_SUCCESS           The range was unmapped.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 CoherentPciIoUnmap (
   IN  EFI_PCI_IO_PROTOCOL          *This,
@@ -480,10 +662,29 @@ CoherentPciIoUnmap (
     FreePool (MapInfo);
   }
   return EFI_SUCCESS;
 }
 
+/**
+  Allocates pages.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Type                  This parameter is not used and must be ignored.
+  @param  MemoryType            The type of memory to allocate, EfiBootServicesData or
+                                EfiRuntimeServicesData.
+  @param  Pages                 The number of pages to allocate.
+  @param  HostAddress           A pointer to store the base system memory address of the
+                                allocated range.
+  @param  Attributes            The requested bit mask of attributes for the allocated range.
+
+  @retval EFI_SUCCESS           The requested memory pages were allocated.
+  @retval EFI_UNSUPPORTED       Attributes is unsupported. The only legal attribute bits are
+                                MEMORY_WRITE_COMBINE and MEMORY_CACHED.
+  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+  @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 CoherentPciIoAllocateBuffer (
   IN  EFI_PCI_IO_PROTOCOL         *This,
@@ -522,10 +723,20 @@ CoherentPciIoAllocateBuffer (
     *HostAddress = (VOID *)(UINTN)AllocAddress;
   }
   return Status;
 }
 
+/**
+  Frees memory that was allocated with AllocatePages().
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Pages                 The number of pages to free.
+  @param  HostAddress           The base system memory address of the allocated range.
+
+  @retval EFI_SUCCESS           The requested memory pages were freed.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 CoherentPciIoFreeBuffer (
   IN  EFI_PCI_IO_PROTOCOL         *This,
@@ -535,21 +746,42 @@ CoherentPciIoFreeBuffer (
 {
   FreePages (HostAddress, Pages);
   return EFI_SUCCESS;
 }
 
+/**
+  Flushes all PCI posted write transactions from a PCI host bridge to system memory.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+
+  @retval EFI_SUCCESS           The PCI posted write transactions were flushed from the PCI host
+                                bridge to system memory.
 
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoFlush (
   IN EFI_PCI_IO_PROTOCOL          *This
   )
 {
   return EFI_SUCCESS;
 }
 
+/**
+  Retrieves this PCI controller's current PCI bus number, device number, and function number.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  SegmentNumber         The PCI controller's current PCI segment number.
+  @param  BusNumber             The PCI controller's current PCI bus number.
+  @param  DeviceNumber          The PCI controller's current PCI device number.
+  @param  FunctionNumber        The PCI controller's current PCI function number.
+
+  @retval EFI_SUCCESS           The PCI controller location was returned.
+  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoGetLocation (
   IN   EFI_PCI_IO_PROTOCOL  *This,
@@ -572,10 +804,29 @@ PciIoGetLocation (
   *FunctionNumber = 0;
 
   return EFI_SUCCESS;
 }
 
+/**
+  Performs an operation on the attributes that this PCI controller supports. The operations include
+  getting the set of supported attributes, retrieving the current attributes, setting the current
+  attributes, enabling attributes, and disabling attributes.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Operation             The operation to perform on the attributes for this PCI controller.
+  @param  Attributes            The mask of attributes that are used for Set, Enable, and Disable
+                                operations.
+  @param  Result                A pointer to the result mask of attributes that are returned for the Get
+                                and Supported operations.
+
+  @retval EFI_SUCCESS           The operation on the PCI controller's attributes was completed.
+  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+  @retval EFI_UNSUPPORTED       one or more of the bits set in
+                                Attributes are not supported by this PCI controller or one of
+                                its parent bridges when Operation is Set, Enable or Disable.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoAttributes (
   IN  EFI_PCI_IO_PROTOCOL                      *This,
@@ -629,10 +880,32 @@ PciIoAttributes (
     Dev->Enabled = TRUE;
   }
   return EFI_SUCCESS;
 }
 
+/**
+  Gets the attributes that this PCI controller supports setting on a BAR using
+  SetBarAttributes(), and retrieves the list of resource descriptors for a BAR.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
+                                base address for resource range. The legal range for this field is 0..5.
+  @param  Supports              A pointer to the mask of attributes that this PCI controller supports
+                                setting for this BAR with SetBarAttributes().
+  @param  Resources             A pointer to the ACPI 2.0 resource descriptors that describe the current
+                                configuration of this BAR of the PCI controller.
+
+  @retval EFI_SUCCESS           If Supports is not NULL, then the attributes that the PCI
+                                controller supports are returned in Supports. If Resources
+                                is not NULL, then the ACPI 2.0 resource descriptors that the PCI
+                                controller is currently using are returned in Resources.
+  @retval EFI_INVALID_PARAMETER Both Supports and Attributes are NULL.
+  @retval EFI_UNSUPPORTED       BarIndex not valid for this PCI controller.
+  @retval EFI_OUT_OF_RESOURCES  There are not enough resources available to allocate
+                                Resources.
+
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoGetBarAttributes (
   IN EFI_PCI_IO_PROTOCOL             *This,
@@ -680,10 +953,23 @@ PciIoGetBarAttributes (
     *Resources = Descriptor;
   }
   return EFI_SUCCESS;
 }
 
+/**
+  Sets the attributes for a range of a BAR on a PCI controller.
+
+  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
+  @param  Attributes            The mask of attributes to set for the resource range specified by
+                                BarIndex, Offset, and Length.
+  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
+                                base address for resource range. The legal range for this field is 0..5.
+  @param  Offset                A pointer to the BAR relative base address of the resource range to be
+                                modified by the attributes specified by Attributes.
+  @param  Length                A pointer to the length of the resource range to be modified by the
+                                attributes specified by Attributes.
+**/
 STATIC
 EFI_STATUS
 EFIAPI
 PciIoSetBarAttributes (
   IN     EFI_PCI_IO_PROTOCOL          *This,
@@ -716,10 +1002,16 @@ STATIC CONST EFI_PCI_IO_PROTOCOL PciIoTemplate =
   PciIoSetBarAttributes,
   0,
   0
 };
 
+/**
+  Initialize PciIo Protocol.
+
+  @param  Dev      Point to NON_DISCOVERABLE_PCI_DEVICE instance.
+
+**/
 VOID
 InitializePciIoProtocol (
   NON_DISCOVERABLE_PCI_DEVICE     *Dev
   )
 {
diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h
index bc0a3d3..0b83aff 100644
--- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h
+++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h
@@ -71,10 +71,16 @@ typedef struct {
   // Whether this device has been enabled
   //
   BOOLEAN                   Enabled;
 } NON_DISCOVERABLE_PCI_DEVICE;
 
+/**
+  Initialize PciIo Protocol.
+
+  @param  Device      Point to NON_DISCOVERABLE_PCI_DEVICE instance.
+
+**/
 VOID
 InitializePciIoProtocol (
   NON_DISCOVERABLE_PCI_DEVICE     *Device
   );
 
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [patch 2/2] MdeModulePkg/NonDiscoverablePciDevice: Make variable definition follow rule
  2016-12-15  7:11 [patch 1/2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Add comments for functions Dandan Bi
@ 2016-12-15  7:11 ` Dandan Bi
  2016-12-15 11:41   ` Ard Biesheuvel
  2016-12-15  7:45 ` [patch 1/2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Add comments for functions Ard Biesheuvel
  1 sibling, 1 reply; 7+ messages in thread
From: Dandan Bi @ 2016-12-15  7:11 UTC (permalink / raw)
  To: edk2-devel; +Cc: Ard Biesheuvel, Ruiyu Ni

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 .../Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c  | 2 +-
 .../Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c   | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
index a868ea2..921225b 100644
--- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
+++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
@@ -19,11 +19,11 @@
 //
 // We only support the following device types
 //
 STATIC
 CONST EFI_GUID * CONST
-SupportedNonDiscoverableDevices [] = {
+SupportedNonDiscoverableDevices[] = {
   &gEdkiiNonDiscoverableAhciDeviceGuid,
   &gEdkiiNonDiscoverableEhciDeviceGuid,
   &gEdkiiNonDiscoverableNvmeDeviceGuid,
   &gEdkiiNonDiscoverableOhciDeviceGuid,
   &gEdkiiNonDiscoverableSdhciDeviceGuid,
diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
index 59b6076..a54313f 100644
--- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
+++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
@@ -913,11 +913,12 @@ PciIoGetBarAttributes (
   OUT UINT64                         *Supports OPTIONAL,
   OUT VOID                           **Resources OPTIONAL
   )
 {
   NON_DISCOVERABLE_PCI_DEVICE       *Dev;
-  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor, *BarDesc;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *BarDesc;
   EFI_ACPI_END_TAG_DESCRIPTOR       *End;
   EFI_STATUS                        Status;
 
   if (Supports == NULL && Resources == NULL) {
     return EFI_INVALID_PARAMETER;
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [patch 1/2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Add comments for functions
  2016-12-15  7:11 [patch 1/2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Add comments for functions Dandan Bi
  2016-12-15  7:11 ` [patch 2/2] MdeModulePkg/NonDiscoverablePciDevice: Make variable definition follow rule Dandan Bi
@ 2016-12-15  7:45 ` Ard Biesheuvel
  2016-12-15  8:31   ` Bi, Dandan
  1 sibling, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2016-12-15  7:45 UTC (permalink / raw)
  To: Dandan Bi, Ruiyu Ni; +Cc: edk2-devel@lists.01.org

Hello Dandan,

Thank you for doing this work. I am happy for these changes to go in.

However, I have a patch pending to implement non-coherent DMA support,
and Ray has not responded to that yet. These changes will likely
conflict with that patch, so could we please get the functionality
completed first before working on enhancements like this?

Ray?

Thanks,
Ard.


On 15 December 2016 at 07:11, Dandan Bi <dandan.bi@intel.com> wrote:
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  .../NonDiscoverablePciDeviceDxe/ComponentName.c    |  47 ++++
>  .../NonDiscoverablePciDeviceDxe.c                  |  50 +++-
>  .../NonDiscoverablePciDeviceDxe.inf                |  18 +-
>  .../NonDiscoverablePciDeviceIo.c                   | 298 ++++++++++++++++++++-
>  .../NonDiscoverablePciDeviceIo.h                   |   6 +
>  5 files changed, 405 insertions(+), 14 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c
> index 6e51d00..0e6ebec 100644
> --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c
> @@ -28,10 +28,31 @@ EFI_UNICODE_STRING_TABLE mDriverNameTable[] = {
>    { NULL,     NULL                   }
>  };
>
>  EFI_COMPONENT_NAME_PROTOCOL gComponentName;
>
> +/**
> +  Retrieves a Unicode string that is the user readable name of the UEFI Driver.
> +
> +  @param This           A pointer to the EFI_COMPONENT_NAME_PROTOCOL instance.
> +  @param Language       A pointer to a three character ISO 639-2 language identifier.
> +                        This is the language of the driver name that that the caller
> +                        is requesting, and it must match one of the languages specified
> +                        in SupportedLanguages.  The number of languages supported by a
> +                        driver is up to the driver writer.
> +  @param DriverName     A pointer to the Unicode string to return.  This Unicode string
> +                        is the name of the driver specified by This in the language
> +                        specified by Language.
> +
> +  @retval EFI_SUCCESS           The Unicode string for the Driver specified by This
> +                                and the language specified by Language was returned
> +                                in DriverName.
> +  @retval EFI_INVALID_PARAMETER Language is NULL.
> +  @retval EFI_INVALID_PARAMETER DriverName is NULL.
> +  @retval EFI_UNSUPPORTED       The driver specified by This does not support the
> +                                language specified by Language.
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  NonDiscoverablePciGetDriverName (
>    IN  EFI_COMPONENT_NAME_PROTOCOL *This,
> @@ -46,10 +67,36 @@ NonDiscoverablePciGetDriverName (
>             DriverName,
>             (BOOLEAN)(This == &gComponentName) // Iso639Language
>             );
>  }
>
> +/**
> +  Retrieves a Unicode string that is the user readable name of the controller
> +  that is being managed by an UEFI Driver.
> +
> +  @param This                   A pointer to the EFI_COMPONENT_NAME_PROTOCOL instance.
> +  @param DeviceHandle           The handle of a controller that the driver specified by
> +                                This is managing.  This handle specifies the controller
> +                                whose name is to be returned.
> +  @param ChildHandle            The handle of the child controller to retrieve the name
> +                                of.  This is an optional parameter that may be NULL.  It
> +                                will be NULL for device drivers.  It will also be NULL
> +                                for a bus drivers that wish to retrieve the name of the
> +                                bus controller.  It will not be NULL for a bus driver
> +                                that wishes to retrieve the name of a child controller.
> +  @param Language               A pointer to a three character ISO 639-2 language
> +                                identifier.  This is the language of the controller name
> +                                that that the caller is requesting, and it must match one
> +                                of the languages specified in SupportedLanguages.  The
> +                                number of languages supported by a driver is up to the
> +                                driver writer.
> +  @param ControllerName         A pointer to the Unicode string to return.  This Unicode
> +                                string is the name of the controller specified by
> +                                ControllerHandle and ChildHandle in the language
> +                                specified by Language from the point of view of the
> +                                driver specified by This.
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  NonDiscoverablePciGetDeviceName (
>    IN  EFI_COMPONENT_NAME_PROTOCOL *This,
> diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
> index ee765d7..a868ea2 100644
> --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
> @@ -45,10 +45,23 @@ SupportedNonDiscoverableDevices [] = {
>  //   - 5.1.3.4 OpenProtocol() and CloseProtocol()
>  // - UEFI Spec 2.3.1 + Errata C
>  //   -  6.3 Protocol Handler Services
>  //
>
> +/**
> +  Supported function of Driver Binding protocol for this driver.
> +  Test to see if this driver supports ControllerHandle.
> +
> +  @param This                   Protocol instance pointer.
> +  @param DeviceHandle           Handle of device to test.
> +  @param RemainingDevicePath    A pointer to the device path.
> +                                it should be ignored by device driver.
> +
> +  @retval EFI_SUCCESS           This driver supports this device.
> +  @retval other                 This driver does not support this device.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  NonDiscoverablePciDeviceSupported (
>    IN EFI_DRIVER_BINDING_PROTOCOL *This,
> @@ -106,10 +119,23 @@ CloseProtocol:
>           This->DriverBindingHandle, DeviceHandle);
>
>    return Status;
>  }
>
> +/**
> +  This routine is called right after the .Supported() called and
> +  Start this driver on ControllerHandle.
> +
> +  @param This                   Protocol instance pointer.
> +  @param DeviceHandle           Handle of device to bind driver to.
> +  @param RemainingDevicePath    A pointer to the device path.
> +                                it should be ignored by device driver.
> +
> +  @retval EFI_SUCCESS           This driver is added to this device.
> +  @retval other                 Some error occurs when binding this driver to this device.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  NonDiscoverablePciDeviceStart (
>    IN EFI_DRIVER_BINDING_PROTOCOL *This,
> @@ -156,11 +182,22 @@ FreeDev:
>    FreePool (Dev);
>
>    return Status;
>  }
>
> +/**
> +  Stop this driver on ControllerHandle.
> +
> +  @param This               Protocol instance pointer.
> +  @param DeviceHandle       Handle of device to stop driver on.
> +  @param NumberOfChildren   Not used.
> +  @param ChildHandleBuffer  Not used.
>
> +  @retval EFI_SUCCESS   This driver is removed from this device.
> +  @retval other         Some error occurs when removing this driver from this device.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  NonDiscoverablePciDeviceStop (
>    IN EFI_DRIVER_BINDING_PROTOCOL *This,
> @@ -212,13 +249,20 @@ STATIC EFI_DRIVER_BINDING_PROTOCOL gDriverBinding = {
>    0x10, // Version, must be in [0x10 .. 0xFFFFFFEF] for IHV-developed drivers
>    NULL,
>    NULL
>  };
>
> -//
> -// Entry point of this driver.
> -//
> +/**
> +  Entry point of this driver.
> +
> +  @param  ImageHandle     Image handle this driver.
> +  @param  SystemTable     Pointer to the System Table.
> +
> +  @retval EFI_SUCCESS     The entry point is executed successfully.
> +  @retval other           Some error occurred when executing this entry point.
> +
> +**/
>  EFI_STATUS
>  EFIAPI
>  NonDiscoverablePciDeviceDxeEntryPoint (
>    IN EFI_HANDLE       ImageHandle,
>    IN EFI_SYSTEM_TABLE *SystemTable
> diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
> index 996fe31..6d3f721 100644
> --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
> @@ -1,6 +1,8 @@
>  ## @file
> +# PCI I/O driver for non-discoverable devices.
> +#
>  # Copyright (C) 2016, Linaro Ltd.
>  #
>  # 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
> @@ -40,13 +42,13 @@ [LibraryClasses]
>  [Protocols]
>    gEfiPciIoProtocolGuid                         ## BY_START
>    gEdkiiNonDiscoverableDeviceProtocolGuid       ## TO_START
>
>  [Guids]
> -  gEdkiiNonDiscoverableAhciDeviceGuid
> -  gEdkiiNonDiscoverableEhciDeviceGuid
> -  gEdkiiNonDiscoverableNvmeDeviceGuid
> -  gEdkiiNonDiscoverableOhciDeviceGuid
> -  gEdkiiNonDiscoverableSdhciDeviceGuid
> -  gEdkiiNonDiscoverableUfsDeviceGuid
> -  gEdkiiNonDiscoverableUhciDeviceGuid
> -  gEdkiiNonDiscoverableXhciDeviceGuid
> +  gEdkiiNonDiscoverableAhciDeviceGuid       ## CONSUMES ## GUID
> +  gEdkiiNonDiscoverableEhciDeviceGuid       ## CONSUMES ## GUID
> +  gEdkiiNonDiscoverableNvmeDeviceGuid       ## CONSUMES ## GUID
> +  gEdkiiNonDiscoverableOhciDeviceGuid       ## CONSUMES ## GUID
> +  gEdkiiNonDiscoverableSdhciDeviceGuid      ## CONSUMES ## GUID
> +  gEdkiiNonDiscoverableUfsDeviceGuid        ## CONSUMES ## GUID
> +  gEdkiiNonDiscoverableUhciDeviceGuid       ## CONSUMES ## GUID
> +  gEdkiiNonDiscoverableXhciDeviceGuid       ## CONSUMES ## GUID
> diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
> index 82ee9d1..59b6076 100644
> --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
> @@ -24,13 +24,18 @@ typedef struct {
>    VOID                            *HostAddress;
>    EFI_PCI_IO_PROTOCOL_OPERATION   Operation;
>    UINTN                           NumberOfBytes;
>  } NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO;
>
> -//
> -// Get the resource associated with BAR number 'BarIndex'.
> -//
> +/**
> +  Get the resource associated with BAR number 'BarIndex'.
> +
> +  @param  Dev           Point to the NON_DISCOVERABLE_PCI_DEVICE instance.
> +  @param  BarIndex      The BAR index of the standard PCI Configuration header to use as the
> +                        base address for the memory operation to perform.
> +  @param  Descriptor    Points to the address space descriptor
> +**/
>  STATIC
>  EFI_STATUS
>  GetBarResource (
>    IN  NON_DISCOVERABLE_PCI_DEVICE         *Dev,
>    IN  UINT8                               BarIndex,
> @@ -57,10 +62,25 @@ GetBarResource (
>      BarIndex -= 1;
>    }
>    return EFI_NOT_FOUND;
>  }
>
> +/**
> +  Reads from the memory space of a PCI controller. Returns either when the polling exit criteria is
> +  satisfied or after a defined duration.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for the memory operation to perform.
> +  @param  Offset                The offset within the selected BAR to start the memory operation.
> +  @param  Mask                  Mask used for the polling criteria.
> +  @param  Value                 The comparison value used for the polling exit criteria.
> +  @param  Delay                 The number of 100 ns units to poll.
> +  @param  Result                Pointer to the last value read from the memory location.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoPollMem (
>    IN  EFI_PCI_IO_PROTOCOL         *This,
> @@ -75,10 +95,25 @@ PciIoPollMem (
>  {
>    ASSERT (FALSE);
>    return EFI_UNSUPPORTED;
>  }
>
> +/**
> +  Reads from the memory space of a PCI controller. Returns either when the polling exit criteria is
> +  satisfied or after a defined duration.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for the memory operation to perform.
> +  @param  Offset                The offset within the selected BAR to start the memory operation.
> +  @param  Mask                  Mask used for the polling criteria.
> +  @param  Value                 The comparison value used for the polling exit criteria.
> +  @param  Delay                 The number of 100 ns units to poll.
> +  @param  Result                Pointer to the last value read from the memory location.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoPollIo (
>    IN  EFI_PCI_IO_PROTOCOL         *This,
> @@ -93,10 +128,23 @@ PciIoPollIo (
>  {
>    ASSERT (FALSE);
>    return EFI_UNSUPPORTED;
>  }
>
> +/**
> +  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
> +
> +  @param  Width         Signifies the width of the memory or I/O operations.
> +  @param  Count         The number of memory or I/O operations to perform.
> +  @param  DstStride     The stride of the destination buffer.
> +  @param  Dst           For read operations, the destination buffer to store the results. For write
> +                        operations, the destination buffer to write data to.
> +  @param  SrcStride     The stride of the source buffer.
> +  @param  Src           For read operations, the source buffer to read data from. For write
> +                        operations, the source buffer to write data from.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoMemRW (
>    IN  EFI_PCI_IO_PROTOCOL_WIDTH   Width,
> @@ -144,10 +192,30 @@ PciIoMemRW (
>    }
>
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for the memory or I/O operation to perform.
> +  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
> +  @param  Count                 The number of memory or I/O operations to perform.
> +  @param  Buffer                For read operations, the destination buffer to store the results. For write
> +                                operations, the source buffer to write data from.
> +
> +  @retval EFI_SUCCESS           The data was read from or written to the PCI controller.
> +  @retval EFI_UNSUPPORTED       BarIndex not valid for this PCI controller.
> +  @retval EFI_UNSUPPORTED       The address range specified by Offset, Width, and Count is not
> +                                valid for the PCI BAR specified by BarIndex.
> +  @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a lack of resources.
> +  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoMemRead (
>    IN     EFI_PCI_IO_PROTOCOL          *This,
> @@ -211,10 +279,30 @@ PciIoMemRead (
>      break;
>    }
>    return EFI_INVALID_PARAMETER;
>  }
>
> +/**
> +  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for the memory or I/O operation to perform.
> +  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
> +  @param  Count                 The number of memory or I/O operations to perform.
> +  @param  Buffer                For read operations, the destination buffer to store the results. For write
> +                                operations, the source buffer to write data from.
> +
> +  @retval EFI_SUCCESS           The data was read from or written to the PCI controller.
> +  @retval EFI_UNSUPPORTED       BarIndex not valid for this PCI controller.
> +  @retval EFI_UNSUPPORTED       The address range specified by Offset, Width, and Count is not
> +                                valid for the PCI BAR specified by BarIndex.
> +  @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a lack of resources.
> +  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoMemWrite (
>    IN     EFI_PCI_IO_PROTOCOL          *This,
> @@ -278,10 +366,23 @@ PciIoMemWrite (
>      break;
>    }
>    return EFI_INVALID_PARAMETER;
>  }
>
> +/**
> +  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for the memory or I/O operation to perform.
> +  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
> +  @param  Count                 The number of memory or I/O operations to perform.
> +  @param  Buffer                For read operations, the destination buffer to store the results. For write
> +                                operations, the source buffer to write data from.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoIoRead (
>    IN EFI_PCI_IO_PROTOCOL              *This,
> @@ -294,10 +395,23 @@ PciIoIoRead (
>  {
>    ASSERT (FALSE);
>    return EFI_UNSUPPORTED;
>  }
>
> +/**
> +  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for the memory or I/O operation to perform.
> +  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
> +  @param  Count                 The number of memory or I/O operations to perform.
> +  @param  Buffer                For read operations, the destination buffer to store the results. For write
> +                                operations, the source buffer to write data from.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoIoWrite (
>    IN     EFI_PCI_IO_PROTOCOL          *This,
> @@ -310,10 +424,21 @@ PciIoIoWrite (
>  {
>    ASSERT (FALSE);
>    return EFI_UNSUPPORTED;
>  }
>
> +/**
> +  Enable a PCI driver to access PCI config space.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
> +  @param  Count                 The number of memory or I/O operations to perform.
> +  @param  Buffer                For read operations, the destination buffer to store the results. For write
> +                                operations, the source buffer to write data from.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoPciRead (
>    IN     EFI_PCI_IO_PROTOCOL        *This,
> @@ -346,10 +471,21 @@ PciIoPciRead (
>      Count -= Length >> ((UINTN)Width & 0x3);
>    }
>    return PciIoMemRW (Width, Count, 1, Buffer, 1, Address);
>  }
>
> +/**
> +  Enable a PCI driver to access PCI config space.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
> +  @param  Count                 The number of memory or I/O operations to perform.
> +  @param  Buffer                For read operations, the destination buffer to store the results. For write
> +                                operations, the source buffer to write data from.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoPciWrite (
>    IN EFI_PCI_IO_PROTOCOL              *This,
> @@ -374,10 +510,28 @@ PciIoPciWrite (
>    }
>
>    return PciIoMemRW (Width, Count, 1, Address, 1, Buffer);
>  }
>
> +/**
> +  Enables a PCI driver to copy one region of PCI memory space to another region of PCI
> +  memory space.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory operations.
> +  @param  DestBarIndex          The BAR index in the standard PCI Configuration header to use as the
> +                                base address for the memory operation to perform.
> +  @param  DestOffset            The destination offset within the BAR specified by DestBarIndex to
> +                                start the memory writes for the copy operation.
> +  @param  SrcBarIndex           The BAR index in the standard PCI Configuration header to use as the
> +                                base address for the memory operation to perform.
> +  @param  SrcOffset             The source offset within the BAR specified by SrcBarIndex to start
> +                                the memory reads for the copy operation.
> +  @param  Count                 The number of memory operations to perform. Bytes moved is Width
> +                                size * Count, starting at DestOffset and SrcOffset.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoCopyMem (
>    IN EFI_PCI_IO_PROTOCOL              *This,
> @@ -391,10 +545,29 @@ PciIoCopyMem (
>  {
>    ASSERT (FALSE);
>    return EFI_UNSUPPORTED;
>  }
>
> +/**
> +  Provides the PCI controller-specific addresses needed to access system memory.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Operation             Indicates if the bus master is going to read or write to system memory.
> +  @param  HostAddress           The system memory address to map to the PCI controller.
> +  @param  NumberOfBytes         On input the number of bytes to map. On output the number of bytes
> +                                that were mapped.
> +  @param  DeviceAddress         The resulting map address for the bus master PCI controller to use to
> +                                access the hosts HostAddress.
> +  @param  Mapping               A resulting value to pass to Unmap().
> +
> +  @retval EFI_SUCCESS           The range was mapped for the returned NumberOfBytes.
> +  @retval EFI_UNSUPPORTED       The HostAddress cannot be mapped as a common buffer.
> +  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
> +  @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a lack of resources.
> +  @retval EFI_DEVICE_ERROR      The system hardware could not map the requested address.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  CoherentPciIoMap (
>    IN     EFI_PCI_IO_PROTOCOL            *This,
> @@ -457,10 +630,19 @@ CoherentPciIoMap (
>      *Mapping = NULL;
>    }
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Completes the Map() operation and releases any corresponding resources.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Mapping               The mapping value returned from Map().
> +
> +  @retval EFI_SUCCESS           The range was unmapped.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  CoherentPciIoUnmap (
>    IN  EFI_PCI_IO_PROTOCOL          *This,
> @@ -480,10 +662,29 @@ CoherentPciIoUnmap (
>      FreePool (MapInfo);
>    }
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Allocates pages.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Type                  This parameter is not used and must be ignored.
> +  @param  MemoryType            The type of memory to allocate, EfiBootServicesData or
> +                                EfiRuntimeServicesData.
> +  @param  Pages                 The number of pages to allocate.
> +  @param  HostAddress           A pointer to store the base system memory address of the
> +                                allocated range.
> +  @param  Attributes            The requested bit mask of attributes for the allocated range.
> +
> +  @retval EFI_SUCCESS           The requested memory pages were allocated.
> +  @retval EFI_UNSUPPORTED       Attributes is unsupported. The only legal attribute bits are
> +                                MEMORY_WRITE_COMBINE and MEMORY_CACHED.
> +  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
> +  @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  CoherentPciIoAllocateBuffer (
>    IN  EFI_PCI_IO_PROTOCOL         *This,
> @@ -522,10 +723,20 @@ CoherentPciIoAllocateBuffer (
>      *HostAddress = (VOID *)(UINTN)AllocAddress;
>    }
>    return Status;
>  }
>
> +/**
> +  Frees memory that was allocated with AllocatePages().
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Pages                 The number of pages to free.
> +  @param  HostAddress           The base system memory address of the allocated range.
> +
> +  @retval EFI_SUCCESS           The requested memory pages were freed.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  CoherentPciIoFreeBuffer (
>    IN  EFI_PCI_IO_PROTOCOL         *This,
> @@ -535,21 +746,42 @@ CoherentPciIoFreeBuffer (
>  {
>    FreePages (HostAddress, Pages);
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Flushes all PCI posted write transactions from a PCI host bridge to system memory.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +
> +  @retval EFI_SUCCESS           The PCI posted write transactions were flushed from the PCI host
> +                                bridge to system memory.
>
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoFlush (
>    IN EFI_PCI_IO_PROTOCOL          *This
>    )
>  {
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Retrieves this PCI controller's current PCI bus number, device number, and function number.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  SegmentNumber         The PCI controller's current PCI segment number.
> +  @param  BusNumber             The PCI controller's current PCI bus number.
> +  @param  DeviceNumber          The PCI controller's current PCI device number.
> +  @param  FunctionNumber        The PCI controller's current PCI function number.
> +
> +  @retval EFI_SUCCESS           The PCI controller location was returned.
> +  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoGetLocation (
>    IN   EFI_PCI_IO_PROTOCOL  *This,
> @@ -572,10 +804,29 @@ PciIoGetLocation (
>    *FunctionNumber = 0;
>
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Performs an operation on the attributes that this PCI controller supports. The operations include
> +  getting the set of supported attributes, retrieving the current attributes, setting the current
> +  attributes, enabling attributes, and disabling attributes.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Operation             The operation to perform on the attributes for this PCI controller.
> +  @param  Attributes            The mask of attributes that are used for Set, Enable, and Disable
> +                                operations.
> +  @param  Result                A pointer to the result mask of attributes that are returned for the Get
> +                                and Supported operations.
> +
> +  @retval EFI_SUCCESS           The operation on the PCI controller's attributes was completed.
> +  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
> +  @retval EFI_UNSUPPORTED       one or more of the bits set in
> +                                Attributes are not supported by this PCI controller or one of
> +                                its parent bridges when Operation is Set, Enable or Disable.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoAttributes (
>    IN  EFI_PCI_IO_PROTOCOL                      *This,
> @@ -629,10 +880,32 @@ PciIoAttributes (
>      Dev->Enabled = TRUE;
>    }
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Gets the attributes that this PCI controller supports setting on a BAR using
> +  SetBarAttributes(), and retrieves the list of resource descriptors for a BAR.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for resource range. The legal range for this field is 0..5.
> +  @param  Supports              A pointer to the mask of attributes that this PCI controller supports
> +                                setting for this BAR with SetBarAttributes().
> +  @param  Resources             A pointer to the ACPI 2.0 resource descriptors that describe the current
> +                                configuration of this BAR of the PCI controller.
> +
> +  @retval EFI_SUCCESS           If Supports is not NULL, then the attributes that the PCI
> +                                controller supports are returned in Supports. If Resources
> +                                is not NULL, then the ACPI 2.0 resource descriptors that the PCI
> +                                controller is currently using are returned in Resources.
> +  @retval EFI_INVALID_PARAMETER Both Supports and Attributes are NULL.
> +  @retval EFI_UNSUPPORTED       BarIndex not valid for this PCI controller.
> +  @retval EFI_OUT_OF_RESOURCES  There are not enough resources available to allocate
> +                                Resources.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoGetBarAttributes (
>    IN EFI_PCI_IO_PROTOCOL             *This,
> @@ -680,10 +953,23 @@ PciIoGetBarAttributes (
>      *Resources = Descriptor;
>    }
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Sets the attributes for a range of a BAR on a PCI controller.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Attributes            The mask of attributes to set for the resource range specified by
> +                                BarIndex, Offset, and Length.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for resource range. The legal range for this field is 0..5.
> +  @param  Offset                A pointer to the BAR relative base address of the resource range to be
> +                                modified by the attributes specified by Attributes.
> +  @param  Length                A pointer to the length of the resource range to be modified by the
> +                                attributes specified by Attributes.
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoSetBarAttributes (
>    IN     EFI_PCI_IO_PROTOCOL          *This,
> @@ -716,10 +1002,16 @@ STATIC CONST EFI_PCI_IO_PROTOCOL PciIoTemplate =
>    PciIoSetBarAttributes,
>    0,
>    0
>  };
>
> +/**
> +  Initialize PciIo Protocol.
> +
> +  @param  Dev      Point to NON_DISCOVERABLE_PCI_DEVICE instance.
> +
> +**/
>  VOID
>  InitializePciIoProtocol (
>    NON_DISCOVERABLE_PCI_DEVICE     *Dev
>    )
>  {
> diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h
> index bc0a3d3..0b83aff 100644
> --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h
> @@ -71,10 +71,16 @@ typedef struct {
>    // Whether this device has been enabled
>    //
>    BOOLEAN                   Enabled;
>  } NON_DISCOVERABLE_PCI_DEVICE;
>
> +/**
> +  Initialize PciIo Protocol.
> +
> +  @param  Device      Point to NON_DISCOVERABLE_PCI_DEVICE instance.
> +
> +**/
>  VOID
>  InitializePciIoProtocol (
>    NON_DISCOVERABLE_PCI_DEVICE     *Device
>    );
>
> --
> 1.9.5.msysgit.1
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch 1/2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Add comments for functions
  2016-12-15  7:45 ` [patch 1/2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Add comments for functions Ard Biesheuvel
@ 2016-12-15  8:31   ` Bi, Dandan
  2016-12-15  8:32     ` Ard Biesheuvel
  0 siblings, 1 reply; 7+ messages in thread
From: Bi, Dandan @ 2016-12-15  8:31 UTC (permalink / raw)
  To: Ard Biesheuvel, Ni, Ruiyu; +Cc: edk2-devel@lists.01.org

Yes, I will do the enhancements base on the latest codes and re-send patches.

Thanks,
Dandan

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
Sent: Thursday, December 15, 2016 3:46 PM
To: Bi, Dandan <dandan.bi@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
Cc: edk2-devel@lists.01.org
Subject: Re: [patch 1/2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Add comments for functions

Hello Dandan,

Thank you for doing this work. I am happy for these changes to go in.

However, I have a patch pending to implement non-coherent DMA support, and Ray has not responded to that yet. These changes will likely conflict with that patch, so could we please get the functionality completed first before working on enhancements like this?

Ray?

Thanks,
Ard.


On 15 December 2016 at 07:11, Dandan Bi <dandan.bi@intel.com> wrote:
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  .../NonDiscoverablePciDeviceDxe/ComponentName.c    |  47 ++++
>  .../NonDiscoverablePciDeviceDxe.c                  |  50 +++-
>  .../NonDiscoverablePciDeviceDxe.inf                |  18 +-
>  .../NonDiscoverablePciDeviceIo.c                   | 298 ++++++++++++++++++++-
>  .../NonDiscoverablePciDeviceIo.h                   |   6 +
>  5 files changed, 405 insertions(+), 14 deletions(-)
>
> diff --git 
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c 
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c
> index 6e51d00..0e6ebec 100644
> --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/ComponentName.c
> @@ -28,10 +28,31 @@ EFI_UNICODE_STRING_TABLE mDriverNameTable[] = {
>    { NULL,     NULL                   }
>  };
>
>  EFI_COMPONENT_NAME_PROTOCOL gComponentName;
>
> +/**
> +  Retrieves a Unicode string that is the user readable name of the UEFI Driver.
> +
> +  @param This           A pointer to the EFI_COMPONENT_NAME_PROTOCOL instance.
> +  @param Language       A pointer to a three character ISO 639-2 language identifier.
> +                        This is the language of the driver name that that the caller
> +                        is requesting, and it must match one of the languages specified
> +                        in SupportedLanguages.  The number of languages supported by a
> +                        driver is up to the driver writer.
> +  @param DriverName     A pointer to the Unicode string to return.  This Unicode string
> +                        is the name of the driver specified by This in the language
> +                        specified by Language.
> +
> +  @retval EFI_SUCCESS           The Unicode string for the Driver specified by This
> +                                and the language specified by Language was returned
> +                                in DriverName.
> +  @retval EFI_INVALID_PARAMETER Language is NULL.
> +  @retval EFI_INVALID_PARAMETER DriverName is NULL.
> +  @retval EFI_UNSUPPORTED       The driver specified by This does not support the
> +                                language specified by Language.
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  NonDiscoverablePciGetDriverName (
>    IN  EFI_COMPONENT_NAME_PROTOCOL *This, @@ -46,10 +67,36 @@ 
> NonDiscoverablePciGetDriverName (
>             DriverName,
>             (BOOLEAN)(This == &gComponentName) // Iso639Language
>             );
>  }
>
> +/**
> +  Retrieves a Unicode string that is the user readable name of the 
> +controller
> +  that is being managed by an UEFI Driver.
> +
> +  @param This                   A pointer to the EFI_COMPONENT_NAME_PROTOCOL instance.
> +  @param DeviceHandle           The handle of a controller that the driver specified by
> +                                This is managing.  This handle specifies the controller
> +                                whose name is to be returned.
> +  @param ChildHandle            The handle of the child controller to retrieve the name
> +                                of.  This is an optional parameter that may be NULL.  It
> +                                will be NULL for device drivers.  It will also be NULL
> +                                for a bus drivers that wish to retrieve the name of the
> +                                bus controller.  It will not be NULL for a bus driver
> +                                that wishes to retrieve the name of a child controller.
> +  @param Language               A pointer to a three character ISO 639-2 language
> +                                identifier.  This is the language of the controller name
> +                                that that the caller is requesting, and it must match one
> +                                of the languages specified in SupportedLanguages.  The
> +                                number of languages supported by a driver is up to the
> +                                driver writer.
> +  @param ControllerName         A pointer to the Unicode string to return.  This Unicode
> +                                string is the name of the controller specified by
> +                                ControllerHandle and ChildHandle in the language
> +                                specified by Language from the point of view of the
> +                                driver specified by This.
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  NonDiscoverablePciGetDeviceName (
>    IN  EFI_COMPONENT_NAME_PROTOCOL *This, diff --git 
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceDxe.c 
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceDxe.c
> index ee765d7..a868ea2 100644
> --- 
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceDxe.c
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> +++ PciDeviceDxe.c
> @@ -45,10 +45,23 @@ SupportedNonDiscoverableDevices [] = {
>  //   - 5.1.3.4 OpenProtocol() and CloseProtocol()
>  // - UEFI Spec 2.3.1 + Errata C
>  //   -  6.3 Protocol Handler Services
>  //
>
> +/**
> +  Supported function of Driver Binding protocol for this driver.
> +  Test to see if this driver supports ControllerHandle.
> +
> +  @param This                   Protocol instance pointer.
> +  @param DeviceHandle           Handle of device to test.
> +  @param RemainingDevicePath    A pointer to the device path.
> +                                it should be ignored by device driver.
> +
> +  @retval EFI_SUCCESS           This driver supports this device.
> +  @retval other                 This driver does not support this device.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  NonDiscoverablePciDeviceSupported (
>    IN EFI_DRIVER_BINDING_PROTOCOL *This, @@ -106,10 +119,23 @@ 
> CloseProtocol:
>           This->DriverBindingHandle, DeviceHandle);
>
>    return Status;
>  }
>
> +/**
> +  This routine is called right after the .Supported() called and
> +  Start this driver on ControllerHandle.
> +
> +  @param This                   Protocol instance pointer.
> +  @param DeviceHandle           Handle of device to bind driver to.
> +  @param RemainingDevicePath    A pointer to the device path.
> +                                it should be ignored by device driver.
> +
> +  @retval EFI_SUCCESS           This driver is added to this device.
> +  @retval other                 Some error occurs when binding this driver to this device.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  NonDiscoverablePciDeviceStart (
>    IN EFI_DRIVER_BINDING_PROTOCOL *This, @@ -156,11 +182,22 @@ 
> FreeDev:
>    FreePool (Dev);
>
>    return Status;
>  }
>
> +/**
> +  Stop this driver on ControllerHandle.
> +
> +  @param This               Protocol instance pointer.
> +  @param DeviceHandle       Handle of device to stop driver on.
> +  @param NumberOfChildren   Not used.
> +  @param ChildHandleBuffer  Not used.
>
> +  @retval EFI_SUCCESS   This driver is removed from this device.
> +  @retval other         Some error occurs when removing this driver from this device.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  NonDiscoverablePciDeviceStop (
>    IN EFI_DRIVER_BINDING_PROTOCOL *This, @@ -212,13 +249,20 @@ STATIC 
> EFI_DRIVER_BINDING_PROTOCOL gDriverBinding = {
>    0x10, // Version, must be in [0x10 .. 0xFFFFFFEF] for IHV-developed drivers
>    NULL,
>    NULL
>  };
>
> -//
> -// Entry point of this driver.
> -//
> +/**
> +  Entry point of this driver.
> +
> +  @param  ImageHandle     Image handle this driver.
> +  @param  SystemTable     Pointer to the System Table.
> +
> +  @retval EFI_SUCCESS     The entry point is executed successfully.
> +  @retval other           Some error occurred when executing this entry point.
> +
> +**/
>  EFI_STATUS
>  EFIAPI
>  NonDiscoverablePciDeviceDxeEntryPoint (
>    IN EFI_HANDLE       ImageHandle,
>    IN EFI_SYSTEM_TABLE *SystemTable
> diff --git 
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceDxe.inf 
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceDxe.inf
> index 996fe31..6d3f721 100644
> --- 
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceDxe.inf
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> +++ PciDeviceDxe.inf
> @@ -1,6 +1,8 @@
>  ## @file
> +# PCI I/O driver for non-discoverable devices.
> +#
>  # Copyright (C) 2016, Linaro Ltd.
>  #
>  # 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 @@ -40,13 +42,13 @@ [LibraryClasses]  [Protocols]
>    gEfiPciIoProtocolGuid                         ## BY_START
>    gEdkiiNonDiscoverableDeviceProtocolGuid       ## TO_START
>
>  [Guids]
> -  gEdkiiNonDiscoverableAhciDeviceGuid
> -  gEdkiiNonDiscoverableEhciDeviceGuid
> -  gEdkiiNonDiscoverableNvmeDeviceGuid
> -  gEdkiiNonDiscoverableOhciDeviceGuid
> -  gEdkiiNonDiscoverableSdhciDeviceGuid
> -  gEdkiiNonDiscoverableUfsDeviceGuid
> -  gEdkiiNonDiscoverableUhciDeviceGuid
> -  gEdkiiNonDiscoverableXhciDeviceGuid
> +  gEdkiiNonDiscoverableAhciDeviceGuid       ## CONSUMES ## GUID
> +  gEdkiiNonDiscoverableEhciDeviceGuid       ## CONSUMES ## GUID
> +  gEdkiiNonDiscoverableNvmeDeviceGuid       ## CONSUMES ## GUID
> +  gEdkiiNonDiscoverableOhciDeviceGuid       ## CONSUMES ## GUID
> +  gEdkiiNonDiscoverableSdhciDeviceGuid      ## CONSUMES ## GUID
> +  gEdkiiNonDiscoverableUfsDeviceGuid        ## CONSUMES ## GUID
> +  gEdkiiNonDiscoverableUhciDeviceGuid       ## CONSUMES ## GUID
> +  gEdkiiNonDiscoverableXhciDeviceGuid       ## CONSUMES ## GUID
> diff --git 
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c 
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> index 82ee9d1..59b6076 100644
> --- 
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> +++ PciDeviceIo.c
> @@ -24,13 +24,18 @@ typedef struct {
>    VOID                            *HostAddress;
>    EFI_PCI_IO_PROTOCOL_OPERATION   Operation;
>    UINTN                           NumberOfBytes;
>  } NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO;
>
> -//
> -// Get the resource associated with BAR number 'BarIndex'.
> -//
> +/**
> +  Get the resource associated with BAR number 'BarIndex'.
> +
> +  @param  Dev           Point to the NON_DISCOVERABLE_PCI_DEVICE instance.
> +  @param  BarIndex      The BAR index of the standard PCI Configuration header to use as the
> +                        base address for the memory operation to perform.
> +  @param  Descriptor    Points to the address space descriptor
> +**/
>  STATIC
>  EFI_STATUS
>  GetBarResource (
>    IN  NON_DISCOVERABLE_PCI_DEVICE         *Dev,
>    IN  UINT8                               BarIndex,
> @@ -57,10 +62,25 @@ GetBarResource (
>      BarIndex -= 1;
>    }
>    return EFI_NOT_FOUND;
>  }
>
> +/**
> +  Reads from the memory space of a PCI controller. Returns either 
> +when the polling exit criteria is
> +  satisfied or after a defined duration.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for the memory operation to perform.
> +  @param  Offset                The offset within the selected BAR to start the memory operation.
> +  @param  Mask                  Mask used for the polling criteria.
> +  @param  Value                 The comparison value used for the polling exit criteria.
> +  @param  Delay                 The number of 100 ns units to poll.
> +  @param  Result                Pointer to the last value read from the memory location.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoPollMem (
>    IN  EFI_PCI_IO_PROTOCOL         *This,
> @@ -75,10 +95,25 @@ PciIoPollMem (
>  {
>    ASSERT (FALSE);
>    return EFI_UNSUPPORTED;
>  }
>
> +/**
> +  Reads from the memory space of a PCI controller. Returns either 
> +when the polling exit criteria is
> +  satisfied or after a defined duration.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for the memory operation to perform.
> +  @param  Offset                The offset within the selected BAR to start the memory operation.
> +  @param  Mask                  Mask used for the polling criteria.
> +  @param  Value                 The comparison value used for the polling exit criteria.
> +  @param  Delay                 The number of 100 ns units to poll.
> +  @param  Result                Pointer to the last value read from the memory location.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoPollIo (
>    IN  EFI_PCI_IO_PROTOCOL         *This,
> @@ -93,10 +128,23 @@ PciIoPollIo (
>  {
>    ASSERT (FALSE);
>    return EFI_UNSUPPORTED;
>  }
>
> +/**
> +  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
> +
> +  @param  Width         Signifies the width of the memory or I/O operations.
> +  @param  Count         The number of memory or I/O operations to perform.
> +  @param  DstStride     The stride of the destination buffer.
> +  @param  Dst           For read operations, the destination buffer to store the results. For write
> +                        operations, the destination buffer to write data to.
> +  @param  SrcStride     The stride of the source buffer.
> +  @param  Src           For read operations, the source buffer to read data from. For write
> +                        operations, the source buffer to write data from.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoMemRW (
>    IN  EFI_PCI_IO_PROTOCOL_WIDTH   Width,
> @@ -144,10 +192,30 @@ PciIoMemRW (
>    }
>
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for the memory or I/O operation to perform.
> +  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
> +  @param  Count                 The number of memory or I/O operations to perform.
> +  @param  Buffer                For read operations, the destination buffer to store the results. For write
> +                                operations, the source buffer to write data from.
> +
> +  @retval EFI_SUCCESS           The data was read from or written to the PCI controller.
> +  @retval EFI_UNSUPPORTED       BarIndex not valid for this PCI controller.
> +  @retval EFI_UNSUPPORTED       The address range specified by Offset, Width, and Count is not
> +                                valid for the PCI BAR specified by BarIndex.
> +  @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a lack of resources.
> +  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoMemRead (
>    IN     EFI_PCI_IO_PROTOCOL          *This,
> @@ -211,10 +279,30 @@ PciIoMemRead (
>      break;
>    }
>    return EFI_INVALID_PARAMETER;
>  }
>
> +/**
> +  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for the memory or I/O operation to perform.
> +  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
> +  @param  Count                 The number of memory or I/O operations to perform.
> +  @param  Buffer                For read operations, the destination buffer to store the results. For write
> +                                operations, the source buffer to write data from.
> +
> +  @retval EFI_SUCCESS           The data was read from or written to the PCI controller.
> +  @retval EFI_UNSUPPORTED       BarIndex not valid for this PCI controller.
> +  @retval EFI_UNSUPPORTED       The address range specified by Offset, Width, and Count is not
> +                                valid for the PCI BAR specified by BarIndex.
> +  @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a lack of resources.
> +  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoMemWrite (
>    IN     EFI_PCI_IO_PROTOCOL          *This,
> @@ -278,10 +366,23 @@ PciIoMemWrite (
>      break;
>    }
>    return EFI_INVALID_PARAMETER;
>  }
>
> +/**
> +  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for the memory or I/O operation to perform.
> +  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
> +  @param  Count                 The number of memory or I/O operations to perform.
> +  @param  Buffer                For read operations, the destination buffer to store the results. For write
> +                                operations, the source buffer to write data from.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoIoRead (
>    IN EFI_PCI_IO_PROTOCOL              *This,
> @@ -294,10 +395,23 @@ PciIoIoRead (
>  {
>    ASSERT (FALSE);
>    return EFI_UNSUPPORTED;
>  }
>
> +/**
> +  Enable a PCI driver to access PCI controller registers in the PCI memory or I/O space.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for the memory or I/O operation to perform.
> +  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
> +  @param  Count                 The number of memory or I/O operations to perform.
> +  @param  Buffer                For read operations, the destination buffer to store the results. For write
> +                                operations, the source buffer to write data from.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoIoWrite (
>    IN     EFI_PCI_IO_PROTOCOL          *This,
> @@ -310,10 +424,21 @@ PciIoIoWrite (
>  {
>    ASSERT (FALSE);
>    return EFI_UNSUPPORTED;
>  }
>
> +/**
> +  Enable a PCI driver to access PCI config space.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
> +  @param  Count                 The number of memory or I/O operations to perform.
> +  @param  Buffer                For read operations, the destination buffer to store the results. For write
> +                                operations, the source buffer to write data from.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoPciRead (
>    IN     EFI_PCI_IO_PROTOCOL        *This,
> @@ -346,10 +471,21 @@ PciIoPciRead (
>      Count -= Length >> ((UINTN)Width & 0x3);
>    }
>    return PciIoMemRW (Width, Count, 1, Buffer, 1, Address);  }
>
> +/**
> +  Enable a PCI driver to access PCI config space.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory or I/O operations.
> +  @param  Offset                The offset within the selected BAR to start the memory or I/O operation.
> +  @param  Count                 The number of memory or I/O operations to perform.
> +  @param  Buffer                For read operations, the destination buffer to store the results. For write
> +                                operations, the source buffer to write data from.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoPciWrite (
>    IN EFI_PCI_IO_PROTOCOL              *This,
> @@ -374,10 +510,28 @@ PciIoPciWrite (
>    }
>
>    return PciIoMemRW (Width, Count, 1, Address, 1, Buffer);  }
>
> +/**
> +  Enables a PCI driver to copy one region of PCI memory space to 
> +another region of PCI
> +  memory space.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Width                 Signifies the width of the memory operations.
> +  @param  DestBarIndex          The BAR index in the standard PCI Configuration header to use as the
> +                                base address for the memory operation to perform.
> +  @param  DestOffset            The destination offset within the BAR specified by DestBarIndex to
> +                                start the memory writes for the copy operation.
> +  @param  SrcBarIndex           The BAR index in the standard PCI Configuration header to use as the
> +                                base address for the memory operation to perform.
> +  @param  SrcOffset             The source offset within the BAR specified by SrcBarIndex to start
> +                                the memory reads for the copy operation.
> +  @param  Count                 The number of memory operations to perform. Bytes moved is Width
> +                                size * Count, starting at DestOffset and SrcOffset.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoCopyMem (
>    IN EFI_PCI_IO_PROTOCOL              *This,
> @@ -391,10 +545,29 @@ PciIoCopyMem (
>  {
>    ASSERT (FALSE);
>    return EFI_UNSUPPORTED;
>  }
>
> +/**
> +  Provides the PCI controller-specific addresses needed to access system memory.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Operation             Indicates if the bus master is going to read or write to system memory.
> +  @param  HostAddress           The system memory address to map to the PCI controller.
> +  @param  NumberOfBytes         On input the number of bytes to map. On output the number of bytes
> +                                that were mapped.
> +  @param  DeviceAddress         The resulting map address for the bus master PCI controller to use to
> +                                access the hosts HostAddress.
> +  @param  Mapping               A resulting value to pass to Unmap().
> +
> +  @retval EFI_SUCCESS           The range was mapped for the returned NumberOfBytes.
> +  @retval EFI_UNSUPPORTED       The HostAddress cannot be mapped as a common buffer.
> +  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
> +  @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a lack of resources.
> +  @retval EFI_DEVICE_ERROR      The system hardware could not map the requested address.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  CoherentPciIoMap (
>    IN     EFI_PCI_IO_PROTOCOL            *This,
> @@ -457,10 +630,19 @@ CoherentPciIoMap (
>      *Mapping = NULL;
>    }
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Completes the Map() operation and releases any corresponding resources.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Mapping               The mapping value returned from Map().
> +
> +  @retval EFI_SUCCESS           The range was unmapped.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  CoherentPciIoUnmap (
>    IN  EFI_PCI_IO_PROTOCOL          *This,
> @@ -480,10 +662,29 @@ CoherentPciIoUnmap (
>      FreePool (MapInfo);
>    }
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Allocates pages.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Type                  This parameter is not used and must be ignored.
> +  @param  MemoryType            The type of memory to allocate, EfiBootServicesData or
> +                                EfiRuntimeServicesData.
> +  @param  Pages                 The number of pages to allocate.
> +  @param  HostAddress           A pointer to store the base system memory address of the
> +                                allocated range.
> +  @param  Attributes            The requested bit mask of attributes for the allocated range.
> +
> +  @retval EFI_SUCCESS           The requested memory pages were allocated.
> +  @retval EFI_UNSUPPORTED       Attributes is unsupported. The only legal attribute bits are
> +                                MEMORY_WRITE_COMBINE and MEMORY_CACHED.
> +  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
> +  @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  CoherentPciIoAllocateBuffer (
>    IN  EFI_PCI_IO_PROTOCOL         *This,
> @@ -522,10 +723,20 @@ CoherentPciIoAllocateBuffer (
>      *HostAddress = (VOID *)(UINTN)AllocAddress;
>    }
>    return Status;
>  }
>
> +/**
> +  Frees memory that was allocated with AllocatePages().
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Pages                 The number of pages to free.
> +  @param  HostAddress           The base system memory address of the allocated range.
> +
> +  @retval EFI_SUCCESS           The requested memory pages were freed.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  CoherentPciIoFreeBuffer (
>    IN  EFI_PCI_IO_PROTOCOL         *This,
> @@ -535,21 +746,42 @@ CoherentPciIoFreeBuffer (  {
>    FreePages (HostAddress, Pages);
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Flushes all PCI posted write transactions from a PCI host bridge to system memory.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +
> +  @retval EFI_SUCCESS           The PCI posted write transactions were flushed from the PCI host
> +                                bridge to system memory.
>
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoFlush (
>    IN EFI_PCI_IO_PROTOCOL          *This
>    )
>  {
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Retrieves this PCI controller's current PCI bus number, device number, and function number.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  SegmentNumber         The PCI controller's current PCI segment number.
> +  @param  BusNumber             The PCI controller's current PCI bus number.
> +  @param  DeviceNumber          The PCI controller's current PCI device number.
> +  @param  FunctionNumber        The PCI controller's current PCI function number.
> +
> +  @retval EFI_SUCCESS           The PCI controller location was returned.
> +  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoGetLocation (
>    IN   EFI_PCI_IO_PROTOCOL  *This,
> @@ -572,10 +804,29 @@ PciIoGetLocation (
>    *FunctionNumber = 0;
>
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Performs an operation on the attributes that this PCI controller 
> +supports. The operations include
> +  getting the set of supported attributes, retrieving the current 
> +attributes, setting the current
> +  attributes, enabling attributes, and disabling attributes.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Operation             The operation to perform on the attributes for this PCI controller.
> +  @param  Attributes            The mask of attributes that are used for Set, Enable, and Disable
> +                                operations.
> +  @param  Result                A pointer to the result mask of attributes that are returned for the Get
> +                                and Supported operations.
> +
> +  @retval EFI_SUCCESS           The operation on the PCI controller's attributes was completed.
> +  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
> +  @retval EFI_UNSUPPORTED       one or more of the bits set in
> +                                Attributes are not supported by this PCI controller or one of
> +                                its parent bridges when Operation is Set, Enable or Disable.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoAttributes (
>    IN  EFI_PCI_IO_PROTOCOL                      *This,
> @@ -629,10 +880,32 @@ PciIoAttributes (
>      Dev->Enabled = TRUE;
>    }
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Gets the attributes that this PCI controller supports setting on a 
> +BAR using
> +  SetBarAttributes(), and retrieves the list of resource descriptors for a BAR.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for resource range. The legal range for this field is 0..5.
> +  @param  Supports              A pointer to the mask of attributes that this PCI controller supports
> +                                setting for this BAR with SetBarAttributes().
> +  @param  Resources             A pointer to the ACPI 2.0 resource descriptors that describe the current
> +                                configuration of this BAR of the PCI controller.
> +
> +  @retval EFI_SUCCESS           If Supports is not NULL, then the attributes that the PCI
> +                                controller supports are returned in Supports. If Resources
> +                                is not NULL, then the ACPI 2.0 resource descriptors that the PCI
> +                                controller is currently using are returned in Resources.
> +  @retval EFI_INVALID_PARAMETER Both Supports and Attributes are NULL.
> +  @retval EFI_UNSUPPORTED       BarIndex not valid for this PCI controller.
> +  @retval EFI_OUT_OF_RESOURCES  There are not enough resources available to allocate
> +                                Resources.
> +
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoGetBarAttributes (
>    IN EFI_PCI_IO_PROTOCOL             *This,
> @@ -680,10 +953,23 @@ PciIoGetBarAttributes (
>      *Resources = Descriptor;
>    }
>    return EFI_SUCCESS;
>  }
>
> +/**
> +  Sets the attributes for a range of a BAR on a PCI controller.
> +
> +  @param  This                  A pointer to the EFI_PCI_IO_PROTOCOL instance.
> +  @param  Attributes            The mask of attributes to set for the resource range specified by
> +                                BarIndex, Offset, and Length.
> +  @param  BarIndex              The BAR index of the standard PCI Configuration header to use as the
> +                                base address for resource range. The legal range for this field is 0..5.
> +  @param  Offset                A pointer to the BAR relative base address of the resource range to be
> +                                modified by the attributes specified by Attributes.
> +  @param  Length                A pointer to the length of the resource range to be modified by the
> +                                attributes specified by Attributes.
> +**/
>  STATIC
>  EFI_STATUS
>  EFIAPI
>  PciIoSetBarAttributes (
>    IN     EFI_PCI_IO_PROTOCOL          *This,
> @@ -716,10 +1002,16 @@ STATIC CONST EFI_PCI_IO_PROTOCOL PciIoTemplate =
>    PciIoSetBarAttributes,
>    0,
>    0
>  };
>
> +/**
> +  Initialize PciIo Protocol.
> +
> +  @param  Dev      Point to NON_DISCOVERABLE_PCI_DEVICE instance.
> +
> +**/
>  VOID
>  InitializePciIoProtocol (
>    NON_DISCOVERABLE_PCI_DEVICE     *Dev
>    )
>  {
> diff --git 
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.h 
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.h
> index bc0a3d3..0b83aff 100644
> --- 
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.h
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> +++ PciDeviceIo.h
> @@ -71,10 +71,16 @@ typedef struct {
>    // Whether this device has been enabled
>    //
>    BOOLEAN                   Enabled;
>  } NON_DISCOVERABLE_PCI_DEVICE;
>
> +/**
> +  Initialize PciIo Protocol.
> +
> +  @param  Device      Point to NON_DISCOVERABLE_PCI_DEVICE instance.
> +
> +**/
>  VOID
>  InitializePciIoProtocol (
>    NON_DISCOVERABLE_PCI_DEVICE     *Device
>    );
>
> --
> 1.9.5.msysgit.1
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch 1/2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Add comments for functions
  2016-12-15  8:31   ` Bi, Dandan
@ 2016-12-15  8:32     ` Ard Biesheuvel
  0 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2016-12-15  8:32 UTC (permalink / raw)
  To: Bi, Dandan; +Cc: Ni, Ruiyu, edk2-devel@lists.01.org

On 15 December 2016 at 08:31, Bi, Dandan <dandan.bi@intel.com> wrote:
> Yes, I will do the enhancements base on the latest codes and re-send patches.
>

Great, thanks!


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch 2/2] MdeModulePkg/NonDiscoverablePciDevice: Make variable definition follow rule
  2016-12-15  7:11 ` [patch 2/2] MdeModulePkg/NonDiscoverablePciDevice: Make variable definition follow rule Dandan Bi
@ 2016-12-15 11:41   ` Ard Biesheuvel
  2016-12-16  1:23     ` Bi, Dandan
  0 siblings, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2016-12-15 11:41 UTC (permalink / raw)
  To: Dandan Bi; +Cc: edk2-devel@lists.01.org, Ruiyu Ni

On 15 December 2016 at 07:11, Dandan Bi <dandan.bi@intel.com> wrote:
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>

I have no objections to this patch, but which rule are we talking about here?

> ---
>  .../Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c  | 2 +-
>  .../Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c   | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
> index a868ea2..921225b 100644
> --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
> @@ -19,11 +19,11 @@
>  //
>  // We only support the following device types
>  //
>  STATIC
>  CONST EFI_GUID * CONST
> -SupportedNonDiscoverableDevices [] = {
> +SupportedNonDiscoverableDevices[] = {
>    &gEdkiiNonDiscoverableAhciDeviceGuid,
>    &gEdkiiNonDiscoverableEhciDeviceGuid,
>    &gEdkiiNonDiscoverableNvmeDeviceGuid,
>    &gEdkiiNonDiscoverableOhciDeviceGuid,
>    &gEdkiiNonDiscoverableSdhciDeviceGuid,
> diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
> index 59b6076..a54313f 100644
> --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
> @@ -913,11 +913,12 @@ PciIoGetBarAttributes (
>    OUT UINT64                         *Supports OPTIONAL,
>    OUT VOID                           **Resources OPTIONAL
>    )
>  {
>    NON_DISCOVERABLE_PCI_DEVICE       *Dev;
> -  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor, *BarDesc;
> +  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;
> +  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *BarDesc;
>    EFI_ACPI_END_TAG_DESCRIPTOR       *End;
>    EFI_STATUS                        Status;
>
>    if (Supports == NULL && Resources == NULL) {
>      return EFI_INVALID_PARAMETER;
> --
> 1.9.5.msysgit.1
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch 2/2] MdeModulePkg/NonDiscoverablePciDevice: Make variable definition follow rule
  2016-12-15 11:41   ` Ard Biesheuvel
@ 2016-12-16  1:23     ` Bi, Dandan
  0 siblings, 0 replies; 7+ messages in thread
From: Bi, Dandan @ 2016-12-16  1:23 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Ni, Ruiyu

Hi Ard,

The rule we are talking about here is :
For variable name,  it should be 1. First character should be upper case 2. Must contain lower case characters 3. No white space characters  4 Global variable name should start with a 'g'...etc.
The EDK II C Coding Standards Specification establishes a set of  coding rules, you can refer to it.
We also have codes scan tool  Ecc.exe in edk2-BaseTools-win32 to check the coding style issues.
You can run below commands to run ECC scan: 
ecc -c config.ini -e exception.xml -t directory you want to scan  -r reportresult.csv
such as: ecc -c config.ini -e exception.xml  -t  edk2\MdeModulePkg\Bus\Pci\NonDiscoverablePciDeviceDxe  -r NonDiscoverablePciDeviceDxe.csv
Then you will find some coding style issues list in the NonDiscoverablePciDeviceDxe.csv.


Thanks,
Dandan

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
Sent: Thursday, December 15, 2016 7:41 PM
To: Bi, Dandan <dandan.bi@intel.com>
Cc: edk2-devel@lists.01.org; Ni, Ruiyu <ruiyu.ni@intel.com>
Subject: Re: [patch 2/2] MdeModulePkg/NonDiscoverablePciDevice: Make variable definition follow rule

On 15 December 2016 at 07:11, Dandan Bi <dandan.bi@intel.com> wrote:
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>

I have no objections to this patch, but which rule are we talking about here?

> ---
>  .../Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c  | 2 +-
>  .../Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c   | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git 
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceDxe.c 
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceDxe.c
> index a868ea2..921225b 100644
> --- 
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceDxe.c
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> +++ PciDeviceDxe.c
> @@ -19,11 +19,11 @@
>  //
>  // We only support the following device types  //  STATIC  CONST 
> EFI_GUID * CONST -SupportedNonDiscoverableDevices [] = {
> +SupportedNonDiscoverableDevices[] = {
>    &gEdkiiNonDiscoverableAhciDeviceGuid,
>    &gEdkiiNonDiscoverableEhciDeviceGuid,
>    &gEdkiiNonDiscoverableNvmeDeviceGuid,
>    &gEdkiiNonDiscoverableOhciDeviceGuid,
>    &gEdkiiNonDiscoverableSdhciDeviceGuid,
> diff --git 
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c 
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> index 59b6076..a54313f 100644
> --- 
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciD
> eviceIo.c
> +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> +++ PciDeviceIo.c
> @@ -913,11 +913,12 @@ PciIoGetBarAttributes (
>    OUT UINT64                         *Supports OPTIONAL,
>    OUT VOID                           **Resources OPTIONAL
>    )
>  {
>    NON_DISCOVERABLE_PCI_DEVICE       *Dev;
> -  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor, *BarDesc;
> +  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;  
> + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *BarDesc;
>    EFI_ACPI_END_TAG_DESCRIPTOR       *End;
>    EFI_STATUS                        Status;
>
>    if (Supports == NULL && Resources == NULL) {
>      return EFI_INVALID_PARAMETER;
> --
> 1.9.5.msysgit.1
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-12-16  1:23 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-15  7:11 [patch 1/2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Add comments for functions Dandan Bi
2016-12-15  7:11 ` [patch 2/2] MdeModulePkg/NonDiscoverablePciDevice: Make variable definition follow rule Dandan Bi
2016-12-15 11:41   ` Ard Biesheuvel
2016-12-16  1:23     ` Bi, Dandan
2016-12-15  7:45 ` [patch 1/2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Add comments for functions Ard Biesheuvel
2016-12-15  8:31   ` Bi, Dandan
2016-12-15  8:32     ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox