public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>
Subject: Re: [PATCH 3/5] EmbeddedPkg: implement generic platform PCI I/O driver
Date: Wed, 2 Nov 2016 16:05:52 +0000	[thread overview]
Message-ID: <20161102160552.GC27644@bivouac.eciton.net> (raw)
In-Reply-To: <CAKv+Gu_wQh0Jha+MSRpgiEwsheCeGwTOPrzp4yBybgPRXX-ukw@mail.gmail.com>

On Wed, Nov 02, 2016 at 01:39:26PM +0000, Ard Biesheuvel wrote:
> On 1 November 2016 at 22:22, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Mon, Oct 31, 2016 at 06:13:08PM +0000, Ard Biesheuvel wrote:
> >> This implements support for platform PCI I/O devices, i.e, devices that
> >> are not on a PCI bus but that can be drived by generic PCI drivers in
> >
> > drived->driven? (or substitute "controlled")
> >
> 
> That's a typo, not a speako. But yes, let me clean that up
>
> >> EDK2.
> >>
> >> This is implemented as a UEFI driver, which means we take full advantage
> >> of the UEFI driver model, and only instantiate those devices that are
> >> necessary for booting.
> >>
> >> Care is taken to deal with DMA addressing limitations: DMA mappings and
> >> allocations are moved below 4 GB if the PCI driver has not informed us
> >> that the device being driven is 64-bit DMA capable.
> >
> > How do we deal with these devices if there is no RAM below 4GB?
> > (Signalling an error and aborting is fine, but worth calling out even
> > here in the commit message.)
> >
> 
> The same thing the normal PCI I/O implementations do: return an error.
> If you plug in a EHCI controller on Seattle that does not support
> 64-bit DMA, calls to AllocateBuffer() and Map() will fail in exactly
> the same way.

OK - if it's that straightforward, please ignore all of my other
comments on that topic.

