[PATCH v4 0/4] update clock handler and proper cpu features

[PATCH v4 0/4] update clock handler and proper cpu features

[Sagar Shrikant Kadam]

U-Boot cmd "cpu detail" shows inconsistent CPU features and is missing
clk_request and free handlers.
The current "cpu detail" sometimes shows "Microcode" as a feature, which
is not the case with FU540-C000 on HiFive Unleashed board.

Patch 1: add clk request handler to check if valid clock id is requested.
Patch 2: add cpu node aliases. 
Patch 3: Correctly parse and update mmu-type.

RISC-V core's on FU540-C000 SoC have separate instruction and data (split) 
L1 cache.
Patch 4:Use i-cache-size dt property as one of identifier to indicate a
	split cache is available.

I have picked few dependent patches from Sean's series from here [1]
and [2].

These have applied on mainline U-Boot commit 2b8692bac1e8 ("Merge tag
'efi-2020-07-rc5-2' of https://gitlab.denx.de/u-boot/custodians/u-boot-efi")

Patch history:
=============================================
V4:
1. Rebased the series to mainline commit.
2. Updated dependency list as few patches are now merged.
3. Added U-Boot log of the flow i.e fsbl + fw_payload.bin (Opensbi+U-Boot)
   
V3:
1. Included the cosmetic change as suggested
   s/L1 feature/L1 cache feature/
2. Added Reviewed-By tags

V2:
1. Incorporate review comments from Bin and Sean Anderson. 
   and dropped 2nd patch as similar work was already done in [1] and [2]
2  Add cpu node aliases to display cpu node's in sequence.
3. Add fix to show mmu as available cpu feature. 
4. Check and append L1 cache feature.

V1: Base version
    Thanks to Vincent Chen <vincent.chen@sifive.com> for testing the V1 
    version of this series.

[1] https://patchwork.ozlabs.org/patch/1295345
[2] https://patchwork.ozlabs.org/patch/1295346

All these together is available here:
https://github.com/sagsifive/u-boot/commits/dev/sagark/clk-v4

U-Boot log:
===========================================================
SiFive FSBL:       2020-02-19-1081db9-dirty
Using new FSBL DTB now
HiFive-U-serial-#: 000002c8
Loading boot payload....

OpenSBI v0.7-31-gd626037
   ____                    _____ ____ _____
  / __ \                  / ____|  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |____) | |_) || |_
  \____/| .__/ \___|_| |_|_____/|____/_____|
        | |
        |_|

Platform Name          : SiFive Freedom U540
Platform HART Features : RV64ACDFIMSU
Platform HART Count    : 4
Current HART ID        : 1
Firmware Base          : 0x80000000
Firmware Size          : 100 KB
Runtime SBI Version    : 0.2

MIDELEG : 0x0000000000000222
MEDELEG : 0x000000000000b109
PMP0    : 0x0000000080000000-0x000000008001ffff (A)
PMP1    : 0x0000000000000000-0x0000007fffffffff (A,R,W,X)


U-Boot 2020.07-rc4-00084-gf824d2c (Jun 21 2020 - 04:58:40 -0700)

CPU:   rv64imac
Model: SiFive HiFive Unleashed A00
DRAM:  8 GiB
MMC:   spi at 10050000:mmc at 0: 0
In:    serial at 10010000
Out:   serial at 10010000
Err:   serial at 10010000
Net:   eth0: ethernet@10090000
Hit any key to stop autoboot:  0
=> cpu detail
  0: cpu at 0      rv64imac
        ID = 0, freq = 999.100 MHz: L1 cache
  1: cpu at 1      rv64imafdc
        ID = 1, freq = 999.100 MHz: L1 cache, MMU
  2: cpu at 2      rv64imafdc
        ID = 2, freq = 999.100 MHz: L1 cache, MMU
  3: cpu at 3      rv64imafdc
        ID = 3, freq = 999.100 MHz: L1 cache, MMU
  4: cpu at 4      rv64imafdc
        ID = 4, freq = 999.100 MHz: L1 cache, MMU
=>

Sagar Shrikant Kadam (4):
  fu540: prci: add request and free clock handlers
  riscv: dts: hifive-unleashed-a00: add cpu aliases
  riscv: cpu: fixes to display proper CPU features
  riscv: cpu: check and append L1 cache to cpu features

 arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi |  5 +++++
 drivers/clk/sifive/fu540-prci.c                 | 21 +++++++++++++++++++++
 drivers/cpu/riscv_cpu.c                         | 10 +++++++++-
 3 files changed, 35 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH v4 1/4] fu540: prci: add request and free clock handlers

[Sagar Shrikant Kadam]

Add clk_request handler to check if a valid clock is requested.
Here clk_free handler is added for debug purpose which will display
details of clock passed to clk_free.

Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
---
 drivers/clk/sifive/fu540-prci.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c
index fe6e0d4..9a9ff6b 100644
--- a/drivers/clk/sifive/fu540-prci.c
+++ b/drivers/clk/sifive/fu540-prci.c
@@ -686,6 +686,25 @@ static ulong sifive_fu540_prci_set_rate(struct clk *clk, ulong rate)
 	return rate;
 }
 
