Skip to content

dts: bindings: dma: gd32: Redesign DMA node's cell properties #48756

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Dec 22, 2022
Merged

dts: bindings: dma: gd32: Redesign DMA node's cell properties #48756

merged 7 commits into from
Dec 22, 2022

Conversation

@soburi
Copy link
Member

@soburi soburi commented Aug 6, 2022 •
edited
Loading

From #47504 review, split out to this PR about gd32-dma related changes.

From result of discussion in this PR, the cell property specs following STM32's interface specs is preferable.

This PR make following changes...

  • Add config cell property to gd,gd32-dma
  • Add mem2mem property to indicate it supports memory to memory transfer
  • Split gd,gd32-dma-v1 from gd,gd32-dma for supporting F4XX specific features (FIFO is defined, but not implemented)
  • Use dma_slot for peripheral selectiong instead of linked_channel.
  • Add/Correct overlay files for dma/loop_transfer test.

I verify the dma/loop_transfer test passed with gd32f407v_start and longan_nano.
Other boards case, I only checked it can compile.

@soburi soburi requested a review from galak August 6, 2022 09:53
@zephyrbot zephyrbot added platform: GD32 GigaDevice area: Devicetree area: DMA Direct Memory Access labels Aug 6, 2022
@soburi soburi mentioned this pull request Aug 6, 2022
Merged
galak
galak previously requested changes Aug 6, 2022
View reviewed changes
Copy link
Contributor

@galak galak left a comment

Choose a reason for hiding this comment

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

Question on devicetree binding.

@soburi soburi requested a review from galak August 7, 2022 22:33
@soburi soburi force-pushed the gd32_dma_cells branch 2 times, most recently from f2d4bc4 to 16445b6 Compare August 8, 2022 11:48
@gmarull
Copy link
Member

gmarull commented Aug 9, 2022

I think GD32 dma bindings must align with STM32, see

https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/plain/Bindings/dma/st,stm32-dma.yaml

and Zephyr counterpart.

@soburi
Copy link
Member Author

soburi commented Aug 29, 2022

Hi @gmarull , @galak

Sorry for late response.

I think GD32 dma bindings must align with STM32, see

https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/plain/Bindings/dma/st,stm32-dma.yaml

and Zephyr counterpart.

I reworked this PR to make it compatible with STM32's dts spec.

I think the STM32's channel-config cell design is a bit complex.
(And almost of these should determine by what peripheral using.)
But making it compatible with STM32 is a preferable policy, and I followed it.

Could you tell me your opinions?

@soburi soburi force-pushed the gd32_dma_cells branch 2 times, most recently from 8e1147b to 67117a6 Compare September 6, 2022 13:26
@carlescufi
Copy link
Member

@galak @gmarull ping

gmarull
gmarull reviewed Sep 20, 2022
View reviewed changes
Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

just one question

@soburi soburi requested a review from gmarull October 26, 2022 10:02
@soburi
Copy link
Member Author

soburi commented Dec 2, 2022

Hi, @gmarull @galak @carlescufi

Could you re-check this PR if you can take a while.

Point to discussion:
IMO, The GD32 DMA property cell's specs should be identical or similar to the STM32.
From some conversations with @galak, I also understanding that simple specs are preferable.
On the other hand, The STM32's DMA property cell specs are a little complex.
(Because the specs make it able to set all properties. So we need to provide a header file that define the property bits.)
But in this case, I think it is better to make the specs compatible with STM32's one for compatibility reasons.

If I can, I want to proceed with this PR and #47504 to bring it into v3.3.

nandojve
nandojve requested changes Dec 11, 2022
View reviewed changes
@soburi soburi changed the title dts: bindings: dma: gd32: Add dma-cells property dts: bindings: dma: gd32: Redesign DMA node's cell properties Dec 12, 2022
@soburi soburi force-pushed the gd32_dma_cells branch 2 times, most recently from 47c4e27 to f35c0ea Compare December 12, 2022 10:18
@nordicjm nordicjm removed their request for review December 12, 2022 10:21
@soburi soburi requested a review from nandojve December 12, 2022 10:33
gmarull
gmarull reviewed Dec 19, 2022
View reviewed changes
@soburi
tests: dma: loop_transfer: Correct DMA definition of gd32f350r_eval
513a838 
The overlay file overrides not existing node `dma0`.
Correct it to `dma`.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi
drivers: dma: dma_gd32: Use dma_slot for peripheral request
40beae1 
Use dma_slot for peripheral request instead of linked_channel.
This is a more suitable usage as described in dma_config.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi
dts: bindings: gd32-dma: add config cell property
568f69e 
Add config cell property to gd,gd32-dma.
For supporting hardware variation, Splitting base definition
to gd,gd32-dma-base.yaml.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi
dts: bindings: gd32-dma-base: add gd,mem2mem property
adfc2cc 
Add `gd,mem2mem` property to indicate the DMA controller supports
memory to memory transfer.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi
dts: bindings: dma: gd32: split gd,gd32-dma-v1 for support F4xx feature
022a540 
Split gd,gd32-dma-v1 from gd,gd32-dma to support F4xx specific features.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi
tests: dma: loop_transfer: add overlay for GD32 boards
cf6337c 
Add overlay file for gd32_e507z_eval, gd32f407v_start,
gd32f450v_start, gd32f450v_start and longan_nano_lite.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@soburi
boards: gd32: add dma as supported feature
ead4c87 
Add `dma` as supported feature to yaml configuration.

Signed-off-by: TOKITA Hiroshi <[email protected]>
@gmarull gmarull dismissed galak’s stale review December 22, 2022 09:17

no longer applies, bindings changed

@carlescufi carlescufi merged commit 20d2dff into zephyrproject-rtos:main Dec 22, 2022
@soburi soburi deleted the gd32_dma_cells branch December 22, 2022 13:01
@nandojve nandojve mentioned this pull request Dec 22, 2022
Open
55 tasks
@soburi soburi mentioned this pull request Feb 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cameled cameled cameled left review comments

@nandojve nandojve nandojve approved these changes

@gmarull gmarull gmarull approved these changes

@kgugala kgugala Awaiting requested review from kgugala

@pgielda pgielda Awaiting requested review from pgielda

@mbolivar-nordic mbolivar-nordic Awaiting requested review from mbolivar-nordic

@teburd teburd Awaiting requested review from teburd

@galak galak Awaiting requested review from galak

@nashif nashif Awaiting requested review from nashif

@carlescufi carlescufi Awaiting requested review from carlescufi

@MaureenHelm MaureenHelm Awaiting requested review from MaureenHelm

@tbursztyka tbursztyka Awaiting requested review from tbursztyka

@fkokosinski fkokosinski Awaiting requested review from fkokosinski

@tgorochowik tgorochowik Awaiting requested review from tgorochowik

@jakub-uC jakub-uC Awaiting requested review from jakub-uC

@nordic-krch nordic-krch Awaiting requested review from nordic-krch

@de-nordic de-nordic Awaiting requested review from de-nordic

Assignees

@nandojve nandojve

Labels

area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree area: DMA Direct Memory Access area: RISCV RISCV Architecture (32-bit & 64-bit) area: SPI SPI bus platform: GD32 GigaDevice

Projects

None yet

Milestone

v3.3.0

Development

Successfully merging this pull request may close these issues.

7 participants

@soburi @gmarull @carlescufi @nandojve @galak @cameled @zephyrbot