diff mbox series

[v1] scsi: ufs: see link lost as fatal

Message ID 1642415846-141110-1-git-send-email-kwmad.kim@samsung.com (mailing list archive)
State Superseded
Headers show
Series [v1] scsi: ufs: see link lost as fatal | expand

Commit Message

Kiwoong Kim Jan. 17, 2022, 10:37 a.m. UTC
This event is raised when link is lost as specified
in UFSHCI spec. At the time, initializing UFS interface
needs to be done.

Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
---
 drivers/scsi/ufs/ufshci.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Bart Van Assche Jan. 19, 2022, 12:40 a.m. UTC | #1
On 1/17/22 02:37, Kiwoong Kim wrote:
> This event is raised when link is lost as specified
> in UFSHCI spec. At the time, initializing UFS interface
> needs to be done.
> 
> Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
> ---
>   drivers/scsi/ufs/ufshci.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
> index 6a295c8..a7ff0e5 100644
> --- a/drivers/scsi/ufs/ufshci.h
> +++ b/drivers/scsi/ufs/ufshci.h
> @@ -142,7 +142,8 @@ static inline u32 ufshci_version(u32 major, u32 minor)
>   #define INT_FATAL_ERRORS	(DEVICE_FATAL_ERROR |\
>   				CONTROLLER_FATAL_ERROR |\
>   				SYSTEM_BUS_FATAL_ERROR |\
> -				CRYPTO_ENGINE_FATAL_ERROR)
> +				CRYPTO_ENGINE_FATAL_ERROR |\
> +				UIC_LINK_LOST)
>   
>   /* HCS - Host Controller Status 30h */
>   #define DEVICE_PRESENT				0x1

A patch description should not only explain what is changed but also why
a change is being made. Will the above patch cause the UFS error handler
to trigger a controller reset after the link has been lost? I'm missing
an explanation of why that change is necessary and also of why that
change is the right thing to do. All I found in the UFSHCI specification
about link loss is the following: "UIC Link Lost Status Enable (ULLSE): 
When set and IS.ULLS is set, the controller shall generate an 
interrupt." and also "UIC Link Lost Status (ULLS): This indicates a 
condition where remote end is trying to reestablish a link and the link 
is lost. This bit corresponds to the UniPro DME_LINKLOST.ind SAP primitive."

Did I perhaps overlook something?

Thanks,

Bart.
Kiwoong Kim Jan. 20, 2022, 3:10 a.m. UTC | #2
> and also "UIC Link Lost Status (ULLS): This indicates a condition where
> remote end is trying to reestablish a link and the link is lost. This bit
> corresponds to the UniPro DME_LINKLOST.ind SAP primitive."

It seems that you've said the why. That is, Link lost presents communication is
not possible and the driver should do recovery from host reset. The way to make it
is that make the driver this as fatal. This patch is to make Link Lost join in
INT_FATAL_ERRORS to do that.

Okay, I'll update the comment.

Thanks.
Kiwoong Kim
diff mbox series

Patch

diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index 6a295c8..a7ff0e5 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -142,7 +142,8 @@  static inline u32 ufshci_version(u32 major, u32 minor)
 #define INT_FATAL_ERRORS	(DEVICE_FATAL_ERROR |\
 				CONTROLLER_FATAL_ERROR |\
 				SYSTEM_BUS_FATAL_ERROR |\
-				CRYPTO_ENGINE_FATAL_ERROR)
+				CRYPTO_ENGINE_FATAL_ERROR |\
+				UIC_LINK_LOST)
 
 /* HCS - Host Controller Status 30h */
 #define DEVICE_PRESENT				0x1