On Tue, 17 Apr 2018, Fabrice Gasnier wrote:
> On 04/17/2018 09:12 AM, Lee Jones wrote:
> > On Mon, 16 Apr 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 v4:
> >> - Lee's comments: Add kerneldoc header, better format comments.
> >> 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 | 227 ++++++++++++++++++++++++++++++++++++++-
> >> include/linux/mfd/stm32-timers.h | 32 ++++++
> >> 2 files changed, 257 insertions(+), 2 deletions(-)
> >
> > [...]
> >
> >> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
> >> index 2aadab6..a04d7a1 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>
> >
> > [...]
> >
> >> +struct stm32_timers_dma;
> >> +
> >> struct stm32_timers {
> >> struct clk *clk;
> >> struct regmap *regmap;
> >> u32 max_arr;
> >> + struct stm32_timers_dma *dma; /* Only to be used by the parent */
> >
> > I'm confused. I thought the point of putting this comment in was so
> > that you could place the definition of 'stm32_timers_dma' and remove
> > the forward declaration?
>
> Hi Lee,
>
> Sorry, if I miss-understood the point then. So, do you wish I both:
> - move the full struct definition in above header ?
> - and keep this comment ?
That was what I thought we agreed.
However, I left the final decision to you. If you do not think this
is a reasonable i.e. the comment alone will not be enough to prevent
people from abusing the API, then leave it as it is.
Bear in mind that I think this introduces a build dependency on the
MFD driver for *each and every* other source file which includes this
header. If you choose the current solution, you will need to handle
that accordingly.
> +/**
> + * struct stm32_timers_dma - STM32 timer DMA handling.
> + * @completion: end of DMA transfer completion
> + * @phys_base: control registers physical base address
> + * @lock: protect DMA access
> + * @chan: DMA channel in use
> + * @chans: DMA channels available for this timer instance
> + */
> +struct stm32_timers_dma {
> + struct completion completion;
> + phys_addr_t phys_base;
> + struct mutex lock;
> + struct dma_chan *chan;
> + struct dma_chan *chans[STM32_TIMERS_MAX_DMAS];
> +};
>
> This will basically expose the struct to child drivers. But I'm ok if
> you think this is acceptable.
>
> I can send a V5 if you wish...
>
> Please advise,
> Best regards,
> Fabrice
>
> >
> >> };
> >> +
> >> +int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
> >> + enum stm32_timers_dmas id, u32 reg,
> >> + unsigned int num_reg, unsigned int bursts,
> >> + unsigned long tmo_ms);
> >> #endif
> >
--
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
