aboutsummaryrefslogtreecommitdiffstats
path: root/include
diff options
context:
space:
mode:
authorLars-Peter Clausen <lars@metafoo.de>2014-02-16 08:21:22 -0500
committerVinod Koul <vinod.koul@intel.com>2014-03-06 10:11:15 -0500
commit7cbccb55f04bef306bc2840185ec8f986bd0df3c (patch)
treef3c60556cc89e3d2b0611a6219fe5e42d05ad39b /include
parent7dedc002c0ec676590bf78ae8d76f2ffd51564f6 (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.h14
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 */
354struct dma_slave_config { 350struct dma_slave_config {
355 enum dma_transfer_direction direction; 351 enum dma_transfer_direction direction;