+static int sifive_fu540_prci_clk_request(struct clk *clk)
+{
+	debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
+	      clk->id);
+
+	if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int sifive_fu540_prci_clk_free(struct clk *clk)
+{
+	debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
+	      clk->id);
+
+	return 0;
+}
+
 static int sifive_fu540_prci_enable(struct clk *clk)
 {
 	struct __prci_clock *pc;
@@ -755,6 +774,8 @@ static struct clk_ops sifive_fu540_prci_ops = {
 	.get_rate = sifive_fu540_prci_get_rate,
 	.enable = sifive_fu540_prci_enable,
 	.disable = sifive_fu540_prci_disable,
+	.request  = sifive_fu540_prci_clk_request,
+	.rfree	  = sifive_fu540_prci_clk_free,
 };
 
 static const struct udevice_id sifive_fu540_prci_ids[] = {
-- 
2.7.4

[PATCH v4 1/4] fu540: prci: add request and free clock handlers

[Rick Chen]

Hi Sagar,

> From: Sagar Shrikant Kadam [mailto:sagar.kadam at sifive.com]
> Sent: Sunday, June 21, 2020 9:10 PM
> To: u-boot at lists.denx.de
> Cc: Rick Jian-Zhi Chen(???); lukma at denx.de; bmeng.cn at gmail.com; jagan at amarulasolutions.com; pragnesh.patel at sifive.com; anup.patel at wdc.com; sjg at chromium.org; seanga2 at gmail.com; Sagar Shrikant Kadam
> Subject: [PATCH v4 1/4] fu540: prci: add request and free clock handlers
>
> Add clk_request handler to check if a valid clock is requested.
> Here clk_free handler is added for debug purpose which will display details of clock passed to clk_free.
>
> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
> ---

This v4 series still have some conflicts with master.
Please check about it.

Applying: fu540: prci: add request and free clock handlers
Applying: riscv: dts: hifive-unleashed-a00: add cpu aliases
Applying: riscv: cpu: fixes to display proper CPU features
error: patch failed: drivers/cpu/riscv_cpu.c:38
error: drivers/cpu/riscv_cpu.c: patch does not apply
Patch failed at 0003 riscv: cpu: fixes to display proper CPU features

Thanks,
Rick

>  drivers/clk/sifive/fu540-prci.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c index fe6e0d4..9a9ff6b 100644
> --- a/drivers/clk/sifive/fu540-prci.c
> +++ b/drivers/clk/sifive/fu540-prci.c
> @@ -686,6 +686,25 @@ static ulong sifive_fu540_prci_set_rate(struct clk *clk, ulong rate)
>         return rate;
>  }
>
> +static int sifive_fu540_prci_clk_request(struct clk *clk) {
> +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> +             clk->id);
> +
> +       if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
> +               return -EINVAL;
> +
> +       return 0;
> +}
> +
> +static int sifive_fu540_prci_clk_free(struct clk *clk) {
> +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> +             clk->id);
> +
> +       return 0;
> +}
> +
>  static int sifive_fu540_prci_enable(struct clk *clk)  {
>         struct __prci_clock *pc;
> @@ -755,6 +774,8 @@ static struct clk_ops sifive_fu540_prci_ops = {
>         .get_rate = sifive_fu540_prci_get_rate,
>         .enable = sifive_fu540_prci_enable,
>         .disable = sifive_fu540_prci_disable,
> +       .request  = sifive_fu540_prci_clk_request,
> +       .rfree    = sifive_fu540_prci_clk_free,
>  };
>
>  static const struct udevice_id sifive_fu540_prci_ids[] = {
> --
> 2.7.4
>

[PATCH v4 1/4] fu540: prci: add request and free clock handlers

[Sagar Kadam]

Hello Rick,

> -----Original Message-----
> From: Rick Chen <rickchen36@gmail.com>
> Sent: Monday, June 22, 2020 7:24 AM
> To: Sagar Kadam <sagar.kadam@sifive.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Lukasz Majewski
> <lukma@denx.de>; Bin Meng <bmeng.cn@gmail.com>; Jagan Teki
> <jagan@amarulasolutions.com>; Pragnesh Patel
> <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>; Simon
> Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>; rick
> <rick@andestech.com>; Alan Kao <alankao@andestech.com>;
> ycliang at andestech.com
> Subject: Re: [PATCH v4 1/4] fu540: prci: add request and free clock handlers
> 
> [External Email] Do not click links or attachments unless you recognize the
> sender and know the content is safe
> 
> Hi Sagar,
> 
> > From: Sagar Shrikant Kadam [mailto:sagar.kadam at sifive.com]
> > Sent: Sunday, June 21, 2020 9:10 PM
> > To: u-boot at lists.denx.de
> > Cc: Rick Jian-Zhi Chen(???); lukma at denx.de; bmeng.cn at gmail.com;
> > jagan at amarulasolutions.com; pragnesh.patel at sifive.com;
> > anup.patel at wdc.com; sjg at chromium.org; seanga2 at gmail.com; Sagar
> > Shrikant Kadam
> > Subject: [PATCH v4 1/4] fu540: prci: add request and free clock
> > handlers
> >
> > Add clk_request handler to check if a valid clock is requested.
> > Here clk_free handler is added for debug purpose which will display
> details of clock passed to clk_free.
> >
> > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> > Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
> > ---
> 
> This v4 series still have some conflicts with master.
> Please check about it.
> 
> Applying: fu540: prci: add request and free clock handlers
> Applying: riscv: dts: hifive-unleashed-a00: add cpu aliases
> Applying: riscv: cpu: fixes to display proper CPU features
> error: patch failed: drivers/cpu/riscv_cpu.c:38
> error: drivers/cpu/riscv_cpu.c: patch does not apply Patch failed at 0003
> riscv: cpu: fixes to display proper CPU features
> 

Thanks for trying this out
This series is dependent on following two patches [1] 
and [2] below which are part of "Add Sipeed Maix support." series
[1] https://patchwork.ozlabs.org/patch/1295345
[2] https://patchwork.ozlabs.org/patch/1295346
and I also mentioned in cover-letter. Sorry If I missed to add some 
additional information there.

The dependent are added like 
-u-boot master [commit 2b8692bac1e8]. 
-Patches [1] + [2].
-This series.

I guess the PR for the series was sent earlier followed with some discussion 
https://www.mail-archive.com/u-boot at lists.denx.de/msg370739.html
and I think this series was deferred in that PR.
Please let me know if you have some suggestions here?

Thanks & BR,
Sagar Kadam

> Thanks,
> Rick
> 
> >  drivers/clk/sifive/fu540-prci.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/drivers/clk/sifive/fu540-prci.c
> > b/drivers/clk/sifive/fu540-prci.c index fe6e0d4..9a9ff6b 100644
> > --- a/drivers/clk/sifive/fu540-prci.c
> > +++ b/drivers/clk/sifive/fu540-prci.c
> > @@ -686,6 +686,25 @@ static ulong sifive_fu540_prci_set_rate(struct clk
> *clk, ulong rate)
> >         return rate;
> >  }
> >
> > +static int sifive_fu540_prci_clk_request(struct clk *clk) {
> > +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> > +             clk->id);
> > +
> > +       if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
> > +               return -EINVAL;
> > +
> > +       return 0;
> > +}
> > +
> > +static int sifive_fu540_prci_clk_free(struct clk *clk) {
> > +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> > +             clk->id);
> > +
> > +       return 0;
> > +}
> > +
> >  static int sifive_fu540_prci_enable(struct clk *clk)  {
> >         struct __prci_clock *pc;
> > @@ -755,6 +774,8 @@ static struct clk_ops sifive_fu540_prci_ops = {
> >         .get_rate = sifive_fu540_prci_get_rate,
> >         .enable = sifive_fu540_prci_enable,
> >         .disable = sifive_fu540_prci_disable,
> > +       .request  = sifive_fu540_prci_clk_request,
> > +       .rfree    = sifive_fu540_prci_clk_free,
> >  };
> >
> >  static const struct udevice_id sifive_fu540_prci_ids[] = {
> > --
> > 2.7.4
> >

[PATCH v4 1/4] fu540: prci: add request and free clock handlers

[Rick Chen]

Hi Sagar

>
> Hello Rick,
>
> > -----Original Message-----
> > From: Rick Chen <rickchen36@gmail.com>
> > Sent: Monday, June 22, 2020 7:24 AM
> > To: Sagar Kadam <sagar.kadam@sifive.com>
> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Lukasz Majewski
> > <lukma@denx.de>; Bin Meng <bmeng.cn@gmail.com>; Jagan Teki
> > <jagan@amarulasolutions.com>; Pragnesh Patel
> > <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>; Simon
> > Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>; rick
> > <rick@andestech.com>; Alan Kao <alankao@andestech.com>;
> > ycliang at andestech.com
> > Subject: Re: [PATCH v4 1/4] fu540: prci: add request and free clock handlers
> >
> > [External Email] Do not click links or attachments unless you recognize the
> > sender and know the content is safe
> >
> > Hi Sagar,
> >
> > > From: Sagar Shrikant Kadam [mailto:sagar.kadam at sifive.com]
> > > Sent: Sunday, June 21, 2020 9:10 PM
> > > To: u-boot at lists.denx.de
> > > Cc: Rick Jian-Zhi Chen(???); lukma at denx.de; bmeng.cn at gmail.com;
> > > jagan at amarulasolutions.com; pragnesh.patel at sifive.com;
> > > anup.patel at wdc.com; sjg at chromium.org; seanga2 at gmail.com; Sagar
> > > Shrikant Kadam
> > > Subject: [PATCH v4 1/4] fu540: prci: add request and free clock
> > > handlers
> > >
> > > Add clk_request handler to check if a valid clock is requested.
> > > Here clk_free handler is added for debug purpose which will display
> > details of clock passed to clk_free.
> > >
> > > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> > > Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
> > > ---
> >
> > This v4 series still have some conflicts with master.
> > Please check about it.
> >
> > Applying: fu540: prci: add request and free clock handlers
> > Applying: riscv: dts: hifive-unleashed-a00: add cpu aliases
> > Applying: riscv: cpu: fixes to display proper CPU features
> > error: patch failed: drivers/cpu/riscv_cpu.c:38
> > error: drivers/cpu/riscv_cpu.c: patch does not apply Patch failed at 0003
> > riscv: cpu: fixes to display proper CPU features
> >
>
> Thanks for trying this out
> This series is dependent on following two patches [1]
> and [2] below which are part of "Add Sipeed Maix support." series
> [1] https://patchwork.ozlabs.org/patch/1295345
> [2] https://patchwork.ozlabs.org/patch/1295346
> and I also mentioned in cover-letter. Sorry If I missed to add some
> additional information there.
>
> The dependent are added like
> -u-boot master [commit 2b8692bac1e8].
> -Patches [1] + [2].
> -This series.

According to this dependency I will apply yours in -next behind of Sean's

Thanks,
Rick

>
> I guess the PR for the series was sent earlier followed with some discussion
> https://www.mail-archive.com/u-boot at lists.denx.de/msg370739.html
> and I think this series was deferred in that PR.
> Please let me know if you have some suggestions here?
>
> Thanks & BR,
> Sagar Kadam
>
> > Thanks,
> > Rick
> >
> > >  drivers/clk/sifive/fu540-prci.c | 21 +++++++++++++++++++++
> > >  1 file changed, 21 insertions(+)
> > >
> > > diff --git a/drivers/clk/sifive/fu540-prci.c
> > > b/drivers/clk/sifive/fu540-prci.c index fe6e0d4..9a9ff6b 100644
> > > --- a/drivers/clk/sifive/fu540-prci.c
> > > +++ b/drivers/clk/sifive/fu540-prci.c
> > > @@ -686,6 +686,25 @@ static ulong sifive_fu540_prci_set_rate(struct clk
> > *clk, ulong rate)
> > >         return rate;
> > >  }
> > >
> > > +static int sifive_fu540_prci_clk_request(struct clk *clk) {
> > > +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> > > +             clk->id);
> > > +
> > > +       if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
> > > +               return -EINVAL;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int sifive_fu540_prci_clk_free(struct clk *clk) {
> > > +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> > > +             clk->id);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  static int sifive_fu540_prci_enable(struct clk *clk)  {
> > >         struct __prci_clock *pc;
> > > @@ -755,6 +774,8 @@ static struct clk_ops sifive_fu540_prci_ops = {
> > >         .get_rate = sifive_fu540_prci_get_rate,
> > >         .enable = sifive_fu540_prci_enable,
> > >         .disable = sifive_fu540_prci_disable,
> > > +       .request  = sifive_fu540_prci_clk_request,
> > > +       .rfree    = sifive_fu540_prci_clk_free,
> > >  };
> > >
> > >  static const struct udevice_id sifive_fu540_prci_ids[] = {
> > > --
> > > 2.7.4
> > >

[PATCH v4 1/4] fu540: prci: add request and free clock handlers

[Bin Meng]

Hi Rick,

On Wed, Jun 24, 2020 at 9:36 AM Rick Chen <rickchen36@gmail.com> wrote:
>
> Hi Sagar
>
> >
> > Hello Rick,
> >
> > > -----Original Message-----
> > > From: Rick Chen <rickchen36@gmail.com>
> > > Sent: Monday, June 22, 2020 7:24 AM
> > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Lukasz Majewski
> > > <lukma@denx.de>; Bin Meng <bmeng.cn@gmail.com>; Jagan Teki
> > > <jagan@amarulasolutions.com>; Pragnesh Patel
> > > <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>; Simon
> > > Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>; rick
> > > <rick@andestech.com>; Alan Kao <alankao@andestech.com>;
> > > ycliang at andestech.com
> > > Subject: Re: [PATCH v4 1/4] fu540: prci: add request and free clock handlers
> > >
> > > [External Email] Do not click links or attachments unless you recognize the
> > > sender and know the content is safe
> > >
> > > Hi Sagar,
> > >
> > > > From: Sagar Shrikant Kadam [mailto:sagar.kadam at sifive.com]
> > > > Sent: Sunday, June 21, 2020 9:10 PM
> > > > To: u-boot at lists.denx.de
> > > > Cc: Rick Jian-Zhi Chen(???); lukma at denx.de; bmeng.cn at gmail.com;
> > > > jagan at amarulasolutions.com; pragnesh.patel at sifive.com;
> > > > anup.patel at wdc.com; sjg at chromium.org; seanga2 at gmail.com; Sagar
> > > > Shrikant Kadam
> > > > Subject: [PATCH v4 1/4] fu540: prci: add request and free clock
> > > > handlers
> > > >
> > > > Add clk_request handler to check if a valid clock is requested.
> > > > Here clk_free handler is added for debug purpose which will display
> > > details of clock passed to clk_free.
> > > >
> > > > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> > > > Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
> > > > ---
> > >
> > > This v4 series still have some conflicts with master.
> > > Please check about it.
> > >
> > > Applying: fu540: prci: add request and free clock handlers
> > > Applying: riscv: dts: hifive-unleashed-a00: add cpu aliases
> > > Applying: riscv: cpu: fixes to display proper CPU features
> > > error: patch failed: drivers/cpu/riscv_cpu.c:38
> > > error: drivers/cpu/riscv_cpu.c: patch does not apply Patch failed at 0003
> > > riscv: cpu: fixes to display proper CPU features
> > >
> >
> > Thanks for trying this out
> > This series is dependent on following two patches [1]
> > and [2] below which are part of "Add Sipeed Maix support." series
> > [1] https://patchwork.ozlabs.org/patch/1295345
> > [2] https://patchwork.ozlabs.org/patch/1295346
> > and I also mentioned in cover-letter. Sorry If I missed to add some
> > additional information there.
> >
> > The dependent are added like
> > -u-boot master [commit 2b8692bac1e8].
> > -Patches [1] + [2].
> > -This series.
>
> According to this dependency I will apply yours in -next behind of Sean's

I just sent comments to this series, please hold on.

Regards,
Bin

[PATCH v4 1/4] fu540: prci: add request and free clock handlers

[Rick Chen]

Hi Bin

> Hi Rick,
>
> On Wed, Jun 24, 2020 at 9:36 AM Rick Chen <rickchen36@gmail.com> wrote:
> >
> > Hi Sagar
> >
> > >
> > > Hello Rick,
> > >
> > > > -----Original Message-----
> > > > From: Rick Chen <rickchen36@gmail.com>
> > > > Sent: Monday, June 22, 2020 7:24 AM
> > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Lukasz Majewski
> > > > <lukma@denx.de>; Bin Meng <bmeng.cn@gmail.com>; Jagan Teki
> > > > <jagan@amarulasolutions.com>; Pragnesh Patel
> > > > <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>; Simon
> > > > Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>; rick
> > > > <rick@andestech.com>; Alan Kao <alankao@andestech.com>;
> > > > ycliang at andestech.com
> > > > Subject: Re: [PATCH v4 1/4] fu540: prci: add request and free clock handlers
> > > >
> > > > [External Email] Do not click links or attachments unless you recognize the
> > > > sender and know the content is safe
> > > >
> > > > Hi Sagar,
> > > >
> > > > > From: Sagar Shrikant Kadam [mailto:sagar.kadam at sifive.com]
> > > > > Sent: Sunday, June 21, 2020 9:10 PM
> > > > > To: u-boot at lists.denx.de
> > > > > Cc: Rick Jian-Zhi Chen(???); lukma at denx.de; bmeng.cn at gmail.com;
> > > > > jagan at amarulasolutions.com; pragnesh.patel at sifive.com;
> > > > > anup.patel at wdc.com; sjg at chromium.org; seanga2 at gmail.com; Sagar
> > > > > Shrikant Kadam
> > > > > Subject: [PATCH v4 1/4] fu540: prci: add request and free clock
> > > > > handlers
> > > > >
> > > > > Add clk_request handler to check if a valid clock is requested.
> > > > > Here clk_free handler is added for debug purpose which will display
> > > > details of clock passed to clk_free.
> > > > >
> > > > > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> > > > > Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
> > > > > ---
> > > >
> > > > This v4 series still have some conflicts with master.
> > > > Please check about it.
> > > >
> > > > Applying: fu540: prci: add request and free clock handlers
> > > > Applying: riscv: dts: hifive-unleashed-a00: add cpu aliases
> > > > Applying: riscv: cpu: fixes to display proper CPU features
> > > > error: patch failed: drivers/cpu/riscv_cpu.c:38
> > > > error: drivers/cpu/riscv_cpu.c: patch does not apply Patch failed at 0003
> > > > riscv: cpu: fixes to display proper CPU features
> > > >
> > >
> > > Thanks for trying this out
> > > This series is dependent on following two patches [1]
> > > and [2] below which are part of "Add Sipeed Maix support." series
> > > [1] https://patchwork.ozlabs.org/patch/1295345
> > > [2] https://patchwork.ozlabs.org/patch/1295346
> > > and I also mentioned in cover-letter. Sorry If I missed to add some
> > > additional information there.
> > >
> > > The dependent are added like
> > > -u-boot master [commit 2b8692bac1e8].
> > > -Patches [1] + [2].
> > > -This series.
> >
> > According to this dependency I will apply yours in -next behind of Sean's
>
> I just sent comments to this series, please hold on.

OK.

Thanks,
Rick

>
> Regards,
> Bin

[PATCH v4 1/4] fu540: prci: add request and free clock handlers

[Sagar Kadam]

Hi Rick,

> -----Original Message-----
> From: Rick Chen <rickchen36@gmail.com>
> Sent: Wednesday, June 24, 2020 7:16 AM
> To: Bin Meng <bmeng.cn@gmail.com>
> Cc: Sagar Kadam <sagar.kadam@sifive.com>; U-Boot Mailing List <u-
> boot at lists.denx.de>; Lukasz Majewski <lukma@denx.de>; Jagan Teki
> <jagan@amarulasolutions.com>; Pragnesh Patel
> <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>; Simon
> Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>; rick
> <rick@andestech.com>; Alan Kao <alankao@andestech.com>;
> ycliang at andestech.com
> Subject: Re: [PATCH v4 1/4] fu540: prci: add request and free clock handlers
> 
> [External Email] Do not click links or attachments unless you recognize the
> sender and know the content is safe
> 
> Hi Bin
> 
> > Hi Rick,
> >
> > On Wed, Jun 24, 2020 at 9:36 AM Rick Chen <rickchen36@gmail.com>
> wrote:
> > >
> > > Hi Sagar
> > >
> > > >
> > > > Hello Rick,
> > > >
> > > > > -----Original Message-----
> > > > > From: Rick Chen <rickchen36@gmail.com>
> > > > > Sent: Monday, June 22, 2020 7:24 AM
> > > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Lukasz Majewski
> > > > > <lukma@denx.de>; Bin Meng <bmeng.cn@gmail.com>; Jagan Teki
> > > > > <jagan@amarulasolutions.com>; Pragnesh Patel
> > > > > <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>;
> > > > > Simon Glass <sjg@chromium.org>; Sean Anderson
> > > > > <seanga2@gmail.com>; rick <rick@andestech.com>; Alan Kao
> > > > > <alankao@andestech.com>; ycliang at andestech.com
> > > > > Subject: Re: [PATCH v4 1/4] fu540: prci: add request and free
> > > > > clock handlers
> > > > >
> > > > > [External Email] Do not click links or attachments unless you
> > > > > recognize the sender and know the content is safe
> > > > >
> > > > > Hi Sagar,
> > > > >
> > > > > > From: Sagar Shrikant Kadam [mailto:sagar.kadam at sifive.com]
> > > > > > Sent: Sunday, June 21, 2020 9:10 PM
> > > > > > To: u-boot at lists.denx.de
> > > > > > Cc: Rick Jian-Zhi Chen(???); lukma at denx.de;
> > > > > > bmeng.cn at gmail.com; jagan at amarulasolutions.com;
> > > > > > pragnesh.patel at sifive.com; anup.patel at wdc.com;
> > > > > > sjg at chromium.org; seanga2 at gmail.com; Sagar Shrikant Kadam
> > > > > > Subject: [PATCH v4 1/4] fu540: prci: add request and free
> > > > > > clock handlers
> > > > > >
> > > > > > Add clk_request handler to check if a valid clock is requested.
> > > > > > Here clk_free handler is added for debug purpose which will
> > > > > > display
> > > > > details of clock passed to clk_free.
> > > > > >
> > > > > > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> > > > > > Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
> > > > > > ---
> > > > >
> > > > > This v4 series still have some conflicts with master.
> > > > > Please check about it.
> > > > >
> > > > > Applying: fu540: prci: add request and free clock handlers
> > > > > Applying: riscv: dts: hifive-unleashed-a00: add cpu aliases
> > > > > Applying: riscv: cpu: fixes to display proper CPU features
> > > > > error: patch failed: drivers/cpu/riscv_cpu.c:38
> > > > > error: drivers/cpu/riscv_cpu.c: patch does not apply Patch
> > > > > failed at 0003
> > > > > riscv: cpu: fixes to display proper CPU features
> > > > >
> > > >
> > > > Thanks for trying this out
> > > > This series is dependent on following two patches [1] and [2]
> > > > below which are part of "Add Sipeed Maix support." series [1]
> > > > https://patchwork.ozlabs.org/patch/1295345
> > > > [2] https://patchwork.ozlabs.org/patch/1295346
> > > > and I also mentioned in cover-letter. Sorry If I missed to add
> > > > some additional information there.
> > > >
> > > > The dependent are added like
> > > > -u-boot master [commit 2b8692bac1e8].
> > > > -Patches [1] + [2].
> > > > -This series.
> > >
> > > According to this dependency I will apply yours in -next behind of
> > > Sean's
> >
> > I just sent comments to this series, please hold on.
> 
> OK.
> 

Thanks for holding on.
Yes, Bin has sent few comments. I will submit V5 for this.

Thanks & BR,
Sagar

> Thanks,
> Rick
> 
> >
> > Regards,
> > Bin

[PATCH v4 1/4] fu540: prci: add request and free clock handlers

[Bin Meng]

Hi Sagar,

On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
<sagar.kadam@sifive.com> wrote:
>
> Add clk_request handler to check if a valid clock is requested.
> Here clk_free handler is added for debug purpose which will display
> details of clock passed to clk_free.
>
> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
> ---
>  drivers/clk/sifive/fu540-prci.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c
> index fe6e0d4..9a9ff6b 100644
> --- a/drivers/clk/sifive/fu540-prci.c
> +++ b/drivers/clk/sifive/fu540-prci.c
> @@ -686,6 +686,25 @@ static ulong sifive_fu540_prci_set_rate(struct clk *clk, ulong rate)
>         return rate;
>  }
>
> +static int sifive_fu540_prci_clk_request(struct clk *clk)
> +{
> +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> +             clk->id);
> +
> +       if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
> +               return -EINVAL;
> +
> +       return 0;
> +}
> +
> +static int sifive_fu540_prci_clk_free(struct clk *clk)
> +{
> +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> +             clk->id);
> +
> +       return 0;
> +}

