Hi Arnd
Thank you for your review
> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > index 3d02a3c1..4c5eda8 100644
> > --- a/drivers/mmc/host/tmio_mmc.h
> > +++ b/drivers/mmc/host/tmio_mmc.h
> > @@ -40,6 +40,16 @@
> >
> > struct tmio_mmc_data;
> >
> > +struct tmio_mmc_dma {
> > + void *chan_priv_tx;
> > + void *chan_priv_rx;
> > + int slave_id_tx;
> > + int slave_id_rx;
> > + int alignment_shift;
> > + dma_addr_t dma_rx_offset;
> > + bool (*filter)(struct dma_chan *chan, void *arg);
> > +};
>
> The slave_id/chan_priv values are now passed three times into the
> driver, and one should really be enough. I'd suggest removing the
> integer fields from both tmio_mmc_dma and tmio_mmc_data (added in
> patch 9), and instead pass it as a void* argument only to tmio_mmc_data.
Hmm. I guess this priv_?x and slave_id are based on filter ?
sh_mobile_sdhi case, and, if it is probed via non-DT,
it will use shdma_chan_filter, and then, it doesn't need both priv_?x and slave_id.
And, if it is probed via DT, it will use other filter,
and yes, it also doesn't need these parameter.
So, from sh_mobile_sdhi point of view, yes, we can do your idea.
But, if other driver want to use it with other filter, we need both ?
(sh_mobile_sdhi is the only dmaengine user at this point, so, there is no problem though...)
> The alignment_shift and dma_rx_offset values seem to always be
> the same for all users (at least the remaining ones, possibly there
> were others originally), so you could hardcode those in tmio_mmc_dma.c
> and remove the tmio_mmc_dma structure entirely.
Unfortunately, alignment_shift and dma_rx_offset value are based on SoC.
we can't hardcode these.
> For some obscure reason, the 'pdata->dma->slave_id_?x' fields currently
> get passed into cfg.slave_id, which is something we really want to
> get rid of so we can remove the slave_id field from dma_slave_config:
> The slave ID is generally considered specific to the channel allocation,
> not the configuration, and not all dmaengine drivers can express it
> as a single integer, so the concept is obsolete. When you remove
> the slave_id_?x fields here, you should also be able to just remove
> the cfg.slave_id assignment without any replacement, unless there is
> a bug in the dmaengine driver that should be fixed instead.
I guess maybe there is no problem about this,
but, actually I don't want do it, because of compatibility.
There are many combination for DMAEngine setting.
In sh-mobile-sdhi case, we are using this driver via non-DT / DT cases,
and different DMAEngine (sh-dma / rcar-dma).
But, I can't test all patterns today. So, I would like to keep it as-is
if possible.
Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
