[edk2] [PATCH edk2-platforms 8/8] Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed

Ard Biesheuvel posted 8 patches 7 years, 1 month ago
[edk2] [PATCH edk2-platforms 8/8] Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed
Posted by Ard Biesheuvel 7 years, 1 month ago
For some reason, the Asmedia 118x PCIe switch needs a little help to
make sure that the downstream links train at Gen2 speed. So add a
PCI I/O protocol notifier that implements this for each PCIe downstream
port that is present on the system.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c          | 140 ++++++++++++++++++++
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c   |  13 +-
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h   |  37 ++++++
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf |   3 +
 4 files changed, 184 insertions(+), 9 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c
new file mode 100644
index 000000000000..b069b42d0a42
--- /dev/null
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c
@@ -0,0 +1,140 @@
+ /** @file
+  SynQuacer DXE platform driver - PCIe support
+
+  Copyright (c) 2017, 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 "PlatformDxe.h"
+
+#define ASMEDIA_VID                         0x1b21
+#define ASM1182E_PID                        0x1182
+#define ASM1184E_PID                        0x1184
+
+#define ASM118x_PCIE_CAPABILITY_OFFSET      0x80
+#define ASM118x_PCIE_LINK_CONTROL_OFFSET    (ASM118x_PCIE_CAPABILITY_OFFSET + \
+                                             OFFSET_OF (PCI_CAPABILITY_PCIEXP, \
+                                                        LinkControl))
+
+STATIC VOID         *mPciProtocolNotifyRegistration;
+STATIC EFI_EVENT    mPciProtocolNotifyEvent;
+
+#pragma pack(1)
+typedef struct {
+  EFI_PCI_CAPABILITY_HDR    CapHdr;
+  PCI_REG_PCIE_CAPABILITY   Pcie;
+} PCIE_CAP;
+#pragma pack()
+
+STATIC
+VOID
+RetrainAsm1184eDownstreamPort (
+  IN  EFI_PCI_IO_PROTOCOL   *PciIo
+  )
+{
+  UINT16                    PciVidPid[2];
+  EFI_STATUS                Status;
+  PCIE_CAP                  Cap;
+  PCI_REG_PCIE_LINK_CONTROL LinkControl;
+
+  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16, PCI_VENDOR_ID_OFFSET,
+                        ARRAY_SIZE (PciVidPid), &PciVidPid);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_WARN, "%a: failed to read PCI vendor/product ID - %r\n",
+      __FUNCTION__, Status));
+    return;
+  }
+
+  if (PciVidPid[0] != ASMEDIA_VID ||
+      (PciVidPid[1] != ASM1182E_PID && PciVidPid[1] != ASM1184E_PID)) {
+    return;
+  }
+
+  //
+  // The upstream and downstream ports share the same PID/VID, so check
+  // the port type. This assumes the PCIe Express capability block lives
+  // at offset 0x80 in the port's config space, which is known to be the
+  // case for these particular chips.
+  //
+  ASSERT (sizeof (Cap) == sizeof (UINT32));
+  ASSERT (sizeof (LinkControl) == sizeof (UINT16));
+
+  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint32,
+                        ASM118x_PCIE_CAPABILITY_OFFSET, 1, &Cap);
+  ASSERT_EFI_ERROR (Status);
+  ASSERT (Cap.CapHdr.CapabilityID == EFI_PCI_CAPABILITY_ID_PCIEXP);
+
+  if (Cap.Pcie.Bits.DevicePortType != PCIE_DEVICE_PORT_TYPE_DOWNSTREAM_PORT) {
+    return;
+  }
+
+  DEBUG ((DEBUG_INFO, "%a: retraining ASM1184x downstream PCIe port\n",
+    __FUNCTION__));
+
+  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16,
+                        ASM118x_PCIE_LINK_CONTROL_OFFSET, 1, &LinkControl);
+  ASSERT_EFI_ERROR (Status);
+
+  LinkControl.Bits.RetrainLink = 1;
+
+  Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16,
+                        ASM118x_PCIE_LINK_CONTROL_OFFSET, 1, &LinkControl);
+  ASSERT_EFI_ERROR (Status);
+}
+
+STATIC
+VOID
+EFIAPI
+OnPciIoProtocolNotify (
+  IN EFI_EVENT      Event,
+  IN VOID           *Context
+  )
+{
+  EFI_PCI_IO_PROTOCOL       *PciIo;
+  EFI_STATUS                Status;
+  EFI_HANDLE                HandleBuffer;
+  UINTN                     BufferSize;
+
+  while (TRUE) {
+    BufferSize = sizeof (EFI_HANDLE);
+    Status = gBS->LocateHandle (ByRegisterNotify, NULL,
+                    mPciProtocolNotifyRegistration, &BufferSize, &HandleBuffer);
+    if (EFI_ERROR (Status)) {
+      break;
+    }
+
+    Status = gBS->HandleProtocol (HandleBuffer, &gEfiPciIoProtocolGuid,
+                    (VOID **)&PciIo);
+    ASSERT_EFI_ERROR (Status);
+
+    //
+    // The ASM1184E 4-port PCIe switch on the DeveloperBox board (and its
+    // 2-port sibling of which samples were used in development) needs a
+    // little nudge to get it to train the downstream links at Gen2 speed.
+    //
+    RetrainAsm1184eDownstreamPort (PciIo);
+  }
+}
+
+EFI_STATUS
+EFIAPI
+RegisterPcieNotifier (
+  VOID
+  )
+{
+  mPciProtocolNotifyEvent = EfiCreateProtocolNotifyEvent (
+                              &gEfiPciIoProtocolGuid,
+                              TPL_CALLBACK,
+                              OnPciIoProtocolNotify,
+                              NULL,
+                              &mPciProtocolNotifyRegistration);
+
+  return EFI_SUCCESS;
+}
diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
index e58a2093eb49..098a4dbd324e 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
@@ -12,15 +12,7 @@
   WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 **/
 