It seems these 2 routines do not actually do anything? Is this for
debugging purposes?

Regards,
Bin

[PATCH v4 1/4] fu540: prci: add request and free clock handlers

[Sagar Kadam]

Hi Bin,

> -----Original Message-----
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: Wednesday, June 24, 2020 6:50 AM
> To: Sagar Kadam <sagar.kadam@sifive.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Rick Chen
> <rick@andestech.com>; Lukasz Majewski <lukma@denx.de>; Jagan Teki
> <jagan@amarulasolutions.com>; Pragnesh Patel
> <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>; Simon
> Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>
> Subject: Re: [PATCH v4 1/4] fu540: prci: add request and free clock handlers
> 
> [External Email] Do not click links or attachments unless you recognize the
> sender and know the content is safe
> 
> Hi Sagar,
> 
> On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
> <sagar.kadam@sifive.com> wrote:
> >
> > Add clk_request handler to check if a valid clock is requested.
> > Here clk_free handler is added for debug purpose which will display
> > details of clock passed to clk_free.
> >
> > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> > Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
> > ---
> >  drivers/clk/sifive/fu540-prci.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/drivers/clk/sifive/fu540-prci.c
> > b/drivers/clk/sifive/fu540-prci.c index fe6e0d4..9a9ff6b 100644
> > --- a/drivers/clk/sifive/fu540-prci.c
> > +++ b/drivers/clk/sifive/fu540-prci.c
> > @@ -686,6 +686,25 @@ static ulong sifive_fu540_prci_set_rate(struct clk
> *clk, ulong rate)
> >         return rate;
> >  }
> >
> > +static int sifive_fu540_prci_clk_request(struct clk *clk) {
> > +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> > +             clk->id);
> > +
> > +       if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
> > +               return -EINVAL;
> > +
> > +       return 0;
> > +}
> > +
> > +static int sifive_fu540_prci_clk_free(struct clk *clk) {
> > +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> > +             clk->id);
> > +
> > +       return 0;
> > +}
> 
> It seems these 2 routines do not actually do anything? Is this for debugging
> purposes?
> 
The sifive_fu540_prci_clk_request will check if the clock requested is valid or not.
While the sifive_fu540_prci_clk_free function is just for debug.
Is it ok if I retain these in V5 or you have some other thought here.

