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

[Bin Meng]

Hi Pragnesh,

On Mon, May 11, 2020 at 6:35 PM Pragnesh Patel
<pragnesh.patel at sifive.com> wrote:
>
> >-----Original Message-----
> >From: Bin Meng <bmeng.cn at gmail.com>
> >Sent: 11 May 2020 15:41
> >To: Pragnesh Patel <pragnesh.patel at sifive.com>
> >Cc: Jagan Teki <jagan at amarulasolutions.com>; U-Boot-Denx <u-
> >boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; Palmer Dabbelt
> ><palmerdabbelt at google.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
> >
> >Hi Pragnesh,
> >
> >On Mon, May 11, 2020 at 5:55 PM Pragnesh Patel
> ><pragnesh.patel at sifive.com> wrote:
> >>
> >> >-----Original Message-----
> >> >From: Bin Meng <bmeng.cn at gmail.com>
> >> >Sent: 11 May 2020 15:17
> >> >To: Pragnesh Patel <pragnesh.patel at sifive.com>
> >> >Cc: Jagan Teki <jagan at amarulasolutions.com>; U-Boot-Denx <u-
> >> >boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; Palmer
> >> >Dabbelt <palmerdabbelt at google.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
> >> >
> >> >Hi Pragnesh,
> >> >
> >> >On Mon, May 11, 2020 at 5:34 PM Pragnesh Patel
> >> ><pragnesh.patel at sifive.com> wrote:
> >> >>
> >> >> >-----Original Message-----
> >> >> >From: Jagan Teki <jagan at amarulasolutions.com>
> >> >> >Sent: 11 May 2020 14:35
> >> >> >To: Bin Meng <bmeng.cn at gmail.com>
> >> >> >Cc: Pragnesh Patel <pragnesh.patel at sifive.com>; U-Boot-Denx <u-
> >> >> >boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; Palmer
> >> >> >Dabbelt <palmerdabbelt at google.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
> >> >> >
> >> >> >Hi Bin,
> >> >> >
> >> >> >On Mon, May 11, 2020 at 2:30 PM Bin Meng <bmeng.cn at gmail.com>
> >> >wrote:
> >> >> >>
> >> >> >> 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/
> >> >> >> > > mas
> >> >> >> > > ter
> >> >> >> > > /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
> >> >> >
> >> >> >So, you mean U-Boot SPL would enable the cache similar like FSBL
> >> >> >does
> >> >right?
> >> >> >but this patch [1] enabling cache only for U-Boot not SPL isn't it?
> >> >> >
> >> >> >[1]
> >> >>
> >>
> >>>https://patchwork.ozlabs.org/project/uboot/patch/20200502100628.2480
> >> >>9
> >> >> >-
> >> >> >20-pragnesh.patel at sifive.com/
> >> >>
> >> >> Yes, you are right. Cache ways are enabled by U-Boot proper not by
> >> >> U-Boot
> >> >SPL.
> >> >> Will pull cache related patches out of this series and send as
> >> >> separate
> >> >patches.
> >> >>
> >> >
> >> >Now I am confused. Are you saying that FSBL does NOT enable L2 cache?
> >> >Based on your statement in this review thread I think you basically
> >> >wanted to replace FSBL with the same featured U-Boot SPL.
> >>
> >> For FSBL flow,
> >> Cache ways are enabled by FSBL and with this series cache ways are
> >> again enabled by U-Boot Proper. Once this series has been merged, we
> >> may remove cache enable ways from FSBL or I Will apply check in U-Boot
> >proper for already enabled cache ways.
> >>
> >> For SPL flow,
> >> Cache ways are enabled by U-Boot proper. We can not enable cache ways
> >> from U-Boot SPL because SPL is running from L2 LIM.
> >>
> >
> >Thanks for the clarification. I wonder where is the FSBL running. Can U-Boot
> >SPL run from where FSBL is running instead of L2 LIM?
>
> FSBL and U-Boot SPL running from the same location.
>
> L2 cache is of 2 MB (16 cache ways) and 1 cache way is of 128 KB.
>
> At runtime, FSBL is located on the latest way of L2 cache. Therefore,
> FSBL can only enable the first 15 L2 cache ways to avoid corrupt itself.
> (Way 0 is enabled by default)
> https://github.com/sifive/freedom-u540-c000-bootloader/blob/master/fsbl/main.c#L428
>
> U-Boot SPL is bigger than FSBL so can not fit in 1 cache way, so I decided to enable all cache ways from
> U-Boot proper.
>

Thanks! This makes sense.

> >
> >> @Jagan Teki Sorry for my misunderstanding, U-Boot SPL got stuck in my
> >mind.

Regards,
Bin