-#include <PiDxe.h>
-#include <Library/BaseMemoryLib.h>
-#include <Library/DebugLib.h>
-#include <Library/DtPlatformDtbLoaderLib.h>
-#include <Library/IoLib.h>
-#include <Library/MemoryAllocationLib.h>
-#include <Library/UefiBootServicesTableLib.h>
-#include <Platform/MemoryMap.h>
-#include <Protocol/NonDiscoverableDevice.h>
+#include "PlatformDxe.h"
 
 STATIC EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR mNetsecDesc[] = {
   {
@@ -202,5 +194,8 @@ PlatformDxeEntryPoint (
 
   SmmuEnableCoherentDma ();
 
+  Status = RegisterPcieNotifier ();
+  ASSERT_EFI_ERROR (Status);
+
   return EFI_SUCCESS;
 }
diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h
new file mode 100644
index 000000000000..d1dad2a3eace
--- /dev/null
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h
@@ -0,0 +1,37 @@
+/** @file
+  SynQuacer DXE platform driver.
+
+  Copyright (c) 2017, 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.
+**/
+
+#ifndef __PLATFORM_DXE_H__
+#define __PLATFORM_DXE_H__
+
+#include <PiDxe.h>
+#include <IndustryStandard/Pci.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DtPlatformDtbLoaderLib.h>
+#include <Library/IoLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+#include <Platform/MemoryMap.h>
+#include <Protocol/NonDiscoverableDevice.h>
+#include <Protocol/PciIo.h>
+
+EFI_STATUS
+EFIAPI
+RegisterPcieNotifier (
+  VOID
+  );
+
+#endif
diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
index 00c1130906c4..84498eaddcef 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
@@ -23,6 +23,7 @@ [Defines]
   ENTRY_POINT                    = PlatformDxeEntryPoint
 
 [Sources]
+  Pcie.c
   PlatformDxe.c
 
 [Packages]
@@ -41,6 +42,7 @@ [LibraryClasses]
   MemoryAllocationLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
+  UefiLib
 
 [Guids]
   gFdtTableGuid
@@ -50,6 +52,7 @@ [Guids]
 
 [Protocols]
   gEdkiiNonDiscoverableDeviceProtocolGuid         ## PRODUCES
+  gEfiPciIoProtocolGuid                           ## CONSUMES
   gPcf8563RealTimeClockLibI2cMasterProtolGuid     ## PRODUCES
 
 [FixedPcd]
-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms 8/8] Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed
Posted by Leif Lindholm 7 years, 1 month ago
On Tue, Dec 12, 2017 at 10:38:07AM +0000, Ard Biesheuvel wrote:
> For some reason, the Asmedia 118x PCIe switch needs a little help to
> make sure that the downstream links train at Gen2 speed. So add a
> PCI I/O protocol notifier that implements this for each PCIe downstream
> port that is present on the system.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c          | 140 ++++++++++++++++++++
>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c   |  13 +-
>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h   |  37 ++++++
>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf |   3 +
>  4 files changed, 184 insertions(+), 9 deletions(-)
> 
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c
> new file mode 100644
> index 000000000000..b069b42d0a42
> --- /dev/null
> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c

Bikeshedding time:
This driver would likely be needed for any other platform including
this switch as well, right?

While it may be premature to create a standalone driver under
Silicon/Asmedia ... how about calling this file something to make it
clear that it is specifically intended to handle Asmedia 118x devices,
to make it easier* to do so in the future? I.e. Asmedia118x.c?

* by avoiding accruing other random bits of platform-specific PCI
  hackery in the same file.

/
    Leif

> @@ -0,0 +1,140 @@
> + /** @file
> +  SynQuacer DXE platform driver - PCIe support
> +
> +  Copyright (c) 2017, 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 "PlatformDxe.h"
> +
> +#define ASMEDIA_VID                         0x1b21
> +#define ASM1182E_PID                        0x1182
> +#define ASM1184E_PID                        0x1184
> +
> +#define ASM118x_PCIE_CAPABILITY_OFFSET      0x80
> +#define ASM118x_PCIE_LINK_CONTROL_OFFSET    (ASM118x_PCIE_CAPABILITY_OFFSET + \
> +                                             OFFSET_OF (PCI_CAPABILITY_PCIEXP, \
> +                                                        LinkControl))
> +
> +STATIC VOID         *mPciProtocolNotifyRegistration;
> +STATIC EFI_EVENT    mPciProtocolNotifyEvent;
> +
> +#pragma pack(1)
> +typedef struct {
> +  EFI_PCI_CAPABILITY_HDR    CapHdr;
> +  PCI_REG_PCIE_CAPABILITY   Pcie;
> +} PCIE_CAP;
> +#pragma pack()
> +
> +STATIC
> +VOID
> +RetrainAsm1184eDownstreamPort (
> +  IN  EFI_PCI_IO_PROTOCOL   *PciIo
> +  )
> +{
> +  UINT16                    PciVidPid[2];
> +  EFI_STATUS                Status;
> +  PCIE_CAP                  Cap;
> +  PCI_REG_PCIE_LINK_CONTROL LinkControl;
> +
> +  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16, PCI_VENDOR_ID_OFFSET,
> +                        ARRAY_SIZE (PciVidPid), &PciVidPid);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_WARN, "%a: failed to read PCI vendor/product ID - %r\n",
> +      __FUNCTION__, Status));
> +    return;
> +  }
> +
> +  if (PciVidPid[0] != ASMEDIA_VID ||
> +      (PciVidPid[1] != ASM1182E_PID && PciVidPid[1] != ASM1184E_PID)) {
> +    return;
> +  }
> +
> +  //
> +  // The upstream and downstream ports share the same PID/VID, so check
> +  // the port type. This assumes the PCIe Express capability block lives
> +  // at offset 0x80 in the port's config space, which is known to be the
> +  // case for these particular chips.
> +  //
> +  ASSERT (sizeof (Cap) == sizeof (UINT32));
> +  ASSERT (sizeof (LinkControl) == sizeof (UINT16));
> +
> +  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint32,
> +                        ASM118x_PCIE_CAPABILITY_OFFSET, 1, &Cap);
> +  ASSERT_EFI_ERROR (Status);
> +  ASSERT (Cap.CapHdr.CapabilityID == EFI_PCI_CAPABILITY_ID_PCIEXP);
> +
> +  if (Cap.Pcie.Bits.DevicePortType != PCIE_DEVICE_PORT_TYPE_DOWNSTREAM_PORT) {
> +    return;
> +  }
> +
> +  DEBUG ((DEBUG_INFO, "%a: retraining ASM1184x downstream PCIe port\n",
> +    __FUNCTION__));
> +
> +  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint16,
> +                        ASM118x_PCIE_LINK_CONTROL_OFFSET, 1, &LinkControl);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  LinkControl.Bits.RetrainLink = 1;
> +
> +  Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16,
> +                        ASM118x_PCIE_LINK_CONTROL_OFFSET, 1, &LinkControl);
> +  ASSERT_EFI_ERROR (Status);
> +}
> +
> +STATIC
> +VOID
> +EFIAPI
> +OnPciIoProtocolNotify (
> +  IN EFI_EVENT      Event,
> +  IN VOID           *Context
> +  )
> +{
> +  EFI_PCI_IO_PROTOCOL       *PciIo;
> +  EFI_STATUS                Status;
> +  EFI_HANDLE                HandleBuffer;
> +  UINTN                     BufferSize;
> +
> +  while (TRUE) {
> +    BufferSize = sizeof (EFI_HANDLE);
> +    Status = gBS->LocateHandle (ByRegisterNotify, NULL,
> +                    mPciProtocolNotifyRegistration, &BufferSize, &HandleBuffer);
> +    if (EFI_ERROR (Status)) {
> +      break;
> +    }
> +
> +    Status = gBS->HandleProtocol (HandleBuffer, &gEfiPciIoProtocolGuid,
> +                    (VOID **)&PciIo);
> +    ASSERT_EFI_ERROR (Status);
> +
> +    //
> +    // The ASM1184E 4-port PCIe switch on the DeveloperBox board (and its
> +    // 2-port sibling of which samples were used in development) needs a
> +    // little nudge to get it to train the downstream links at Gen2 speed.
> +    //
> +    RetrainAsm1184eDownstreamPort (PciIo);
> +  }
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +RegisterPcieNotifier (
> +  VOID
> +  )
> +{
> +  mPciProtocolNotifyEvent = EfiCreateProtocolNotifyEvent (
> +                              &gEfiPciIoProtocolGuid,
> +                              TPL_CALLBACK,
> +                              OnPciIoProtocolNotify,
> +                              NULL,
> +                              &mPciProtocolNotifyRegistration);
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> index e58a2093eb49..098a4dbd324e 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
> @@ -12,15 +12,7 @@
>    WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  **/
>  
> -#include <PiDxe.h>
> -#include <Library/BaseMemoryLib.h>
> -#include <Library/DebugLib.h>
> -#include <Library/DtPlatformDtbLoaderLib.h>
> -#include <Library/IoLib.h>
> -#include <Library/MemoryAllocationLib.h>
> -#include <Library/UefiBootServicesTableLib.h>
> -#include <Platform/MemoryMap.h>
> -#include <Protocol/NonDiscoverableDevice.h>
> +#include "PlatformDxe.h"
>  
>  STATIC EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR mNetsecDesc[] = {
>    {
> @@ -202,5 +194,8 @@ PlatformDxeEntryPoint (
>  
>    SmmuEnableCoherentDma ();
>  
> +  Status = RegisterPcieNotifier ();
> +  ASSERT_EFI_ERROR (Status);
> +
>    return EFI_SUCCESS;
>  }
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h
> new file mode 100644
> index 000000000000..d1dad2a3eace
> --- /dev/null
> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h
> @@ -0,0 +1,37 @@
> +/** @file
> +  SynQuacer DXE platform driver.
> +
> +  Copyright (c) 2017, 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.
> +**/
> +
> +#ifndef __PLATFORM_DXE_H__
> +#define __PLATFORM_DXE_H__
> +
> +#include <PiDxe.h>
> +#include <IndustryStandard/Pci.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DtPlatformDtbLoaderLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +#include <Platform/MemoryMap.h>
> +#include <Protocol/NonDiscoverableDevice.h>
> +#include <Protocol/PciIo.h>
> +
> +EFI_STATUS
> +EFIAPI
> +RegisterPcieNotifier (
> +  VOID
> +  );
> +
> +#endif
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
> index 00c1130906c4..84498eaddcef 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -23,6 +23,7 @@ [Defines]
>    ENTRY_POINT                    = PlatformDxeEntryPoint
>  
>  [Sources]
> +  Pcie.c
>    PlatformDxe.c
>  
>  [Packages]
> @@ -41,6 +42,7 @@ [LibraryClasses]
>    MemoryAllocationLib
>    UefiBootServicesTableLib
>    UefiDriverEntryPoint
> +  UefiLib
>  
>  [Guids]
>    gFdtTableGuid
> @@ -50,6 +52,7 @@ [Guids]
>  
>  [Protocols]
>    gEdkiiNonDiscoverableDeviceProtocolGuid         ## PRODUCES
> +  gEfiPciIoProtocolGuid                           ## CONSUMES
>    gPcf8563RealTimeClockLibI2cMasterProtolGuid     ## PRODUCES
>  
>  [FixedPcd]
> -- 
> 2.11.0
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms 8/8] Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed
Posted by Ard Biesheuvel 7 years, 1 month ago
On 12 December 2017 at 17:47, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Dec 12, 2017 at 10:38:07AM +0000, Ard Biesheuvel wrote:
>> For some reason, the Asmedia 118x PCIe switch needs a little help to
>> make sure that the downstream links train at Gen2 speed. So add a
>> PCI I/O protocol notifier that implements this for each PCIe downstream
>> port that is present on the system.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c          | 140 ++++++++++++++++++++
>>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c   |  13 +-
>>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h   |  37 ++++++
>>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf |   3 +
>>  4 files changed, 184 insertions(+), 9 deletions(-)
>>
>> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c
>> new file mode 100644
>> index 000000000000..b069b42d0a42
>> --- /dev/null
>> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c
>
> Bikeshedding time:
> This driver would likely be needed for any other platform including
> this switch as well, right?
>
> While it may be premature to create a standalone driver under
> Silicon/Asmedia ... how about calling this file something to make it
> clear that it is specifically intended to handle Asmedia 118x devices,
> to make it easier* to do so in the future? I.e. Asmedia118x.c?
>
> * by avoiding accruing other random bits of platform-specific PCI
>   hackery in the same file.
>

To be honest, I am not entirely sure. I need this hack for the
standalone card as well as the onboard switch, so it is not related to
a board level defect on developerbox. However, it could be related to
how the Synopsys IP manages the reset and training etc.

But I agree, let's move this to Asmedia118x.c and not create a generic
looking file that invites more PCI quirks to be parked there
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms 8/8] Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed
Posted by Leif Lindholm 7 years, 1 month ago
On Tue, Dec 12, 2017 at 05:51:18PM +0000, Ard Biesheuvel wrote:
> On 12 December 2017 at 17:47, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Tue, Dec 12, 2017 at 10:38:07AM +0000, Ard Biesheuvel wrote:
> >> For some reason, the Asmedia 118x PCIe switch needs a little help to
> >> make sure that the downstream links train at Gen2 speed. So add a
> >> PCI I/O protocol notifier that implements this for each PCIe downstream
> >> port that is present on the system.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c          | 140 ++++++++++++++++++++
> >>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c   |  13 +-
> >>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h   |  37 ++++++
> >>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf |   3 +
> >>  4 files changed, 184 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c
> >> new file mode 100644
> >> index 000000000000..b069b42d0a42
> >> --- /dev/null
> >> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c
> >
> > Bikeshedding time:
> > This driver would likely be needed for any other platform including
> > this switch as well, right?
> >
> > While it may be premature to create a standalone driver under
> > Silicon/Asmedia ... how about calling this file something to make it
> > clear that it is specifically intended to handle Asmedia 118x devices,
> > to make it easier* to do so in the future? I.e. Asmedia118x.c?
> >
> > * by avoiding accruing other random bits of platform-specific PCI
> >   hackery in the same file.
> >
> 
> To be honest, I am not entirely sure. I need this hack for the
> standalone card as well as the onboard switch, so it is not related to
> a board level defect on developerbox. However, it could be related to
> how the Synopsys IP manages the reset and training etc.
> 
> But I agree, let's move this to Asmedia118x.c and not create a generic
> looking file that invites more PCI quirks to be parked there

With that:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel