Skip to content

Make BFLB not use the SDK code #91977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 26 additions & 13 deletions boards/aithinker/ai_wb2_12f/ai_wb2_12f.dts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

/dts-v1/;

#include <bflb/bl60x.dtsi>
#include <bflb/bl602.dtsi>
#include "ai_wb2_12f-pinctrl.dtsi"

/ {
Expand All @@ -15,6 +15,7 @@

chosen {
zephyr,flash = &flash0;
zephyr,code-partition = &slot0_partition;
zephyr,itcm = &itcm;
zephyr,dtcm = &dtcm;
zephyr,sram = &sram0;
Expand All @@ -27,18 +28,30 @@
clock-frequency = <DT_FREQ_M(192)>;
};

&spi1 {
#address-cells = <1>;
#size-cells = <0>;
reg = <0x4000b000 0x1000 0x23000000 0x400000>;

flash0: flash@0 {
compatible = "zb,25vq32", "jedec,spi-nor";
status = "disabled";
size = <DT_SIZE_M(128)>;
jedec-id = [5e 40 16];
reg = <0>;
spi-max-frequency = <DT_FREQ_M(133)>;
&flashctrl {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand your intention here. As far I know, Bouffalo uses a SPI bus to have access to flash memory. This means that we should keep that clear in the devicetree.

For instance, ST defines all the internal memory only for MCUboot and external NOR to application:

&flash0 {
partitions {
compatible = "fixed-partitions";
#address-cells = <1>;
#size-cells = <1>;
/* Set the partitions with first MB to make use of the whole Bank1 */
boot_partition: partition@0 {
label = "mcuboot";
reg = <0x00000000 DT_SIZE_K(64)>;
};
};
};
&xspi2 {
pinctrl-0 = <&xspim_p2_clk_pn6 &xspim_p2_ncs1_pn1
&xspim_p2_io0_pn2 &xspim_p2_io1_pn3
&xspim_p2_io2_pn4 &xspim_p2_io3_pn5
&xspim_p2_io4_pn8 &xspim_p2_io5_pn9
&xspim_p2_io6_pn10 &xspim_p2_io7_pn11
&xspim_p2_dqs0_pn0>;
pinctrl-names = "default";
status = "okay";
mx25uw25645: xspi-nor-flash@0 {
compatible = "st,stm32-xspi-nor";
reg = <0>;
size = <DT_SIZE_M(256)>; /* 256Mbits */
ospi-max-frequency = <DT_FREQ_M(50)>;
spi-bus-width = <XSPI_OCTO_MODE>;
data-rate = <XSPI_DTR_TRANSFER>;
four-byte-opcodes;
status = "okay";
partitions {
compatible = "fixed-partitions";
#address-cells = <1>;
#size-cells = <1>;
slot0_partition: partition@0 {
label = "image-0";
reg = <0x00000000 DT_SIZE_K(512)>;
};
slot1_partition: partition@80000 {
label = "image-1";
reg = <0x0080000 DT_SIZE_K(512)>;
};
scratch_partition: partition@100000 {
label = "image-scratch";
reg = <0x00100000 DT_SIZE_K(64)>;
};
storage_partition: partition@110000 {
label = "storage";
reg = <0x00110000 DT_SIZE_K(64)>;
};
};
};
};

Copy link
Collaborator Author

@VynDragon VynDragon Jul 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a dedicated flash controller that is not capable of doing general SPI, and that manages both internal OR external flash (or 'and' for bl616, but on a bank basis), transport is irrelevant and it cant be made to fit as anything purely SPI. ST example is completly irrelevant here, there is only one memory chip, either outside or inside, for bl60x.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm getting confuse because of this:

https://github.com/bouffalolab/bouffalo_sdk/blob/811e8c1f1cd62695cedd47367f0bf2c624707bc6/drivers/lhal/src/flash/bflb_sflash.c#L160-L178

and here there is an API to get the jedecid:
https://github.com/bouffalolab/bouffalo_sdk/blob/811e8c1f1cd62695cedd47367f0bf2c624707bc6/drivers/lhal/src/flash/bflb_xip_sflash.h#L81

So, the impression I have is that, this serial flash controller really rely on a bus like SPI or QSPI and it could maybe, autoconfigure using the JEDEC parameters. In some cases, we still need to define the memory parameters for some devices in Zephyr.

In this case, if this SFLASH CTRL uses a NOR memory it seems that we still need to define the phandle or the NOR memory definitions in the devicetree on my understanding. Then, inside of this NOR block we define the partitions.

At end, this SoC runs XIP from external NOR (or internal NOR). It seems to be a multi-chip module (MCM) or multi-chip package (MCP) which means that there is no real internal flash.

Copy link
Collaborator Author

@VynDragon VynDragon Jul 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could auto-configure from the jedec settings (this is what the flash tool does in fact, and it puts those parameters in the boot header, which is why we dont need to do it ourselves out of using bank2 on bl616), but there is really no direct relation between them and the jedec ID (it's a mix of timing settings and commands to use to drive the flash), they are only the ones associated with a specific flash type, so we would need to copy the data store somewhere for those settings.

Do you simply mean replacing soc-nv-flash with jedec-nor?

flash0: flash@23000000 {
compatible = "soc-nv-flash", "zb,25vq32";
reg = <0x23000000 (0x400000 - 0x2000)>;
write-block-size = <256>;
erase-block-size = <DT_SIZE_K(4)>;
/* jedec-id = [5e 40 16]; */

partitions {
compatible = "fixed-partitions";
#address-cells = <1>;
#size-cells = <1>;

slot0_partition: partition@0 {
label = "image-0";
reg = <0x00000000 0x00100000>;
read-only;
};

storage_partition: partition@100000 {
label = "storage";
reg = <0x00100000 (0x300000 - 0x2000)>;
};
};
};
};

Expand Down
11 changes: 5 additions & 6 deletions boards/aithinker/ai_wb2_12f/support/bl60x.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,22 @@ echo "Ready for Remote Connections"

$_TARGETNAME.0 configure -event reset-assert-pre {
echo "reset-assert-pre"
adapter speed 100
adapter speed 400
}

$_TARGETNAME.0 configure -event reset-deassert-post {
echo "reset-deassert-post"

adapter speed 100
adapter speed 400

reg mstatus 0x7800
reg mie 0x0
# reg pc 0x23000000
reg mstatus 0x0
reg pc 0x21000000
}

$_TARGETNAME.0 configure -event reset-init {
echo "reset-init"

adapter speed 3000
adapter speed 400
}

$_TARGETNAME.0 configure -event gdb-attach {
Expand Down
2 changes: 1 addition & 1 deletion boards/aithinker/ai_wb2_12f/support/openocd.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

interface cmsis-dap

adapter speed 1000
adapter speed 400
39 changes: 26 additions & 13 deletions boards/bflb/bl60x/bl604e_iot_dvk/bl604e_iot_dvk.dts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

/dts-v1/;

#include <bflb/bl60x.dtsi>
#include <bflb/bl604.dtsi>
#include "bl604e_iot_dvk-pinctrl.dtsi"

/ {
Expand All @@ -15,6 +15,7 @@

chosen {
zephyr,flash = &flash0;
zephyr,code-partition = &slot0_partition;
zephyr,itcm = &itcm;
zephyr,dtcm = &dtcm;
zephyr,sram = &sram0;
Expand All @@ -27,18 +28,30 @@
clock-frequency = <DT_FREQ_M(192)>;
};

&spi1 {
#address-cells = <1>;
#size-cells = <0>;
reg = <0x4000b000 0x1000 0x23000000 0xc00000>;

flash0: flash@0 {
compatible = "issi,is25lp128", "jedec,spi-nor";
status = "disabled";
size = <DT_SIZE_M(128)>;
jedec-id = [96 60 18];
reg = <0>;
spi-max-frequency = <DT_FREQ_M(133)>;
&flashctrl {
flash0: flash@23000000 {
compatible = "soc-nv-flash", "issi,is25lp128";
reg = <0x23000000 (0x1000000 - 0x2000)>;
write-block-size = <256>;
erase-block-size = <DT_SIZE_K(4)>;
/* jedec-id = [96 60 18]; */

partitions {
compatible = "fixed-partitions";
#address-cells = <1>;
#size-cells = <1>;

slot0_partition: partition@0 {
label = "image-0";
reg = <0x00000000 0x100000>;
read-only;
};

storage_partition: partition@100000 {
label = "storage";
reg = <0x00100000 (0xF00000 - 0x2000)>;
};
};
};
};

Expand Down
37 changes: 25 additions & 12 deletions boards/doiting/dt_bl10_devkit/dt_bl10_devkit.dts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

chosen {
zephyr,flash = &flash0;
zephyr,code-partition = &slot0_partition;
zephyr,itcm = &itcm;
zephyr,dtcm = &dtcm;
zephyr,sram = &sram0;
Expand All @@ -26,18 +27,30 @@
clock-frequency = <DT_FREQ_M(192)>;
};

&spi1 {
#address-cells = <1>;
#size-cells = <0>;
reg = <0x4000b000 0x1000 0x23000000 0xc00000>;

flash0: flash@0 {
compatible = "issi,is25lp128", "jedec,spi-nor";
status = "disabled";
size = <DT_SIZE_M(128)>;
jedec-id = [96 60 18];
reg = <0>;
spi-max-frequency = <DT_FREQ_M(133)>;
&flashctrl {
flash0: flash@23000000 {
compatible = "soc-nv-flash", "issi,is25lp128";
reg = <0x23000000 (0x1000000 - 0x2000)>;
write-block-size = <256>;
erase-block-size = <DT_SIZE_K(4)>;
/* jedec-id = [96 60 18]; */

partitions {
compatible = "fixed-partitions";
#address-cells = <1>;
#size-cells = <1>;

slot0_partition: partition@0 {
label = "image-0";
reg = <0x00000000 0x100000>;
read-only;
};

storage_partition: partition@100000 {
label = "storage";
reg = <0x00100000 (0xF00000 - 0x2000)>;
};
};
};
};

Expand Down
4 changes: 3 additions & 1 deletion drivers/clock_control/clock_control_bl60x.c
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ const static uint32_t clock_control_bl60x_crystal_SDMIN_table[5] = {

static inline void clock_control_bl60x_clock_settle(void)
{
__asm__ volatile(".rept 15 ; nop ; .endr");
__asm__ volatile(".rept 20 ; nop ; .endr");
}

/* 32 Mhz Oscillator: 0
Expand Down Expand Up @@ -848,6 +848,8 @@ static int clock_control_bl60x_init(const struct device *dev)

clock_control_bl60x_peripheral_clock_init();

clock_control_bl60x_clock_settle();

irq_unlock(key);

return 0;
Expand Down
6 changes: 5 additions & 1 deletion drivers/pinctrl/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ zephyr_library_sources_ifdef(CONFIG_PINCTRL_ARM_MPS2 pinctrl_arm_mps2.c)
zephyr_library_sources_ifdef(CONFIG_PINCTRL_ARM_MPS3 pinctrl_arm_mps3.c)
zephyr_library_sources_ifdef(CONFIG_PINCTRL_ARM_MPS4 pinctrl_arm_mps4.c)
zephyr_library_sources_ifdef(CONFIG_PINCTRL_ARM_V2M_BEETLE pinctrl_arm_v2m_beetle.c)
zephyr_library_sources_ifdef(CONFIG_PINCTRL_BFLB pinctrl_bflb.c)
zephyr_library_sources_ifdef(CONFIG_PINCTRL_GD32_AF pinctrl_gd32_af.c)
zephyr_library_sources_ifdef(CONFIG_PINCTRL_GD32_AFIO pinctrl_gd32_afio.c)
zephyr_library_sources_ifdef(CONFIG_PINCTRL_ITE_IT8XXX2 pinctrl_ite_it8xxx2.c)
Expand Down Expand Up @@ -58,3 +57,8 @@ zephyr_library_sources_ifdef(CONFIG_PINCTRL_WCH_20X_30X_AFIO pinctrl_wch_20x_30x
zephyr_library_sources_ifdef(CONFIG_PINCTRL_WCH_00X_AFIO pinctrl_wch_00x_afio.c)

add_subdirectory(renesas)

if (CONFIG_PINCTRL_BFLB)
zephyr_library_sources(pinctrl_bflb.c)
zephyr_library_sources_ifdef(CONFIG_SOC_SERIES_BL60X pinctrl_bflb_bl60x_70x.c)
endif()
41 changes: 18 additions & 23 deletions drivers/pinctrl/pinctrl_bflb.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,40 +6,35 @@

#include <zephyr/kernel.h>
#include <zephyr/drivers/pinctrl.h>
#include <bflb_pinctrl.h>
#include <bflb_glb.h>
#include <bflb_gpio.h>

/* clang-format off */
#include <zephyr/dt-bindings/pinctrl/bflb-common-pinctrl.h>

int pinctrl_configure_pins(const pinctrl_soc_pin_t *pins, uint8_t pin_cnt,
uintptr_t reg)
#if defined(CONFIG_SOC_SERIES_BL60X)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to have a separated file to handle each SoC or combine then when appropriate.
I really want to avoid preprocessor to select code. We can use build infrastructure to do that to us.

Copy link
Collaborator Author

@VynDragon VynDragon Jul 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what i wanted to do initially but I remember being told this wasn't the way.

Edit: need 2, one for bl70x and bl60x pinctrl, one for bl61x, same for GPIO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: need 2, one for bl70x and bl60x pinctrl, one for bl61x, same for GPIO.

No problem I think.

I would like to have a common pinctrl definition that we can use for all SoCs. Then we share the same pinctrl_configure_pins function and have a separated drivers/pinctrl/pinctrl_bflb_bl60x_70x.c with the pinctrl_configure_uart and pinctrl_init_pin in functions inside of it.

Then we do something like

if (CONFIG_PINCTRL_BFLB)
  zephyr_library_sources(pinctrl_bflb.c)
  zephyr_library_sources_ifdef(CONFIG_SOC_SERIES_BL60X pinctrl_bflb_bl60x_70x.c)
  zephyr_library_sources_ifdef(CONFIG_SOC_SERIES_BL70X pinctrl_bflb_bl60x_70x.c)
  zephyr_library_sources_ifdef(CONFIG_SOC_SERIES_BL80X pinctrl_bflb_bl80x.c)
  ...
endif()

This way we keep code very well organized I supposed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

#include <zephyr/dt-bindings/pinctrl/bl60x-pinctrl.h>
#else
#error "Unsupported Platform"
#endif

void pinctrl_bflb_configure_uart(uint8_t pin, uint8_t func);
void pinctrl_bflb_init_pin(pinctrl_soc_pin_t pin);

int pinctrl_configure_pins(const pinctrl_soc_pin_t *pins, uint8_t pin_cnt, uintptr_t reg)
{
GLB_GPIO_Cfg_Type pincfg;
uint8_t i;

ARG_UNUSED(reg);

for (i = 0U; i < pin_cnt; i++) {
pincfg.gpioFun = BFLB_PINMUX_GET_FUN(pins[i]);
pincfg.gpioMode = BFLB_PINMUX_GET_MODE(pins[i]);
pincfg.gpioPin = BFLB_PINMUX_GET_PIN(pins[i]);
pincfg.pullType = BFLB_PINMUX_GET_PULL_MODES(pins[i]);
pincfg.smtCtrl = BFLB_PINMUX_GET_SMT(pins[i]);
pincfg.drive = BFLB_PINMUX_GET_DRIVER_STRENGTH(pins[i]);

if (pincfg.gpioFun == BFLB_PINMUX_FUN_INST_uart0) {
GLB_UART_Fun_Sel(pincfg.gpioPin % 8,
(BFLB_PINMUX_GET_INST(pins[i]))
* 0x4U /* rts, cts, rx, tx */
+ BFLB_PINMUX_GET_SIGNAL(pins[i])
);

if ((BFLB_PINMUX_GET_FUN(pins[i]) & BFLB_PINMUX_FUN_MASK)
== BFLB_PINMUX_FUN_INST_uart0) {
pinctrl_bflb_configure_uart(BFLB_PINMUX_GET_PIN(pins[i]),
BFLB_PINMUX_GET_SIGNAL(pins[i]) + 4 * BFLB_PINMUX_GET_INST(pins[i]));
}

GLB_GPIO_Init(&pincfg);
/* gpio init*/
pinctrl_bflb_init_pin(pins[i]);
}

return 0;
}

/* clang-format on */
Loading