From: "Leif Lindholm" <quic_llindhol@quicinc.com>
To: <devel@edk2.groups.io>, <caiyuqing_hz@163.com>
Cc: <sunilvl@ventanamicro.com>,
caiyuqing379 <202235273@mail.sdu.edu.cn>,
USER0FISH <libing1202@outlook.com>
Subject: Re: [edk2-devel] [PATCH v2 0/8] EDK2 on RISC-V Sophgo SG2042 platform
Date: Mon, 4 Sep 2023 16:16:46 +0100 [thread overview]
Message-ID: <ZPX03tXNiGTDonrl@qc-i7.hemma.eciton.net> (raw)
In-Reply-To: <cover.1693483202.git.202235273@mail.sdu.edu.cn>
Hi Yuqing (?),
Thank you for this contribution.
I have a few high-level comments, but I will also look into the
individual patches and comment if I find things I feel will make the
port easier to maintain.
1) Git
If it is possible for you to push these patches to some public git
tree, that makes it easier for us to deal with the binary files (the
.png files in 8/8). If you do so, you can add a link to the branch in
the 0/8 blurb
On Thu, Aug 31, 2023 at 21:44:16 +0800, caiyuqing_hz@163.com wrote:
> From: caiyuqing379 <202235273@mail.sdu.edu.cn>
>
> Signed-off-by: caiyuqing379 <202235273@mail.sdu.edu.cn>
> Co-authored-by: USER0FISH <libing1202@outlook.com>
> Cc: dahogn <dahogn@hotmail.com>
> Cc: meng-cz <mengcz1126@gmail.com>
> Cc: yli147 <yong.li@intel.com>
> Cc: ChaiEvan <evan.chai@intel.com>
> Cc: Sunil V L <sunilvl@ventanamicro.com>
>
> *** BLURB HERE ***
2) Blurb
It would be nice if the blurb could be replaced by a small description
of what is being contributed, and why.
For example, some quick searching on the Internet tells me that the
SoC in question is the same as will be in the Milk-V Pioneer board.
Is the EVB compatible with the Pioneer board?
If not, are you planning to add support for the Pioneer board later?
3) Layout
The layout of this port does not follow the usual pattern for edk2
platform ports. We split SoC support (Silicon/) apart from platform
port (Platform/).
Superficially, it looks like it would make sense to move most of the
current port from
Platform/Sophgo/SG2042Pkg/
to
Silicon/Sophgo/SG2042Pkg/
except for
Platform/Sophgo/SG2042Pkg/SG2042_EVB_Board/
which would make sense to keep under Platform/ as
Platform/Sophgo/SG2042_EVB_Board/
(Knowing what the plan is for future work would also help us to
provide useful giudance.)
4) Clang toolchain support
I attempted to build the port using the CLANGDWARF toolchain profile.
This leads to the same problem as described by
https://www.mail-archive.com/devel@edk2.groups.io/msg61953.html
and the same solution makes the build succeed. (I don't know if the
resulting build *works*.)
Best Regards,
Leif
> caiyuqing379 (8):
> Sophgo/SG2042Pkg: Add SmbiosPlatformDxe module.
> Sophgo/SG2042Pkg: Add PlatformUpdateMmuDxe module.
> Sophgo/SG2042Pkg: Add Sophgo SDHCI driver.
> Sophgo/SG2042Pkg: Add base MMC driver.
> Sophgo/SG2042Pkg: Add SEC module.
> SG2042Pkg/SG2042_EVB_Board: Add Sophgo SG2042 platform.
> Sophgo/SG2042Pkg: Add SG2042Pkg.
> Sophgo/SG2042Pkg: Add platform readme and document.
>
> Platform/Sophgo/SG2042Pkg/SG2042Pkg.dec | 35 +
> .../SG2042Pkg/SG2042_EVB_Board/SG2042.dec | 19 +
> .../SG2042Pkg/SG2042_EVB_Board/SG2042.dsc | 548 +++++++++++
> .../SG2042Pkg/SG2042_EVB_Board/SG2042.fdf | 247 +++++
> Platform/Sophgo/SG2042Pkg/Sec/SecMain.inf | 68 ++
> .../SG2042Pkg/Universal/Dxe/MmcDxe/MmcDxe.inf | 46 +
> .../PlatformUpdateMmuDxe.inf | 34 +
> .../Universal/Dxe/SdHostDxe/SdHostDxe.inf | 47 +
> .../SmbiosPlatformDxe/SmbiosPlatformDxe.inf | 39 +
> Platform/Sophgo/SG2042Pkg/Include/MmcHost.h | 225 +++++
> Platform/Sophgo/SG2042Pkg/Sec/SecMain.h | 103 ++
> .../SG2042Pkg/Universal/Dxe/MmcDxe/Mmc.h | 513 ++++++++++
> .../SG2042Pkg/Universal/Dxe/SdHostDxe/SdHci.h | 309 ++++++
> Platform/Sophgo/SG2042Pkg/Sec/Cpu.c | 29 +
> Platform/Sophgo/SG2042Pkg/Sec/Memory.c | 363 +++++++
> Platform/Sophgo/SG2042Pkg/Sec/Platform.c | 141 +++
> Platform/Sophgo/SG2042Pkg/Sec/SecMain.c | 116 +++
> .../Universal/Dxe/MmcDxe/ComponentName.c | 156 +++
> .../Universal/Dxe/MmcDxe/Diagnostics.c | 323 ++++++
> .../SG2042Pkg/Universal/Dxe/MmcDxe/Mmc.c | 527 ++++++++++
> .../Universal/Dxe/MmcDxe/MmcBlockIo.c | 643 ++++++++++++
> .../SG2042Pkg/Universal/Dxe/MmcDxe/MmcDebug.c | 194 ++++
> .../Universal/Dxe/MmcDxe/MmcIdentification.c | 719 ++++++++++++++
> .../PlatformUpdateMmuDxe.c | 591 +++++++++++
> .../SG2042Pkg/Universal/Dxe/SdHostDxe/SdHci.c | 929 ++++++++++++++++++
> .../Universal/Dxe/SdHostDxe/SdHostDxe.c | 450 +++++++++
> .../Dxe/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 805 +++++++++++++++
> .../Sophgo/SG2042Pkg/About_Sophgo_platform.md | 39 +
> .../Documents/Media/EDK2_SDU_Programme.png | Bin 0 -> 59830 bytes
> .../SG2042Pkg/Documents/Media/SG2042_CPU.png | Bin 0 -> 806062 bytes
> .../Documents/Media/Sophgo_SG2042_EVB.png | Bin 0 -> 1445528 bytes
> Platform/Sophgo/SG2042Pkg/Maintainers.md | 107 ++
> Platform/Sophgo/SG2042Pkg/Readme.md | 79 ++
> Platform/Sophgo/SG2042Pkg/SG2042Pkg.uni | 13 +
> Platform/Sophgo/SG2042Pkg/SG2042PkgExtra.uni | 12 +
> .../SG2042Pkg/SG2042_EVB_Board/SG2042.fdf.inc | 62 ++
> .../SG2042_EVB_Board/VarStore.fdf.inc | 77 ++
> Platform/Sophgo/SG2042Pkg/Sec/SecEntry.S | 18 +
> 38 files changed, 8626 insertions(+)
> create mode 100644 Platform/Sophgo/SG2042Pkg/SG2042Pkg.dec
> create mode 100644 Platform/Sophgo/SG2042Pkg/SG2042_EVB_Board/SG2042.dec
> create mode 100644 Platform/Sophgo/SG2042Pkg/SG2042_EVB_Board/SG2042.dsc
> create mode 100644 Platform/Sophgo/SG2042Pkg/SG2042_EVB_Board/SG2042.fdf
> create mode 100644 Platform/Sophgo/SG2042Pkg/Sec/SecMain.inf
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/MmcDxe/MmcDxe.inf
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/PlatformUpdateMmuDxe/PlatformUpdateMmuDxe.inf
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/SdHostDxe/SdHostDxe.inf
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> create mode 100644 Platform/Sophgo/SG2042Pkg/Include/MmcHost.h
> create mode 100644 Platform/Sophgo/SG2042Pkg/Sec/SecMain.h
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/MmcDxe/Mmc.h
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/SdHostDxe/SdHci.h
> create mode 100644 Platform/Sophgo/SG2042Pkg/Sec/Cpu.c
> create mode 100644 Platform/Sophgo/SG2042Pkg/Sec/Memory.c
> create mode 100644 Platform/Sophgo/SG2042Pkg/Sec/Platform.c
> create mode 100644 Platform/Sophgo/SG2042Pkg/Sec/SecMain.c
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/MmcDxe/ComponentName.c
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/MmcDxe/Diagnostics.c
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/MmcDxe/Mmc.c
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/MmcDxe/MmcBlockIo.c
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/MmcDxe/MmcDebug.c
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/MmcDxe/MmcIdentification.c
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/PlatformUpdateMmuDxe/PlatformUpdateMmuDxe.c
> create mode 100755 Platform/Sophgo/SG2042Pkg/Universal/Dxe/SdHostDxe/SdHci.c
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/SdHostDxe/SdHostDxe.c
> create mode 100644 Platform/Sophgo/SG2042Pkg/Universal/Dxe/SmbiosPlatformDxe/SmbiosPlatformDxe.c
> create mode 100644 Platform/Sophgo/SG2042Pkg/About_Sophgo_platform.md
> create mode 100644 Platform/Sophgo/SG2042Pkg/Documents/Media/EDK2_SDU_Programme.png
> create mode 100644 Platform/Sophgo/SG2042Pkg/Documents/Media/SG2042_CPU.png
> create mode 100644 Platform/Sophgo/SG2042Pkg/Documents/Media/Sophgo_SG2042_EVB.png
> create mode 100644 Platform/Sophgo/SG2042Pkg/Maintainers.md
> create mode 100644 Platform/Sophgo/SG2042Pkg/Readme.md
> create mode 100644 Platform/Sophgo/SG2042Pkg/SG2042Pkg.uni
> create mode 100644 Platform/Sophgo/SG2042Pkg/SG2042PkgExtra.uni
> create mode 100644 Platform/Sophgo/SG2042Pkg/SG2042_EVB_Board/SG2042.fdf.inc
> create mode 100644 Platform/Sophgo/SG2042Pkg/SG2042_EVB_Board/VarStore.fdf.inc
> create mode 100644 Platform/Sophgo/SG2042Pkg/Sec/SecEntry.S
>
> --
> 2.34.1
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108262): https://edk2.groups.io/g/devel/message/108262
Mute This Topic: https://groups.io/mt/101084028/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2023-09-04 15:17 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-31 13:44 [edk2-devel] [PATCH v2 0/8] EDK2 on RISC-V Sophgo SG2042 platform caiyuqing_hz
2023-08-31 13:44 ` [edk2-devel] [PATCH v2 1/8] Sophgo/SG2042Pkg: Add SmbiosPlatformDxe module caiyuqing_hz
2023-08-31 13:44 ` [edk2-devel] [PATCH v2 2/8] Sophgo/SG2042Pkg: Add PlatformUpdateMmuDxe module caiyuqing_hz
2023-08-31 13:44 ` [edk2-devel] [PATCH v2 3/8] Sophgo/SG2042Pkg: Add Sophgo SDHCI driver caiyuqing_hz
2023-08-31 13:44 ` [edk2-devel] [PATCH v2 4/8] Sophgo/SG2042Pkg: Add base MMC driver caiyuqing_hz
2023-08-31 13:44 ` [edk2-devel] [PATCH v2 5/8] Sophgo/SG2042Pkg: Add SEC module caiyuqing_hz
2023-08-31 13:44 ` [edk2-devel] [PATCH v2 6/8] SG2042Pkg/SG2042_EVB_Board: Add Sophgo SG2042 platform caiyuqing_hz
2023-08-31 13:44 ` [edk2-devel] [PATCH v2 7/8] Sophgo/SG2042Pkg: Add SG2042Pkg caiyuqing_hz
2023-08-31 13:44 ` [edk2-devel] [PATCH v2 8/8] Sophgo/SG2042Pkg: Add platform readme and document caiyuqing_hz
2023-09-04 15:16 ` Leif Lindholm [this message]
2023-09-07 10:33 ` [edk2-devel] [PATCH v2 0/8] EDK2 on RISC-V Sophgo SG2042 platform caiyuqing_hz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZPX03tXNiGTDonrl@qc-i7.hemma.eciton.net \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox