Commit 7cbccb55 authored by Lars-Peter Clausen's avatar Lars-Peter Clausen Committed by Vinod Koul

dma: Remove comment about embedding dma_slave_config into custom structs

The documentation for the dma_slave_config struct recommends that if a DMA
controller has special configuration options, which can not be configured
through the dma_slave_config struct, the driver should create its own custom
config struct and embed the dma_slave_config struct in it and pass the custom
config struct to dmaengine_slave_config(). This overloads the generic
dmaengine_slave_config() API with custom semantics and any caller of the
dmaengine_slave_config() that is not aware of these special semantics will cause
undefined behavior. This means that it is impossible for generic code to make
use of dmaengine_slave_config(). Such a restriction contradicts the very idea of
having a generic API.

E.g. consider the following case of a DMA controller that has an option to
reverse the field polarity of the DMA transfer with the following implementation
for setting the configuration:

	struct my_slave_config {
		struct dma_slave_config config;
		unsigned int field_polarity;
	};

	static int my_dma_controller_slave_config(struct dma_chan *chan,
		struct dma_slave_config *config)
	{
		struct my_slave_config *my_cfg = container_of(config,
				struct my_slave_config, config);

		...
		my_dma_set_field_polarity(chan, my_cfg->field_polarity);
		...
	}

Now a generic user of the dmaengine API might want to configure a DMA channel
for this DMA controller that it obtained using the following code:

	struct dma_slave_config config;

	config.src_addr = ...;
	...
	dmaengine_slave_config(chan, &config);

The call to dmaengine_slave_config() will eventually call into
my_dma_controller_slave_config() which will cast from dma_slave_config to
my_slave_config and then tries to access the field_polarity member. Since the
dma_slave_config struct that was passed in was never embedded into a
my_slave_config struct this attempt will just read random stack garbage and use
that to configure the DMA controller. This is bad. Instead, if a DMA controller
really needs to have custom configuration options, the driver should create a
custom API for it. This makes it very clear that there is a direct dependency
of a user of such an API and the implementer. E.g.:

	int my_dma_set_field_polarity(struct dma_chan *chan,
		unsigned int field_polarity) {
		if (chan->device->dev->driver != &my_dma_controller_driver.driver)
			return -EINVAL;
		...
	}
	EXPORT_SYMBOL_GPL(my_dma_set_field_polarity);
Signed-off-by: default avatarLars-Peter Clausen <lars@metafoo.de>
Signed-off-by: default avatarVinod Koul <vinod.koul@intel.com>
parent 7dedc002
...@@ -341,15 +341,11 @@ enum dma_slave_buswidth { ...@@ -341,15 +341,11 @@ enum dma_slave_buswidth {
* and this struct will then be passed in as an argument to the * and this struct will then be passed in as an argument to the
* DMA engine device_control() function. * DMA engine device_control() function.
* *
* The rationale for adding configuration information to this struct * The rationale for adding configuration information to this struct is as
* is as follows: if it is likely that most DMA slave controllers in * follows: if it is likely that more than one DMA slave controllers in
* the world will support the configuration option, then make it * the world will support the configuration option, then make it generic.
* generic. If not: if it is fixed so that it be sent in static from * If not: if it is fixed so that it be sent in static from the platform
* the platform data, then prefer to do that. Else, if it is neither * data, then prefer to do that.
* fixed at runtime, nor generic enough (such as bus mastership on
* some CPU family and whatnot) then create a custom slave config
* struct and pass that, then make this config a member of that
* struct, if applicable.
*/ */
struct dma_slave_config { struct dma_slave_config {
enum dma_transfer_direction direction; enum dma_transfer_direction direction;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment