Re: [PATCH v7 0/8] Add FU740 chip and HiFive Unmatched board support
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.
[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.
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.
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.
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
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
Hi Green,
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. =]
OK. Maybe let's just wait for a period of time, if it still no furthermore response. I can help to pull them via riscv tree.
Thanks, Rick
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
participants (2)
-
Green Wan -
Rick Chen