| Message ID | 20240129-ufs-core-reset-fix-v1-2-7ac628aa735f@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | ufs: qcom: Mark "core_reset" as required property | expand |
On Mon, Jan 29, 2024 at 01:22:05PM +0530, Manivannan Sadhasivam wrote: > core_reset is not an optional property for the platforms supported in > upstream. Only for the non-upstreamed legacy platforms it is optional. > But somehow a few of the upstreamed platforms do not pass this property > by mistake. > > So clarify the comment to make it clear that even though core_reset is > required, it is kept as optional to support the DTs that do not pass this > property. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/ufs/host/ufs-qcom.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > index 39eef470f8fa..32760506dfeb 100644 > --- a/drivers/ufs/host/ufs-qcom.c > +++ b/drivers/ufs/host/ufs-qcom.c > @@ -1027,7 +1027,11 @@ static int ufs_qcom_init(struct ufs_hba *hba) > host->hba = hba; > ufshcd_set_variant(hba, host); > > - /* Setup the optional reset control of HCI */ > + /* > + * Even though core_reset is required on all platforms, some DTs never > + * passed this property. So we have to keep it optional for supporting > + * them. > + */ Any desire to print a warning if !host->core_reset? I'll defer to Qualcomm to review since they can confirm the accuracy past Can's comment, but this looks good to me for what its worth. > host->core_reset = devm_reset_control_get_optional(hba->dev, "rst"); > if (IS_ERR(host->core_reset)) { > err = dev_err_probe(dev, PTR_ERR(host->core_reset), > > -- > 2.25.1 > >
On Mon, Jan 29, 2024 at 02:57:20PM -0600, Andrew Halaney wrote: > On Mon, Jan 29, 2024 at 01:22:05PM +0530, Manivannan Sadhasivam wrote: > > core_reset is not an optional property for the platforms supported in > > upstream. Only for the non-upstreamed legacy platforms it is optional. > > But somehow a few of the upstreamed platforms do not pass this property > > by mistake. > > > > So clarify the comment to make it clear that even though core_reset is > > required, it is kept as optional to support the DTs that do not pass this > > property. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > drivers/ufs/host/ufs-qcom.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > > index 39eef470f8fa..32760506dfeb 100644 > > --- a/drivers/ufs/host/ufs-qcom.c > > +++ b/drivers/ufs/host/ufs-qcom.c > > @@ -1027,7 +1027,11 @@ static int ufs_qcom_init(struct ufs_hba *hba) > > host->hba = hba; > > ufshcd_set_variant(hba, host); > > > > - /* Setup the optional reset control of HCI */ > > + /* > > + * Even though core_reset is required on all platforms, some DTs never > > + * passed this property. So we have to keep it optional for supporting > > + * them. > > + */ > > Any desire to print a warning if !host->core_reset? I'll defer to > Qualcomm to review since they can confirm the accuracy past Can's > comment, but this looks good to me for what its worth. > My only worry is that the existing users of the legacy DTs will get annoyed by the warning. And I'm not sure if we can do that. - Mani
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index 39eef470f8fa..32760506dfeb 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -1027,7 +1027,11 @@ static int ufs_qcom_init(struct ufs_hba *hba) host->hba = hba; ufshcd_set_variant(hba, host); - /* Setup the optional reset control of HCI */ + /* + * Even though core_reset is required on all platforms, some DTs never + * passed this property. So we have to keep it optional for supporting + * them. + */ host->core_reset = devm_reset_control_get_optional(hba->dev, "rst"); if (IS_ERR(host->core_reset)) { err = dev_err_probe(dev, PTR_ERR(host->core_reset),
core_reset is not an optional property for the platforms supported in upstream. Only for the non-upstreamed legacy platforms it is optional. But somehow a few of the upstreamed platforms do not pass this property by mistake. So clarify the comment to make it clear that even though core_reset is required, it is kept as optional to support the DTs that do not pass this property. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- drivers/ufs/host/ufs-qcom.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)