s3cmci: port DMA code to dmaengine API
PATCH 1/1 s3cmci: port DMA code to dmaengine API
Utilise new s3c24xx-dma dmaengine driver for DMA ops.
--- drivers/mmc/host/Kconfig | 9 --- drivers/mmc/host/s3cmci.c | 183 +++++++++++++++------------------------------- drivers/mmc/host/s3cmci.h | 6 +- 3 files changed, 61 insertions(+), 137 deletions(-)
drivers/mmc/host/Kconfig +0 -9
@@ -457,15 +457,6 @@ config MMC_S3C_DMA working properly and needs to be debugged before this option is useful. -config MMC_S3C_PIODMA - bool "Support for both PIO and DMA" - help - Compile both the PIO and DMA transfer routines into the - driver and let the platform select at run-time which one - is best. - - See notes for the DMA option. - endchoice config MMC_SDRICOH_CS
drivers/mmc/host/s3cmci.c +58 -125
291 lines not on this page · raw message
drivers/mmc/host/s3cmci.h +3 -3
@@ -26,7 +26,9 @@ struct s3cmci_host {void __iomem *base;int irq;int irq_cd; -int dma; +struct dma_chan *dma; +struct dma_async_tx_descriptor + *desc;unsigned long clk_rate;unsigned long clk_div;
@@ -36,8 +38,6 @@ struct s3cmci_host {int is2440;unsigned sdiimsk;unsigned sdidata; -int dodma; -int dmatogo;bool irq_disabled;bool irq_enabled;
Signature · 1 line
1.9.3
PATCH 1/1 s3cmci: port DMA code to dmaengine API
Quoted 3 lines · On Monday 19 May 2014 21:03:13 Vasily Khoruzhick wrote:
Utilise new s3c24xx-dma dmaengine driver for DMA ops.
Signed-off-by Vasily Khoruzhick <[email protected]>
Very nice!
I had done a similar conversion earlier but not tested and only posted in a reply to another thread. I'd assume that your version is better and that you have actually tested it, but just for reference, here is what I had. Maybe it helps you to take a look at my version to see if there is something you missed.
Here are the differences I found:
- I don't save the 'desc' pointer as you do, but I don't see it used anywhere.
- I did not remove MMC_S3C_PIODMA, but I don't see anything wrong with yours there
- the slave config setup is slightly different
- I hardcode the DMA request number, while you pass it as a resource. Actually both are wrong I guess, it should be platform data instead.
Arnd
drivers/mmc/host/s3cmci.c +49 -82
@@ -14,6 +14,7 @@#include <linux/module.h> #include <linux/dma-mapping.h> #include <linux/clk.h> +#include <linux/dmaengine.h> #include <linux/mmc/host.h> #include <linux/platform_device.h> #include <linux/cpufreq.h>
@@ -28,6 +29,7 @@#include <mach/gpio-samsung.h> #include <linux/platform_data/mmc-s3cmci.h> +#include <linux/platform_data/dma-s3c24xx.h> #include "s3cmci.h"
@@ -140,10 +142,6 @@ static const int dbgmap_debug = dbg_err | dbg_debug;dev_dbg (&host->pdev->dev, args); \ }while (0 ) -static struct s3c2410_dma_client s3cmci_dma_client = { - .name ="s3c-mci" , -}; - static void finalize_request(struct s3cmci_host *host);static void s3cmci_send_request (struct mmc_host *mmc);static void s3cmci_reset (struct s3cmci_host *host);
@@ -841,9 +839,7 @@ static irqreturn_t s3cmci_irq_cd(int irq, void *dev_id)return IRQ_HANDLED ; } -static void s3cmci_dma_done_callback (struct s3c2410_dma_chan *dma_ch, -void *buf_id,int size, -enum s3c2410_dma_buffresult result) +static void s3cmci_dma_done_callback (void *buf_id) {struct s3cmci_host *host = buf_id;unsigned long iflags;
@@ -856,45 +852,18 @@ static void s3cmci_dma_done_callback(struct s3c2410_dma_chan *dma_ch,BUG_ON (!host->mrq);BUG_ON (!host->mrq->data); -BUG_ON (!host->dmatogo);spin_lock_irqsave (&host->complete_lock, iflags); -if (result !=S3C2410_RES_OK ) { -dbg (host, dbg_fail,"DMA FAILED: csta=0x%08x dsta=0x%08x " -"fsta=0x%08x dcnt:0x%08x result:0x%08x toGo:%u\n" , - mci_csta, mci_dsta, mci_fsta, - mci_dcnt, result, host->dmatogo); - - goto fail_request; - } - - host->dmatogo--; -if (host->dmatogo) { -dbg (host, dbg_dma,"DMA DONE Size:%i DSTA:[%08x] " -"DCNT:[%08x] toGo:%u\n" , - size, mci_dsta, mci_dcnt, host->dmatogo); - - goto out; - } - -dbg (host, dbg_dma,"DMA FINISHED Size:%i DSTA:%08x DCNT:%08x\n" , - size, mci_dsta, mci_dcnt); +dbg (host, dbg_dma,"DMA FINISHED DSTA:%08x DCNT:%08x\n" , + mci_dsta, mci_dcnt); host->dma_complete =1 ; host->complete_what =COMPLETION_FINALIZE ; -out:tasklet_schedule (&host->pio_tasklet);spin_unlock_irqrestore (&host->complete_lock, iflags);return ; - -fail_request: - host->mrq->data->error = -EINVAL ; - host->complete_what =COMPLETION_FINALIZE ; -clear_imask (host); - - goto out; }static void finalize_request (struct s3cmci_host *host)
@@ -966,7 +935,7 @@ static void finalize_request(struct s3cmci_host *host)* DMA channel and the fifo to clear out any garbage. */ if (mrq->data->error !=0 ) {if (s3cmci_host_usedma (host)) -s3c2410_dma_ctrl (host->dma,S3C2410_DMAOP_FLUSH ); +dmaengine_terminate_all (host->dma);if (host->is2440) {/* Clear failure register and reset fifo. */
@@ -993,26 +962,27 @@ request_done: }static void s3cmci_dma_setup (struct s3cmci_host *host, -enum dma_data_direction source) +enum dma_transfer_direction source) { -static enum dma_data_direction last_source =-1 ; -static int setup_ok; +static enum dma_transfer_direction last_source =-1 ; +struct dma_slave_config slave_config;if (last_source == source)return ; - last_source = source; - -s3c2410_dma_devconfig (host->dma, source, - host->mem->start + host->sdidata); - -if (!setup_ok) { -s3c2410_dma_config (host->dma,4 ); -s3c2410_dma_set_buffdone_fn (host->dma, - s3cmci_dma_done_callback); -s3c2410_dma_setflags (host->dma,S3C2410_DMAF_AUTOSTART ); - setup_ok =1 ; +memset (&slave_config,0 ,sizeof (struct dma_slave_config )); + slave_config.direction = source; +if (source ==DMA_DEV_TO_MEM ) { + slave_config.src_addr = host->mem->start + host->sdidata; + slave_config.src_addr_width =4 ; + slave_config.src_maxburst =1 ; + }else { + slave_config.dst_addr = host->mem->start + host->sdidata; + slave_config.dst_addr_width =4 ; + slave_config.dst_maxburst =1 ; } + last_source = source; +dmaengine_slave_config (host->dma, &slave_config); } static void s3cmci_send_command(struct s3cmci_host *host,
@@ -1162,41 +1132,33 @@ static int s3cmci_prepare_pio(struct s3cmci_host *host, struct mmc_data *data)static int s3cmci_prepare_dma (struct s3cmci_host *host,struct mmc_data *data) { -int dma_len, i; -int rw = data->flags &MMC_DATA_WRITE ; +int dma_len; +int dir = data->flags &MMC_DATA_WRITE ?DMA_TO_DEVICE :DMA_FROM_DEVICE ; +struct dma_async_tx_descriptor *desc;BUG_ON ((data->flags &BOTH_DIR ) ==BOTH_DIR ); -s3cmci_dma_setup (host, rw ?DMA_TO_DEVICE :DMA_FROM_DEVICE ); -s3c2410_dma_ctrl (host->dma,S3C2410_DMAOP_FLUSH ); - - dma_len =dma_map_sg (mmc_dev (host->mmc), data->sg, data->sg_len, - rw ?DMA_TO_DEVICE :DMA_FROM_DEVICE ); +s3cmci_dma_setup (host, dir); + dma_len =dma_map_sg (mmc_dev (host->mmc), data->sg, data->sg_len, dir);if (dma_len ==0 )return -ENOMEM ; - host->dma_complete =0 ; - host->dmatogo = dma_len; - -for (i =0 ; i < dma_len; i++) { -int res; + desc =dmaengine_prep_slave_sg (host->dma, data->sg, dma_len, + dir,DMA_CTRL_ACK ); -dbg (host, dbg_dma,"enqueue %i: %08x@%u\n" , i, -sg_dma_address (&data->sg[i]), -sg_dma_len (&data->sg[i])); +if (!desc) { +dma_unmap_sg (mmc_dev (host->mmc), data->sg, dma_len, dir); +return -EIO ; + } - res =s3c2410_dma_enqueue (host->dma, host, -sg_dma_address (&data->sg[i]), -sg_dma_len (&data->sg[i])); + host->dma_complete =0 ; -if (res) { -s3c2410_dma_ctrl (host->dma,S3C2410_DMAOP_FLUSH ); -return -EBUSY ; - } - } + desc->callback = s3cmci_dma_done_callback; + desc->callback_param = host; -s3c2410_dma_ctrl (host->dma,S3C2410_DMAOP_START ); +dmaengine_submit (desc); +dma_async_issue_pending (host->dma);return 0 ; }
@@ -1765,9 +1727,14 @@ static int s3cmci_probe(struct platform_device *pdev)/* depending on the dma state, get a dma channel to use. */ if (s3cmci_host_usedma (host)) { - host->dma =s3c2410_dma_request (DMACH_SDI , &s3cmci_dma_client, - host); -if (host->dma <0 ) { +dma_cap_mask_t mask; + +dma_cap_zero (mask); +dma_cap_set (DMA_SLAVE , mask); + + host->dma =dma_request_channel (mask, s3c24xx_dma_filter, (void *)DMACH_SDI ); + +if (!host->dma) {dev_err (&pdev->dev,"cannot get DMA channel.\n" );if (!s3cmci_host_canpio ()) { ret = -EBUSY ;
@@ -1816,9 +1783,9 @@ static int s3cmci_probe(struct platform_device *pdev) mmc->max_segs =128 ;dbg (host, dbg_debug, -"probe: mode:%s mapped mci_base:%p irq:%u irq_cd:%u dma:%u.\n" , +"probe: mode:%s mapped mci_base:%p irq:%u irq_cd:%u.\n" , (host->is2440?"2440" :"" ), - host->base, host->irq, host->irq_cd, host->dma); + host->base, host->irq, host->irq_cd); ret =s3cmci_cpufreq_register (host);if (ret) {
@@ -1852,7 +1819,7 @@ static int s3cmci_probe(struct platform_device *pdev) probe_free_dma:if (s3cmci_host_usedma (host)) -s3c2410_dma_free (host->dma, &s3cmci_dma_client); +dma_release_channel (host->dma); probe_free_gpio_wp:if (!host->pdata->no_wprotect)
@@ -1914,7 +1881,7 @@ static int s3cmci_remove(struct platform_device *pdev)tasklet_disable (&host->pio_tasklet);if (s3cmci_host_usedma (host)) -s3c2410_dma_free (host->dma, &s3cmci_dma_client); +dma_release_channel (host->dma);free_irq (host->irq, host);
drivers/mmc/host/s3cmci.h +1 -1
@@ -26,7 +26,7 @@ struct s3cmci_host {void __iomem *base;int irq;int irq_cd; -int dma; +struct dma_chan *dma;unsigned long clk_rate;unsigned long clk_div;
PATCH 1/1 s3cmci: port DMA code to dmaengine API
Quoted 4 lines · Arnd Bergmann
Quoted 3 lines · On Monday 19 May 2014 21:03:13 Vasily Khoruzhick wrote:
Utilise new s3c24xx-dma dmaengine driver for DMA ops.
Signed-off-by Vasily Khoruzhick <[email protected]>
Hi Arnd,
Quoted 1 line
Very nice!
Thanks! :)
Quoted 5 lines
I had done a similar conversion earlier but not tested and only posted in a reply to another thread. I'd assume that your version is better and that you have actually tested it, but just for reference, here is what I had. Maybe it helps you to take a look at my version to see if there is something you missed.
Hm, I did search for s3c24xx-i2s conversion, but didn't for s3cmci, my fault. Your version looks pretty close to mine, however I don't think that much variations
are possible here. I've tested mine on s3c2410 machine.
Quoted 4 lines
Here are the differences I found:
- I don't save the 'desc' pointer as you do, but I don't see it used anywhere.
Yeah, I should remove it. Will send v2 shortly.
Quoted 2 lines
- I did not remove
MMC_S3C_PIODMA, but I don't see anything wrong with yours there
It didn't work even with legacy code for me. I don't want to fix it, so it's just dropped.
Anyway, I don't see much sense in PIODMA mode.
Quoted 1 line
- the slave config setup is slightly different
Yep.
Quoted 2 lines
- I hardcode the DMA request number, while you pass it as a resource. Actually both are wrong I guess, it should be platform data instead.
Well, at least s3c64xx uses resources for passing dma channel number, and so did I.
Btw, It'll make transition to device tree a bit easier.
There's one more difference: original s3cmci doesn't restore prescaler value when necessary on s3c2410,
my version has a fix for it.
Quoted 1 line
Arnd
Regards
Vasily
PATCH 1/1 s3cmci: port DMA code to dmaengine API
Quoted 4 lines · On Monday 19 May 2014 21:54:28 Vasily Khoruzhick wrote:
Quoted 3 lines · Arnd Bergmann
Quoted 2 lines · On Monday 19 May 2014 21:03:13 Vasily Khoruzhick wrote:
Utilise new s3c24xx-dma dmaengine driver for DMA ops.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Vasily Khoruzhick <[email protected]>- I did not remove
MMC_S3C_PIODMA, but I don't see anything wrong with yours thereIt didn't work even with legacy code for me. I don't want to fix it, so it's just dropped.
Anyway, I don't see much sense in PIODMA mode.
Ah, that makes sense.
Quoted 10 lines
Quoted 1 line
- the slave config setup is slightly different
Yep.
Quoted 2 lines
- I hardcode the DMA request number, while you pass it as a resource. Actually both are wrong I guess, it should be platform data instead.
Well, at least s3c64xx uses resources for passing dma channel number, and so did I.
Btw, It'll make transition to device tree a bit easier.
Actually it doesn't help with the transition to DT at all, because DT does not use resources for DMA requests. Instead there is a dma_request_slave_channel() interface that gets passed a string to match the DT descriptor of the DMA channel. If you want to enable DT probing while keeping the traditional way alive, you can use dma_request_slave_channel_compat(), although I find it not much easier than open-coding it.
There is one issue with dma_request_slave_channel_compat and dma_request_channel, which is that you need a pointer to the filter function. In portable drivers that pointer should get passed in platform_data for the non-DT case, along with the data it needs, because there are dma engines that need more than just an integer to identify a slave.
For this driver, you don't have to go that far, as long as it's ensure that the pointer to the filter function is available to the driver, i.e. you can't have a built-in s3mci driver when the dmaengine driver is a loadable module.
Quoted 3 lines
There's one more difference: original s3cmci doesn't restore prescaler value when necessary on s3c2410,
my version has a fix for it.
Ok.
Arnd
PATCH 1/1 s3cmci: port DMA code to dmaengine API
Quoted 19 lines · Arnd Bergmann
Quoted 3 lines
Well, at least s3c64xx uses resources for passing dma channel number, and so did I.
Btw, It'll make transition to device tree a bit easier.Actually it doesn't help with the transition to DT at all, because DT does not use resources for DMA requests. Instead there is a
dma_request_slave_channel()interface that gets passed a string to match the DT descriptor of the DMA channel. If you want to enable DT probing while keeping the traditional way alive, you can usedma_request_slave_channel_compat(), although I find it not much easier than open-coding it.There is one issue with
dma_request_slave_channel_compatanddma_request_channel, which is that you need a pointer to the filter function. In portable drivers that pointer should get passed inplatform_datafor the non-DT case, along with the data it needs, because there are dma engines that need more than just an integer to identify a slave.
Quoted 4 lines
For this driver, you don't have to go that far, as long as it's ensure that the pointer to the filter function is available to the driver, i.e. you can't have a built-in s3mci driver when the dmaengine driver is a loadable module.
I'll take a look at dma_request_slave_channel_compat(), thanks for a hint!
Regards,
Vasily
PATCH 1/1 s3cmci: port DMA code to dmaengine API
In reply to Arnd BergmannHi Arnd,
Quoted 4 lines · Arnd Bergmann
For this driver, you don't have to go that far, as long as it's ensure that the pointer to the filter function is available to the driver, i.e. you can't have a built-in s3mci driver when the dmaengine driver is a loadable module.
OK, so I guess I need to add explicit dependency on CONFIG_S3C24XX_DMAC for s3cmci driver.
Btw, I didn't understand if it's acceptable to pass DMA channel number through DMA resource for non-DT case or not?
Regards
Vasily
PATCH 1/1 s3cmci: port DMA code to dmaengine API
Quoted 8 lines · On Tuesday 20 May 2014 13:22:35 Vasily Khoruzhick wrote:
Hi Arnd,
Quoted 4 lines · Arnd Bergmann
For this driver, you don't have to go that far, as long as it's ensure that the pointer to the filter function is available to the driver, i.e. you can't have a built-in s3mci driver when the dmaengine driver is a loadable module.
Quoted 5 lines
OK, so I guess I need to add explicit dependency on
CONFIG_S3C24XX_DMACfor s3cmci driver.Btw, I didn't understand if it's acceptable to pass DMA channel number through DMA resource for non-DT case or not?
It's commonly done on certain SoCs, but I'd prefer to not start doing it on those that don't do it today.
At the moment, we do it only on s3c64xx, s5p, davinci, omap1, and pxa on ARM, as well as arch/blackfin and one MIPS machine.
I suppose we have to introduce it on s3c24xx in order to keep supporting sound, unless we put the audio dma channels into s3c_audio_pdata.
I think that would be a better approach, given that you also need to put the filter function pointer for the audio stuff somewhere.
Maybe Mark Brown has a strong preference to how he wants this done in the audio drivers, then you can do it the same way for s3cmci.
Arnd
PATCH 1/1 s3cmci: port DMA code to dmaengine API
Quoted 24 lines · Arnd Bergmann
Quoted 8 lines · On Tuesday 20 May 2014 13:22:35 Vasily Khoruzhick wrote:
Hi Arnd,
Quoted 4 lines · Arnd Bergmann
For this driver, you don't have to go that far, as long as it's ensure that the pointer to the filter function is available to the driver, i.e. you can't have a built-in s3mci driver when the dmaengine driver is a loadable module.
Quoted 5 lines
OK, so I guess I need to add explicit dependency on
CONFIG_S3C24XX_DMACfor s3cmci driver.Btw, I didn't understand if it's acceptable to pass DMA channel number through DMA resource for non-DT case or not?
It's commonly done on certain SoCs, but I'd prefer to not start doing it on those that don't do it today.
At the moment, we do it only on s3c64xx, s5p, davinci, omap1, and pxa on ARM, as well as
arch/blackfinand one MIPS machine.
I suppose we have to introduce it on s3c24xx in order to keep supporting sound, unless we put the audio dma channels intos3c_audio_pdata.
Quoted 5 lines
I think that would be a better approach, given that you also need to put the filter function pointer for the audio stuff somewhere.
Maybe Mark Brown has a strong preference to how he wants this done in the audio drivers, then you can do it the same way for s3cmci.
Let's just leave channel number hardcoded, how it was before conversion. It doesn't seem to be a good idea to clean non-DT machine code now, since it's better to put some effort into converting s3c2410 and s3c244{0,2} to DT.
Regards
Vasily
Quoted 2 lines
Arnd
PATCH 1/1 s3cmci: port DMA code to dmaengine API
Quoted 28 lines · On Tuesday 20 May 2014 14:36:49 Vasily Khoruzhick wrote:
Quoted 27 lines · Arnd Bergmann
Quoted 8 lines · On Tuesday 20 May 2014 13:22:35 Vasily Khoruzhick wrote:
Hi Arnd,
Quoted 4 lines · Arnd Bergmann
For this driver, you don't have to go that far, as long as it's ensure that the pointer to the filter function is available to the driver, i.e. you can't have a built-in s3mci driver when the dmaengine driver is a loadable module.
Quoted 5 lines
OK, so I guess I need to add explicit dependency on
CONFIG_S3C24XX_DMACfor s3cmci driver.Btw, I didn't understand if it's acceptable to pass DMA channel number through DMA resource for non-DT case or not?
It's commonly done on certain SoCs, but I'd prefer to not start doing it on those that don't do it today.
At the moment, we do it only on s3c64xx, s5p, davinci, omap1, and pxa on ARM, as well as
arch/blackfinand one MIPS machine.
I suppose we have to introduce it on s3c24xx in order to keep supporting sound, unless we put the audio dma channels intos3c_audio_pdata.I think that would be a better approach, given that you also need to put the filter function pointer for the audio stuff somewhere.
Quoted 6 lines
Quoted 2 lines
Maybe Mark Brown has a strong preference to how he wants this done in the audio drivers, then you can do it the same way for s3cmci.
Let's just leave channel number hardcoded, how it was before conversion. It doesn't seem to be a good idea to clean non-DT machine code now, since it's better to put some effort into converting s3c2410 and s3c244{0,2} to DT.
Ok, fair enough.
I'm actually more interested in making s3c24xx multiplatform capable than moving it to DT, but we that requires a few other patches as well, and we can fix this one along with those.
Arnd
PATCH 1/1 s3cmci: port DMA code to dmaengine API
Quoted 9 lines · Arnd Bergmann
Quoted 3 lines
Let's just leave channel number hardcoded, how it was before conversion. It doesn't seem to be a good idea to clean non-DT machine code now, since it's better to put some effort into converting s3c2410 and s3c244{0,2} to DT.
Ok, fair enough.
I'm actually more interested in making s3c24xx multiplatform capable than moving it to DT, but we that requires a few other patches as well, and we can fix this one along with those.
Hm, but s3c2410, s3c2440 and s3c2442 are armv4t. Is it possible at all to make them multiplatform?
Regards
Vasily
Quoted 1 line
Arnd
PATCH 1/1 s3cmci: port DMA code to dmaengine API
Quoted 5 lines · On Tuesday 20 May 2014 14:55:40 Vasily Khoruzhick wrote:
Quoted 4 lines · Arnd Bergmann
Quoted 3 lines
Let's just leave channel number hardcoded, how it was before conversion. It doesn't seem to be a good idea to clean non-DT machine code now, since it's better to put some effort into converting s3c2410 and s3c244{0,2} to DT.
Quoted 8 lines
Quoted 5 lines
Ok, fair enough.
I'm actually more interested in making s3c24xx multiplatform capable than moving it to DT, but we that requires a few other patches as well, and we can fix this one along with those.
Hm, but s3c2410, s3c2440 and s3c2442 are armv4t. Is it possible at all to make them multiplatform?
Yes, of course. We can run any combination of armv4, armv4t and armv5 CPUs in a multiplatform, we just can't combine them with armv6, armv6k or armv7.
There is an interesting dependency here: for armv6/v7, we have multiple Samsung platforms that are mutually exclusive as long as they use ATAGS based boot: s3c64xx, s5p64xx, s5pc100, s5pv210 and exynos, because the plat-samsung directory uses mutually exclusive options to pick which set of mach/*.h headers it uses. The plan here is to move s5pv210 to DT (same as Exynos already is) and delete s5p64xx and s5pc100, which don't seem to be used by anybody really. Then all that is left is s3c64xx using board files and all the non-DT code from plat-samsung becomes s3c64xx-only.
For s3c24xx, we don't have that problem at all, we can just build plat-samsung with the s3c24xx headers into the same kernel as things like mxs, imx1/2, kirkwood, orion, or versatile.
Arnd