Question
Logic errors in stm32f7xx_ll_fmc.c FMC_NORSRAM_Init and FMC_NORSRAM_Timing_Init?
While debugging my FMC interface I came across what look like errors to me in logic / arithmetic in the stm32f7xx_ll_fmc file.
Here are the two functions of interest to me:
/**
* @brief Initialize the FMC_NORSRAM device according to the specified
* control parameters in the FMC_NORSRAM_InitTypeDef
* @param Device Pointer to NORSRAM device instance
* @param Init Pointer to NORSRAM Initialization structure
* @retval HAL status
*/
HAL_StatusTypeDef FMC_NORSRAM_Init(FMC_NORSRAM_TypeDef *Device, FMC_NORSRAM_InitTypeDef* Init)
{
uint32_t tmpr = 0;
/* Check the parameters */
assert_param(IS_FMC_NORSRAM_DEVICE(Device));
assert_param(IS_FMC_NORSRAM_BANK(Init->NSBank));
assert_param(IS_FMC_MUX(Init->DataAddressMux));
assert_param(IS_FMC_MEMORY(Init->MemoryType));
assert_param(IS_FMC_NORSRAM_MEMORY_WIDTH(Init->MemoryDataWidth));
assert_param(IS_FMC_BURSTMODE(Init->BurstAccessMode));
assert_param(IS_FMC_WAIT_POLARITY(Init->WaitSignalPolarity));
assert_param(IS_FMC_WAIT_SIGNAL_ACTIVE(Init->WaitSignalActive));
assert_param(IS_FMC_WRITE_OPERATION(Init->WriteOperation));
assert_param(IS_FMC_WAITE_SIGNAL(Init->WaitSignal));
assert_param(IS_FMC_EXTENDED_MODE(Init->ExtendedMode));
assert_param(IS_FMC_ASYNWAIT(Init->AsynchronousWait));
assert_param(IS_FMC_WRITE_BURST(Init->WriteBurst));
assert_param(IS_FMC_CONTINOUS_CLOCK(Init->ContinuousClock));
assert_param(IS_FMC_WRITE_FIFO(Init->WriteFifo));
assert_param(IS_FMC_PAGESIZE(Init->PageSize));
/* Get the BTCR register value */
tmpr = Device->BTCR[Init->NSBank];
/* Clear MBKEN, MUXEN, MTYP, MWID, FACCEN, BURSTEN, WAITPOL, WAITCFG, WREN,
WAITEN, EXTMOD, ASYNCWAIT, CBURSTRW and CCLKEN bits */
tmpr &= ((uint32_t)~(FMC_BCR1_MBKEN | FMC_BCR1_MUXEN | FMC_BCR1_MTYP | \
FMC_BCR1_MWID | FMC_BCR1_FACCEN | FMC_BCR1_BURSTEN | \
FMC_BCR1_WAITPOL | FMC_BCR1_CPSIZE | FMC_BCR1_WAITCFG | \
FMC_BCR1_WREN | FMC_BCR1_WAITEN | FMC_BCR1_EXTMOD | \
FMC_BCR1_ASYNCWAIT | FMC_BCR1_CBURSTRW | FMC_BCR1_CCLKEN | FMC_BCR1_WFDIS));
/* Set NORSRAM device control parameters */
tmpr |= (uint32_t)(Init->DataAddressMux |\
Init->MemoryType |\
Init->MemoryDataWidth |\
Init->BurstAccessMode |\
Init->WaitSignalPolarity |\
Init->WaitSignalActive |\
Init->WriteOperation |\
Init->WaitSignal |\
Init->ExtendedMode |\
Init->AsynchronousWait |\
Init->WriteBurst |\
Init->ContinuousClock |\
Init->PageSize |\
Init->WriteFifo);
if(Init->MemoryType == FMC_MEMORY_TYPE_NOR)
{
tmpr |= (uint32_t)FMC_NORSRAM_FLASH_ACCESS_ENABLE;
}
Device->BTCR[Init->NSBank] = tmpr;
/* Configure synchronous mode when Continuous clock is enabled for bank2..4 */
if((Init->ContinuousClock == FMC_CONTINUOUS_CLOCK_SYNC_ASYNC) && (Init->NSBank != FMC_NORSRAM_BANK1))
{
Device->BTCR[FMC_NORSRAM_BANK1] |= (uint32_t)(Init->ContinuousClock);
}
if(Init->NSBank != FMC_NORSRAM_BANK1)
{
Device->BTCR[FMC_NORSRAM_BANK1] |= (uint32_t)(Init->WriteFifo);
}
return HAL_OK;
}
/**
* @brief Initialize the FMC_NORSRAM Timing according to the specified
* parameters in the FMC_NORSRAM_TimingTypeDef
* @param Device Pointer to NORSRAM device instance
* @param Timing Pointer to NORSRAM Timing structure
* @param Bank NORSRAM bank number
* @retval HAL status
*/
HAL_StatusTypeDef FMC_NORSRAM_Timing_Init(FMC_NORSRAM_TypeDef *Device, FMC_NORSRAM_TimingTypeDef *Timing, uint32_t Bank)
{
uint32_t tmpr = 0;
/* Check the parameters */
assert_param(IS_FMC_NORSRAM_DEVICE(Device));
assert_param(IS_FMC_ADDRESS_SETUP_TIME(Timing->AddressSetupTime));
assert_param(IS_FMC_ADDRESS_HOLD_TIME(Timing->AddressHoldTime));
assert_param(IS_FMC_DATASETUP_TIME(Timing->DataSetupTime));
assert_param(IS_FMC_TURNAROUND_TIME(Timing->BusTurnAroundDuration));
assert_param(IS_FMC_CLK_DIV(Timing->CLKDivision));
assert_param(IS_FMC_DATA_LATENCY(Timing->DataLatency));
assert_param(IS_FMC_ACCESS_MODE(Timing->AccessMode));
assert_param(IS_FMC_NORSRAM_BANK(Bank));
/* Get the BTCR register value */
tmpr = Device->BTCR[Bank + 1];
/* Clear ADDSET, ADDHLD, DATAST, BUSTURN, CLKDIV, DATLAT and ACCMOD bits */
tmpr &= ((uint32_t)~(FMC_BTR1_ADDSET | FMC_BTR1_ADDHLD | FMC_BTR1_DATAST | \
FMC_BTR1_BUSTURN | FMC_BTR1_CLKDIV | FMC_BTR1_DATLAT | \
FMC_BTR1_ACCMOD));
/* Set FMC_NORSRAM device timing parameters */
tmpr |= (uint32_t)(Timing->AddressSetupTime |\
((Timing->AddressHoldTime) << 4) |\
((Timing->DataSetupTime) << 8) |\
((Timing->BusTurnAroundDuration) << 16) |\
(((Timing->CLKDivision)-1) << 20) |\
(((Timing->DataLatency)-2) << 24) |\
(Timing->AccessMode)
);
Device->BTCR[Bank + 1] = tmpr;
/* Configure Clock division value (in NORSRAM bank 1) when continuous clock is enabled */
if(HAL_IS_BIT_SET(Device->BTCR[FMC_NORSRAM_BANK1], FMC_BCR1_CCLKEN))
{
tmpr = (uint32_t)(Device->BTCR[FMC_NORSRAM_BANK1 + 1] & ~(((uint32_t)0x0F) << 20));
tmpr |= (uint32_t)(((Timing->CLKDivision)-1) << 20);
Device->BTCR[FMC_NORSRAM_BANK1 + 1] = tmpr;
}
return HAL_OK;
}The lines of concern for me are lines 42 and 109.
At line 109, each field of the FMC_NORSRAM_TimingTypeDef struct is shifted appropriately for the corresponding FMC_BTRx register parameters (AddressSetupTime is FMC_BTRx[7:4], DataSetupTime is FMC_BTRx[15:8], etc).
But AccessMode is BTRx[29:28], and in the above code AccessMode is not bit-shifted at all, it is just bitwise OR'd with all the other fields from position 0?
And at line 42, none of the fields of the FMC_NORSRAM_TypeDef struct are bit-shifted!
I am pretty sure either both of these or at least one of these instances is a bug, but maybe I am missing something about the implementation? My code runs fine with the above as-is, but that's because almost all of the fields for my particular case are "don't care"s. I have no idea how this file holds for more nuanced interfaces.