[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <062001d617b1$af5f0aa0$0e1d1fe0$@samsung.com>
Date: Tue, 21 Apr 2020 16:51:37 +0900
From: "Kiwoong Kim" <kwmad.kim@...sung.com>
To: "'Avri Altman'" <Avri.Altman@....com>,
"'Alim Akhtar'" <alim.akhtar@...sung.com>, <robh@...nel.org>,
<cpgs@...sung.com>
Cc: <devicetree@...r.kernel.org>, <linux-scsi@...r.kernel.org>,
<krzk@...nel.org>, <martin.petersen@...cle.com>,
<cang@...eaurora.org>, <linux-samsung-soc@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v6 05/10] scsi: ufs: add quirk to fix abnormal ocs fatal
error
> -----Original Message-----
> From: Avri Altman <Avri.Altman@....com>
> Sent: Monday, April 20, 2020 5:56 PM
> To: Alim Akhtar <alim.akhtar@...sung.com>; robh@...nel.org
> Cc: devicetree@...r.kernel.org; linux-scsi@...r.kernel.org;
> krzk@...nel.org; martin.petersen@...cle.com; kwmad.kim@...sung.com;
> stanley.chu@...iatek.com; cang@...eaurora.org; linux-samsung-
> soc@...r.kernel.org; linux-arm-kernel@...ts.infradead.org; linux-
> kernel@...r.kernel.org
> Subject: RE: [PATCH v6 05/10] scsi: ufs: add quirk to fix abnormal ocs
> fatal error
>
> >
> > From: Kiwoong Kim <kwmad.kim@...sung.com>
> >
> > Some architectures determines if fatal error for OCS occurrs to check
> > status in response upiu. This patch
> Typo - occurs
>
> > is to prevent from reporting command results with that.
> >
> > Signed-off-by: Kiwoong Kim <kwmad.kim@...sung.com>
> > Signed-off-by: Alim Akhtar <alim.akhtar@...sung.com>
> > ---
> > drivers/scsi/ufs/ufshcd.c | 6 ++++++
> > drivers/scsi/ufs/ufshcd.h | 6 ++++++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index b32fcedcdcb9..8c07caff0a5c 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -4794,6 +4794,12 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
> > struct ufshcd_lrb *lrbp)
> > /* overall command status of utrd */
> > ocs = ufshcd_get_tr_ocs(lrbp);
> >
> > + if (hba->quirks & UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR) {
> > + if (be32_to_cpu(lrbp->ucd_rsp_ptr->header.dword_1) &
> > + MASK_RSP_UPIU_RESULT)
> > + ocs = OCS_SUCCESS;
> > + }
> > +
> Not sure that I follow what this quirk is all about.
> Your code overrides ocs by open coding ufshcd_get_rsp_upiu_result.
>
> Normally OCS is in utp transfer req descriptor, dword 2, bits 0..7.
> My understanding from your description, is that some fatal error might
> occur, But the host controller does not report it, and it still needs to
> be checked in the response upiu.
> Evidently you are not doing so.
> Please elaborate your description.
>
> P.S.
> The ocs is being evaluated in device management commands as well, Isn't
> this something you need to attend?
>
> Thanks,
> Avri
>
> > switch (ocs) {
> > case OCS_SUCCESS:
> > result = ufshcd_get_req_rsp(lrbp->ucd_rsp_ptr);
> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index a9b9ace9fc72..e1d09c2c4302 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -541,6 +541,12 @@ enum ufshcd_quirks {
> > * resolution of the values of PRDTO and PRDTL in UTRD as byte.
> > */
> > UFSHCD_QUIRK_PRDT_BYTE_GRAN = 1 << 9,
> > +
> > + /*
> > + * This quirk needs to be enabled if the host controller reports
> > + * OCS FATAL ERROR with device error through sense data
> > + */
> > + UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR = 1 << 10,
> > };
> >
> > enum ufshcd_caps {
> > --
> > 2.17.1
Avri
As specified in the spec, OCS isn't supposed to refer to the contents of RESPONSE UPIU.
But, Exynos host behaves like that in some cases, e.g. a value of 'state' in is isn't GOOD(00h).
For QUERY RESPONSE, its offset, i.e. " dword_1" is reserved, so currently no impact, I think.
But if you feel another condition is necessary to identify if this request is QUERY REQEUST or not, we can add more.
Thanks
Powered by blists - more mailing lists