Skip to content

Conversation

@erwango
Copy link
Member

@erwango erwango commented Dec 18, 2020

Add dma based QSPI NOR flash controller for stm32l4 and stm32wb series.

Driver configures both NOR flash and also QSPI hardware block.
Forked from #25806 with addition of optional DMA support and reworked to make use jesd216 library.

Uses #31303

@github-actions
Copy link

github-actions bot commented Dec 18, 2020

The following projects have a revision update in this Pull Request:

Name Old Revision New Revision
hal_stm32 zephyrproject-rtos/hal_stm32@5a10f27 zephyrproject-rtos/hal_stm32@cc8731d (master)

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@erwango erwango force-pushed the dev_qspi_dma_stm32 branch 2 times, most recently from a29bbbd to b8c3c32 Compare December 18, 2020 13:32
@erwango
Copy link
Member Author

erwango commented Dec 18, 2020

^^ @giansta, @r2r0

@erwango
Copy link
Member Author

erwango commented Dec 18, 2020

^^@mtahirbutt @hjuul

Copy link
Contributor

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

Preliminary comments. I hope to have hardware to test this within the next two weeks.

The spi-nor sample and probably at least one flash subsystem sample should be updated to support a board (preferably B-L475E-IOT01A since that's what I've got coming...).

stream->dma_callback(dev, stream->user_data, callback_arg, 0);
} else if (dma_stm32_is_ht_active(dma, id)) {
dma_stm32_clear_ht(dma, id);
dma_stm32_clear_tc(dma, id);
Copy link
Contributor

Choose a reason for hiding this comment

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

The change to clear tc after the callback instead of before is intentional? Depending on what that means I could see this introducing a race condition and lost data (e.g. a second event comes in during/after the callback and is discarded).

Just want to be sure.

Copy link
Member Author

@erwango erwango Dec 18, 2020

Choose a reason for hiding this comment

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

The other way round didn't work with my purpose as DMA HAL needs flags to be still available when the callback is called to work as expected.
This being said, I might need to test more thoroughly other cases to be sure I didn't break anything. In which case, I would have to use this behavior only in case of "DMA channel override"

Copy link
Contributor

Choose a reason for hiding this comment

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

is this still an issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

is this still an issue?

For sure I can't make it work the other way round due to use of HAL API. So this is still needed, yes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant, did you still need to test more thoroughly, or are you satisfied that we can ignore the change?

Copy link
Member Author

@erwango erwango Jan 12, 2021

Choose a reason for hiding this comment

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

It was tested as much as I could and no issue was find. Anyway, I've just protected this behavior so we should be safe on that front now.

@erwango
Copy link
Member Author

erwango commented Dec 18, 2020

Txs @pabigot for the initial review. Will take those into account.

The spi-nor sample and probably at least one flash subsystem sample should be updated to support a board (preferably B-L475E-IOT01A since that's what I've got coming...).

I agree that we need a way to test this driver in CI (or at least compile it). I think I can find a way to tweak samples/drivers/flash_shell to achieve this.
Though, I'm not sure about the spi-nor sample, do you mean samples/drivers/jesd216 ?

EDIT: Found it: spi-nor sample is samples/drivers/spi_flash/src/main.c.

/* TODO: This init is limited to G0/G4/L0/L4/L5/WB */
hdma.Init.PeriphDataAlignment = table_p_size[index];
hdma.Init.MemDataAlignment = table_m_size[index];
hdma.Init.Request = dma_cfg.dma_slot;
Copy link
Contributor

@hjuul hjuul Dec 21, 2020

Choose a reason for hiding this comment

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

I was trying to make this work for stm32f7, but get a compile error here when building samples/drivers/flash_shell.

If you could extend support to DMA_STM32_V1 also I can help to verify it on stm32f746g_disco and stm32f769i_disco boards.

If not, perhaps use #ifndef CONFIG_DMA_STM32_V1 or similar to make it compile.

In case you choose to add this, here's my attempt at dts files for stm32f746i_disco. I can also make such patch for stm32f769g_disco if you like.

boards/arm/stm32f756g_disco/stm32f756_disco.dts:

&dma2 {
	status = "okay";
};

&quadspi {
	pinctrl-0 = <&quadspi_clk_pb2 &quadspi_bk1_ncs_pb6
		     &quadspi_bk1_io0_pd11 &quadspi_bk1_io1_pd12
		     &quadspi_bk1_io2_pe2 &quadspi_bk1_io3_pd13>;
	dmas = <&dma2 7 3 0x0000 0x03>;
	dma-names = "tx_rx";

	status = "okay";

	n26q128a13ef840e: qspi-nor-flash@0 {
		compatible = "st,stm32-qspi-nor";
		label = "N25Q128A13EF840E";
		reg = <0>;
		spi-max-frequency = <108000000>;
		/* 128 Megabits = 16 Megabytes */
		size = <0x8000000>;
		status = "okay";
	};
};

dts/arm/st/f7/stm32f7.dtsi:

		quadspi: quadspi@a0001000 {
			compatible = "st,stm32-qspi";
			#address-cells = <1>;
			#size-cells = <0>;
			reg = <0xa0001000 0x34>;
			interrupts = <92 0>;
			clocks = <&rcc STM32_CLOCK_BUS_AHB3 0x00000002>;
			status = "disabled";
			label = "QUADSPI";
		};

Copy link
Member Author

Choose a reason for hiding this comment

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

@hjuul thanks for your interest. I'll try to add F7 support later this week.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not much time to look at that right now, but the following should get things compile for F7:

#ifdef CONFIG_DMA_STM32_V1
	hdma.Instance = __LL_DMA_GET_STREAM_INSTANCE(dev_data->dma.reg,
						      dev_data->dma.channel);
	hdma.Init.Channel = dma_cfg.dma_slot;
#else
	hdma.Instance = __LL_DMA_GET_CHANNEL_INSTANCE(dev_data->dma.reg,
						      dev_data->dma.channel-1);
	hdma.Init.Request = dma_cfg.dma_slot;
#endif /* CONFIG_DMA_STM32_V1 */

Copy link
Contributor

@FRASTM FRASTM left a comment

Choose a reason for hiding this comment

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

could the commit cac002c7a013549fd7b1324d4f9fb65ba8e981fa be another PR ?

@erwango
Copy link
Member Author

erwango commented Jan 14, 2021

could the commit cac002c be another PR ?

ok, right. Created #31303

In preparation for QSPI DMA mode:
-Add a possibility to override driver by the HAL DMA. In that case
stream is set as busy and no configuration nor treatment is done.
In case of interrupt, flags clearing is let to HAL.
-Treat Half Transfer interrupt prior to Transfer Complete for the
cases were both IRQ are both raised at the time IRQ handler is called

Signed-off-by: Erwan Gouriou <[email protected]>
Add support for DMA based STM32 QSPI NOR flash controller.
Driver configures both NOR flash and also QSPI hardware block.
Reuses existing jesd216 library.

QSPI hardware block handling is done through the use of Cube HAL API.
This requires the use of HAL interface also for DMA besides zephyr
DMA driver.
Zephyr DMA driver is used only for IRQ routing while HAL driver
handles the IP block. To achieve this it is required to:
-Configure both Cube and Zephyr drivers at init.
-Inform Zephyr driver that current channel handling will be done
by another instance and only a limited configuration should be done.
For this last part, a unused parameter is overridden in order to
transmit the information.

Signed-off-by: Erwan Gouriou <[email protected]>
Signed-off-by: Piotr Mienkowski <[email protected]>
Add device tree description for qspi node on STM32L4 and
signals on whole family.

Signed-off-by: Erwan Gouriou <[email protected]>
Signed-off-by: Piotr Mienkowski <[email protected]>
Configure QSPI NOR support and MX25R6435F on disco_l475_iot1 board.
Set MX25R6435F as flash controller and arrange partitions to take
newlay available space into account.

Signed-off-by: Erwan Gouriou <[email protected]>
Enable using st,stm32-qspi-nor compatible driver for this
sample.

Signed-off-by: Erwan Gouriou <[email protected]>
Add disco_l475_iot1 as build platform for this sample

Signed-off-by: Erwan Gouriou <[email protected]>
Add dt_node_has_prop function to query the presence of 'prop'
for given node label.

Signed-off-by: Erwan Gouriou <[email protected]>
In order to ease reuse on other series, set DMA as optional
and use IT if no DMA channel is specified in the qspi node.

Tested on disco_l475_iot1

Signed-off-by: Erwan Gouriou <[email protected]>
Add quadspi support to stm32wb series.
Tested on nucleo_wb55rg + nor flash.

Signed-off-by: Erwan Gouriou <[email protected]>
@erwango erwango force-pushed the dev_qspi_dma_stm32 branch from dcd0dad to cd48c83 Compare January 14, 2021 10:31
@erwango erwango requested a review from carlescufi as a code owner January 14, 2021 10:31
@github-actions github-actions bot added the area: API Changes to public APIs label Jan 14, 2021
@fzawadiak
Copy link

fzawadiak commented Jan 14, 2021

DTS node for H7

			compatible = "st,stm32-qspi";
			#address-cells = <1>;
			#size-cells = <0>;
			reg = <0x52005000 0x400>;
			interrupts = <92 0>;
			clocks = <&rcc STM32_CLOCK_BUS_AHB3 0x00004000>;
			status = "disabled";
			label = "QUADSPI";
		};

