diff mbox series

[edk2,v2] Silicon/SynQuacer/PlatformDxe: enable spread spectrum mode for ASM1061 SATA

Message ID 20180124115711.18797-1-ard.biesheuvel@linaro.org
State New
Headers show
Series [edk2,v2] Silicon/SynQuacer/PlatformDxe: enable spread spectrum mode for ASM1061 SATA | expand

Commit Message

Ard Biesheuvel Jan. 24, 2018, 11:57 a.m. UTC
The ASM1061 SATA controller integrated into the DeveloperBox board
emits too much electromagnetic radiation, so it needs spread spectrum
mode enabled.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
v2: - rename AsmediaXXX.c to Pci.c
    - rename RegisterAsm1184Notifier() to RegisterPciIoNotifier() and make
      its invocation unconditional (rather than only if PCI domain #0 is
      not limited to Gen1 speed)
    - use switch() rather than if() in PID/VID check

 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/{Asmedia118x.c => Pci.c} | 87 +++++++++++++++-----
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c            | 18 ++--
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h            |  5 +-
 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf          |  2 +-
 4 files changed, 80 insertions(+), 32 deletions(-)

-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Jan. 24, 2018, 12:02 p.m. UTC | #1
On Wed, Jan 24, 2018 at 11:57:11AM +0000, Ard Biesheuvel wrote:
> The ASM1061 SATA controller integrated into the DeveloperBox board

> emits too much electromagnetic radiation, so it needs spread spectrum

> mode enabled.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


> ---

> v2: - rename AsmediaXXX.c to Pci.c

>     - rename RegisterAsm1184Notifier() to RegisterPciIoNotifier() and make

>       its invocation unconditional (rather than only if PCI domain #0 is

>       not limited to Gen1 speed)

>     - use switch() rather than if() in PID/VID check


Thanks!

> 

>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/{Asmedia118x.c => Pci.c} | 87 +++++++++++++++-----

>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c            | 18 ++--

>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h            |  5 +-

>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf          |  2 +-

>  4 files changed, 80 insertions(+), 32 deletions(-)

> 

> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pci.c

> similarity index 63%

> rename from Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c

> rename to Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pci.c

> index c4cbacd3dff9..7ac96ab22b7a 100644

> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c

> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pci.c

> @@ -15,9 +15,12 @@

>  #include "PlatformDxe.h"

>  

>  #define ASMEDIA_VID                         0x1b21

> +#define ASM1061_PID                         0x0612

>  #define ASM1182E_PID                        0x1182

>  #define ASM1184E_PID                        0x1184

>  

> +#define ASM1061_SSC_OFFSET                  0xA10

> +

>  #define ASM118x_PCIE_CAPABILITY_OFFSET      0x80

>  #define ASM118x_PCIE_LINK_CONTROL_OFFSET    (ASM118x_PCIE_CAPABILITY_OFFSET + \

>                                               OFFSET_OF (PCI_CAPABILITY_PCIEXP, \

> @@ -39,24 +42,10 @@ 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

> @@ -91,6 +80,34 @@ RetrainAsm1184eDownstreamPort (

>  

>  STATIC

>  VOID

> +EnableAsm1061SpreadSpectrum (

> +  IN  EFI_PCI_IO_PROTOCOL   *PciIo

> +  )

> +{

> +  EFI_STATUS  Status;

> +  UINT8       SscVal;

> +

> +  DEBUG ((DEBUG_INFO, "%a: enabling spread spectrum mode 0 for ASM1061\n",

> +    __FUNCTION__));

> +

> +  // SSC mode 0~-4000 ppm, 1:1 modulation

> +

> +  SscVal = 0;

> +  Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint8, ASM1061_SSC_OFFSET, 1,

> +                        &SscVal);

> +  ASSERT_EFI_ERROR (Status);

> +

> +  MemoryFence ();

> +  gBS->Stall (1); // delay at least 100 ns between writes of the same register

> +

> +  SscVal = 1;

> +  Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint8, ASM1061_SSC_OFFSET, 1,

> +                        &SscVal);

> +  ASSERT_EFI_ERROR (Status);

> +}

