[PATCH 4/4] DONOTMERGE: arm: mach-k3: j784s4_init: Trigger LBIST and PBIST on MAIN R5 2_0

[Neha Malcom Francis]

On 28/11/24 09:38, Kumar, Udit wrote:
> Thanks Neha.
> 
> On 11/27/2024 8:23 PM, 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-francis at ti.com>
>> ---
>>   arch/arm/mach-k3/j784s4/j784s4_init.c | 48 +++++++++++++++++++++++++++
>>   1 file changed, 48 insertions(+)
>>
>> diff --git a/arch/arm/mach-k3/j784s4/j784s4_init.c 
>> b/arch/arm/mach-k3/j784s4/j784s4_init.c
>> index 07b5d7d7504..f9bd13d962b 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"
>> @@ -233,9 +234,56 @@ void k3_mem_init(void)
>>   void board_init_f(ulong dummy)
>>   {
>> +    struct udevice *dev;
>> +    int ret;
>> +
>>       k3_spl_init();
>>       k3_mem_init();
>> +    if (!IS_ENABLED(CONFIG_CPU_V7R) && IS_ENABLED(CONFIG_K3_BIST)) {
> 
> My suggestion will be , keep this patch to be merged and don't enable the BIST 
> config , patch 3/4 as default

Oh okay, can do that. My reasoning was this patch was to showcase how to run the 
calls on an example PBIST14 on the MAIN2_x cores.

> 
>> +        ret = uclass_get_device_by_driver(UCLASS_MISC,
>> +                          DM_DRIVER_GET(k3_bist),
>> +                          &dev);
>> +        if (ret)
>> +            printf("Failed to run BIST: %d\n", ret);
> 
> in whole of this function, please take care of error handling
> 
> eg if uclass_get_device_by_driver returns error then there is not point to get 
> device_get_ops or so

Okay since I'll not mark this as DONOTMERGE will make it a proper patch.

> 
>> +
>> +        struct bist_ops *ops = (struct bist_ops *)device_get_ops(dev);
>> +        struct ti_sci_handle *handle = get_ti_sci_handle();
>> +
>> +        /* get status of HW POST PBIST on MCU1_x */
>> +        ret = ops->run_pbist_post();
>> +        if (ret)
>> +            panic("HW POST LBIST on MCU1_x failed: %d\n", ret);
>> +
>> +        /* trigger PBIST tests on MCU4_0 */
>> +        prepare_pbist(handle);
>> +        ret = ops->run_pbist_neg();
>> +        deprepare_pbist(handle);
>> +
> 
> Can prepare_pbist and deprepare_pbist be static in driver.

Yes should be possible, will do that.
> 
> 
>> +        prepare_pbist(handle);
>> +        ret |= ops->run_pbist();
>> +        deprepare_pbist(handle);
>> +
>> +        prepare_pbist(handle);
>> +        ret |= ops->run_pbist_rom();
>> +        deprepare_pbist(handle);
>> +
>> +        if (ret)
>> +            panic("PBIST on MCU4_0 failed: %d\n", ret);
>> +
>> +        /* get status of HW POST PBIST on MCU1_x */
>> +        ret = ops->run_lbist_post();
>> +        if (ret)
>> +            panic("HW POST LBIST on MCU1_x failed: %d\n", ret);
>> +
>> +        /* trigger LBIST tests on MCU1_x */
>> +        prepare_lbist(handle);
>> +        ret = ops->run_lbist();
>> +        deprepare_lbist(handle);
>> +        if (ret)
>> +            panic("LBIST on MCU4_0 failed: %d\n", ret);
>> +    }
>> +
>>       if (IS_ENABLED(CONFIG_CPU_V7R))
>>           setup_navss_nb();

-- 
Thanking You
Neha Malcom Francis