| Message ID | 20220423140245.394092-2-manivannan.sadhasivam@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Qcom UFS driver updates | expand |
On Sat 23 Apr 07:02 PDT 2022, 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..bee81b45299e 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_probe() does this comparison internally, so you can omit it here. With that removed, feel free to add my: Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > + dev_err_probe(dev, err, "Failed to get reset control\n"); > + goto out_variant_clear; > } > > /* Fire up the reset controller. Failure here is non-fatal. */ > -- > 2.25.1 >
On Sat, Apr 23, 2022 at 07:59:17AM -0700, Bjorn Andersson wrote: > On Sat 23 Apr 07:02 PDT 2022, 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..bee81b45299e 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_probe() does this comparison internally, so you can omit it > here. > > With that removed, feel free to add my: > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > Regards, > Bjorn +1; well with that change in place: Reviewed-by: Andrew Halaney <ahalaney@redhat.com> Thanks, Andrew > > > + dev_err_probe(dev, err, "Failed to get reset control\n"); > > + 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..bee81b45299e 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_probe(dev, err, "Failed to get reset control\n"); + 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(-)