LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Kiwoong Kim" <kwmad.kim@samsung.com>
To: "'Bart Van Assche'" <bvanassche@acm.org>,
	<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>, <cang@codeaurora.org>,
	<adrian.hunter@intel.com>, <sc.suh@samsung.com>,
	<hy50.seo@samsung.com>, <sh425.lee@samsung.com>,
	<bhoon95.kim@samsung.com>
Subject: RE: [RFC PATCH v1 2/2] scsi: ufs: ufs-exynos: implement exynos isr
Date: Mon, 9 Aug 2021 16:31:50 +0900	[thread overview]
Message-ID: <000001d78cf0$9eb0de30$dc129a90$@samsung.com> (raw)
In-Reply-To: <597a96f4-9cd4-c1bb-5c8d-dd5d00f0948d@acm.org>

> On 8/5/21 11:34 PM, Kiwoong Kim wrote:
> > Based on some events in the real world
> 
> Which events? Please clarify.
> 
> > I implement
> > this to block the host's working in some abnormal conditions using an
> > vendor specific interrupt for cases that some contexts of a pending
> > request in the host isn't the same with those of its corresponding
> > UPIUs if they should have been the same exactly.
> 
> The entire patch description sounds very vague to me. Please make the
> description more clear.

I'll describe a bit clearer in the next version.

> 
> > +enum exynos_ufs_vs_interrupt {
> > +	/*
> > +	 * This occurs when information of a pending request isn't
> > +	 * the same with incoming UPIU for the request. For example,
> > +	 * if UFS driver rings with task tag #1, subsequential UPIUs
> > +	 * for this must have one as the value of task tag. But if
> > +	 * it's corrutped until the host receives it or incoming UPIUs
> > +	 * has an unexpected value for task tag, this raises.
> > +	 */
> > +	RX_UPIU_HIT_ERROR	= 1 << 19,
> > +};
> 
> The above description needs to be improved. If a request is submitted with
> task tag one, only one UPIU can have that task tag instead of all
> subsequent UPIUs.

Thank you for your opinion. In an ideal situation where there is no negative impact
from outside the host, yes, you're right, but in the real world, it could be not.
Let me give you one representative example.
There has been some events that a host has one as tag number of a pending request
that should have been originally two, or a device sends a UPIU with two of tag number even
when a host has one as tag number of its corresponding request.
I remember that the first case occurred because of integrity problems
from such as lack of voltage margin of a specific power domain or whatever
and the second case did because of malfunctions of the device.
If those events are temporary, it might not raise some errors.
That means delivering wrong data to file system could be also possible
even if they're rare, and its consequences would be unpredictable, I think.

Speaking the point of view of UFS specifications, yes, those events should
not happen. Now I'm just trying to make the driver fit with the real situations,
especially for cases with abnormal conditions.
In this case, my choice is to block the host's operations and
these situations could be architecture-specific.

> 
> >   	hci_writel(ufs, UFS_SW_RST_MASK, HCI_SW_RST);
> > -
> >   	do {
> >   		if (!(hci_readl(ufs, HCI_SW_RST) & UFS_SW_RST_MASK))
> > -			goto out;
> > +			return 0;
> >   	} while (time_before(jiffies, timeout));
> 
> Since the above loop is a busy-waiting loop, please insert an msleep() or
> cpu_relax() call.
> 
> > +	 * some unexpected events could happen, such as tranferring
>                                                          ^^^^^^^^^^^ Please fix the
> spelling of this word.

Got it.
> 
> Thanks,
> 
> Bart.


  reply	other threads:[~2021-08-09  7:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20210806064923epcas2p13dd6b442eed02404d87684afd9c1b229@epcas2p1.samsung.com>
2021-08-06  6:34 ` [RFC PATCH v1 0/2] scsi: ufs: introduce vendor isr Kiwoong Kim
     [not found]   ` <CGME20210806064924epcas2p4572538fd1fa7a73d8262737e38a9b537@epcas2p4.samsung.com>
2021-08-06  6:34     ` [RFC PATCH v1 1/2] " Kiwoong Kim
2021-08-06 16:18       ` Bart Van Assche
2021-08-09  7:33         ` Kiwoong Kim
     [not found]   ` <CGME20210806064925epcas2p2ba7e711758614384c17648d4924d025c@epcas2p2.samsung.com>
2021-08-06  6:34     ` [RFC PATCH v1 2/2] scsi: ufs: ufs-exynos: implement exynos isr Kiwoong Kim
2021-08-06 16:37       ` Bart Van Assche
2021-08-09  7:31         ` Kiwoong Kim [this message]
2021-08-06 16:14   ` [RFC PATCH v1 0/2] scsi: ufs: introduce vendor isr Bart Van Assche
2021-08-08  5:56     ` Avri Altman
2021-08-09  7:46     ` Kiwoong Kim
2021-08-09 16:08       ` Bart Van Assche
2021-08-13  5:31         ` Kiwoong Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='000001d78cf0$9eb0de30$dc129a90$@samsung.com' \
    --to=kwmad.kim@samsung.com \
    --cc=adrian.hunter@intel.com \
    --cc=alim.akhtar@samsung.com \
    --cc=avri.altman@wdc.com \
    --cc=beanhuo@micron.com \
    --cc=bhoon95.kim@samsung.com \
    --cc=bvanassche@acm.org \
    --cc=cang@codeaurora.org \
    --cc=hy50.seo@samsung.com \
    --cc=jejb@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=sc.suh@samsung.com \
    --cc=sh425.lee@samsung.com \
    --subject='RE: [RFC PATCH v1 2/2] scsi: ufs: ufs-exynos: implement exynos isr' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).