Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
From:   "Hoyoung SEO" <hy50.seo@samsung.com>
To:     "'SEO HOYOUNG'" <hy50.seo@samsung.com>,
        <linux-scsi@vger.kernel.org>, <linux-kernel@vger.kernel.org>
In-Reply-To: <20220522171847.66373-1-hy50.seo@samsung.com>
Subject: RE: [RESEND PATCH v1] scsi: ufs: Change the shutting_down flag
 setting position
Date:   Mon, 23 May 2022 14:50:08 +0900
Message-ID: <221501d86e68$f5e613a0$e1b23ae0$@samsung.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Thread-Index: AQHYKRR1NoX3jnWOYKYl5kKfNMG5oQK7eI1prRai7gA=
Content-Language: ko
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 102P
DLP-Filter: Pass
References: <CGME20220523053658epcas2p2400c4c92126baa05d5fb4b5d6ed7e274@epcas2p2.samsung.com>
        <20220522171847.66373-1-hy50.seo@samsung.com>
Precedence: bulk

> -----Original Message-----
> From: SEO HOYOUNG [mailto:hy50.seo@samsung.com]
> Sent: Monday, May 23, 2022 2:19 AM
> To: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org;
> alim.akhtar@samsung.com; avri.altman@wdc.com; jejb@linux.ibm.com;
> martin.petersen@oracle.com; beanhuo@micron.com; asutoshd@codeaurora.org;
> cang@codeaurora.org; bvanassche@acm.org; bhoon95.kim@samsung.com;
> kwmad.kim@samsung.com
> Cc: SEO HOYOUNG
> Subject: [RESEND PATCH v1] scsi: ufs: Change the shutting_down flag
> setting position
> 
> When System shutdown, will called ufshcd_wl_shutdown().
> Then the shutting_down falg set to 1 at ufshcd_wl_shutdown().
> And send to SSU cmd at ufshcd_wl_shutdown()->__ufshcd_wl_suspend()->
> ufshcd_set_dev_pwr_mode() function.
> 
> If occurred fail during send SSU cmd,
> then will called ufshcd_err_handler for recovery.
> But already set shutting_down to 1, so ufs driver could not recovery.
> Then System wait for SSU cmd to be completed and eventaully it will tak
> Hang.
> So changed the position where shutting_down is set.
> 
> Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index
> 1fb3a8b9b03e..65d47cd5afbe 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -9235,10 +9235,6 @@ static void ufshcd_wl_shutdown(struct device *dev)
> 
>  	hba = shost_priv(sdev->host);
> 
> -	down(&hba->host_sem);
> -	hba->shutting_down = true;
> -	up(&hba->host_sem);
> -
>  	/* Turn on everything while shutting down */
>  	ufshcd_rpm_get_sync(hba);
>  	scsi_device_quiesce(sdev);
> @@ -9248,6 +9244,10 @@ static void ufshcd_wl_shutdown(struct device *dev)
>  		scsi_device_quiesce(sdev);
>  	}
>  	__ufshcd_wl_suspend(hba, UFS_SHUTDOWN_PM);
> +
> +	down(&hba->host_sem);
> +	hba->shutting_down = true;
> +	up(&hba->host_sem);
>  }
> 
>  /**
> --
> 2.26.0

Hi,

It seems that same patch below this.
https://lore.kernel.org/lkml/74cade30-6dde-c5f7-e009-b34423d22c12@intel.com/

I think that will occurred error with called __ufshcd_wl_suspend().

So the hba->shutting_down need to set after call __ufshcd_wl_suspend().
Otherwise, the error_handler cannot operate when an error occurs.
Please give opinion.

Thanks.