Hi Vinod,
On Monday 08 December 2014 18:15:25 Vinod Koul wrote:
> On Fri, Nov 28, 2014 at 12:17:59AM +0000, Kuninori Morimoto wrote:
> > From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> >
> > Current Renesas Audio DMAC Peri Peri driver is for Renesas R-Car series,
> > but it is based on SH_DMAE_BASE driver which is used for Renesas
> > SH-Mobile. But it is not fully cared about DT.
> >
> > For example, current SH_DMAE_BASE base driver will return non-matching
> > DMA channel if some non-SH_DMAE_BASE drivers are probed.
> > So, sound driver will get wrong DMA channel if Audio DMAC (= rcar-dma)
> > was not probed and Audio DMAC Peri Peri (= SH_DMAE_BASE) was probed.
> >
> > OTOH, many SH-Mobile series drivers are using SH_DMAE_BASE driver, and
> > Renesas R-Car series will not use it anymore. Maintenance cost for fully
> > cared DT support on SH_DMAE_BASE will be very high
> > (and keeping compatibility will be very complex).
> >
> > In addition, Audio DMAC Peri Peri itself is very simple device, and,
> > no SoC/board is usign it from non-DT environment.
> >
> > This patch remakes rcar-audmapp driver simply independents
> > from SH_DMAE_BASE.
>
> Hi Kuninori
>
> Can you please split these changes into multiple smaller changes so that we
> can review this propely...
I was about to ask the same when I realized this is a complete rewrite of the
driver. I'm not sure whether it could be split properly. Reviewing the result
might be easier.
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> > ---
> >
> > drivers/dma/sh/Kconfig | 3 +-
> > drivers/dma/sh/rcar-audmapp.c | 399 +++++++++---------
> > include/linux/platform_data/dma-rcar-audmapp.h | 34 --
> > 3 files changed, 184 insertions(+), 252 deletions(-)
> > delete mode 100644 include/linux/platform_data/dma-rcar-audmapp.h
> >
> > diff --git a/drivers/dma/sh/Kconfig b/drivers/dma/sh/Kconfig
> > index 8190ad2..41e5c97 100644
> > --- a/drivers/dma/sh/Kconfig
> > +++ b/drivers/dma/sh/Kconfig
> > @@ -53,7 +53,8 @@ config RCAR_HPB_DMAE
> >
> > config RCAR_AUDMAC_PP
> > tristate "Renesas R-Car Audio DMAC Peripheral Peripheral support"
> > - depends on SH_DMAE_BASE
> > + depends on ARCH_SHMOBILE || COMPILE_TEST
>
> Am bit wary of this, given that folks dont test on lots of archs, like arm,
> powerpc, x86 etc before turning this on, so was this done here?
The whole point of COMPILE_TEST is to catch compilation failures on other
architectures. I agree it should be tested at least on x86.
> > +static struct dma_async_tx_descriptor *
> > +audmapp_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
> > + size_t buf_len, size_t period_len,
> > + enum dma_transfer_direction dir, unsigned long flags)
> > {
> > - struct audmapp_chan *auchan = to_chan(schan);
> > - u32 chcr;
> > - dma_addr_t dst;
> > - int ret;
> > -
> > - ret = audmapp_get_config(auchan, slave_id, &chcr, &dst);
> > - if (ret < 0)
> > - return ret;
> > -
> > - if (try)
> > - return 0;
> > -
> > - auchan->chcr = chcr;
> > - auchan->slave_addr = slave_addr ? : dst;
> > + struct audmapp_chan *achan = chan_to_achan(chan);
> >
> > - return 0;
> > + return &achan->async_tx;
>
> That doesn't look right to me. So for prepare you are returning descriptor
> but not really preparing one, did I miss something here
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
