[RFC] Changes to clean and speed up integration with other systems

[nandojve]

Is your enhancement proposal related to a problem? Please describe. I start to integrate bl_mcu_sdk with Zephyr RTOS. I noted that there are duplicated code between devices and would like propose changes that may help to easy integrate with another systems, like Zephyr RTOS.

My steps can be checked at:

• bl_mcu_sdk as zephyr module
  • WIP to allow use vendor peripheral drivers
• zephyr port
  • WIP port

Zephyr RTOS currently is a Work In Process (WIP) and only have a hello world.

Describe the solution you'd like

• At drivers, move std_drv outside SoC driver and unify as a common base. Why? To avoid duplicated code since IPs are same.

from

├── bl602_driver
│   └── std_drv
│       ├── inc
│       └── src
└── bl702_driver
    └── std_drv
        ├── inc
        └── src

to

├── bl602_driver
├── bl702_driver
└── std_drv
    ├── inc
    └── src

• Change API signature to use register address instead instance index. Why? This will keep code clean and will drop a table at low level driver. In fact, BL already uses register address but it is necessary get it from a table. You can continue to use this at hal_drv to have better compatibility. This will avoid unnecessary overhead low level driver. examples:

from

- uint32_t UART_GetBaudrate(UART_ID_Type uartId)
{
    ...
    uint32_t UARTx = uartAddr[uartId];
    ...
    tmpVal = BL_RD_REG(UARTx, UART_BIT_PRD);
    ...
}

to

- uint32_t UART_GetBaudrate(uint32_t UARTx)
{
    ...
    tmpVal = BL_RD_REG(UARTx, UART_BIT_PRD);
    ...
}

• Refactor BL_Err_Type to int and use errno.h definitions, see errno-base.h. Why? This will generalize the functions and allow better portability.

from

BL_Err_Type UART_SendDataBlock(UART_ID_Type uartId, uint8_t *data, uint32_t len)
{
    ...
                return TIMEOUT;
    ...

    return SUCCESS;
}

to

int UART_SendDataBlock(uint32_t UARTx, uint8_t *data, uint32_t len)
{
    ...
                return -EAGAIN;
    ...

    return 0;
}

• Refactor all public functions to return errno value or use bool or even void . Why? Portability.

from

BL_Sts_Type UART_GetIntStatus(UART_ID_Type uartId, UART_INT_Type intType)
BL_Err_Type UART_AutoBaudDetection(UART_ID_Type uartId, BL_Fun_Type autoBaud)
BL_Err_Type UART_IntMask(UART_ID_Type uartId, UART_INT_Type intType, BL_Mask_Type intMask)

to

bool UART_GetIntStatus(uint32_t UARTx, UART_INT_Type intType)
bool UART_AutoBaudDetection(uint32_t UARTx, bool autoBaud)
bool UART_IntMask(uint32_t UARTx, UART_INT_Type intType, bool intMask)

• Drop interrupt register handlers. Why? Each RTOS probably will handle it differently. On a low level driver Bouffalo register interrupts from another code base. In fact, there are entries at uart_open, which corroborate with this change.

from

BL_Err_Type UART_Init(UART_ID_Type uartId, UART_CFG_Type *uartCfg)
{
...
    Interrupt_Handler_Register(UART0_IRQn, UART0_IRQHandler);
    Interrupt_Handler_Register(UART1_IRQn, UART1_IRQHandler);
...
    return SUCCESS;
}

to

int UART_Init(uint32_t UARTx, UART_CFG_Type *uartCfg)
{
...
    return 0;
}

• Refactor all #include directives from c header files to c source files. This is a good practice to clean up references and allow better portability and avoid include problems. This is best way to have a clean header to be used on any place.

from

#ifndef __BL602_UART_H__
#define __BL602_UART_H__

#include "uart_reg.h"
#include "bl602_common.h"
    ...
#endif /* __BL602_UART_H__ */

to

// bl602_uart.h
#ifndef __BL602_UART_H__
#define __BL602_UART_H__
    ...
