soc: qcom: ice: Stop probe deferring once ICE isn't detected

[PATCH v2] soc: qcom: ice: Stop probe deferring once ICE isn't detected

Posted by Sumit Garg 5 days, 3 hours ago

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

Re: [PATCH v2] soc: qcom: ice: Stop probe deferring once ICE isn't detected

Posted by Konrad Dybcio 5 days, 1 hour ago

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);

Re: [PATCH v2] soc: qcom: ice: Stop probe deferring once ICE isn't detected

Posted by Manivannan Sadhasivam 5 days, 1 hour ago

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

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

Re: [PATCH v2] soc: qcom: ice: Stop probe deferring once ICE isn't detected

Posted by Neeraj Soni 4 days, 5 hours ago


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

Re: [PATCH v2] soc: qcom: ice: Stop probe deferring once ICE isn't detected

Posted by Manivannan Sadhasivam 3 days, 23 hours ago

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

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