Skip to content

api: dma: add circular buffer support #15885

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

Closed
wants to merge 1 commit into from

Conversation

jli157
Copy link
Contributor

@jli157 jli157 commented May 3, 2019

Add one more bitfield to support buf configuration, thus
to support using circular buffer config.

And implement circular buffer support for STM32 SoCs.

The PR will be the one of dependencies for PR #14916

Signed-off-by: Jun Li [email protected]

Add one more bitfield to support buf configuration, thus
to support using circular buffer config.

And implement circular buffer support for STM32 SoCs.

Signed-off-by: Jun Li <[email protected]>
@jli157
Copy link
Contributor Author

jli157 commented May 3, 2019

@erwango Please help to review. Thanks!

@erwango erwango requested a review from avisconti May 6, 2019 12:51
@erwango
Copy link
Member

erwango commented May 6, 2019

@cybertale can you review?

@cybertale
Copy link
Collaborator

Hi, in reference maunals of STM32, circular mode is explained as 'reloading address pointer'. As an example from RM0090, it is explained as:

When the circular mode is activated, the number of data items to be transferred is
automatically reloaded with the initial value programmed during the stream configuration
phase, and the DMA requests continue to be served.

And there are two configuration options in 'struct dma_block_config', which is 'source_reload_en' and 'dest_reload_en', I think these two options can just do the job, right? For STM32 series SOCs, these two options have to be enabled and disabled togather since circular mode in STM32 reload both dest and src address pointers.

@jli157
Copy link
Contributor Author

jli157 commented May 6, 2019

Hi, in reference maunals of STM32, circular mode is explained as 'reloading address pointer'. As an example from RM0090, it is explained as:

When the circular mode is activated, the number of data items to be transferred is
automatically reloaded with the initial value programmed during the stream configuration
phase, and the DMA requests continue to be served.

And there are two configuration options in 'struct dma_block_config', which is 'source_reload_en' and 'dest_reload_en', I think these two options can just do the job, right? For STM32 series SOCs, these two options have to be enabled and disabled together since circular mode in STM32 reload both dest and src address pointers.

Hmm, it seems a good idea to use the fields source_reload_en and dest_reload_en for enabling the circular mode. However, STM32 seems not to differentiate them: either enabling both or disabling both. it just restarts the transfer once the register DMA_SxNDTR hits zero.

@avisconti
Copy link
Collaborator

Hi, in reference maunals of STM32, circular mode is explained as 'reloading address pointer'. As an example from RM0090, it is explained as:

When the circular mode is activated, the number of data items to be transferred is
automatically reloaded with the initial value programmed during the stream configuration
phase, and the DMA requests continue to be served.

And there are two configuration options in 'struct dma_block_config', which is 'source_reload_en' and 'dest_reload_en', I think these two options can just do the job, right? For STM32 series SOCs, these two options have to be enabled and disabled together since circular mode in STM32 reload both dest and src address pointers.

Hmm, it seems a good idea to use the fields source_reload_en and dest_reload_en for enabling the circular mode. However, STM32 seems not to differentiate them: either enabling both or disabling both. it just restarts the transfer once the register DMA_SxNDTR hits zero.

To be honest I think the purpose of source_reload_en and dest_reload_en was a little different, but maybe we can use them for this purpose. Nevertheless, it seems to me more clear to have a separate flag like this commit does (DMA_BUF_CFG_CIRCULAR).

@cybertale
Copy link
Collaborator

@avisconti I agree with aa standalone option for this mode, but I'm worried that if this feature is generic enough for ADC's api. I'm thinking it may be better to have a 'void *args' parameter pointing to a DMA controller related structure to convey these special options in dma_config().

@erwango erwango added the area: DMA Direct Memory Access label May 7, 2019
@jli157
Copy link
Contributor Author

jli157 commented May 9, 2019

@avisconti I agree with aa standalone option for this mode, but I'm worried that if this feature is generic enough for ADC's api. I'm thinking it may be better to have a 'void *args' parameter pointing to a DMA controller related structure to convey these special options in dma_config().

So, the client can access any DMA registers through that pointer? Sounds good to developers but the behaviors could be hard to control.

@cybertale
Copy link
Collaborator

@jli157 What I'm thinking is that they pass driver specific structure like what the clock control subsystem of stm32 do.
There should be a

typedef struct{
    bool circular_mode;
    ...
} stm32_dma_properties;

in dma_stm32.h or something like that. And when users use dma clients, they pass a pointer pointing to a instance of this structure.
This would be nice for us to add more options that only stm32 has, like double buffer mode.

@cybertale
Copy link
Collaborator

So the answer is no, client would not be able to access registers directly or through the pointer, they just pass options.

@jli157
Copy link
Contributor Author

jli157 commented May 10, 2019

@jli157 What I'm thinking is that they pass driver specific structure like what the clock control subsystem of stm32 do.
There should be a

typedef struct{
    bool circular_mode;
    ...
} stm32_dma_properties;

in dma_stm32.h or something like that. And when users use dma clients, they pass a pointer pointing to a instance of this structure.
This would be nice for us to add more options that only stm32 has, like double buffer mode.

The problem from this way is that a client needs to include a header file inside the driver directory which is kind of internal use. Also, some applications expect driver behaviors agnostic to specific hardware; thus its requirement can't be met by this way.

@cybertale
Copy link
Collaborator

From my experience, main usage of dma in stm32 is for stm32's internal IP blocks, ADC, DMA, etc. Other devices or apps using stm32's DMA wouldn't need this parameter so they would need to include that header file. For example, i2s driver of stm32 should include this header, while if some app needs only memory copy, they can pass NULL to this.
How clock control system works is very similar to this, like polymorphism in object-oriented programming, which could pass an base class object with a sub class object.

@github-actions github-actions bot added has-conflicts Issue/PR has conflicts with another issue/PR and removed has-conflicts Issue/PR has conflicts with another issue/PR labels Jun 29, 2020
@github-actions github-actions bot added the platform: STM32 ST Micro STM32 label Jul 7, 2020
@erwango
Copy link
Member

erwango commented Jul 8, 2020

@jli157 Is this PR still valid?

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

Requires heavy rebase

@jli157
Copy link
Contributor Author

jli157 commented Jul 8, 2020

It has been addressed and solved in the current code base. So, I can close this PR.

@jli157 jli157 closed this Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: DMA Direct Memory Access has-conflicts Issue/PR has conflicts with another issue/PR platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants