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?

[_richard]

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?

[TLLAM.1]

Hi,

Thanks for reaching our ST community.

The purpose of this code is to show you the features of the ST EEPROM products with :

• Software examples for standard EE (I²C and SPI) and Page EEPROM.
• Sample implementations of the drivers running on the X-NUCLEO-EEPRMA1, X-NUCLEO-EEPRMA2, and X-NUCLEO-PGEEZ1 expansion boards
• STM32Cube portability for STM32 microcontrollers

Because of the flexibility it can offer, the code might be complex to read with different level of abstraction.

This code is dedicated for ST EEPROM only, the software we propose for other ST products should be different.

What is the final usage of the code you want to achieve ? Do you want to build an application ? Which products you aim to use ?

Regarding your specific questions about the code, do you want me to involve my software teammates to get all the information you need ?

In the mean time do not hesitate to have a look to the X-CUBE-EEPRMA1 user manual :

https://www.st.com/resource/en/user_manual/um2481-getting-started-with-the-xcubeeeprma1-software-expansion-for-stm32cube-stmicroelectronics.pdf

Thanks,

Best regards,

Thomas

[_richard]

Hi Thomas

reading and writing the eeproms is in context of a tracker / data collection custom board based on s2-lp and L0 nucleo boards.

For that reading / writing to the eeproms on the nucleo sigfox boards is one step.

i think the demo software should build upon reusable ll / hal drivers for the eeproms. then the more complex demo (several eeproms, different possible jumper settings) could show how the reusable ll / hal drivers could work for one or more eeproms in different scenarios.

with the current code in X-CUBE-EEPRMA1 it is easier to write the eeprom access from scratch what i did now, after spending some time on looking how to reuse the code in X-CUBE-EEPRMA1

[TLLAM.1]

Hi Richard,

Thanks for your reply. I'm going to pass on your feedbacks to the software development team.

Thanks,

Best regards,

Thomas