| Message ID | 20231201151417.65500-6-manivannan.sadhasivam@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [01/13] scsi: ufs: qcom: Use clk_bulk APIs for managing lane clocks | expand |
On Fri, Dec 01, 2023 at 08:44:09PM +0530, Manivannan Sadhasivam wrote: > core_reset is optional, so there is no need to warn the user if it is not > available (that too not while doing host reset each time). What's the bit in the parenthesis mean here? I'm having a hard time following. Otherwise, this looks good to me. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/ufs/host/ufs-qcom.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > index dc93b1c5ca74..d474de0739e4 100644 > --- a/drivers/ufs/host/ufs-qcom.c > +++ b/drivers/ufs/host/ufs-qcom.c > @@ -296,10 +296,8 @@ static int ufs_qcom_host_reset(struct ufs_hba *hba) > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > bool reenable_intr; > > - if (!host->core_reset) { > - dev_warn(hba->dev, "%s: reset control not set\n", __func__); > + if (!host->core_reset) > return 0; > - } > > reenable_intr = hba->is_irq_enabled; > disable_irq(hba->irq); > -- > 2.25.1 > >
On Wed, Dec 06, 2023 at 12:36:41PM -0600, Andrew Halaney wrote: > On Fri, Dec 01, 2023 at 08:44:09PM +0530, Manivannan Sadhasivam wrote: > > core_reset is optional, so there is no need to warn the user if it is not > > available (that too not while doing host reset each time). > > What's the bit in the parenthesis mean here? I'm having a hard time > following. Otherwise, this looks good to me. > I was just mentioning that the core reset can happen multiple times depending on the scenario, so it doesn't make sense to print a warning each time if the reset was not available. - Mani > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > drivers/ufs/host/ufs-qcom.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > > index dc93b1c5ca74..d474de0739e4 100644 > > --- a/drivers/ufs/host/ufs-qcom.c > > +++ b/drivers/ufs/host/ufs-qcom.c > > @@ -296,10 +296,8 @@ static int ufs_qcom_host_reset(struct ufs_hba *hba) > > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > > bool reenable_intr; > > > > - if (!host->core_reset) { > > - dev_warn(hba->dev, "%s: reset control not set\n", __func__); > > + if (!host->core_reset) > > return 0; > > - } > > > > reenable_intr = hba->is_irq_enabled; > > disable_irq(hba->irq); > > -- > > 2.25.1 > > > > >
On Thu, Dec 07, 2023 at 10:48:35AM +0530, Manivannan Sadhasivam wrote: > On Wed, Dec 06, 2023 at 12:36:41PM -0600, Andrew Halaney wrote: > > On Fri, Dec 01, 2023 at 08:44:09PM +0530, Manivannan Sadhasivam wrote: > > > core_reset is optional, so there is no need to warn the user if it is not > > > available (that too not while doing host reset each time). > > > > What's the bit in the parenthesis mean here? I'm having a hard time > > following. Otherwise, this looks good to me. > > > > I was just mentioning that the core reset can happen multiple times depending on > the scenario, so it doesn't make sense to print a warning each time if the reset > was not available. Ahh gotcha, maybe be a little more verbose in that part on the next spin? As is I'm struggling to get that from the commit message's text. Reviewed-by: Andrew Halaney <ahalaney@redhat.com> > > - Mani > > > > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > > --- > > > drivers/ufs/host/ufs-qcom.c | 4 +--- > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > > > index dc93b1c5ca74..d474de0739e4 100644 > > > --- a/drivers/ufs/host/ufs-qcom.c > > > +++ b/drivers/ufs/host/ufs-qcom.c > > > @@ -296,10 +296,8 @@ static int ufs_qcom_host_reset(struct ufs_hba *hba) > > > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > > > bool reenable_intr; > > > > > > - if (!host->core_reset) { > > > - dev_warn(hba->dev, "%s: reset control not set\n", __func__); > > > + if (!host->core_reset) > > > return 0; > > > - } > > > > > > reenable_intr = hba->is_irq_enabled; > > > disable_irq(hba->irq); > > > -- > > > 2.25.1 > > > > > > > > > > -- > மணிவண்ணன் சதாசிவம் >
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index dc93b1c5ca74..d474de0739e4 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -296,10 +296,8 @@ static int ufs_qcom_host_reset(struct ufs_hba *hba) struct ufs_qcom_host *host = ufshcd_get_variant(hba); bool reenable_intr; - if (!host->core_reset) { - dev_warn(hba->dev, "%s: reset control not set\n", __func__); + if (!host->core_reset) return 0; - } reenable_intr = hba->is_irq_enabled; disable_irq(hba->irq);
core_reset is optional, so there is no need to warn the user if it is not available (that too not while doing host reset each time). Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- drivers/ufs/host/ufs-qcom.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)