[edk2-devel] [PATCH v3 00/10] Added support for FT2000/4 chip

Leif Lindholm leif at nuviainc.com
Thu May 27 12:43:08 UTC 2021


Hi Ling,

Many apologies for delay. I have been juggling too many things and making
little progress on any of them. I owe you some of the good 白酒 when we
meet in person.

I have now looked over all of v3, and have some feedback, but most of it is minor.

First of all, some process comments:
I responded for v2 that you should add "Reviewed-by: Leif Lindholm <leif at nuviainc.com>"
to those patches where I had said so. But you appear to have added it to *all*
patches in v3.

Also, some of the feedback/comments on v2 has not been acted on, and I had no
reply explaining why not.

As I commented on v3 - there should be no revision history in the commit messages.
Please drop these from v4.

The following patches were given Reviewed-by for v2:
1/10
3/10
10/10

2, 4, 5, 6, 7, 8, 9 should not have been sent out with my Revied-by added.
However, after looking at v3, you can add
Reviewed-by: Leif Lindholm <leif at nuviainc.com>
to 2, 5, 6, 8.

While I did give a Reviewed-by for 1/10 in v2, I spotted a few minor issues
when looking at v3 1/10 (commented 13 April):
- A typo - CORE spelled as COURE.
- Some of the structures defined in SystemServiceInterface.h have too common names
  and should be given PHYTIUM_ prefix:
  MCU_DIMM, MCU_DIMMS, MEMORY_BLOCK, MEMORY_INFO, PCI_BLOCK, PCI_HOST_BLOCK

In addition to this, edk2 changes means the following diff needs to be folded in:

<<<
diff --git a/Platform/Phytium/DurianPkg/DurianPkg.dsc b/Platform/Phytium/DurianPkg/DurianPkg.dsc
index 9579f8e9b7d0..19009106a2bf 100644
--- a/Platform/Phytium/DurianPkg/DurianPkg.dsc
+++ b/Platform/Phytium/DurianPkg/DurianPkg.dsc
@@ -23,6 +23,7 @@ [Defines]
   SKUID_IDENTIFIER               = DEFAULT
   FLASH_DEFINITION               = Platform/Phytium/DurianPkg/DurianPkg.fdf

+!include MdePkg/MdeLibs.dsc.inc
 !include Silicon/Phytium/PhytiumCommonPkg/PhytiumCommonPkg.dsc.inc

 [LibraryClasses.common]
>>>

If you do all of these things mentioned, you can keep Reviewed-by on 1/10 when sending out v4.

I gave comments on patch 4 in v2 that were not acted on for v3.
These were regarding suspicious use of the volatile keyword.

I gave comments on patch 7 in v2 that were not acted on for v3.
- Include files should only themselves include files needed for its internal definitions.
  .c files should include all .h files they depend on themselves.
- Typos of "norfalsh" (for "norflash") and "eares" (for "erase") were not corrected.
- I also asked some questions that were not answered.
- Other feedback I gave was addressed in v3.

I gave comments on patch 7 in v2 that were not acted on for v3.
- Use IsValidTimeZone from EmbeddedPkg TimeBaseLib instead of implementing the test yourself.
- Other feedback I gave was addressed in v3.

If you can address the above comments and send out a v4, I promise I will be *much*
more responsive from this point onwards.

Best Regards,

Leif

