dts/arm: stm32h7: quadspi added #34695

SpooniSpoon · 2021-04-29T11:44:26Z

dts/arm: stm32h7: quadspi added

quadspi added to device tree of stm32h7 family.

validated with samples/subsys/fs/littlefs

Signed-off-by: Raphael Löffel [email protected]

erwango

@SpooniSpoon have you been able to validate that this is sufficient to use quadspi on H7 ?

SpooniSpoon · 2021-04-29T15:14:37Z

I used samples/subsys/fs/littlefs without DMA support for validation of the dts add-on.

erwango · 2021-04-29T15:39:08Z

I used samples/subsys/fs/littlefs without DMA support for validation of the dts add-on.

Nice! Was it done on an in tree board ?
If yes, would you be able to push related changes (so it can demonstrate how to use it and we could be able to at least compile the driver with H7 configuration in CI) ?

SpooniSpoon · 2021-04-30T07:56:17Z

Nice! Was it done on an in tree board ?
If yes, would you be able to push related changes (so it can demonstrate how to use it and we could be able to at least compile the driver with H7 configuration in CI) ?

Unfortunately not, I am using a custom board with a W25Q128JVSIM QSPI device.

erwango · 2021-04-30T08:19:41Z

Unfortunately not, I am using a custom board with a W25Q128JVSIM QSPI device.

Ok. From what I can see we have an in-tree board with quad-spi memory (STM32H747I-DISCO),
so we'd need to enable on this board.
Did it work out of the box by just adding this node ? (board configuration put aside)

SpooniSpoon · 2021-04-30T09:38:56Z

Ok. From what I can see we have an in-tree board with quad-spi memory (STM32H747I-DISCO),
so we'd need to enable on this board.
Did it work out of the box by just adding this node ? (board configuration put aside)

First of all I had to undefined the DMA node within the QSPI node. After that I still had some compiler errors related to the MDMA. As a quick fix I added select USE_STM32_HAL_MDMA to the file Kconfig.stm32_qspi. The board configuration I used is

&quadspi {
    pinctrl-0 = <&quadspi_clk_pb2
                 &quadspi_bk1_ncs_pb10
                 &quadspi_bk1_io0_pd11
                 &quadspi_bk1_io1_pd12
                 &quadspi_bk1_io2_pe2
                 &quadspi_bk1_io3_pd13>;
//    dmas = <&dma1 5 5 0x0000 0x03>;
//    dma-names = "tx_rx";

    status = "okay";

    w25q128jvsim: qspi-nor-flash@0 {
        compatible = "st,stm32-qspi-nor";
        label = "W25Q128JVSIM";
        reg = <0>;
        qspi-max-frequency = <100000000>;
        /* 128 Megabits = 16 Megabytes */
        size = <0x8000000>;

        status = "okay";

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

            fs_storage_partition: partition@0 {
                label = "storage";
                reg = <0x00000000 DT_SIZE_M(16)>;
            };
        };
    };
};

erwango · 2021-04-30T10:10:23Z

First of all I had to undefined the DMA node within the QSPI node.

This is not strictly an issue as we can have QPSI working w/o dma.
We'd have QSPI supported on H7 with the limitation that DMA is not yet supported.
But it would need to be clearly specified and documented. Typically generate a dedicated error message
in case one would try to enable DMA with QSPI on H7.

After that I still had some compiler errors related to the MDMA. As a quick fix I added select USE_STM32_HAL_MDMA to the file Kconfig.stm32_qspi.

Likely this is because QSPI support requires MDMA enabling in Cube H7 HAL, we can live with that,
but impacts of having DMA required in HAL and disabled in Zephyr would need a minimum investigation and information to users.

In any case we'd need these both points to be addressed and provided as part of this change so others don't face and report the same issues.

erwango

Please provide related drivers changes.

SpooniSpoon · 2021-04-30T11:17:30Z

But it would need to be clearly specified and documented.

Where is the right place for documentation in the related yml file of the qspi binding or in the help section of the related Kconfig file?

erwango · 2021-04-30T12:16:05Z

But it would need to be clearly specified and documented.

Where is the right place for documentation in the related yml file of the qspi binding or in the help section of the related Kconfig file?

I'd say both Kconfig and yaml.
Also, in the code directly, by generating a dedicated error message in case one would try to enable DMA with QSPI on H7.

FRASTM · 2021-05-06T14:49:03Z

commit message drivers/falsh --> drivers/flash

erwango

Please fix commit message for first commit ("quadspi added to device tree of stm32h7 familie")

dts/arm/st/h7/stm32h7.dtsi

quadspi added to device tree of stm32h7 family. validated with samples/subsys/fs/littlefs Signed-off-by: Raphael Löffel <[email protected]>

whitespace issue fixed to pass compliance checks. Signed-off-by: Raphael Löffel <[email protected]>

erwango

One more comment.
Also please squash your 2 commits.

erwango · 2021-05-11T06:47:26Z

drivers/flash/Kconfig.stm32_qspi

 	select FLASH_HAS_PAGE_LAYOUT
 	select DMA if $(DT_STM32_QUADSPI_HAS_DMA)
 	select USE_STM32_HAL_DMA if $(DT_STM32_QUADSPI_HAS_DMA)
+	select USE_STM32_HAL_MDMA if !$(DT_STM32_QUADSPI_HAS_DMA)


