* [PATCH 0/2] scsi: ufs: Add an enum for ufs_trace_str_t to check uic cmd error
[not found] <CGME20250417023415epcas1p47da6d269afcb5d1807004c9b708675a5@epcas1p4.samsung.com>
@ 2025-04-17 2:34 ` DooHyun Hwang
[not found] ` <CGME20250417023417epcas1p31338c05e70e61b0a5e96d0ac0910713d@epcas1p3.samsung.com>
[not found] ` <CGME20250417023419epcas1p343060855c4470f8056116a207a584956@epcas1p3.samsung.com>
0 siblings, 2 replies; 16+ messages in thread
From: DooHyun Hwang @ 2025-04-17 2:34 UTC (permalink / raw)
To: linux-scsi, linux-kernel, alim.akhtar, avri.altman, bvanassche,
James.Bottomley, martin.petersen, peter.wang,
manivannan.sadhasivam, quic_mnaresh
Cc: grant.jung, jt77.jang, junwoo80.lee, jangsub.yi, sh043.lee,
cw9316.lee, sh8267.baek, wkon.kim, DooHyun Hwang
There is no trace when a ufs uic command error occurs.
So, add "UFS_CMD_ERR" enum to ufs_trace_str_t and add trace function calls when a uic
command error happens.
DooHyun Hwang (2):
scsi: ufs: Add an enum for ufs_trace to check ufs cmd error
scsi: ufs: core: Add a trace function calling when uic command error
occurs
drivers/ufs/core/ufs_trace.h | 1 +
drivers/ufs/core/ufshcd.c | 5 +++++
include/ufs/ufs.h | 2 +-
3 files changed, 7 insertions(+), 1 deletion(-)
--
2.48.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] scsi: ufs: Add an enum for ufs_trace to check ufs cmd error
[not found] ` <CGME20250417023417epcas1p31338c05e70e61b0a5e96d0ac0910713d@epcas1p3.samsung.com>
@ 2025-04-17 2:34 ` DooHyun Hwang
2025-04-17 4:59 ` Avri Altman
2025-08-29 15:15 ` Bart Van Assche
0 siblings, 2 replies; 16+ messages in thread
From: DooHyun Hwang @ 2025-04-17 2:34 UTC (permalink / raw)
To: linux-scsi, linux-kernel, alim.akhtar, avri.altman, bvanassche,
James.Bottomley, martin.petersen, peter.wang,
manivannan.sadhasivam, quic_mnaresh
Cc: grant.jung, jt77.jang, junwoo80.lee, jangsub.yi, sh043.lee,
cw9316.lee, sh8267.baek, wkon.kim, DooHyun Hwang
There is no trace when a ufs uic cmd error occurs.
So, add "UFS_CMD_ERR" enumeration to ufs_trace_str_t.
Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com>
---
drivers/ufs/core/ufs_trace.h | 1 +
include/ufs/ufs.h | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/ufs/core/ufs_trace.h b/drivers/ufs/core/ufs_trace.h
index caa32e23ffa5..43830a092637 100644
--- a/drivers/ufs/core/ufs_trace.h
+++ b/drivers/ufs/core/ufs_trace.h
@@ -41,6 +41,7 @@
#define UFS_CMD_TRACE_STRINGS \
EM(UFS_CMD_SEND, "send_req") \
EM(UFS_CMD_COMP, "complete_rsp") \
+ EM(UFS_CMD_ERR, "req_complete_err") \
EM(UFS_DEV_COMP, "dev_complete") \
EM(UFS_QUERY_SEND, "query_send") \
EM(UFS_QUERY_COMP, "query_complete") \
diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
index c0c59a8f7256..7f2d418bdd86 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -631,7 +631,7 @@ struct ufs_dev_info {
* This enum is used in string mapping in ufs_trace.h.
*/
enum ufs_trace_str_t {
- UFS_CMD_SEND, UFS_CMD_COMP, UFS_DEV_COMP,
+ UFS_CMD_SEND, UFS_CMD_COMP, UFS_CMD_ERR, UFS_DEV_COMP,
UFS_QUERY_SEND, UFS_QUERY_COMP, UFS_QUERY_ERR,
UFS_TM_SEND, UFS_TM_COMP, UFS_TM_ERR
};
--
2.48.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] scsi: ufs: core: Add a trace function calling when uic command error occurs
[not found] ` <CGME20250417023419epcas1p343060855c4470f8056116a207a584956@epcas1p3.samsung.com>
@ 2025-04-17 2:34 ` DooHyun Hwang
2025-04-17 21:35 ` Bart Van Assche
2025-04-22 12:48 ` Peter Wang (王信友)
0 siblings, 2 replies; 16+ messages in thread
From: DooHyun Hwang @ 2025-04-17 2:34 UTC (permalink / raw)
To: linux-scsi, linux-kernel, alim.akhtar, avri.altman, bvanassche,
James.Bottomley, martin.petersen, peter.wang,
manivannan.sadhasivam, quic_mnaresh
Cc: grant.jung, jt77.jang, junwoo80.lee, jangsub.yi, sh043.lee,
cw9316.lee, sh8267.baek, wkon.kim, DooHyun Hwang
When a uic command error occurs, there is no trace function calling.
Therefore, trace function calls are added when a uic command error happens.
Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com>
---
drivers/ufs/core/ufshcd.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index ab98acd982b5..baac1ae94efc 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -2534,6 +2534,9 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
hba->active_uic_cmd = NULL;
spin_unlock_irqrestore(hba->host->host_lock, flags);
+ if (ret)
+ ufshcd_add_uic_command_trace(hba, uic_cmd, UFS_CMD_ERR);
+
return ret;
}
@@ -4306,6 +4309,8 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd)
}
out:
if (ret) {
+ ufshcd_add_uic_command_trace(hba, hba->active_uic_cmd,
+ UFS_CMD_ERR);
ufshcd_print_host_state(hba);
ufshcd_print_pwr_info(hba);
ufshcd_print_evt_hist(hba);
--
2.48.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [PATCH 1/2] scsi: ufs: Add an enum for ufs_trace to check ufs cmd error
2025-04-17 2:34 ` [PATCH 1/2] scsi: ufs: Add an enum for ufs_trace to check ufs " DooHyun Hwang
@ 2025-04-17 4:59 ` Avri Altman
2025-04-17 8:21 ` DooHyun Hwang
2025-08-29 15:15 ` Bart Van Assche
1 sibling, 1 reply; 16+ messages in thread
From: Avri Altman @ 2025-04-17 4:59 UTC (permalink / raw)
To: DooHyun Hwang, linux-scsi@vger.kernel.org,
linux-kernel@vger.kernel.org, alim.akhtar@samsung.com,
avri.altman@wdc.com, bvanassche@acm.org,
James.Bottomley@HansenPartnership.com, martin.petersen@oracle.com,
peter.wang@mediatek.com, manivannan.sadhasivam@linaro.org,
quic_mnaresh@quicinc.com
Cc: grant.jung@samsung.com, jt77.jang@samsung.com,
junwoo80.lee@samsung.com, jangsub.yi@samsung.com,
sh043.lee@samsung.com, cw9316.lee@samsung.com,
sh8267.baek@samsung.com, wkon.kim@samsung.com
> There is no trace when a ufs uic cmd error occurs.
> So, add "UFS_CMD_ERR" enumeration to ufs_trace_str_t.
>
> Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com>
> ---
> drivers/ufs/core/ufs_trace.h | 1 +
> include/ufs/ufs.h | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ufs/core/ufs_trace.h b/drivers/ufs/core/ufs_trace.h index
> caa32e23ffa5..43830a092637 100644
> --- a/drivers/ufs/core/ufs_trace.h
> +++ b/drivers/ufs/core/ufs_trace.h
> @@ -41,6 +41,7 @@
> #define UFS_CMD_TRACE_STRINGS \
> EM(UFS_CMD_SEND, "send_req") \
> EM(UFS_CMD_COMP, "complete_rsp") \
> + EM(UFS_CMD_ERR, "req_complete_err") \
> EM(UFS_DEV_COMP, "dev_complete") \
> EM(UFS_QUERY_SEND, "query_send") \
> EM(UFS_QUERY_COMP, "query_complete") \
> diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h index
> c0c59a8f7256..7f2d418bdd86 100644
> --- a/include/ufs/ufs.h
> +++ b/include/ufs/ufs.h
> @@ -631,7 +631,7 @@ struct ufs_dev_info {
> * This enum is used in string mapping in ufs_trace.h.
> */
> enum ufs_trace_str_t {
> - UFS_CMD_SEND, UFS_CMD_COMP, UFS_DEV_COMP,
> + UFS_CMD_SEND, UFS_CMD_COMP, UFS_CMD_ERR, UFS_DEV_COMP,
> UFS_QUERY_SEND, UFS_QUERY_COMP, UFS_QUERY_ERR,
> UFS_TM_SEND, UFS_TM_COMP, UFS_TM_ERR };
It seems strange to me that scsi & uic commands are designated by the same enum.
Has it been considered to add UFS_UIC_SEND, UFS_UIC_COMP, UFS_UIC_ERR to enum ufs_trace_str_t ?
Also looks like UFS_DEV_COMP is unused ?
Thanks,
Avri
> --
> 2.48.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 1/2] scsi: ufs: Add an enum for ufs_trace to check ufs cmd error
2025-04-17 4:59 ` Avri Altman
@ 2025-04-17 8:21 ` DooHyun Hwang
0 siblings, 0 replies; 16+ messages in thread
From: DooHyun Hwang @ 2025-04-17 8:21 UTC (permalink / raw)
To: 'Avri Altman', linux-scsi, linux-kernel, alim.akhtar,
avri.altman, bvanassche, James.Bottomley, martin.petersen,
peter.wang, manivannan.sadhasivam, quic_mnaresh
Cc: grant.jung, jt77.jang, junwoo80.lee, jangsub.yi, sh043.lee,
cw9316.lee, sh8267.baek, wkon.kim
> > There is no trace when a ufs uic cmd error occurs.
> > So, add "UFS_CMD_ERR" enumeration to ufs_trace_str_t.
> >
> > Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com>
> > ---
> > drivers/ufs/core/ufs_trace.h | 1 +
> > include/ufs/ufs.h | 2 +-
> > 2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/ufs/core/ufs_trace.h b/drivers/ufs/core/ufs_trace.h
> index
> > caa32e23ffa5..43830a092637 100644
> > --- a/drivers/ufs/core/ufs_trace.h
> > +++ b/drivers/ufs/core/ufs_trace.h
> > @@ -41,6 +41,7 @@
> > #define UFS_CMD_TRACE_STRINGS \
> > EM(UFS_CMD_SEND, "send_req") \
> > EM(UFS_CMD_COMP, "complete_rsp") \
> > + EM(UFS_CMD_ERR, "req_complete_err") \
> > EM(UFS_DEV_COMP, "dev_complete") \
> > EM(UFS_QUERY_SEND, "query_send") \
> > EM(UFS_QUERY_COMP, "query_complete") \
> > diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h index
> > c0c59a8f7256..7f2d418bdd86 100644
> > --- a/include/ufs/ufs.h
> > +++ b/include/ufs/ufs.h
> > @@ -631,7 +631,7 @@ struct ufs_dev_info {
> > * This enum is used in string mapping in ufs_trace.h.
> > */
> > enum ufs_trace_str_t {
> > - UFS_CMD_SEND, UFS_CMD_COMP, UFS_DEV_COMP,
> > + UFS_CMD_SEND, UFS_CMD_COMP, UFS_CMD_ERR, UFS_DEV_COMP,
> > UFS_QUERY_SEND, UFS_QUERY_COMP, UFS_QUERY_ERR,
> > UFS_TM_SEND, UFS_TM_COMP, UFS_TM_ERR };
> It seems strange to me that scsi & uic commands are designated by the same
> enum.
> Has it been considered to add UFS_UIC_SEND, UFS_UIC_COMP, UFS_UIC_ERR to
> enum ufs_trace_str_t ?
> Also looks like UFS_DEV_COMP is unused ?
>
> Thanks,
> Avri
It is correct that the same enumeration is used for both SCSI and UIC
commands.
However, the trace function differs for SCSI and UIC commands.
The enum is solely for handling the sending and completion of each command
within the respective trace functions.
Therefore, I think there is no need to add new enums like "UFS_UIC_SEND".
It seems better to discuss "UFS_DEV_COMP" in another commit.
Thank you.
DooHyun Hwang.
> > --
> > 2.48.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: core: Add a trace function calling when uic command error occurs
2025-04-17 2:34 ` [PATCH 2/2] scsi: ufs: core: Add a trace function calling when uic command error occurs DooHyun Hwang
@ 2025-04-17 21:35 ` Bart Van Assche
2025-04-18 4:48 ` DooHyun Hwang
2025-04-22 12:48 ` Peter Wang (王信友)
1 sibling, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2025-04-17 21:35 UTC (permalink / raw)
To: DooHyun Hwang, linux-scsi, linux-kernel, alim.akhtar, avri.altman,
James.Bottomley, martin.petersen, peter.wang,
manivannan.sadhasivam, quic_mnaresh
Cc: grant.jung, jt77.jang, junwoo80.lee, jangsub.yi, sh043.lee,
cw9316.lee, sh8267.baek, wkon.kim
On 4/16/25 7:34 PM, DooHyun Hwang wrote:
> When a uic command error occurs, there is no trace function calling.
> Therefore, trace function calls are added when a uic command error happens.
>
> Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com>
> ---
> drivers/ufs/core/ufshcd.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index ab98acd982b5..baac1ae94efc 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -2534,6 +2534,9 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd)
> hba->active_uic_cmd = NULL;
> spin_unlock_irqrestore(hba->host->host_lock, flags);
>
> + if (ret)
> + ufshcd_add_uic_command_trace(hba, uic_cmd, UFS_CMD_ERR);
> +
> return ret;
> }
>
> @@ -4306,6 +4309,8 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd)
> }
> out:
> if (ret) {
> + ufshcd_add_uic_command_trace(hba, hba->active_uic_cmd,
> + UFS_CMD_ERR);
> ufshcd_print_host_state(hba);
> ufshcd_print_pwr_info(hba);
> ufshcd_print_evt_hist(hba);
Shouldn't the value of 'ret' be included in the UFS_CMD_ERR trace output?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 2/2] scsi: ufs: core: Add a trace function calling when uic command error occurs
2025-04-17 21:35 ` Bart Van Assche
@ 2025-04-18 4:48 ` DooHyun Hwang
0 siblings, 0 replies; 16+ messages in thread
From: DooHyun Hwang @ 2025-04-18 4:48 UTC (permalink / raw)
To: 'Bart Van Assche', linux-scsi, linux-kernel, alim.akhtar,
avri.altman, James.Bottomley, martin.petersen, peter.wang,
manivannan.sadhasivam, quic_mnaresh
Cc: grant.jung, jt77.jang, junwoo80.lee, jangsub.yi, sh043.lee,
cw9316.lee, sh8267.baek, wkon.kim
> On 4/16/25 7:34 PM, DooHyun Hwang wrote:
> > When a uic command error occurs, there is no trace function calling.
> > Therefore, trace function calls are added when a uic command error
> happens.
> >
> > Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com>
> > ---
> > drivers/ufs/core/ufshcd.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > index ab98acd982b5..baac1ae94efc 100644
> > --- a/drivers/ufs/core/ufshcd.c
> > +++ b/drivers/ufs/core/ufshcd.c
> > @@ -2534,6 +2534,9 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba,
> struct uic_command *uic_cmd)
> > hba->active_uic_cmd = NULL;
> > spin_unlock_irqrestore(hba->host->host_lock, flags);
> >
> > + if (ret)
> > + ufshcd_add_uic_command_trace(hba, uic_cmd, UFS_CMD_ERR);
> > +
> > return ret;
> > }
> >
> > @@ -4306,6 +4309,8 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba,
> struct uic_command *cmd)
> > }
> > out:
> > if (ret) {
> > + ufshcd_add_uic_command_trace(hba, hba->active_uic_cmd,
> > + UFS_CMD_ERR);
> > ufshcd_print_host_state(hba);
> > ufshcd_print_pwr_info(hba);
> > ufshcd_print_evt_hist(hba);
>
> Shouldn't the value of 'ret' be included in the UFS_CMD_ERR trace output?
>
> Thanks,
>
> Bart.
Thank you for your insightful feedback.
Upon further consideration, it seems that the existing trace, which captures detailed information through the UICCMDARG1 to UICCMDARG3 fields.
That already provides sufficient context for analyzing command-related issues.
The UFS_CMD_ERR trace output, combined with UICCMDARG* values, should be adequate for diagnosing errors and understanding the command's execution status.
BR,
Thank you.
DooHyun Hwang.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] scsi: ufs: core: Add a trace function calling when uic command error occurs
2025-04-17 2:34 ` [PATCH 2/2] scsi: ufs: core: Add a trace function calling when uic command error occurs DooHyun Hwang
2025-04-17 21:35 ` Bart Van Assche
@ 2025-04-22 12:48 ` Peter Wang (王信友)
2025-04-23 6:45 ` DooHyun Hwang
1 sibling, 1 reply; 16+ messages in thread
From: Peter Wang (王信友) @ 2025-04-22 12:48 UTC (permalink / raw)
To: avri.altman@wdc.com, dh0421.hwang@samsung.com,
quic_mnaresh@quicinc.com, linux-scsi@vger.kernel.org,
bvanassche@acm.org, linux-kernel@vger.kernel.org,
alim.akhtar@samsung.com, manivannan.sadhasivam@linaro.org,
martin.petersen@oracle.com, James.Bottomley@HansenPartnership.com
Cc: sh043.lee@samsung.com, wkon.kim@samsung.com,
cw9316.lee@samsung.com, grant.jung@samsung.com,
sh8267.baek@samsung.com, jt77.jang@samsung.com,
jangsub.yi@samsung.com, junwoo80.lee@samsung.com
On Thu, 2025-04-17 at 11:34 +0900, DooHyun Hwang wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> When a uic command error occurs, there is no trace function calling.
> Therefore, trace function calls are added when a uic command error
> happens.
>
> Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com>
> ---
> drivers/ufs/core/ufshcd.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index ab98acd982b5..baac1ae94efc 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -2534,6 +2534,9 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba,
> struct uic_command *uic_cmd)
> hba->active_uic_cmd = NULL;
> spin_unlock_irqrestore(hba->host->host_lock, flags);
>
> + if (ret)
> + ufshcd_add_uic_command_trace(hba, uic_cmd,
> UFS_CMD_ERR);
> +
> return ret;
> }
>
> @@ -4306,6 +4309,8 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> *hba, struct uic_command *cmd)
> }
> out:
> if (ret) {
> + ufshcd_add_uic_command_trace(hba, hba-
> >active_uic_cmd,
> + UFS_CMD_ERR);
> ufshcd_print_host_state(hba);
> ufshcd_print_pwr_info(hba);
> ufshcd_print_evt_hist(hba);
> --
> 2.48.1
>
Hi DooHyun,
Is it possible to receive UFS_CMD_COMP and UFS_CMD_ERR at the same
time?
Wouldn't it be a bit strange for a command to receive two completions?
Additionally, should we also add tracing for general SCSI commands
that receive errors?
Thanks
Peter
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 2/2] scsi: ufs: core: Add a trace function calling when uic command error occurs
2025-04-22 12:48 ` Peter Wang (王信友)
@ 2025-04-23 6:45 ` DooHyun Hwang
0 siblings, 0 replies; 16+ messages in thread
From: DooHyun Hwang @ 2025-04-23 6:45 UTC (permalink / raw)
To: 'Peter Wang (王信友)', avri.altman,
quic_mnaresh, linux-scsi, bvanassche, linux-kernel, alim.akhtar,
manivannan.sadhasivam, martin.petersen, James.Bottomley
Cc: sh043.lee, wkon.kim, cw9316.lee, grant.jung, sh8267.baek,
jt77.jang, jangsub.yi, junwoo80.lee
> On Thu, 2025-04-17 at 11:34 +0900, DooHyun Hwang wrote:
> >
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> >
> >
> > When a uic command error occurs, there is no trace function calling.
> > Therefore, trace function calls are added when a uic command error
> > happens.
> >
> > Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com>
> > ---
> > drivers/ufs/core/ufshcd.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > index ab98acd982b5..baac1ae94efc 100644
> > --- a/drivers/ufs/core/ufshcd.c
> > +++ b/drivers/ufs/core/ufshcd.c
> > @@ -2534,6 +2534,9 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba,
> > struct uic_command *uic_cmd)
> > hba->active_uic_cmd = NULL;
> > spin_unlock_irqrestore(hba->host->host_lock, flags);
> >
> > + if (ret)
> > + ufshcd_add_uic_command_trace(hba, uic_cmd,
> > UFS_CMD_ERR);
> > +
> > return ret;
> > }
> >
> > @@ -4306,6 +4309,8 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> > *hba, struct uic_command *cmd)
> > }
> > out:
> > if (ret) {
> > + ufshcd_add_uic_command_trace(hba, hba-
> > >active_uic_cmd,
> > + UFS_CMD_ERR);
> > ufshcd_print_host_state(hba);
> > ufshcd_print_pwr_info(hba);
> > ufshcd_print_evt_hist(hba);
> > --
> > 2.48.1
> >
>
>
> Hi DooHyun,
>
> Is it possible to receive UFS_CMD_COMP and UFS_CMD_ERR at the same time?
> Wouldn't it be a bit strange for a command to receive two completions?
> Additionally, should we also add tracing for general SCSI commands that
> receive errors?
>
> Thanks
> Peter
>
Thank you for your review.
Yes, that’s correct. There is a case where both UFS_CMD_COMP and UFS_CMD_ERR are logged simultaneously.
In the typical case of a UIC command timeout, only UFS_CMD_ERR would be recorded.
However, the scenario you described earlier, where both UFS_CMD_COMP and UFS_CMD_ERR are received in the same UIC command, is distinct from a timeout situation.
In this case, the command completes in time, and UFS_CMD_COMP is received when handling the UIC command completion interrupt in ufshcd_uic_cmd_compl().
However, the UPMCRS status indicates an error because PWR_LOCAL is not in its expected state (when checking upmcrs in the "check_upmcrs:" section of the ufshcd_uic_pwr_ctrl() function).
UFS_CMD_COMP is generated when the system recognizes through interrupt status that the command has completed its execution, while UFS_CMD_ERR is recorded when an error associated with the command is detected.
While it may be cumbersome, it is possible to trace the timeout case and this case separately.
Regarding tracing general SCSI command errors directly through UFS_CMD_ERR, it might be challenging because SCSI command errors are typically handled through various mechanisms such as timeout detection, scsi_status value checks, sense error checks, and OCS (Overall Command Status) value verification.
Additionally, tracing SCSI command errors does not align with the purpose of this commit.
BR,
Thank you.
DooHyun Hwang
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] scsi: ufs: Add an enum for ufs_trace to check ufs cmd error
2025-04-17 2:34 ` [PATCH 1/2] scsi: ufs: Add an enum for ufs_trace to check ufs " DooHyun Hwang
2025-04-17 4:59 ` Avri Altman
@ 2025-08-29 15:15 ` Bart Van Assche
2025-09-01 1:31 ` DooHyun Hwang
1 sibling, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2025-08-29 15:15 UTC (permalink / raw)
To: DooHyun Hwang, linux-scsi, linux-kernel, alim.akhtar, avri.altman,
James.Bottomley, martin.petersen, peter.wang,
manivannan.sadhasivam, quic_mnaresh
Cc: grant.jung, jt77.jang, junwoo80.lee, jangsub.yi, sh043.lee,
cw9316.lee, sh8267.baek, wkon.kim
On 4/16/25 7:34 PM, DooHyun Hwang wrote:
> + EM(UFS_CMD_ERR, "req_complete_err") \
Does UFS_CMD_ERR stand for "command error" or "completion error"? Please
make the enum label and the text that is displayed in error messages
consistent.
Bart.
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 1/2] scsi: ufs: Add an enum for ufs_trace to check ufs cmd error
2025-08-29 15:15 ` Bart Van Assche
@ 2025-09-01 1:31 ` DooHyun Hwang
2025-09-01 14:03 ` Bart Van Assche
0 siblings, 1 reply; 16+ messages in thread
From: DooHyun Hwang @ 2025-09-01 1:31 UTC (permalink / raw)
To: 'Bart Van Assche', linux-scsi, linux-kernel, alim.akhtar,
avri.altman, James.Bottomley, martin.petersen, peter.wang,
manivannan.sadhasivam, quic_mnaresh
Cc: grant.jung, jt77.jang, junwoo80.lee, jangsub.yi, sh043.lee,
cw9316.lee, sh8267.baek, wkon.kim
On 8/29/25 8:15 PM, Bart Van Assche wrote:
> On 4/16/25 7:34 PM, DooHyun Hwang wrote:
> > + EM(UFS_CMD_ERR, "req_complete_err") \
>
> Does UFS_CMD_ERR stand for "command error" or "completion error"? Please
> make the enum label and the text that is displayed in error messages
> consistent.
>
> Bart.
UFS_CMD_ERR stands for "completion error".
The enum is converted to a string in the trace log according
to the definition below.
in include/trace/events/ufs.h
#define UFS_CMD_TRACE_STRINGS \
EM(UFS_CMD_SEND, "send_req") \
EM(UFS_CMD_COMP, "complete_rsp") \
+ EM(UFS_CMD_ERR, "req_complete_err") \
EM(UFS_DEV_COMP, "dev_complete") \
EM(UFS_QUERY_SEND, "query_send") \
EM(UFS_QUERY_COMP, "query_complete") \
EM(UFS_QUERY_ERR, "query_complete_err") \
EM(UFS_TM_SEND, "tm_send") \
EM(UFS_TM_COMP, "tm_complete") \
EMe(UFS_TM_ERR, "tm_complete_err")
Thank you.
DooHyun Hwang.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] scsi: ufs: Add an enum for ufs_trace to check ufs cmd error
2025-09-01 1:31 ` DooHyun Hwang
@ 2025-09-01 14:03 ` Bart Van Assche
2025-09-02 1:09 ` DooHyun Hwang
0 siblings, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2025-09-01 14:03 UTC (permalink / raw)
To: DooHyun Hwang, linux-scsi, linux-kernel, alim.akhtar, avri.altman,
James.Bottomley, martin.petersen, peter.wang,
manivannan.sadhasivam, quic_mnaresh
Cc: grant.jung, jt77.jang, junwoo80.lee, jangsub.yi, sh043.lee,
cw9316.lee, sh8267.baek, wkon.kim
On 8/31/25 6:31 PM, DooHyun Hwang wrote:
> UFS_CMD_ERR stands for "completion error".
No it doesn't. In the UFS driver and also in many other driver "cmd"
stands for "command" and not for "completion". Please change the
"UFS_CMD_ERR" enum label.
Bart.
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 1/2] scsi: ufs: Add an enum for ufs_trace to check ufs cmd error
2025-09-01 14:03 ` Bart Van Assche
@ 2025-09-02 1:09 ` DooHyun Hwang
2025-09-02 16:34 ` Bart Van Assche
0 siblings, 1 reply; 16+ messages in thread
From: DooHyun Hwang @ 2025-09-02 1:09 UTC (permalink / raw)
To: 'Bart Van Assche', linux-scsi, linux-kernel, alim.akhtar,
avri.altman, James.Bottomley, martin.petersen, peter.wang,
manivannan.sadhasivam, quic_mnaresh
Cc: grant.jung, jt77.jang, junwoo80.lee, jangsub.yi, sh043.lee,
cw9316.lee, sh8267.baek, wkon.kim
On 9/1/25 7:4 AM, Bart Van Assche wrote:
> On 8/31/25 6:31 PM, DooHyun Hwang wrote:
> > UFS_CMD_ERR stands for "completion error".
>
> No it doesn't. In the UFS driver and also in many other driver "cmd"
> stands for "command" and not for "completion". Please change the
> "UFS_CMD_ERR" enum label.
>
> Bart.
I'm sorry. There was a misunderstanding in my previous comment.
The UFS_CMD_ERR stands for "command completion error."
I wanted to make it consistent with the other enums.
For example, query errors are defined as UFS_QUERY_ERR,
and Task Management errors are defined as UFS_TM_ERR.
I hope to use UFS_CMD_ERR for the same classification purposes
as UFS_CMD_SEND or UFS_CMD_COMP.
Therefore, UFS_CMD_ERR can be used for logging SCSI or UIC commands.
Thank you.
DooHyun Hwang.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] scsi: ufs: Add an enum for ufs_trace to check ufs cmd error
2025-09-02 1:09 ` DooHyun Hwang
@ 2025-09-02 16:34 ` Bart Van Assche
2025-09-03 6:39 ` DooHyun Hwang
0 siblings, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2025-09-02 16:34 UTC (permalink / raw)
To: DooHyun Hwang, linux-scsi, linux-kernel, alim.akhtar, avri.altman,
James.Bottomley, martin.petersen, peter.wang,
manivannan.sadhasivam, quic_mnaresh
Cc: grant.jung, jt77.jang, junwoo80.lee, jangsub.yi, sh043.lee,
cw9316.lee, sh8267.baek, wkon.kim
On 9/1/25 6:09 PM, DooHyun Hwang wrote:
> The UFS_CMD_ERR stands for "command completion error."
I've never before seen anyone abbreviating "completion" as "CMD".
Please choose a better enumeration label name.
Bart.
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 1/2] scsi: ufs: Add an enum for ufs_trace to check ufs cmd error
2025-09-02 16:34 ` Bart Van Assche
@ 2025-09-03 6:39 ` DooHyun Hwang
2025-09-03 15:56 ` Bart Van Assche
0 siblings, 1 reply; 16+ messages in thread
From: DooHyun Hwang @ 2025-09-03 6:39 UTC (permalink / raw)
To: 'Bart Van Assche', linux-scsi, linux-kernel, alim.akhtar,
avri.altman, James.Bottomley, martin.petersen, peter.wang,
manivannan.sadhasivam, quic_mnaresh
Cc: grant.jung, jt77.jang, junwoo80.lee, jangsub.yi, sh043.lee,
cw9316.lee, sh8267.baek, wkon.kim
On 9/2/25 9:35 AM, Bart Van Assche wrote:
> On 9/1/25 6:09 PM, DooHyun Hwang wrote:
> > The UFS_CMD_ERR stands for "command completion error."
>
> I've never before seen anyone abbreviating "completion" as "CMD".
> Please choose a better enumeration label name.
>
> Bart.
When I reviewed the UFS_CMD_TRACE_STRINGS definition,
I noticed that "query_complete_err" is referred to as UFS_QUERY_ERR,
and "tm_complete_err" is referred to as UFS_TM_ERR.
I followed the naming convention specified in this STRINGS definition,
but I’m not entirely clear on what changes are being requested.
Could you please provide some guidance or suggestions?
Thank you.
DooHyun Hwang.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] scsi: ufs: Add an enum for ufs_trace to check ufs cmd error
2025-09-03 6:39 ` DooHyun Hwang
@ 2025-09-03 15:56 ` Bart Van Assche
0 siblings, 0 replies; 16+ messages in thread
From: Bart Van Assche @ 2025-09-03 15:56 UTC (permalink / raw)
To: DooHyun Hwang, linux-scsi, linux-kernel, alim.akhtar, avri.altman,
James.Bottomley, martin.petersen, peter.wang,
manivannan.sadhasivam, quic_mnaresh
Cc: grant.jung, jt77.jang, junwoo80.lee, jangsub.yi, sh043.lee,
cw9316.lee, sh8267.baek, wkon.kim
On 9/2/25 11:39 PM, DooHyun Hwang wrote:
> Could you please provide some guidance or suggestions?
How about this approach?
* In a separate patch, remove UFS_DEV_COMP because it is not used.
* Call the new enumeration label UFS_UIC_ERR (UFS UIC command error)
since "UIC" is how the UFS standard refers to UIC commands (these are
called "device commands" in the UFS driver).
If anyone else has a different opinion, please speak up.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2025-09-03 15:56 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20250417023415epcas1p47da6d269afcb5d1807004c9b708675a5@epcas1p4.samsung.com>
2025-04-17 2:34 ` [PATCH 0/2] scsi: ufs: Add an enum for ufs_trace_str_t to check uic cmd error DooHyun Hwang
[not found] ` <CGME20250417023417epcas1p31338c05e70e61b0a5e96d0ac0910713d@epcas1p3.samsung.com>
2025-04-17 2:34 ` [PATCH 1/2] scsi: ufs: Add an enum for ufs_trace to check ufs " DooHyun Hwang
2025-04-17 4:59 ` Avri Altman
2025-04-17 8:21 ` DooHyun Hwang
2025-08-29 15:15 ` Bart Van Assche
2025-09-01 1:31 ` DooHyun Hwang
2025-09-01 14:03 ` Bart Van Assche
2025-09-02 1:09 ` DooHyun Hwang
2025-09-02 16:34 ` Bart Van Assche
2025-09-03 6:39 ` DooHyun Hwang
2025-09-03 15:56 ` Bart Van Assche
[not found] ` <CGME20250417023419epcas1p343060855c4470f8056116a207a584956@epcas1p3.samsung.com>
2025-04-17 2:34 ` [PATCH 2/2] scsi: ufs: core: Add a trace function calling when uic command error occurs DooHyun Hwang
2025-04-17 21:35 ` Bart Van Assche
2025-04-18 4:48 ` DooHyun Hwang
2025-04-22 12:48 ` Peter Wang (王信友)
2025-04-23 6:45 ` DooHyun Hwang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).