On Wed, Mar 17, 2021 at 15:26:37 +0800, Ling Jia wrote:
> This series added packages to support FT2000/4 chip.
> Platform/Phytium: Added DurianPkg, include DurianPkg.dsc and DurianPkg.fdf.
> Silicon/Phytium: Added FT2000-4Pkg and PhytiumCommonPkg.
> 
> The modules could be runed at the silicon of FT2000/4.
> They supported Acpi parameter configuration, Pci bus scaning,
> flash read-write and erase abd operating system boot function.
> Maintainers.txt: Added maintainers and reviewers for the DurianPkg.
> 
> The public git repository is :
> https://github.com/jialing2020/edk2-platforms/tree/Phytium_Opensource_For_FT2000-4_v3
> 
> v3:
> Optimized the codes to meet the edk2 coding  specification.
> 
> Ling Jia (10):
>   Silicon/Phytium: Added PlatformLib to FT2000/4
>   Silicon/Phytium: Added Acpi support to FT2000/4
>   Silicon/Phytium: Added SMBIOS support to FT2000/4
>   Silicon/Phytium: Added PciSegmentLib to FT2000/4
>   Silicon/Phytium: Added PciHostBridgeLib to FT2000/4
>   Silicon/Phytium: Added Spi driver support to FT2000/4
>   Silicon/Phytium: Added flash driver support to Phytium Silicon
>   Silicon/Phytium: Added fvb driver for norflash
>   Silicon/Phytium: Added Rtc driver to FT2000/4
>   Maintainers.txt: Added maintainers and reviewers for the DurianPkg
> 
>  Silicon/Phytium/PhytiumCommonPkg/PhytiumCommonPkg.dec                           |   52 +
>  Silicon/Phytium/PhytiumCommonPkg/PhytiumCommonPkg.dsc.inc                       |  345 +++++
>  Platform/Phytium/DurianPkg/DurianPkg.dsc                                        |  331 +++++
>  Platform/Phytium/DurianPkg/DurianPkg.fdf                                        |  235 ++++
>  Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/AcpiTables.inf                   |   56 +
>  Silicon/Phytium/FT2000-4Pkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf     |   47 +
>  Silicon/Phytium/FT2000-4Pkg/Drivers/SpiDxe/SpiDxe.inf                           |   44 +
>  Silicon/Phytium/FT2000-4Pkg/Drivers/SpiNorFlashDxe/SpiNorFlashDxe.inf           |   48 +
>  Silicon/Phytium/FT2000-4Pkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf       |   47 +
>  Silicon/Phytium/FT2000-4Pkg/Library/PciSegmentLib/PciSegmentLib.inf             |   28 +
>  Silicon/Phytium/FT2000-4Pkg/Library/PlatformLib/PlatformLib.inf                 |   55 +
>  Silicon/Phytium/FT2000-4Pkg/Library/RealTimeClockLib/RealTimeClockLib.inf       |   39 +
>  Silicon/Phytium/PhytiumCommonPkg/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf    |   53 +
>  Silicon/Phytium/PhytiumCommonPkg/Drivers/FlashFvbDxe/FlashFvbDxe.inf            |   61 +
>  Silicon/Phytium/FT2000-4Pkg/Drivers/SpiDxe/SpiDxe.h                             |   64 +
>  Silicon/Phytium/FT2000-4Pkg/Drivers/SpiNorFlashDxe/SpiNorFlashDxe.h             |   99 ++
>  Silicon/Phytium/FT2000-4Pkg/Library/RealTimeClockLib/RealTimeClockLib.h         |   24 +
>  Silicon/Phytium/PhytiumCommonPkg/Drivers/FlashFvbDxe/FlashFvbDxe.h              |  104 ++
>  Silicon/Phytium/PhytiumCommonPkg/Include/Platform.h                             |   80 ++
>  Silicon/Phytium/PhytiumCommonPkg/Include/Protocol/SpiNorFlashProtocol.h         |   74 +
>  Silicon/Phytium/PhytiumCommonPkg/Include/Protocol/SpiProtocol.h                 |   51 +
>  Silicon/Phytium/PhytiumCommonPkg/Include/SystemServiceInterface.h               |  112 ++
>  Silicon/Phytium/FT2000-4Pkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c       |  943 +++++++++++++
>  Silicon/Phytium/FT2000-4Pkg/Drivers/SpiDxe/SpiDxe.c                             |  198 +++
>  Silicon/Phytium/FT2000-4Pkg/Drivers/SpiNorFlashDxe/SpiNorFlashDxe.c             |  424 ++++++
>  Silicon/Phytium/FT2000-4Pkg/Library/PciHostBridgeLib/PciHostBridgeLib.c         |  181 +++
>  Silicon/Phytium/FT2000-4Pkg/Library/PciSegmentLib/PciSegmentLib.c               | 1434 ++++++++++++++++++++
>  Silicon/Phytium/FT2000-4Pkg/Library/PlatformLib/PlatformLib.c                   |  137 ++
>  Silicon/Phytium/FT2000-4Pkg/Library/PlatformLib/PlatformLibMem.c                |  156 +++
>  Silicon/Phytium/FT2000-4Pkg/Library/RealTimeClockLib/RealTimeClockLib.c         |  462 +++++++
>  Silicon/Phytium/PhytiumCommonPkg/Drivers/AcpiPlatformDxe/AcpiPlatform.c         |  250 ++++
>  Silicon/Phytium/PhytiumCommonPkg/Drivers/FlashFvbDxe/FlashFvbDxe.c              | 1304 ++++++++++++++++++
>  Maintainers.txt                                                                 |    8 +
>  Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/AcpiSsdtRootPci.asl              |  209 +++
>  Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Dbg2.aslc                        |   80 ++
>  Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Dsdt/Cpu.asl                     |   85 ++
>  Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Dsdt/Dsdt.asl                    |   15 +
>  Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Dsdt/Uart.asl                    |   65 +
>  Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Fadt.aslc                        |   77 ++
>  Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Gtdt.aslc                        |   83 ++
>  Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Iort.aslc                        |   89 ++
>  Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Madt.aslc                        |   67 +
>  Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Mcfg.aslc                        |   65 +
>  Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Pptt.aslc                        |  219 +++
>  Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Spcr.aslc                        |   73 +
>  Silicon/Phytium/FT2000-4Pkg/Library/PlatformLib/AArch64/PhytiumPlatformHelper.S |   76 ++
>  Silicon/Phytium/PhytiumCommonPkg/PhytiumCommonPkg.fdf.inc                       |  119 ++
>  47 files changed, 8868 insertions(+)
>  create mode 100644 Silicon/Phytium/PhytiumCommonPkg/PhytiumCommonPkg.dec
>  create mode 100644 Silicon/Phytium/PhytiumCommonPkg/PhytiumCommonPkg.dsc.inc
>  create mode 100644 Platform/Phytium/DurianPkg/DurianPkg.dsc
>  create mode 100644 Platform/Phytium/DurianPkg/DurianPkg.fdf
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/AcpiTables.inf
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/SpiDxe/SpiDxe.inf
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/SpiNorFlashDxe/SpiNorFlashDxe.inf
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Library/PciSegmentLib/PciSegmentLib.inf
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Library/PlatformLib/PlatformLib.inf
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Library/RealTimeClockLib/RealTimeClockLib.inf
>  create mode 100644 Silicon/Phytium/PhytiumCommonPkg/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
>  create mode 100644 Silicon/Phytium/PhytiumCommonPkg/Drivers/FlashFvbDxe/FlashFvbDxe.inf
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/SpiDxe/SpiDxe.h
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/SpiNorFlashDxe/SpiNorFlashDxe.h
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Library/RealTimeClockLib/RealTimeClockLib.h
>  create mode 100644 Silicon/Phytium/PhytiumCommonPkg/Drivers/FlashFvbDxe/FlashFvbDxe.h
>  create mode 100644 Silicon/Phytium/PhytiumCommonPkg/Include/Platform.h
>  create mode 100644 Silicon/Phytium/PhytiumCommonPkg/Include/Protocol/SpiNorFlashProtocol.h
>  create mode 100644 Silicon/Phytium/PhytiumCommonPkg/Include/Protocol/SpiProtocol.h
>  create mode 100644 Silicon/Phytium/PhytiumCommonPkg/Include/SystemServiceInterface.h
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/SmbiosPlatformDxe/SmbiosPlatformDxe.c
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/SpiDxe/SpiDxe.c
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/SpiNorFlashDxe/SpiNorFlashDxe.c
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Library/PciSegmentLib/PciSegmentLib.c
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Library/PlatformLib/PlatformLib.c
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Library/PlatformLib/PlatformLibMem.c
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Library/RealTimeClockLib/RealTimeClockLib.c
>  create mode 100644 Silicon/Phytium/PhytiumCommonPkg/Drivers/AcpiPlatformDxe/AcpiPlatform.c
>  create mode 100644 Silicon/Phytium/PhytiumCommonPkg/Drivers/FlashFvbDxe/FlashFvbDxe.c
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/AcpiSsdtRootPci.asl
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Dbg2.aslc
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Dsdt/Cpu.asl
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Dsdt/Dsdt.asl
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Dsdt/Uart.asl
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Fadt.aslc
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Gtdt.aslc
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Iort.aslc
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Madt.aslc
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Mcfg.aslc
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Pptt.aslc
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Drivers/AcpiTables/Spcr.aslc
>  create mode 100644 Silicon/Phytium/FT2000-4Pkg/Library/PlatformLib/AArch64/PhytiumPlatformHelper.S
>  create mode 100644 Silicon/Phytium/PhytiumCommonPkg/PhytiumCommonPkg.fdf.inc
> 
> -- 
> 2.25.1
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#75809): https://edk2.groups.io/g/devel/message/75809
Mute This Topic: https://groups.io/mt/81410910/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list