> >>
> >> For now, this driver supports coherent DMA only, but support for
> >> non-coherent DMA is planned as well.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>  EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.c      | 649 ++++++++++++++++++++
> >>  EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.h      |  67 ++
> >>  EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.c   | 268 ++++++++
> >>  EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.inf |  41 ++
> >>  EmbeddedPkg/EmbeddedPkg.dsc                               |   1 +
> >>  5 files changed, 1026 insertions(+)
> >>
> >> diff --git a/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.c b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.c
> >> new file mode 100644
> >> index 000000000000..97ed19353347
> >> --- /dev/null
> >> +++ b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.c
> >> @@ -0,0 +1,649 @@
> >> +/** @file
> >> +
> >> +  Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
> >> +  Copyright (c) 2016, Linaro, Ltd. All rights reserved.<BR>
> >> +
> >> +  This program and the accompanying materials
> >> +  are licensed and made available under the terms and conditions of the BSD License
> >> +  which accompanies this distribution.  The full text of the license may be found at
> >> +  http://opensource.org/licenses/bsd-license.php
> >> +
> >> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> >> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> >> +
> >> +**/
> >> +
> >> +#include "PlatformPciIo.h"
> >> +
> >> +#include <Protocol/PciRootBridgeIo.h>
> >> +
> >> +typedef struct {
> >> +  EFI_PHYSICAL_ADDRESS            AllocAddress;
> >> +  VOID                            *HostAddress;
> >> +  EFI_PCI_IO_PROTOCOL_OPERATION   Operation;
> >> +  UINTN                           NumberOfBytes;
> >> +} PLATFORM_PCI_IO_MAP_INFO;
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoPollMem (
> >> +  IN  EFI_PCI_IO_PROTOCOL          *This,
> >> +  IN  EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> >> +  IN  UINT8                        BarIndex,
> >> +  IN  UINT64                       Offset,
> >> +  IN  UINT64                       Mask,
> >> +  IN  UINT64                       Value,
> >> +  IN  UINT64                       Delay,
> >> +  OUT UINT64                       *Result
> >> +  )
> >> +{
> >> +  ASSERT (FALSE);
> >> +  return EFI_UNSUPPORTED;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoPollIo (
> >> +  IN  EFI_PCI_IO_PROTOCOL          *This,
> >> +  IN  EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> >> +  IN  UINT8                        BarIndex,
> >> +  IN  UINT64                       Offset,
> >> +  IN  UINT64                       Mask,
> >> +  IN  UINT64                       Value,
> >> +  IN  UINT64                       Delay,
> >> +  OUT UINT64                       *Result
> >> +  )
> >> +{
> >> +  ASSERT (FALSE);
> >> +  return EFI_UNSUPPORTED;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoMemRW (
> >> +  IN    EFI_PCI_IO_PROTOCOL_WIDTH Width,
> >> +  IN    UINTN                     Count,
> >> +  IN    UINTN                     DstStride,
> >> +  IN    VOID                      *Dst,
> >> +  IN    UINTN                     SrcStride,
> >> +  OUT   CONST VOID                *Src
> >> +  )
> >> +{
> >> +  UINT8         *Dst8;
> >> +  UINT16        *Dst16;
> >> +  UINT32        *Dst32;
> >> +  CONST UINT8   *Src8;
> >> +  CONST UINT16  *Src16;
> >> +  CONST UINT32  *Src32;
> >
> > Do any or all of these need to be volatile to ensure retaining access
> > order and size (and if not, could we call that out explicitly)?
> >
> 
> Good question. But perhaps it is better to put a MemoryFence() at the
> end of each loop iteration?

That wouldn't prevent repeated reads from the same location, or
reads done as a larger element. It would make either very unlikely,
but it wouldn't prevent.

> >> + >> +  //
> >> +  // Loop for each iteration and move the data
> >> +  //
> >> +  switch (Width & 0x3) {
> >> +  case EfiPciWidthUint8:
> >> +    Dst8 = (UINT8 *)Dst;
> >> +    Src8 = (UINT8 *)Src;
> >> +    for (;Count > 0; Count--, Dst8 += DstStride, Src8 += SrcStride) {
> >> +      *Dst8 = *Src8;
> >> +    }
> >> +    break;
> >> +  case EfiPciWidthUint16:
> >> +    Dst16 = (UINT16 *)Dst;
> >> +    Src16 = (UINT16 *)Src;
> >> +    for (;Count > 0; Count--, Dst16 += DstStride, Src16 += SrcStride) {
> >> +      *Dst16 = *Src16;
> >> +    }
> >> +    break;
> >> +  case EfiPciWidthUint32:
> >> +    Dst32 = (UINT32 *)Dst;
> >> +    Src32 = (UINT32 *)Src;
> >> +    for (;Count > 0; Count--, Dst32 += DstStride, Src32 += SrcStride) {
> >> +      *Dst32 = *Src32;
> >> +    }
> >> +    break;
> >> +  default:
> >> +    return EFI_INVALID_PARAMETER;
> >> +  }
> >> +
> >> +  return EFI_SUCCESS;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoMemRead (
> >> +  IN     EFI_PCI_IO_PROTOCOL          *This,
> >> +  IN     EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> >> +  IN     UINT8                        BarIndex,
> >> +  IN     UINT64                       Offset,
> >> +  IN     UINTN                        Count,
> >> +  IN OUT VOID                         *Buffer
> >> +  )
> >> +{
> >> +  PLATFORM_PCI_IO_DEV   *Dev;
> >> +  UINTN                 AlignMask;
> >> +  VOID                  *Address;
> >> +
> >> +  if (Buffer == NULL) {
> >> +    return EFI_INVALID_PARAMETER;
> >> +  }
> >> +
> >> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO(This);
> >> +
> >> +  //
> >> +  // Only allow accesses to the single BAR we emulate
> >> +  //
> >> +  if (BarIndex != Dev->BarIndex || Offset >= Dev->BarSize) {
> >> +    return EFI_UNSUPPORTED;
> >> +  }
> >> +
> >> +  Address = (VOID*)(UINTN)(Dev->ConfigSpace.Device.Bar[BarIndex] + Offset);
> >> +  AlignMask = (1 << (Width & 0x03)) - 1;
> >> +  if ((UINTN)Address & AlignMask) {
> >> +    return EFI_INVALID_PARAMETER;
> >> +  }
> >> +
> >> +  switch (Width) {
> >> +  case EfiPciWidthUint8:
> >> +  case EfiPciWidthUint16:
> >> +  case EfiPciWidthUint32:
> >> +  case EfiPciWidthUint64:
> >> +    return PciIoMemRW (Width, Count, 1, Buffer, 1, Address);
> >> +
> >> +  case EfiPciWidthFifoUint8:
> >> +  case EfiPciWidthFifoUint16:
> >> +  case EfiPciWidthFifoUint32:
> >> +  case EfiPciWidthFifoUint64:
> >> +    return PciIoMemRW (Width, Count, 1, Buffer, 0, Address);
> >> +
> >> +  case EfiPciWidthFillUint8:
> >> +  case EfiPciWidthFillUint16:
> >> +  case EfiPciWidthFillUint32:
> >> +  case EfiPciWidthFillUint64:
> >> +    return PciIoMemRW (Width, Count, 0, Buffer, 1, Address);
> >> +
> >> +  default:
> >> +    break;
> >> +  }
> >> +  return EFI_INVALID_PARAMETER;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoMemWrite (
> >> +  IN     EFI_PCI_IO_PROTOCOL          *This,
> >> +  IN     EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> >> +  IN     UINT8                        BarIndex,
> >> +  IN     UINT64                       Offset,
> >> +  IN     UINTN                        Count,
> >> +  IN OUT VOID                         *Buffer
> >> +  )
> >> +{
> >> +  PLATFORM_PCI_IO_DEV   *Dev;
> >> +  UINTN                 AlignMask;
> >> +  VOID                  *Address;
> >> +
> >> +  if (Buffer == NULL) {
> >> +    return EFI_INVALID_PARAMETER;
> >> +  }
> >> +
> >> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO(This);
> >> +
> >> +  //
> >> +  // Only allow accesses to the single BAR we emulate
> >> +  //
> >> +  if (BarIndex != Dev->BarIndex || Offset >= Dev->BarSize) {
> >> +    return EFI_UNSUPPORTED;
> >> +  }
> >> +
> >> +  Address = (VOID*)(UINTN)(Dev->ConfigSpace.Device.Bar[BarIndex] + Offset);
> >> +  AlignMask = (1 << (Width & 0x03)) - 1;
> >> +  if ((UINTN)Address & AlignMask) {
> >> +    return EFI_INVALID_PARAMETER;
> >> +  }
> >> +
> >> +  switch (Width) {
> >> +  case EfiPciWidthUint8:
> >> +  case EfiPciWidthUint16:
> >> +  case EfiPciWidthUint32:
> >> +  case EfiPciWidthUint64:
> >> +    return PciIoMemRW (Width, Count, 1, Address, 1, Buffer);
> >> +
> >> +  case EfiPciWidthFifoUint8:
> >> +  case EfiPciWidthFifoUint16:
> >> +  case EfiPciWidthFifoUint32:
> >> +  case EfiPciWidthFifoUint64:
> >> +    return PciIoMemRW (Width, Count, 0, Address, 1, Buffer);
> >> +
> >> +  case EfiPciWidthFillUint8:
> >> +  case EfiPciWidthFillUint16:
> >> +  case EfiPciWidthFillUint32:
> >> +  case EfiPciWidthFillUint64:
> >> +    return PciIoMemRW (Width, Count, 1, Address, 0, Buffer);
> >> +
> >> +  default:
> >> +    break;
> >> +  }
> >> +  return EFI_INVALID_PARAMETER;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoIoRead (
> >> +  IN EFI_PCI_IO_PROTOCOL              *This,
> >> +  IN     EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> >> +  IN     UINT8                        BarIndex,
> >> +  IN     UINT64                       Offset,
> >> +  IN     UINTN                        Count,
> >> +  IN OUT VOID                         *Buffer
> >> +  )
> >> +{
> >> +  ASSERT (FALSE);
> >> +  return EFI_UNSUPPORTED;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoIoWrite (
> >> +  IN EFI_PCI_IO_PROTOCOL              *This,
> >> +  IN     EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> >> +  IN     UINT8                        BarIndex,
> >> +  IN     UINT64                       Offset,
> >> +  IN     UINTN                        Count,
> >> +  IN OUT VOID                         *Buffer
> >> +  )
> >> +{
> >> +  ASSERT (FALSE);
> >> +  return EFI_UNSUPPORTED;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoPciRead (
> >> +  IN     EFI_PCI_IO_PROTOCOL       *This,
> >> +  IN     EFI_PCI_IO_PROTOCOL_WIDTH  Width,
> >> +  IN     UINT32                     Offset,
> >> +  IN     UINTN                      Count,
> >> +  IN OUT VOID                      *Buffer
> >> +  )
> >> +{
> >> +  PLATFORM_PCI_IO_DEV *Dev;
> >> +  VOID                *Address;
> >> +
> >> +  if ((Width < 0) || (Width >= EfiPciIoWidthMaximum) || (Buffer == NULL)) {
> >> +    return EFI_INVALID_PARAMETER;
> >> +  }
> >> +
> >> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO(This);
> >> +
> >> +  Address = (UINT8 *)&Dev->ConfigSpace + Offset;
> >> +
> >> +  return PciIoMemRW (Width, Count, 1, Buffer, 1, Address);
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoPciWrite (
> >> +  IN EFI_PCI_IO_PROTOCOL              *This,
> >> +  IN     EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> >> +  IN     UINT32                       Offset,
> >> +  IN     UINTN                        Count,
> >> +  IN OUT VOID                         *Buffer
> >> +  )
> >> +{
> >> +  PLATFORM_PCI_IO_DEV *Dev;
> >> +  VOID                *Address;
> >> +
> >> +  if ((Width < 0) || (Width >= EfiPciIoWidthMaximum) || (Buffer == NULL)) {
> >> +    return EFI_INVALID_PARAMETER;
> >> +  }
> >> +
> >> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO(This);
> >> +
> >> +  Address = (UINT8 *)&Dev->ConfigSpace + Offset;
> >> +
> >> +  return PciIoMemRW (Width, Count, 1, Address, 1, Buffer);
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoCopyMem (
> >> +  IN EFI_PCI_IO_PROTOCOL              *This,
> >> +  IN     EFI_PCI_IO_PROTOCOL_WIDTH    Width,
> >> +  IN     UINT8                        DestBarIndex,
> >> +  IN     UINT64                       DestOffset,
> >> +  IN     UINT8                        SrcBarIndex,
> >> +  IN     UINT64                       SrcOffset,
> >> +  IN     UINTN                        Count
> >> +  )
> >> +{
> >> +  ASSERT (FALSE);
> >> +  return EFI_UNSUPPORTED;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +CoherentPciIoMap (
> >> +  IN     EFI_PCI_IO_PROTOCOL            *This,
> >> +  IN     EFI_PCI_IO_PROTOCOL_OPERATION  Operation,
> >> +  IN     VOID                           *HostAddress,
> >> +  IN OUT UINTN                          *NumberOfBytes,
> >> +  OUT    EFI_PHYSICAL_ADDRESS           *DeviceAddress,
> >> +  OUT    VOID                           **Mapping
> >> +  )
> >> +{
> >> +  PLATFORM_PCI_IO_DEV       *Dev;
> >> +  EFI_STATUS                Status;
> >> +  PLATFORM_PCI_IO_MAP_INFO  *MapInfo;
> >> +
> >> +  //
> >> +  // If this device does not support 64-bit DMA addressing, we need to allocate
> >> +  // a bounce buffer and copy over the data if HostAddress is above 4 GB.
> >> +  //
> >> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO(This);
> >> +  if ((Dev->Attributes & EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0 &&
> >> +      (UINTN) HostAddress >= SIZE_4GB) {
> >> +
> >> +    //
> >> +    // Bounce buffering is not possible for consistent mappings
> >> +    //
> >> +    if (Operation == EfiPciIoOperationBusMasterCommonBuffer) {
> >> +      return EFI_UNSUPPORTED;
> >> +    }
> >> +
> >> +    MapInfo = (PLATFORM_PCI_IO_MAP_INFO *)AllocatePool (sizeof *MapInfo);
> >> +    if (MapInfo == NULL) {
> >> +      return EFI_OUT_OF_RESOURCES;
> >> +    }
> >> +
> >> +    MapInfo->AllocAddress = SIZE_4GB - 1;
> >> +    MapInfo->HostAddress = HostAddress;
> >> +    MapInfo->Operation = Operation;
> >> +    MapInfo->NumberOfBytes = *NumberOfBytes;
> >> +
> >> +    Status = gBS->AllocatePages (AllocateMaxAddress, EfiBootServicesData,
> >> +                    EFI_SIZE_TO_PAGES (MapInfo->NumberOfBytes),
> >> +                    &MapInfo->AllocAddress);
> >> +    if (EFI_ERROR (Status)) {
> >
> > Comment on how this can mean the system does not have RAM < 4GB and
> > cannot support this device?
> >
> 
> Sure
> 
> >> +      FreePool (MapInfo);
> >> +      return Status;
> >> +    }
> >> +    if (Operation == EfiPciIoOperationBusMasterRead) {
> >> +      gBS->CopyMem ((VOID *)MapInfo->AllocAddress, HostAddress, *NumberOfBytes);
> >> +    }
> >> +    *DeviceAddress = MapInfo->AllocAddress;
> >> +    *Mapping = MapInfo;
> >> +  } else {
> >> +    *DeviceAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)HostAddress;
> >> +    *Mapping = NULL;
> >> +  }
> >> +  return EFI_SUCCESS;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +CoherentPciIoUnmap (
> >> +  IN  EFI_PCI_IO_PROTOCOL          *This,
> >> +  IN  VOID                         *Mapping
> >> +  )
> >> +{
> >> +  PLATFORM_PCI_IO_MAP_INFO  *MapInfo;
> >> +
> >> +  MapInfo = Mapping;
> >> +  if (MapInfo != NULL) {
> >> +    if (MapInfo->Operation == EfiPciIoOperationBusMasterWrite) {
> >> +      gBS->CopyMem (MapInfo->HostAddress, (VOID *)MapInfo->AllocAddress,
> >> +             MapInfo->NumberOfBytes);
> >> +    }
> >> +    gBS->FreePages (MapInfo->AllocAddress,
> >> +           EFI_SIZE_TO_PAGES (MapInfo->NumberOfBytes));
> >> +    FreePool (MapInfo);
> >> +  }
> >> +  return EFI_SUCCESS;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +CoherentPciIoAllocateBuffer (
> >> +  IN  EFI_PCI_IO_PROTOCOL         *This,
> >> +  IN  EFI_ALLOCATE_TYPE           Type,
> >> +  IN  EFI_MEMORY_TYPE             MemoryType,
> >> +  IN  UINTN                       Pages,
> >> +  OUT VOID                        **HostAddress,
> >> +  IN  UINT64                      Attributes
> >> +  )
> >> +{
> >> +  PLATFORM_PCI_IO_DEV       *Dev;
> >> +  EFI_PHYSICAL_ADDRESS      AllocAddress;
> >> +  EFI_ALLOCATE_TYPE         AllocType;
> >> +  EFI_STATUS                Status;
> >> +
> >> +  if (Attributes &
> >> +      (~(EFI_PCI_ATTRIBUTE_MEMORY_WRITE_COMBINE |
> >> +         EFI_PCI_ATTRIBUTE_MEMORY_CACHED         ))) {
> >
> > Is that indentation before ))) intentional?
> >
> 
> That was simply copied from the Omap35xxPkg PciEmulation code. I will
> clean that up
> 
> 
> >> +    return EFI_UNSUPPORTED;
> >> +  }
> >> +
> >> +  //
> >> +  // Allocate below 4 GB if the dual address cycle attribute has not
> >> +  // been set.
> >
> > Comment on incompatibility if no RAM below 4 GB?
> >
> 
> Ack
> 
> >> +  //
> >> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO(This);
> >> +  if ((Dev->Attributes & EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0) {
> >> +    AllocAddress = SIZE_4GB - 1;
> >> +    AllocType = AllocateMaxAddress;
> >> +  } else {
> >> +    AllocType = AllocateAnyPages;
> >> +  }
> >> +
> >> +  Status = gBS->AllocatePages (AllocType, MemoryType, Pages, &AllocAddress);
> >> +  if (!EFI_ERROR (Status)) {
> >> +    *HostAddress = (VOID *)(UINTN)AllocAddress;
> >> +  }
> >> +  return Status;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +CoherentPciIoFreeBuffer (
> >> +  IN  EFI_PCI_IO_PROTOCOL         *This,
> >> +  IN  UINTN                       Pages,
> >> +  IN  VOID                        *HostAddress
> >> +  )
> >> +{
> >> +  FreePages (HostAddress, Pages);
> >> +  return EFI_SUCCESS;
> >> +}
> >> +
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoFlush (
> >> +  IN EFI_PCI_IO_PROTOCOL          *This
> >> +  )
> >> +{
> >> +  return EFI_SUCCESS;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoGetLocation (
> >> +  IN   EFI_PCI_IO_PROTOCOL  *This,
> >> +  OUT  UINTN                *SegmentNumber,
> >> +  OUT  UINTN                *BusNumber,
> >> +  OUT  UINTN                *DeviceNumber,
> >> +  OUT  UINTN                *FunctionNumber
> >> +  )
> >> +{
> >> +  if ((SegmentNumber == NULL) || (BusNumber      == NULL) ||
> >> +      (DeviceNumber  == NULL) || (FunctionNumber == NULL)    ) {
> >> +    return EFI_INVALID_PARAMETER;
> >
> > Indentation before ) intentional?
> >
> 
> Same here
> 
> >> +  }
> >> +
> >> +  *SegmentNumber  = 0;
> >> +  *BusNumber      = 0xff;
> >> +  *DeviceNumber   = 0;
> >> +  *FunctionNumber = 0;
> >> +
> >> +  return EFI_SUCCESS;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoAttributes (
> >> +  IN  EFI_PCI_IO_PROTOCOL                      *This,
> >> +  IN  EFI_PCI_IO_PROTOCOL_ATTRIBUTE_OPERATION  Operation,
> >> +  IN  UINT64                                   Attributes,
> >> +  OUT UINT64                                   *Result OPTIONAL
> >> +  )
> >> +{
> >> +  PLATFORM_PCI_IO_DEV *Dev;
> >> +  BOOLEAN             Enable;
> >> +
> >> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO(This);
> >> +
> >> +  Enable = FALSE;
> >> +  switch (Operation) {
> >> +  case EfiPciIoAttributeOperationGet:
> >> +    if (Result == NULL) {
> >> +      return EFI_INVALID_PARAMETER;
> >> +    }
> >> +    *Result = Dev->Attributes;
> >> +    break;
> >> +
> >> +  case EfiPciIoAttributeOperationSupported:
> >> +    if (Result == NULL) {
> >> +      return EFI_INVALID_PARAMETER;
> >> +    }
> >> +    *Result = EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE;
> >> +    break;
> >> +
> >> +  case EfiPciIoAttributeOperationEnable:
> >> +    Attributes |= Dev->Attributes;
> >> +  case EfiPciIoAttributeOperationSet:
> >> +    Enable = ((~Dev->Attributes & Attributes) & EFI_PCI_DEVICE_ENABLE) != 0;
> >> +    Dev->Attributes = Attributes;
> >> +    break;
> >> +
> >> +  case EfiPciIoAttributeOperationDisable:
> >> +    Dev->Attributes &= ~Attributes;
> >> +    break;
> >> +
> >> +  default:
> >> +    return EFI_INVALID_PARAMETER;
> >> +  };
> >> +
> >> +  //
> >> +  // If we're setting any of the EFI_PCI_DEVICE_ENABLE bits, perform
> >> +  // the platform device specific initialization now.
> >> +  //
> >> +  if (Enable && !Dev->Enabled && Dev->PlatformPciIo->Initialize != NULL) {
> >> +    Dev->PlatformPciIo->Initialize (Dev->PlatformPciIo);
> >> +    Dev->Enabled = TRUE;
> >> +  }
> >> +  return EFI_SUCCESS;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoGetBarAttributes (
> >> +  IN EFI_PCI_IO_PROTOCOL             *This,
> >> +  IN  UINT8                          BarIndex,
> >> +  OUT UINT64                         *Supports, OPTIONAL
> >> +  OUT VOID                           **Resources OPTIONAL
> >> +  )
> >> +{
> >> +  ASSERT (FALSE);
> >> +  return EFI_UNSUPPORTED;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +PciIoSetBarAttributes (
> >> +  IN EFI_PCI_IO_PROTOCOL              *This,
> >> +  IN     UINT64                       Attributes,
> >> +  IN     UINT8                        BarIndex,
> >> +  IN OUT UINT64                       *Offset,
> >> +  IN OUT UINT64                       *Length
> >> +  )
> >> +{
> >> +  ASSERT (FALSE);
> >> +  return EFI_UNSUPPORTED;
> >> +}
> >> +
> >> +STATIC CONST EFI_PCI_IO_PROTOCOL PciIoTemplate =
> >> +{
> >> +  PciIoPollMem,
> >> +  PciIoPollIo,
> >> +  { PciIoMemRead, PciIoMemWrite },
> >> +  { PciIoIoRead,  PciIoIoWrite },
> >> +  { PciIoPciRead, PciIoPciWrite },
> >> +  PciIoCopyMem,
> >> +  CoherentPciIoMap,
> >> +  CoherentPciIoUnmap,
> >> +  CoherentPciIoAllocateBuffer,
> >> +  CoherentPciIoFreeBuffer,
> >> +  PciIoFlush,
> >> +  PciIoGetLocation,
> >> +  PciIoAttributes,
> >> +  PciIoGetBarAttributes,
> >> +  PciIoSetBarAttributes,
> >> +  0,
> >> +  0
> >> +};
> >> +
> >> +VOID
> >> +InitializePciIoProtocol (
> >> +  PLATFORM_PCI_IO_DEV     *PlatformPciIoDev
> >> +  )
> >> +{
> >> +  PlatformPciIoDev->ConfigSpace.Hdr.VendorId = 0xFFFF;    // no vendor
> >> +  PlatformPciIoDev->ConfigSpace.Hdr.DeviceId = 0x0000;    // device id ignored
> >
> > Are these IDs noncontroversial?
> > Would it be preferable to allocate some real ones?
> >
> 
> They are unallocated, and unpopulated PCI config space reads back as
> all zeroes, so 0xFFFF is the least likely to ever become allocated.
> Device ID is scoped by vendor ID, so it's a don't care in any case.
> 
> On top of that, the whole point of this driver is to match on class
> codes, so we are by definition not interested in attaching drivers
> that look for a particular Vendor/Device ID

I don't disagree with any of what you're saying - I'd just like some
input from someone who feels competent to comment. If 0xFFFF is
acceptable to everyone, it coud make sense to have a PCI_VENDOR_NONE
define.

> >> +
> >> +  switch (PlatformPciIoDev->PlatformPciIo->DeviceType) {
> >> +  case PlatformPciIoDeviceOhci:
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_OHCI;
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB;
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL;
> >> +    PlatformPciIoDev->BarSize = SIZE_64KB;
> >
> > Is the 64KB mainly for SBSA compliance, or is it implicit for OHCI?
> >
> 
> Copy/paste from Juno. No clue tbh

Ah :)

> >> +    break;
> >> +
> >> +  case PlatformPciIoDeviceUhci:
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_UHCI;
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB;
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL;
> >> +    PlatformPciIoDev->BarSize = SIZE_64KB;
> >> +    break;
> >> +
> >> +  case PlatformPciIoDeviceEhci:
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_EHCI;
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB;
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL;
> >> +    PlatformPciIoDev->BarSize = SIZE_64KB;
> >> +    break;
> >> +
> >> +  case PlatformPciIoDeviceXhci:
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_XHCI;
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_SERIAL_USB;
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_SERIAL;
> >> +    PlatformPciIoDev->BarIndex = 0;
> >> +    PlatformPciIoDev->BarSize = SIZE_64KB;
> >> +    break;
> >> +
> >> +  case PlatformPciIoDeviceAhci:
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[0] = PCI_IF_MASS_STORAGE_AHCI;
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[1] = PCI_CLASS_MASS_STORAGE_SATADPA;
> >> +    PlatformPciIoDev->ConfigSpace.Hdr.ClassCode[2] = PCI_CLASS_MASS_STORAGE;
> >> +    PlatformPciIoDev->BarIndex = 5;
> >> +    PlatformPciIoDev->BarSize = SIZE_1KB;
> >> +    break;
> >> +
> >
> > And as a follow-on to comment on previous patch: SdMmc and Ufs?
> >
> 
> Indeed. Added in v2

Thanks!

> 
> >> +  default:
> >> +    ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> >> +  }
> >> +
> >> +  PlatformPciIoDev->ConfigSpace.Device.Bar[PlatformPciIoDev->BarIndex] =
> >> +                                        PlatformPciIoDev->PlatformPciIo->BaseAddress;
> >> +
> >> +  // Copy protocol structure
> >> +  CopyMem(&PlatformPciIoDev->PciIo, &PciIoTemplate, sizeof PciIoTemplate);
> >> +}
> >> diff --git a/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.h b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.h
> >> new file mode 100644
> >> index 000000000000..8fd8dc5e4a11
> >> --- /dev/null
> >> +++ b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIo.h
> >> @@ -0,0 +1,67 @@
> >> +/** @file
> >> +
> >> +  Copyright (C) 2016, Linaro Ltd. All rights reserved.<BR>
> >> +
> >> +  This program and the accompanying materials are licensed and made available
> >> +  under the terms and conditions of the BSD License which accompanies this
> >> +  distribution. The full text of the license may be found at
> >> +  http://opensource.org/licenses/bsd-license.php
> >> +
> >> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> >> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> >> +
> >> +**/
> >> +
> >> +#include <Library/BaseMemoryLib.h>
> >> +#include <Library/DebugLib.h>
> >> +#include <Library/MemoryAllocationLib.h>
> >> +#include <Library/UefiBootServicesTableLib.h>
> >> +#include <Library/UefiLib.h>
> >> +
> >> +#include <IndustryStandard/Pci.h>
> >> +
> >> +#include <Protocol/PciIo.h>
> >> +#include <Protocol/PlatformPciIo.h>
> >> +
> >> +#define PLATFORM_PCI_IO_SIG SIGNATURE_32 ('P', 'P', 'I', 'D')
> >> +
> >> +#define PLATFORM_PCI_IO_DEV_FROM_PCI_IO(PciIoPointer) \
> >> +          CR (PciIoPointer, PLATFORM_PCI_IO_DEV, PciIo, PLATFORM_PCI_IO_SIG)
> >> +
> >> +typedef struct {
> >> +  UINT32                  Signature;
> >> +  //
> >> +  // The bound platform PCI I/O protocol instance
> >> +  //
> >> +  PLATFORM_PCI_IO         *PlatformPciIo;
> >> +  //
> >> +  // The exposed PCI I/O protocol instance.
> >> +  //
> >> +  EFI_PCI_IO_PROTOCOL     PciIo;
> >> +  //
> >> +  // The emulated PCI config space of the device. Only the minimally required
> >> +  // items are assigned.
> >> +  //
> >> +  PCI_TYPE00              ConfigSpace;
> >> +  //
> >> +  // The BAR index which exposes the MMIO control region of the device
> >> +  //
> >> +  UINTN                   BarIndex;
> >> +  //
> >> +  // The size of the MMIO control region of the device
> >> +  //
> >> +  UINTN                   BarSize;
> >> +  //
> >> +  // The PCI I/O attributes for this device
> >> +  //
> >> +  UINT64                  Attributes;
> >> +  //
> >> +  // Whether this device has been enabled
> >> +  //
> >> +  BOOLEAN                 Enabled;
> >> +} PLATFORM_PCI_IO_DEV;
> >> +
> >> +VOID
> >> +InitializePciIoProtocol (
> >> +  PLATFORM_PCI_IO_DEV     *PlatformPciIoDev
> >> +  );
> >> diff --git a/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.c b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.c
> >> new file mode 100644
> >> index 000000000000..7f3306e7e891
> >> --- /dev/null
> >> +++ b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.c
> >> @@ -0,0 +1,268 @@
> >> +/** @file
> >> +
> >> +  Copyright (C) 2016, Linaro Ltd. All rights reserved.<BR>
> >> +
> >> +  This program and the accompanying materials are licensed and made available
> >> +  under the terms and conditions of the BSD License which accompanies this
> >> +  distribution. The full text of the license may be found at
> >> +  http://opensource.org/licenses/bsd-license.php
> >> +
> >> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> >> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> >> +
> >> +**/
> >> +
> >> +#include "PlatformPciIo.h"
> >> +
> >> +#include <Protocol/ComponentName.h>
> >> +#include <Protocol/DriverBinding.h>
> >> +
> >> +//
> >> +// Probe, start and stop functions of this driver, called by the DXE core for
> >> +// specific devices.
> >> +//
> >> +// The following specifications document these interfaces:
> >> +// - Driver Writer's Guide for UEFI 2.3.1 v1.01, 9 Driver Binding Protocol
> >> +// - UEFI Spec 2.3.1 + Errata C, 10.1 EFI Driver Binding Protocol
> >> +//
> >> +// The implementation follows:
> >> +// - Driver Writer's Guide for UEFI 2.3.1 v1.01
> >> +//   - 5.1.3.4 OpenProtocol() and CloseProtocol()
> >> +// - UEFI Spec 2.3.1 + Errata C
> >> +//   -  6.3 Protocol Handler Services
> >> +//
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +EFIAPI
> >> +PlatformPciIoDriverBindingSupported (
> >> +  IN EFI_DRIVER_BINDING_PROTOCOL *This,
> >> +  IN EFI_HANDLE                  DeviceHandle,
> >> +  IN EFI_DEVICE_PATH_PROTOCOL    *RemainingDevicePath
> >> +  )
> >> +{
> >> +  PLATFORM_PCI_IO       *PlatformPciIo;
> >> +  EFI_STATUS            Status;
> >> +
> >> +  Status = gBS->OpenProtocol (DeviceHandle, &gPlatformPciIoProtocolGuid,
> >> +                  (VOID **)&PlatformPciIo, This->DriverBindingHandle,
> >> +                  DeviceHandle, EFI_OPEN_PROTOCOL_BY_DRIVER);
> >> +  if (EFI_ERROR (Status)) {
> >> +    return Status;
> >> +  }
> >> +
> >> +  //
> >> +  // We only support the following device types
> >> +  //
> >> +  switch (PlatformPciIo->DeviceType) {
> >> +  case PlatformPciIoDeviceOhci:
> >> +  case PlatformPciIoDeviceUhci:
> >> +  case PlatformPciIoDeviceEhci:
> >> +  case PlatformPciIoDeviceXhci:
> >> +  case PlatformPciIoDeviceAhci:
> >
> > SdMmc, Ufs?
> >
> 
> Yeah yeah
> 
> >> +    //
> >> +    // Restricted to DMA coherent for now
> >> +    //
> >> +    if (PlatformPciIo->DmaType == PlatformPciIoDmaCoherent) {
> >> +      Status = EFI_SUCCESS;
> >> +      break;
> >> +    }
> >> +  default:
> >> +    Status = EFI_UNSUPPORTED;
> >> +  }
> >> +
> >> +  gBS->CloseProtocol (DeviceHandle, &gPlatformPciIoProtocolGuid,
> >> +         This->DriverBindingHandle, DeviceHandle);
> >> +
> >> +  return Status;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +EFIAPI
> >> +PlatformPciIoDriverBindingStart (
> >> +  IN EFI_DRIVER_BINDING_PROTOCOL *This,
> >> +  IN EFI_HANDLE                  DeviceHandle,
> >> +  IN EFI_DEVICE_PATH_PROTOCOL    *RemainingDevicePath
> >> +  )
> >> +{
> >> +  PLATFORM_PCI_IO_DEV   *Dev;
> >> +  EFI_STATUS            Status;
> >> +
> >> +  Dev = (PLATFORM_PCI_IO_DEV *) AllocateZeroPool (sizeof *Dev);
> >
> > You usually don't put a space after the cast.
> >
> 
> Ack
> 
> >> +  if (Dev == NULL) {
> >> +    return EFI_OUT_OF_RESOURCES;
> >> +  }
> >> +
> >> +  Status = gBS->OpenProtocol (DeviceHandle, &gPlatformPciIoProtocolGuid,
> >> +                  (VOID **)&Dev->PlatformPciIo, This->DriverBindingHandle,
> >> +                  DeviceHandle, EFI_OPEN_PROTOCOL_BY_DRIVER);
> >> +  if (EFI_ERROR (Status)) {
> >> +    goto FreeDev;
> >> +  }
> >> +
> >> +  InitializePciIoProtocol (Dev);
> >> +
> >> +  //
> >> +  // Setup complete, attempt to export the driver instance's EFI_PCI_IO_PROTOCOL
> >> +  // interface.
> >> +  //
> >> +  Dev->Signature = PLATFORM_PCI_IO_SIG;
> >> +  Status = gBS->InstallProtocolInterface (&DeviceHandle, &gEfiPciIoProtocolGuid,
> >> +                  EFI_NATIVE_INTERFACE, &Dev->PciIo);
> >> +  if (EFI_ERROR (Status)) {
> >> +    goto CloseProtocol;
> >> +  }
> >> +
> >> +  return EFI_SUCCESS;
> >> +
> >> +CloseProtocol:
> >> +  gBS->CloseProtocol (DeviceHandle, &gPlatformPciIoProtocolGuid,
> >> +         This->DriverBindingHandle, DeviceHandle);
> >> +
> >> +FreeDev:
> >> +  FreePool (Dev);
> >> +
> >> +  return Status;
> >> +}
> >> +
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +EFIAPI
> >> +PlatformPciIoDriverBindingStop (
> >> +  IN EFI_DRIVER_BINDING_PROTOCOL *This,
> >> +  IN EFI_HANDLE                  DeviceHandle,
> >> +  IN UINTN                       NumberOfChildren,
> >> +  IN EFI_HANDLE                  *ChildHandleBuffer
> >> +  )
> >> +{
> >> +  EFI_STATUS                      Status;
> >> +  EFI_PCI_IO_PROTOCOL             *PciIo;
> >> +  PLATFORM_PCI_IO_DEV             *Dev;
> >> +
> >> +  Status = gBS->OpenProtocol (DeviceHandle, &gEfiPciIoProtocolGuid,
> >> +                  (VOID **)&PciIo, This->DriverBindingHandle, DeviceHandle,
> >> +                  EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> >> +  if (EFI_ERROR (Status)) {
> >> +    return Status;
> >> +  }
> >> +
> >> +  Dev = PLATFORM_PCI_IO_DEV_FROM_PCI_IO (PciIo);
> >> +
> >> +  //
> >> +  // Handle Stop() requests for in-use driver instances gracefully.
> >> +  //
> >> +  Status = gBS->UninstallProtocolInterface (DeviceHandle,
> >> +                  &gEfiPciIoProtocolGuid, &Dev->PciIo);
> >> +  if (EFI_ERROR (Status)) {
> >> +    return Status;
> >> +  }
> >> +
> >> +  gBS->CloseProtocol (DeviceHandle, &gPlatformPciIoProtocolGuid,
> >> +         This->DriverBindingHandle, DeviceHandle);
> >> +
> >> +  FreePool (Dev);
> >> +
> >> +  return EFI_SUCCESS;
> >> +}
> >> +
> >> +
> >> +//
> >> +// The static object that groups the Supported() (ie. probe), Start() and
> >> +// Stop() functions of the driver together. Refer to UEFI Spec 2.3.1 + Errata
> >> +// C, 10.1 EFI Driver Binding Protocol.
> >> +//
> >> +STATIC EFI_DRIVER_BINDING_PROTOCOL gDriverBinding = {
> >> +  &PlatformPciIoDriverBindingSupported,
> >> +  &PlatformPciIoDriverBindingStart,
> >> +  &PlatformPciIoDriverBindingStop,
> >> +  0x10, // Version, must be in [0x10 .. 0xFFFFFFEF] for IHV-developed drivers
> >> +  NULL,
> >> +  NULL
> >> +};
> >> +
> >> +
> >> +//
> >> +// The purpose of the following scaffolding (EFI_COMPONENT_NAME_PROTOCOL and
> >> +// EFI_COMPONENT_NAME2_PROTOCOL implementation) is to format the driver's name
> >> +// in English, for display on standard console devices. This is recommended for
> >> +// UEFI drivers that follow the UEFI Driver Model. Refer to the Driver Writer's
> >> +// Guide for UEFI 2.3.1 v1.01, 11 UEFI Driver and Controller Names.
> >> +//
> >
> > Should the strings be split out in order to permit proper unicode
> > strings without polluting the C file? As this will be a core library,
> > I would quite like to see translations to all kinds of languages.
> >
> 
> Do you mean a separate ComponentName.c like other drivers?

Yeah.

Regards,

Leif

> >> +
> >> +STATIC
> >> +EFI_UNICODE_STRING_TABLE mDriverNameTable[] = {
> >> +  { "eng;en", L"PCI I/O protocol emulation driver for platform devices" },
> >> +  { NULL,     NULL                   }
> >> +};
> >> +
> >> +STATIC
> >> +EFI_COMPONENT_NAME_PROTOCOL gComponentName;
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +EFIAPI
> >> +PlatformPciIoGetDriverName (
> >> +  IN  EFI_COMPONENT_NAME_PROTOCOL *This,
> >> +  IN  CHAR8                       *Language,
> >> +  OUT CHAR16                      **DriverName
> >> +  )
> >> +{
> >> +  return LookupUnicodeString2 (
> >> +           Language,
> >> +           This->SupportedLanguages,
> >> +           mDriverNameTable,
> >> +           DriverName,
> >> +           (BOOLEAN)(This == &gComponentName) // Iso639Language
> >> +           );
> >> +}
> >> +
> >> +STATIC
> >> +EFI_STATUS
> >> +EFIAPI
> >> +PlatformPciIoGetDeviceName (
> >> +  IN  EFI_COMPONENT_NAME_PROTOCOL *This,
> >> +  IN  EFI_HANDLE                  DeviceHandle,
> >> +  IN  EFI_HANDLE                  ChildHandle,
> >> +  IN  CHAR8                       *Language,
> >> +  OUT CHAR16                      **ControllerName
> >> +  )
> >> +{
> >> +  return EFI_UNSUPPORTED;
> >> +}
> >> +
> >> +STATIC
> >> +EFI_COMPONENT_NAME_PROTOCOL gComponentName = {
> >> +  &PlatformPciIoGetDriverName,
> >> +  &PlatformPciIoGetDeviceName,
> >> +  "eng" // SupportedLanguages, ISO 639-2 language codes
> >> +};
> >> +
> >> +STATIC
> >> +EFI_COMPONENT_NAME2_PROTOCOL gComponentName2 = {
> >> +  (EFI_COMPONENT_NAME2_GET_DRIVER_NAME)     &PlatformPciIoGetDriverName,
> >> +  (EFI_COMPONENT_NAME2_GET_CONTROLLER_NAME) &PlatformPciIoGetDeviceName,
> >> +  "en" // SupportedLanguages, RFC 4646 language codes
> >> +};
> >> +
> >> +
> >> +//
> >> +// Entry point of this driver.
> >> +//
> >> +EFI_STATUS
> >> +EFIAPI
> >> +PlatformPciIoDxeEntryPoint (
> >> +  IN EFI_HANDLE       ImageHandle,
> >> +  IN EFI_SYSTEM_TABLE *SystemTable
> >> +  )
> >> +{
> >> +  return EfiLibInstallDriverBindingComponentName2 (
> >> +           ImageHandle,
> >> +           SystemTable,
> >> +           &gDriverBinding,
> >> +           ImageHandle,
> >> +           &gComponentName,
> >> +           &gComponentName2
> >> +           );
> >> +}
> >> diff --git a/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.inf b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.inf
> >> new file mode 100644
> >> index 000000000000..2b0baf06732c
> >> --- /dev/null
> >> +++ b/EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.inf
> >> @@ -0,0 +1,41 @@
> >> +## @file
> >> +# 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
> >> +# http://opensource.org/licenses/bsd-license.php
> >> +#
> >> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> >> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> >> +#
> >> +##
> >> +
> >> +[Defines]
> >> +  INF_VERSION                    = 0x00010017
> >
> > 0019?
> >
> >> +  BASE_NAME                      = PlatformPciIoDxe
> >> +  FILE_GUID                      = 71fd84cd-353b-464d-b7a4-6ea7b96995cb
> >> +  MODULE_TYPE                    = UEFI_DRIVER
> >> +  VERSION_STRING                 = 1.0
> >> +  ENTRY_POINT                    = PlatformPciIoDxeEntryPoint
> >> +
> >> +[Sources]
> >> +  PlatformPciIoDxe.c
> >> +  PlatformPciIo.c
> >> +  PlatformPciIo.h
> >> +
> >> +[Packages]
> >> +  EmbeddedPkg/EmbeddedPkg.dec
> >> +  MdePkg/MdePkg.dec
> >> +
> >> +[LibraryClasses]
> >> +  BaseMemoryLib
> >> +  DebugLib
> >> +  MemoryAllocationLib
> >> +  UefiBootServicesTableLib
> >> +  UefiDriverEntryPoint
> >> +  UefiLib
> >> +
> >> +[Protocols]
> >> +  gEfiPciIoProtocolGuid           ## BY_START
> >> +  gPlatformPciIoProtocolGuid      ## TO_START
> >> diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
> >> index d47c836379c9..47f9d47eb378 100644
> >> --- a/EmbeddedPkg/EmbeddedPkg.dsc
> >> +++ b/EmbeddedPkg/EmbeddedPkg.dsc
> >> @@ -291,6 +291,7 @@ [Components.common]
> >>    EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
> >>
> >>    EmbeddedPkg/Library/PlatformPciIoDeviceRegistrationLib/PlatformPciIoDeviceRegistrationLib.inf
> >> +  EmbeddedPkg/Drivers/PlatformPciIoDxe/PlatformPciIoDxe.inf
> >>
> >>  [Components.IA32, Components.X64, Components.IPF, Components.ARM]
> >>    EmbeddedPkg/GdbStub/GdbStub.inf
> >> --
> >> 2.7.4
> >>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


  reply	other threads:[~2016-11-02 16:05 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-31 18:13 [PATCH 0/5] EmbeddedPkg: generic support for reusing PCI drivers for platform devices Ard Biesheuvel
2016-10-31 18:13 ` [PATCH 1/5] EmbeddedPkg: introduce platform PCI I/O protocol Ard Biesheuvel
2016-11-01 21:54   ` Leif Lindholm
2016-11-02 13:29     ` Ard Biesheuvel
2016-11-02 15:42       ` Leif Lindholm
2016-10-31 18:13 ` [PATCH 2/5] EmbeddedPkg: introduce platform PCI I/O registration library Ard Biesheuvel
2016-11-01 21:57   ` Leif Lindholm
2016-11-02 13:30     ` Ard Biesheuvel
2016-11-02 15:39       ` Leif Lindholm
2016-11-07 14:54       ` Evan Lloyd
2016-10-31 18:13 ` [PATCH 3/5] EmbeddedPkg: implement generic platform PCI I/O driver Ard Biesheuvel
2016-11-01 22:22   ` Leif Lindholm
2016-11-02 13:39     ` Ard Biesheuvel
2016-11-02 16:05       ` Leif Lindholm [this message]
2016-10-31 18:13 ` [PATCH 4/5] ArmPkg/CpuDxe: set DmaBufferAlignment according to CWG Ard Biesheuvel
2016-11-01 22:32   ` Leif Lindholm
2016-11-02 13:40     ` Ard Biesheuvel
2016-11-02 16:10       ` Leif Lindholm
2016-11-02 16:17         ` Ard Biesheuvel
2016-11-02 16:21           ` Leif Lindholm
2016-11-02 16:23             ` Ard Biesheuvel
2016-10-31 18:13 ` [PATCH 5/5] EmbeddedPkg/PlatformPciIoDxe: add support for non-coherent DMA Ard Biesheuvel
2016-11-01 22:43   ` Leif Lindholm
2016-11-02 13:43     ` Ard Biesheuvel
2016-11-02 16:17       ` Leif Lindholm
2016-11-01 21:45 ` [PATCH 0/5] EmbeddedPkg: generic support for reusing PCI drivers for platform devices Leif Lindholm

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161102160552.GC27644@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox