mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] soc: qcom: ice: Stop probe deferring once ICE isn't detected
@ 2026-02-02  8:25 Sumit Garg
  2026-02-02 10:51 ` Konrad Dybcio
  0 siblings, 1 reply; 5+ messages in thread
From: Sumit Garg @ 2026-02-02  8:25 UTC (permalink / raw)
  To: linux-arm-msm
  Cc: andersson, konradybcio, konrad.dybcio, robh+dt, abelvesa, mani,
	linux-kernel, Sumit Garg

From: Sumit Garg <sumit.garg@oss.qualcomm.com>

ICE related SCM calls may not be supported in every TZ environment like
OP-TEE or a no-TZ environment too. So let's try to stop probe deferring
when it's known that ICE feature isn't supported.

This problem only came to notice after the inline encryption drivers were
enabled in the arm64 defconfig by: commit 5f37788adedd ("arm64: defconfig:
Enable SCSI UFS Crypto and Block Inline encryption drivers").

Fixes: 2afbf43a4aec ("soc: qcom: Make the Qualcomm UFS/SDCC ICE a dedicated driver")
Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
---

Changes in v2:
- Keep the probe deferring intact but stop it once it's know ICE SCM
  calls aren't supported by the TZ firmware.

 drivers/soc/qcom/ice.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index b203bc685cad..5a630c9010ee 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -559,7 +559,7 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
 
 	if (!qcom_scm_ice_available()) {
 		dev_warn(dev, "ICE SCM interface not found\n");
-		return NULL;
+		return ERR_PTR(-EOPNOTSUPP);
 	}
 
 	engine = devm_kzalloc(dev, sizeof(*engine), GFP_KERNEL);
@@ -648,11 +648,14 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
 	}
 
 	ice = platform_get_drvdata(pdev);
-	if (!ice) {
+	if (IS_ERR_OR_NULL(ice)) {
 		dev_err(dev, "Cannot get ice instance from %s\n",
 			dev_name(&pdev->dev));
 		platform_device_put(pdev);
-		return ERR_PTR(-EPROBE_DEFER);
+		if (PTR_ERR(ice) == -EOPNOTSUPP)
+			return NULL;
+		else
+			return ERR_PTR(-EPROBE_DEFER);
 	}
 
 	link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_SUPPLIER);
@@ -726,7 +729,7 @@ static int qcom_ice_probe(struct platform_device *pdev)
 	}
 
 	engine = qcom_ice_create(&pdev->dev, base);
-	if (IS_ERR(engine))
+	if (IS_ERR(engine) && PTR_ERR(engine) != -EOPNOTSUPP)
 		return PTR_ERR(engine);
 
 	platform_set_drvdata(pdev, engine);
-- 
2.51.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] soc: qcom: ice: Stop probe deferring once ICE isn't detected
  2026-02-02  8:25 [PATCH v2] soc: qcom: ice: Stop probe deferring once ICE isn't detected Sumit Garg
@ 2026-02-02 10:51 ` Konrad Dybcio
  2026-02-02 10:55   ` Manivannan Sadhasivam
  0 siblings, 1 reply; 5+ messages in thread
From: Konrad Dybcio @ 2026-02-02 10:51 UTC (permalink / raw)
  To: Sumit Garg, linux-arm-msm, Dmitry Baryshkov
  Cc: andersson, konradybcio, robh+dt, abelvesa, mani, linux-kernel,
	Sumit Garg

On 2/2/26 9:25 AM, Sumit Garg wrote:
> From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> 
> ICE related SCM calls may not be supported in every TZ environment like
> OP-TEE or a no-TZ environment too. So let's try to stop probe deferring
> when it's known that ICE feature isn't supported.
> 
> This problem only came to notice after the inline encryption drivers were
> enabled in the arm64 defconfig by: commit 5f37788adedd ("arm64: defconfig:
> Enable SCSI UFS Crypto and Block Inline encryption drivers").
> 
> Fixes: 2afbf43a4aec ("soc: qcom: Make the Qualcomm UFS/SDCC ICE a dedicated driver")
> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> ---
> 
> Changes in v2:
> - Keep the probe deferring intact but stop it once it's know ICE SCM
>   calls aren't supported by the TZ firmware.
> 
>  drivers/soc/qcom/ice.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index b203bc685cad..5a630c9010ee 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -559,7 +559,7 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
>  
>  	if (!qcom_scm_ice_available()) {
>  		dev_warn(dev, "ICE SCM interface not found\n");
> -		return NULL;
> +		return ERR_PTR(-EOPNOTSUPP);
>  	}
>  
>  	engine = devm_kzalloc(dev, sizeof(*engine), GFP_KERNEL);
> @@ -648,11 +648,14 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
>  	}
>  
>  	ice = platform_get_drvdata(pdev);
> -	if (!ice) {
> +	if (IS_ERR_OR_NULL(ice)) {
>  		dev_err(dev, "Cannot get ice instance from %s\n",
>  			dev_name(&pdev->dev));
>  		platform_device_put(pdev);
> -		return ERR_PTR(-EPROBE_DEFER);
> +		if (PTR_ERR(ice) == -EOPNOTSUPP)
> +			return NULL;

The consumer drivers check specifically for -EOPNOTSUPP, let's
just return that

> +		else
> +			return ERR_PTR(-EPROBE_DEFER);
>  	}
>  
>  	link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_SUPPLIER);
> @@ -726,7 +729,7 @@ static int qcom_ice_probe(struct platform_device *pdev)
>  	}
>  
>  	engine = qcom_ice_create(&pdev->dev, base);
> -	if (IS_ERR(engine))
> +	if (IS_ERR(engine) && PTR_ERR(engine) != -EOPNOTSUPP)
>  		return PTR_ERR(engine);

This essentially says "probe succeeded, device not operational",
I have mixed feelings.. That said I'm not sure about the lifecycle
of a platform_device, i.e. can we set the drvdata and return an error
in .probe anyway?

Konrad

>  
>  	platform_set_drvdata(pdev, engine);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] soc: qcom: ice: Stop probe deferring once ICE isn't detected
  2026-02-02 10:51 ` Konrad Dybcio
@ 2026-02-02 10:55   ` Manivannan Sadhasivam
  2026-02-03  6:50     ` Neeraj Soni
  0 siblings, 1 reply; 5+ messages in thread
From: Manivannan Sadhasivam @ 2026-02-02 10:55 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Sumit Garg, linux-arm-msm, Dmitry Baryshkov, andersson,
	konradybcio, robh+dt, abelvesa, linux-kernel, Sumit Garg

On Mon, Feb 02, 2026 at 11:51:51AM +0100, Konrad Dybcio wrote:
> On 2/2/26 9:25 AM, Sumit Garg wrote:
> > From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > 
> > ICE related SCM calls may not be supported in every TZ environment like
> > OP-TEE or a no-TZ environment too. So let's try to stop probe deferring
> > when it's known that ICE feature isn't supported.
> > 
> > This problem only came to notice after the inline encryption drivers were
> > enabled in the arm64 defconfig by: commit 5f37788adedd ("arm64: defconfig:
> > Enable SCSI UFS Crypto and Block Inline encryption drivers").
> > 
> > Fixes: 2afbf43a4aec ("soc: qcom: Make the Qualcomm UFS/SDCC ICE a dedicated driver")
> > Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > ---
> > 
> > Changes in v2:
> > - Keep the probe deferring intact but stop it once it's know ICE SCM
> >   calls aren't supported by the TZ firmware.
> > 
> >  drivers/soc/qcom/ice.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> > index b203bc685cad..5a630c9010ee 100644
> > --- a/drivers/soc/qcom/ice.c
> > +++ b/drivers/soc/qcom/ice.c
> > @@ -559,7 +559,7 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
> >  
> >  	if (!qcom_scm_ice_available()) {
> >  		dev_warn(dev, "ICE SCM interface not found\n");
> > -		return NULL;
> > +		return ERR_PTR(-EOPNOTSUPP);
> >  	}
> >  
> >  	engine = devm_kzalloc(dev, sizeof(*engine), GFP_KERNEL);
> > @@ -648,11 +648,14 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
> >  	}
> >  
> >  	ice = platform_get_drvdata(pdev);
> > -	if (!ice) {
> > +	if (IS_ERR_OR_NULL(ice)) {
> >  		dev_err(dev, "Cannot get ice instance from %s\n",
> >  			dev_name(&pdev->dev));
> >  		platform_device_put(pdev);
> > -		return ERR_PTR(-EPROBE_DEFER);
> > +		if (PTR_ERR(ice) == -EOPNOTSUPP)
> > +			return NULL;
> 
> The consumer drivers check specifically for -EOPNOTSUPP, let's
> just return that
> 
> > +		else
> > +			return ERR_PTR(-EPROBE_DEFER);
> >  	}
> >  
> >  	link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_SUPPLIER);
> > @@ -726,7 +729,7 @@ static int qcom_ice_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	engine = qcom_ice_create(&pdev->dev, base);
> > -	if (IS_ERR(engine))
> > +	if (IS_ERR(engine) && PTR_ERR(engine) != -EOPNOTSUPP)
> >  		return PTR_ERR(engine);
> 
> This essentially says "probe succeeded, device not operational",
> I have mixed feelings.. That said I'm not sure about the lifecycle
> of a platform_device, i.e. can we set the drvdata and return an error
> in .probe anyway?
> 

