cancel
Showing results for 
Search instead for 
Did you mean: 

Error in stm32_tamp.c driver

AGamb.4
Associate III

Hello,

I just wanted to let you know there is an error in the tamper driver written for Tamper configuration in TF-A fork repository.

When enabling an external tamper in passive mode, the configuration is writting the passive mode in the wrong register, disabling the external tamper. This should be written in the atcr1 register but it is being written in the cr1 register.

if ((tamp_ext->mode & TAMP_ACTIVE) == TAMP_ACTIVE)

{

SETBITS(*cr1atcr1, _TAMP_ATCR1_ETAMPAM(id));

}

else

{

CLRBITS(*cr1atcr1, _TAMP_ATCR1_ETAMPAM(id));

}

PS: I am not sure this is the right place to put this kind of messages, where should I contact you if I find any other error/bug?

Best regards

1 ACCEPTED SOLUTION

Accepted Solutions
Bernard PUEL
ST Employee

Correction will be included in next releases onwards (v3.1.1, w23 and V4.0.0, w25).

Thanks again for your contribution.

View solution in original post

3 REPLIES 3
AGamb.4
Associate III

In the same driver I saw something that I do not understand and might be wrong too.

When the masking configuration is being set for the external tampers both branches TAMP_EVT_MASK mode active and not active are applying the same configuration (see code below). I think the correct code might be as follows:

if (id < _TAMP_CR2_ETAMPMSK_MAX_ID) {

/*

* Only external TAMP 1, 2 and 3 can be masked

*/

if ((tamp_ext->mode & TAMP_EVT_MASK) == TAMP_EVT_MASK) {

/*

* ETAMP(id) event generates a trigger event. This ETAMP(id) is masked

* and internally cleared by hardware. The backup registers are not erased.

*/

CLRBITS(*ier, _TAMP_IER_ETAMP(id));

CLRBITSSETBITS(*cr2, _TAMP_CR2_ETAMPMSK(id));

} else {

/*

* normal ETAMP interrupt:

* ETAMP(id) event generates a trigger event and TAMP(id)F must be cleared

* by software to * allow next tamper event detection.

*/

CLRBITS(*cr2, _TAMP_CR2_ETAMPMSK(id));

CLRBITSSETBITS(*ier, _TAMP_IER_ETAMP(id));

}

} else {

/* other than 1,2,3 external TAMP, we want its interruption */ Wrong comment?

CLRBITS(*ier, _TAMP_IER_ETAMP(id)); This is disabling the interrupt, correct?

}

Best regards

Bernard PUEL
ST Employee

Thanks for your contribution. We (ST) are looking at it.

To contribute, you can also use a pull request directly in Github (after validation of the CLA).

Bernard PUEL
ST Employee

Correction will be included in next releases onwards (v3.1.1, w23 and V4.0.0, w25).

Thanks again for your contribution.