Thanks & BR,
Sagar

> Regards,
> Bin

[PATCH v4 1/4] fu540: prci: add request and free clock handlers

[Bin Meng]

Hi Sagar,

On Wed, Jun 24, 2020 at 6:58 PM Sagar Kadam <sagar.kadam@sifive.com> wrote:
>
> Hi Bin,
>
> > -----Original Message-----
> > From: Bin Meng <bmeng.cn@gmail.com>
> > Sent: Wednesday, June 24, 2020 6:50 AM
> > To: Sagar Kadam <sagar.kadam@sifive.com>
> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Rick Chen
> > <rick@andestech.com>; Lukasz Majewski <lukma@denx.de>; Jagan Teki
> > <jagan@amarulasolutions.com>; Pragnesh Patel
> > <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>; Simon
> > Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>
> > Subject: Re: [PATCH v4 1/4] fu540: prci: add request and free clock handlers
> >
> > [External Email] Do not click links or attachments unless you recognize the
> > sender and know the content is safe
> >
> > Hi Sagar,
> >
> > On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
> > <sagar.kadam@sifive.com> wrote:
> > >
> > > Add clk_request handler to check if a valid clock is requested.
> > > Here clk_free handler is added for debug purpose which will display
> > > details of clock passed to clk_free.
> > >
> > > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> > > Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
> > > ---
> > >  drivers/clk/sifive/fu540-prci.c | 21 +++++++++++++++++++++
> > >  1 file changed, 21 insertions(+)
> > >
> > > diff --git a/drivers/clk/sifive/fu540-prci.c
> > > b/drivers/clk/sifive/fu540-prci.c index fe6e0d4..9a9ff6b 100644
> > > --- a/drivers/clk/sifive/fu540-prci.c
> > > +++ b/drivers/clk/sifive/fu540-prci.c
> > > @@ -686,6 +686,25 @@ static ulong sifive_fu540_prci_set_rate(struct clk
> > *clk, ulong rate)
> > >         return rate;
> > >  }
> > >
> > > +static int sifive_fu540_prci_clk_request(struct clk *clk) {
> > > +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> > > +             clk->id);
> > > +
> > > +       if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
> > > +               return -EINVAL;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static int sifive_fu540_prci_clk_free(struct clk *clk) {
> > > +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> > > +             clk->id);
> > > +
> > > +       return 0;
> > > +}
> >
> > It seems these 2 routines do not actually do anything? Is this for debugging
> > purposes?
> >
> The sifive_fu540_prci_clk_request will check if the clock requested is valid or not.
> While the sifive_fu540_prci_clk_free function is just for debug.
> Is it ok if I retain these in V5 or you have some other thought here.
>

OK, but I suspect the parameter check in
sifive_fu540_prci_clk_request() is not necessary too as currently the
codes work well.

Regards,
Bin

[PATCH v4 1/4] fu540: prci: add request and free clock handlers

[Sagar Kadam]

Hi Bin,

> -----Original Message-----
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: Thursday, June 25, 2020 4:51 AM
> To: Sagar Kadam <sagar.kadam@sifive.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Rick Chen
> <rick@andestech.com>; Lukasz Majewski <lukma@denx.de>; Jagan Teki
> <jagan@amarulasolutions.com>; Pragnesh Patel
> <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>; Simon
> Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>
> Subject: Re: [PATCH v4 1/4] fu540: prci: add request and free clock handlers
> 
> [External Email] Do not click links or attachments unless you recognize the
> sender and know the content is safe
> 
> Hi Sagar,
> 
> On Wed, Jun 24, 2020 at 6:58 PM Sagar Kadam <sagar.kadam@sifive.com>
> wrote:
> >
> > Hi Bin,
> >
> > > -----Original Message-----
> > > From: Bin Meng <bmeng.cn@gmail.com>
> > > Sent: Wednesday, June 24, 2020 6:50 AM
> > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Rick Chen
> > > <rick@andestech.com>; Lukasz Majewski <lukma@denx.de>; Jagan Teki
> > > <jagan@amarulasolutions.com>; Pragnesh Patel
> > > <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>;
> Simon
> > > Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>
> > > Subject: Re: [PATCH v4 1/4] fu540: prci: add request and free clock
> handlers
> > >
> > > [External Email] Do not click links or attachments unless you recognize
> the
> > > sender and know the content is safe
> > >
> > > Hi Sagar,
> > >
> > > On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
> > > <sagar.kadam@sifive.com> wrote:
> > > >
> > > > Add clk_request handler to check if a valid clock is requested.
> > > > Here clk_free handler is added for debug purpose which will display
> > > > details of clock passed to clk_free.
> > > >
> > > > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> > > > Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
> > > > ---
> > > >  drivers/clk/sifive/fu540-prci.c | 21 +++++++++++++++++++++
> > > >  1 file changed, 21 insertions(+)
> > > >
> > > > diff --git a/drivers/clk/sifive/fu540-prci.c
> > > > b/drivers/clk/sifive/fu540-prci.c index fe6e0d4..9a9ff6b 100644
> > > > --- a/drivers/clk/sifive/fu540-prci.c
> > > > +++ b/drivers/clk/sifive/fu540-prci.c
> > > > @@ -686,6 +686,25 @@ static ulong sifive_fu540_prci_set_rate(struct
> clk
> > > *clk, ulong rate)
> > > >         return rate;
> > > >  }
> > > >
> > > > +static int sifive_fu540_prci_clk_request(struct clk *clk) {
> > > > +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> > > > +             clk->id);
> > > > +
> > > > +       if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
> > > > +               return -EINVAL;
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static int sifive_fu540_prci_clk_free(struct clk *clk) {
> > > > +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> > > > +             clk->id);
> > > > +
> > > > +       return 0;
> > > > +}
> > >
> > > It seems these 2 routines do not actually do anything? Is this for
> debugging
> > > purposes?
> > >
> > The sifive_fu540_prci_clk_request will check if the clock requested is valid
> or not.
> > While the sifive_fu540_prci_clk_free function is just for debug.
> > Is it ok if I retain these in V5 or you have some other thought here.
> >
> 
> OK, but I suspect the parameter check in
> sifive_fu540_prci_clk_request() is not necessary too as currently the
> codes work well.
> 

Ok. In that case I will than drop the check in sifive_fu540_prci_clk_request()
and just keep the debug info.

Thanks & BR,
Sagar

> Regards,
> Bin

[PATCH v4 1/4] fu540: prci: add request and free clock handlers

[Sagar Kadam]

Hi Bin,

> -----Original Message-----
> From: Sagar Kadam
> Sent: Thursday, June 25, 2020 10:17 AM
> To: Bin Meng <bmeng.cn@gmail.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Rick Chen
> <rick@andestech.com>; Lukasz Majewski <lukma@denx.de>; Jagan Teki
> <jagan@amarulasolutions.com>; Pragnesh Patel
> <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>; Simon
> Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>
> Subject: RE: [PATCH v4 1/4] fu540: prci: add request and free clock handlers
> 
> Hi Bin,
> 
> > -----Original Message-----
> > From: Bin Meng <bmeng.cn@gmail.com>
> > Sent: Thursday, June 25, 2020 4:51 AM
> > To: Sagar Kadam <sagar.kadam@sifive.com>
> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Rick Chen
> > <rick@andestech.com>; Lukasz Majewski <lukma@denx.de>; Jagan Teki
> > <jagan@amarulasolutions.com>; Pragnesh Patel
> > <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>; Simon
> > Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>
> > Subject: Re: [PATCH v4 1/4] fu540: prci: add request and free clock
> > handlers
> >
> > [External Email] Do not click links or attachments unless you
> > recognize the sender and know the content is safe
> >
> > Hi Sagar,
> >
> > On Wed, Jun 24, 2020 at 6:58 PM Sagar Kadam <sagar.kadam@sifive.com>
> > wrote:
> > >
> > > Hi Bin,
> > >
> > > > -----Original Message-----
> > > > From: Bin Meng <bmeng.cn@gmail.com>
> > > > Sent: Wednesday, June 24, 2020 6:50 AM
> > > > To: Sagar Kadam <sagar.kadam@sifive.com>
> > > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Rick Chen
> > > > <rick@andestech.com>; Lukasz Majewski <lukma@denx.de>; Jagan
> Teki
> > > > <jagan@amarulasolutions.com>; Pragnesh Patel
> > > > <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>;
> > Simon
> > > > Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>
> > > > Subject: Re: [PATCH v4 1/4] fu540: prci: add request and free
> > > > clock
> > handlers
> > > >
> > > > [External Email] Do not click links or attachments unless you
> > > > recognize
> > the
> > > > sender and know the content is safe
> > > >
> > > > Hi Sagar,
> > > >
> > > > On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
> > > > <sagar.kadam@sifive.com> wrote:
> > > > >
> > > > > Add clk_request handler to check if a valid clock is requested.
> > > > > Here clk_free handler is added for debug purpose which will
> > > > > display details of clock passed to clk_free.
> > > > >
> > > > > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> > > > > Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
> > > > > ---
> > > > >  drivers/clk/sifive/fu540-prci.c | 21 +++++++++++++++++++++
> > > > >  1 file changed, 21 insertions(+)
> > > > >
> > > > > diff --git a/drivers/clk/sifive/fu540-prci.c
> > > > > b/drivers/clk/sifive/fu540-prci.c index fe6e0d4..9a9ff6b 100644
> > > > > --- a/drivers/clk/sifive/fu540-prci.c
> > > > > +++ b/drivers/clk/sifive/fu540-prci.c
> > > > > @@ -686,6 +686,25 @@ static ulong
> > > > > sifive_fu540_prci_set_rate(struct
> > clk
> > > > *clk, ulong rate)
> > > > >         return rate;
> > > > >  }
> > > > >
> > > > > +static int sifive_fu540_prci_clk_request(struct clk *clk) {
> > > > > +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk-
> >dev,
> > > > > +             clk->id);
> > > > > +
> > > > > +       if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
> > > > > +               return -EINVAL;
> > > > > +
> > > > > +       return 0;
> > > > > +}
> > > > > +
> > > > > +static int sifive_fu540_prci_clk_free(struct clk *clk) {
> > > > > +       debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk-
> >dev,
> > > > > +             clk->id);
> > > > > +
> > > > > +       return 0;
> > > > > +}
> > > >
> > > > It seems these 2 routines do not actually do anything? Is this for
> > debugging
> > > > purposes?
> > > >
> > > The sifive_fu540_prci_clk_request will check if the clock requested
> > > is valid
> > or not.
> > > While the sifive_fu540_prci_clk_free function is just for debug.
> > > Is it ok if I retain these in V5 or you have some other thought here.
> > >
> >
> > OK, but I suspect the parameter check in
> > sifive_fu540_prci_clk_request() is not necessary too as currently the
> > codes work well.
> >
> 
> Ok. In that case I will than drop the check in sifive_fu540_prci_clk_request()
> and just keep the debug info.
> 
> Thanks & BR,
> Sagar
> 
Missed to add:
I had referred to other reference driver's and thought of keeping it.
So I'll drop this patch in V5 as it is not much value addition here.
Sorry for the confusion

