diff mbox series

[v1,1/1] board: ns3: check bnxt chimp handshake status

Message ID 20200820174134.17100-1-rayagonda.kokatanur@broadcom.com
State Superseded
Delegated to: Tom Rini
Headers show
Series [v1,1/1] board: ns3: check bnxt chimp handshake status | expand

Commit Message

Rayagonda Kokatanur Aug. 20, 2020, 5:41 p.m. UTC
Chimp is a core in Broadcom netxtream controller (bnxt).
Add support to check bnxt's chimp component status.

Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
---
 board/broadcom/bcmns3/ns3.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Simon Glass Aug. 22, 2020, 3:09 p.m. UTC | #1
Hi Rayagonda,

On Thu, 20 Aug 2020 at 11:42, Rayagonda Kokatanur
<rayagonda.kokatanur@broadcom.com> wrote:
>
> Chimp is a core in Broadcom netxtream controller (bnxt).
> Add support to check bnxt's chimp component status.
>
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> ---
>  board/broadcom/bcmns3/ns3.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c
> index 0357cd0e32..8540c99286 100644
> --- a/board/broadcom/bcmns3/ns3.c
> +++ b/board/broadcom/bcmns3/ns3.c
> @@ -12,6 +12,7 @@
>  #include <asm/armv8/mmu.h>
>  #include <asm/arch-bcmns3/bl33_info.h>
>  #include <dt-bindings/memory/bcm-ns3-mc.h>
> +#include <broadcom/chimp.h>
>
>  /* Default reset-level = 3 and strap-val = 0 */
>  #define L3_RESET       30
> @@ -210,8 +211,20 @@ void reset_cpu(ulong level)
>  #ifdef CONFIG_OF_BOARD_SETUP
>  int ft_board_setup(void *fdt, struct bd_info *bd)
>  {
> +       u32 chimp_hs = CHIMP_HANDSHAKE_WAIT_TIMEOUT;
> +
>         gic_lpi_tables_init();
>
> +       /*
> +        * Check for chimp handshake status.
> +        * Zero timeout value will actually fall to default timeout.
> +        */
> +       chimp_handshake_status_optee(0, &chimp_hs);
> +       if (chimp_hs == CHIMP_HANDSHAKE_SUCCESS)
> +               printf("ChiMP handshake successful\n");
> +       else
> +               printf("ERROR: ChiMP handshake status 0x%x\n", chimp_hs);

Where is this error handled? Don't you need to return the error from
this function?

> +
>         return mem_info_parse_fixup(fdt);
>  }
>  #endif /* CONFIG_OF_BOARD_SETUP */
> --
> 2.17.1
>

Regards,
Simon
Rayagonda Kokatanur Aug. 24, 2020, 4:35 p.m. UTC | #2
Hi Simon,

On Sat, Aug 22, 2020 at 8:39 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Rayagonda,
>
> On Thu, 20 Aug 2020 at 11:42, Rayagonda Kokatanur
> <rayagonda.kokatanur@broadcom.com> wrote:
> >
> > Chimp is a core in Broadcom netxtream controller (bnxt).
> > Add support to check bnxt's chimp component status.
> >
> > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> > ---
> >  board/broadcom/bcmns3/ns3.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c
> > index 0357cd0e32..8540c99286 100644
> > --- a/board/broadcom/bcmns3/ns3.c
> > +++ b/board/broadcom/bcmns3/ns3.c
> > @@ -12,6 +12,7 @@
> >  #include <asm/armv8/mmu.h>
> >  #include <asm/arch-bcmns3/bl33_info.h>
> >  #include <dt-bindings/memory/bcm-ns3-mc.h>
> > +#include <broadcom/chimp.h>
> >
> >  /* Default reset-level = 3 and strap-val = 0 */
> >  #define L3_RESET       30
> > @@ -210,8 +211,20 @@ void reset_cpu(ulong level)
> >  #ifdef CONFIG_OF_BOARD_SETUP
> >  int ft_board_setup(void *fdt, struct bd_info *bd)
> >  {
> > +       u32 chimp_hs = CHIMP_HANDSHAKE_WAIT_TIMEOUT;
> > +
> >         gic_lpi_tables_init();
> >
> > +       /*
> > +        * Check for chimp handshake status.
> > +        * Zero timeout value will actually fall to default timeout.
> > +        */
> > +       chimp_handshake_status_optee(0, &chimp_hs);
> > +       if (chimp_hs == CHIMP_HANDSHAKE_SUCCESS)
> > +               printf("ChiMP handshake successful\n");
> > +       else
> > +               printf("ERROR: ChiMP handshake status 0x%x\n", chimp_hs);
>
> Where is this error handled? Don't you need to return the error from
> this function?

System boot is independent of chimp hand shake.
Chimp/bnxt handshake failure is not a catastrophic error.
We just want to warn the user that the chimp handshake has failed.

Best regards,
Rayagonda
>
> > +
> >         return mem_info_parse_fixup(fdt);
> >  }
> >  #endif /* CONFIG_OF_BOARD_SETUP */
> > --
> > 2.17.1
> >
>
> Regards,
> Simon
Simon Glass Aug. 25, 2020, 4:58 p.m. UTC | #3
Hi Rayagonda,

