Re: [edk2-devel] [PATCH] XiangshanSeriesPkg:Add Support for Xilinx RC(PCIE) Driver

2024-05-17 Thread suijingfeng 
On Thu, May 16, 2024 at 10:54 AM, WangYang wrote:

Hi,

>
> Hi,Ray
> 
> Thank you very much for your attention.
>

The reviewer Ray told you that  you patch has some small problems, 
For example, the clause "if ((Bus == 1) || (Bus == 2) || (Bus == 3) || (Bus == 
4)) "  is useless.
As both code path returns same value. 

You should solve this small problems, also cut this big patch into smaller 
pieces as smaller patch is easier to review.
and when all problem solved, you should send the updated patch as V2. Since 
this version is V1 as far as I know.





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119052): https://edk2.groups.io/g/devel/message/119052
Mute This Topic: https://groups.io/mt/105572700/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] XiangshanSeriesPkg:Add Support for Xilinx RC(PCIE) Driver

2024-05-15 Thread WangYang 
Hi,Ray

   Thank you very much for your attention.



-原始邮件-
发件人:"Ni, Ray" 
发送时间:2024-05-15 16:36:02 (星期三)
收件人: "devel@edk2.groups.io" , "wangy...@bosc.ac.cn" 

抄送: "suni...@ventanamicro.com" , 
"g...@danielschaefer.me" , "Ran Wang" 
, "Leif Lindholm" , "Kinney, 
Michael D" 
主题: Re: [edk2-devel] [PATCH] XiangshanSeriesPkg:Add Support for Xilinx RC(PCIE) 
Driver



The patch is too big. Can you split it to multiple smaller patches?



As you said, this patch is indeed a bit big.  The main reference is 
file “./Silicon/Phytium/FT2000-4Pkg/Library/PciSegmentLib/PciSegmentLib.c” .




> +STATIC
> +UINT64
> +PciSegmentLibGetConfigBase (
> +  IN  UINT64  Address
> +  )

> +{







> +  UINT8 Bus;
> +  UINT8 Device;
> +  UINT8 Function;
> +
> +  EXTRACT_PCIE_ADDRESS (Address, Bus, Device, Function);
> +  if ((Bus == 1) || (Bus == 2) || (Bus == 3) || (Bus == 4)) {
> +return PCI_SEG_CONFIG_BASE;
> +  }

> +




This part of the code is redundant and should be deleted​

> +  return PCI_SEG_CONFIG_BASE;


Both paths return the same PCI_SEG_CONFIG_BASE. Then why do you check the Bus 
number?
This part of the code is redundant and should be deleted.​


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118946): https://edk2.groups.io/g/devel/message/118946
Mute This Topic: https://groups.io/mt/105572700/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] XiangshanSeriesPkg:Add Support for Xilinx RC(PCIE) Driver

2024-05-15 Thread Ni, Ray 
  1.  The patch is too big. Can you split it to multiple smaller patches?

> +STATIC
> +UINT64
> +PciSegmentLibGetConfigBase (
> +  IN  UINT64  Address
> +  )
> +{
> +  UINT8 Bus;
> +  UINT8 Device;
> +  UINT8 Function;
> +
> +  EXTRACT_PCIE_ADDRESS (Address, Bus, Device, Function);
> +  if ((Bus == 1) || (Bus == 2) || (Bus == 3) || (Bus == 4)) {
> +return PCI_SEG_CONFIG_BASE;
> +  }
> +
> +  return PCI_SEG_CONFIG_BASE;


  1.
Both paths return the same PCI_SEG_CONFIG_BASE. Then why do you check the Bus 
number?



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118913): https://edk2.groups.io/g/devel/message/118913
Mute This Topic: https://groups.io/mt/105572700/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] XiangshanSeriesPkg:Add Support for Xilinx RC(PCIE) Driver

2024-05-15 Thread WangYang 
Hi,Sunil V L

   Thank you very much for your attention.
   How about this status.
   This patch is based on  
https://edk2.groups.io/g/devel/topic/105437172#msg118579.   
   I also want to know the status of this 
patch（https://edk2.groups.io/g/devel/topic/105437172#msg118579）.  
   
Best Regards,
Yang Wang

