[PATCH v3 0/4] soc: qcom: ice: Fix race between qcom_ice_probe() and of_qcom_ice_get()

[PATCH v3 0/4] soc: qcom: ice: Fix race between qcom_ice_probe() and of_qcom_ice_get()

[Manivannan Sadhasivam via B4 Relay]

Hi,

This series fixes the race betwen qcom_ice_probe() and of_qcom_ice_get()
but synchronizing the two APIs and properly propagating the error codes to
clients.

Merge Strategy
==============

Due to dependency, all patches should go through Qcom SoC tree.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
Changes in v3:
- Dropped the platform driver removal patch and used the ice_handle to pass
  error codes. This was done as I learned that we need to have the platform
  driver design going forward and also removing it introduces other issues.
- Link to v2: https://lore.kernel.org/r/20260210-qcom-ice-fix-v2-0-9c1ab5d6502c@oss.qualcomm.com

Changes in v2:

- Added MODULE_* macros back
- Removed spurious platform_device_put()
- Added patches to remove NULL return

---
Manivannan Sadhasivam (4):
      soc: qcom: ice: Fix race between qcom_ice_probe() and of_qcom_ice_get()
      soc: qcom: ice: Return proper error codes from devm_of_qcom_ice_get() instead of NULL
      mmc: sdhci-msm: Remove NULL check from devm_of_qcom_ice_get()
      scsi: ufs: ufs-qcom: Remove NULL check from devm_of_qcom_ice_get()

 drivers/mmc/host/sdhci-msm.c | 10 ++++-----
 drivers/soc/qcom/ice.c       | 53 ++++++++++++++++++++++++++------------------
 drivers/ufs/host/ufs-qcom.c  | 10 ++++-----
 3 files changed, 41 insertions(+), 32 deletions(-)
---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20260210-qcom-ice-fix-d2a3a045b32d

Best regards,
-- 
Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

[PATCH v3 1/4] soc: qcom: ice: Fix race between qcom_ice_probe() and of_qcom_ice_get()

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

The current platform driver design causes probe ordering races with
consumers (UFS, eMMC) due to ICE's dependency on SCM firmware calls. If ICE
probe fails (missing ICE SCM or DT registers), devm_of_qcom_ice_get() loops
with -EPROBE_DEFER, leaving consumers non-functional even when ICE should
be gracefully disabled. devm_of_qcom_ice_get() doesn't know if the ICE
driver probe has failed due to above reasons or it is waiting for the SCM
driver.

Moreover, there is no devlink dependency between ICE and consumer drivers
as 'qcom,ice' is not considered as a DT 'supplier'. So the consumer drivers
have no idea of when the ICE driver is going to probe.

To address these issues, introduce a global ice_handle to store the valid
ICE handle pointer, and set during successful ICE driver probe. On probe
failure, set it to an error pointer and propagate the error from
of_qcom_ice_get().

Additionally, add a global ice_mutex to synchronize qcom_ice_probe() and
of_qcom_ice_get().

Note that this change only fixes the standalone ICE DT node bindings and
not the ones with 'ice' range embedded in the consumer nodes, where there
is no issue.

Cc: <stable@vger.kernel.org> # 6.4
Fixes: 2afbf43a4aec ("soc: qcom: Make the Qualcomm UFS/SDCC ICE a dedicated driver")
Reported-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/soc/qcom/ice.c | 44 +++++++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index b203bc685cad..3c3c189e24f9 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -113,6 +113,9 @@ struct qcom_ice {
 	u8 hwkm_version;
 };
 