On Mon, 24 Aug 2020 at 10:35, Rayagonda Kokatanur
<rayagonda.kokatanur@broadcom.com> wrote:
>
> Hi Simon,
>
> On Sat, Aug 22, 2020 at 8:39 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Rayagonda,
> >
> > On Thu, 20 Aug 2020 at 11:42, Rayagonda Kokatanur
> > <rayagonda.kokatanur@broadcom.com> wrote:
> > >
> > > Chimp is a core in Broadcom netxtream controller (bnxt).
> > > Add support to check bnxt's chimp component status.
> > >
> > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> > > ---
> > >  board/broadcom/bcmns3/ns3.c | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > >
> > > diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c
> > > index 0357cd0e32..8540c99286 100644
> > > --- a/board/broadcom/bcmns3/ns3.c
> > > +++ b/board/broadcom/bcmns3/ns3.c
> > > @@ -12,6 +12,7 @@
> > >  #include <asm/armv8/mmu.h>
> > >  #include <asm/arch-bcmns3/bl33_info.h>
> > >  #include <dt-bindings/memory/bcm-ns3-mc.h>
> > > +#include <broadcom/chimp.h>
> > >
> > >  /* Default reset-level = 3 and strap-val = 0 */
> > >  #define L3_RESET       30
> > > @@ -210,8 +211,20 @@ void reset_cpu(ulong level)
> > >  #ifdef CONFIG_OF_BOARD_SETUP
> > >  int ft_board_setup(void *fdt, struct bd_info *bd)
> > >  {
> > > +       u32 chimp_hs = CHIMP_HANDSHAKE_WAIT_TIMEOUT;
> > > +
> > >         gic_lpi_tables_init();
> > >
> > > +       /*
> > > +        * Check for chimp handshake status.
> > > +        * Zero timeout value will actually fall to default timeout.
> > > +        */
> > > +       chimp_handshake_status_optee(0, &chimp_hs);
> > > +       if (chimp_hs == CHIMP_HANDSHAKE_SUCCESS)
> > > +               printf("ChiMP handshake successful\n");
> > > +       else
> > > +               printf("ERROR: ChiMP handshake status 0x%x\n", chimp_hs);
> >
> > Where is this error handled? Don't you need to return the error from
> > this function?
>
> System boot is independent of chimp hand shake.
> Chimp/bnxt handshake failure is not a catastrophic error.
> We just want to warn the user that the chimp handshake has failed.

OK then I really think you need some comments here in the code. E.g.
if this fails it means x which is not critical because y...

Regards,
SImon
diff mbox series

Patch

diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c
index 0357cd0e32..8540c99286 100644
--- a/board/broadcom/bcmns3/ns3.c
+++ b/board/broadcom/bcmns3/ns3.c
@@ -12,6 +12,7 @@ 
 #include <asm/armv8/mmu.h>
 #include <asm/arch-bcmns3/bl33_info.h>
 #include <dt-bindings/memory/bcm-ns3-mc.h>
+#include <broadcom/chimp.h>
 
 /* Default reset-level = 3 and strap-val = 0 */
 #define L3_RESET	30
@@ -210,8 +211,20 @@  void reset_cpu(ulong level)
 #ifdef CONFIG_OF_BOARD_SETUP
 int ft_board_setup(void *fdt, struct bd_info *bd)
 {
+	u32 chimp_hs = CHIMP_HANDSHAKE_WAIT_TIMEOUT;
+
 	gic_lpi_tables_init();
 
+	/*
+	 * Check for chimp handshake status.
+	 * Zero timeout value will actually fall to default timeout.
+	 */
+	chimp_handshake_status_optee(0, &chimp_hs);
+	if (chimp_hs == CHIMP_HANDSHAKE_SUCCESS)
+		printf("ChiMP handshake successful\n");
+	else
+		printf("ERROR: ChiMP handshake status 0x%x\n", chimp_hs);
+
 	return mem_info_parse_fixup(fdt);
 }
 #endif /* CONFIG_OF_BOARD_SETUP */