Thanks & BR,
Sagar 

> > Regards,
> > Bin

[PATCH v4 2/4] riscv: dts: hifive-unleashed-a00: add cpu aliases

[Sagar Shrikant Kadam]

Add cpu aliases to U-Boot specific dtsi for hifive-unleashed.
Without aliases we see that the CPU device sequence numbers are set
randomly and the cpu list/detail command will show it as follows:
=> cpu list
  1: cpu at 0      rv64imac
  0: cpu at 1      rv64imafdc
  2: cpu at 2      rv64imafdc
  3: cpu at 3      rv64imafdc
  4: cpu at 4      rv64imafdc

Seems like CPU probing with dm-model also relies on aliases as observed
in case spi. The fu540-c000-u-boot.dtsi has cpu0/1/2/3/4 nodes and so
adding corresponding aliases we can ensure that cpu devices are assigned
proper sequence as follows:
=> cpu list
  0: cpu at 0      rv64imac
  1: cpu at 1      rv64imafdc
  2: cpu at 2      rv64imafdc
  3: cpu at 3      rv64imafdc
  4: cpu at 4      rv64imafdc

Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
Reviewed-by: Pragnesh Patel <pragnesh.patel@sifive.com>
---
 arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi
index 3038064..69a3cd3 100644
--- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi
+++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi
@@ -8,6 +8,11 @@
 
 / {
 	aliases {
+		cpu0 = &cpu0;
+		cpu1 = &cpu1;
+		cpu2 = &cpu2;
+		cpu3 = &cpu3;
+		cpu4 = &cpu4;
 		spi0 = &qspi0;
 		spi2 = &qspi2;
 	};
-- 
2.7.4

[PATCH v4 2/4] riscv: dts: hifive-unleashed-a00: add cpu aliases

[Bin Meng]

Hi Sagar,

On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
<sagar.kadam@sifive.com> wrote:
>
> Add cpu aliases to U-Boot specific dtsi for hifive-unleashed.
> Without aliases we see that the CPU device sequence numbers are set
> randomly and the cpu list/detail command will show it as follows:
> => cpu list
>   1: cpu at 0      rv64imac
>   0: cpu at 1      rv64imafdc
>   2: cpu at 2      rv64imafdc
>   3: cpu at 3      rv64imafdc
>   4: cpu at 4      rv64imafdc

Without this patch, my output does not show the cpu at 0 node in the "cpu
list" output.

Could you please clarify?

>
> Seems like CPU probing with dm-model also relies on aliases as observed
> in case spi. The fu540-c000-u-boot.dtsi has cpu0/1/2/3/4 nodes and so
> adding corresponding aliases we can ensure that cpu devices are assigned
> proper sequence as follows:
> => cpu list
>   0: cpu at 0      rv64imac
>   1: cpu at 1      rv64imafdc
>   2: cpu at 2      rv64imafdc
>   3: cpu at 3      rv64imafdc
>   4: cpu at 4      rv64imafdc
>
> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> Reviewed-by: Pragnesh Patel <pragnesh.patel@sifive.com>
> ---

Regards,
Bin

[PATCH v4 2/4] riscv: dts: hifive-unleashed-a00: add cpu aliases

[Sagar Kadam]

Hi Bin,

> -----Original Message-----
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: Wednesday, June 24, 2020 6:52 AM
> To: Sagar Kadam <sagar.kadam@sifive.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Rick Chen
> <rick@andestech.com>; Lukasz Majewski <lukma@denx.de>; Jagan Teki
> <jagan@amarulasolutions.com>; Pragnesh Patel
> <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>; Simon
> Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>
> Subject: Re: [PATCH v4 2/4] riscv: dts: hifive-unleashed-a00: add cpu aliases
> 
> [External Email] Do not click links or attachments unless you recognize the
> sender and know the content is safe
> 
> Hi Sagar,
> 
> On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
> <sagar.kadam@sifive.com> wrote:
> >
> > Add cpu aliases to U-Boot specific dtsi for hifive-unleashed.
> > Without aliases we see that the CPU device sequence numbers are set
> > randomly and the cpu list/detail command will show it as follows:
> > => cpu list
> >   1: cpu at 0      rv64imac
> >   0: cpu at 1      rv64imafdc
> >   2: cpu at 2      rv64imafdc
> >   3: cpu at 3      rv64imafdc
> >   4: cpu at 4      rv64imafdc
> 
> Without this patch, my output does not show the cpu at 0 node in the "cpu
> list" output.
> 
> Could you please clarify?

I guess my explanation for your query in cover letter follow's here to.
I will repost this based on spl.

Thanks,
Sagar

> 
> >
> > Seems like CPU probing with dm-model also relies on aliases as
> > observed in case spi. The fu540-c000-u-boot.dtsi has cpu0/1/2/3/4
> > nodes and so adding corresponding aliases we can ensure that cpu
> > devices are assigned proper sequence as follows:
> > => cpu list
> >   0: cpu at 0      rv64imac
> >   1: cpu at 1      rv64imafdc
> >   2: cpu at 2      rv64imafdc
> >   3: cpu at 3      rv64imafdc
> >   4: cpu at 4      rv64imafdc
> >
> > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> > Reviewed-by: Pragnesh Patel <pragnesh.patel@sifive.com>
> > ---
> 
> Regards,
> Bin

[PATCH v4 3/4] riscv: cpu: fixes to display proper CPU features

[Sagar Shrikant Kadam]

The cmd "cpu detail" fetches uninitialized cpu feature information
and thus displays wrong / inconsitent details as below. FU540-C000 doesn't
have any microcode, yet the cmd display's it.
=> cpu detail
  0: cpu at 0      rv64imac
        ID = 0, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
        Microcode version 0x0
        Device ID 0x0
  1: cpu at 1      rv64imafdc
        ID = 1, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
        Microcode version 0x0
        Device ID 0x0
  2: cpu at 2      rv64imafdc
        ID = 2, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
        Microcode version 0x0
        Device ID 0x0
  3: cpu at 3      rv64imafdc
        ID = 3, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
        Microcode version 0x0
        Device ID 0x0
  4: cpu at 4      rv64imafdc
        ID = 4, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
        Microcode version 0x0
        Device ID 0x0

The L1 cache or MMU entry seen above is also displayed inconsistently.
So initialize features to zero before fetching from device tree.
Additionally the conditional check to read "mmu-type" from device tree
is not rightly handled due to which the cpu feature doesn't include
CPU_FEAT_MMU even if it's corresponding entry is present in device tree.

We now see correct features as:

=> cpu detail
  0: cpu at 0      rv64imac
        ID = 0, freq = 999.100 MHz
  1: cpu at 1      rv64imafdc
        ID = 1, freq = 999.100 MHz: MMU
  2: cpu at 2      rv64imafdc
        ID = 2, freq = 999.100 MHz: MMU
  3: cpu at 3      rv64imafdc
        ID = 3, freq = 999.100 MHz: MMU
  4: cpu at 4      rv64imafdc
        ID = 4, freq = 999.100 MHz: MMU

Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
Reviewed-by: Pragnesh Patel <pragnesh.patel@sifive.com>
---
 drivers/cpu/riscv_cpu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c
index 76b0489..8c4b5e7 100644
--- a/drivers/cpu/riscv_cpu.c
+++ b/drivers/cpu/riscv_cpu.c
@@ -38,6 +38,8 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info)
 
 	/* Zero out the frequency, in case sizeof(ulong) != sizeof(u32) */
 	info->cpu_freq = 0;
+	/* Initialise cpu features before updating from device tree */
+	info->features = 0;
 
 	/* First try getting the frequency from the assigned clock */
 	ret = clk_get_by_index(dev, 0, &clk);
@@ -52,7 +54,7 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info)
 		dev_read_u32(dev, "clock-frequency", (u32 *)&info->cpu_freq);
 
 	mmu = dev_read_string(dev, "mmu-type");
-	if (!mmu)
+	if (mmu)
 		info->features |= BIT(CPU_FEAT_MMU);
 
 	return 0;
-- 
2.7.4

[PATCH v4 3/4] riscv: cpu: fixes to display proper CPU features

[Bin Meng]

Hi Simon,