The proposed logic is not correct.
User always has the possibility of not using DMA wth QSPI, even if supported on the series in use.
With this change, MDMA is systematically enabled in this case.

The MDMA is always required by the STM32 QSPI HAL driver also without DMA support. As a workaround to avoid compiler errors I enabled the MDMA HAL driver if DMA support is disabled.

This maybe the case on H7, but not on all STM32 series.
You can have a try by doing the folloing change in disco_l475_iot1.dts:

&quadspi { pinctrl-0 = <&quadspi_clk_pe10 &quadspi_ncs_pe11 &quadspi_bk1_io0_pe12 &quadspi_bk1_io1_pe13 &quadspi_bk1_io2_pe14 &quadspi_bk1_io3_pe15>; - dmas = <&dma1 5 5 0x0000 0x03>; - dma-names = "tx_rx"; + //dmas = <&dma1 5 5 0x0000 0x03>; + //dma-names = "tx_rx";

Then run:

west build -b disco_l475_iot1 samples/drivers/flash_shell

github-actions · 2021-07-14T00:28:36Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

cfriedt · 2021-07-16T23:08:58Z

@SpooniSpoon - please rebase and address the last comment

cfriedt · 2021-09-02T10:56:19Z

@SpooniSpoon , @erwango - is this one still moving forward?

erwango · 2021-11-03T18:01:43Z

@SpooniSpoon Do you plan to continue this PR ?

github-actions · 2022-01-03T00:29:23Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

nvlsianpu

similar patch is one of parts of #40859

SpooniSpoon requested a review from erwango as a code owner April 29, 2021 11:44

github-actions bot added area: Devicetree platform: STM32 ST Micro STM32 labels Apr 29, 2021

erwango reviewed Apr 29, 2021

View reviewed changes

FRASTM approved these changes Apr 29, 2021

View reviewed changes

zephyrbot requested a review from ABOSTM April 29, 2021 13:48

zephyrbot assigned erwango Apr 29, 2021

SpooniSpoon changed the title ~~quadspi added to device tree of stm32h7 familie~~ dts/arm: stm32h7: quadspi added Apr 29, 2021

erwango requested changes Apr 30, 2021

View reviewed changes

SpooniSpoon requested review from nashif and nvlsianpu as code owners May 3, 2021 06:12

github-actions bot added the area: Devicetree Binding PR modifies or adds a Device Tree binding label May 3, 2021

erwango requested changes May 6, 2021

View reviewed changes

dts/arm/st/h7/stm32h7.dtsi Outdated Show resolved Hide resolved

dts/arm: stm32h7: quadspi added

b36db74

quadspi added to device tree of stm32h7 family. validated with samples/subsys/fs/littlefs Signed-off-by: Raphael Löffel <[email protected]>

SpooniSpoon force-pushed the dev_dts_stm32h7 branch from 2b35285 to b36db74 Compare May 10, 2021 08:12

erwango added this to the v2.7.0 milestone May 10, 2021

dts/arm: stm32h7: whitespace issue fixed

7563995

whitespace issue fixed to pass compliance checks. Signed-off-by: Raphael Löffel <[email protected]>

SpooniSpoon force-pushed the dev_dts_stm32h7 branch from eb31168 to 7563995 Compare May 11, 2021 06:31

erwango requested changes May 11, 2021

View reviewed changes

github-actions bot added the Stale label Jul 14, 2021

github-actions bot removed the Stale label Jul 17, 2021

cfriedt modified the milestones: v2.7.0, v3.0.0 Sep 20, 2021

github-actions bot added the Stale label Jan 3, 2022

nvlsianpu reviewed Jan 3, 2022

View reviewed changes

github-actions bot closed this Jan 18, 2022

dts/arm: stm32h7: quadspi added #34695

dts/arm: stm32h7: quadspi added #34695

Uh oh!

Conversation

SpooniSpoon commented Apr 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erwango left a comment

Choose a reason for hiding this comment

Uh oh!

SpooniSpoon commented Apr 29, 2021

Uh oh!

erwango commented Apr 29, 2021

Uh oh!

SpooniSpoon commented Apr 30, 2021

Uh oh!

erwango commented Apr 30, 2021

Uh oh!

SpooniSpoon commented Apr 30, 2021

Uh oh!

erwango commented Apr 30, 2021

Uh oh!

erwango left a comment

Choose a reason for hiding this comment

Uh oh!

SpooniSpoon commented Apr 30, 2021

Uh oh!

erwango commented Apr 30, 2021

Uh oh!

FRASTM commented May 6, 2021

Uh oh!

erwango left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

erwango left a comment

Choose a reason for hiding this comment

Uh oh!

erwango May 11, 2021

Choose a reason for hiding this comment

Uh oh!

SpooniSpoon May 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erwango May 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 14, 2021

Uh oh!

cfriedt commented Jul 16, 2021

Uh oh!

cfriedt commented Sep 2, 2021

Uh oh!

erwango commented Nov 3, 2021

Uh oh!

github-actions bot commented Jan 3, 2022

Uh oh!

nvlsianpu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SpooniSpoon commented Apr 29, 2021 •

edited

Loading

SpooniSpoon May 11, 2021 •

edited

Loading

erwango May 11, 2021 •

edited

Loading

nvlsianpu left a comment •

edited

Loading