>-----Original Message-----
>From: Bin Meng <[email protected]>
>Sent: 11 May 2020 15:17
>To: Pragnesh Patel <[email protected]>
>Cc: Jagan Teki <[email protected]>; U-Boot-Denx <u-
>[email protected]>; Atish Patra <[email protected]>; Palmer Dabbelt
><[email protected]>; Paul Walmsley <[email protected]>;
>Troy Benjegerdes <[email protected]>; Anup Patel
><[email protected]>; Sagar Kadam <[email protected]>; Rick Chen
><[email protected]>
>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
><[email protected]> wrote:
>>
>> >-----Original Message-----
>> >From: Jagan Teki <[email protected]>
>> >Sent: 11 May 2020 14:35
>> >To: Bin Meng <[email protected]>
>> >Cc: Pragnesh Patel <[email protected]>; U-Boot-Denx <u-
>> >[email protected]>; Atish Patra <[email protected]>; Palmer
>> >Dabbelt <[email protected]>; Paul Walmsley
>> ><[email protected]>; Troy Benjegerdes
>> ><[email protected]>; Anup Patel <[email protected]>; Sagar
>> >Kadam <[email protected]>; Rick Chen <[email protected]>
>> >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 <[email protected]>
>wrote:
>> >>
>> >> Hi Jagan,
>> >>
>> >> On Mon, May 11, 2020 at 4:48 PM Jagan Teki
>> ><[email protected]> wrote:
>> >> >
>> >> > On Mon, May 11, 2020 at 1:15 PM Pragnesh Patel
>> >> > <[email protected]> wrote:
>> >> > >
>> >> > > >-----Original Message-----
>> >> > > >From: Jagan Teki <[email protected]>
>> >> > > >Sent: 11 May 2020 12:56
>> >> > > >To: Pragnesh Patel <[email protected]>
>> >> > > >Cc: U-Boot-Denx <[email protected]>; Atish Patra
>> >> > > ><[email protected]>; Palmer Dabbelt
>> ><[email protected]>;
>> >> > > >Bin Meng <[email protected]>; Paul Walmsley
>> >> > > ><[email protected]>; Troy Benjegerdes
>> >> > > ><[email protected]>; Anup Patel
>> >> > > ><[email protected]>; Sagar Kadam <[email protected]>;
>> >> > > >Rick Chen <[email protected]>
>> >> > > >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
>> >> > > ><[email protected]> wrote:
>> >> > > >>
>> >> > > >> >-----Original Message-----
>> >> > > >> >From: Jagan Teki <[email protected]>
>> >> > > >> >Sent: 11 May 2020 12:25
>> >> > > >> >To: Pragnesh Patel <[email protected]>
>> >> > > >> >Cc: U-Boot-Denx <[email protected]>; Atish Patra
>> >> > > >> ><[email protected]>; Palmer Dabbelt
>> >> > > >> ><[email protected]>;
>> >> > > >Bin
>> >> > > >> >Meng <[email protected]>; Paul Walmsley
>> >> > > ><[email protected]>;
>> >> > > >> >Troy Benjegerdes <[email protected]>; Anup Patel
>> >> > > >> ><[email protected]>; Sagar Kadam
><[email protected]>;
>> >> > > >> >Rick
>> >> > > >Chen
>> >> > > >> ><[email protected]>
>> >> > > >> >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
>> >> > > >> ><[email protected]> wrote:
>> >> > > >> >>
>> >> > > >> >> >-----Original Message-----
>> >> > > >> >> >From: Jagan Teki <[email protected]>
>> >> > > >> >> >Sent: 10 May 2020 15:02
>> >> > > >> >> >To: Pragnesh Patel <[email protected]>
>> >> > > >> >> >Cc: U-Boot-Denx <[email protected]>; Atish Patra
>> >> > > >> >> ><[email protected]>; Palmer Dabbelt
>> >> > > ><[email protected]>;
>> >> > > >> >Bin
>> >> > > >> >> >Meng <[email protected]>; Paul Walmsley
>> >> > > >> ><[email protected]>;
>> >> > > >> >> >Troy Benjegerdes <[email protected]>; Anup
>> >> > > >> >> >Patel <[email protected]>; Sagar Kadam
>> ><[email protected]>;
>> >> > > >> >> >Rick
>> >> > > >> >Chen
>> >> > > >> >> ><[email protected]>
>> >> > > >> >> >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
>> >> > > >> >> ><[email protected]> wrote:
>> >> > > >> >> >>
>> >> > > >> >> >> Hi jagan,
>> >> > > >> >> >>
>> >> > > >> >> >> >-----Original Message-----
>> >> > > >> >> >> >From: Jagan Teki <[email protected]>
>> >> > > >> >> >> >Sent: 02 May 2020 22:43
>> >> > > >> >> >> >To: Pragnesh Patel <[email protected]>
>> >> > > >> >> >> >Cc: U-Boot-Denx <[email protected]>; Atish Patra
>> >> > > >> >> >> ><[email protected]>; Palmer Dabbelt
>> >> > > >> ><[email protected]>;
>> >> > > >> >> >Bin
>> >> > > >> >> >> >Meng <[email protected]>; Paul Walmsley
>> >> > > >> >> ><[email protected]>;
>> >> > > >> >> >> >Troy Benjegerdes <[email protected]>; Anup
>> >> > > >> >> >> >Patel <[email protected]>; Sagar Kadam
>> >> > > >> >> >> ><[email protected]>; Rick
>> >> > > >> >> >Chen
>> >> > > >> >> >> ><[email protected]>
>> >> > > >> >> >> >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
>> >> > > >> >> >> ><[email protected]>
>> >> > > >> >> >> >wrote:
>> >> > > >> >> >> >>
>> >> > > >> >> >> >> Hi Jagan,
>> >> > > >> >> >> >>
>> >> > > >> >> >> >> >-----Original Message-----
>> >> > > >> >> >> >> >From: Jagan Teki <[email protected]>
>> >> > > >> >> >> >> >Sent: 02 May 2020 21:49
>> >> > > >> >> >> >> >To: Pragnesh Patel <[email protected]>
>> >> > > >> >> >> >> >Cc: U-Boot-Denx <[email protected]>; Atish
>> >> > > >> >> >> >> >Patra <[email protected]>; Palmer Dabbelt
>> >> > > >> >> ><[email protected]>;
>> >> > > >> >> >> >Bin
>> >> > > >> >> >> >> >Meng <[email protected]>; Paul Walmsley
>> >> > > >> >> >> ><[email protected]>;
>> >> > > >> >> >> >> >Troy Benjegerdes <[email protected]>;
>> >> > > >> >> >> >> >Anup Patel <[email protected]>; Sagar Kadam
>> >> > > ><[email protected]>;
>> >> > > >> >> >> >> >Rick
>> >> > > >> >> >> >Chen
>> >> > > >> >> >> >> ><[email protected]>
>> >> > > >> >> >> >> >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
>> >> > > >> >> >> >> ><[email protected]>
>> >> > > >> >> >> >> >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.24809
>> >-
>> >[email protected]/
>>
>> 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.

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


>
>Regards,
>Bin

Reply via email to