> -原始邮件-
> 发件人: WangYang 
> 发送时间: 2024-04-17 15:08:06 (星期三)
> 收件人: suni...@ventanamicro.com, g...@danielschaefer.me, devel@edk2.groups.io
> 抄送: "Yang Wang" , "Ran Wang" , "Leif 
> Lindholm" , "Michael D Kinney" 
> 
> 主题: [edk2-devel] [PATCH] XiangshanSeriesPkg:Add Support for Xilinx RC(PCIE) 
> Driver
> 
> 1.Xilinx RC is XDMA
> 2.Support NVME storage
> 
> Nvme storage needs to be formatted to FAT32 format.
> 
> Reviewed-by: Ran Wang 
> Cc: Leif Lindholm 
> Cc: Michael D Kinney 
> Cc: Sunil V L 
> Cc: Daniel Schaefer 
> Signed-off-by: Yang Wang 
> ---
>  .../XiangshanSeriesPkg/NanhuDev/NanhuDev.dsc  |   30 +-
>  .../XiangshanSeriesPkg/NanhuDev/NanhuDev.fdf  |   13 +-
>  .../NanhuDev/NanhuDev.fdf.inc |1 +
>  .../PciHostBridgeLib/PciHostBridgeLib.c   |  273 
>  .../PciHostBridgeLib/PciHostBridgeLib.inf |   48 +
>  .../Library/PciSegmentLib/PciSegmentLib.c | 1391 +
>  .../Library/PciSegmentLib/PciSegmentLib.inf   |   29 +
>  Silicon/Bosc/NanHuPkg/NanHuDevPkg.dec |   20 +
>  8 files changed, 1802 insertions(+), 3 deletions(-)
>  create mode 100644 
> Silicon/Bosc/NanHuPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
>  create mode 100644 
> Silicon/Bosc/NanHuPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>  create mode 100755 
> Silicon/Bosc/NanHuPkg/Library/PciSegmentLib/PciSegmentLib.c
>  create mode 100755 
> Silicon/Bosc/NanHuPkg/Library/PciSegmentLib/PciSegmentLib.inf
> 
> diff --git a/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.dsc 
> b/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.dsc
> index 7dcd7c4313..85934f66be 100644
> --- a/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.dsc
> +++ b/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.dsc
> @@ -239,6 +239,9 @@
>
> PlatformBootManagerLib|Platform/RISC-V/PlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>
> PlatformMemoryTestLib|Platform/RISC-V/PlatformPkg/Library/PlatformMemoryTestLibNull/PlatformMemoryTestLibNull.inf
>
> PlatformUpdateProgressLib|Platform/RISC-V/PlatformPkg/Library/PlatformUpdateProgressLibNull/PlatformUpdateProgressLibNull.inf
> +  # Pci dependencies
> +  PciSegmentLib|Silicon/Bosc/NanHuPkg/Library/PciSegmentLib/PciSegmentLib.inf
> +  
> PciHostBridgeLib|Silicon/Bosc/NanHuPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>  
>  [LibraryClasses.common.UEFI_APPLICATION]
>PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> @@ -262,6 +265,24 @@
>gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
>  
>  [PcdsFixedAtBuild]
> +  #
> +  # XILINX PCI Root Complex
> +  #
> +  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x4000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|FALSE
> +  gEfiMdePkgTokenSpaceGuid.PcdPciIoTranslation|0x0
> +  gEfiMdePkgTokenSpaceGuid.PcdPciMmio32Translation|0x5000
> +  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciConfigBase|0x4000
> +  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciConfigSize|0x1000
> +  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciBusMin|0
> +  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciBusMax|255
> +  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciIoBase|0x0
> +  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciIoSize|0xf0
> +  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciMmio32Base|0x5000
> +  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciMmio32Size|0x1000
> +  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciMmio64Base|0x10
> +  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciMmio64Size|0x00
> +
>gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FALSE
>gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TRUE
>gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
> @@ -427,11 +448,13 @@
>MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
>  !endif
>  
> -  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
> +  Silicon/RISC-V/ProcessorPkg/Universal/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
> +  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
>MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
>  
>PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>}
> +  
> MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
>MdeModulePkg/Universal/Metronome/Metronome.inf
>MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
>

[edk2-devel] [PATCH] XiangshanSeriesPkg:Add Support for Xilinx RC(PCIE) Driver

2024-04-17 Thread WangYang 
1.Xilinx RC is XDMA
2.Support NVME storage

Nvme storage needs to be formatted to FAT32 format.