No. Let's remove the probe() altogether and expose this driver as a pure
library.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] soc: qcom: ice: Stop probe deferring once ICE isn't detected
  2026-02-02 10:55   ` Manivannan Sadhasivam
@ 2026-02-03  6:50     ` Neeraj Soni
  2026-02-03 12:22       ` Manivannan Sadhasivam
  0 siblings, 1 reply; 5+ messages in thread
From: Neeraj Soni @ 2026-02-03  6:50 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Konrad Dybcio
  Cc: Sumit Garg, linux-arm-msm, Dmitry Baryshkov, andersson,
	konradybcio, robh+dt, abelvesa, linux-kernel, Sumit Garg



On 2/2/2026 4:25 PM, Manivannan Sadhasivam wrote:
> On Mon, Feb 02, 2026 at 11:51:51AM +0100, Konrad Dybcio wrote:
>> On 2/2/26 9:25 AM, Sumit Garg wrote:
>>> From: Sumit Garg <sumit.garg@oss.qualcomm.com>
>>>
>>> ICE related SCM calls may not be supported in every TZ environment like
>>> OP-TEE or a no-TZ environment too. So let's try to stop probe deferring
>>> when it's known that ICE feature isn't supported.
>>>
>>> This problem only came to notice after the inline encryption drivers were
>>> enabled in the arm64 defconfig by: commit 5f37788adedd ("arm64: defconfig:
>>> Enable SCSI UFS Crypto and Block Inline encryption drivers").
>>>
>>> Fixes: 2afbf43a4aec ("soc: qcom: Make the Qualcomm UFS/SDCC ICE a dedicated driver")
>>> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
>>> ---
>>>
>>> Changes in v2:
>>> - Keep the probe deferring intact but stop it once it's know ICE SCM
>>>   calls aren't supported by the TZ firmware.
>>>
>>>  drivers/soc/qcom/ice.c | 11 +++++++----
>>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
>>> index b203bc685cad..5a630c9010ee 100644
>>> --- a/drivers/soc/qcom/ice.c
>>> +++ b/drivers/soc/qcom/ice.c
>>> @@ -559,7 +559,7 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
>>>  
>>>  	if (!qcom_scm_ice_available()) {
>>>  		dev_warn(dev, "ICE SCM interface not found\n");
>>> -		return NULL;
>>> +		return ERR_PTR(-EOPNOTSUPP);
>>>  	}
>>>  
>>>  	engine = devm_kzalloc(dev, sizeof(*engine), GFP_KERNEL);
>>> @@ -648,11 +648,14 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
>>>  	}
>>>  
>>>  	ice = platform_get_drvdata(pdev);
>>> -	if (!ice) {
>>> +	if (IS_ERR_OR_NULL(ice)) {
>>>  		dev_err(dev, "Cannot get ice instance from %s\n",
>>>  			dev_name(&pdev->dev));
>>>  		platform_device_put(pdev);
>>> -		return ERR_PTR(-EPROBE_DEFER);
>>> +		if (PTR_ERR(ice) == -EOPNOTSUPP)
>>> +			return NULL;
>>
>> The consumer drivers check specifically for -EOPNOTSUPP, let's
>> just return that
>>
>>> +		else
>>> +			return ERR_PTR(-EPROBE_DEFER);
>>>  	}
>>>  
>>>  	link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_SUPPLIER);
>>> @@ -726,7 +729,7 @@ static int qcom_ice_probe(struct platform_device *pdev)
>>>  	}
>>>  
>>>  	engine = qcom_ice_create(&pdev->dev, base);
>>> -	if (IS_ERR(engine))
>>> +	if (IS_ERR(engine) && PTR_ERR(engine) != -EOPNOTSUPP)
>>>  		return PTR_ERR(engine);
>>
>> This essentially says "probe succeeded, device not operational",
>> I have mixed feelings.. That said I'm not sure about the lifecycle
>> of a platform_device, i.e. can we set the drvdata and return an error
>> in .probe anyway?
>>
> 
> No. Let's remove the probe() altogether and expose this driver as a pure
> library.
> 
The ICE driver already acts as a library for legacy DT case where consumer device provides 'ice'
reg range. It was made dedicated platform driver as ICE is a common IP for UFS and SDCC. See here:
https://lore.kernel.org/all/20230407105029.2274111-4-abel.vesa@linaro.org/
I think it will be better if we resolve the race between probe() and *of_qcom_ice_get().

> - Mani
> Regards
Neeraj

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] soc: qcom: ice: Stop probe deferring once ICE isn't detected
  2026-02-03  6:50     ` Neeraj Soni