On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
<sagar.kadam@sifive.com> wrote:
>
> The cmd "cpu detail" fetches uninitialized cpu feature information
> and thus displays wrong / inconsitent details as below. FU540-C000 doesn't
> have any microcode, yet the cmd display's it.
> => cpu detail
>   0: cpu at 0      rv64imac
>         ID = 0, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
>         Microcode version 0x0
>         Device ID 0x0
>   1: cpu at 1      rv64imafdc
>         ID = 1, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
>         Microcode version 0x0
>         Device ID 0x0
>   2: cpu at 2      rv64imafdc
>         ID = 2, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
>         Microcode version 0x0
>         Device ID 0x0
>   3: cpu at 3      rv64imafdc
>         ID = 3, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
>         Microcode version 0x0
>         Device ID 0x0
>   4: cpu at 4      rv64imafdc
>         ID = 4, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
>         Microcode version 0x0
>         Device ID 0x0
>
> The L1 cache or MMU entry seen above is also displayed inconsistently.
> So initialize features to zero before fetching from device tree.
> Additionally the conditional check to read "mmu-type" from device tree
> is not rightly handled due to which the cpu feature doesn't include
> CPU_FEAT_MMU even if it's corresponding entry is present in device tree.
>
> We now see correct features as:
>
> => cpu detail
>   0: cpu at 0      rv64imac
>         ID = 0, freq = 999.100 MHz
>   1: cpu at 1      rv64imafdc
>         ID = 1, freq = 999.100 MHz: MMU
>   2: cpu at 2      rv64imafdc
>         ID = 2, freq = 999.100 MHz: MMU
>   3: cpu at 3      rv64imafdc
>         ID = 3, freq = 999.100 MHz: MMU
>   4: cpu at 4      rv64imafdc
>         ID = 4, freq = 999.100 MHz: MMU
>
> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> Reviewed-by: Pragnesh Patel <pragnesh.patel@sifive.com>
> ---
>  drivers/cpu/riscv_cpu.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c
> index 76b0489..8c4b5e7 100644
> --- a/drivers/cpu/riscv_cpu.c
> +++ b/drivers/cpu/riscv_cpu.c
> @@ -38,6 +38,8 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info)
>
>         /* Zero out the frequency, in case sizeof(ulong) != sizeof(u32) */
>         info->cpu_freq = 0;
> +       /* Initialise cpu features before updating from device tree */
> +       info->features = 0;

For this one, do you think we should fix the cpu_get_info() in
cpu-uclass driver instead? With fix in the cpu-uclass driver we can
avoid similar issue in any single CPU driver.

>
>         /* First try getting the frequency from the assigned clock */
>         ret = clk_get_by_index(dev, 0, &clk);
> @@ -52,7 +54,7 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info)
>                 dev_read_u32(dev, "clock-frequency", (u32 *)&info->cpu_freq);
>
>         mmu = dev_read_string(dev, "mmu-type");
> -       if (!mmu)
> +       if (mmu)
>                 info->features |= BIT(CPU_FEAT_MMU);
>
>         return 0;

Regards,
Bin

[PATCH v4 3/4] riscv: cpu: fixes to display proper CPU features

[Simon Glass]

Hi Bin,

On Tue, 23 Jun 2020 at 19:26, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
> <sagar.kadam@sifive.com> wrote:
> >
> > The cmd "cpu detail" fetches uninitialized cpu feature information
> > and thus displays wrong / inconsitent details as below. FU540-C000 doesn't
> > have any microcode, yet the cmd display's it.
> > => cpu detail
> >   0: cpu at 0      rv64imac
> >         ID = 0, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
> >         Microcode version 0x0
> >         Device ID 0x0
> >   1: cpu at 1      rv64imafdc
> >         ID = 1, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
> >         Microcode version 0x0
> >         Device ID 0x0
> >   2: cpu at 2      rv64imafdc
> >         ID = 2, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
> >         Microcode version 0x0
> >         Device ID 0x0
> >   3: cpu at 3      rv64imafdc
> >         ID = 3, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
> >         Microcode version 0x0
> >         Device ID 0x0
> >   4: cpu at 4      rv64imafdc
> >         ID = 4, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
> >         Microcode version 0x0
> >         Device ID 0x0
> >
> > The L1 cache or MMU entry seen above is also displayed inconsistently.
> > So initialize features to zero before fetching from device tree.
> > Additionally the conditional check to read "mmu-type" from device tree
> > is not rightly handled due to which the cpu feature doesn't include
> > CPU_FEAT_MMU even if it's corresponding entry is present in device tree.
> >
> > We now see correct features as:
> >
> > => cpu detail
> >   0: cpu at 0      rv64imac
> >         ID = 0, freq = 999.100 MHz
> >   1: cpu at 1      rv64imafdc
> >         ID = 1, freq = 999.100 MHz: MMU
> >   2: cpu at 2      rv64imafdc
> >         ID = 2, freq = 999.100 MHz: MMU
> >   3: cpu at 3      rv64imafdc
> >         ID = 3, freq = 999.100 MHz: MMU
> >   4: cpu at 4      rv64imafdc
> >         ID = 4, freq = 999.100 MHz: MMU
> >
> > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> > Reviewed-by: Pragnesh Patel <pragnesh.patel@sifive.com>
> > ---
> >  drivers/cpu/riscv_cpu.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c
> > index 76b0489..8c4b5e7 100644
> > --- a/drivers/cpu/riscv_cpu.c
> > +++ b/drivers/cpu/riscv_cpu.c
> > @@ -38,6 +38,8 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info)
> >
> >         /* Zero out the frequency, in case sizeof(ulong) != sizeof(u32) */
> >         info->cpu_freq = 0;
> > +       /* Initialise cpu features before updating from device tree */
> > +       info->features = 0;
>
> For this one, do you think we should fix the cpu_get_info() in
> cpu-uclass driver instead? With fix in the cpu-uclass driver we can
> avoid similar issue in any single CPU driver.

Yes it would be best to fix that function to zero the data.

Regards,
Simon

[PATCH v4 4/4] riscv: cpu: check and append L1 cache to cpu features

[Sagar Shrikant Kadam]

All cpu cores within FU540-C000 having split I/D caches.
Set the L1 cache feature bit using the i-cache-size as one of the
property from device tree indicating that L1 cache is present
on the cpu core.

=> cpu detail
  0: cpu at 0      rv64imac
        ID = 0, freq = 999.100 MHz: L1 cache
  1: cpu at 1      rv64imafdc
        ID = 1, freq = 999.100 MHz: L1 cache, MMU
  2: cpu at 2      rv64imafdc
        ID = 2, freq = 999.100 MHz: L1 cache, MMU
  3: cpu at 3      rv64imafdc
        ID = 3, freq = 999.100 MHz: L1 cache, MMU
  4: cpu at 4      rv64imafdc
        ID = 4, freq = 999.100 MHz: L1 cache, MMU

Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
---
 drivers/cpu/riscv_cpu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c
index 8c4b5e7..ce722cb 100644
--- a/drivers/cpu/riscv_cpu.c
+++ b/drivers/cpu/riscv_cpu.c
@@ -35,6 +35,7 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info)
 	int ret;
 	struct clk clk;
 	const char *mmu;
+	u32 split_cache_size;
 
 	/* Zero out the frequency, in case sizeof(ulong) != sizeof(u32) */
 	info->cpu_freq = 0;
@@ -57,6 +58,11 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info)
 	if (mmu)
 		info->features |= BIT(CPU_FEAT_MMU);
 
+	/* check if I/D cache is present  */
+	ret = dev_read_u32(dev, "i-cache-size", &split_cache_size);
+	if (!ret)
+		info->features |= BIT(CPU_FEAT_L1_CACHE);
+
 	return 0;
 }
 
-- 
2.7.4

[PATCH v4 4/4] riscv: cpu: check and append L1 cache to cpu features

[Bin Meng]

On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
<sagar.kadam@sifive.com> wrote:
>
> All cpu cores within FU540-C000 having split I/D caches.
> Set the L1 cache feature bit using the i-cache-size as one of the
> property from device tree indicating that L1 cache is present
> on the cpu core.
>
> => cpu detail
>   0: cpu at 0      rv64imac
>         ID = 0, freq = 999.100 MHz: L1 cache
>   1: cpu at 1      rv64imafdc
>         ID = 1, freq = 999.100 MHz: L1 cache, MMU
>   2: cpu at 2      rv64imafdc
>         ID = 2, freq = 999.100 MHz: L1 cache, MMU
>   3: cpu at 3      rv64imafdc
>         ID = 3, freq = 999.100 MHz: L1 cache, MMU
>   4: cpu at 4      rv64imafdc
>         ID = 4, freq = 999.100 MHz: L1 cache, MMU
>
> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
> ---
>  drivers/cpu/riscv_cpu.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c
> index 8c4b5e7..ce722cb 100644
> --- a/drivers/cpu/riscv_cpu.c
> +++ b/drivers/cpu/riscv_cpu.c
> @@ -35,6 +35,7 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info)
>         int ret;
>         struct clk clk;
>         const char *mmu;
> +       u32 split_cache_size;
>
>         /* Zero out the frequency, in case sizeof(ulong) != sizeof(u32) */
>         info->cpu_freq = 0;
> @@ -57,6 +58,11 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info)
>         if (mmu)
>                 info->features |= BIT(CPU_FEAT_MMU);
>
> +       /* check if I/D cache is present  */
> +       ret = dev_read_u32(dev, "i-cache-size", &split_cache_size);

What about testing either "i-cache-size" and "d-cache-size", and if
either one exists, set CPU_FEAT_L1_CACHE

> +       if (!ret)
> +               info->features |= BIT(CPU_FEAT_L1_CACHE);
> +
>         return 0;
>  }

Regards,
Bin

[PATCH v4 4/4] riscv: cpu: check and append L1 cache to cpu features

[Sagar Kadam]

Hi Bin,

