[PATCH v7 0/8] Add FU740 chip and HiFive Unmatched board support

Green Wan green.wan at sifive.com
Mon May 3 05:35:28 CEST 2021


Hi Rick,

Thanks for quick response. See my reply below.

On Mon, May 3, 2021 at 10:34 AM Rick Chen <rickchen36 at 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 at 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-green.wan@sifive.com/
>
> [v7,7/8] board: sifive: add HiFive Unmatched board support
> https://patchwork.ozlabs.org/project/uboot/patch/20210422091202.396956-8-green.wan@sifive.com/
>
> 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 at gmail.com>
> > Sent: Thursday, April 29, 2021 8:27 PM
> > To: Green Wan <green.wan at sifive.com>
> > Cc: Rick Jian-Zhi Chen(陳建志) <rick at andestech.com>; Paul Walmsley <paul.walmsley at sifive.com>; Palmer Dabbelt <palmer at dabbelt.com>; Anup Patel <anup.patel at wdc.com>; Atish Patra <atish.patra at wdc.com>; Lukasz Majewski <lukma at denx.de>; Joe Hershberger <joe.hershberger at ni.com>; Ramon Fried <rfried.dev at gmail.com>; U-Boot Mailing List <u-boot at 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 at 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


More information about the U-Boot mailing list