Re: [PATCH 2/9] mmc: tmio: tmio_mmc_host has .dma

Kuninori Morimoto
2015-01-06T00:20:59Z

Hi Arnd, Ulf

Thank you for your feedback

> > > 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 ?
>
> priv_?x needs to be in a format that matches the filter function,
> passing slave_id is basically always wrong, but dmaengine drivers
> generally just ignore it.
(snip)
> Maybe we can hide the slave_id field in dma_slave_config within
> '#if defined(CONFIG_ARCH_SH_MOBILE) && defined(CONFIG_ATAGS)' then?
>
> I would really like to prevent other people from copying the mistake.
> Apparently it has already happened on MIPS jz4740, but that one seems
> easy enough to fix. Tegra used to use slave_id as well, but it's been
> converted to DT-only a while ago and that is just dead code for them.
>
> I've had a closer look at the existing code now and came up with a
> patch that should work for all out-of-tree drivers you may be worried
> about.

I don't want to use #ifdef in driver :P
OK, I understand your opinion. I can try fix this DMAEngine issue
(without #ifdef :)

But, I want "step by step" for this cleanup.
So, can you please accept about current "header cleanup" patch-set as 1st step ?
Then, I want to try "dmaengine cleanup" patch-set as 2nd step if possible.
I guess [8/9] and [9/9] are not good for "header cleanup" (?) I don't know.
Arnd, Ulf, what is your opinion ?

Best regards
---
Kuninori Morimoto