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@xxxxxx>
> >> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxx>
> >> ---
> >> 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.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