> +

> +STATIC

> +VOID

>  EFIAPI

>  OnPciIoProtocolNotify (

>    IN EFI_EVENT      Event,

> @@ -101,6 +118,7 @@ OnPciIoProtocolNotify (

>    EFI_STATUS                Status;

>    EFI_HANDLE                HandleBuffer;

>    UINTN                     BufferSize;

> +  UINT16                    PciVidPid[2];

>  

>    while (TRUE) {

>      BufferSize = sizeof (EFI_HANDLE);

> @@ -114,18 +132,45 @@ OnPciIoProtocolNotify (

>                      (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);

> +    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));

> +      continue;

> +    }

> +

> +    if (PciVidPid[0] != ASMEDIA_VID) {

> +      continue;

> +    }

> +

> +    switch (PciVidPid[1]) {

> +    case ASM1061_PID:

> +      //

> +      // The ASM1061 SATA controller as integrated into the DeveloperBox design

> +      // emits too much electromagnetic radiation. So enable spread spectrum

> +      // mode.

> +      //

> +      EnableAsm1061SpreadSpectrum (PciIo);

> +      break;

> +    case ASM1182E_PID:

> +    case ASM1184E_PID:

> +      //

> +      // 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.

> +      //

> +      if (mHiiSettings->Pcie0MaxSpeed != PCIE_MAX_SPEED_GEN1) {

> +        RetrainAsm1184eDownstreamPort (PciIo);

> +      }

> +      break;

> +    }

>    }

>  }

>  

>  EFI_STATUS

>  EFIAPI

