Re: [PATCH] mmc: sdhi: Convert from tasklet to BH workqueue

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Allen and Wolfram,

Nice work.

On 2024-06-26 10:48:21 +0200, Wolfram Sang wrote:
> From: Allen Pais <allen.lkml@xxxxxxxxx>
> 
> The only generic interface to execute asynchronously in the BH context is
> tasklet; however, it's marked deprecated and has some design flaws. To
> replace tasklets, BH workqueue support was recently added. A BH workqueue
> behaves similarly to regular workqueues except that the queued work items
> are executed in the BH context.
> 
> This patch converts the SDHI driver from tasklet to BH workqueue.
> 
> Based on the work done by Tejun Heo <tj@xxxxxxxxxx>
> 
> Signed-off-by: Allen Pais <allen.lkml@xxxxxxxxx>
> [wsa: fixed build faliures, corrected whitespace issues]
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>

> ---
> 
> Tested on a Renesas Salvator X board with a R-Car M3-W SoC. Same
> performance as with tasklets. Thank you Allen for your work!
> 
> 
>  drivers/mmc/host/renesas_sdhi.h               |  4 ++-
>  drivers/mmc/host/renesas_sdhi_core.c          |  2 ++
>  drivers/mmc/host/renesas_sdhi_internal_dmac.c | 26 +++++++++----------
>  drivers/mmc/host/renesas_sdhi_sys_dmac.c      |  9 +++----
>  drivers/mmc/host/tmio_mmc.h                   |  3 ++-
>  drivers/mmc/host/tmio_mmc_core.c              |  4 +--
>  6 files changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
> index 586f94d4dbfd..f12a87442338 100644
> --- a/drivers/mmc/host/renesas_sdhi.h
> +++ b/drivers/mmc/host/renesas_sdhi.h
> @@ -11,6 +11,7 @@
>  
>  #include <linux/dmaengine.h>
>  #include <linux/platform_device.h>
> +#include <linux/workqueue.h>
>  #include "tmio_mmc.h"
>  
>  struct renesas_sdhi_scc {
> @@ -67,7 +68,7 @@ struct renesas_sdhi_dma {
>  	dma_filter_fn filter;
>  	void (*enable)(struct tmio_mmc_host *host, bool enable);
>  	struct completion dma_dataend;
> -	struct tasklet_struct dma_complete;
> +	struct work_struct dma_complete;
>  };
>  
>  struct renesas_sdhi {
> @@ -93,6 +94,7 @@ struct renesas_sdhi {
>  	unsigned int tap_set;
>  
>  	struct reset_control *rstc;
> +	struct tmio_mmc_host *host;
>  };
>  
>  #define host_to_priv(host) \
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index 58536626e6c5..04874791541f 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -970,6 +970,8 @@ int renesas_sdhi_probe(struct platform_device *pdev,
>  	if (IS_ERR(host))
>  		return PTR_ERR(host);
>  
> +	priv->host = host;
> +
>  	if (of_data) {
>  		mmc_data->flags |= of_data->tmio_flags;
>  		mmc_data->ocr_mask = of_data->tmio_ocr_mask;
> diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> index 422fa63a2e99..d4b66daeda66 100644
> --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> @@ -337,7 +337,7 @@ static bool renesas_sdhi_internal_dmac_dma_irq(struct tmio_mmc_host *host)
>  		writel(status ^ dma_irqs, host->ctl + DM_CM_INFO1);
>  		set_bit(SDHI_DMA_END_FLAG_DMA, &dma_priv->end_flags);
>  		if (test_bit(SDHI_DMA_END_FLAG_ACCESS, &dma_priv->end_flags))
> -			tasklet_schedule(&dma_priv->dma_complete);
> +			queue_work(system_bh_wq, &dma_priv->dma_complete);
>  	}
>  
>  	return status & dma_irqs;
> @@ -352,7 +352,7 @@ renesas_sdhi_internal_dmac_dataend_dma(struct tmio_mmc_host *host)
>  	set_bit(SDHI_DMA_END_FLAG_ACCESS, &dma_priv->end_flags);
>  	if (test_bit(SDHI_DMA_END_FLAG_DMA, &dma_priv->end_flags) ||
>  	    host->data->error)
> -		tasklet_schedule(&dma_priv->dma_complete);
> +		queue_work(system_bh_wq, &dma_priv->dma_complete);
>  }
>  
>  /*
> @@ -440,9 +440,9 @@ renesas_sdhi_internal_dmac_start_dma(struct tmio_mmc_host *host,
>  	renesas_sdhi_internal_dmac_enable_dma(host, false);
>  }
>  
> -static void renesas_sdhi_internal_dmac_issue_tasklet_fn(unsigned long arg)
> +static void renesas_sdhi_internal_dmac_issue_work_fn(struct work_struct *work)
>  {
> -	struct tmio_mmc_host *host = (struct tmio_mmc_host *)arg;
> +	struct tmio_mmc_host *host = from_work(host, work, dma_issue);
>  	struct renesas_sdhi *priv = host_to_priv(host);
>  
>  	tmio_mmc_enable_mmc_irqs(host, TMIO_STAT_DATAEND);
> @@ -454,7 +454,7 @@ static void renesas_sdhi_internal_dmac_issue_tasklet_fn(unsigned long arg)
>  		/* on CMD errors, simulate DMA end immediately */
>  		set_bit(SDHI_DMA_END_FLAG_DMA, &priv->dma_priv.end_flags);
>  		if (test_bit(SDHI_DMA_END_FLAG_ACCESS, &priv->dma_priv.end_flags))
> -			tasklet_schedule(&priv->dma_priv.dma_complete);
> +			queue_work(system_bh_wq, &priv->dma_priv.dma_complete);
>  	}
>  }
>  
> @@ -484,9 +484,11 @@ static bool renesas_sdhi_internal_dmac_complete(struct tmio_mmc_host *host)
>  	return true;
>  }
>  
> -static void renesas_sdhi_internal_dmac_complete_tasklet_fn(unsigned long arg)
> +static void renesas_sdhi_internal_dmac_complete_work_fn(struct work_struct *work)
>  {
> -	struct tmio_mmc_host *host = (struct tmio_mmc_host *)arg;
> +	struct renesas_sdhi_dma *dma_priv = from_work(dma_priv, work, dma_complete);
> +	struct renesas_sdhi *priv = container_of(dma_priv, typeof(*priv), dma_priv);
> +	struct tmio_mmc_host *host = priv->host;
>  
>  	spin_lock_irq(&host->lock);
>  	if (!renesas_sdhi_internal_dmac_complete(host))
> @@ -544,12 +546,10 @@ renesas_sdhi_internal_dmac_request_dma(struct tmio_mmc_host *host,
>  	/* Each value is set to non-zero to assume "enabling" each DMA */
>  	host->chan_rx = host->chan_tx = (void *)0xdeadbeaf;
>  
> -	tasklet_init(&priv->dma_priv.dma_complete,
> -		     renesas_sdhi_internal_dmac_complete_tasklet_fn,
> -		     (unsigned long)host);
> -	tasklet_init(&host->dma_issue,
> -		     renesas_sdhi_internal_dmac_issue_tasklet_fn,
> -		     (unsigned long)host);
> +	INIT_WORK(&priv->dma_priv.dma_complete,
> +		  renesas_sdhi_internal_dmac_complete_work_fn);
> +	INIT_WORK(&host->dma_issue,
> +		  renesas_sdhi_internal_dmac_issue_work_fn);
>  
>  	/* Add pre_req and post_req */
>  	host->ops.pre_req = renesas_sdhi_internal_dmac_pre_req;
> diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> index 9cf7f9feab72..5a6f41318645 100644
> --- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> +++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> @@ -312,9 +312,9 @@ static void renesas_sdhi_sys_dmac_start_dma(struct tmio_mmc_host *host,
>  	}
>  }
>  
> -static void renesas_sdhi_sys_dmac_issue_tasklet_fn(unsigned long priv)
> +static void renesas_sdhi_sys_dmac_issue_work_fn(struct work_struct *work)
>  {
> -	struct tmio_mmc_host *host = (struct tmio_mmc_host *)priv;
> +	struct tmio_mmc_host *host = from_work(host, work, dma_issue);
>  	struct dma_chan *chan = NULL;
>  
>  	spin_lock_irq(&host->lock);
> @@ -401,9 +401,8 @@ static void renesas_sdhi_sys_dmac_request_dma(struct tmio_mmc_host *host,
>  			goto ebouncebuf;
>  
>  		init_completion(&priv->dma_priv.dma_dataend);
> -		tasklet_init(&host->dma_issue,
> -			     renesas_sdhi_sys_dmac_issue_tasklet_fn,
> -			     (unsigned long)host);
> +		INIT_WORK(&host->dma_issue,
> +			  renesas_sdhi_sys_dmac_issue_work_fn);
>  	}
>  
>  	renesas_sdhi_sys_dmac_enable_dma(host, true);
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 2af5730c21f4..a75755f31d31 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -21,6 +21,7 @@
>  #include <linux/scatterlist.h>
>  #include <linux/spinlock.h>
>  #include <linux/interrupt.h>
> +#include <linux/workqueue.h>
>  
>  #define CTL_SD_CMD 0x00
>  #define CTL_ARG_REG 0x04
> @@ -153,7 +154,7 @@ struct tmio_mmc_host {
>  	bool			dma_on;
>  	struct dma_chan		*chan_rx;
>  	struct dma_chan		*chan_tx;
> -	struct tasklet_struct	dma_issue;
> +	struct work_struct	dma_issue;
>  	struct scatterlist	bounce_sg;
>  	u8			*bounce_buf;
>  
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index 2780f0a29871..b61a6310311d 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -608,7 +608,7 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host, unsigned int stat)
>  			} else {
>  				tmio_mmc_disable_mmc_irqs(host,
>  							  TMIO_MASK_READOP);
> -				tasklet_schedule(&host->dma_issue);
> +				queue_work(system_bh_wq, &host->dma_issue);
>  			}
>  		} else {
>  			if (!host->dma_on) {
> @@ -616,7 +616,7 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host, unsigned int stat)
>  			} else {
>  				tmio_mmc_disable_mmc_irqs(host,
>  							  TMIO_MASK_WRITEOP);
> -				tasklet_schedule(&host->dma_issue);
> +				queue_work(system_bh_wq, &host->dma_issue);
>  			}
>  		}
>  	} else {
> -- 
> 2.43.0
> 
> 

-- 
Kind Regards,
Niklas Söderlund




[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux