* [PATCH edk2-platforms 1/8] Silicon/SynQuacer: enable CPU idle states in device tree
2017-12-12 10:37 [PATCH edk2-platforms 0/8] SynQuacer updates Ard Biesheuvel
@ 2017-12-12 10:38 ` Ard Biesheuvel
2017-12-12 10:38 ` [PATCH edk2-platforms 2/8] Platform/Socionext/SynQuacer: expose build number as firmware version Ard Biesheuvel
` (7 subsequent siblings)
8 siblings, 0 replies; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 10:38 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, daniel.thompson, masami.hiramatsu, Ard Biesheuvel
It appears that whatever was preventing us from using CPU idle with
PSCI low power states has disappeared, so let's enable the low power
states in the DT.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 48 ++++++++++----------
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
index ec784c70afe7..c9fee5d1f350 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
@@ -47,168 +47,168 @@
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x0>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU1: cpu@1 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x1>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU2: cpu@100 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x100>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU3: cpu@101 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x101>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU4: cpu@200 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x200>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU5: cpu@201 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x201>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU6: cpu@300 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x300>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU7: cpu@301 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x301>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU8: cpu@400 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x400>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU9: cpu@401 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x401>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU10: cpu@500 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x500>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU11: cpu@501 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x501>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU12: cpu@600 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x600>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU13: cpu@601 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x601>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU14: cpu@700 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x700>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU15: cpu@701 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x701>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU16: cpu@800 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x800>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU17: cpu@801 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x801>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU18: cpu@900 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x900>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU19: cpu@901 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0x901>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU20: cpu@a00 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0xa00>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU21: cpu@a01 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0xa01>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU22: cpu@b00 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0xb00>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
CPU23: cpu@b01 {
device_type = "cpu";
compatible = "arm,cortex-a53","arm,armv8";
reg = <0xb01>;
enable-method = "psci";
- //cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+ cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
};
cpu-map {
--
2.11.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH edk2-platforms 2/8] Platform/Socionext/SynQuacer: expose build number as firmware version
2017-12-12 10:37 [PATCH edk2-platforms 0/8] SynQuacer updates Ard Biesheuvel
2017-12-12 10:38 ` [PATCH edk2-platforms 1/8] Silicon/SynQuacer: enable CPU idle states in device tree Ard Biesheuvel
@ 2017-12-12 10:38 ` Ard Biesheuvel
2017-12-12 18:17 ` Leif Lindholm
2017-12-12 10:38 ` [PATCH edk2-platforms 3/8] Silicon/SynQuacerPciHostBridgeLib: stall for 150 ms during PERST# Ard Biesheuvel
` (6 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 10:38 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, daniel.thompson, masami.hiramatsu, Ard Biesheuvel
Expose the contents of the .DSC macro BUILD_NUMBER via the
PCD gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString (if > 1),
and as the FMP system firmware version (for capsule update).
Also, set the firmware vendor to 'Linaro Enterprise Group', to
distinguish our builds from builds by other parties.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 +++++++-
Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 ++++--
Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 8 +++++++-
Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 ++++--
6 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
index 8fbd7b2d908f..5ec26f9cdd34 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
@@ -26,6 +26,7 @@ [Defines]
BUILD_TARGETS = DEBUG|RELEASE
SKUID_IDENTIFIER = DEFAULT
FLASH_DEFINITION = Platform/Socionext/DeveloperBox/DeveloperBox.fdf
+ BUILD_NUMBER = 1
[BuildOptions]
RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
@@ -222,7 +223,7 @@ [PcdsFeatureFlag]
gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
[PcdsFixedAtBuild.common]
- gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"Linaro"
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"Linaro Enterprise Group"
# non-secure SRAM
gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
@@ -384,6 +385,11 @@ [PcdsFixedAtBuild.common]
# set DIP switch DSW3-PIN1 (GPIO pin PD[0] on the SoC) to clear the varstore
gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin|0
+!if $(BUILD_NUMBER) > 1
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(BUILD_NUMBER)"
+!endif
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
+
[PcdsPatchableInModule]
gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0
gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|0
diff --git a/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf b/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
index f5272c0f0d37..95a5e482a713 100644
--- a/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
+++ b/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
@@ -38,6 +38,7 @@ [LibraryClasses]
[FixedPcd]
gArmTokenSpaceGuid.PcdFdSize
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision
[Pcd]
gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareImageDescriptor
diff --git a/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc b/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc
index bc47e696da7a..fb69de078313 100644
--- a/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc
+++ b/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc
@@ -21,8 +21,10 @@
#define PACKAGE_VERSION 0xFFFFFFFF
#define PACKAGE_VERSION_STRING L"Unknown"
-#define CURRENT_FIRMWARE_VERSION 0x00000001
-#define CURRENT_FIRMWARE_VERSION_STRING L"0x00000001"
+#define __BUILD_STRING(x) L ## #x
+#define BUILD_STRING(x) L"build #" __BUILD_STRING(x)
+#define CURRENT_FIRMWARE_VERSION FixedPcdGet32 (PcdFirmwareRevision)
+#define CURRENT_FIRMWARE_VERSION_STRING BUILD_STRING (FixedPcdGet32 (PcdFirmwareRevision))
#define LOWEST_SUPPORTED_FIRMWARE_VERSION 0x00000001
#define IMAGE_ID SIGNATURE_64('S', 'N', 'D', 'E', 'V', 'B', 'O', 'X')
diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
index 895d3b09fdc9..bc8ddd452d4b 100644
--- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
+++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
@@ -26,6 +26,7 @@ [Defines]
BUILD_TARGETS = DEBUG|RELEASE
SKUID_IDENTIFIER = DEFAULT
FLASH_DEFINITION = Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf
+ BUILD_NUMBER = 1
[BuildOptions]
RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
@@ -214,7 +215,7 @@ [PcdsFeatureFlag]
gEfiMdeModulePkgTokenSpaceGuid.PcdSupportUpdateCapsuleReset|TRUE
[PcdsFixedAtBuild.common]
- gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"Linaro"
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"Linaro Enterprise Group"
# non-secure SRAM
gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
@@ -372,6 +373,11 @@ [PcdsFixedAtBuild.common]
# set DIP switch DSW3-PIN1 (GPIO pin PD[0] on the SoC) to clear the varstore
gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin|0
+!if $(BUILD_NUMBER) > 1
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(BUILD_NUMBER)"
+!endif
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
+
[PcdsPatchableInModule]
gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0
gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|0
diff --git a/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf b/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
index f5272c0f0d37..95a5e482a713 100644
--- a/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
+++ b/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
@@ -38,6 +38,7 @@ [LibraryClasses]
[FixedPcd]
gArmTokenSpaceGuid.PcdFdSize
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision
[Pcd]
gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareImageDescriptor
diff --git a/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc b/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc
index 3413f76f95c7..daf26c79dff1 100644
--- a/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc
+++ b/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc
@@ -21,8 +21,10 @@
#define PACKAGE_VERSION 0xFFFFFFFF
#define PACKAGE_VERSION_STRING L"Unknown"
-#define CURRENT_FIRMWARE_VERSION 0x00000001
-#define CURRENT_FIRMWARE_VERSION_STRING L"0x00000001"
+#define __BUILD_STRING(x) L ## #x
+#define BUILD_STRING(x) L"build #" __BUILD_STRING(x)
+#define CURRENT_FIRMWARE_VERSION FixedPcdGet32 (PcdFirmwareRevision)
+#define CURRENT_FIRMWARE_VERSION_STRING BUILD_STRING (FixedPcdGet32 (PcdFirmwareRevision))
#define LOWEST_SUPPORTED_FIRMWARE_VERSION 0x00000001
#define IMAGE_ID SIGNATURE_64('S', 'N', 'I', 'S', 'Y', 'N', 'Q', 'U')
--
2.11.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 2/8] Platform/Socionext/SynQuacer: expose build number as firmware version
2017-12-12 10:38 ` [PATCH edk2-platforms 2/8] Platform/Socionext/SynQuacer: expose build number as firmware version Ard Biesheuvel
@ 2017-12-12 18:17 ` Leif Lindholm
2017-12-12 18:20 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Leif Lindholm @ 2017-12-12 18:17 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, daniel.thompson, masami.hiramatsu
On Tue, Dec 12, 2017 at 10:38:01AM +0000, Ard Biesheuvel wrote:
> Expose the contents of the .DSC macro BUILD_NUMBER via the
> PCD gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString (if > 1),
> and as the FMP system firmware version (for capsule update).
>
> Also, set the firmware vendor to 'Linaro Enterprise Group', to
> distinguish our builds from builds by other parties.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 +++++++-
> Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
> Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 ++++--
> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 8 +++++++-
> Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
> Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 ++++--
> 6 files changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> index 8fbd7b2d908f..5ec26f9cdd34 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> @@ -26,6 +26,7 @@ [Defines]
> BUILD_TARGETS = DEBUG|RELEASE
> SKUID_IDENTIFIER = DEFAULT
> FLASH_DEFINITION = Platform/Socionext/DeveloperBox/DeveloperBox.fdf
> + BUILD_NUMBER = 1
>
> [BuildOptions]
> RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
> @@ -222,7 +223,7 @@ [PcdsFeatureFlag]
> gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
>
> [PcdsFixedAtBuild.common]
> - gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"Linaro"
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"Linaro Enterprise Group"
Actually, could you just delete this Pcd and let it fall back to the
default value of "EDK II"?
/
Leif
>
> # non-secure SRAM
> gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
> @@ -384,6 +385,11 @@ [PcdsFixedAtBuild.common]
> # set DIP switch DSW3-PIN1 (GPIO pin PD[0] on the SoC) to clear the varstore
> gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin|0
>
> +!if $(BUILD_NUMBER) > 1
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(BUILD_NUMBER)"
> +!endif
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
> +
> [PcdsPatchableInModule]
> gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0
> gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|0
> diff --git a/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf b/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
> index f5272c0f0d37..95a5e482a713 100644
> --- a/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
> +++ b/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
> @@ -38,6 +38,7 @@ [LibraryClasses]
>
> [FixedPcd]
> gArmTokenSpaceGuid.PcdFdSize
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision
>
> [Pcd]
> gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareImageDescriptor
> diff --git a/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc b/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc
> index bc47e696da7a..fb69de078313 100644
> --- a/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc
> +++ b/Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc
> @@ -21,8 +21,10 @@
> #define PACKAGE_VERSION 0xFFFFFFFF
> #define PACKAGE_VERSION_STRING L"Unknown"
>
> -#define CURRENT_FIRMWARE_VERSION 0x00000001
> -#define CURRENT_FIRMWARE_VERSION_STRING L"0x00000001"
> +#define __BUILD_STRING(x) L ## #x
> +#define BUILD_STRING(x) L"build #" __BUILD_STRING(x)
> +#define CURRENT_FIRMWARE_VERSION FixedPcdGet32 (PcdFirmwareRevision)
> +#define CURRENT_FIRMWARE_VERSION_STRING BUILD_STRING (FixedPcdGet32 (PcdFirmwareRevision))
> #define LOWEST_SUPPORTED_FIRMWARE_VERSION 0x00000001
>
> #define IMAGE_ID SIGNATURE_64('S', 'N', 'D', 'E', 'V', 'B', 'O', 'X')
> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> index 895d3b09fdc9..bc8ddd452d4b 100644
> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> @@ -26,6 +26,7 @@ [Defines]
> BUILD_TARGETS = DEBUG|RELEASE
> SKUID_IDENTIFIER = DEFAULT
> FLASH_DEFINITION = Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.fdf
> + BUILD_NUMBER = 1
>
> [BuildOptions]
> RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
> @@ -214,7 +215,7 @@ [PcdsFeatureFlag]
> gEfiMdeModulePkgTokenSpaceGuid.PcdSupportUpdateCapsuleReset|TRUE
>
> [PcdsFixedAtBuild.common]
> - gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"Linaro"
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"Linaro Enterprise Group"
>
> # non-secure SRAM
> gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
> @@ -372,6 +373,11 @@ [PcdsFixedAtBuild.common]
> # set DIP switch DSW3-PIN1 (GPIO pin PD[0] on the SoC) to clear the varstore
> gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin|0
>
> +!if $(BUILD_NUMBER) > 1
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(BUILD_NUMBER)"
> +!endif
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
> +
> [PcdsPatchableInModule]
> gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0
> gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|0
> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf b/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
> index f5272c0f0d37..95a5e482a713 100644
> --- a/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
> +++ b/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
> @@ -38,6 +38,7 @@ [LibraryClasses]
>
> [FixedPcd]
> gArmTokenSpaceGuid.PcdFdSize
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision
>
> [Pcd]
> gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareImageDescriptor
> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc b/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc
> index 3413f76f95c7..daf26c79dff1 100644
> --- a/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc
> +++ b/Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc
> @@ -21,8 +21,10 @@
> #define PACKAGE_VERSION 0xFFFFFFFF
> #define PACKAGE_VERSION_STRING L"Unknown"
>
> -#define CURRENT_FIRMWARE_VERSION 0x00000001
> -#define CURRENT_FIRMWARE_VERSION_STRING L"0x00000001"
> +#define __BUILD_STRING(x) L ## #x
> +#define BUILD_STRING(x) L"build #" __BUILD_STRING(x)
> +#define CURRENT_FIRMWARE_VERSION FixedPcdGet32 (PcdFirmwareRevision)
> +#define CURRENT_FIRMWARE_VERSION_STRING BUILD_STRING (FixedPcdGet32 (PcdFirmwareRevision))
> #define LOWEST_SUPPORTED_FIRMWARE_VERSION 0x00000001
>
> #define IMAGE_ID SIGNATURE_64('S', 'N', 'I', 'S', 'Y', 'N', 'Q', 'U')
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 2/8] Platform/Socionext/SynQuacer: expose build number as firmware version
2017-12-12 18:17 ` Leif Lindholm
@ 2017-12-12 18:20 ` Ard Biesheuvel
2017-12-12 18:24 ` Leif Lindholm
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 18:20 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Daniel Thompson, Masami Hiramatsu
On 12 December 2017 at 18:17, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Dec 12, 2017 at 10:38:01AM +0000, Ard Biesheuvel wrote:
>> Expose the contents of the .DSC macro BUILD_NUMBER via the
>> PCD gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString (if > 1),
>> and as the FMP system firmware version (for capsule update).
>>
>> Also, set the firmware vendor to 'Linaro Enterprise Group', to
>> distinguish our builds from builds by other parties.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 +++++++-
>> Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
>> Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 ++++--
>> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 8 +++++++-
>> Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
>> Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 ++++--
>> 6 files changed, 24 insertions(+), 6 deletions(-)
>>
>> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> index 8fbd7b2d908f..5ec26f9cdd34 100644
>> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> @@ -26,6 +26,7 @@ [Defines]
>> BUILD_TARGETS = DEBUG|RELEASE
>> SKUID_IDENTIFIER = DEFAULT
>> FLASH_DEFINITION = Platform/Socionext/DeveloperBox/DeveloperBox.fdf
>> + BUILD_NUMBER = 1
>>
>> [BuildOptions]
>> RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
>> @@ -222,7 +223,7 @@ [PcdsFeatureFlag]
>> gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
>>
>> [PcdsFixedAtBuild.common]
>> - gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"Linaro"
>> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"Linaro Enterprise Group"
>
> Actually, could you just delete this Pcd and let it fall back to the
> default value of "EDK II"?
>
Yes. But perhaps it makes sense to put $(FW_VENDOR) in there if it is
defined, so we can set it at build time? I would like to have a way to
put something in the FirmwareVendor field in the UEFI system table
that can help us identify firmware builds done by Linaro.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 2/8] Platform/Socionext/SynQuacer: expose build number as firmware version
2017-12-12 18:20 ` Ard Biesheuvel
@ 2017-12-12 18:24 ` Leif Lindholm
2017-12-12 18:28 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Leif Lindholm @ 2017-12-12 18:24 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Daniel Thompson, Masami Hiramatsu
On Tue, Dec 12, 2017 at 06:20:00PM +0000, Ard Biesheuvel wrote:
> On 12 December 2017 at 18:17, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Tue, Dec 12, 2017 at 10:38:01AM +0000, Ard Biesheuvel wrote:
> >> Expose the contents of the .DSC macro BUILD_NUMBER via the
> >> PCD gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString (if > 1),
> >> and as the FMP system firmware version (for capsule update).
> >>
> >> Also, set the firmware vendor to 'Linaro Enterprise Group', to
> >> distinguish our builds from builds by other parties.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 +++++++-
> >> Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
> >> Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 ++++--
> >> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 8 +++++++-
> >> Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
> >> Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 ++++--
> >> 6 files changed, 24 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> index 8fbd7b2d908f..5ec26f9cdd34 100644
> >> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> @@ -26,6 +26,7 @@ [Defines]
> >> BUILD_TARGETS = DEBUG|RELEASE
> >> SKUID_IDENTIFIER = DEFAULT
> >> FLASH_DEFINITION = Platform/Socionext/DeveloperBox/DeveloperBox.fdf
> >> + BUILD_NUMBER = 1
> >>
> >> [BuildOptions]
> >> RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
> >> @@ -222,7 +223,7 @@ [PcdsFeatureFlag]
> >> gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
> >>
> >> [PcdsFixedAtBuild.common]
> >> - gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"Linaro"
> >> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"Linaro Enterprise Group"
> >
> > Actually, could you just delete this Pcd and let it fall back to the
> > default value of "EDK II"?
> >
>
> Yes. But perhaps it makes sense to put $(FW_VENDOR) in there if it is
> defined, so we can set it at build time? I would like to have a way to
> put something in the FirmwareVendor field in the UEFI system table
> that can help us identify firmware builds done by Linaro.
Sure, that makes sense. Although we already have FIRMWARE_VER for
similar purposes, so something to match that name?
I.e. FIRMWARE_VENDOR.
/
Leif
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 2/8] Platform/Socionext/SynQuacer: expose build number as firmware version
2017-12-12 18:24 ` Leif Lindholm
@ 2017-12-12 18:28 ` Ard Biesheuvel
2017-12-12 18:33 ` Leif Lindholm
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 18:28 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Daniel Thompson, Masami Hiramatsu
On 12 December 2017 at 18:24, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Dec 12, 2017 at 06:20:00PM +0000, Ard Biesheuvel wrote:
>> On 12 December 2017 at 18:17, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> > On Tue, Dec 12, 2017 at 10:38:01AM +0000, Ard Biesheuvel wrote:
>> >> Expose the contents of the .DSC macro BUILD_NUMBER via the
>> >> PCD gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString (if > 1),
>> >> and as the FMP system firmware version (for capsule update).
>> >>
>> >> Also, set the firmware vendor to 'Linaro Enterprise Group', to
>> >> distinguish our builds from builds by other parties.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >> ---
>> >> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 +++++++-
>> >> Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
>> >> Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 ++++--
>> >> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 8 +++++++-
>> >> Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
>> >> Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 ++++--
>> >> 6 files changed, 24 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> >> index 8fbd7b2d908f..5ec26f9cdd34 100644
>> >> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> >> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> >> @@ -26,6 +26,7 @@ [Defines]
>> >> BUILD_TARGETS = DEBUG|RELEASE
>> >> SKUID_IDENTIFIER = DEFAULT
>> >> FLASH_DEFINITION = Platform/Socionext/DeveloperBox/DeveloperBox.fdf
>> >> + BUILD_NUMBER = 1
>> >>
>> >> [BuildOptions]
>> >> RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
>> >> @@ -222,7 +223,7 @@ [PcdsFeatureFlag]
>> >> gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
>> >>
>> >> [PcdsFixedAtBuild.common]
>> >> - gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"Linaro"
>> >> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"Linaro Enterprise Group"
>> >
>> > Actually, could you just delete this Pcd and let it fall back to the
>> > default value of "EDK II"?
>> >
>>
>> Yes. But perhaps it makes sense to put $(FW_VENDOR) in there if it is
>> defined, so we can set it at build time? I would like to have a way to
>> put something in the FirmwareVendor field in the UEFI system table
>> that can help us identify firmware builds done by Linaro.
>
> Sure, that makes sense. Although we already have FIRMWARE_VER for
> similar purposes, so something to match that name?
> I.e. FIRMWARE_VENDOR.
>
Yes, so replace the hunk above with
@@ -222,7 +227,9 @@ [PcdsFeatureFlag]
gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
[PcdsFixedAtBuild.common]
- gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"Linaro"
+!ifdef $(FIRMWARE_VENDOR)
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"$(FIRMWARE_VENDOR)"
+!endif
# non-secure SRAM
gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 2/8] Platform/Socionext/SynQuacer: expose build number as firmware version
2017-12-12 18:28 ` Ard Biesheuvel
@ 2017-12-12 18:33 ` Leif Lindholm
0 siblings, 0 replies; 27+ messages in thread
From: Leif Lindholm @ 2017-12-12 18:33 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Daniel Thompson, Masami Hiramatsu
On Tue, Dec 12, 2017 at 06:28:31PM +0000, Ard Biesheuvel wrote:
> On 12 December 2017 at 18:24, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Tue, Dec 12, 2017 at 06:20:00PM +0000, Ard Biesheuvel wrote:
> >> On 12 December 2017 at 18:17, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >> > On Tue, Dec 12, 2017 at 10:38:01AM +0000, Ard Biesheuvel wrote:
> >> >> Expose the contents of the .DSC macro BUILD_NUMBER via the
> >> >> PCD gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString (if > 1),
> >> >> and as the FMP system firmware version (for capsule update).
> >> >>
> >> >> Also, set the firmware vendor to 'Linaro Enterprise Group', to
> >> >> distinguish our builds from builds by other parties.
> >> >>
> >> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> >> ---
> >> >> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 +++++++-
> >> >> Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
> >> >> Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 ++++--
> >> >> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 8 +++++++-
> >> >> Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
> >> >> Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 ++++--
> >> >> 6 files changed, 24 insertions(+), 6 deletions(-)
> >> >>
> >> >> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> >> index 8fbd7b2d908f..5ec26f9cdd34 100644
> >> >> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> >> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> >> @@ -26,6 +26,7 @@ [Defines]
> >> >> BUILD_TARGETS = DEBUG|RELEASE
> >> >> SKUID_IDENTIFIER = DEFAULT
> >> >> FLASH_DEFINITION = Platform/Socionext/DeveloperBox/DeveloperBox.fdf
> >> >> + BUILD_NUMBER = 1
> >> >>
> >> >> [BuildOptions]
> >> >> RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
> >> >> @@ -222,7 +223,7 @@ [PcdsFeatureFlag]
> >> >> gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
> >> >>
> >> >> [PcdsFixedAtBuild.common]
> >> >> - gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"Linaro"
> >> >> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"Linaro Enterprise Group"
> >> >
> >> > Actually, could you just delete this Pcd and let it fall back to the
> >> > default value of "EDK II"?
> >> >
> >>
> >> Yes. But perhaps it makes sense to put $(FW_VENDOR) in there if it is
> >> defined, so we can set it at build time? I would like to have a way to
> >> put something in the FirmwareVendor field in the UEFI system table
> >> that can help us identify firmware builds done by Linaro.
> >
> > Sure, that makes sense. Although we already have FIRMWARE_VER for
> > similar purposes, so something to match that name?
> > I.e. FIRMWARE_VENDOR.
> >
>
> Yes, so replace the hunk above with
>
> @@ -222,7 +227,9 @@ [PcdsFeatureFlag]
> gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
>
> [PcdsFixedAtBuild.common]
> - gArmPlatformTokenSpaceGuid.PcdFirmwareVendor|"Linaro"
> +!ifdef $(FIRMWARE_VENDOR)
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"$(FIRMWARE_VENDOR)"
> +!endif
>
> # non-secure SRAM
> gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
>
> ?
Works for me:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH edk2-platforms 3/8] Silicon/SynQuacerPciHostBridgeLib: stall for 150 ms during PERST#
2017-12-12 10:37 [PATCH edk2-platforms 0/8] SynQuacer updates Ard Biesheuvel
2017-12-12 10:38 ` [PATCH edk2-platforms 1/8] Silicon/SynQuacer: enable CPU idle states in device tree Ard Biesheuvel
2017-12-12 10:38 ` [PATCH edk2-platforms 2/8] Platform/Socionext/SynQuacer: expose build number as firmware version Ard Biesheuvel
@ 2017-12-12 10:38 ` Ard Biesheuvel
2017-12-12 17:24 ` Leif Lindholm
2017-12-12 10:38 ` [PATCH edk2-platforms 4/8] Silicon/SynQuacerPciHostBridgeLib: enable RCs based on PCD setting Ard Biesheuvel
` (5 subsequent siblings)
8 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 10:38 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, daniel.thompson, masami.hiramatsu, Ard Biesheuvel
Attempt to adhere more closely to the PCIe spec by ensuring that PERST#
remains asserted for at least 100 ms. Give it a good margin, and delay
for 150 ms; the additional boot time delay is not going to be noticeable
by anyone anyway.
So split the init routine in a pre and post part, and put the delay in
the middle so we only need to do it once.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf | 1 +
Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 46 +++++++++++++++-----
2 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf
index 08484f4f8b1a..5d87727c73ba 100644
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf
@@ -45,6 +45,7 @@ [LibraryClasses]
DebugLib
DevicePathLib
MemoryAllocationLib
+ UefiBootServicesTableLib
[FixedPcd]
gArmTokenSpaceGuid.PcdPciIoTranslation
diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
index e63b3a4bb23b..3da94945f96a 100644
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
@@ -19,6 +19,7 @@
#include <Library/DebugLib.h>
#include <Library/IoLib.h>
#include <Library/PciHostBridgeLib.h>
+#include <Library/UefiBootServicesTableLib.h>
#include <Platform/Pcie.h>
#include <Protocol/PciHostBridgeResourceAllocation.h>
@@ -176,6 +177,8 @@ SnPcieSetData (
}
MmioWrite32 (Base + Offset, Data);
+
+ ArmDataMemoryBarrier ();
}
STATIC
@@ -194,6 +197,8 @@ SnPcieReadData (
Shift++;
}
+ ArmDataMemoryBarrier ();
+
return (MmioRead32 (Base + Offset) >> Shift) & Mask;
}
@@ -219,12 +224,8 @@ SnDbiRoWrEn (
STATIC
VOID
-PciInitController (
- IN EFI_PHYSICAL_ADDRESS ExsBase,
- IN EFI_PHYSICAL_ADDRESS DbiBase,
- IN EFI_PHYSICAL_ADDRESS ConfigBase,
- IN EFI_PHYSICAL_ADDRESS IoMemBase,
- IN CONST PCI_ROOT_BRIDGE *RootBridge
+PciInitControllerPre (
+ IN EFI_PHYSICAL_ADDRESS ExsBase
)
{
SnPcieSetData (ExsBase, EM_SELECT, PRE_DET_STT_SEL, 0);
@@ -256,7 +257,18 @@ PciInitController (
// 3: Set device_type (RC)
SnPcieSetData (ExsBase, CORE_CONTROL, DEVICE_TYPE, 4);
+}
+STATIC
+VOID
+PciInitControllerPost (
+ IN EFI_PHYSICAL_ADDRESS ExsBase,
+ IN EFI_PHYSICAL_ADDRESS DbiBase,
+ IN EFI_PHYSICAL_ADDRESS ConfigBase,
+ IN EFI_PHYSICAL_ADDRESS IoMemBase,
+ IN CONST PCI_ROOT_BRIDGE *RootBridge
+ )
+{
// 4: Set Bifurcation 1=disable 4=able
// 5: Supply Reference (It has executed)
// 6: Wait for 10usec (Reference Clocks is stable)
@@ -389,11 +401,23 @@ SynQuacerPciHostBridgeLibConstructor (
}
for (Idx = 0; Idx < Count; Idx++) {
- PciInitController (mBaseAddresses[Idx].ExsBase,
- mBaseAddresses[Idx].DbiBase,
- mBaseAddresses[Idx].ConfigBase,
- mBaseAddresses[Idx].IoMemBase,
- &RootBridges[Idx]);
+ PciInitControllerPre (mBaseAddresses[Idx].ExsBase);
+ }
+
+ //
+ // The PCIe spec requires that PERST# is asserted for at least 100 ms after
+ // the power and clocks have become stable. So let's give a bit or margin,
+ // and stall for 150 ms between asserting PERST# on both controllers and
+ // de-asserting it again.
+ //
+ gBS->Stall (150 * 1000);
+
+ for (Idx = 0; Idx < Count; Idx++) {
+ PciInitControllerPost (mBaseAddresses[Idx].ExsBase,
+ mBaseAddresses[Idx].DbiBase,
+ mBaseAddresses[Idx].ConfigBase,
+ mBaseAddresses[Idx].IoMemBase,
+ &RootBridges[Idx]);
}
return EFI_SUCCESS;
--
2.11.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 3/8] Silicon/SynQuacerPciHostBridgeLib: stall for 150 ms during PERST#
2017-12-12 10:38 ` [PATCH edk2-platforms 3/8] Silicon/SynQuacerPciHostBridgeLib: stall for 150 ms during PERST# Ard Biesheuvel
@ 2017-12-12 17:24 ` Leif Lindholm
0 siblings, 0 replies; 27+ messages in thread
From: Leif Lindholm @ 2017-12-12 17:24 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, daniel.thompson, masami.hiramatsu
On Tue, Dec 12, 2017 at 10:38:02AM +0000, Ard Biesheuvel wrote:
> Attempt to adhere more closely to the PCIe spec by ensuring that PERST#
> remains asserted for at least 100 ms. Give it a good margin, and delay
> for 150 ms; the additional boot time delay is not going to be noticeable
> by anyone anyway.
>
> So split the init routine in a pre and post part, and put the delay in
> the middle so we only need to do it once.
>
This patch also adds two missing memory barriers.
Please add this to commit message. If you do:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf | 1 +
> Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 46 +++++++++++++++-----
> 2 files changed, 36 insertions(+), 11 deletions(-)
>
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf
> index 08484f4f8b1a..5d87727c73ba 100644
> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf
> @@ -45,6 +45,7 @@ [LibraryClasses]
> DebugLib
> DevicePathLib
> MemoryAllocationLib
> + UefiBootServicesTableLib
>
> [FixedPcd]
> gArmTokenSpaceGuid.PcdPciIoTranslation
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> index e63b3a4bb23b..3da94945f96a 100644
> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> @@ -19,6 +19,7 @@
> #include <Library/DebugLib.h>
> #include <Library/IoLib.h>
> #include <Library/PciHostBridgeLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> #include <Platform/Pcie.h>
> #include <Protocol/PciHostBridgeResourceAllocation.h>
>
> @@ -176,6 +177,8 @@ SnPcieSetData (
> }
>
> MmioWrite32 (Base + Offset, Data);
> +
> + ArmDataMemoryBarrier ();
> }
>
> STATIC
> @@ -194,6 +197,8 @@ SnPcieReadData (
> Shift++;
> }
>
> + ArmDataMemoryBarrier ();
> +
> return (MmioRead32 (Base + Offset) >> Shift) & Mask;
> }
>
> @@ -219,12 +224,8 @@ SnDbiRoWrEn (
>
> STATIC
> VOID
> -PciInitController (
> - IN EFI_PHYSICAL_ADDRESS ExsBase,
> - IN EFI_PHYSICAL_ADDRESS DbiBase,
> - IN EFI_PHYSICAL_ADDRESS ConfigBase,
> - IN EFI_PHYSICAL_ADDRESS IoMemBase,
> - IN CONST PCI_ROOT_BRIDGE *RootBridge
> +PciInitControllerPre (
> + IN EFI_PHYSICAL_ADDRESS ExsBase
> )
> {
> SnPcieSetData (ExsBase, EM_SELECT, PRE_DET_STT_SEL, 0);
> @@ -256,7 +257,18 @@ PciInitController (
>
> // 3: Set device_type (RC)
> SnPcieSetData (ExsBase, CORE_CONTROL, DEVICE_TYPE, 4);
> +}
>
> +STATIC
> +VOID
> +PciInitControllerPost (
> + IN EFI_PHYSICAL_ADDRESS ExsBase,
> + IN EFI_PHYSICAL_ADDRESS DbiBase,
> + IN EFI_PHYSICAL_ADDRESS ConfigBase,
> + IN EFI_PHYSICAL_ADDRESS IoMemBase,
> + IN CONST PCI_ROOT_BRIDGE *RootBridge
> + )
> +{
> // 4: Set Bifurcation 1=disable 4=able
> // 5: Supply Reference (It has executed)
> // 6: Wait for 10usec (Reference Clocks is stable)
> @@ -389,11 +401,23 @@ SynQuacerPciHostBridgeLibConstructor (
> }
>
> for (Idx = 0; Idx < Count; Idx++) {
> - PciInitController (mBaseAddresses[Idx].ExsBase,
> - mBaseAddresses[Idx].DbiBase,
> - mBaseAddresses[Idx].ConfigBase,
> - mBaseAddresses[Idx].IoMemBase,
> - &RootBridges[Idx]);
> + PciInitControllerPre (mBaseAddresses[Idx].ExsBase);
> + }
> +
> + //
> + // The PCIe spec requires that PERST# is asserted for at least 100 ms after
> + // the power and clocks have become stable. So let's give a bit or margin,
> + // and stall for 150 ms between asserting PERST# on both controllers and
> + // de-asserting it again.
> + //
> + gBS->Stall (150 * 1000);
> +
> + for (Idx = 0; Idx < Count; Idx++) {
> + PciInitControllerPost (mBaseAddresses[Idx].ExsBase,
> + mBaseAddresses[Idx].DbiBase,
> + mBaseAddresses[Idx].ConfigBase,
> + mBaseAddresses[Idx].IoMemBase,
> + &RootBridges[Idx]);
> }
>
> return EFI_SUCCESS;
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH edk2-platforms 4/8] Silicon/SynQuacerPciHostBridgeLib: enable RCs based on PCD setting
2017-12-12 10:37 [PATCH edk2-platforms 0/8] SynQuacer updates Ard Biesheuvel
` (2 preceding siblings ...)
2017-12-12 10:38 ` [PATCH edk2-platforms 3/8] Silicon/SynQuacerPciHostBridgeLib: stall for 150 ms during PERST# Ard Biesheuvel
@ 2017-12-12 10:38 ` Ard Biesheuvel
2017-12-12 10:38 ` [PATCH edk2-platforms 5/8] Silicon/SynQuacer: disable PCI RC #0 DT node if disabled Ard Biesheuvel
` (4 subsequent siblings)
8 siblings, 0 replies; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 10:38 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, daniel.thompson, masami.hiramatsu, Ard Biesheuvel
In order to accommodate the EVB, whose PCIe RC #0 should not be touched
by software if no card is inserted, add a PCD that tells the PCIe driver
code which RCs should be initialized and exposed to the PCI host bridge
driver.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c | 19 ++++++++++---
Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf | 3 ++
Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 30 +++++++++-----------
Silicon/Socionext/SynQuacer/SynQuacer.dec | 4 +++
4 files changed, 36 insertions(+), 20 deletions(-)
diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
index 42cdce24b2c4..596862baf469 100644
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c
@@ -92,7 +92,7 @@ CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = {
#define PCI_ALLOCATION_ATTRIBUTES EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM
#endif
-STATIC PCI_ROOT_BRIDGE mPciRootBridges[] = {
+PCI_ROOT_BRIDGE mPciRootBridges[] = {
{
0, // Segment
0, // Supports
@@ -149,9 +149,20 @@ PciHostBridgeGetRootBridges (
OUT UINTN *Count
)
{
- *Count = ARRAY_SIZE (mPciRootBridges);
-
- return mPciRootBridges;
+ switch (PcdGet8 (PcdPcieEnableMask)) {
+ default:
+ ASSERT (FALSE);
+ case 0x0:
+ *Count = 0;
+ return NULL;
+ case 0x1:
+ case 0x2:
+ *Count = 1;
+ return &mPciRootBridges[PcdGet8 (PcdPcieEnableMask) - 1];
+ case 0x3:
+ *Count = ARRAY_SIZE (mPciRootBridges);
+ return mPciRootBridges;
+ }
}
/**
diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf
index 5d87727c73ba..27fcba034418 100644
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf
@@ -49,3 +49,6 @@ [LibraryClasses]
[FixedPcd]
gArmTokenSpaceGuid.PcdPciIoTranslation
+
+[Pcd]
+ gSynQuacerTokenSpaceGuid.PcdPcieEnableMask
diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
index 3da94945f96a..bea40e3dcfe8 100644
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
@@ -120,6 +120,8 @@
#define MISC_CONTROL_1_OFF 0x8BC
#define DBI_RO_WR_EN BIT0
+extern PCI_ROOT_BRIDGE mPciRootBridges[];
+
STATIC
VOID
ConfigureWindow (
@@ -390,18 +392,12 @@ SynQuacerPciHostBridgeLibConstructor (
IN EFI_SYSTEM_TABLE *SystemTable
)
{
- PCI_ROOT_BRIDGE *RootBridges;
- UINTN Count;
UINTN Idx;
- RootBridges = PciHostBridgeGetRootBridges (&Count);
- ASSERT (Count == ARRAY_SIZE(mBaseAddresses));
- if (Count != ARRAY_SIZE(mBaseAddresses)) {
- return EFI_INVALID_PARAMETER;
- }
-
- for (Idx = 0; Idx < Count; Idx++) {
- PciInitControllerPre (mBaseAddresses[Idx].ExsBase);
+ for (Idx = 0; Idx < ARRAY_SIZE (mBaseAddresses); Idx++) {
+ if (PcdGet8 (PcdPcieEnableMask) & (1 << Idx)) {
+ PciInitControllerPre (mBaseAddresses[Idx].ExsBase);
+ }
}
//
@@ -412,12 +408,14 @@ SynQuacerPciHostBridgeLibConstructor (
//
gBS->Stall (150 * 1000);
- for (Idx = 0; Idx < Count; Idx++) {
- PciInitControllerPost (mBaseAddresses[Idx].ExsBase,
- mBaseAddresses[Idx].DbiBase,
- mBaseAddresses[Idx].ConfigBase,
- mBaseAddresses[Idx].IoMemBase,
- &RootBridges[Idx]);
+ for (Idx = 0; Idx < ARRAY_SIZE (mBaseAddresses); Idx++) {
+ if (PcdGet8 (PcdPcieEnableMask) & (1 << Idx)) {
+ PciInitControllerPost (mBaseAddresses[Idx].ExsBase,
+ mBaseAddresses[Idx].DbiBase,
+ mBaseAddresses[Idx].ConfigBase,
+ mBaseAddresses[Idx].IoMemBase,
+ &mPciRootBridges[Idx]);
+ }
}
return EFI_SUCCESS;
diff --git a/Silicon/Socionext/SynQuacer/SynQuacer.dec b/Silicon/Socionext/SynQuacer/SynQuacer.dec
index 02dd6ac417f9..2e18cb33346d 100644
--- a/Silicon/Socionext/SynQuacer/SynQuacer.dec
+++ b/Silicon/Socionext/SynQuacer/SynQuacer.dec
@@ -38,3 +38,7 @@ [PcdsFixedAtBuild]
gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin|0xFF|UINT8|0x00000004
gSynQuacerTokenSpaceGuid.PcdI2cReferenceClock|62500000|UINT32|0x00000005
+
+[PcdsPatchableInModule, PcdsDynamic]
+ # Enable both RC #0 and RC #1 by default
+ gSynQuacerTokenSpaceGuid.PcdPcieEnableMask|0x3|UINT8|0x00000007
--
2.11.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH edk2-platforms 5/8] Silicon/SynQuacer: disable PCI RC #0 DT node if disabled
2017-12-12 10:37 [PATCH edk2-platforms 0/8] SynQuacer updates Ard Biesheuvel
` (3 preceding siblings ...)
2017-12-12 10:38 ` [PATCH edk2-platforms 4/8] Silicon/SynQuacerPciHostBridgeLib: enable RCs based on PCD setting Ard Biesheuvel
@ 2017-12-12 10:38 ` Ard Biesheuvel
2017-12-12 14:54 ` Ard Biesheuvel
2017-12-12 17:32 ` Leif Lindholm
2017-12-12 10:38 ` [PATCH edk2-platforms 6/8] Silicon/SynQuacerEvalBoard: enable PCI #0 only when card is detected Ard Biesheuvel
` (3 subsequent siblings)
8 siblings, 2 replies; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 10:38 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, daniel.thompson, masami.hiramatsu, Ard Biesheuvel
If PCIe RC #0 is not enabled (due to the fact that the slot is not
populated), set its DT node 'status' property to 'disabled' so that
the OS will not attempt to attach to it.
This means we will need to switch from the default DtPlatformDtbLoaderLib
to a special one for our platform.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 +-
Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 3 +-
Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c | 94 ++++++++++++++++++++
Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 42 +++++++++
4 files changed, 141 insertions(+), 6 deletions(-)
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
index 5ec26f9cdd34..80728fedbc20 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
@@ -160,7 +160,8 @@ [LibraryClasses.common.DXE_CORE]
PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
[LibraryClasses.common.DXE_DRIVER]
- DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
+ DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
+ FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
@@ -611,10 +612,7 @@ [Components.common]
#
# Console preference selection
#
- EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf {
- <LibraryClasses>
- FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
- }
+ EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf
#
# DT support
diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
index bc8ddd452d4b..c71425664bdc 100644
--- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
+++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
@@ -159,7 +159,8 @@ [LibraryClasses.common.DXE_CORE]
PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
[LibraryClasses.common.DXE_DRIVER]
- DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
+ DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
+ FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
new file mode 100644
index 000000000000..a93a6027e64d
--- /dev/null
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
@@ -0,0 +1,94 @@
+/** @file
+*
+* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
+*
+* 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 <PiDxe.h>
+
+#include <libfdt.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesLib.h>
+#include <Library/MemoryAllocationLib.h>
+
+#define DTB_PADDING 64
+
+STATIC
+VOID
+DisableDtNode (
+ IN VOID *Dtb,
+ IN CONST CHAR8 *NodePath
+ )
+{
+ INT32 Node;
+ INT32 Rc;
+
+ Node = fdt_path_offset (Dtb, NodePath);
+ if (Node < 0) {
+ DEBUG ((DEBUG_ERROR, "%a: failed to locate DT path '%a': %a\n",
+ __FUNCTION__, NodePath, fdt_strerror (Node)));
+ return;
+ }
+ Rc = fdt_setprop_string (Dtb, Node, "status", "disabled");
+ if (Rc < 0) {
+ DEBUG ((DEBUG_ERROR, "%a: failed to set status to 'disabled' on '%a': %a\n",
+ __FUNCTION__, NodePath, fdt_strerror (Rc)));
+ }
+}
+
+/**
+ Return a pool allocated copy of the DTB image that is appropriate for
+ booting the current platform via DT.
+
+ @param[out] Dtb Pointer to the DTB copy
+ @param[out] DtbSize Size of the DTB copy
+
+ @retval EFI_SUCCESS Operation completed successfully
+ @retval EFI_NOT_FOUND No suitable DTB image could be located
+ @retval EFI_OUT_OF_RESOURCES No pool memory available
+
+**/
+EFI_STATUS
+EFIAPI
+DtPlatformLoadDtb (
+ OUT VOID **Dtb,
+ OUT UINTN *DtbSize
+ )
+{
+ EFI_STATUS Status;
+ VOID *OrigDtb;
+ VOID *CopyDtb;
+ UINTN OrigDtbSize;
+
+ Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
+ EFI_SECTION_RAW, 0, &OrigDtb, &OrigDtbSize);
+ if (EFI_ERROR (Status)) {
+ return EFI_NOT_FOUND;
+ }
+
+ CopyDtb = AllocateCopyPool (OrigDtbSize + DTB_PADDING, OrigDtb);
+ if (CopyDtb == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ if (!(PcdGet8 (PcdPcieEnableMask) & BIT0)) {
+ DisableDtNode (CopyDtb, "/pcie@60000000");
+ }
+ if (!(PcdGet8 (PcdPcieEnableMask) & BIT1)) {
+ DisableDtNode (CopyDtb, "/pcie@70000000");
+ }
+
+ *Dtb = CopyDtb;
+ *DtbSize = OrigDtbSize + DTB_PADDING;
+
+ return EFI_SUCCESS;
+}
diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
new file mode 100644
index 000000000000..e1f564f73078
--- /dev/null
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
@@ -0,0 +1,42 @@
+/** @file
+*
+* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
+*
+* 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 = 0x0001001A
+ BASE_NAME = SynQuacerDtbLoaderLib
+ FILE_GUID = 59df69c4-8724-4e49-8974-d0691364338c
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = DtPlatformDtbLoaderLib|DXE_DRIVER
+
+[Sources]
+ SynQuacerDtbLoaderLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ EmbeddedPkg/EmbeddedPkg.dec
+ Silicon/Socionext/SynQuacer/SynQuacer.dec
+
+[LibraryClasses]
+ BaseLib
+ DebugLib
+ DxeServicesLib
+ FdtLib
+ MemoryAllocationLib
+
+[Pcd]
+ gSynQuacerTokenSpaceGuid.PcdPcieEnableMask
+
+[Guids]
+ gDtPlatformDefaultDtbFileGuid
--
2.11.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 5/8] Silicon/SynQuacer: disable PCI RC #0 DT node if disabled
2017-12-12 10:38 ` [PATCH edk2-platforms 5/8] Silicon/SynQuacer: disable PCI RC #0 DT node if disabled Ard Biesheuvel
@ 2017-12-12 14:54 ` Ard Biesheuvel
2017-12-12 17:32 ` Leif Lindholm
1 sibling, 0 replies; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 14:54 UTC (permalink / raw)
To: edk2-devel@lists.01.org
Cc: Leif Lindholm, Daniel Thompson, Masami Hiramatsu, Ard Biesheuvel
On 12 December 2017 at 10:38, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> If PCIe RC #0 is not enabled (due to the fact that the slot is not
> populated), set its DT node 'status' property to 'disabled' so that
> the OS will not attempt to attach to it.
>
> This means we will need to switch from the default DtPlatformDtbLoaderLib
> to a special one for our platform.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 +-
> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 3 +-
> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c | 94 ++++++++++++++++++++
> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 42 +++++++++
> 4 files changed, 141 insertions(+), 6 deletions(-)
>
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> index 5ec26f9cdd34..80728fedbc20 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> @@ -160,7 +160,8 @@ [LibraryClasses.common.DXE_CORE]
> PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>
> [LibraryClasses.common.DXE_DRIVER]
> - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> + DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> + FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
>
> @@ -611,10 +612,7 @@ [Components.common]
> #
> # Console preference selection
> #
> - EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf {
> - <LibraryClasses>
> - FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> - }
> + EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf
>
> #
> # DT support
> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> index bc8ddd452d4b..c71425664bdc 100644
> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> @@ -159,7 +159,8 @@ [LibraryClasses.common.DXE_CORE]
> PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>
> [LibraryClasses.common.DXE_DRIVER]
> - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> + DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> + FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
>
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> new file mode 100644
> index 000000000000..a93a6027e64d
> --- /dev/null
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> @@ -0,0 +1,94 @@
> +/** @file
> +*
> +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> +*
> +* 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 <PiDxe.h>
> +
> +#include <libfdt.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DxeServicesLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +
> +#define DTB_PADDING 64
> +
> +STATIC
> +VOID
> +DisableDtNode (
> + IN VOID *Dtb,
> + IN CONST CHAR8 *NodePath
> + )
> +{
> + INT32 Node;
> + INT32 Rc;
> +
> + Node = fdt_path_offset (Dtb, NodePath);
> + if (Node < 0) {
> + DEBUG ((DEBUG_ERROR, "%a: failed to locate DT path '%a': %a\n",
> + __FUNCTION__, NodePath, fdt_strerror (Node)));
> + return;
> + }
> + Rc = fdt_setprop_string (Dtb, Node, "status", "disabled");
> + if (Rc < 0) {
> + DEBUG ((DEBUG_ERROR, "%a: failed to set status to 'disabled' on '%a': %a\n",
> + __FUNCTION__, NodePath, fdt_strerror (Rc)));
> + }
> +}
> +
> +/**
> + Return a pool allocated copy of the DTB image that is appropriate for
> + booting the current platform via DT.
> +
> + @param[out] Dtb Pointer to the DTB copy
> + @param[out] DtbSize Size of the DTB copy
> +
> + @retval EFI_SUCCESS Operation completed successfully
> + @retval EFI_NOT_FOUND No suitable DTB image could be located
> + @retval EFI_OUT_OF_RESOURCES No pool memory available
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +DtPlatformLoadDtb (
> + OUT VOID **Dtb,
> + OUT UINTN *DtbSize
> + )
> +{
> + EFI_STATUS Status;
> + VOID *OrigDtb;
> + VOID *CopyDtb;
> + UINTN OrigDtbSize;
> +
> + Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
> + EFI_SECTION_RAW, 0, &OrigDtb, &OrigDtbSize);
> + if (EFI_ERROR (Status)) {
> + return EFI_NOT_FOUND;
> + }
> +
> + CopyDtb = AllocateCopyPool (OrigDtbSize + DTB_PADDING, OrigDtb);
> + if (CopyDtb == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + if (!(PcdGet8 (PcdPcieEnableMask) & BIT0)) {
> + DisableDtNode (CopyDtb, "/pcie@60000000");
> + }
> + if (!(PcdGet8 (PcdPcieEnableMask) & BIT1)) {
> + DisableDtNode (CopyDtb, "/pcie@70000000");
> + }
> +
> + *Dtb = CopyDtb;
> + *DtbSize = OrigDtbSize + DTB_PADDING;
> +
> + return EFI_SUCCESS;
> +}
This needs a tweak: I forgot to incorporate the fdt_copy_into() call
which grows the DTB image before adding properties. The following
needs to be applied on top.
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
@@ -68,6 +68,8 @@ DtPlatformLoadDtb (
VOID *OrigDtb;
VOID *CopyDtb;
UINTN OrigDtbSize;
+ UINTN CopyDtbSize;
+ INT32 Rc;
Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
EFI_SECTION_RAW, 0, &OrigDtb, &OrigDtbSize);
@@ -75,11 +77,19 @@ DtPlatformLoadDtb (
return EFI_NOT_FOUND;
}
- CopyDtb = AllocateCopyPool (OrigDtbSize + DTB_PADDING, OrigDtb);
+ CopyDtbSize = OrigDtbSize + DTB_PADDING;
+ CopyDtb = AllocatePool (CopyDtbSize);
if (CopyDtb == NULL) {
return EFI_OUT_OF_RESOURCES;
}
+ Rc = fdt_open_into (OrigDtb, CopyDtb, CopyDtbSize);
+ if (Rc < 0) {
+ DEBUG ((DEBUG_ERROR, "%a: fdt_open_into () failed: %a\n", __FUNCTION__,
+ fdt_strerror (Rc)));
+ return EFI_NOT_FOUND;
+ }
+
if (!(PcdGet8 (PcdPcieEnableMask) & BIT0)) {
DisableDtNode (CopyDtb, "/pcie@60000000");
}
@@ -88,7 +98,7 @@ DtPlatformLoadDtb (
}
*Dtb = CopyDtb;
- *DtbSize = OrigDtbSize + DTB_PADDING;
+ *DtbSize = CopyDtbSize;
return EFI_SUCCESS;
}
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 5/8] Silicon/SynQuacer: disable PCI RC #0 DT node if disabled
2017-12-12 10:38 ` [PATCH edk2-platforms 5/8] Silicon/SynQuacer: disable PCI RC #0 DT node if disabled Ard Biesheuvel
2017-12-12 14:54 ` Ard Biesheuvel
@ 2017-12-12 17:32 ` Leif Lindholm
2017-12-12 17:35 ` Ard Biesheuvel
1 sibling, 1 reply; 27+ messages in thread
From: Leif Lindholm @ 2017-12-12 17:32 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, daniel.thompson, masami.hiramatsu
Suggested subject tweak:
Silicon/SynQuacer: disable PCI RC #0 DT node if disabled ->
Silicon/SynQuacer: disable PCI RC DT node if RC disabled.
The code below will disable either #0 or #1 node if not enabled by
Pcd.
On Tue, Dec 12, 2017 at 10:38:04AM +0000, Ard Biesheuvel wrote:
> If PCIe RC #0 is not enabled (due to the fact that the slot is not
> populated), set its DT node 'status' property to 'disabled' so that
> the OS will not attempt to attach to it.
>
> This means we will need to switch from the default DtPlatformDtbLoaderLib
> to a special one for our platform.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 +-
> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 3 +-
> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c | 94 ++++++++++++++++++++
> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 42 +++++++++
> 4 files changed, 141 insertions(+), 6 deletions(-)
>
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> index 5ec26f9cdd34..80728fedbc20 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> @@ -160,7 +160,8 @@ [LibraryClasses.common.DXE_CORE]
> PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>
> [LibraryClasses.common.DXE_DRIVER]
> - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> + DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> + FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
>
> @@ -611,10 +612,7 @@ [Components.common]
> #
> # Console preference selection
> #
> - EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf {
> - <LibraryClasses>
> - FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> - }
> + EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf
>
> #
> # DT support
> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> index bc8ddd452d4b..c71425664bdc 100644
> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> @@ -159,7 +159,8 @@ [LibraryClasses.common.DXE_CORE]
> PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>
> [LibraryClasses.common.DXE_DRIVER]
> - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> + DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> + FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
>
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> new file mode 100644
> index 000000000000..a93a6027e64d
> --- /dev/null
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> @@ -0,0 +1,94 @@
> +/** @file
> +*
> +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> +*
> +* 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 <PiDxe.h>
> +
> +#include <libfdt.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DxeServicesLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +
> +#define DTB_PADDING 64
If there isn't a simplish way of determining this value
arithmetically, can you add a comment explaining why 64 is sufficient?
/
Leif
> +
> +STATIC
> +VOID
> +DisableDtNode (
> + IN VOID *Dtb,
> + IN CONST CHAR8 *NodePath
> + )
> +{
> + INT32 Node;
> + INT32 Rc;
> +
> + Node = fdt_path_offset (Dtb, NodePath);
> + if (Node < 0) {
> + DEBUG ((DEBUG_ERROR, "%a: failed to locate DT path '%a': %a\n",
> + __FUNCTION__, NodePath, fdt_strerror (Node)));
> + return;
> + }
> + Rc = fdt_setprop_string (Dtb, Node, "status", "disabled");
> + if (Rc < 0) {
> + DEBUG ((DEBUG_ERROR, "%a: failed to set status to 'disabled' on '%a': %a\n",
> + __FUNCTION__, NodePath, fdt_strerror (Rc)));
> + }
> +}
> +
> +/**
> + Return a pool allocated copy of the DTB image that is appropriate for
> + booting the current platform via DT.
> +
> + @param[out] Dtb Pointer to the DTB copy
> + @param[out] DtbSize Size of the DTB copy
> +
> + @retval EFI_SUCCESS Operation completed successfully
> + @retval EFI_NOT_FOUND No suitable DTB image could be located
> + @retval EFI_OUT_OF_RESOURCES No pool memory available
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +DtPlatformLoadDtb (
> + OUT VOID **Dtb,
> + OUT UINTN *DtbSize
> + )
> +{
> + EFI_STATUS Status;
> + VOID *OrigDtb;
> + VOID *CopyDtb;
> + UINTN OrigDtbSize;
> +
> + Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
> + EFI_SECTION_RAW, 0, &OrigDtb, &OrigDtbSize);
> + if (EFI_ERROR (Status)) {
> + return EFI_NOT_FOUND;
> + }
> +
> + CopyDtb = AllocateCopyPool (OrigDtbSize + DTB_PADDING, OrigDtb);
> + if (CopyDtb == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + if (!(PcdGet8 (PcdPcieEnableMask) & BIT0)) {
> + DisableDtNode (CopyDtb, "/pcie@60000000");
> + }
> + if (!(PcdGet8 (PcdPcieEnableMask) & BIT1)) {
> + DisableDtNode (CopyDtb, "/pcie@70000000");
> + }
> +
> + *Dtb = CopyDtb;
> + *DtbSize = OrigDtbSize + DTB_PADDING;
> +
> + return EFI_SUCCESS;
> +}
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> new file mode 100644
> index 000000000000..e1f564f73078
> --- /dev/null
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> @@ -0,0 +1,42 @@
> +/** @file
> +*
> +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> +*
> +* 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 = 0x0001001A
> + BASE_NAME = SynQuacerDtbLoaderLib
> + FILE_GUID = 59df69c4-8724-4e49-8974-d0691364338c
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = DtPlatformDtbLoaderLib|DXE_DRIVER
> +
> +[Sources]
> + SynQuacerDtbLoaderLib.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + EmbeddedPkg/EmbeddedPkg.dec
> + Silicon/Socionext/SynQuacer/SynQuacer.dec
> +
> +[LibraryClasses]
> + BaseLib
> + DebugLib
> + DxeServicesLib
> + FdtLib
> + MemoryAllocationLib
> +
> +[Pcd]
> + gSynQuacerTokenSpaceGuid.PcdPcieEnableMask
> +
> +[Guids]
> + gDtPlatformDefaultDtbFileGuid
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 5/8] Silicon/SynQuacer: disable PCI RC #0 DT node if disabled
2017-12-12 17:32 ` Leif Lindholm
@ 2017-12-12 17:35 ` Ard Biesheuvel
2017-12-12 17:50 ` Leif Lindholm
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 17:35 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Daniel Thompson, Masami Hiramatsu
On 12 December 2017 at 17:32, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> Suggested subject tweak:
> Silicon/SynQuacer: disable PCI RC #0 DT node if disabled ->
> Silicon/SynQuacer: disable PCI RC DT node if RC disabled.
>
Ack. It's out of date, as you probably suspected
> The code below will disable either #0 or #1 node if not enabled by
> Pcd.
>
> On Tue, Dec 12, 2017 at 10:38:04AM +0000, Ard Biesheuvel wrote:
>> If PCIe RC #0 is not enabled (due to the fact that the slot is not
>> populated), set its DT node 'status' property to 'disabled' so that
>> the OS will not attempt to attach to it.
>>
>> This means we will need to switch from the default DtPlatformDtbLoaderLib
>> to a special one for our platform.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 +-
>> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 3 +-
>> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c | 94 ++++++++++++++++++++
>> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 42 +++++++++
>> 4 files changed, 141 insertions(+), 6 deletions(-)
>>
>> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> index 5ec26f9cdd34..80728fedbc20 100644
>> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> @@ -160,7 +160,8 @@ [LibraryClasses.common.DXE_CORE]
>> PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>>
>> [LibraryClasses.common.DXE_DRIVER]
>> - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
>> + DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
>> + FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
>> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
>> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
>>
>> @@ -611,10 +612,7 @@ [Components.common]
>> #
>> # Console preference selection
>> #
>> - EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf {
>> - <LibraryClasses>
>> - FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
>> - }
>> + EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf
>>
>> #
>> # DT support
>> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
>> index bc8ddd452d4b..c71425664bdc 100644
>> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
>> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
>> @@ -159,7 +159,8 @@ [LibraryClasses.common.DXE_CORE]
>> PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>>
>> [LibraryClasses.common.DXE_DRIVER]
>> - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
>> + DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
>> + FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
>> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
>> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
>>
>> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
>> new file mode 100644
>> index 000000000000..a93a6027e64d
>> --- /dev/null
>> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
>> @@ -0,0 +1,94 @@
>> +/** @file
>> +*
>> +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
>> +*
>> +* 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 <PiDxe.h>
>> +
>> +#include <libfdt.h>
>> +#include <Library/BaseLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/DxeServicesLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +
>> +#define DTB_PADDING 64
>
> If there isn't a simplish way of determining this value
> arithmetically, can you add a comment explaining why 64 is sufficient?
>
Something like
// add enough space for two instances of 'status = "disabled"'
?
>> +STATIC
>> +VOID
>> +DisableDtNode (
>> + IN VOID *Dtb,
>> + IN CONST CHAR8 *NodePath
>> + )
>> +{
>> + INT32 Node;
>> + INT32 Rc;
>> +
>> + Node = fdt_path_offset (Dtb, NodePath);
>> + if (Node < 0) {
>> + DEBUG ((DEBUG_ERROR, "%a: failed to locate DT path '%a': %a\n",
>> + __FUNCTION__, NodePath, fdt_strerror (Node)));
>> + return;
>> + }
>> + Rc = fdt_setprop_string (Dtb, Node, "status", "disabled");
>> + if (Rc < 0) {
>> + DEBUG ((DEBUG_ERROR, "%a: failed to set status to 'disabled' on '%a': %a\n",
>> + __FUNCTION__, NodePath, fdt_strerror (Rc)));
>> + }
>> +}
>> +
>> +/**
>> + Return a pool allocated copy of the DTB image that is appropriate for
>> + booting the current platform via DT.
>> +
>> + @param[out] Dtb Pointer to the DTB copy
>> + @param[out] DtbSize Size of the DTB copy
>> +
>> + @retval EFI_SUCCESS Operation completed successfully
>> + @retval EFI_NOT_FOUND No suitable DTB image could be located
>> + @retval EFI_OUT_OF_RESOURCES No pool memory available
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +DtPlatformLoadDtb (
>> + OUT VOID **Dtb,
>> + OUT UINTN *DtbSize
>> + )
>> +{
>> + EFI_STATUS Status;
>> + VOID *OrigDtb;
>> + VOID *CopyDtb;
>> + UINTN OrigDtbSize;
>> +
>> + Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
>> + EFI_SECTION_RAW, 0, &OrigDtb, &OrigDtbSize);
>> + if (EFI_ERROR (Status)) {
>> + return EFI_NOT_FOUND;
>> + }
>> +
>> + CopyDtb = AllocateCopyPool (OrigDtbSize + DTB_PADDING, OrigDtb);
>> + if (CopyDtb == NULL) {
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> +
>> + if (!(PcdGet8 (PcdPcieEnableMask) & BIT0)) {
>> + DisableDtNode (CopyDtb, "/pcie@60000000");
>> + }
>> + if (!(PcdGet8 (PcdPcieEnableMask) & BIT1)) {
>> + DisableDtNode (CopyDtb, "/pcie@70000000");
>> + }
>> +
>> + *Dtb = CopyDtb;
>> + *DtbSize = OrigDtbSize + DTB_PADDING;
>> +
>> + return EFI_SUCCESS;
>> +}
>> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
>> new file mode 100644
>> index 000000000000..e1f564f73078
>> --- /dev/null
>> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
>> @@ -0,0 +1,42 @@
>> +/** @file
>> +*
>> +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
>> +*
>> +* 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 = 0x0001001A
>> + BASE_NAME = SynQuacerDtbLoaderLib
>> + FILE_GUID = 59df69c4-8724-4e49-8974-d0691364338c
>> + MODULE_TYPE = DXE_DRIVER
>> + VERSION_STRING = 1.0
>> + LIBRARY_CLASS = DtPlatformDtbLoaderLib|DXE_DRIVER
>> +
>> +[Sources]
>> + SynQuacerDtbLoaderLib.c
>> +
>> +[Packages]
>> + MdePkg/MdePkg.dec
>> + EmbeddedPkg/EmbeddedPkg.dec
>> + Silicon/Socionext/SynQuacer/SynQuacer.dec
>> +
>> +[LibraryClasses]
>> + BaseLib
>> + DebugLib
>> + DxeServicesLib
>> + FdtLib
>> + MemoryAllocationLib
>> +
>> +[Pcd]
>> + gSynQuacerTokenSpaceGuid.PcdPcieEnableMask
>> +
>> +[Guids]
>> + gDtPlatformDefaultDtbFileGuid
>> --
>> 2.11.0
>>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 5/8] Silicon/SynQuacer: disable PCI RC #0 DT node if disabled
2017-12-12 17:35 ` Ard Biesheuvel
@ 2017-12-12 17:50 ` Leif Lindholm
2017-12-12 18:09 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Leif Lindholm @ 2017-12-12 17:50 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Daniel Thompson, Masami Hiramatsu
On Tue, Dec 12, 2017 at 05:35:15PM +0000, Ard Biesheuvel wrote:
> On 12 December 2017 at 17:32, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > Suggested subject tweak:
> > Silicon/SynQuacer: disable PCI RC #0 DT node if disabled ->
> > Silicon/SynQuacer: disable PCI RC DT node if RC disabled.
> >
>
> Ack. It's out of date, as you probably suspected
>
> > The code below will disable either #0 or #1 node if not enabled by
> > Pcd.
> >
> > On Tue, Dec 12, 2017 at 10:38:04AM +0000, Ard Biesheuvel wrote:
> >> If PCIe RC #0 is not enabled (due to the fact that the slot is not
> >> populated), set its DT node 'status' property to 'disabled' so that
> >> the OS will not attempt to attach to it.
> >>
> >> This means we will need to switch from the default DtPlatformDtbLoaderLib
> >> to a special one for our platform.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 +-
> >> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 3 +-
> >> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c | 94 ++++++++++++++++++++
> >> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 42 +++++++++
> >> 4 files changed, 141 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> index 5ec26f9cdd34..80728fedbc20 100644
> >> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> @@ -160,7 +160,8 @@ [LibraryClasses.common.DXE_CORE]
> >> PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
> >>
> >> [LibraryClasses.common.DXE_DRIVER]
> >> - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> >> + DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> >> + FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> >> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
> >> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> >>
> >> @@ -611,10 +612,7 @@ [Components.common]
> >> #
> >> # Console preference selection
> >> #
> >> - EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf {
> >> - <LibraryClasses>
> >> - FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> >> - }
> >> + EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf
> >>
> >> #
> >> # DT support
> >> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> >> index bc8ddd452d4b..c71425664bdc 100644
> >> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> >> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> >> @@ -159,7 +159,8 @@ [LibraryClasses.common.DXE_CORE]
> >> PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
> >>
> >> [LibraryClasses.common.DXE_DRIVER]
> >> - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> >> + DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> >> + FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> >> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
> >> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> >>
> >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> >> new file mode 100644
> >> index 000000000000..a93a6027e64d
> >> --- /dev/null
> >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> >> @@ -0,0 +1,94 @@
> >> +/** @file
> >> +*
> >> +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> >> +*
> >> +* 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 <PiDxe.h>
> >> +
> >> +#include <libfdt.h>
> >> +#include <Library/BaseLib.h>
> >> +#include <Library/DebugLib.h>
> >> +#include <Library/DxeServicesLib.h>
> >> +#include <Library/MemoryAllocationLib.h>
> >> +
> >> +#define DTB_PADDING 64
> >
> > If there isn't a simplish way of determining this value
> > arithmetically, can you add a comment explaining why 64 is sufficient?
> >
>
> Something like
>
> // add enough space for two instances of 'status = "disabled"'
>
> ?
Good enough for me.
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
> >> +STATIC
> >> +VOID
> >> +DisableDtNode (
> >> + IN VOID *Dtb,
> >> + IN CONST CHAR8 *NodePath
> >> + )
> >> +{
> >> + INT32 Node;
> >> + INT32 Rc;
> >> +
> >> + Node = fdt_path_offset (Dtb, NodePath);
> >> + if (Node < 0) {
> >> + DEBUG ((DEBUG_ERROR, "%a: failed to locate DT path '%a': %a\n",
> >> + __FUNCTION__, NodePath, fdt_strerror (Node)));
> >> + return;
> >> + }
> >> + Rc = fdt_setprop_string (Dtb, Node, "status", "disabled");
> >> + if (Rc < 0) {
> >> + DEBUG ((DEBUG_ERROR, "%a: failed to set status to 'disabled' on '%a': %a\n",
> >> + __FUNCTION__, NodePath, fdt_strerror (Rc)));
> >> + }
> >> +}
> >> +
> >> +/**
> >> + Return a pool allocated copy of the DTB image that is appropriate for
> >> + booting the current platform via DT.
> >> +
> >> + @param[out] Dtb Pointer to the DTB copy
> >> + @param[out] DtbSize Size of the DTB copy
> >> +
> >> + @retval EFI_SUCCESS Operation completed successfully
> >> + @retval EFI_NOT_FOUND No suitable DTB image could be located
> >> + @retval EFI_OUT_OF_RESOURCES No pool memory available
> >> +
> >> +**/
> >> +EFI_STATUS
> >> +EFIAPI
> >> +DtPlatformLoadDtb (
> >> + OUT VOID **Dtb,
> >> + OUT UINTN *DtbSize
> >> + )
> >> +{
> >> + EFI_STATUS Status;
> >> + VOID *OrigDtb;
> >> + VOID *CopyDtb;
> >> + UINTN OrigDtbSize;
> >> +
> >> + Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
> >> + EFI_SECTION_RAW, 0, &OrigDtb, &OrigDtbSize);
> >> + if (EFI_ERROR (Status)) {
> >> + return EFI_NOT_FOUND;
> >> + }
> >> +
> >> + CopyDtb = AllocateCopyPool (OrigDtbSize + DTB_PADDING, OrigDtb);
> >> + if (CopyDtb == NULL) {
> >> + return EFI_OUT_OF_RESOURCES;
> >> + }
> >> +
> >> + if (!(PcdGet8 (PcdPcieEnableMask) & BIT0)) {
> >> + DisableDtNode (CopyDtb, "/pcie@60000000");
> >> + }
> >> + if (!(PcdGet8 (PcdPcieEnableMask) & BIT1)) {
> >> + DisableDtNode (CopyDtb, "/pcie@70000000");
> >> + }
> >> +
> >> + *Dtb = CopyDtb;
> >> + *DtbSize = OrigDtbSize + DTB_PADDING;
> >> +
> >> + return EFI_SUCCESS;
> >> +}
> >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> >> new file mode 100644
> >> index 000000000000..e1f564f73078
> >> --- /dev/null
> >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> >> @@ -0,0 +1,42 @@
> >> +/** @file
> >> +*
> >> +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> >> +*
> >> +* 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 = 0x0001001A
> >> + BASE_NAME = SynQuacerDtbLoaderLib
> >> + FILE_GUID = 59df69c4-8724-4e49-8974-d0691364338c
> >> + MODULE_TYPE = DXE_DRIVER
> >> + VERSION_STRING = 1.0
> >> + LIBRARY_CLASS = DtPlatformDtbLoaderLib|DXE_DRIVER
> >> +
> >> +[Sources]
> >> + SynQuacerDtbLoaderLib.c
> >> +
> >> +[Packages]
> >> + MdePkg/MdePkg.dec
> >> + EmbeddedPkg/EmbeddedPkg.dec
> >> + Silicon/Socionext/SynQuacer/SynQuacer.dec
> >> +
> >> +[LibraryClasses]
> >> + BaseLib
> >> + DebugLib
> >> + DxeServicesLib
> >> + FdtLib
> >> + MemoryAllocationLib
> >> +
> >> +[Pcd]
> >> + gSynQuacerTokenSpaceGuid.PcdPcieEnableMask
> >> +
> >> +[Guids]
> >> + gDtPlatformDefaultDtbFileGuid
> >> --
> >> 2.11.0
> >>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 5/8] Silicon/SynQuacer: disable PCI RC #0 DT node if disabled
2017-12-12 17:50 ` Leif Lindholm
@ 2017-12-12 18:09 ` Ard Biesheuvel
2017-12-12 18:15 ` Leif Lindholm
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 18:09 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Daniel Thompson, Masami Hiramatsu
On 12 December 2017 at 17:50, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Dec 12, 2017 at 05:35:15PM +0000, Ard Biesheuvel wrote:
>> On 12 December 2017 at 17:32, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> > Suggested subject tweak:
>> > Silicon/SynQuacer: disable PCI RC #0 DT node if disabled ->
>> > Silicon/SynQuacer: disable PCI RC DT node if RC disabled.
>> >
>>
>> Ack. It's out of date, as you probably suspected
>>
>> > The code below will disable either #0 or #1 node if not enabled by
>> > Pcd.
>> >
>> > On Tue, Dec 12, 2017 at 10:38:04AM +0000, Ard Biesheuvel wrote:
>> >> If PCIe RC #0 is not enabled (due to the fact that the slot is not
>> >> populated), set its DT node 'status' property to 'disabled' so that
>> >> the OS will not attempt to attach to it.
>> >>
>> >> This means we will need to switch from the default DtPlatformDtbLoaderLib
>> >> to a special one for our platform.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.1
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >> ---
>> >> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 +-
>> >> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 3 +-
>> >> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c | 94 ++++++++++++++++++++
>> >> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 42 +++++++++
>> >> 4 files changed, 141 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> >> index 5ec26f9cdd34..80728fedbc20 100644
>> >> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> >> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
>> >> @@ -160,7 +160,8 @@ [LibraryClasses.common.DXE_CORE]
>> >> PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>> >>
>> >> [LibraryClasses.common.DXE_DRIVER]
>> >> - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
>> >> + DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
>> >> + FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
>> >> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
>> >> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
>> >>
>> >> @@ -611,10 +612,7 @@ [Components.common]
>> >> #
>> >> # Console preference selection
>> >> #
>> >> - EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf {
>> >> - <LibraryClasses>
>> >> - FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
>> >> - }
>> >> + EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf
>> >>
>> >> #
>> >> # DT support
>> >> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
>> >> index bc8ddd452d4b..c71425664bdc 100644
>> >> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
>> >> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
>> >> @@ -159,7 +159,8 @@ [LibraryClasses.common.DXE_CORE]
>> >> PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>> >>
>> >> [LibraryClasses.common.DXE_DRIVER]
>> >> - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
>> >> + DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
>> >> + FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
>> >> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
>> >> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
>> >>
>> >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
>> >> new file mode 100644
>> >> index 000000000000..a93a6027e64d
>> >> --- /dev/null
>> >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
>> >> @@ -0,0 +1,94 @@
>> >> +/** @file
>> >> +*
>> >> +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
>> >> +*
>> >> +* 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 <PiDxe.h>
>> >> +
>> >> +#include <libfdt.h>
>> >> +#include <Library/BaseLib.h>
>> >> +#include <Library/DebugLib.h>
>> >> +#include <Library/DxeServicesLib.h>
>> >> +#include <Library/MemoryAllocationLib.h>
>> >> +
>> >> +#define DTB_PADDING 64
>> >
>> > If there isn't a simplish way of determining this value
>> > arithmetically, can you add a comment explaining why 64 is sufficient?
>> >
>>
>> Something like
>>
>> // add enough space for two instances of 'status = "disabled"'
>>
>> ?
>
> Good enough for me.
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
Thanks. Does that cover the delta patch I added in my own reply?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 5/8] Silicon/SynQuacer: disable PCI RC #0 DT node if disabled
2017-12-12 18:09 ` Ard Biesheuvel
@ 2017-12-12 18:15 ` Leif Lindholm
0 siblings, 0 replies; 27+ messages in thread
From: Leif Lindholm @ 2017-12-12 18:15 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Daniel Thompson, Masami Hiramatsu
On Tue, Dec 12, 2017 at 06:09:13PM +0000, Ard Biesheuvel wrote:
> On 12 December 2017 at 17:50, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Tue, Dec 12, 2017 at 05:35:15PM +0000, Ard Biesheuvel wrote:
> >> On 12 December 2017 at 17:32, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >> > Suggested subject tweak:
> >> > Silicon/SynQuacer: disable PCI RC #0 DT node if disabled ->
> >> > Silicon/SynQuacer: disable PCI RC DT node if RC disabled.
> >> >
> >>
> >> Ack. It's out of date, as you probably suspected
> >>
> >> > The code below will disable either #0 or #1 node if not enabled by
> >> > Pcd.
> >> >
> >> > On Tue, Dec 12, 2017 at 10:38:04AM +0000, Ard Biesheuvel wrote:
> >> >> If PCIe RC #0 is not enabled (due to the fact that the slot is not
> >> >> populated), set its DT node 'status' property to 'disabled' so that
> >> >> the OS will not attempt to attach to it.
> >> >>
> >> >> This means we will need to switch from the default DtPlatformDtbLoaderLib
> >> >> to a special one for our platform.
> >> >>
> >> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> >> ---
> >> >> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 8 +-
> >> >> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 3 +-
> >> >> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c | 94 ++++++++++++++++++++
> >> >> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 42 +++++++++
> >> >> 4 files changed, 141 insertions(+), 6 deletions(-)
> >> >>
> >> >> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> >> index 5ec26f9cdd34..80728fedbc20 100644
> >> >> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> >> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> >> >> @@ -160,7 +160,8 @@ [LibraryClasses.common.DXE_CORE]
> >> >> PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
> >> >>
> >> >> [LibraryClasses.common.DXE_DRIVER]
> >> >> - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> >> >> + DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> >> >> + FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> >> >> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
> >> >> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> >> >>
> >> >> @@ -611,10 +612,7 @@ [Components.common]
> >> >> #
> >> >> # Console preference selection
> >> >> #
> >> >> - EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf {
> >> >> - <LibraryClasses>
> >> >> - FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> >> >> - }
> >> >> + EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf
> >> >>
> >> >> #
> >> >> # DT support
> >> >> diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> >> >> index bc8ddd452d4b..c71425664bdc 100644
> >> >> --- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> >> >> +++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
> >> >> @@ -159,7 +159,8 @@ [LibraryClasses.common.DXE_CORE]
> >> >> PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
> >> >>
> >> >> [LibraryClasses.common.DXE_DRIVER]
> >> >> - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> >> >> + DtPlatformDtbLoaderLib|Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> >> >> + FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> >> >> SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
> >> >> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> >> >>
> >> >> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> >> >> new file mode 100644
> >> >> index 000000000000..a93a6027e64d
> >> >> --- /dev/null
> >> >> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> >> >> @@ -0,0 +1,94 @@
> >> >> +/** @file
> >> >> +*
> >> >> +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> >> >> +*
> >> >> +* 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 <PiDxe.h>
> >> >> +
> >> >> +#include <libfdt.h>
> >> >> +#include <Library/BaseLib.h>
> >> >> +#include <Library/DebugLib.h>
> >> >> +#include <Library/DxeServicesLib.h>
> >> >> +#include <Library/MemoryAllocationLib.h>
> >> >> +
> >> >> +#define DTB_PADDING 64
> >> >
> >> > If there isn't a simplish way of determining this value
> >> > arithmetically, can you add a comment explaining why 64 is sufficient?
> >> >
> >>
> >> Something like
> >>
> >> // add enough space for two instances of 'status = "disabled"'
> >>
> >> ?
> >
> > Good enough for me.
> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> >
>
> Thanks. Does that cover the delta patch I added in my own reply?
Yes. Sorry, should have been explicit.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH edk2-platforms 6/8] Silicon/SynQuacerEvalBoard: enable PCI #0 only when card is detected
2017-12-12 10:37 [PATCH edk2-platforms 0/8] SynQuacer updates Ard Biesheuvel
` (4 preceding siblings ...)
2017-12-12 10:38 ` [PATCH edk2-platforms 5/8] Silicon/SynQuacer: disable PCI RC #0 DT node if disabled Ard Biesheuvel
@ 2017-12-12 10:38 ` Ard Biesheuvel
2017-12-12 10:38 ` [PATCH edk2-platforms 7/8] Silicon/Socionext/SynQuacer/DeviceTree: expose SCP serial port to the OS Ard Biesheuvel
` (2 subsequent siblings)
8 siblings, 0 replies; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 10:38 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, daniel.thompson, masami.hiramatsu, Ard Biesheuvel
The EVB does not boot if PCI RC #0 has no card inserted, and will hang in
the PCIe initialization code. So let's check the presence detect GPIO,
and only enable PCI RC #0 if it is asserted.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 7 ++
Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c | 70 ++++++++++++++------
Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.inf | 2 +
Silicon/Socionext/SynQuacer/SynQuacer.dec | 1 +
4 files changed, 59 insertions(+), 21 deletions(-)
diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
index c71425664bdc..917632c2b4c1 100644
--- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
+++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
@@ -374,6 +374,10 @@ [PcdsFixedAtBuild.common]
# set DIP switch DSW3-PIN1 (GPIO pin PD[0] on the SoC) to clear the varstore
gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin|0
+ # On the EVB, PCIe RC #0 should not be enabled from software if no card
+ # was inserted, or the boot will hang.
+ gSynQuacerTokenSpaceGuid.PcdPcie0PresenceDetectGpioPin|15
+
!if $(BUILD_NUMBER) > 1
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(BUILD_NUMBER)"
!endif
@@ -395,6 +399,9 @@ [PcdsDynamicDefault]
gArmTokenSpaceGuid.PcdSystemMemoryBase|0x0000000000000000
gArmTokenSpaceGuid.PcdSystemMemorySize|0xFFFFFFFFFFFFFFFF
+ # enable RC #1 only by default, RC #0 will be enabled if an endpoint is detected
+ gSynQuacerTokenSpaceGuid.PcdPcieEnableMask|0x2
+
################################################################################
#
# Components Section - list of all EDK II Modules needed by this Platform
diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c
index e4aec8b09169..7c529a22c6ef 100644
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c
@@ -24,7 +24,10 @@
#include <Ppi/EmbeddedGpio.h>
#include <Ppi/MemoryDiscovered.h>
-#define CLEAR_SETTINGS_GPIO_NOT_IMPLEMENTED MAX_UINT8
+#define GPIO_NOT_IMPLEMENTED MAX_UINT8
+
+#define CLEAR_SETTINGS_GPIO_ASSERTED 1
+#define PCIE_GPIO_CARD_PRESENT 0
STATIC
CONST DRAM_INFO *mDramInfo = (VOID *)(UINTN)FixedPcdGet64 (PcdDramInfoBase);
@@ -100,6 +103,35 @@ STATIC CONST EFI_PEI_PPI_DESCRIPTOR mDramInfoPpiDescriptor = {
&mDramInfoPpi
};
+STATIC
+EFI_STATUS
+ReadGpioInput (
+ IN EMBEDDED_GPIO_PPI *Gpio,
+ IN UINT8 Pin,
+ OUT UINTN *Value
+ )
+{
+ EFI_STATUS Status;
+
+ if (Pin == GPIO_NOT_IMPLEMENTED) {
+ return EFI_NOT_FOUND;
+ }
+
+ Status = Gpio->Set (Gpio, Pin, GPIO_MODE_INPUT);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "%a: failed to set GPIO %d as input - %r\n",
+ __FUNCTION__, Pin, Status));
+ return Status;
+ }
+
+ Status = Gpio->Get (Gpio, Pin, Value);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "%a: failed to get GPIO %d state - %r\n",
+ __FUNCTION__, Pin, Status));
+ }
+ return Status;
+}
+
EFI_STATUS
EFIAPI
PlatformPeim (
@@ -109,30 +141,26 @@ PlatformPeim (
EMBEDDED_GPIO_PPI *Gpio;
EFI_STATUS Status;
UINTN Value;
- UINT8 Pin;
ASSERT (mDramInfo->NumRegions > 0);
- Pin = FixedPcdGet8 (PcdClearSettingsGpioPin);
- if (Pin != CLEAR_SETTINGS_GPIO_NOT_IMPLEMENTED) {
- Status = PeiServicesLocatePpi (&gEdkiiEmbeddedGpioPpiGuid, 0, NULL,
- (VOID **)&Gpio);
- ASSERT_EFI_ERROR (Status);
+ Status = PeiServicesLocatePpi (&gEdkiiEmbeddedGpioPpiGuid, 0, NULL,
+ (VOID **)&Gpio);
+ ASSERT_EFI_ERROR (Status);
- Status = Gpio->Set (Gpio, Pin, GPIO_MODE_INPUT);
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_WARN, "%a: failed to set GPIO as input - %r\n",
- __FUNCTION__, Status));
- } else {
- Status = Gpio->Get (Gpio, Pin, &Value);
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_WARN, "%a: failed to get GPIO state - %r\n",
- __FUNCTION__, Status));
- } else if (Value > 0) {
- DEBUG ((DEBUG_INFO, "%a: clearing NVRAM\n", __FUNCTION__));
- PeiServicesSetBootMode (BOOT_WITH_DEFAULT_SETTINGS);
- }
- }
+ Status = ReadGpioInput (Gpio, FixedPcdGet8 (PcdClearSettingsGpioPin), &Value);
+ if (!EFI_ERROR (Status) && Value == CLEAR_SETTINGS_GPIO_ASSERTED) {
+ DEBUG ((DEBUG_INFO, "%a: clearing NVRAM\n", __FUNCTION__));
+ PeiServicesSetBootMode (BOOT_WITH_DEFAULT_SETTINGS);
+ }
+
+ Status = ReadGpioInput (Gpio, FixedPcdGet8 (PcdPcie0PresenceDetectGpioPin),
+ &Value);
+ if (!EFI_ERROR (Status) && Value == PCIE_GPIO_CARD_PRESENT) {
+ DEBUG ((DEBUG_INFO,
+ "%a: card detected in PCIe RC #0, enabling\n", __FUNCTION__));
+ Status = PcdSet8S (PcdPcieEnableMask, PcdGet8 (PcdPcieEnableMask) | BIT0);
+ ASSERT_EFI_ERROR (Status);
}
//
diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.inf
index a6501fb205e1..eb6a5bf9ac1a 100644
--- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.inf
+++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.inf
@@ -43,6 +43,7 @@ [FixedPcd]
gArmTokenSpaceGuid.PcdFvSize
gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin
gSynQuacerTokenSpaceGuid.PcdDramInfoBase
+ gSynQuacerTokenSpaceGuid.PcdPcie0PresenceDetectGpioPin
[Ppis]
gEdkiiEmbeddedGpioPpiGuid ## CONSUMES
@@ -51,6 +52,7 @@ [Ppis]
[Pcd]
gArmTokenSpaceGuid.PcdSystemMemoryBase
gArmTokenSpaceGuid.PcdSystemMemorySize
+ gSynQuacerTokenSpaceGuid.PcdPcieEnableMask
[Depex]
gEdkiiEmbeddedGpioPpiGuid
diff --git a/Silicon/Socionext/SynQuacer/SynQuacer.dec b/Silicon/Socionext/SynQuacer/SynQuacer.dec
index 2e18cb33346d..a21f12b5bc32 100644
--- a/Silicon/Socionext/SynQuacer/SynQuacer.dec
+++ b/Silicon/Socionext/SynQuacer/SynQuacer.dec
@@ -36,6 +36,7 @@ [PcdsFixedAtBuild]
# GPIO pin index [0 .. 31] or MAX_UINT8 for not implemented
gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin|0xFF|UINT8|0x00000004
+ gSynQuacerTokenSpaceGuid.PcdPcie0PresenceDetectGpioPin|0xFF|UINT8|0x00000006
gSynQuacerTokenSpaceGuid.PcdI2cReferenceClock|62500000|UINT32|0x00000005
--
2.11.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH edk2-platforms 7/8] Silicon/Socionext/SynQuacer/DeviceTree: expose SCP serial port to the OS
2017-12-12 10:37 [PATCH edk2-platforms 0/8] SynQuacer updates Ard Biesheuvel
` (5 preceding siblings ...)
2017-12-12 10:38 ` [PATCH edk2-platforms 6/8] Silicon/SynQuacerEvalBoard: enable PCI #0 only when card is detected Ard Biesheuvel
@ 2017-12-12 10:38 ` Ard Biesheuvel
2017-12-12 17:37 ` Leif Lindholm
2017-12-12 10:38 ` [PATCH edk2-platforms 8/8] Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed Ard Biesheuvel
2017-12-12 18:20 ` [PATCH edk2-platforms 0/8] SynQuacer updates Leif Lindholm
8 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 10:38 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, daniel.thompson, masami.hiramatsu, Masahisa KOJIMA,
Ard Biesheuvel
From: Masahisa KOJIMA <kojima.masahisa@socionext.com>
In order to be able to use UART #0 on the DeveloperBox's 96boards low
speed connector, expose it to the OS by adding a node to the device
tree. This requires a CM3 firmware build that makes the SCP detach
from the serial port after boot.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Masahisa KOJIMA <kojima.masahisa@socionext.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
index c9fee5d1f350..37a3981f0360 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
@@ -440,6 +440,15 @@
clock-names = "uartclk", "apb_pclk";
};
+ fuart: fuart@51040000 {
+ compatible = "snps,dw-apb-uart";
+ reg = <0x0 0x51040000 0x0 0x1000>;
+ interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>;
+ clock-frequency = <62500000>;
+ reg-io-width = <4>;
+ reg-shift = <2>;
+ };
+
clk_netsec: refclk125mhz {
compatible = "fixed-clock";
clock-frequency = <125000000>;
--
2.11.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 7/8] Silicon/Socionext/SynQuacer/DeviceTree: expose SCP serial port to the OS
2017-12-12 10:38 ` [PATCH edk2-platforms 7/8] Silicon/Socionext/SynQuacer/DeviceTree: expose SCP serial port to the OS Ard Biesheuvel
@ 2017-12-12 17:37 ` Leif Lindholm
0 siblings, 0 replies; 27+ messages in thread
From: Leif Lindholm @ 2017-12-12 17:37 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel, daniel.thompson, masami.hiramatsu, Masahisa KOJIMA
Suggested subject tweak:
Silicon/Socionext/SynQuacer/DeviceTree: expose SCP serial port to the OS ->
Silicon/Socionext/SynQuacer: add UART #0 node to DT
with that:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
On Tue, Dec 12, 2017 at 10:38:06AM +0000, Ard Biesheuvel wrote:
> From: Masahisa KOJIMA <kojima.masahisa@socionext.com>
>
> In order to be able to use UART #0 on the DeveloperBox's 96boards low
> speed connector, expose it to the OS by adding a node to the device
> tree. This requires a CM3 firmware build that makes the SCP detach
> from the serial port after boot.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Masahisa KOJIMA <kojima.masahisa@socionext.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> index c9fee5d1f350..37a3981f0360 100644
> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> @@ -440,6 +440,15 @@
> clock-names = "uartclk", "apb_pclk";
> };
>
> + fuart: fuart@51040000 {
> + compatible = "snps,dw-apb-uart";
> + reg = <0x0 0x51040000 0x0 0x1000>;
> + interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>;
> + clock-frequency = <62500000>;
> + reg-io-width = <4>;
> + reg-shift = <2>;
> + };
> +
> clk_netsec: refclk125mhz {
> compatible = "fixed-clock";
> clock-frequency = <125000000>;
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH edk2-platforms 8/8] Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed
2017-12-12 10:37 [PATCH edk2-platforms 0/8] SynQuacer updates Ard Biesheuvel
` (6 preceding siblings ...)
2017-12-12 10:38 ` [PATCH edk2-platforms 7/8] Silicon/Socionext/SynQuacer/DeviceTree: expose SCP serial port to the OS Ard Biesheuvel
@ 2017-12-12 10:38 ` Ard Biesheuvel
2017-12-12 17:47 ` Leif Lindholm
2017-12-12 18:20 ` [PATCH edk2-platforms 0/8] SynQuacer updates Leif Lindholm
8 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 10:38 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, daniel.thompson, masami.hiramatsu, Ard Biesheuvel
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
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 8/8] Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed
2017-12-12 10:38 ` [PATCH edk2-platforms 8/8] Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed Ard Biesheuvel
@ 2017-12-12 17:47 ` Leif Lindholm
2017-12-12 17:51 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Leif Lindholm @ 2017-12-12 17:47 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, daniel.thompson, masami.hiramatsu
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
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 8/8] Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed
2017-12-12 17:47 ` Leif Lindholm
@ 2017-12-12 17:51 ` Ard Biesheuvel
2017-12-12 18:15 ` Leif Lindholm
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 17:51 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Daniel Thompson, Masami Hiramatsu
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 8/8] Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed
2017-12-12 17:51 ` Ard Biesheuvel
@ 2017-12-12 18:15 ` Leif Lindholm
0 siblings, 0 replies; 27+ messages in thread
From: Leif Lindholm @ 2017-12-12 18:15 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Daniel Thompson, Masami Hiramatsu
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>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 0/8] SynQuacer updates
2017-12-12 10:37 [PATCH edk2-platforms 0/8] SynQuacer updates Ard Biesheuvel
` (7 preceding siblings ...)
2017-12-12 10:38 ` [PATCH edk2-platforms 8/8] Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed Ard Biesheuvel
@ 2017-12-12 18:20 ` Leif Lindholm
2017-12-12 18:38 ` Ard Biesheuvel
8 siblings, 1 reply; 27+ messages in thread
From: Leif Lindholm @ 2017-12-12 18:20 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, daniel.thompson, masami.hiramatsu
On Tue, Dec 12, 2017 at 10:37:59AM +0000, Ard Biesheuvel wrote:
> A round of updates for Socionext SynQuacer:
>
> - enable CPU idle states in the DT, so that the OS can put cores to sleep
> using PSCI (#1)
> - add the build number to PCDs that end up in user visible strings (#2)
> - fix a PCIe detection issue in the DeveloperBox x16 slot, by keeping PERST#
> asserted for at least 100 ms before link training (#3)
> - ignore PCIe RC #0 if no card is inserted on EVB (#4 - #6)
> - add the secondary UART to the DT for the OS to use (this is UART #0 on the
> LS connector on DeveloperBox) (#7)
> - explicitly retrain the downstream links on the Asmedia 1182/1184 PCIe
> switch, to enable Gen2 speeds
For the patches I haven't commented on individually (1,4,6):
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> Ard Biesheuvel (7):
> Silicon/SynQuacer: enable CPU idle states in device tree
> Platform/Socionext/SynQuacer: expose build number as firmware version
> Silicon/SynQuacerPciHostBridgeLib: stall for 150 ms during PERST#
> Silicon/SynQuacerPciHostBridgeLib: enable RCs based on PCD setting
> Silicon/SynQuacer: disable PCI RC #0 DT node if disabled
> Silicon/SynQuacerEvalBoard: enable PCI #0 only when card is detected
> Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed
>
> Masahisa KOJIMA (1):
> Silicon/Socionext/SynQuacer/DeviceTree: expose SCP serial port to the
> OS
>
> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 16 ++-
> Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
> Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 +-
> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 18 ++-
> Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
> Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 +-
> Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 57 ++++----
> 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 +
> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c | 94 +++++++++++++
> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 42 ++++++
> Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c | 19 ++-
> Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf | 4 +
> Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 58 +++++---
> Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c | 70 +++++++---
> Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.inf | 2 +
> Silicon/Socionext/SynQuacer/SynQuacer.dec | 5 +
> 19 files changed, 504 insertions(+), 88 deletions(-)
> create mode 100644 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c
> create mode 100644 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h
> create mode 100644 Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> create mode 100644 Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
>
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH edk2-platforms 0/8] SynQuacer updates
2017-12-12 18:20 ` [PATCH edk2-platforms 0/8] SynQuacer updates Leif Lindholm
@ 2017-12-12 18:38 ` Ard Biesheuvel
0 siblings, 0 replies; 27+ messages in thread
From: Ard Biesheuvel @ 2017-12-12 18:38 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Daniel Thompson, Masami Hiramatsu
On 12 December 2017 at 18:20, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Dec 12, 2017 at 10:37:59AM +0000, Ard Biesheuvel wrote:
>> A round of updates for Socionext SynQuacer:
>>
>> - enable CPU idle states in the DT, so that the OS can put cores to sleep
>> using PSCI (#1)
>> - add the build number to PCDs that end up in user visible strings (#2)
>> - fix a PCIe detection issue in the DeveloperBox x16 slot, by keeping PERST#
>> asserted for at least 100 ms before link training (#3)
>> - ignore PCIe RC #0 if no card is inserted on EVB (#4 - #6)
>> - add the secondary UART to the DT for the OS to use (this is UART #0 on the
>> LS connector on DeveloperBox) (#7)
>> - explicitly retrain the downstream links on the Asmedia 1182/1184 PCIe
>> switch, to enable Gen2 speeds
>
> For the patches I haven't commented on individually (1,4,6):
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
Thanks. Pushed as 2b3b95cb9fc9..054921cef0f1
>> Ard Biesheuvel (7):
>> Silicon/SynQuacer: enable CPU idle states in device tree
>> Platform/Socionext/SynQuacer: expose build number as firmware version
>> Silicon/SynQuacerPciHostBridgeLib: stall for 150 ms during PERST#
>> Silicon/SynQuacerPciHostBridgeLib: enable RCs based on PCD setting
>> Silicon/SynQuacer: disable PCI RC #0 DT node if disabled
>> Silicon/SynQuacerEvalBoard: enable PCI #0 only when card is detected
>> Silicon/SynQuacer/PlatformDxe: retrain PCIe switch links to Gen2 speed
>>
>> Masahisa KOJIMA (1):
>> Silicon/Socionext/SynQuacer/DeviceTree: expose SCP serial port to the
>> OS
>>
>> Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 16 ++-
>> Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
>> Platform/Socionext/DeveloperBox/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 +-
>> Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 18 ++-
>> Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 1 +
>> Platform/Socionext/SynQuacerEvalBoard/SystemFirmwareDescriptor/SystemFirmwareDescriptorTable.aslc | 6 +-
>> Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 57 ++++----
>> 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 +
>> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c | 94 +++++++++++++
>> Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf | 42 ++++++
>> Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.c | 19 ++-
>> Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLib.inf | 4 +
>> Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 58 +++++---
>> Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.c | 70 +++++++---
>> Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformPeiLib/SynQuacerPlatformPeiLib.inf | 2 +
>> Silicon/Socionext/SynQuacer/SynQuacer.dec | 5 +
>> 19 files changed, 504 insertions(+), 88 deletions(-)
>> create mode 100644 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Pcie.c
>> create mode 100644 Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/PlatformDxe.h
>> create mode 100644 Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
>> create mode 100644 Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
>>
>> --
>> 2.11.0
>>
^ permalink raw reply [flat|nested] 27+ messages in thread