> -RegisterAsm1184Notifier (

> +RegisterPciIoNotifier (

>    VOID

>    )

>  {

> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c

> index 11b31e77bd3f..f3b05fc973ed 100644

> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c

> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c

> @@ -14,6 +14,9 @@

>  

>  #include "PlatformDxe.h"

>  

> +UINT64                            mHiiSettingsVal;

> +SYNQUACER_PLATFORM_VARSTORE_DATA  *mHiiSettings;

> +

>  typedef struct {

>    VENDOR_DEVICE_PATH              VendorDevicePath;

>    EFI_DEVICE_PATH_PROTOCOL        End;

> @@ -253,8 +256,9 @@ PlatformDxeEntryPoint (

>    VOID                              *Dtb;

>    UINTN                             DtbSize;

>    EFI_HANDLE                        Handle;

> -  UINT64                            SettingsVal;

> -  SYNQUACER_PLATFORM_VARSTORE_DATA  *Settings;

> +

> +  mHiiSettingsVal = PcdGet64 (PcdPlatformSettings);

> +  mHiiSettings = (SYNQUACER_PLATFORM_VARSTORE_DATA *)&mHiiSettingsVal;

>  

>    Dtb = NULL;

>    Status = DtPlatformLoadDtb (&Dtb, &DtbSize);

> @@ -294,17 +298,13 @@ PlatformDxeEntryPoint (

>  

>    SmmuEnableCoherentDma ();

>  

> -  SettingsVal = PcdGet64 (PcdPlatformSettings);

> -  Settings = (SYNQUACER_PLATFORM_VARSTORE_DATA *)&SettingsVal;

> -  if (Settings->Pcie0MaxSpeed != PCIE_MAX_SPEED_GEN1) {

> -    Status = RegisterAsm1184Notifier ();

> -    ASSERT_EFI_ERROR (Status);

> -  }

> +  Status = RegisterPciIoNotifier ();

> +  ASSERT_EFI_ERROR (Status);

>  

>    Status = EnableSettingsForm ();

>    ASSERT_EFI_ERROR (Status);

>  

> -  if (Settings->EnableEmmc == EMMC_ENABLED) {

> +  if (mHiiSettings->EnableEmmc == EMMC_ENABLED) {

>      Status = RegisterEmmc ();

>      ASSERT_EFI_ERROR (Status);

>    }

> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h

> index de7de12cec97..b634e8be99ad 100644

> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h

> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h

> @@ -40,9 +40,12 @@

>  extern  UINT8                             PlatformDxeHiiBin[];

>  extern  UINT8                             PlatformDxeStrings[];

>  

> +extern  UINT64                            mHiiSettingsVal;

> +extern  SYNQUACER_PLATFORM_VARSTORE_DATA  *mHiiSettings;

> +

>  EFI_STATUS

>  EFIAPI

> -RegisterAsm1184Notifier (

> +RegisterPciIoNotifier (

>    VOID

>    );

>  

> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf

> index 16412b999a40..e13e16f3da6b 100644

> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf

> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf

> @@ -23,8 +23,8 @@ [Defines]

>    ENTRY_POINT                    = PlatformDxeEntryPoint

>  

>  [Sources]

> -  Asmedia118x.c

>    Emmc.c

> +  Pci.c

>    PlatformDxe.c

>    PlatformDxeHii.uni

>    PlatformDxeHii.vfr

> -- 

> 2.11.0

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pci.c
similarity index 63%
rename from Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c
rename to Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pci.c
index c4cbacd3dff9..7ac96ab22b7a 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pci.c
@@ -15,9 +15,12 @@ 
 #include "PlatformDxe.h"
 
 #define ASMEDIA_VID                         0x1b21
+#define ASM1061_PID                         0x0612
 #define ASM1182E_PID                        0x1182
 #define ASM1184E_PID                        0x1184
 
+#define ASM1061_SSC_OFFSET                  0xA10
+
 #define ASM118x_PCIE_CAPABILITY_OFFSET      0x80
 #define ASM118x_PCIE_LINK_CONTROL_OFFSET    (ASM118x_PCIE_CAPABILITY_OFFSET + \
                                              OFFSET_OF (PCI_CAPABILITY_PCIEXP, \
@@ -39,24 +42,10 @@  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
@@ -91,6 +80,34 @@  RetrainAsm1184eDownstreamPort (
 
 STATIC
 VOID
+EnableAsm1061SpreadSpectrum (
+  IN  EFI_PCI_IO_PROTOCOL   *PciIo
+  )
+{
+  EFI_STATUS  Status;
+  UINT8       SscVal;
+
+  DEBUG ((DEBUG_INFO, "%a: enabling spread spectrum mode 0 for ASM1061\n",
+    __FUNCTION__));
+
+  // SSC mode 0~-4000 ppm, 1:1 modulation
+
+  SscVal = 0;
+  Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint8, ASM1061_SSC_OFFSET, 1,
+                        &SscVal);
+  ASSERT_EFI_ERROR (Status);
+
+  MemoryFence ();
+  gBS->Stall (1); // delay at least 100 ns between writes of the same register
+
+  SscVal = 1;
+  Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint8, ASM1061_SSC_OFFSET, 1,
+                        &SscVal);
+  ASSERT_EFI_ERROR (Status);
+}
+
+STATIC
+VOID
 EFIAPI
 OnPciIoProtocolNotify (
   IN EFI_EVENT      Event,
@@ -101,6 +118,7 @@  OnPciIoProtocolNotify (
   EFI_STATUS                Status;
   EFI_HANDLE                HandleBuffer;
   UINTN                     BufferSize;
+  UINT16                    PciVidPid[2];
 
   while (TRUE) {
     BufferSize = sizeof (EFI_HANDLE);
@@ -114,18 +132,45 @@  OnPciIoProtocolNotify (
                     (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);
+    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));
+      continue;
+    }
+
+    if (PciVidPid[0] != ASMEDIA_VID) {
+      continue;
+    }
+
+    switch (PciVidPid[1]) {
+    case ASM1061_PID:
+      //
+      // The ASM1061 SATA controller as integrated into the DeveloperBox design
+      // emits too much electromagnetic radiation. So enable spread spectrum
+      // mode.
+      //
+      EnableAsm1061SpreadSpectrum (PciIo);
+      break;
+    case ASM1182E_PID:
+    case ASM1184E_PID:
+      //
+      // 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.
+      //
+      if (mHiiSettings->Pcie0MaxSpeed != PCIE_MAX_SPEED_GEN1) {
+        RetrainAsm1184eDownstreamPort (PciIo);
+      }
+      break;
+    }
   }
 }
 
 EFI_STATUS
 EFIAPI
-RegisterAsm1184Notifier (
+RegisterPciIoNotifier (
   VOID
   )
 {
diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
index 11b31e77bd3f..f3b05fc973ed 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.c
@@ -14,6 +14,9 @@ 
 
 #include "PlatformDxe.h"
 
+UINT64                            mHiiSettingsVal;
+SYNQUACER_PLATFORM_VARSTORE_DATA  *mHiiSettings;
+
 typedef struct {
   VENDOR_DEVICE_PATH              VendorDevicePath;
   EFI_DEVICE_PATH_PROTOCOL        End;
@@ -253,8 +256,9 @@  PlatformDxeEntryPoint (
   VOID                              *Dtb;
   UINTN                             DtbSize;
   EFI_HANDLE                        Handle;
-  UINT64                            SettingsVal;
-  SYNQUACER_PLATFORM_VARSTORE_DATA  *Settings;
+
+  mHiiSettingsVal = PcdGet64 (PcdPlatformSettings);
+  mHiiSettings = (SYNQUACER_PLATFORM_VARSTORE_DATA *)&mHiiSettingsVal;
 
   Dtb = NULL;
   Status = DtPlatformLoadDtb (&Dtb, &DtbSize);
@@ -294,17 +298,13 @@  PlatformDxeEntryPoint (
 
   SmmuEnableCoherentDma ();
 
-  SettingsVal = PcdGet64 (PcdPlatformSettings);
-  Settings = (SYNQUACER_PLATFORM_VARSTORE_DATA *)&SettingsVal;
-  if (Settings->Pcie0MaxSpeed != PCIE_MAX_SPEED_GEN1) {
-    Status = RegisterAsm1184Notifier ();
-    ASSERT_EFI_ERROR (Status);
-  }
+  Status = RegisterPciIoNotifier ();
+  ASSERT_EFI_ERROR (Status);
 
   Status = EnableSettingsForm ();
   ASSERT_EFI_ERROR (Status);
 
-  if (Settings->EnableEmmc == EMMC_ENABLED) {
+  if (mHiiSettings->EnableEmmc == EMMC_ENABLED) {
     Status = RegisterEmmc ();
     ASSERT_EFI_ERROR (Status);
   }
diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h
index de7de12cec97..b634e8be99ad 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h
@@ -40,9 +40,12 @@ 
 extern  UINT8                             PlatformDxeHiiBin[];
 extern  UINT8                             PlatformDxeStrings[];
 
+extern  UINT64                            mHiiSettingsVal;
+extern  SYNQUACER_PLATFORM_VARSTORE_DATA  *mHiiSettings;
+
 EFI_STATUS
 EFIAPI
-RegisterAsm1184Notifier (
+RegisterPciIoNotifier (
   VOID
   );
 
diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
index 16412b999a40..e13e16f3da6b 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
@@ -23,8 +23,8 @@  [Defines]
   ENTRY_POINT                    = PlatformDxeEntryPoint
 
 [Sources]
-  Asmedia118x.c
   Emmc.c
+  Pci.c
   PlatformDxe.c
   PlatformDxeHii.uni
   PlatformDxeHii.vfr