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
>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