[PATCH] mmc: core: Remove redundant rescan_disable flag

[PATCH] mmc: core: Remove redundant rescan_disable flag

[Ulf Hansson]

As cancel_delayed_work_sync() prevents an executing work for re-scheduling
itself, the rescan_disable flag is redundant.

Morover, during host registration phase, it's anyway not safe to schedule a
detect work until mmc_start_host() has been invoked. Therfore, there's no
need to use the rescan_disable flag to protect a rescan work from being
executed in-between mmc_alloc_host() and mmc_start_host().

For these reasons, let's remove the rescan_disable flag.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/core.c  | 12 ------------
 drivers/mmc/core/host.c  |  3 ---
 include/linux/mmc/host.h |  1 -
 3 files changed, 16 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 99275e4..b8c6a11 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2580,9 +2580,6 @@ void mmc_rescan(struct work_struct *work)
 		container_of(work, struct mmc_host, detect.work);
 	int i;
 
-	if (host->rescan_disable)
-		return;
-
 	/* If there is a non-removable card registered, only scan once */
 	if (!mmc_card_is_removable(host) && host->rescan_entered)
 		return;
@@ -2649,7 +2646,6 @@ void mmc_rescan(struct work_struct *work)
 void mmc_start_host(struct mmc_host *host)
 {
 	host->f_init = max(freqs[0], host->f_min);
-	host->rescan_disable = 0;
 	host->ios.power_mode = MMC_POWER_UNDEFINED;
 
 	mmc_claim_host(host);
@@ -2674,7 +2670,6 @@ void mmc_stop_host(struct mmc_host *host)
 	if (host->slot.cd_irq >= 0)
 		disable_irq(host->slot.cd_irq);
 
-	host->rescan_disable = 1;
 	cancel_delayed_work_sync(&host->detect);
 
 	/* clear pm flags now and let card drivers set them as needed */
@@ -2788,9 +2783,6 @@ static int mmc_pm_notify(struct notifier_block *notify_block,
 	case PM_HIBERNATION_PREPARE:
 	case PM_SUSPEND_PREPARE:
 	case PM_RESTORE_PREPARE:
-		spin_lock_irqsave(&host->lock, flags);
-		host->rescan_disable = 1;
-		spin_unlock_irqrestore(&host->lock, flags);
 		cancel_delayed_work_sync(&host->detect);
 
 		if (!host->bus_ops)
@@ -2814,10 +2806,6 @@ static int mmc_pm_notify(struct notifier_block *notify_block,
 	case PM_POST_SUSPEND:
 	case PM_POST_HIBERNATION:
 	case PM_POST_RESTORE:
-
-		spin_lock_irqsave(&host->lock, flags);
-		host->rescan_disable = 0;
-		spin_unlock_irqrestore(&host->lock, flags);
 		_mmc_detect_change(host, 0, false);
 
 	}
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index e0a3ee1..e972489 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -319,9 +319,6 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
 	if (!host)
 		return NULL;
 
-	/* scanning will be enabled when we're ready */
-	host->rescan_disable = 1;
-
 again:
 	if (!ida_pre_get(&mmc_host_ida, GFP_KERNEL)) {
 		kfree(host);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 85800b4..7fbe5c0 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -330,7 +330,6 @@ struct mmc_host {
 	unsigned int		doing_retune:1;	/* re-tuning in progress */
 	unsigned int		retune_now:1;	/* do re-tuning at next req */
 
-	int			rescan_disable;	/* disable card detection */
 	int			rescan_entered;	/* used with nonremovable devices */
 
 	int			need_retune;	/* re-tuning is needed */
-- 
1.9.1

Re: [PATCH] mmc: core: Remove redundant rescan_disable flag

[Jaehoon Chung]

On 04/28/2016 07:55 PM, Ulf Hansson wrote:
> As cancel_delayed_work_sync() prevents an executing work for re-scheduling
> itself, the rescan_disable flag is redundant.
> 
> Morover, during host registration phase, it's anyway not safe to schedule a
> detect work until mmc_start_host() has been invoked. Therfore, there's no
> need to use the rescan_disable flag to protect a rescan work from being
> executed in-between mmc_alloc_host() and mmc_start_host().
> 
> For these reasons, let's remove the rescan_disable flag.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/mmc/core/core.c  | 12 ------------
>  drivers/mmc/core/host.c  |  3 ---
>  include/linux/mmc/host.h |  1 -
>  3 files changed, 16 deletions(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 99275e4..b8c6a11 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2580,9 +2580,6 @@ void mmc_rescan(struct work_struct *work)
>  		container_of(work, struct mmc_host, detect.work);
>  	int i;
>  
> -	if (host->rescan_disable)
> -		return;
> -
>  	/* If there is a non-removable card registered, only scan once */
>  	if (!mmc_card_is_removable(host) && host->rescan_entered)
>  		return;
> @@ -2649,7 +2646,6 @@ void mmc_rescan(struct work_struct *work)
>  void mmc_start_host(struct mmc_host *host)
>  {
>  	host->f_init = max(freqs[0], host->f_min);
> -	host->rescan_disable = 0;
>  	host->ios.power_mode = MMC_POWER_UNDEFINED;
>  
>  	mmc_claim_host(host);
> @@ -2674,7 +2670,6 @@ void mmc_stop_host(struct mmc_host *host)
>  	if (host->slot.cd_irq >= 0)
>  		disable_irq(host->slot.cd_irq);
>  
> -	host->rescan_disable = 1;
>  	cancel_delayed_work_sync(&host->detect);
>  
>  	/* clear pm flags now and let card drivers set them as needed */
> @@ -2788,9 +2783,6 @@ static int mmc_pm_notify(struct notifier_block *notify_block,
>  	case PM_HIBERNATION_PREPARE:
>  	case PM_SUSPEND_PREPARE:
>  	case PM_RESTORE_PREPARE:
> -		spin_lock_irqsave(&host->lock, flags);
> -		host->rescan_disable = 1;
> -		spin_unlock_irqrestore(&host->lock, flags);
>  		cancel_delayed_work_sync(&host->detect);

Maybe you can remove the defined "flags" variable.

Best Regards,
Jaehoon Chung

>  
>  		if (!host->bus_ops)
> @@ -2814,10 +2806,6 @@ static int mmc_pm_notify(struct notifier_block *notify_block,
>  	case PM_POST_SUSPEND:
>  	case PM_POST_HIBERNATION:
>  	case PM_POST_RESTORE:
> -
> -		spin_lock_irqsave(&host->lock, flags);
> -		host->rescan_disable = 0;
> -		spin_unlock_irqrestore(&host->lock, flags);
>  		_mmc_detect_change(host, 0, false);
>  
>  	}
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index e0a3ee1..e972489 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -319,9 +319,6 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
>  	if (!host)
>  		return NULL;
>  
> -	/* scanning will be enabled when we're ready */
> -	host->rescan_disable = 1;
> -
>  again:
>  	if (!ida_pre_get(&mmc_host_ida, GFP_KERNEL)) {
>  		kfree(host);
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 85800b4..7fbe5c0 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -330,7 +330,6 @@ struct mmc_host {
>  	unsigned int		doing_retune:1;	/* re-tuning in progress */
>  	unsigned int		retune_now:1;	/* do re-tuning at next req */
>  
> -	int			rescan_disable;	/* disable card detection */
>  	int			rescan_entered;	/* used with nonremovable devices */
>  
>  	int			need_retune;	/* re-tuning is needed */
> 

Re: [PATCH] mmc: core: Remove redundant rescan_disable flag

[Adrian Hunter]

On 28/04/16 13:55, Ulf Hansson wrote:
> As cancel_delayed_work_sync() prevents an executing work for re-scheduling
> itself, the rescan_disable flag is redundant.

Are you sure rescan_disable is not needed for mmc_pm_notify()?

> 
> Morover, during host registration phase, it's anyway not safe to schedule a
> detect work until mmc_start_host() has been invoked. Therfore, there's no
> need to use the rescan_disable flag to protect a rescan work from being
> executed in-between mmc_alloc_host() and mmc_start_host().
> 
> For these reasons, let's remove the rescan_disable flag.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/mmc/core/core.c  | 12 ------------
>  drivers/mmc/core/host.c  |  3 ---
>  include/linux/mmc/host.h |  1 -
>  3 files changed, 16 deletions(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 99275e4..b8c6a11 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2580,9 +2580,6 @@ void mmc_rescan(struct work_struct *work)
>  		container_of(work, struct mmc_host, detect.work);
>  	int i;
>  
> -	if (host->rescan_disable)
> -		return;
> -
>  	/* If there is a non-removable card registered, only scan once */
>  	if (!mmc_card_is_removable(host) && host->rescan_entered)
>  		return;
> @@ -2649,7 +2646,6 @@ void mmc_rescan(struct work_struct *work)
>  void mmc_start_host(struct mmc_host *host)
>  {
>  	host->f_init = max(freqs[0], host->f_min);
> -	host->rescan_disable = 0;
>  	host->ios.power_mode = MMC_POWER_UNDEFINED;
>  
>  	mmc_claim_host(host);
> @@ -2674,7 +2670,6 @@ void mmc_stop_host(struct mmc_host *host)
>  	if (host->slot.cd_irq >= 0)
>  		disable_irq(host->slot.cd_irq);
>  
> -	host->rescan_disable = 1;
>  	cancel_delayed_work_sync(&host->detect);
>  
>  	/* clear pm flags now and let card drivers set them as needed */
> @@ -2788,9 +2783,6 @@ static int mmc_pm_notify(struct notifier_block *notify_block,
>  	case PM_HIBERNATION_PREPARE:
>  	case PM_SUSPEND_PREPARE:
>  	case PM_RESTORE_PREPARE:
> -		spin_lock_irqsave(&host->lock, flags);
> -		host->rescan_disable = 1;
> -		spin_unlock_irqrestore(&host->lock, flags);
>  		cancel_delayed_work_sync(&host->detect);
>  
>  		if (!host->bus_ops)
> @@ -2814,10 +2806,6 @@ static int mmc_pm_notify(struct notifier_block *notify_block,
>  	case PM_POST_SUSPEND:
>  	case PM_POST_HIBERNATION:
>  	case PM_POST_RESTORE:
> -
> -		spin_lock_irqsave(&host->lock, flags);
> -		host->rescan_disable = 0;
> -		spin_unlock_irqrestore(&host->lock, flags);
>  		_mmc_detect_change(host, 0, false);
>  
>  	}
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index e0a3ee1..e972489 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -319,9 +319,6 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
>  	if (!host)
>  		return NULL;
>  
> -	/* scanning will be enabled when we're ready */
> -	host->rescan_disable = 1;
> -
>  again:
>  	if (!ida_pre_get(&mmc_host_ida, GFP_KERNEL)) {
>  		kfree(host);
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 85800b4..7fbe5c0 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -330,7 +330,6 @@ struct mmc_host {
>  	unsigned int		doing_retune:1;	/* re-tuning in progress */
>  	unsigned int		retune_now:1;	/* do re-tuning at next req */
>  
> -	int			rescan_disable;	/* disable card detection */
>  	int			rescan_entered;	/* used with nonremovable devices */
>  
>  	int			need_retune;	/* re-tuning is needed */
> 

Re: [PATCH] mmc: core: Remove redundant rescan_disable flag

[Ulf Hansson]

On 28 April 2016 at 14:44, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 28/04/16 13:55, Ulf Hansson wrote:
>> As cancel_delayed_work_sync() prevents an executing work for re-scheduling
>> itself, the rescan_disable flag is redundant.
>
> Are you sure rescan_disable is not needed for mmc_pm_notify()?

That's a good question!

Because we are using the system_freezable_wq for the detect works, the
works will be frozen when userspace is becomes frozen during system
PM.

New detect works that might get scheduled after that point will not be
scheduled to run until the queue becomes un-frozen.

Let's me re-spin the patch and update the changelog as that is of
course relevant information.

[...]

Kind regards
Uffe