Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/{Asmedia118x.c => Asmedia.c} | 79 +++++++++++++++-----
Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf | 2 +-
2 files changed, 60 insertions(+), 21 deletions(-)
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>
---
Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/{Asmedia118x.c => Asmedia.c} | 79 +++++++++++++++-----
Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf | 2 +-
2 files changed, 60 insertions(+), 21 deletions(-)
diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia.c
similarity index 64%
rename from Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c
rename to Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia.c
index c4cbacd3dff9..6c289fa1892e 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia.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,12 +132,33 @@ 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;
+ }
+
+ if (PciVidPid[1] == ASM1061_PID) {
+ //
+ // The ASM1061 SATA controller as integrated into the DeveloperBox design
+ // emits too much electromagnetic radiation. So enable spread spectrum
+ // mode.
+ //
+ EnableAsm1061SpreadSpectrum (PciIo);
+ } else if (PciVidPid[1] == ASM1182E_PID || PciVidPid[1] == 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.
+ //
+ RetrainAsm1184eDownstreamPort (PciIo);
+ }
}
}
diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
index 16412b999a40..64c90ac74e8e 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
+++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
@@ -23,7 +23,7 @@ [Defines]
ENTRY_POINT = PlatformDxeEntryPoint
[Sources]
- Asmedia118x.c
+ Asmedia.c
Emmc.c
PlatformDxe.c
PlatformDxeHii.uni
--
2.11.0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, Jan 24, 2018 at 10:40:52AM +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.
I presume the spectrum mode is on the PCIe side rather than the SATA
side? Can this be explicitly mentioned?
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/{Asmedia118x.c => Asmedia.c} | 79 +++++++++++++++-----
Since you started renaming things, I invoke the right of bikeshedding.
The SATA controller is only Asmedia as well due to happenstance - if
this file is becoming "various PCI tweaks", can we just call it Pci.c
(or PciTweaks.c by all means)?
> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf | 2 +-
> 2 files changed, 60 insertions(+), 21 deletions(-)
>
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia.c
> similarity index 64%
> rename from Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c
> rename to Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia.c
> index c4cbacd3dff9..6c289fa1892e 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia.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,12 +132,33 @@ 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;
> + }
> +
> + if (PciVidPid[1] == ASM1061_PID) {
> + //
> + // The ASM1061 SATA controller as integrated into the DeveloperBox design
> + // emits too much electromagnetic radiation. So enable spread spectrum
> + // mode.
> + //
> + EnableAsm1061SpreadSpectrum (PciIo);
> + } else if (PciVidPid[1] == ASM1182E_PID || PciVidPid[1] == ASM1184E_PID) {
Does this "else if" prevent the SATA controller from training GEN2?
Could it be just another if?
/
Leif
> + //
> + // 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);
> + }
> }
> }
>
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
> index 16412b999a40..64c90ac74e8e 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
> @@ -23,7 +23,7 @@ [Defines]
> ENTRY_POINT = PlatformDxeEntryPoint
>
> [Sources]
> - Asmedia118x.c
> + Asmedia.c
> Emmc.c
> PlatformDxe.c
> PlatformDxeHii.uni
> --
> 2.11.0
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 24 January 2018 at 11:10, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, Jan 24, 2018 at 10:40:52AM +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.
>
> I presume the spectrum mode is on the PCIe side rather than the SATA
> side? Can this be explicitly mentioned?
>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/{Asmedia118x.c => Asmedia.c} | 79 +++++++++++++++-----
>
> Since you started renaming things, I invoke the right of bikeshedding.
>
> The SATA controller is only Asmedia as well due to happenstance - if
> this file is becoming "various PCI tweaks", can we just call it Pci.c
> (or PciTweaks.c by all means)?
>
Sure
>> Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf | 2 +-
>> 2 files changed, 60 insertions(+), 21 deletions(-)
>>
>> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia.c
>> similarity index 64%
>> rename from Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c
>> rename to Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia.c
>> index c4cbacd3dff9..6c289fa1892e 100644
>> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia118x.c
>> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Asmedia.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,12 +132,33 @@ 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;
>> + }
>> +
>> + if (PciVidPid[1] == ASM1061_PID) {
>> + //
>> + // The ASM1061 SATA controller as integrated into the DeveloperBox design
>> + // emits too much electromagnetic radiation. So enable spread spectrum
>> + // mode.
>> + //
>> + EnableAsm1061SpreadSpectrum (PciIo);
>> + } else if (PciVidPid[1] == ASM1182E_PID || PciVidPid[1] == ASM1184E_PID) {
>
> Does this "else if" prevent the SATA controller from training GEN2?
> Could it be just another if?
>
Only if PciVidPid[1] equals ASM1061_PID and ASM1182E_PID/ASM1184E_PID
at the same time.
>
>> + //
>> + // 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);
>> + }
>> }
>> }
>>
>> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
>> index 16412b999a40..64c90ac74e8e 100644
>> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
>> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.inf
>> @@ -23,7 +23,7 @@ [Defines]
>> ENTRY_POINT = PlatformDxeEntryPoint
>>
>> [Sources]
>> - Asmedia118x.c
>> + Asmedia.c
>> Emmc.c
>> PlatformDxe.c
>> PlatformDxeHii.uni
>> --
>> 2.11.0
>>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, Jan 24, 2018 at 11:13:17AM +0000, Ard Biesheuvel wrote:
> >> + if (PciVidPid[1] == ASM1061_PID) {
> >> + //
> >> + // The ASM1061 SATA controller as integrated into the DeveloperBox design
> >> + // emits too much electromagnetic radiation. So enable spread spectrum
> >> + // mode.
> >> + //
> >> + EnableAsm1061SpreadSpectrum (PciIo);
> >> + } else if (PciVidPid[1] == ASM1182E_PID || PciVidPid[1] == ASM1184E_PID) {
> >
> > Does this "else if" prevent the SATA controller from training GEN2?
> > Could it be just another if?
> >
>
> Only if PciVidPid[1] equals ASM1061_PID and ASM1182E_PID/ASM1184E_PID
> at the same time.
Ah, gotcha.
The flattened topology threw me off :)
/
Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.