lkml.org 
[lkml]   [2018]   [Apr]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 2/6] mfd: stm32-timers: add support for dmas
From
Date
On 04/16/2018 04:47 PM, Lee Jones wrote:
> On Mon, 16 Apr 2018, Fabrice Gasnier wrote:
>
>> On 04/16/2018 02:22 PM, Lee Jones wrote:
>>> On Fri, 30 Mar 2018, Fabrice Gasnier wrote:
>>>
>>>> STM32 Timers can support up to 7 DMA requests:
>>>> - 4 channels, update, compare and trigger.
>>>> Optionally request part, or all DMAs from stm32-timers MFD core.
>>>>
>>>> Also add routine to implement burst reads using DMA from timer registers.
>>>> This is exported. So, it can be used by child drivers, PWM capture
>>>> for instance (but not limited to).
>>>>
>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>>> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>>>> ---
>>>> Changes in v3:
>>>> - Basically Lee's comments:
>>>> - rather create a struct stm32_timers_dma, and place a reference to it
>>>> in existing ddata (instead of adding priv struct).
>>>> - rather use a struct device in exported routine prototype, and use
>>>> standard helpers instead of ddata. Get rid of to_stm32_timers_priv().
>>>> - simplify error handling in probe (remove a goto)
>>>> - comment on devm_of_platform_*populate() usage.
>>>>
>>>> Changes in v2:
>>>> - Abstract DMA handling from child driver: move it to MFD core
>>>> - Add comments on optional dma support
>>>> ---
>>>> drivers/mfd/stm32-timers.c | 219 ++++++++++++++++++++++++++++++++++++++-
>>>> include/linux/mfd/stm32-timers.h | 32 ++++++
>>>> 2 files changed, 249 insertions(+), 2 deletions(-)
>
> [...]
>
>>>> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
>>>> index 2aadab6..585a4de 100644
>>>> --- a/include/linux/mfd/stm32-timers.h
>>>> +++ b/include/linux/mfd/stm32-timers.h
>>>> @@ -8,6 +8,8 @@
>>>> #define _LINUX_STM32_GPTIMER_H_
>>>>
>>>> #include <linux/clk.h>
>>>> +#include <linux/dmaengine.h>
>>>> +#include <linux/dma-mapping.h>
>>>> #include <linux/regmap.h>
>>>
>>> [...]
>>>
>>>> +enum stm32_timers_dmas {
>>>> + STM32_TIMERS_DMA_CH1,
>>>> + STM32_TIMERS_DMA_CH2,
>>>> + STM32_TIMERS_DMA_CH3,
>>>> + STM32_TIMERS_DMA_CH4,
>>>> + STM32_TIMERS_DMA_UP,
>>>> + STM32_TIMERS_DMA_TRIG,
>>>> + STM32_TIMERS_DMA_COM,
>>>> + STM32_TIMERS_MAX_DMAS,
>>>> +};
>>>> +
>>>> +struct stm32_timers_dma;
>>>
>>> Why don't you move the declaration into here?
>>
>> To follow previous discussions we had in V1 and V2, this is to avoid
>> sharing all the information with child drivers, e.g. passing physical
>> address of parent MFD into child devices.
>>
>> I should probably add a comment there ? Something like:
>>
>> /* STM32 timers MFD parent internal struct to handle DMA transfers */
>> struct stm32_timers_dma;
>>
>> Do you agree with this ?
>>
>> Thanks for reviewing,
>> BR,
>> Fabrice
>>
>>>
>>> Then you don't need to forward declare.
>
> Yes, I remember our previous conversation.
>
> Perhaps you could always put a comment like:
>
>>>> struct stm32_timers {
>>>> struct clk *clk;
>>>> struct regmap *regmap;
>>>> u32 max_arr;
>>>> + struct stm32_timers_dma *dma; /* Only to be used by the parent */
>>>> };
>
> If this is not acceptable, then the current solution will do.

Hi Lee,

I'll update it with your proposal,

Many thanks,
Fabrice

>

\
 
 \ /
  Last update: 2018-04-16 17:14    [W:0.046 / U:1.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site