> -----Original Message-----
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: Wednesday, June 24, 2020 6:58 AM
> To: Sagar Kadam <sagar.kadam@sifive.com>; Simon Glass
> <sjg@chromium.org>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Rick Chen
> <rick@andestech.com>; Lukasz Majewski <lukma@denx.de>; Jagan Teki
> <jagan@amarulasolutions.com>; Pragnesh Patel
> <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>; Sean
> Anderson <seanga2@gmail.com>
> Subject: Re: [PATCH v4 4/4] riscv: cpu: check and append L1 cache to cpu
> features
> 
> [External Email] Do not click links or attachments unless you recognize the
> sender and know the content is safe
> 
> On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
> <sagar.kadam@sifive.com> wrote:
> >
> > All cpu cores within FU540-C000 having split I/D caches.
> > Set the L1 cache feature bit using the i-cache-size as one of the
> > property from device tree indicating that L1 cache is present
> > on the cpu core.
> >
> > => cpu detail
> >   0: cpu at 0      rv64imac
> >         ID = 0, freq = 999.100 MHz: L1 cache
> >   1: cpu at 1      rv64imafdc
> >         ID = 1, freq = 999.100 MHz: L1 cache, MMU
> >   2: cpu at 2      rv64imafdc
> >         ID = 2, freq = 999.100 MHz: L1 cache, MMU
> >   3: cpu at 3      rv64imafdc
> >         ID = 3, freq = 999.100 MHz: L1 cache, MMU
> >   4: cpu at 4      rv64imafdc
> >         ID = 4, freq = 999.100 MHz: L1 cache, MMU
> >
> > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com>
> > Reviewed-by: Pragnesh Patel <Pragnesh.patel@sifive.com>
> > ---
> >  drivers/cpu/riscv_cpu.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c
> > index 8c4b5e7..ce722cb 100644
> > --- a/drivers/cpu/riscv_cpu.c
> > +++ b/drivers/cpu/riscv_cpu.c
> > @@ -35,6 +35,7 @@ static int riscv_cpu_get_info(struct udevice *dev,
> struct cpu_info *info)
> >         int ret;
> >         struct clk clk;
> >         const char *mmu;
> > +       u32 split_cache_size;
> >
> >         /* Zero out the frequency, in case sizeof(ulong) != sizeof(u32) */
> >         info->cpu_freq = 0;
> > @@ -57,6 +58,11 @@ static int riscv_cpu_get_info(struct udevice *dev,
> struct cpu_info *info)
> >         if (mmu)
> >                 info->features |= BIT(CPU_FEAT_MMU);
> >
> > +       /* check if I/D cache is present  */
> > +       ret = dev_read_u32(dev, "i-cache-size", &split_cache_size);
> 
> What about testing either "i-cache-size" and "d-cache-size", and if
> either one exists, set CPU_FEAT_L1_CACHE
>
Ok. I will repost it with d-cache check as well.

Thanks & BR,
Sagar Kadam
> > +       if (!ret)
> > +               info->features |= BIT(CPU_FEAT_L1_CACHE);
> > +
> >         return 0;
> >  }
> 
> Regards,
> Bin

[PATCH v4 0/4] update clock handler and proper cpu features

[Bin Meng]

Hi Sagar,

On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
<sagar.kadam@sifive.com> wrote:
>
> U-Boot cmd "cpu detail" shows inconsistent CPU features and is missing
> clk_request and free handlers.
> The current "cpu detail" sometimes shows "Microcode" as a feature, which
> is not the case with FU540-C000 on HiFive Unleashed board.
>
> Patch 1: add clk request handler to check if valid clock id is requested.
> Patch 2: add cpu node aliases.
> Patch 3: Correctly parse and update mmu-type.
>
> RISC-V core's on FU540-C000 SoC have separate instruction and data (split)
> L1 cache.
> Patch 4:Use i-cache-size dt property as one of identifier to indicate a
>         split cache is available.
>
> I have picked few dependent patches from Sean's series from here [1]
> and [2].
>
> These have applied on mainline U-Boot commit 2b8692bac1e8 ("Merge tag
> 'efi-2020-07-rc5-2' of https://gitlab.denx.de/u-boot/custodians/u-boot-efi")
>
> Patch history:
> =============================================
> V4:
> 1. Rebased the series to mainline commit.
> 2. Updated dependency list as few patches are now merged.
> 3. Added U-Boot log of the flow i.e fsbl + fw_payload.bin (Opensbi+U-Boot)
>
> V3:
> 1. Included the cosmetic change as suggested
>    s/L1 feature/L1 cache feature/
> 2. Added Reviewed-By tags
>
> V2:
> 1. Incorporate review comments from Bin and Sean Anderson.
>    and dropped 2nd patch as similar work was already done in [1] and [2]
> 2  Add cpu node aliases to display cpu node's in sequence.
> 3. Add fix to show mmu as available cpu feature.
> 4. Check and append L1 cache feature.
>
> V1: Base version
>     Thanks to Vincent Chen <vincent.chen@sifive.com> for testing the V1
>     version of this series.
>
> [1] https://patchwork.ozlabs.org/patch/1295345
> [2] https://patchwork.ozlabs.org/patch/1295346
>
> All these together is available here:
> https://github.com/sagsifive/u-boot/commits/dev/sagark/clk-v4
>
> U-Boot log:
> ===========================================================
> SiFive FSBL:       2020-02-19-1081db9-dirty
> Using new FSBL DTB now
> HiFive-U-serial-#: 000002c8
> Loading boot payload....
>
> OpenSBI v0.7-31-gd626037
>    ____                    _____ ____ _____
>   / __ \                  / ____|  _ \_   _|
>  | |  | |_ __   ___ _ __ | (___ | |_) || |
>  | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
>  | |__| | |_) |  __/ | | |____) | |_) || |_
>   \____/| .__/ \___|_| |_|_____/|____/_____|
>         | |
>         |_|
>
> Platform Name          : SiFive Freedom U540
> Platform HART Features : RV64ACDFIMSU
> Platform HART Count    : 4
> Current HART ID        : 1
> Firmware Base          : 0x80000000
> Firmware Size          : 100 KB
> Runtime SBI Version    : 0.2
>
> MIDELEG : 0x0000000000000222
> MEDELEG : 0x000000000000b109
> PMP0    : 0x0000000080000000-0x000000008001ffff (A)
> PMP1    : 0x0000000000000000-0x0000007fffffffff (A,R,W,X)
>
>
> U-Boot 2020.07-rc4-00084-gf824d2c (Jun 21 2020 - 04:58:40 -0700)
>
> CPU:   rv64imac
> Model: SiFive HiFive Unleashed A00
> DRAM:  8 GiB
> MMC:   spi at 10050000:mmc at 0: 0
> In:    serial at 10010000
> Out:   serial at 10010000
> Err:   serial at 10010000
> Net:   eth0: ethernet at 10090000
> Hit any key to stop autoboot:  0
> => cpu detail
>   0: cpu at 0      rv64imac
>         ID = 0, freq = 999.100 MHz: L1 cache
>   1: cpu at 1      rv64imafdc
>         ID = 1, freq = 999.100 MHz: L1 cache, MMU
>   2: cpu at 2      rv64imafdc
>         ID = 2, freq = 999.100 MHz: L1 cache, MMU
>   3: cpu at 3      rv64imafdc
>         ID = 3, freq = 999.100 MHz: L1 cache, MMU
>   4: cpu at 4      rv64imafdc
>         ID = 4, freq = 999.100 MHz: L1 cache, MMU
> =>
>

It's strange that I am seeing different output without your patch:

=> cpu list
  1: cpu at 1      rv64imafdc
  2: cpu at 2      rv64imafdc
  0: cpu at 3      rv64imafdc
  3: cpu at 4      rv64imafdc
=> cpu detail
  1: cpu at 1      rv64imafdc
        ID = 1, freq = 12 Hz: L1 cache
  2: cpu at 2      rv64imafdc
        ID = 2, freq = 12 Hz: L1 cache
  0: cpu at 3      rv64imafdc
        ID = 3, freq = 12 Hz: L1 cache
  3: cpu at 4      rv64imafdc
        ID = 4, freq = 12 Hz: L1 cache

It looks like your patch included the E51 core (hartid 0) in the output?

Regards,
Bin

[PATCH v4 0/4] update clock handler and proper cpu features

[Sagar Kadam]

Hi Bin,

> -----Original Message-----
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: Wednesday, June 24, 2020 6:46 AM
> To: Sagar Kadam <sagar.kadam@sifive.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Rick Chen
> <rick@andestech.com>; Lukasz Majewski <lukma@denx.de>; Jagan Teki
> <jagan@amarulasolutions.com>; Pragnesh Patel
> <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>; Simon
> Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>
> Subject: Re: [PATCH v4 0/4] update clock handler and proper cpu features
> 
> [External Email] Do not click links or attachments unless you recognize the
> sender and know the content is safe
> 
> Hi Sagar,
> 
> On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
> <sagar.kadam@sifive.com> wrote:
> >
> > U-Boot cmd "cpu detail" shows inconsistent CPU features and is missing
> > clk_request and free handlers.
> > The current "cpu detail" sometimes shows "Microcode" as a feature,
> > which is not the case with FU540-C000 on HiFive Unleashed board.
> >
> > Patch 1: add clk request handler to check if valid clock id is requested.
> > Patch 2: add cpu node aliases.
> > Patch 3: Correctly parse and update mmu-type.
> >
> > RISC-V core's on FU540-C000 SoC have separate instruction and data
> > (split)
> > L1 cache.
> > Patch 4:Use i-cache-size dt property as one of identifier to indicate a
> >         split cache is available.
> >
> > I have picked few dependent patches from Sean's series from here [1]
> > and [2].
> >
> > These have applied on mainline U-Boot commit 2b8692bac1e8 ("Merge
> tag
> > 'efi-2020-07-rc5-2' of
> > https://gitlab.denx.de/u-boot/custodians/u-boot-efi")
> >
> > Patch history:
> > =============================================
> > V4:
> > 1. Rebased the series to mainline commit.
> > 2. Updated dependency list as few patches are now merged.
> > 3. Added U-Boot log of the flow i.e fsbl + fw_payload.bin
> > (Opensbi+U-Boot)
> >
> > V3:
> > 1. Included the cosmetic change as suggested
> >    s/L1 feature/L1 cache feature/
> > 2. Added Reviewed-By tags
> >
> > V2:
> > 1. Incorporate review comments from Bin and Sean Anderson.
> >    and dropped 2nd patch as similar work was already done in [1] and
> > [2]
> > 2  Add cpu node aliases to display cpu node's in sequence.
> > 3. Add fix to show mmu as available cpu feature.
> > 4. Check and append L1 cache feature.
> >
> > V1: Base version
> >     Thanks to Vincent Chen <vincent.chen@sifive.com> for testing the V1
> >     version of this series.
> >
> > [1] https://patchwork.ozlabs.org/patch/1295345
> > [2] https://patchwork.ozlabs.org/patch/1295346
> >
> > All these together is available here:
> > https://github.com/sagsifive/u-boot/commits/dev/sagark/clk-v4
> >
> > U-Boot log:
> > ===========================================================
> > SiFive FSBL:       2020-02-19-1081db9-dirty
> > Using new FSBL DTB now
> > HiFive-U-serial-#: 000002c8
> > Loading boot payload....
> >
> > OpenSBI v0.7-31-gd626037
> >    ____                    _____ ____ _____
> >   / __ \                  / ____|  _ \_   _|
> >  | |  | |_ __   ___ _ __ | (___ | |_) || |
> >  | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |  | |__| | |_) |  __/ | |
> > |____) | |_) || |_
> >   \____/| .__/ \___|_| |_|_____/|____/_____|
> >         | |
> >         |_|
> >
> > Platform Name          : SiFive Freedom U540
> > Platform HART Features : RV64ACDFIMSU
> > Platform HART Count    : 4
> > Current HART ID        : 1
> > Firmware Base          : 0x80000000
> > Firmware Size          : 100 KB
> > Runtime SBI Version    : 0.2
> >
> > MIDELEG : 0x0000000000000222
> > MEDELEG : 0x000000000000b109
> > PMP0    : 0x0000000080000000-0x000000008001ffff (A)
> > PMP1    : 0x0000000000000000-0x0000007fffffffff (A,R,W,X)
> >
> >
> > U-Boot 2020.07-rc4-00084-gf824d2c (Jun 21 2020 - 04:58:40 -0700)
> >
> > CPU:   rv64imac
> > Model: SiFive HiFive Unleashed A00
> > DRAM:  8 GiB
> > MMC:   spi at 10050000:mmc at 0: 0
> > In:    serial at 10010000
> > Out:   serial at 10010000
> > Err:   serial at 10010000
> > Net:   eth0: ethernet at 10090000
> > Hit any key to stop autoboot:  0
> > => cpu detail
> >   0: cpu at 0      rv64imac
> >         ID = 0, freq = 999.100 MHz: L1 cache
> >   1: cpu at 1      rv64imafdc
> >         ID = 1, freq = 999.100 MHz: L1 cache, MMU
> >   2: cpu at 2      rv64imafdc
> >         ID = 2, freq = 999.100 MHz: L1 cache, MMU
> >   3: cpu at 3      rv64imafdc
> >         ID = 3, freq = 999.100 MHz: L1 cache, MMU
> >   4: cpu at 4      rv64imafdc
> >         ID = 4, freq = 999.100 MHz: L1 cache, MMU =>
> >
> 
> It's strange that I am seeing different output without your patch:
> 
> => cpu list
>   1: cpu at 1      rv64imafdc
>   2: cpu at 2      rv64imafdc
>   0: cpu at 3      rv64imafdc
>   3: cpu at 4      rv64imafdc
> => cpu detail
>   1: cpu at 1      rv64imafdc
>         ID = 1, freq = 12 Hz: L1 cache
>   2: cpu at 2      rv64imafdc
>         ID = 2, freq = 12 Hz: L1 cache
>   0: cpu at 3      rv64imafdc
>         ID = 3, freq = 12 Hz: L1 cache
>   3: cpu at 4      rv64imafdc
>         ID = 4, freq = 12 Hz: L1 cache
> 
> It looks like your patch included the E51 core (hartid 0) in the output?
> 