+static DEFINE_MUTEX(ice_mutex);
+static struct qcom_ice *ice_handle;
+
 static bool qcom_ice_check_supported(struct qcom_ice *ice)
 {
 	u32 regval = qcom_ice_readl(ice, QCOM_ICE_REG_VERSION);
@@ -608,7 +611,6 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
 static struct qcom_ice *of_qcom_ice_get(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
-	struct qcom_ice *ice;
 	struct resource *res;
 	void __iomem *base;
 	struct device_link *link;
@@ -631,6 +633,22 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
 		return qcom_ice_create(&pdev->dev, base);
 	}
 
+	guard(mutex)(&ice_mutex);
+
+	/*
+	 * If ice_handle is NULL, then it means the ICE driver is not probed
+	 * yet. So return -EPROBE_DEFER to let the client try later.
+	 */
+	if (!ice_handle)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	/*
+	 * If ice_handle has error code, then it means the ICE driver has probe
+	 * failed. So return the handle for the client to digest it.
+	 */
+	if (IS_ERR(ice_handle))
+		return ice_handle;
+
 	/*
 	 * If the consumer node does not provider an 'ice' reg range
 	 * (legacy DT binding), then it must at least provide a phandle
@@ -647,24 +665,16 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
 		return ERR_PTR(-EPROBE_DEFER);
 	}
 
-	ice = platform_get_drvdata(pdev);
-	if (!ice) {
-		dev_err(dev, "Cannot get ice instance from %s\n",
-			dev_name(&pdev->dev));
-		platform_device_put(pdev);
-		return ERR_PTR(-EPROBE_DEFER);
-	}
-
 	link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_SUPPLIER);
 	if (!link) {
 		dev_err(&pdev->dev,
 			"Failed to create device link to consumer %s\n",
 			dev_name(dev));
 		platform_device_put(pdev);
-		ice = ERR_PTR(-EINVAL);
+		return ERR_PTR(-EINVAL);
 	}
 
-	return ice;
+	return ice_handle;
 }
 
 static void qcom_ice_put(const struct qcom_ice *ice)
@@ -716,20 +726,20 @@ EXPORT_SYMBOL_GPL(devm_of_qcom_ice_get);
 
 static int qcom_ice_probe(struct platform_device *pdev)
 {
-	struct qcom_ice *engine;
 	void __iomem *base;
 
+	guard(mutex)(&ice_mutex);
+
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base)) {
 		dev_warn(&pdev->dev, "ICE registers not found\n");
+		ice_handle = base;
 		return PTR_ERR(base);
 	}
 
-	engine = qcom_ice_create(&pdev->dev, base);
-	if (IS_ERR(engine))
-		return PTR_ERR(engine);
-
-	platform_set_drvdata(pdev, engine);
+	ice_handle = qcom_ice_create(&pdev->dev, base);
+	if (IS_ERR(ice_handle))
+		return PTR_ERR(ice_handle);
 
 	return 0;
 }

-- 
2.51.0

Re: [PATCH v3 1/4] soc: qcom: ice: Fix race between qcom_ice_probe() and of_qcom_ice_get()

[Bjorn Andersson]

On Mon, Feb 23, 2026 at 01:32:52PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> The current platform driver design causes probe ordering races with
> consumers (UFS, eMMC) due to ICE's dependency on SCM firmware calls. If ICE
> probe fails (missing ICE SCM or DT registers), devm_of_qcom_ice_get() loops
> with -EPROBE_DEFER, leaving consumers non-functional even when ICE should
> be gracefully disabled. devm_of_qcom_ice_get() doesn't know if the ICE
> driver probe has failed due to above reasons or it is waiting for the SCM
> driver.
> 
> Moreover, there is no devlink dependency between ICE and consumer drivers
> as 'qcom,ice' is not considered as a DT 'supplier'. So the consumer drivers
> have no idea of when the ICE driver is going to probe.
> 
> To address these issues, introduce a global ice_handle to store the valid
> ICE handle pointer, and set during successful ICE driver probe. On probe
> failure, set it to an error pointer and propagate the error from
> of_qcom_ice_get().
> 
> Additionally, add a global ice_mutex to synchronize qcom_ice_probe() and
> of_qcom_ice_get().
> 
> Note that this change only fixes the standalone ICE DT node bindings and
> not the ones with 'ice' range embedded in the consumer nodes, where there
> is no issue.
> 
> Cc: <stable@vger.kernel.org> # 6.4
> Fixes: 2afbf43a4aec ("soc: qcom: Make the Qualcomm UFS/SDCC ICE a dedicated driver")
> Reported-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  drivers/soc/qcom/ice.c | 44 +++++++++++++++++++++++++++-----------------
>  1 file changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index b203bc685cad..3c3c189e24f9 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -113,6 +113,9 @@ struct qcom_ice {
>  	u8 hwkm_version;
>  };
>  
> +static DEFINE_MUTEX(ice_mutex);
> +static struct qcom_ice *ice_handle;

Did we get confirmation that in the UFS + SDCC case, there's only a
single ICE instance per SoC?

Regards,
Bjorn

> +
>  static bool qcom_ice_check_supported(struct qcom_ice *ice)
>  {
>  	u32 regval = qcom_ice_readl(ice, QCOM_ICE_REG_VERSION);
> @@ -608,7 +611,6 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
>  static struct qcom_ice *of_qcom_ice_get(struct device *dev)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
> -	struct qcom_ice *ice;
>  	struct resource *res;
>  	void __iomem *base;
>  	struct device_link *link;
> @@ -631,6 +633,22 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
>  		return qcom_ice_create(&pdev->dev, base);
>  	}
>  
> +	guard(mutex)(&ice_mutex);
> +
> +	/*
> +	 * If ice_handle is NULL, then it means the ICE driver is not probed
> +	 * yet. So return -EPROBE_DEFER to let the client try later.
> +	 */
> +	if (!ice_handle)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	/*
> +	 * If ice_handle has error code, then it means the ICE driver has probe
> +	 * failed. So return the handle for the client to digest it.
> +	 */
> +	if (IS_ERR(ice_handle))
> +		return ice_handle;
> +
>  	/*
>  	 * If the consumer node does not provider an 'ice' reg range
>  	 * (legacy DT binding), then it must at least provide a phandle
> @@ -647,24 +665,16 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
>  		return ERR_PTR(-EPROBE_DEFER);
>  	}
>  
> -	ice = platform_get_drvdata(pdev);
> -	if (!ice) {
> -		dev_err(dev, "Cannot get ice instance from %s\n",
> -			dev_name(&pdev->dev));
> -		platform_device_put(pdev);
> -		return ERR_PTR(-EPROBE_DEFER);
> -	}
> -
>  	link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_SUPPLIER);
>  	if (!link) {
>  		dev_err(&pdev->dev,
>  			"Failed to create device link to consumer %s\n",
>  			dev_name(dev));
>  		platform_device_put(pdev);
> -		ice = ERR_PTR(-EINVAL);
> +		return ERR_PTR(-EINVAL);
>  	}
>  
> -	return ice;
> +	return ice_handle;
>  }
>  
>  static void qcom_ice_put(const struct qcom_ice *ice)
> @@ -716,20 +726,20 @@ EXPORT_SYMBOL_GPL(devm_of_qcom_ice_get);
>  
>  static int qcom_ice_probe(struct platform_device *pdev)
>  {
> -	struct qcom_ice *engine;
>  	void __iomem *base;
>  
> +	guard(mutex)(&ice_mutex);
> +
>  	base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(base)) {
>  		dev_warn(&pdev->dev, "ICE registers not found\n");
> +		ice_handle = base;
>  		return PTR_ERR(base);
>  	}
>  
> -	engine = qcom_ice_create(&pdev->dev, base);
> -	if (IS_ERR(engine))
> -		return PTR_ERR(engine);
> -
> -	platform_set_drvdata(pdev, engine);
> +	ice_handle = qcom_ice_create(&pdev->dev, base);
> +	if (IS_ERR(ice_handle))
> +		return PTR_ERR(ice_handle);
>  
>  	return 0;
>  }
> 
> -- 
> 2.51.0
> 
> 

Re: [PATCH v3 1/4] soc: qcom: ice: Fix race between qcom_ice_probe() and of_qcom_ice_get()

[Manivannan Sadhasivam]

+ Neeraj

On Mon, Feb 23, 2026 at 02:35:04PM -0600, Bjorn Andersson wrote:
> On Mon, Feb 23, 2026 at 01:32:52PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > 
> > The current platform driver design causes probe ordering races with
> > consumers (UFS, eMMC) due to ICE's dependency on SCM firmware calls. If ICE
> > probe fails (missing ICE SCM or DT registers), devm_of_qcom_ice_get() loops
> > with -EPROBE_DEFER, leaving consumers non-functional even when ICE should
> > be gracefully disabled. devm_of_qcom_ice_get() doesn't know if the ICE
> > driver probe has failed due to above reasons or it is waiting for the SCM
> > driver.
> > 
> > Moreover, there is no devlink dependency between ICE and consumer drivers
> > as 'qcom,ice' is not considered as a DT 'supplier'. So the consumer drivers
> > have no idea of when the ICE driver is going to probe.
> > 
> > To address these issues, introduce a global ice_handle to store the valid
> > ICE handle pointer, and set during successful ICE driver probe. On probe
> > failure, set it to an error pointer and propagate the error from
> > of_qcom_ice_get().
> > 
> > Additionally, add a global ice_mutex to synchronize qcom_ice_probe() and
> > of_qcom_ice_get().
> > 
> > Note that this change only fixes the standalone ICE DT node bindings and
> > not the ones with 'ice' range embedded in the consumer nodes, where there
> > is no issue.
> > 
> > Cc: <stable@vger.kernel.org> # 6.4
> > Fixes: 2afbf43a4aec ("soc: qcom: Make the Qualcomm UFS/SDCC ICE a dedicated driver")
> > Reported-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >  drivers/soc/qcom/ice.c | 44 +++++++++++++++++++++++++++-----------------
> >  1 file changed, 27 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> > index b203bc685cad..3c3c189e24f9 100644
> > --- a/drivers/soc/qcom/ice.c
> > +++ b/drivers/soc/qcom/ice.c
> > @@ -113,6 +113,9 @@ struct qcom_ice {
> >  	u8 hwkm_version;
> >  };
> >  
> > +static DEFINE_MUTEX(ice_mutex);
> > +static struct qcom_ice *ice_handle;
> 
> Did we get confirmation that in the UFS + SDCC case, there's only a
> single ICE instance per SoC?
> 

Right now there is only a single instance per SoC. But Neeraj told me that
upcoming SoCs are going to have multiple instances. But I don't want to spend
too much time on *upcoming* support, but rather fix the current
implementations.

Extending this to multiple instances would just require storing the ice_handle
with node name/address pair in xarray or in some other data structures.

- Mani

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

Re: [PATCH v3 1/4] soc: qcom: ice: Fix race between qcom_ice_probe() and of_qcom_ice_get()

[Neeraj Soni]

On 2/24/2026 10:16 AM, Manivannan Sadhasivam wrote:
> + Neeraj
> 
> On Mon, Feb 23, 2026 at 02:35:04PM -0600, Bjorn Andersson wrote:
>> On Mon, Feb 23, 2026 at 01:32:52PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
>>> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>>>
>>> The current platform driver design causes probe ordering races with
>>> consumers (UFS, eMMC) due to ICE's dependency on SCM firmware calls. If ICE
>>> probe fails (missing ICE SCM or DT registers), devm_of_qcom_ice_get() loops
>>> with -EPROBE_DEFER, leaving consumers non-functional even when ICE should
>>> be gracefully disabled. devm_of_qcom_ice_get() doesn't know if the ICE
>>> driver probe has failed due to above reasons or it is waiting for the SCM
>>> driver.
>>>
>>> Moreover, there is no devlink dependency between ICE and consumer drivers
>>> as 'qcom,ice' is not considered as a DT 'supplier'. So the consumer drivers
>>> have no idea of when the ICE driver is going to probe.
>>>
>>> To address these issues, introduce a global ice_handle to store the valid
>>> ICE handle pointer, and set during successful ICE driver probe. On probe
>>> failure, set it to an error pointer and propagate the error from
>>> of_qcom_ice_get().
>>>
>>> Additionally, add a global ice_mutex to synchronize qcom_ice_probe() and
>>> of_qcom_ice_get().
>>>
>>> Note that this change only fixes the standalone ICE DT node bindings and
>>> not the ones with 'ice' range embedded in the consumer nodes, where there
>>> is no issue.
>>>
>>> Cc: <stable@vger.kernel.org> # 6.4
>>> Fixes: 2afbf43a4aec ("soc: qcom: Make the Qualcomm UFS/SDCC ICE a dedicated driver")
>>> Reported-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>>> ---
>>>  drivers/soc/qcom/ice.c | 44 +++++++++++++++++++++++++++-----------------
>>>  1 file changed, 27 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
>>> index b203bc685cad..3c3c189e24f9 100644
>>> --- a/drivers/soc/qcom/ice.c
>>> +++ b/drivers/soc/qcom/ice.c
>>> @@ -113,6 +113,9 @@ struct qcom_ice {
>>>  	u8 hwkm_version;
>>>  };
>>>  
>>> +static DEFINE_MUTEX(ice_mutex);
>>> +static struct qcom_ice *ice_handle;
>>
>> Did we get confirmation that in the UFS + SDCC case, there's only a
>> single ICE instance per SoC?
>>
> 
> Right now there is only a single instance per SoC. But Neeraj told me that
> upcoming SoCs are going to have multiple instances. But I don't want to spend

Yes and patches for same are under review here:
https://lore.kernel.org/all/20260217052526.2335759-1-neeraj.soni@oss.qualcomm.com/

> too much time on *upcoming* support, but rather fix the current
> implementations.
> 
> Extending this to multiple instances would just require storing the ice_handle
> with node name/address pair in xarray or in some other data structures.
> 
> - Mani
> 

Re: [PATCH v3 1/4] soc: qcom: ice: Fix race between qcom_ice_probe() and of_qcom_ice_get()

[Neeraj Soni]

On 2/23/2026 1:32 PM, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> The current platform driver design causes probe ordering races with
> consumers (UFS, eMMC) due to ICE's dependency on SCM firmware calls. If ICE
> probe fails (missing ICE SCM or DT registers), devm_of_qcom_ice_get() loops
> with -EPROBE_DEFER, leaving consumers non-functional even when ICE should
> be gracefully disabled. devm_of_qcom_ice_get() doesn't know if the ICE
> driver probe has failed due to above reasons or it is waiting for the SCM
> driver.
> 
> Moreover, there is no devlink dependency between ICE and consumer drivers
> as 'qcom,ice' is not considered as a DT 'supplier'. So the consumer drivers
> have no idea of when the ICE driver is going to probe.
> 
> To address these issues, introduce a global ice_handle to store the valid
> ICE handle pointer, and set during successful ICE driver probe. On probe
> failure, set it to an error pointer and propagate the error from
> of_qcom_ice_get().
> 
> Additionally, add a global ice_mutex to synchronize qcom_ice_probe() and
> of_qcom_ice_get().
> 
> Note that this change only fixes the standalone ICE DT node bindings and
> not the ones with 'ice' range embedded in the consumer nodes, where there
> is no issue.
> 
> Cc: <stable@vger.kernel.org> # 6.4
> Fixes: 2afbf43a4aec ("soc: qcom: Make the Qualcomm UFS/SDCC ICE a dedicated driver")
> Reported-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  drivers/soc/qcom/ice.c | 44 +++++++++++++++++++++++++++-----------------
>  1 file changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index b203bc685cad..3c3c189e24f9 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -113,6 +113,9 @@ struct qcom_ice {
>  	u8 hwkm_version;
>  };
>  
> +static DEFINE_MUTEX(ice_mutex);
> +static struct qcom_ice *ice_handle;
> +
>  static bool qcom_ice_check_supported(struct qcom_ice *ice)
>  {
>  	u32 regval = qcom_ice_readl(ice, QCOM_ICE_REG_VERSION);
> @@ -608,7 +611,6 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
>  static struct qcom_ice *of_qcom_ice_get(struct device *dev)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
> -	struct qcom_ice *ice;
>  	struct resource *res;
>  	void __iomem *base;
>  	struct device_link *link;
> @@ -631,6 +633,22 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
>  		return qcom_ice_create(&pdev->dev, base);
>  	}
>  
> +	guard(mutex)(&ice_mutex);
> +
> +	/*
> +	 * If ice_handle is NULL, then it means the ICE driver is not probed
> +	 * yet. So return -EPROBE_DEFER to let the client try later.
> +	 */
> +	if (!ice_handle)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	/*
> +	 * If ice_handle has error code, then it means the ICE driver has probe
> +	 * failed. So return the handle for the client to digest it.
> +	 */
> +	if (IS_ERR(ice_handle))
> +		return ice_handle;
> +
>  	/*
>  	 * If the consumer node does not provider an 'ice' reg range
>  	 * (legacy DT binding), then it must at least provide a phandle
> @@ -647,24 +665,16 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
>  		return ERR_PTR(-EPROBE_DEFER);
>  	}
>  
> -	ice = platform_get_drvdata(pdev);
> -	if (!ice) {
> -		dev_err(dev, "Cannot get ice instance from %s\n",
> -			dev_name(&pdev->dev));
> -		platform_device_put(pdev);
> -		return ERR_PTR(-EPROBE_DEFER);
> -	}
> -
>  	link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_SUPPLIER);
>  	if (!link) {
>  		dev_err(&pdev->dev,
>  			"Failed to create device link to consumer %s\n",
>  			dev_name(dev));
>  		platform_device_put(pdev);
> -		ice = ERR_PTR(-EINVAL);
> +		return ERR_PTR(-EINVAL);
>  	}
>  
> -	return ice;
> +	return ice_handle;
>  }
>  
>  static void qcom_ice_put(const struct qcom_ice *ice)
> @@ -716,20 +726,20 @@ EXPORT_SYMBOL_GPL(devm_of_qcom_ice_get);
>  
>  static int qcom_ice_probe(struct platform_device *pdev)
>  {
> -	struct qcom_ice *engine;
>  	void __iomem *base;
>  
> +	guard(mutex)(&ice_mutex);
> +
>  	base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(base)) {
>  		dev_warn(&pdev->dev, "ICE registers not found\n");
> +		ice_handle = base;
>  		return PTR_ERR(base);
>  	}
>  
> -	engine = qcom_ice_create(&pdev->dev, base);
> -	if (IS_ERR(engine))
> -		return PTR_ERR(engine);
> -
> -	platform_set_drvdata(pdev, engine);

This allows the driver to set the data per ICE device instance which allows
the addition of multiple ICE platform devices. For example this patch:
https://lore.kernel.org/all/20260217052526.2335759-1-neeraj.soni@oss.qualcomm.com/
utilizes this capability. I think it doesen't harm to keep this support. 
Moreover, the issue which your patch intends to address do not need this to be removed.

> +	ice_handle = qcom_ice_create(&pdev->dev, base);
> +	if (IS_ERR(ice_handle))
> +		return PTR_ERR(ice_handle);
>  
>  	return 0;
>  }
> 
Regards,
Neeraj

Re: [PATCH v3 1/4] soc: qcom: ice: Fix race between qcom_ice_probe() and of_qcom_ice_get()

[Manivannan Sadhasivam]

On Mon, Mar 02, 2026 at 03:41:54PM +0530, Neeraj Soni wrote:
> 
> 
> On 2/23/2026 1:32 PM, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > 
> > The current platform driver design causes probe ordering races with
> > consumers (UFS, eMMC) due to ICE's dependency on SCM firmware calls. If ICE
> > probe fails (missing ICE SCM or DT registers), devm_of_qcom_ice_get() loops
> > with -EPROBE_DEFER, leaving consumers non-functional even when ICE should
> > be gracefully disabled. devm_of_qcom_ice_get() doesn't know if the ICE
> > driver probe has failed due to above reasons or it is waiting for the SCM
> > driver.
> > 
> > Moreover, there is no devlink dependency between ICE and consumer drivers
> > as 'qcom,ice' is not considered as a DT 'supplier'. So the consumer drivers
> > have no idea of when the ICE driver is going to probe.
> > 
> > To address these issues, introduce a global ice_handle to store the valid
> > ICE handle pointer, and set during successful ICE driver probe. On probe
> > failure, set it to an error pointer and propagate the error from
> > of_qcom_ice_get().
> > 
> > Additionally, add a global ice_mutex to synchronize qcom_ice_probe() and
> > of_qcom_ice_get().
> > 
> > Note that this change only fixes the standalone ICE DT node bindings and
> > not the ones with 'ice' range embedded in the consumer nodes, where there
> > is no issue.
> > 
> > Cc: <stable@vger.kernel.org> # 6.4
> > Fixes: 2afbf43a4aec ("soc: qcom: Make the Qualcomm UFS/SDCC ICE a dedicated driver")
> > Reported-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >  drivers/soc/qcom/ice.c | 44 +++++++++++++++++++++++++++-----------------
> >  1 file changed, 27 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> > index b203bc685cad..3c3c189e24f9 100644
> > --- a/drivers/soc/qcom/ice.c
> > +++ b/drivers/soc/qcom/ice.c
> > @@ -113,6 +113,9 @@ struct qcom_ice {
> >  	u8 hwkm_version;
> >  };
> >  
> > +static DEFINE_MUTEX(ice_mutex);
> > +static struct qcom_ice *ice_handle;
> > +
> >  static bool qcom_ice_check_supported(struct qcom_ice *ice)
> >  {
> >  	u32 regval = qcom_ice_readl(ice, QCOM_ICE_REG_VERSION);
> > @@ -608,7 +611,6 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
> >  static struct qcom_ice *of_qcom_ice_get(struct device *dev)
> >  {
> >  	struct platform_device *pdev = to_platform_device(dev);
> > -	struct qcom_ice *ice;
> >  	struct resource *res;
> >  	void __iomem *base;
> >  	struct device_link *link;
> > @@ -631,6 +633,22 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
> >  		return qcom_ice_create(&pdev->dev, base);
> >  	}
> >  
> > +	guard(mutex)(&ice_mutex);
> > +
> > +	/*
> > +	 * If ice_handle is NULL, then it means the ICE driver is not probed
> > +	 * yet. So return -EPROBE_DEFER to let the client try later.
> > +	 */
> > +	if (!ice_handle)
> > +		return ERR_PTR(-EPROBE_DEFER);
> > +
> > +	/*
> > +	 * If ice_handle has error code, then it means the ICE driver has probe
> > +	 * failed. So return the handle for the client to digest it.
> > +	 */
> > +	if (IS_ERR(ice_handle))
> > +		return ice_handle;
> > +
> >  	/*
> >  	 * If the consumer node does not provider an 'ice' reg range
> >  	 * (legacy DT binding), then it must at least provide a phandle
> > @@ -647,24 +665,16 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
> >  		return ERR_PTR(-EPROBE_DEFER);
> >  	}
> >  
> > -	ice = platform_get_drvdata(pdev);
> > -	if (!ice) {
> > -		dev_err(dev, "Cannot get ice instance from %s\n",
> > -			dev_name(&pdev->dev));
> > -		platform_device_put(pdev);
> > -		return ERR_PTR(-EPROBE_DEFER);
> > -	}
> > -
> >  	link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_SUPPLIER);
> >  	if (!link) {
> >  		dev_err(&pdev->dev,
> >  			"Failed to create device link to consumer %s\n",
> >  			dev_name(dev));
> >  		platform_device_put(pdev);
> > -		ice = ERR_PTR(-EINVAL);
> > +		return ERR_PTR(-EINVAL);
> >  	}
> >  
> > -	return ice;
> > +	return ice_handle;
> >  }
> >  
> >  static void qcom_ice_put(const struct qcom_ice *ice)
> > @@ -716,20 +726,20 @@ EXPORT_SYMBOL_GPL(devm_of_qcom_ice_get);
> >  
> >  static int qcom_ice_probe(struct platform_device *pdev)
> >  {
> > -	struct qcom_ice *engine;
> >  	void __iomem *base;
> >  
> > +	guard(mutex)(&ice_mutex);
> > +
> >  	base = devm_platform_ioremap_resource(pdev, 0);
> >  	if (IS_ERR(base)) {
> >  		dev_warn(&pdev->dev, "ICE registers not found\n");
> > +		ice_handle = base;
> >  		return PTR_ERR(base);
> >  	}
> >  
> > -	engine = qcom_ice_create(&pdev->dev, base);
> > -	if (IS_ERR(engine))
> > -		return PTR_ERR(engine);
> > -
> > -	platform_set_drvdata(pdev, engine);
> 
> This allows the driver to set the data per ICE device instance which allows
> the addition of multiple ICE platform devices. For example this patch:
> https://lore.kernel.org/all/20260217052526.2335759-1-neeraj.soni@oss.qualcomm.com/
> utilizes this capability. I think it doesen't harm to keep this support. 
> Moreover, the issue which your patch intends to address do not need this to be removed.
> 

Hmm, ok. I figured out that if we store the err ptr in drvdata, we can fix the
race and also allow multiple ice instances in a SoC. So sent v4 incorporating
this change.

- Mani

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