Hi Pragnesh, On Mon, May 11, 2020 at 6:35 PM Pragnesh Patel <pragnesh.pa...@sifive.com> wrote: > > >-----Original Message----- > >From: Bin Meng <bmeng...@gmail.com> > >Sent: 11 May 2020 15:41 > >To: Pragnesh Patel <pragnesh.pa...@sifive.com> > >Cc: Jagan Teki <ja...@amarulasolutions.com>; U-Boot-Denx <u- > >b...@lists.denx.de>; Atish Patra <atish.pa...@wdc.com>; Palmer Dabbelt > ><palmerdabb...@google.com>; Paul Walmsley <paul.walms...@sifive.com>; > >Troy Benjegerdes <troy.benjeger...@sifive.com>; Anup Patel > ><anup.pa...@wdc.com>; Sagar Kadam <sagar.ka...@sifive.com>; Rick Chen > ><r...@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.pa...@sifive.com> wrote: > >> > >> >-----Original Message----- > >> >From: Bin Meng <bmeng...@gmail.com> > >> >Sent: 11 May 2020 15:17 > >> >To: Pragnesh Patel <pragnesh.pa...@sifive.com> > >> >Cc: Jagan Teki <ja...@amarulasolutions.com>; U-Boot-Denx <u- > >> >b...@lists.denx.de>; Atish Patra <atish.pa...@wdc.com>; Palmer > >> >Dabbelt <palmerdabb...@google.com>; Paul Walmsley > >> ><paul.walms...@sifive.com>; Troy Benjegerdes > >> ><troy.benjeger...@sifive.com>; Anup Patel <anup.pa...@wdc.com>; Sagar > >> >Kadam <sagar.ka...@sifive.com>; Rick Chen <r...@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.pa...@sifive.com> wrote: > >> >> > >> >> >-----Original Message----- > >> >> >From: Jagan Teki <ja...@amarulasolutions.com> > >> >> >Sent: 11 May 2020 14:35 > >> >> >To: Bin Meng <bmeng...@gmail.com> > >> >> >Cc: Pragnesh Patel <pragnesh.pa...@sifive.com>; U-Boot-Denx <u- > >> >> >b...@lists.denx.de>; Atish Patra <atish.pa...@wdc.com>; Palmer > >> >> >Dabbelt <palmerdabb...@google.com>; Paul Walmsley > >> >> ><paul.walms...@sifive.com>; Troy Benjegerdes > >> >> ><troy.benjeger...@sifive.com>; Anup Patel <anup.pa...@wdc.com>; > >> >> >Sagar Kadam <sagar.ka...@sifive.com>; Rick Chen > >> >> ><r...@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...@gmail.com> > >> >wrote: > >> >> >> > >> >> >> Hi Jagan, > >> >> >> > >> >> >> On Mon, May 11, 2020 at 4:48 PM Jagan Teki > >> >> ><ja...@amarulasolutions.com> wrote: > >> >> >> > > >> >> >> > On Mon, May 11, 2020 at 1:15 PM Pragnesh Patel > >> >> >> > <pragnesh.pa...@sifive.com> wrote: > >> >> >> > > > >> >> >> > > >-----Original Message----- > >> >> >> > > >From: Jagan Teki <ja...@amarulasolutions.com> > >> >> >> > > >Sent: 11 May 2020 12:56 > >> >> >> > > >To: Pragnesh Patel <pragnesh.pa...@sifive.com> > >> >> >> > > >Cc: U-Boot-Denx <u-boot@lists.denx.de>; Atish Patra > >> >> >> > > ><atish.pa...@wdc.com>; Palmer Dabbelt > >> >> ><palmerdabb...@google.com>; > >> >> >> > > >Bin Meng <bmeng...@gmail.com>; Paul Walmsley > >> >> >> > > ><paul.walms...@sifive.com>; Troy Benjegerdes > >> >> >> > > ><troy.benjeger...@sifive.com>; Anup Patel > >> >> >> > > ><anup.pa...@wdc.com>; Sagar Kadam > ><sagar.ka...@sifive.com>; > >> >> >> > > >Rick Chen <r...@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.pa...@sifive.com> wrote: > >> >> >> > > >> > >> >> >> > > >> >-----Original Message----- > >> >> >> > > >> >From: Jagan Teki <ja...@amarulasolutions.com> > >> >> >> > > >> >Sent: 11 May 2020 12:25 > >> >> >> > > >> >To: Pragnesh Patel <pragnesh.pa...@sifive.com> > >> >> >> > > >> >Cc: U-Boot-Denx <u-boot@lists.denx.de>; Atish Patra > >> >> >> > > >> ><atish.pa...@wdc.com>; Palmer Dabbelt > >> >> >> > > >> ><palmerdabb...@google.com>; > >> >> >> > > >Bin > >> >> >> > > >> >Meng <bmeng...@gmail.com>; Paul Walmsley > >> >> >> > > ><paul.walms...@sifive.com>; > >> >> >> > > >> >Troy Benjegerdes <troy.benjeger...@sifive.com>; Anup > >> >> >> > > >> >Patel <anup.pa...@wdc.com>; Sagar Kadam > >> ><sagar.ka...@sifive.com>; > >> >> >> > > >> >Rick > >> >> >> > > >Chen > >> >> >> > > >> ><r...@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.pa...@sifive.com> wrote: > >> >> >> > > >> >> > >> >> >> > > >> >> >-----Original Message----- > >> >> >> > > >> >> >From: Jagan Teki <ja...@amarulasolutions.com> > >> >> >> > > >> >> >Sent: 10 May 2020 15:02 > >> >> >> > > >> >> >To: Pragnesh Patel <pragnesh.pa...@sifive.com> > >> >> >> > > >> >> >Cc: U-Boot-Denx <u-boot@lists.denx.de>; Atish Patra > >> >> >> > > >> >> ><atish.pa...@wdc.com>; Palmer Dabbelt > >> >> >> > > ><palmerdabb...@google.com>; > >> >> >> > > >> >Bin > >> >> >> > > >> >> >Meng <bmeng...@gmail.com>; Paul Walmsley > >> >> >> > > >> ><paul.walms...@sifive.com>; > >> >> >> > > >> >> >Troy Benjegerdes <troy.benjeger...@sifive.com>; Anup > >> >> >> > > >> >> >Patel <anup.pa...@wdc.com>; Sagar Kadam > >> >> ><sagar.ka...@sifive.com>; > >> >> >> > > >> >> >Rick > >> >> >> > > >> >Chen > >> >> >> > > >> >> ><r...@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.pa...@sifive.com> wrote: > >> >> >> > > >> >> >> > >> >> >> > > >> >> >> Hi jagan, > >> >> >> > > >> >> >> > >> >> >> > > >> >> >> >-----Original Message----- > >> >> >> > > >> >> >> >From: Jagan Teki <ja...@amarulasolutions.com> > >> >> >> > > >> >> >> >Sent: 02 May 2020 22:43 > >> >> >> > > >> >> >> >To: Pragnesh Patel <pragnesh.pa...@sifive.com> > >> >> >> > > >> >> >> >Cc: U-Boot-Denx <u-boot@lists.denx.de>; Atish > >> >> >> > > >> >> >> >Patra <atish.pa...@wdc.com>; Palmer Dabbelt > >> >> >> > > >> ><palmerdabb...@google.com>; > >> >> >> > > >> >> >Bin > >> >> >> > > >> >> >> >Meng <bmeng...@gmail.com>; Paul Walmsley > >> >> >> > > >> >> ><paul.walms...@sifive.com>; > >> >> >> > > >> >> >> >Troy Benjegerdes <troy.benjeger...@sifive.com>; > >> >> >> > > >> >> >> >Anup Patel <anup.pa...@wdc.com>; Sagar Kadam > >> >> >> > > >> >> >> ><sagar.ka...@sifive.com>; Rick > >> >> >> > > >> >> >Chen > >> >> >> > > >> >> >> ><r...@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.pa...@sifive.com> > >> >> >> > > >> >> >> >wrote: > >> >> >> > > >> >> >> >> > >> >> >> > > >> >> >> >> Hi Jagan, > >> >> >> > > >> >> >> >> > >> >> >> > > >> >> >> >> >-----Original Message----- > >> >> >> > > >> >> >> >> >From: Jagan Teki <ja...@amarulasolutions.com> > >> >> >> > > >> >> >> >> >Sent: 02 May 2020 21:49 > >> >> >> > > >> >> >> >> >To: Pragnesh Patel <pragnesh.pa...@sifive.com> > >> >> >> > > >> >> >> >> >Cc: U-Boot-Denx <u-boot@lists.denx.de>; Atish > >> >> >> > > >> >> >> >> >Patra <atish.pa...@wdc.com>; Palmer Dabbelt > >> >> >> > > >> >> ><palmerdabb...@google.com>; > >> >> >> > > >> >> >> >Bin > >> >> >> > > >> >> >> >> >Meng <bmeng...@gmail.com>; Paul Walmsley > >> >> >> > > >> >> >> ><paul.walms...@sifive.com>; > >> >> >> > > >> >> >> >> >Troy Benjegerdes <troy.benjeger...@sifive.com>; > >> >> >> > > >> >> >> >> >Anup Patel <anup.pa...@wdc.com>; Sagar Kadam > >> >> >> > > ><sagar.ka...@sifive.com>; > >> >> >> > > >> >> >> >> >Rick > >> >> >> > > >> >> >> >Chen > >> >> >> > > >> >> >> >> ><r...@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.pa...@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.pa...@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