@ 2026-02-03 12:22       ` Manivannan Sadhasivam
  0 siblings, 0 replies; 5+ messages in thread
From: Manivannan Sadhasivam @ 2026-02-03 12:22 UTC (permalink / raw)
  To: Neeraj Soni
  Cc: Konrad Dybcio, Sumit Garg, linux-arm-msm, Dmitry Baryshkov,
	andersson, konradybcio, robh+dt, abelvesa, linux-kernel,
	Sumit Garg

On Tue, Feb 03, 2026 at 12:20:56PM +0530, Neeraj Soni wrote:
> 
> 
> On 2/2/2026 4:25 PM, Manivannan Sadhasivam wrote:
> > On Mon, Feb 02, 2026 at 11:51:51AM +0100, Konrad Dybcio wrote:
> >> On 2/2/26 9:25 AM, Sumit Garg wrote:
> >>> From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> >>>
> >>> ICE related SCM calls may not be supported in every TZ environment like
> >>> OP-TEE or a no-TZ environment too. So let's try to stop probe deferring
> >>> when it's known that ICE feature isn't supported.
> >>>
> >>> This problem only came to notice after the inline encryption drivers were
> >>> enabled in the arm64 defconfig by: commit 5f37788adedd ("arm64: defconfig:
> >>> Enable SCSI UFS Crypto and Block Inline encryption drivers").
> >>>
> >>> Fixes: 2afbf43a4aec ("soc: qcom: Make the Qualcomm UFS/SDCC ICE a dedicated driver")
> >>> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> >>> ---
> >>>
> >>> Changes in v2:
> >>> - Keep the probe deferring intact but stop it once it's know ICE SCM
> >>>   calls aren't supported by the TZ firmware.
> >>>
> >>>  drivers/soc/qcom/ice.c | 11 +++++++----
> >>>  1 file changed, 7 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> >>> index b203bc685cad..5a630c9010ee 100644
> >>> --- a/drivers/soc/qcom/ice.c
> >>> +++ b/drivers/soc/qcom/ice.c
> >>> @@ -559,7 +559,7 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
> >>>  
> >>>  	if (!qcom_scm_ice_available()) {
> >>>  		dev_warn(dev, "ICE SCM interface not found\n");
> >>> -		return NULL;
> >>> +		return ERR_PTR(-EOPNOTSUPP);
> >>>  	}
> >>>  
> >>>  	engine = devm_kzalloc(dev, sizeof(*engine), GFP_KERNEL);
> >>> @@ -648,11 +648,14 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
> >>>  	}
> >>>  
> >>>  	ice = platform_get_drvdata(pdev);
> >>> -	if (!ice) {
> >>> +	if (IS_ERR_OR_NULL(ice)) {
> >>>  		dev_err(dev, "Cannot get ice instance from %s\n",
> >>>  			dev_name(&pdev->dev));
> >>>  		platform_device_put(pdev);
> >>> -		return ERR_PTR(-EPROBE_DEFER);
> >>> +		if (PTR_ERR(ice) == -EOPNOTSUPP)
> >>> +			return NULL;
> >>
> >> The consumer drivers check specifically for -EOPNOTSUPP, let's
> >> just return that
> >>
> >>> +		else
> >>> +			return ERR_PTR(-EPROBE_DEFER);
> >>>  	}
> >>>  
> >>>  	link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_SUPPLIER);
> >>> @@ -726,7 +729,7 @@ static int qcom_ice_probe(struct platform_device *pdev)
> >>>  	}
> >>>  
> >>>  	engine = qcom_ice_create(&pdev->dev, base);
> >>> -	if (IS_ERR(engine))
> >>> +	if (IS_ERR(engine) && PTR_ERR(engine) != -EOPNOTSUPP)
> >>>  		return PTR_ERR(engine);
> >>
> >> This essentially says "probe succeeded, device not operational",
> >> I have mixed feelings.. That said I'm not sure about the lifecycle
> >> of a platform_device, i.e. can we set the drvdata and return an error
> >> in .probe anyway?
> >>
> > 
> > No. Let's remove the probe() altogether and expose this driver as a pure
> > library.
> > 
> The ICE driver already acts as a library for legacy DT case where consumer device provides 'ice'
> reg range. It was made dedicated platform driver as ICE is a common IP for UFS and SDCC. See here:
> https://lore.kernel.org/all/20230407105029.2274111-4-abel.vesa@linaro.org/

I know. That's why I said 'pure' library.

> I think it will be better if we resolve the race between probe() and *of_qcom_ice_get().
> 

I don't yet see a need to make it as a platform driver. So I removed it and made
the driver as a pure library:
https://lore.kernel.org/linux-arm-msm/20260203080712.15480-1-manivannan.sadhasivam@oss.qualcomm.com/

- Mani

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-02-03 12:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-02  8:25 [PATCH v2] soc: qcom: ice: Stop probe deferring once ICE isn't detected Sumit Garg
2026-02-02 10:51 ` Konrad Dybcio
2026-02-02 10:55   ` Manivannan Sadhasivam
2026-02-03  6:50     ` Neeraj Soni
2026-02-03 12:22       ` Manivannan Sadhasivam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®