RM48L952: SafeTI: Flash error forcing tests does not generate errors nor return settings

Managed to get FLASH_ADDRESS_PARITY_FAULT_INJECT test run after I first fixed actual test for it
e2e.ti.com/.../602737

This required to modify sl_esm.c, this FPAROVR could have been done also in esm-callback, but since it is the SafeTI tests which manipulates the value it should also manipulate it back. Also when diagnostics key is removed here the SR[1]bit does not pop up again, like mentioned in the comments.
case ESM_G2ERR_FMC_UNCORR:
if (TRUE == SL_FLAG_GET(FLASH_ADDRESS_PARITY_SELF_TEST)) {
sl_flashWREG->FPAROVR = 0x5400U;
callbackCancelCount++;
cancelCallback = TRUE;
}

if (TRUE == SL_FLAG_GET(FLASH_ADDRESS_PARITY_FAULT_INJECT)) {
sl_flashWREG->FPAROVR = 0x5400U; // NOTE: JSI: without this errors is kept generating - if here manual CH ack for SR[1] is not needed
}


also this closing bracket should be before register backup restore so that settings are restored despite of was it fault inject test or not (in linked thread, FPAROVR are restored immediately after error generation before fault inject check so basically this change is cosmetic...
/* Anyways clear flash & ESM status registers */
sl_flashWREG->FEDACSTATUS = F021F_FEDACSTATUS_ADD_PAR_ERR;
sl_esmREG->SR1[1] = GET_ESM_BIT_NUM(ESM_G2ERR_FMC_UNCORR);
sl_esmREG->SSR2 = GET_ESM_BIT_NUM(ESM_G2ERR_FMC_UNCORR);
flashread = sl_flashWREG->FUNCERRADD;
}

/* Clear the diag mode settings */
sl_flashWREG->FPAROVR = regBkupFparOvr;
sl_flashWREG->FDIAGCTRL = regBckupFdiagctrl;

====================

Also noticed yet another documentation bug in TRM, in table 5-43 this is said for PAR_OVR_KEY
"When this value is “101”, the selected ADD_INV_PAR and DAT_INV_PAR fields will become active."

Does it mean that enabling the key does not have any other effect, of course not since what is said in PAR_OVR_KEY is immediately undoed in BNK_INV_PAR, since for that bit it is said
"When this value is one and PAR_OVR_KEY is ‘101’ then the current system parity signal SYS_ODD_PARITY is inverted when doing bank parity calculations."

So looks like that PAR_OVR_KEY activates all 3 (ADD_INV_PAR, DAT_INV_PAR and BNK_INV_PAR) not 2 as said in field description...
=====================

Also noticed that FLASH_ADDRESS_ECC_SELF_TEST is never set active, during that test the FLASH_ECC_TEST_MODE_2BIT is marked active instead :).

Here is the SafeTI code, and as can be seen in RM48 branch is entered in case test is not fault injection and inside that branch (void)SL_FLAG_SET(FLASH_ECC_TEST_MODE_2BIT); is called despite of the test...

