[PATCH v7 19/22] sifive: dts: fu540: Enable L2 Cache in U-Boot

[Bin Meng]

Hi Jagan,

On Mon, May 11, 2020 at 4:48 PM Jagan Teki <jagan at amarulasolutions.com> wrote:
>
> On Mon, May 11, 2020 at 1:15 PM Pragnesh Patel
> <pragnesh.patel at sifive.com> wrote:
> >
> > >-----Original Message-----
> > >From: Jagan Teki <jagan at amarulasolutions.com>
> > >Sent: 11 May 2020 12:56
> > >To: Pragnesh Patel <pragnesh.patel at sifive.com>
> > >Cc: U-Boot-Denx <u-boot at lists.denx.de>; Atish Patra
> > ><atish.patra at wdc.com>; Palmer Dabbelt <palmerdabbelt at google.com>; Bin
> > >Meng <bmeng.cn at gmail.com>; Paul Walmsley <paul.walmsley at sifive.com>;
> > >Troy Benjegerdes <troy.benjegerdes at sifive.com>; Anup Patel
> > ><anup.patel at wdc.com>; Sagar Kadam <sagar.kadam at sifive.com>; Rick Chen
> > ><rick at andestech.com>
> > >Subject: Re: [PATCH v7 19/22] sifive: dts: fu540: Enable L2 Cache in U-Boot
> > >
> > >[External Email] Do not click links or attachments unless you recognize the
> > >sender and know the content is safe
> > >
> > >On Mon, May 11, 2020 at 12:37 PM Pragnesh Patel
> > ><pragnesh.patel at sifive.com> wrote:
> > >>
> > >> >-----Original Message-----
> > >> >From: Jagan Teki <jagan at amarulasolutions.com>
> > >> >Sent: 11 May 2020 12:25
> > >> >To: Pragnesh Patel <pragnesh.patel at sifive.com>
> > >> >Cc: U-Boot-Denx <u-boot at lists.denx.de>; Atish Patra
> > >> ><atish.patra at wdc.com>; Palmer Dabbelt <palmerdabbelt at google.com>;
> > >Bin
> > >> >Meng <bmeng.cn at gmail.com>; Paul Walmsley
> > ><paul.walmsley at sifive.com>;
> > >> >Troy Benjegerdes <troy.benjegerdes at sifive.com>; Anup Patel
> > >> ><anup.patel at wdc.com>; Sagar Kadam <sagar.kadam at sifive.com>; Rick
> > >Chen
> > >> ><rick at andestech.com>
> > >> >Subject: Re: [PATCH v7 19/22] sifive: dts: fu540: Enable L2 Cache in
> > >> >U-Boot
> > >> >
> > >> >[External Email] Do not click links or attachments unless you
> > >> >recognize the sender and know the content is safe
> > >> >
> > >> >On Mon, May 11, 2020 at 11:35 AM Pragnesh Patel
> > >> ><pragnesh.patel at sifive.com> wrote:
> > >> >>
> > >> >> >-----Original Message-----
> > >> >> >From: Jagan Teki <jagan at amarulasolutions.com>
> > >> >> >Sent: 10 May 2020 15:02
> > >> >> >To: Pragnesh Patel <pragnesh.patel at sifive.com>
> > >> >> >Cc: U-Boot-Denx <u-boot at lists.denx.de>; Atish Patra
> > >> >> ><atish.patra at wdc.com>; Palmer Dabbelt
> > ><palmerdabbelt at google.com>;
> > >> >Bin
> > >> >> >Meng <bmeng.cn at gmail.com>; Paul Walmsley
> > >> ><paul.walmsley at sifive.com>;
> > >> >> >Troy Benjegerdes <troy.benjegerdes at sifive.com>; Anup Patel
> > >> >> ><anup.patel at wdc.com>; Sagar Kadam <sagar.kadam at sifive.com>; Rick
> > >> >Chen
> > >> >> ><rick at andestech.com>
> > >> >> >Subject: Re: [PATCH v7 19/22] sifive: dts: fu540: Enable L2 Cache
> > >> >> >in U-Boot
> > >> >> >
> > >> >> >[External Email] Do not click links or attachments unless you
> > >> >> >recognize the sender and know the content is safe
> > >> >> >
> > >> >> >On Sun, May 3, 2020 at 12:57 PM Pragnesh Patel
> > >> >> ><pragnesh.patel at sifive.com> wrote:
> > >> >> >>
> > >> >> >> Hi jagan,
> > >> >> >>
> > >> >> >> >-----Original Message-----
> > >> >> >> >From: Jagan Teki <jagan at amarulasolutions.com>
> > >> >> >> >Sent: 02 May 2020 22:43
> > >> >> >> >To: Pragnesh Patel <pragnesh.patel at sifive.com>
> > >> >> >> >Cc: U-Boot-Denx <u-boot at lists.denx.de>; Atish Patra
> > >> >> >> ><atish.patra at wdc.com>; Palmer Dabbelt
> > >> ><palmerdabbelt at google.com>;
> > >> >> >Bin
> > >> >> >> >Meng <bmeng.cn at gmail.com>; Paul Walmsley
> > >> >> ><paul.walmsley at sifive.com>;
> > >> >> >> >Troy Benjegerdes <troy.benjegerdes at sifive.com>; Anup Patel
> > >> >> >> ><anup.patel at wdc.com>; Sagar Kadam <sagar.kadam at sifive.com>;
> > >> >> >> >Rick
> > >> >> >Chen
> > >> >> >> ><rick at andestech.com>
> > >> >> >> >Subject: Re: [PATCH v7 19/22] sifive: dts: fu540: Enable L2
> > >> >> >> >Cache in U-Boot
> > >> >> >> >
> > >> >> >> >[External Email] Do not click links or attachments unless you
> > >> >> >> >recognize the sender and know the content is safe
> > >> >> >> >
> > >> >> >> >On Sat, May 2, 2020 at 10:12 PM Pragnesh Patel
> > >> >> >> ><pragnesh.patel at sifive.com>
> > >> >> >> >wrote:
> > >> >> >> >>
> > >> >> >> >> Hi Jagan,
> > >> >> >> >>
> > >> >> >> >> >-----Original Message-----
> > >> >> >> >> >From: Jagan Teki <jagan at amarulasolutions.com>
> > >> >> >> >> >Sent: 02 May 2020 21:49
> > >> >> >> >> >To: Pragnesh Patel <pragnesh.patel at sifive.com>
> > >> >> >> >> >Cc: U-Boot-Denx <u-boot at lists.denx.de>; Atish Patra
> > >> >> >> >> ><atish.patra at wdc.com>; Palmer Dabbelt
> > >> >> ><palmerdabbelt at google.com>;
> > >> >> >> >Bin
> > >> >> >> >> >Meng <bmeng.cn at gmail.com>; Paul Walmsley
> > >> >> >> ><paul.walmsley at sifive.com>;
> > >> >> >> >> >Troy Benjegerdes <troy.benjegerdes at sifive.com>; Anup Patel
> > >> >> >> >> ><anup.patel at wdc.com>; Sagar Kadam
> > ><sagar.kadam at sifive.com>;
> > >> >> >> >> >Rick
> > >> >> >> >Chen
> > >> >> >> >> ><rick at andestech.com>
> > >> >> >> >> >Subject: Re: [PATCH v7 19/22] sifive: dts: fu540: Enable L2
> > >> >> >> >> >Cache in U-Boot
> > >> >> >> >> >
> > >> >> >> >> >[External Email] Do not click links or attachments unless
> > >> >> >> >> >you recognize the sender and know the content is safe
> > >> >> >> >> >
> > >> >> >> >> >On Sat, May 2, 2020 at 3:39 PM Pragnesh Patel
> > >> >> >> >> ><pragnesh.patel at sifive.com>
> > >> >> >> >> >wrote:
> > >> >> >> >> >>
> > >> >> >> >> >> Add L2 cache node to enable cache ways from U-Boot
> > >> >> >> >> >
> > >> >> >> >> >This and 20/22 doesn't relate to SPL MMC boot?, if yes
> > >> >> >> >> >please send them separately.
> > >> >> >> >>
> > >> >> >> >> This series is for replacing FSBL and all the patches are related to
> > >that.
> > >> >> >> >> IMHO it's better to add all FSBL functionality in one series.
> > >> >> >> >
> > >> >> >> >You mean does it break existing FSBL flow? if yes add proper
> > >> >> >> >commit message, but I am able to boot SPL MMC w/o this?
> > >> >> >>
> > >> >> >> Cache ways are enabled by FSBL also and if I will send cache
> > >> >> >> ways patches separately then it will a duplicate way of enabling
> > >> >> >> cache ways if
> > >> >> >someone using FSBL.
> > >> >> >
> > >> >> >Sorry I didn't get you.
> > >> >> >
> > >> >> >If we cannot include these changes does U-Boot SPL break existing FSBL?
> > >> >>
> > >> >> No, U-Boot SPL does not break without this.
> > >> >>
> > >> >> As of now, we also want to support FSBL flow and FSBL also enabled
> > >> >> the Cache ways for U-Boot proper and if someone use this patches of
> > >> >> L2 cache enable ways will FSBL then it will be a duplicate work of
> > >> >> cache enable
> > >> >ways.
> > >> >
> > >> >My question is what if we don't add this change at all?
> > >>
> > >> U-Boot SPL will work without L2 cache enable patches but why we want to
> > >do this.
> > >> This series is not just for SPL mmc booting but also replacing FSBL
> > >> functionality so better to cover all FSBL stuff in one series.
> > >
> > >So, FSBL flow would break if we add U-Boot SPL so this patch fixing by enabling
> > >cache's explicitly to avoid that break. isn't it?
> >
> > No, you are not getting this.
> >
> > Initially FSBL enable the cache ways before U-Boot SPL.
> > https://github.com/sifive/freedom-u540-c000-bootloader/blob/master/fsbl/main.c#L428
>
> This is not Mainline. enabling cache's are a new thing for Mainline
> for you. So this code is pretty much new. Is that clear?
>
> >
> > Now U-Boot SPL doing the same in [v7 19/22] and [v7 20/22].
>
> But these two patches enable caches for U-Boot proper in case of
> U-Boot SPL flow and U-Boot in case of FSBL. am I correct? if so this
> code is purely U-Boot specific neither FSBL nor U-Boot SPL. then what
> is the problem of having a series or separate patches with cache
> enablement.

My understanding is that:

1. This patch series is adding SiFive FU540 U-Boot SPL support
2. The purpose is to replace SiFive provided FSBL with U-Boot SPL
3. SiFive FSBL initializes L2 cache before loading next stage payload
4. U-Boot SPL should provide the same features, like initializing L2 cache

I don't think we should separate the patch to another series.

Regards,
Bin