| Message ID | 20231201151417.65500-8-manivannan.sadhasivam@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | scsi: ufs: qcom: Minor code cleanups | expand |
On Fri, Dec 01, 2023 at 08:44:11PM +0530, Manivannan Sadhasivam wrote: > Even though core_reset is optional, a failure during assert/deassert should > be considered fatal, if core_reset is available. So fail > ufs_qcom_power_up_sequence() if an error happens during reset and also get > rid of the redundant warning as the ufs_qcom_host_reset() function itself > prints error messages. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Reviewed-by: Andrew Halaney <ahalaney@redhat.com> > --- > drivers/ufs/host/ufs-qcom.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > index 604273a22afd..4948dd732aae 100644 > --- a/drivers/ufs/host/ufs-qcom.c > +++ b/drivers/ufs/host/ufs-qcom.c > @@ -359,8 +359,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) > /* Reset UFS Host Controller and PHY */ > ret = ufs_qcom_host_reset(hba); I noticed that ufs_qcom_host_reset() doesn't return an error if reset_control_deassert() fails. Can you address this in the next spin of the series (I don't think its in the following patches that I glanced through). Thanks, Andrew > if (ret) > - dev_warn(hba->dev, "%s: host reset returned %d\n", > - __func__, ret); > + return ret; > > /* phy initialization - calibrate the phy */ > ret = phy_init(phy); > -- > 2.25.1 > >
On Wed, Dec 06, 2023 at 12:41:28PM -0600, Andrew Halaney wrote: > On Fri, Dec 01, 2023 at 08:44:11PM +0530, Manivannan Sadhasivam wrote: > > Even though core_reset is optional, a failure during assert/deassert should > > be considered fatal, if core_reset is available. So fail > > ufs_qcom_power_up_sequence() if an error happens during reset and also get > > rid of the redundant warning as the ufs_qcom_host_reset() function itself > > prints error messages. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > Reviewed-by: Andrew Halaney <ahalaney@redhat.com> > > --- > > drivers/ufs/host/ufs-qcom.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > > index 604273a22afd..4948dd732aae 100644 > > --- a/drivers/ufs/host/ufs-qcom.c > > +++ b/drivers/ufs/host/ufs-qcom.c > > @@ -359,8 +359,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) > > /* Reset UFS Host Controller and PHY */ > > ret = ufs_qcom_host_reset(hba); > > I noticed that ufs_qcom_host_reset() doesn't return an error if > reset_control_deassert() fails. Can you address this in the next spin of > the series (I don't think its in the following patches that I glanced > through). > Right. I'll fix it in next version. - Mani > Thanks, > Andrew > > > if (ret) > > - dev_warn(hba->dev, "%s: host reset returned %d\n", > > - __func__, ret); > > + return ret; > > > > /* phy initialization - calibrate the phy */ > > ret = phy_init(phy); > > -- > > 2.25.1 > > > > > >
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index 604273a22afd..4948dd732aae 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -359,8 +359,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) /* Reset UFS Host Controller and PHY */ ret = ufs_qcom_host_reset(hba); if (ret) - dev_warn(hba->dev, "%s: host reset returned %d\n", - __func__, ret); + return ret; /* phy initialization - calibrate the phy */ ret = phy_init(phy);
Even though core_reset is optional, a failure during assert/deassert should be considered fatal, if core_reset is available. So fail ufs_qcom_power_up_sequence() if an error happens during reset and also get rid of the redundant warning as the ufs_qcom_host_reset() function itself prints error messages. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- drivers/ufs/host/ufs-qcom.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)