| Message ID | 20220422132140.313390-2-manivannan.sadhasivam@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Qcom UFS driver updates | expand |
On Fri, Apr 22, 2022 at 06:51:36PM +0530, Manivannan Sadhasivam wrote: > On Qcom UFS platforms, the reset control line seems to be optional > (for SoCs like MSM8996 and probably for others too). The current logic > tries to mimic the `devm_reset_control_get_optional()` API but it also > continues the probe if there is an error with the declared reset line in > DT/ACPI. > > In an ideal case, if the reset line is not declared in DT/ACPI, the probe > should continue. But if there is problem in acquiring the declared reset > line (like EPROBE_DEFER) it should fail and return the appropriate error > code. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/scsi/ufs/ufs-qcom.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c > index 0d2e950d0865..5db0fd922062 100644 > --- a/drivers/scsi/ufs/ufs-qcom.c > +++ b/drivers/scsi/ufs/ufs-qcom.c > @@ -1002,13 +1002,13 @@ static int ufs_qcom_init(struct ufs_hba *hba) > host->hba = hba; > ufshcd_set_variant(hba, host); > > - /* Setup the reset control of HCI */ > - host->core_reset = devm_reset_control_get(hba->dev, "rst"); > + /* Setup the optional reset control of HCI */ > + host->core_reset = devm_reset_control_get_optional(hba->dev, "rst"); > if (IS_ERR(host->core_reset)) { > err = PTR_ERR(host->core_reset); > - dev_warn(dev, "Failed to get reset control %d\n", err); > - host->core_reset = NULL; > - err = 0; > + if (err != -EPROBE_DEFER) > + dev_err(dev, "Failed to get reset control %d\n", err); Could we use dev_err_probe() here? Otherwise, looks good to me. > + goto out_variant_clear; > } > > /* Fire up the reset controller. Failure here is non-fatal. */ > -- > 2.25.1 >
On Fri, Apr 22, 2022 at 10:40:10AM -0500, Andrew Halaney wrote: > On Fri, Apr 22, 2022 at 06:51:36PM +0530, Manivannan Sadhasivam wrote: > > On Qcom UFS platforms, the reset control line seems to be optional > > (for SoCs like MSM8996 and probably for others too). The current logic > > tries to mimic the `devm_reset_control_get_optional()` API but it also > > continues the probe if there is an error with the declared reset line in > > DT/ACPI. > > > > In an ideal case, if the reset line is not declared in DT/ACPI, the probe > > should continue. But if there is problem in acquiring the declared reset > > line (like EPROBE_DEFER) it should fail and return the appropriate error > > code. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > drivers/scsi/ufs/ufs-qcom.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c > > index 0d2e950d0865..5db0fd922062 100644 > > --- a/drivers/scsi/ufs/ufs-qcom.c > > +++ b/drivers/scsi/ufs/ufs-qcom.c > > @@ -1002,13 +1002,13 @@ static int ufs_qcom_init(struct ufs_hba *hba) > > host->hba = hba; > > ufshcd_set_variant(hba, host); > > > > - /* Setup the reset control of HCI */ > > - host->core_reset = devm_reset_control_get(hba->dev, "rst"); > > + /* Setup the optional reset control of HCI */ > > + host->core_reset = devm_reset_control_get_optional(hba->dev, "rst"); > > if (IS_ERR(host->core_reset)) { > > err = PTR_ERR(host->core_reset); > > - dev_warn(dev, "Failed to get reset control %d\n", err); > > - host->core_reset = NULL; > > - err = 0; > > + if (err != -EPROBE_DEFER) > > + dev_err(dev, "Failed to get reset control %d\n", err); > > Could we use dev_err_probe() here? > Yes. Will do the same for patch 2/5 as well. Thanks, Mani > Otherwise, looks good to me. > > > + goto out_variant_clear; > > } > > > > /* Fire up the reset controller. Failure here is non-fatal. */ > > -- > > 2.25.1 > > >
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c index 0d2e950d0865..5db0fd922062 100644 --- a/drivers/scsi/ufs/ufs-qcom.c +++ b/drivers/scsi/ufs/ufs-qcom.c @@ -1002,13 +1002,13 @@ static int ufs_qcom_init(struct ufs_hba *hba) host->hba = hba; ufshcd_set_variant(hba, host); - /* Setup the reset control of HCI */ - host->core_reset = devm_reset_control_get(hba->dev, "rst"); + /* Setup the optional reset control of HCI */ + host->core_reset = devm_reset_control_get_optional(hba->dev, "rst"); if (IS_ERR(host->core_reset)) { err = PTR_ERR(host->core_reset); - dev_warn(dev, "Failed to get reset control %d\n", err); - host->core_reset = NULL; - err = 0; + if (err != -EPROBE_DEFER) + dev_err(dev, "Failed to get reset control %d\n", err); + goto out_variant_clear; } /* Fire up the reset controller. Failure here is non-fatal. */
On Qcom UFS platforms, the reset control line seems to be optional (for SoCs like MSM8996 and probably for others too). The current logic tries to mimic the `devm_reset_control_get_optional()` API but it also continues the probe if there is an error with the declared reset line in DT/ACPI. In an ideal case, if the reset line is not declared in DT/ACPI, the probe should continue. But if there is problem in acquiring the declared reset line (like EPROBE_DEFER) it should fail and return the appropriate error code. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- drivers/scsi/ufs/ufs-qcom.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)