I had observed that with spl+u-boot proper we do see the necessary
u-cores only (only 4 hart's). I guess you are also using spl->u-boot proper to 
check this.
My earlier series was based with fsbl + u-boot approach where all harts are listed 
on the prompt I used the same in v4 which show's 1-E and 4-U cores.
Now since SPL is part of mainline. I can repost this which will show only 4 U-cores.
Please let me know if that's ok.

> Regards,
> Bin

[PATCH v4 0/4] update clock handler and proper cpu features

[Bin Meng]

Hi Sagar,

On Wed, Jun 24, 2020 at 2:01 PM Sagar Kadam <sagar.kadam@sifive.com> wrote:
>
> Hi Bin,
>
> > -----Original Message-----
> > From: Bin Meng <bmeng.cn@gmail.com>
> > Sent: Wednesday, June 24, 2020 6:46 AM
> > To: Sagar Kadam <sagar.kadam@sifive.com>
> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Rick Chen
> > <rick@andestech.com>; Lukasz Majewski <lukma@denx.de>; Jagan Teki
> > <jagan@amarulasolutions.com>; Pragnesh Patel
> > <pragnesh.patel@sifive.com>; Anup Patel <anup.patel@wdc.com>; Simon
> > Glass <sjg@chromium.org>; Sean Anderson <seanga2@gmail.com>
> > Subject: Re: [PATCH v4 0/4] update clock handler and proper cpu features
> >
> > [External Email] Do not click links or attachments unless you recognize the
> > sender and know the content is safe
> >
> > Hi Sagar,
> >
> > On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
> > <sagar.kadam@sifive.com> wrote:
> > >
> > > U-Boot cmd "cpu detail" shows inconsistent CPU features and is missing
> > > clk_request and free handlers.
> > > The current "cpu detail" sometimes shows "Microcode" as a feature,
> > > which is not the case with FU540-C000 on HiFive Unleashed board.
> > >
> > > Patch 1: add clk request handler to check if valid clock id is requested.
> > > Patch 2: add cpu node aliases.
> > > Patch 3: Correctly parse and update mmu-type.
> > >
> > > RISC-V core's on FU540-C000 SoC have separate instruction and data
> > > (split)
> > > L1 cache.
> > > Patch 4:Use i-cache-size dt property as one of identifier to indicate a
> > >         split cache is available.
> > >
> > > I have picked few dependent patches from Sean's series from here [1]
> > > and [2].
> > >
> > > These have applied on mainline U-Boot commit 2b8692bac1e8 ("Merge
> > tag
> > > 'efi-2020-07-rc5-2' of
> > > https://gitlab.denx.de/u-boot/custodians/u-boot-efi")
> > >
> > > Patch history:
> > > =============================================
> > > V4:
> > > 1. Rebased the series to mainline commit.
> > > 2. Updated dependency list as few patches are now merged.
> > > 3. Added U-Boot log of the flow i.e fsbl + fw_payload.bin
> > > (Opensbi+U-Boot)
> > >
> > > V3:
> > > 1. Included the cosmetic change as suggested
> > >    s/L1 feature/L1 cache feature/
> > > 2. Added Reviewed-By tags
> > >
> > > V2:
> > > 1. Incorporate review comments from Bin and Sean Anderson.
> > >    and dropped 2nd patch as similar work was already done in [1] and
> > > [2]
> > > 2  Add cpu node aliases to display cpu node's in sequence.
> > > 3. Add fix to show mmu as available cpu feature.
> > > 4. Check and append L1 cache feature.
> > >
> > > V1: Base version
> > >     Thanks to Vincent Chen <vincent.chen@sifive.com> for testing the V1
> > >     version of this series.
> > >
> > > [1] https://patchwork.ozlabs.org/patch/1295345
> > > [2] https://patchwork.ozlabs.org/patch/1295346
> > >
> > > All these together is available here:
> > > https://github.com/sagsifive/u-boot/commits/dev/sagark/clk-v4
> > >
> > > U-Boot log:
> > > ===========================================================
> > > SiFive FSBL:       2020-02-19-1081db9-dirty
> > > Using new FSBL DTB now
> > > HiFive-U-serial-#: 000002c8
> > > Loading boot payload....
> > >
> > > OpenSBI v0.7-31-gd626037
> > >    ____                    _____ ____ _____
> > >   / __ \                  / ____|  _ \_   _|
> > >  | |  | |_ __   ___ _ __ | (___ | |_) || |
> > >  | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |  | |__| | |_) |  __/ | |
> > > |____) | |_) || |_
> > >   \____/| .__/ \___|_| |_|_____/|____/_____|
> > >         | |
> > >         |_|
> > >
> > > Platform Name          : SiFive Freedom U540
> > > Platform HART Features : RV64ACDFIMSU
> > > Platform HART Count    : 4
> > > Current HART ID        : 1
> > > Firmware Base          : 0x80000000
> > > Firmware Size          : 100 KB
> > > Runtime SBI Version    : 0.2
> > >
> > > MIDELEG : 0x0000000000000222
> > > MEDELEG : 0x000000000000b109
> > > PMP0    : 0x0000000080000000-0x000000008001ffff (A)
> > > PMP1    : 0x0000000000000000-0x0000007fffffffff (A,R,W,X)
> > >
> > >
> > > U-Boot 2020.07-rc4-00084-gf824d2c (Jun 21 2020 - 04:58:40 -0700)
> > >
> > > CPU:   rv64imac
> > > Model: SiFive HiFive Unleashed A00
> > > DRAM:  8 GiB
> > > MMC:   spi at 10050000:mmc at 0: 0
> > > In:    serial at 10010000
> > > Out:   serial at 10010000
> > > Err:   serial at 10010000
> > > Net:   eth0: ethernet at 10090000
> > > Hit any key to stop autoboot:  0
> > > => cpu detail
> > >   0: cpu at 0      rv64imac
> > >         ID = 0, freq = 999.100 MHz: L1 cache
> > >   1: cpu at 1      rv64imafdc
> > >         ID = 1, freq = 999.100 MHz: L1 cache, MMU
> > >   2: cpu at 2      rv64imafdc
> > >         ID = 2, freq = 999.100 MHz: L1 cache, MMU
> > >   3: cpu at 3      rv64imafdc
> > >         ID = 3, freq = 999.100 MHz: L1 cache, MMU
> > >   4: cpu at 4      rv64imafdc
> > >         ID = 4, freq = 999.100 MHz: L1 cache, MMU =>
> > >
> >
> > It's strange that I am seeing different output without your patch:
> >
> > => cpu list
> >   1: cpu at 1      rv64imafdc
> >   2: cpu at 2      rv64imafdc
> >   0: cpu at 3      rv64imafdc
> >   3: cpu at 4      rv64imafdc
> > => cpu detail
> >   1: cpu at 1      rv64imafdc
> >         ID = 1, freq = 12 Hz: L1 cache
> >   2: cpu at 2      rv64imafdc
> >         ID = 2, freq = 12 Hz: L1 cache
> >   0: cpu at 3      rv64imafdc
> >         ID = 3, freq = 12 Hz: L1 cache
> >   3: cpu at 4      rv64imafdc
> >         ID = 4, freq = 12 Hz: L1 cache
> >
> > It looks like your patch included the E51 core (hartid 0) in the output?
> >
>
> I had observed that with spl+u-boot proper we do see the necessary
> u-cores only (only 4 hart's). I guess you are also using spl->u-boot proper to
> check this.
> My earlier series was based with fsbl + u-boot approach where all harts are listed
> on the prompt I used the same in v4 which show's 1-E and 4-U cores.
> Now since SPL is part of mainline. I can repost this which will show only 4 U-cores.
> Please let me know if that's ok.

Thanks for the clarifications. Yes, please repost with U-Boot SPL + proper logs.

Regards,
Bin