And for stm32h747i_disco_m7.dts (bk2 pins can be removed, but they are there for dual flash)

&quadspi {
	pinctrl-0 = <&quadspi_clk_pb2
		     &quadspi_bk1_ncs_pg6
		     &quadspi_bk1_io0_pd11 &quadspi_bk1_io1_pf9
		     &quadspi_bk1_io2_pf7 &quadspi_bk1_io3_pf6
		     &quadspi_bk2_io0_ph2 &quadspi_bk2_io1_ph3
		     &quadspi_bk2_io2_pg9 &quadspi_bk2_io3_pg14>;

	status = "okay";

	mt25tl01g: qspi-nor-flash@0 {
		compatible = "st,stm32-qspi-nor";
		label = "MT25TL01G";
		reg = <0>;
		spi-max-frequency = <50000000>;
		/* Dual 512 MBits => Dual 64MBytes => 128MBytes */
		size = <0x40000000>;
		status = "okay";
	};
};

@erwango
Copy link
Member Author

erwango commented Jan 14, 2021

DTS node for H7

Thanks. I had a test, this is working but I'm getting MDMA HAL warnings (even when not using DMA). If I manage to get rid of thoss, I'll include support in current PR. Otherwise this will have to wait a bit.

Copy link
Contributor

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

Still want JESD216 API support but this is good and works for me on small devices. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.