Hi Rick,
Thanks for quick response. See my reply below.
On Mon, May 3, 2021 at 10:34 AM Rick Chen rickchen36@gmail.com wrote:
Hi Green,
I did not sign the Reviewed-by for this patch "board: sifive: add HiFive Unmatched board support" from v1 to v6. But it just has been tagged in [v7,7/8] board: sifive: add HiFive Unmatched board support by yourself.
Sorry, I might have a quick conclusion when Reviewed-by is on our previous discussion in [v6 1/7]. We ended up splitting dts into two. One for fu740 and the other for Unmatched board specifically. And put them closer. Then, I added the review-by to them.
---- Here is the previous discussion. ---- "Makefile need the dts file, but it is not exist in this patch. It doesn't make sense.
Maybe you can combine with the dts relative files in [PATCH v6 6/7] into one patch and name as : riscv: dts: ...
LGTM. Other than that,
Reviewed-by: Rick Chen rick@andestech.com "
"It is OK.
You may arrange them nearby as below: 6/x riscv: dts: support fu740 7/x riscv: dst: support HiFive Unmatched board
Thanks, Rick"
[v6,6/7] board: sifive: add HiFive Unmatched board support https://patchwork.ozlabs.org/project/uboot/patch/20210408134020.238658-7-gre...
[v7,7/8] board: sifive: add HiFive Unmatched board support https://patchwork.ozlabs.org/project/uboot/patch/20210422091202.396956-8-gre...
Actually I don't like this patch that you mix every things (arch/, drivers/, common/, doc/)together in this patch. But it is OK for now.
Noted. Thanks for reminding me.
BTW, in [PATCH v7 1/8] riscv: cpu: fu740: Add support for cpu fu740 I found that arch/riscv/cpu/fu740/cpu.c and arch/riscv/fu540/cpu.c are 100% the same.
And about spl.c, they are only different in the annotation of Copyright diff fu540/spl.c fu740/spl.c 3c3
< * Copyright (C) 2020 SiFive, Inc
- Copyright (C) 2020-201 SiFive, Inc
About the cache.c, they are just different in one character
diff fu540/cache.c fu740/cache.c 3c3
< * Copyright (C) 2020 SiFive, Inc
- Copyright (C) 2020-2021 SiFive, Inc
10d9 < #include <asm/global_data.h> 12a12
#include <asm/global_data.h>
34c34
< "sifive,fu540-c000-ccache");
"sifive,fu740-c000-ccache");Originally, I am considering to tell you to re-use the same code base instead of just copy and create. After a few days of consideration, I feel it's OK for now.
You're right. As you mentioned, I also considered having common code. But I conservatively decided to keep separated files to handle possible differences among chips.
About [PATCH v7 2/8] drivers: clk: add fu740 support and [PATCH v7 4/8] drivers: pci: add pcie support for fu740, there are still not get any Reviewed-by till now. For me, it will be better if someone can tag a Reviewed-by here.
Principally, it will be suggested to split drivers from RISC-V relevant, do not mix them together as Palmer said.
Not sure whether we'll have the driver reviewers very quick. What do you think of moving forward? I can try to ping some driver maintainers and see if we have someone to review them. =]
Thanks,
Thanks, Rick
From: Bin Meng bmeng.cn@gmail.com Sent: Thursday, April 29, 2021 8:27 PM To: Green Wan green.wan@sifive.com Cc: Rick Jian-Zhi Chen(陳建志) rick@andestech.com; Paul Walmsley paul.walmsley@sifive.com; Palmer Dabbelt palmer@dabbelt.com; Anup Patel anup.patel@wdc.com; Atish Patra atish.patra@wdc.com; Lukasz Majewski lukma@denx.de; Joe Hershberger joe.hershberger@ni.com; Ramon Fried rfried.dev@gmail.com; U-Boot Mailing List u-boot@lists.denx.de Subject: Re: [PATCH v7 0/8] Add FU740 chip and HiFive Unmatched board support
Hi Green,
On Thu, Apr 29, 2021 at 7:11 PM Green Wan green.wan@sifive.com wrote:
Hi Bin,
How should this patch set be proceeded?
To summary the major changes,
- I've rebased to mainstream and merged pcie refactoring code which
based on pcie_dw_common.c
- separate unmatched dts into separated patch.
I don't have specific comments. Rick should pick this up via the riscv tree. Thanks!
Regards, Bin