/*flag is set to indicate the current test which is ongoing and
These flags are used in the sl_esm.c so as to mask the esm callback*/
#if defined(_TMS570LS31x_) || defined(_TMS570LS12x_) || defined(_RM48x_) || defined(_RM46x_) || defined(_RM42x_) || defined(_TMS570LS04x_)
if ((FLASH_ECC_TEST_MODE_2BIT_FAULT_INJECT != testType)&&(FLASH_ADDRESS_ECC_FAULT_INJECT != testType))
#endif
#if defined(_TMS570LC43x_) || defined(_RM57Lx_)
if (FLASH_ECC_TEST_MODE_2BIT == testType)
#endif
{
#if defined(_TMS570LS31x_) || defined(_TMS570LS12x_) || defined(_RM48x_) || defined(_RM46x_) || defined(_RM42x_) || defined(_TMS570LS04x_)
(void)SL_FLAG_SET(FLASH_ECC_TEST_MODE_2BIT);
#endif


What does demo application? It never checks is FLASH_ADDRESS_ECC_SELF_TEST active it only checks that FLASH_ECC_TEST_MODE_2BIT, in case you fix that activity flag in SafeTI side this abort handler breaks immediately... This also does not work in case you add secondary application side tag that which test is currently running and you check SL_FLAG_GET against that in exception handler since wrong test is set active...

#if defined(_TMS570LS31x_) || defined(_TMS570LS12x_) || defined(_RM48x_) || defined(_RM46x_) || defined(_RM42x_) || defined(_TMS570LS04x_)
/* DAbort due to an Flash Wrapper test ? */
if (((TRUE == SL_FLAG_GET(FLASH_ECC_TEST_MODE_2BIT)))||(TRUE == SL_FLAG_GET(FLASH_ECC_ADDR_TAG_REG_MODE)))
{
#if !OPTIMISATION_ENABLED /*Required due to wrong behavior on optimisation*/
sl_flashWREG->FDIAGCTRL &= 0xFFF0FFF8; /* Clear Diagnostics enable key */
#endif

maskDAbort = TRUE;
}


===================

for other fault injection tests
FLASH_ECC_TEST_MODE_2BIT_FAULT_INJECT
FLASH_ADDRESS_ECC_FAULT_INJECT

this same if prevents fault injection ones to even generate the error as said in first post
#if defined(_TMS570LS31x_) || defined(_TMS570LS12x_) || defined(_RM48x_) || defined(_RM46x_) || defined(_RM42x_) || defined(_TMS570LS04x_)
if ((FLASH_ECC_TEST_MODE_2BIT_FAULT_INJECT != testType)&&(FLASH_ADDRESS_ECC_FAULT_INJECT != testType))
#endif
#if defined(_TMS570LC43x_) || defined(_RM57Lx_)
if (FLASH_ECC_TEST_MODE_2BIT == testType)
#endif
{


So looks like that before even thinking of running these 2 tests you immediately need to yet again modify SafeTI code so that it generates those errors. This means basically moving those if's much lower in the code

==================

There is also weird in demo application
#if OPTIMISATION_ENABLED == 0U /*Required due to wrong behavior on optimisation*/
sl_flashWREG->FDIAGCTRL &= 0xFFF0FFF8U; /* Clear Diagnostics enable key */
#endif

In other part of the code that OPTIMISATION_ENABLED effects only in case TI_COMPILER is used not for IAR, how then this error handling code part can behave according to that OPTIMISATION flag when using IAR since that flag does not have any effect to anything else, so basically based on code with IAR the system works despite of will the diagnostics key be cleared or not...

In tther part of demo application is said that on this particular case one can clear the key or let it be be like it is, crazy this shouldn't be optional choice other one is certainly much better than other and it should be selected...
/* Though it's not necessary, turn-off the diag mode */
sl_flashWREG->FDIAGCTRL &= 0xFFF0FFF8U; /* Clear Diagnostics enable key */


Shouldn't the diagnostics keys be removed ASAP in order to prevent error's popping like it was seen in FLASH_ADDRESS_PARITY_FAULT_INJECT test (first part of this message), if you remove similar keys in first FIQ handler the error does not raise itself again which would then need extra manual handling and possibly masking of some simultaneously occuring real errors...

=======

Current status after digging & fixing: these tests looks to work:
FLASH_ECC_TEST_MODE_1BIT
FLASH_ECC_TEST_MODE_2BIT
FLASH_ECC_ADDR_TAG_REG_MODE
FLASH_ADDRESS_ECC_SELF_TEST

FLASH_ADDRESS_PARITY_SELF_TEST
FLASH_ADDRESS_PARITY_FAULT_INJECT

But cannot run (maybe tomorrow after yet anothet modifications):
FLASH_ADDRESS_ECC_FAULT_INJECT
FLASH_ECC_TEST_MODE_2BIT_FAULT_INJECT

Got the rest 2 fault injection test to run, required just that if-sentence moving so that DIAG_TRIG and flag is set by test type not fixed FLASH_ECC_TEST_MODE_2BIT and address read causing the error is before fault-injection check (added also diag trig revert since actual test sets those again for second round...

/*flag is set to indicate the current test which is ongoing and
These flags are used in the sl_esm.c so as to mask the esm callback*/
#if defined(_TMS570LS31x_) || defined(_TMS570LS12x_) || defined(_RM48x_) || defined(_RM46x_) || defined(_RM42x_) || defined(_TMS570LS04x_)
(void)SL_FLAG_SET(testType);
#endif
#if defined(_TMS570LC43x_) || defined(_RM57Lx_)
/* Test must run from RAM - cannot execute the function SL_FLAG_SET
* from Flash, so copying the functionality inline. */
extern boolean sl_priv_flag_set[];
sl_priv_flag_set[testType-TESTTYPE_MIN] = TRUE;
#endif

#if FUNCTION_PROFILING_ENABLED
SL_Record_Errorcreationtick(testType);
#endif

sl_flashWREG->FDIAGCTRL |= F021F_FDIAGCTRL_DIAG_TRIG;

/*SAFETYMCUSW 58 S MR:14.3 <APPROVED> Comment_16*/
flashread = *(volatile uint32 *)flashBadECC2;

sl_flashWREG->FDIAGCTRL &= ~F021F_FDIAGCTRL_DIAG_TRIG; // remove immediately just in case

#if defined(_TMS570LS31x_) || defined(_TMS570LS12x_) || defined(_RM48x_) || defined(_RM46x_) || defined(_RM42x_) || defined(_TMS570LS04x_)
if ((FLASH_ECC_TEST_MODE_2BIT_FAULT_INJECT != testType)&&(FLASH_ADDRESS_ECC_FAULT_INJECT != testType))
#endif
#if defined(_TMS570LC43x_) || defined(_RM57Lx_)
if (FLASH_ECC_TEST_MODE_2BIT == testType)
#endif
{


And in the end of the case modify code so that flag is cleared by test type, not fixed FLASH_ECC_TEST_MODE_2BIT and move '}' so that register backup is restored also in case on fault injection

}
/* Clear the diag mode settings */
#if defined(_TMS570LS31x_) || defined(_TMS570LS12x_) || defined(_RM48x_) || defined(_RM46x_) || defined(_RM42x_) || defined(_TMS570LS04x_)
sl_flashWREG->FPAROVR = regBkupFparOvr;
#endif
sl_flashWREG->FDIAGCTRL = regBckupFdiagctrl;


/* Clear the flags which indicate tests ongoing*/
SL_FLAG_CLEAR(testType);

=================================
Thanks for support :)!
=================================

Also wondering why user manual says :
Note: For FLASH_ECC_TEST_MODE_2BIT_FAULT_INJECT the fault is injected in the subsequent read of the corrupted flash area by the application. The application must take care to restore the flash diagnostic control registers .

Since in original code the TRIG bit is not set there won't ever be any kind of error despite what and how much the application reads... Based on original code the FLASH_ECC_TEST_MODE_2BIT_FAULT_INJECT and FLASH_ADDRESS_ECC_FAULT_INJECT and exatcly identical except FPAROVR has different bit set up. Why only other test is mentioned in note???

Why in that one test application should restore the flash diagnostics control register, should it be restored in data abort handler? If yes, then why real tests doesn't clear it (in demo app nor that is required in user manual). Real tests re-enables that register which is some kind of an indication that it maybe should at some place always be turned off (in data abort handler?)

Here is the re-enabling
/* Repeat the test for a different address to ensure that the FCORERRADD register is updated correctly */
/*SAFETYMCUSW 58 S MR:14.3 <APPROVED> Comment_16*/
sl_flashWREG->FDIAGCTRL = fdiagCtrl;
sl_flashWREG->FDIAGCTRL |= F021F_FDIAGCTRL_DIAG_TRIG;
flashread = *(volatile uint32 *)flashBadECC1;


And here is demo app's exception hanlder (diag control register is tuned only if this conditional compile flag has certain value...
if (((TRUE == SL_FLAG_GET(FLASH_ECC_TEST_MODE_2BIT)))||(TRUE == SL_FLAG_GET(FLASH_ECC_ADDR_TAG_REG_MODE)))
{
#if !OPTIMISATION_ENABLED /*Required due to wrong behavior on optimisation*/
sl_flashWREG->FDIAGCTRL &= 0xFFF0FFF8; /* Clear Diagnostics enable key */
#endif

maskDAbort = TRUE;
}


Please educate me, since I must have obviously missed something (most likely critical)... I assume that this diagnostics control key should be always turned off (in data abort) just in case but it is not done in demo app...

=======

Now every flash test runs, down side was that I needed to modify several lines of SafeTI code (well wasn't first time)...

Implemented both register content checks ( FDIAGCTRL and FPAROVR) to all flash in data_abort handler. I think that these checks should be in place so that any aborts are not mistakenly masked out in case some real errors occur when only a part of the settings are done.

For same reason SafeTI should be tuned so that content of these registers are restore ASAP after the error generation line so that integrator can really be sure that time slot first mistakenly masked error is as small as possible.

Also maybe the register content should be manipulated in data-abort when it was noticed that the call most likely originated from SafeTI and masked away to prevent multiple calls to be masked out in case real errors happens before the content is restored in SafeTI. There is nothing mentioned about such behavior or need in user nor safety manual of SafeTI.

Based on demo application and SafeTI manual it looks like that there has not been think at all that actual errors may be masked away accidentally, that could for sure happen in demo application.

Actually this checking and modifying the register content ASAP should be offered by SafeTI code, not so that integrator needs to determine what these tests actually does...
e2e.ti.com/.../603064


Basically now checked that in FDIAGCTRL the 'mode' and 'key' are as expected (trig-bit has autoclear feature so cannot check that) and that in other tests than ADD_TAG_MODE also the FPAROVR has proper content (please note that FLASH_ADDRESS_PARITY_SELF_TEST test clears the content in FIQ handler manually before the "abort is mistakenly generated" so 0x5400 must be expected for that test when abort is entered (e2e.ti.com/.../602737))


=============================
Also noticed (again) some interesting naming of variables when seeking what is exactly set in to register when tests are made, why in the earth some has 'FDIAGCTRL' and some 'FDCTRL' in the name while all values are to be meant to be used into same field in register FDIAGCTRL :), reviews performed?
#define F021F_FDCTRL_DMODE_DATA_CORR (uint32)0x00000001U
#define F021F_FDIAGCTRL_DMODE_SYN_RPT (uint32)0x00000002U
/*macro names changed so as to adhere to misra rule MISRA-C:2004 5.1 */
#define F021F_FDIAGCTRL_DMODE_MALFUNC1 (uint32)0x00000003U
/*macro names changed so as to adhere to misra rule MISRA-C:2004 5.1 */
#define F021F_FDIAGCTRL_DMODE_MALFUNC2 (uint32)0x00000004U
#define F021F_FDIAGCTRL_DMODE_ADDR_TAG (uint32)0x00000005U
#define F021F_FDCTRL_DMODE_P_BUF_PAR (uint32)0x00000006U