* [RFC PATCH v2] scsi: ufs: set STATE_ERROR when ufshcd_probe_hba() failed [not found] <CGME20200717073950epcas2p3fe023138e3c04e706a1afb887998eb5c@epcas2p3.samsung.com> @ 2020-07-17 7:31 ` Lee Sang Hyun 2020-07-19 7:14 ` Avri Altman 2020-07-19 10:19 ` Can Guo 0 siblings, 2 replies; 4+ messages in thread From: Lee Sang Hyun @ 2020-07-17 7:31 UTC (permalink / raw) To: linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche, grant.jung, sc.suh, hy50.seo, sh425.lee, kwmad.kim set STATE_ERR like below to prevent a lockup(IO stuck) when ufshcd_probe_hba() returns error. Change-Id: I6c85ff290503cc9414d7f5fdd934295497b854ff Signed-off-by: Lee Sang Hyun <sh425.lee@samsung.com> --- drivers/scsi/ufs/ufshcd.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index ad4fc82..37e4105 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -7368,6 +7368,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool async) { int ret; ktime_t start = ktime_get(); + unsigned long flags; ret = ufshcd_link_startup(hba); if (ret) @@ -7439,6 +7440,11 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool async) ufshcd_auto_hibern8_enable(hba); out: + if (ret) { + spin_lock_irqsave(hba->host->host_lock, flags); + hba->ufshcd_state = UFSHCD_STATE_ERROR; + spin_unlock_irqrestore(hba->host->host_lock, flags); + } trace_ufshcd_init(dev_name(hba->dev), ret, ktime_to_us(ktime_sub(ktime_get(), start)), -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [RFC PATCH v2] scsi: ufs: set STATE_ERROR when ufshcd_probe_hba() failed 2020-07-17 7:31 ` [RFC PATCH v2] scsi: ufs: set STATE_ERROR when ufshcd_probe_hba() failed Lee Sang Hyun @ 2020-07-19 7:14 ` Avri Altman 2020-07-19 10:19 ` Can Guo 1 sibling, 0 replies; 4+ messages in thread From: Avri Altman @ 2020-07-19 7:14 UTC (permalink / raw) To: Lee Sang Hyun, linux-scsi@vger.kernel.org, alim.akhtar@samsung.com, jejb@linux.ibm.com, martin.petersen@oracle.com, beanhuo@micron.com, asutoshd@codeaurora.org, cang@codeaurora.org, bvanassche@acm.org, grant.jung@samsung.com, sc.suh@samsung.com, hy50.seo@samsung.com, kwmad.kim@samsung.com > > set STATE_ERR like below to prevent a lockup(IO stuck) > when ufshcd_probe_hba() returns error. > > Change-Id: I6c85ff290503cc9414d7f5fdd934295497b854ff No need for the gerrit change id > Signed-off-by: Lee Sang Hyun <sh425.lee@samsung.com> Reviewed-by: Avri Altman <avri.altman@wdc.com> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH v2] scsi: ufs: set STATE_ERROR when ufshcd_probe_hba() failed 2020-07-17 7:31 ` [RFC PATCH v2] scsi: ufs: set STATE_ERROR when ufshcd_probe_hba() failed Lee Sang Hyun 2020-07-19 7:14 ` Avri Altman @ 2020-07-19 10:19 ` Can Guo 2020-07-21 12:10 ` ??? 1 sibling, 1 reply; 4+ messages in thread From: Can Guo @ 2020-07-19 10:19 UTC (permalink / raw) To: Lee Sang Hyun Cc: linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, asutoshd, bvanassche, grant.jung, sc.suh, hy50.seo, kwmad.kim Hi Sang Hyun, On 2020-07-17 15:31, Lee Sang Hyun wrote: > set STATE_ERR like below to prevent a lockup(IO stuck) > when ufshcd_probe_hba() returns error. > > Change-Id: I6c85ff290503cc9414d7f5fdd934295497b854ff > Signed-off-by: Lee Sang Hyun <sh425.lee@samsung.com> > --- > drivers/scsi/ufs/ufshcd.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index ad4fc82..37e4105 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -7368,6 +7368,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, > bool async) > { > int ret; > ktime_t start = ktime_get(); > + unsigned long flags; > > ret = ufshcd_link_startup(hba); > if (ret) > @@ -7439,6 +7440,11 @@ static int ufshcd_probe_hba(struct ufs_hba > *hba, bool async) > ufshcd_auto_hibern8_enable(hba); > > out: > + if (ret) { > + spin_lock_irqsave(hba->host->host_lock, flags); > + hba->ufshcd_state = UFSHCD_STATE_ERROR; > + spin_unlock_irqrestore(hba->host->host_lock, flags); > + } > > trace_ufshcd_init(dev_name(hba->dev), ret, > ktime_to_us(ktime_sub(ktime_get(), start)), This change is included in my change "scsi: ufs: Fix up and simplify error recovery mechanism". Besides, this change seems not complete because #1 You are only protecting your changes with spin lock in ufshcd_probe_hba, what about the other line "hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;"? #2 As you are giving "hba->ufshcd_state = UFSHCD_STATE_ERROR;" in ufshcd_probe_hba, why keep the same lines in ufshcd_error_handler and ufshcd_eh_host_reset_handler? Thanks, Can Guo. ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [RFC PATCH v2] scsi: ufs: set STATE_ERROR when ufshcd_probe_hba() failed 2020-07-19 10:19 ` Can Guo @ 2020-07-21 12:10 ` ??? 0 siblings, 0 replies; 4+ messages in thread From: ??? @ 2020-07-21 12:10 UTC (permalink / raw) To: 'Can Guo' Cc: linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, asutoshd, bvanassche, grant.jung, sc.suh, hy50.seo, kwmad.kim > Hi Sang Hyun, > > On 2020-07-17 15:31, Lee Sang Hyun wrote: > > set STATE_ERR like below to prevent a lockup(IO stuck) when > > ufshcd_probe_hba() returns error. > > > > Change-Id: I6c85ff290503cc9414d7f5fdd934295497b854ff > > Signed-off-by: Lee Sang Hyun <sh425.lee@samsung.com> > > --- > > drivers/scsi/ufs/ufshcd.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > index ad4fc82..37e4105 100644 > > --- a/drivers/scsi/ufs/ufshcd.c > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -7368,6 +7368,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, > > bool async) { > > int ret; > > ktime_t start = ktime_get(); > > + unsigned long flags; > > > > ret = ufshcd_link_startup(hba); > > if (ret) > > @@ -7439,6 +7440,11 @@ static int ufshcd_probe_hba(struct ufs_hba > > *hba, bool async) > > ufshcd_auto_hibern8_enable(hba); > > > > out: > > + if (ret) { > > + spin_lock_irqsave(hba->host->host_lock, flags); > > + hba->ufshcd_state = UFSHCD_STATE_ERROR; > > + spin_unlock_irqrestore(hba->host->host_lock, flags); > > + } > > > > trace_ufshcd_init(dev_name(hba->dev), ret, > > ktime_to_us(ktime_sub(ktime_get(), start)), > > This change is included in my change > "scsi: ufs: Fix up and simplify error recovery mechanism". "scsi: ufs: Fix up and simplify error recovery mechanism". I checked the patch, and confirmed that the contents of my patch were also included. > > Besides, this change seems not complete because > > #1 You are only protecting your changes with spin lock in > ufshcd_probe_hba, what about the other line "hba->ufshcd_state = > UFSHCD_STATE_OPERATIONAL;"? > > #2 As you are giving "hba->ufshcd_state = UFSHCD_STATE_ERROR;" > in ufshcd_probe_hba, why keep the same lines in ufshcd_error_handler and > ufshcd_eh_host_reset_handler? > > Thanks, > > Can Guo. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-21 12:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20200717073950epcas2p3fe023138e3c04e706a1afb887998eb5c@epcas2p3.samsung.com>
2020-07-17 7:31 ` [RFC PATCH v2] scsi: ufs: set STATE_ERROR when ufshcd_probe_hba() failed Lee Sang Hyun
2020-07-19 7:14 ` Avri Altman
2020-07-19 10:19 ` Can Guo
2020-07-21 12:10 ` ???
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.