diff mbox series

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

Message ID 20200502100628.24809-20-pragnesh.patel@sifive.com
State Superseded
Headers show
Series RISC-V SiFive FU540 support SPL | expand

Commit Message

Pragnesh Patel May 2, 2020, 10:06 a.m. UTC
Add L2 cache node to enable cache ways from U-Boot

Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com>
Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
---
 arch/riscv/dts/fu540-c000-u-boot.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Jagan Teki May 2, 2020, 4:18 p.m. UTC | #1
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.

Jagan.
Pragnesh Patel May 2, 2020, 4:42 p.m. UTC | #2
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.

>
>Jagan.
Jagan Teki May 2, 2020, 5:13 p.m. UTC | #3
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?

Jagan.
Pragnesh Patel May 3, 2020, 7:27 a.m. UTC | #4
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.

Let me know if you still want me to send cache ways patches separately ?

>
>Jagan.
Jagan Teki May 10, 2020, 9:31 a.m. UTC | #5
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?

Jagan.
Pragnesh Patel May 11, 2020, 6:05 a.m. UTC | #6
>-----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.

>
>Jagan.
Jagan Teki May 11, 2020, 6:54 a.m. UTC | #7
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?

Jagan.
Pragnesh Patel May 11, 2020, 7:07 a.m. UTC | #8
>-----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.

I am not in favour of pulling out L2 cache patches out of this series.

>
>Jagan.
Jagan Teki May 11, 2020, 7:25 a.m. UTC | #9
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?

Jagan.
Pragnesh Patel May 11, 2020, 7:45 a.m. UTC | #10
>-----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

Now U-Boot SPL doing the same in [v7 19/22] and [v7 20/22].

User can use U-Boot SPL or FSBL anyone at a time as a bootloader, so better to replace all FSBL
stuff in one series.

>
>Jagan.
Jagan Teki May 11, 2020, 8:48 a.m. UTC | #11
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.

Jagan.
Bin Meng May 11, 2020, 9 a.m. UTC | #12
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
Jagan Teki May 11, 2020, 9:05 a.m. UTC | #13
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/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

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-20-pragnesh.patel at sifive.com/
Pragnesh Patel May 11, 2020, 9:34 a.m. UTC | #14
>-----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/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
>
>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-
>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.
Bin Meng May 11, 2020, 9:47 a.m. UTC | #15
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/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
> >
> >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-
> >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.

Regards,
Bin
Pragnesh Patel May 11, 2020, 9:55 a.m. UTC | #16
>-----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.24809
>> >-
>> >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.

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


>
>Regards,
>Bin
Bin Meng May 11, 2020, 10:10 a.m. UTC | #17
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.24809
> >> >-
> >> >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?

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

Regards,
Bin
Pragnesh Patel May 11, 2020, 10:35 a.m. UTC | #18
>-----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.

>
>> @Jagan Teki Sorry for my misunderstanding, U-Boot SPL got stuck in my
>mind.
>>
>
>Regards,
>Bin
Bin Meng May 12, 2020, 1:20 a.m. UTC | #19
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
Jagan Teki May 12, 2020, 7:45 a.m. UTC | #20
On Mon, May 11, 2020 at 3:25 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.24809
> >> >-
> >> >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.
>
> @Jagan Teki Sorry for my misunderstanding, U-Boot SPL got stuck in my mind.

No problem at all, It is just a discussion of how things would get
into perfect shape. Why I was more concerned about patches should be
out of the series which are unrelated is you can get a clean series of
what changes would require for U-Boot SPL to support. This is general
upstream workflow. That would indeed be easy for reviewers to
understand and comment quickly. Moreover you could have a clear
history and better bisectability in future, thanks!

Jagan.
diff mbox series

Patch

diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi
index ab697ab93c..3c3b810998 100644
--- a/arch/riscv/dts/fu540-c000-u-boot.dtsi
+++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi
@@ -85,3 +85,7 @@ 
 &qspi2 {
 	u-boot,dm-spl;
 };
+
+&l2cache {
+	status = "okay";
+};