Hi Neha,
On 11:19-20250610, Neha Malcom Francis wrote:
> On 09/06/25 17:56, Manorit Chawdhry wrote:
> > Hi Neha,
> >
> > On 15:57-20250609, Neha Malcom Francis wrote:
> >> Trigger all tests of PBIST and LBIST using appropriate calls to set the
> >> core under test (MAIN R5 2_0) to it's required state.
> >>
> >> Signed-off-by: Neha Malcom Francis <n-fran...@ti.com>
> >> ---
> >> No change since v3
> >>
> >> arch/arm/mach-k3/j784s4/j784s4_init.c | 52 +++++++++++++++++++++++++++
> >> 1 file changed, 52 insertions(+)
> >>
> >> diff --git a/arch/arm/mach-k3/j784s4/j784s4_init.c
> >> b/arch/arm/mach-k3/j784s4/j784s4_init.c
> >> index 787cf6261e4..6e21d057fb8 100644
> >> --- a/arch/arm/mach-k3/j784s4/j784s4_init.c
> >> +++ b/arch/arm/mach-k3/j784s4/j784s4_init.c
> >> @@ -17,6 +17,7 @@
> >> #include <dm/pinctrl.h>
> >> #include <mmc.h>
> >> #include <remoteproc.h>
> >> +#include <k3_bist.h>
> >>
> >> #include "../sysfw-loader.h"
> >> #include "../common.h"
> >> @@ -122,6 +123,48 @@ static void setup_navss_nb(void)
> >> writel(NB_THREADMAP_BIT2, (uintptr_t)NAVSS0_NBSS_NB1_CFG_NB_THREADMAP);
> >> }
> >>
> >> +/* Execute and check results of BIST executed on MCU1_x and MCU4_O */
> >> +static void run_bist_j784s4(struct udevice *dev)
> >> +{
> >> + struct bist_ops *ops;
> >> + struct ti_sci_handle *handle;
> >> + int ret;
> >> +
> >> + ops = (struct bist_ops *)device_get_ops(dev);
> >> + handle = get_ti_sci_handle();
> >> +
> >> + /* get status of HW POST PBIST on MCU1_x */
> >> + if (ops->run_pbist_post())
> >> + panic("HW POST LBIST on MCU1_x failed\n");
> >> +
> >> + /* trigger PBIST tests on MCU4_0 */
> >> + ret = prepare_pbist(handle);
> >> + ret |= ops->run_pbist_neg();
> >> + ret |= deprepare_pbist(handle);
> >> +
> >> + ret |= prepare_pbist(handle);
> >> + ret |= ops->run_pbist();
> >> + ret |= deprepare_pbist(handle);
> >> +
> >> + ret |= prepare_pbist(handle);
> >> + ret |= ops->run_pbist_rom();
> >> + ret |= deprepare_pbist(handle);
> >> +
> >> + if (ret)
> >> + panic("PBIST on MCU4_0 failed: %d\n", ret);
> >> +
> >> + /* get status of HW POST PBIST on MCU1_x */
> >> + if (ops->run_lbist_post())
> >> + panic("HW POST LBIST on MCU1_x failed\n");
> >> +
> >> + /* trigger LBIST tests on MCU1_x */
> >> + ret = prepare_lbist(handle);
> >> + ret |= ops->run_lbist();
> >> + ret |= deprepare_lbist(handle);
> >> + if (ret)
> >> + panic("LBIST on MCU4_0 failed: %d\n", ret);
> >> +}
> >> +
> >> /*
> >> * This uninitialized global variable would normal end up in the .bss
> >> section,
> >> * but the .bss is cleared between writing and reading this variable, so
> >> move
> >> @@ -251,6 +294,15 @@ void board_init_f(ulong dummy)
> >> printf("AVS init failed: %d\n", ret);
> >> }
> >>
> >> + if (!IS_ENABLED(CONFIG_CPU_V7R) && IS_ENABLED(CONFIG_K3_BIST)) {
> >> + ret = uclass_get_device_by_driver(UCLASS_MISC,
> >> + DM_DRIVER_GET(k3_bist),
> >> + &dev);
> >> + if (ret)
> >> + panic("Failed to get BIST device: %d\n", ret);
> >
> > nit: Add "\n" here
>
> Didn't get you, there is already a "\n" here?
>
I meant an additional newline.
Before:
```
if (ret)
panic("Failed to get BIST device: %d\n", ret);
run_bist_j784s4(dev);
```
After:
```
if (ret)
panic("Failed to get BIST device: %d\n", ret);
<additional newline here>
run_bist_j784s4(dev);
```
Mostly after conditional statements I have seen newline and I feel it
make the code much more readable as well, wondering if it's an unsaid
convention or written somewhere else but noticed this off so commented.
> >
> >> + run_bist_j784s4(dev);
> >> + }
> >> +
> >> if (IS_ENABLED(CONFIG_CPU_V7R))
> >> setup_navss_nb();
> >>
> >> --
> >> 2.34.1
> >>
> >
> > Regards,
> > Manorit
>
> --
> Thanking You
> Neha Malcom Francis
Regards,
Manorit