Reviewed-by: Ran Wang 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Cc: Sunil V L 
Cc: Daniel Schaefer 
Signed-off-by: Yang Wang 
---
 .../XiangshanSeriesPkg/NanhuDev/NanhuDev.dsc  |   30 +-
 .../XiangshanSeriesPkg/NanhuDev/NanhuDev.fdf  |   13 +-
 .../NanhuDev/NanhuDev.fdf.inc |1 +
 .../PciHostBridgeLib/PciHostBridgeLib.c   |  273 
 .../PciHostBridgeLib/PciHostBridgeLib.inf |   48 +
 .../Library/PciSegmentLib/PciSegmentLib.c | 1391 +
 .../Library/PciSegmentLib/PciSegmentLib.inf   |   29 +
 Silicon/Bosc/NanHuPkg/NanHuDevPkg.dec |   20 +
 8 files changed, 1802 insertions(+), 3 deletions(-)
 create mode 100644 
Silicon/Bosc/NanHuPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
 create mode 100644 
Silicon/Bosc/NanHuPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
 create mode 100755 Silicon/Bosc/NanHuPkg/Library/PciSegmentLib/PciSegmentLib.c
 create mode 100755 
Silicon/Bosc/NanHuPkg/Library/PciSegmentLib/PciSegmentLib.inf

diff --git a/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.dsc 
b/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.dsc
index 7dcd7c4313..85934f66be 100644
--- a/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.dsc
+++ b/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.dsc
@@ -239,6 +239,9 @@
   
PlatformBootManagerLib|Platform/RISC-V/PlatformPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
   
PlatformMemoryTestLib|Platform/RISC-V/PlatformPkg/Library/PlatformMemoryTestLibNull/PlatformMemoryTestLibNull.inf
   
PlatformUpdateProgressLib|Platform/RISC-V/PlatformPkg/Library/PlatformUpdateProgressLibNull/PlatformUpdateProgressLibNull.inf
+  # Pci dependencies
+  PciSegmentLib|Silicon/Bosc/NanHuPkg/Library/PciSegmentLib/PciSegmentLib.inf
+  
PciHostBridgeLib|Silicon/Bosc/NanHuPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
 
 [LibraryClasses.common.UEFI_APPLICATION]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
@@ -262,6 +265,24 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
 
 [PcdsFixedAtBuild]
+  #
+  # XILINX PCI Root Complex
+  #
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x4000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|FALSE
+  gEfiMdePkgTokenSpaceGuid.PcdPciIoTranslation|0x0
+  gEfiMdePkgTokenSpaceGuid.PcdPciMmio32Translation|0x5000
+  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciConfigBase|0x4000
+  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciConfigSize|0x1000
+  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciBusMin|0
+  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciBusMax|255
+  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciIoBase|0x0
+  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciIoSize|0xf0
+  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciMmio32Base|0x5000
+  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciMmio32Size|0x1000
+  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciMmio64Base|0x10
+  gBoscNanHuDdevPlatformPkgTokenSpaceGuid.PcdPciMmio64Size|0x00
+
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
@@ -427,11 +448,13 @@
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 !endif
 
-  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
+  Silicon/RISC-V/ProcessorPkg/Universal/PciCpuIo2Dxe/PciCpuIo2Dxe.inf
+  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
   MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
 
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
   }
+  
MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
   MdeModulePkg/Universal/Metronome/Metronome.inf
   MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
   MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf {
@@ -440,6 +463,11 @@
   }
   EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
 
+  #
+  # NVME Support
+  #
+  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf
+
   #
   # RISC-V Platform module
   #
diff --git a/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.fdf 
b/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.fdf
index d54bb73353..45c96f58a0 100644
--- a/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.fdf
+++ b/Platform/Bosc/XiangshanSeriesPkg/NanhuDev/NanhuDev.fdf
@@ -69,8 +69,6 @@ INF  OvmfPkg/Fdt/HighMemDxe/HighMemDxe.inf
 
 INF  MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 INF  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
-INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
-INF  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
 INF  MdeModulePkg/Universal/Metronome/Metronome.inf
 INF  EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
 
@@ -127,6 +125,17 @@ INF  ShellPkg/Application/Shell/Shell.inf
 
 !include NetworkPkg/Network.fdf.inc
 
+#
+# PCI Support
+#
+INF Silicon/RISC-V/ProcessorPkg/Universal/PciCpuIo2