diff options
author | Lars-Peter Clausen <lars@metafoo.de> | 2014-02-16 08:21:22 -0500 |
---|---|---|
committer | Vinod Koul <vinod.koul@intel.com> | 2014-03-06 10:11:15 -0500 |
commit | 7cbccb55f04bef306bc2840185ec8f986bd0df3c (patch) | |
tree | f3c60556cc89e3d2b0611a6219fe5e42d05ad39b /include | |
parent | 7dedc002c0ec676590bf78ae8d76f2ffd51564f6 (diff) |
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: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Diffstat (limited to 'include')
-rw-r--r-- | include/linux/dmaengine.h | 14 |
1 files changed, 5 insertions, 9 deletions
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h index c5c92d59e531..8300fb87b84a 100644 --- a/include/linux/dmaengine.h +++ b/include/linux/dmaengine.h | |||
@@ -341,15 +341,11 @@ enum dma_slave_buswidth { | |||
341 | * and this struct will then be passed in as an argument to the | 341 | * and this struct will then be passed in as an argument to the |
342 | * DMA engine device_control() function. | 342 | * DMA engine device_control() function. |
343 | * | 343 | * |
344 | * The rationale for adding configuration information to this struct | 344 | * The rationale for adding configuration information to this struct is as |
345 | * is as follows: if it is likely that most DMA slave controllers in | 345 | * follows: if it is likely that more than one DMA slave controllers in |
346 | * the world will support the configuration option, then make it | 346 | * the world will support the configuration option, then make it generic. |
347 | * generic. If not: if it is fixed so that it be sent in static from | 347 | * If not: if it is fixed so that it be sent in static from the platform |
348 | * the platform data, then prefer to do that. Else, if it is neither | 348 | * data, then prefer to do that. |
349 | * fixed at runtime, nor generic enough (such as bus mastership on | ||
350 | * some CPU family and whatnot) then create a custom slave config | ||
351 | * struct and pass that, then make this config a member of that | ||
352 | * struct, if applicable. | ||
353 | */ | 349 | */ |
354 | struct dma_slave_config { | 350 | struct dma_slave_config { |
355 | enum dma_transfer_direction direction; | 351 | enum dma_transfer_direction direction; |