#endif /* __BL602_UART_H__ */

// bl602_uart.c
...
#include <errno.h>
#include <stdint.h>
#include <stdbool.h>
#include "uart_reg.h"
#include "bl602_uart.h"
#include "bl602_glb.h"
...

Describe alternatives you've considered This changes are not a block to port code. However, changes will clean up, integrate and improve portability. In other words, it will help to integrate faster. The other alternative is write native driver code to improve performance.

Additional context All this changes can be implemented in parallel and allow deprecate <mcu>_driver/std_drv without break code. People can be encouraged to move to new model until n releases, which will drop <mcu>_driver/std_drv .

[sakumisue]

IPS are the same ,but perpherial reg bases and designs are not the same.and it is not convenient to manage.Your codes i think are not belong to std but hal layer.

[sakumisue]

And,the actual application will not use std , hal drv will be better. Std codes are more used for verification .So,std codes can not be dropped,and app demo will not use them,using hal drv instead.

[profound1987]

@nandojve Thanks very much for your attention to bl_mcu_sdk and your sincere advice. 1."At drivers, move std_drv outside SoC driver and unify as a common base." since more bl chips are on the way and some ips are different , for examples, we re-design PWM to support BLDC driver in our next generation soc., we improve UART and more feature is added, so maybe using different c code is easier to maintain. 2.“Change API signature to use register address instead instance index.” It's a good idea to make low level driver clean and simple, at the begaining, actually we only have app level and std level, we will take your advice into consideration. 3. "Refactor BL_Err_Type to int and use errno.h definitions, see errno-base.h", yes,it will make std driver better portability, when we wrote std driver ,what we think first is writting "bl style" since low driver is for soc and hal driver for application, we would be honoured if you can pay some attention to hal driver. 4. "Refactor all #include directives from c header files to c source files. " we will check if the header need include other header file due to struct,enum,etc. In a word, thank you again, we will make some changes according to your advice, and hope you enjoy it.

[nandojve]

@nandojve Thanks very much for your attention to bl_mcu_sdk and your sincere advice.

Thank you Bouffalo Lab to care and listen community.

1."At drivers, move std_drv outside SoC driver and unify as a common base." since more bl chips are on the way and some ips are different , for examples, we re-design PWM to support BLDC driver in our next generation soc., we improve UART and more feature is added, so maybe using different c code is easier to maintain.

If drivers use same registers base, you can add API by features or version.

#ifdef HAS_BLDC_FEATURE /* Add PWM BLCD API */ #endif

if it is possible, at least consider to have c headers files on a common place. Because independent of the chip, you can define and share same API description.

2.“Change API signature to use register address instead instance index.” It's a good idea to make low level driver clean and simple, at the begaining, actually we only have app level and std level, we will take your advice into consideration.

Nice!

3. "Refactor BL_Err_Type to int and use errno.h definitions, see errno-base.h", yes,it will make std driver better portability, when we wrote std driver ,what we think first is writting "bl style" since low driver is for soc and hal driver for application, we would be honoured if you can pay some attention to hal driver.

My use case only uses the minimal. I'm heavy focused on use register definition and minimal driver API. The up layer uses RTOS API and it can be prohibitive to use your hal, unfortunately.

You can see a simple example at uart_bl.c. This implements minimal driver for ZephyrRTOS, see uart API.

My intention is write only one driver for RTOS instead have many versions. Because of this [1] is so important and help us to write only 1 time one driver for all SoC series. The peripheral is defined at device tree, see bl6.dtsi for reference.

4. "Refactor all #include directives from c header files to c source files. " we will check if the header need include other header file due to struct,enum,etc.

If you have any doubts about this, no c header files requires includes to work, even for structs and enum definitions. You only need add include dependencies at c source files before you api header definition. This is not necessarily a easy to implement clean code rules. You can easily test creating a simple header and source files and try build.

In a word, thank you again, we will make some changes according to your advice, and hope you enjoy it.

Thank you for considering, happy to contribute somehow.