Re: [PATCH 1/3] DMAENGINE: generic slave channel control v2

[Linus Walleij]

2010/7/22 Koul, Vinod <vinod.koul@xxxxxxxxx>:

> Shouldn't this patch remove the private member in dma_chan structure

I don't think so.

> Currently chan->private is used for sending slave or similar channel specific
> information. Now if we want to add struct dma_slave_config, then IMHO it
> would make sense to remove private variable and replace with dma_slave_config
> struture. That way we can reuse this struture there as well and if someone wants
> to add more stuff he can use the private_config.

That member is described like this:
@private: private data for certain client-channel associations

chan->private is supposed to only be used inside the dmaengine
drivers themselves AFAICT.

Some drivers (like the txx9dmac.c) use this to hold the state of the
slave, whereas the void *private in the runtime config is supposed
to contain some custom configuration struct which is created
outside of the dmaengine framework and can be discarded
after use (if you so wish), so that's vastly different.

You could however argue (but this is another discussion altogether)
that using chan->private to hold any states is superfluous since
you could just have  your struct dma_channel as a member of
your custom channel struct and use container_of to dereference
it (as we do in the coh901318 and ste_dma40 drivers) and then
stockpile any other struct members or substructs into your custom
struct but that's another issue, which would require major refactoring
of these drivers.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/