is the code in https://github.com/STMicroelectronics/X-CUBE-EEPRMA1/tree/main/Documentation ment as demo code for the nucleo boards or as reference implementation for the eeprom components?
some question concerning the stm eeprom m95xx drivers from a not primarily firmware developer with limited experience with stm libraries.
i was looking for some m95xx spi drivers. one can write them completely by one self, but as a not primarily firmware developer i look if there is a established / best practice / common solution (as in many cases is) before reinventing the wheel.
What i found is the code in https://github.com/STMicroelectronics/X-CUBE-EEPRMA1, referenced from https://www.st.com/en/evaluation-tools/x-nucleo-eeprma2.html#tools-software , https://www.st.com/en/evaluation-tools/x-nucleo-eeprma1.html#tools-software and https://www.st.com/en/ecosystems/x-nucleo-pgeez1.html#tools-software -> the code in X-CUBE-EEPRMA1 covers all the boards, seems to be the reference code by stm.
The code in https://github.com/STMicroelectronics/X-CUBE-EEPRMA1/tree/main/Drivers/BSP/Components/M95xx should be fine for M95xx.
Having a look at https://github.com/STMicroelectronics/X-CUBE-EEPRMA1/blob/main/Drivers/BSP/Components/M95xx/m95xx.h
...
typedef struct
{
M95_Init_Func Init;
M95_DeInit_Func DeInit;
M95_Read_Func Read;
M95_Write_Func Write;
M95_Transmit_Func WriteBuffer;
M95_Receive_Func ReadBuffer;
M95_IsReady_Func IsReady;
M95_Delay Delay;
uint8_t Address;
} M95_IO_t;
typedef struct
{
M95_IO_t IO;
} M95_Object_t;
/**
* @brief EEPROMEX COMMON driver structure definition
*/
typedef struct
{
int32_t (*Init)( M95_Object_t *);
int32_t (*DeInit)( M95_Object_t *);
int32_t (*IsReady)(M95_Object_t *);
int32_t (*ReadRegister)( M95_Object_t *,uint8_t * );
int32_t (*WriteRegister)(M95_Object_t *, uint8_t );
int32_t (*ReadByte)( M95_Object_t *, uint8_t *, const uint32_t);
int32_t (*WriteByte)( M95_Object_t *, uint8_t * , const uint32_t);
int32_t (*ReadPage)(M95_Object_t *,uint8_t * , const uint32_t , const uint16_t );
int32_t (*WritePage)(M95_Object_t *, uint8_t * , const uint32_t , const uint16_t ,const uint16_t );
int32_t (*ReadData)( M95_Object_t *, uint8_t * , const uint32_t, const uint16_t);
int32_t (*WriteData)(M95_Object_t *,uint8_t *, const uint32_t, const uint16_t, const uint16_t );
int32_t (*WriteID)( M95_Object_t *,uint8_t *, const uint32_t ,const uint16_t , const uint16_t);
int32_t (*ReadID)( M95_Object_t *,uint8_t * , const uint32_t ,const uint16_t , const uint16_t );
int32_t (*LockID)(M95_Object_t *);
int32_t (*LockStatus)( M95_Object_t *, uint8_t * );
void *pData;
} M95_EEPROM_CommonDrv_t;
/**
* @brief M95 driver structure definition
*/
typedef struct
{
int32_t (*Init)( M95_Object_t * );
int32_t (*DeInit)( M95_Object_t * );
int32_t (*IsReady)(M95_Object_t *);
int32_t (*ReadRegister)( M95_Object_t *,uint8_t *);
int32_t (*WriteRegister)( M95_Object_t *,uint8_t );
int32_t (*ReadByte)(M95_Object_t *, uint8_t *, const uint32_t);
int32_t (*WriteByte)(M95_Object_t *, uint8_t * , const uint32_t);
int32_t (*ReadPage)(M95_Object_t *,uint8_t * , const uint32_t , const uint16_t );
int32_t (*WritePage)(M95_Object_t *, uint8_t * , const uint32_t , const uint16_t ,const uint16_t );
int32_t (*ReadData)( M95_Object_t *,uint8_t * , const uint32_t, const uint16_t);
int32_t (*WriteData)(M95_Object_t *,uint8_t * , const uint32_t, const uint16_t, const uint16_t );
int32_t (*WriteID)( M95_Object_t *,uint8_t *, const uint32_t ,const uint16_t , const uint16_t );
int32_t (*ReadID)( M95_Object_t *,uint8_t * , const uint32_t ,const uint16_t , const uint16_t );
int32_t (*LockID)(M95_Object_t *);
int32_t (*LockStatus)( M95_Object_t *,uint8_t * );
void *pData;
} M95_Drv_t;
there are are sevaral points that are unclear.
- why duplicate the code for M95_Drv_t and M95_EEPROM_CommonDrv_t, why have two different typedefs?
- looking at https://www.kernel.org/doc/html/latest/process/coding-style.html#typedefs
- Is using typedefs here for M95_Drv_t and M95_EEPROM_CommonDrv_t a good idea? Why?
- Is using typedefs here for M95_IO_t and M95_IO_t a good idea? Why?
With my limited experience I'm not starting a judgement of naming, but these two pair off essential content identical structs hidden after typedefs look like multiple persons worked and code and didn't care to refactor after copying it together. Am I right?
Anyway the M95_IO_t is mostly a subset of M95_Drv_t with diverging names and also an addition of an Address, what i think is strange for SPI.
In Drivers/BSP/Components/M95xx/m95xx.c one finds
...
int32_t M95_RegisterBusIO(M95_Object_t *pObj, M95_IO_t *pIO)
{
pObj->IO.Address = pIO->Address;
pObj->IO.Init = pIO->Init;
...
int32_t M95_spi_IsDeviceReady( M95_Object_t *pObj )
{
return pObj->IO.IsReady( pObj->IO.Address );
...
int32_t M95_spi_ReadReg( M95_Object_t *pObj, uint8_t * pData)
{
pObj->IO.IsReady( pObj->IO.Address );
...
Does the copying of code provide a rela advantage here?
Dois the the IO. abstration level provide an advantage here?
The code to initialize M95_IO_t/M95_Object_t is not in the component folder but in Drivers/BSP/EEPRMA1/eeprma1_m95.c , decoupled from the code in components, but connected to a specific board.
Thus the m95xx component code not really self contained.
Is there a advantag?
static int32_t M95M01_0_Probe(void)
{
M95_IO_t io_ctxm01;
int32_t ret = BSP_ERROR_NONE;
static M95_Object_t M95M01_obj_0;
io_ctxm01.Address = M95M01_SPI_ADDR;
io_ctxm01.Init = EEPRMA1_SPI_Init;
io_ctxm01.DeInit = EEPRMA1_SPI_DeInit;
io_ctxm01.Read = EEPRMA1_SPI_ReadReg;
io_ctxm01.Write = EEPRMA1_SPI_WriteReg;
io_ctxm01.WriteBuffer = EEPRMA1_SPI_SendBuffer;
io_ctxm01.ReadBuffer = EEPRMA1_SPI_RecvBuffer;
io_ctxm01.IsReady = EEPRMA1_M95_IsDeviceReady;
io_ctxm01.Delay = EEPRMA1_M95_Delay;
...
static int32_t M95256_0_Probe(void)
{
M95_IO_t io_ctx256;
int32_t ret = BSP_ERROR_NONE;
static M95_Object_t M95256_obj_0;
io_ctx256.Address = M95256_SPI_ADDR;
...
static int32_t M95040_0_Probe(void)
{
M95_IO_t io_ctx040;
int32_t ret = BSP_ERROR_NONE;
static M95_Object_t M95040_obj_0;
io_ctx040.Address = M95040_SPI_ADDR;
...
Naming a function, that initializes, as probe make it hard at least for me to read the reference code.
Besides that i have the impression that the copy and paste coding is not really maintainable, ... . Or am i wrong?
The code does note cover all stm m95 eeproms, but just the ones on the nucleo boards.
I started to look at the code as I need to access a m95640, currently on another nucleo board.
In Drivers/BSP/EEPRMA1/eeprma1_m95.c
...
int32_t EEPRMA1_M95_WriteCmd( uint8_t Cmd, uint8_t Devaddr)
{
int32_t status = BSP_ERROR_NONE;
EEPROMEX_CTRL_LOW( Devaddr & EEPROMEX_SPI_SLAVESEL ); /* For M95M04 0xCC & 0x03 = 0*/
status = EEPRMA1_SPI_Send( &Cmd, 1 );
EEPROMEX_CTRL_HIGH( Devaddr & EEPROMEX_SPI_SLAVESEL );
return status;
}
...
void EEPROMEX_CTRL_LOW(const uint8_t pin)
{
HAL_GPIO_WritePin(EEPROMEX_CtrlPin[pin].EEPROMEX_PIN_PORT,EEPROMEX_CtrlPin[pin].EEPROMEX_PIN,GPIO_PIN_RESET);
}
...the usage of the "Address" can be seen. I think that the WriteCmd based is related to the component and should be in the components code, or am i wrong.
I could go one longer about how hard
- this code is to read for me (naming, structuring, ...)
- it would be to use it as a basis for own code that covers e.g. the missing components and then keeping it updated based on updates in stm code.
E.g. https://github.com/nopnop2002/esp-idf-spi-eeprom/blob/main/components/spi-eeprom/spi-eeprom.c looks way clearer
The questions for me are:
- is the code ment as reference code/implementation or is it primarily ment as specific demo code for the specific boards?
- if this is ment as reference code - should i expect the same level of completeness / usability / maintainability for other stm components?