2022-07-27 03:56:51

by Stanley Chu

[permalink] [raw]
Subject: [PATCH v2 2/5] scsi: ufs: ufs-mediatek: Provide detailed description for UIC errors

Provide detailed description in logs for UIC errors for
eaiser issue breakdown.

Signed-off-by: Stanley Chu <[email protected]>
---
drivers/ufs/host/ufs-mediatek.c | 20 +++++++++++++++++
drivers/ufs/host/ufs-mediatek.h | 38 +++++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+)

diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index ff6fd8f52ebc..b590fb267c20 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -1309,8 +1309,28 @@ static void ufs_mtk_event_notify(struct ufs_hba *hba,
enum ufs_event_type evt, void *data)
{
unsigned int val = *(u32 *)data;
+ unsigned long reg;
+ int bit;

trace_ufs_mtk_event(evt, val);
+
+ /* Print details of UIC Errors */
+ if (evt <= UFS_EVT_DME_ERR) {
+ dev_info(hba->dev,
+ "Host UIC Error Code (%s): %08x\n",
+ ufs_uic_err_str[evt], val);
+ reg = val;
+ }
+
+ if (evt == UFS_EVT_PA_ERR) {
+ for_each_set_bit(bit, &reg, ARRAY_SIZE(ufs_uic_pa_err_str))
+ dev_info(hba->dev, "%s\n", ufs_uic_pa_err_str[bit]);
+ }
+
+ if (evt == UFS_EVT_DL_ERR) {
+ for_each_set_bit(bit, &reg, ARRAY_SIZE(ufs_uic_dl_err_str))
+ dev_info(hba->dev, "%s\n", ufs_uic_dl_err_str[bit]);
+ }
}

/*
diff --git a/drivers/ufs/host/ufs-mediatek.h b/drivers/ufs/host/ufs-mediatek.h
index aa26d415527b..9017ab8f9867 100644
--- a/drivers/ufs/host/ufs-mediatek.h
+++ b/drivers/ufs/host/ufs-mediatek.h
@@ -26,6 +26,44 @@
#define REG_UFS_DEBUG_SEL_B2 0x22D8
#define REG_UFS_DEBUG_SEL_B3 0x22DC

+/*
+ * Details of UIC Errors
+ */
+static const u8 *ufs_uic_err_str[] = {
+ "PHY Adapter Layer",
+ "Data Link Layer",
+ "Network Link Layer",
+ "Transport Link Layer",
+ "DME"
+};
+
+static const u8 *ufs_uic_pa_err_str[] = {
+ "PHY error on Lane 0",
+ "PHY error on Lane 1",
+ "PHY error on Lane 2",
+ "PHY error on Lane 3",
+ "Generic PHY Adapter Error. This should be the LINERESET indication"
+};
+
+static const u8 *ufs_uic_dl_err_str[] = {
+ "NAC_RECEIVED",
+ "TCx_REPLAY_TIMER_EXPIRED",
+ "AFCx_REQUEST_TIMER_EXPIRED",
+ "FCx_PROTECTION_TIMER_EXPIRED",
+ "CRC_ERROR",
+ "RX_BUFFER_OVERFLOW",
+ "MAX_FRAME_LENGTH_EXCEEDED",
+ "WRONG_SEQUENCE_NUMBER",
+ "AFC_FRAME_SYNTAX_ERROR",
+ "NAC_FRAME_SYNTAX_ERROR",
+ "EOF_SYNTAX_ERROR",
+ "FRAME_SYNTAX_ERROR",
+ "BAD_CTRL_SYMBOL_TYPE",
+ "PA_INIT_ERROR",
+ "PA_ERROR_IND_RECEIVED",
+ "PA_INIT"
+};
+
/*
* Ref-clk control
*
--
2.18.0


2022-07-27 19:25:39

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] scsi: ufs: ufs-mediatek: Provide detailed description for UIC errors

On 7/26/22 20:44, Stanley Chu wrote:
> diff --git a/drivers/ufs/host/ufs-mediatek.h b/drivers/ufs/host/ufs-mediatek.h
> index aa26d415527b..9017ab8f9867 100644
> --- a/drivers/ufs/host/ufs-mediatek.h
> +++ b/drivers/ufs/host/ufs-mediatek.h
> @@ -26,6 +26,44 @@
> #define REG_UFS_DEBUG_SEL_B2 0x22D8
> #define REG_UFS_DEBUG_SEL_B3 0x22DC
>
> +/*
> + * Details of UIC Errors
> + */
> +static const u8 *ufs_uic_err_str[] = {
> + "PHY Adapter Layer",
> + "Data Link Layer",
> + "Network Link Layer",
> + "Transport Link Layer",
> + "DME"
> +};

Why type u8 for strings instead of char?

Please define arrays in .c files. Otherwise one copy of the array will
be included in each source file this header file is included in.

Thanks,

Bart.

2022-07-28 02:30:13

by Stanley Jhu

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] scsi: ufs: ufs-mediatek: Provide detailed description for UIC errors

Hi Bart,

On Thu, Jul 28, 2022 at 3:04 AM Bart Van Assche <[email protected]> wrote:
>
> On 7/26/22 20:44, Stanley Chu wrote:
> > diff --git a/drivers/ufs/host/ufs-mediatek.h b/drivers/ufs/host/ufs-mediatek.h
> > index aa26d415527b..9017ab8f9867 100644
> > --- a/drivers/ufs/host/ufs-mediatek.h
> > +++ b/drivers/ufs/host/ufs-mediatek.h
> > @@ -26,6 +26,44 @@
> > #define REG_UFS_DEBUG_SEL_B2 0x22D8
> > #define REG_UFS_DEBUG_SEL_B3 0x22DC
> >
> > +/*
> > + * Details of UIC Errors
> > + */
> > +static const u8 *ufs_uic_err_str[] = {
> > + "PHY Adapter Layer",
> > + "Data Link Layer",
> > + "Network Link Layer",
> > + "Transport Link Layer",
> > + "DME"
> > +};
>
> Why type u8 for strings instead of char?

Will use char instead in the next version.

>
> Please define arrays in .c files. Otherwise one copy of the array will
> be included in each source file this header file is included in.

Will move them to .c in the next version.

Thanks, Stanley



--
Yours truly,

朱原陞 (Stanley Chu)