Re: [PATCH] soc: qcom: ice: Remove platform_driver support and expose as a pure library

[Neeraj Soni]

On 2/19/2026 12:17 PM, Manivannan Sadhasivam wrote:
> On Thu, Feb 19, 2026 at 11:27:07AM +0530, Neeraj Soni wrote:
>>
>>
>> On 2/18/2026 7:11 PM, Manivannan Sadhasivam wrote:
>>> On Tue, Feb 17, 2026 at 10:03:27AM +0530, Neeraj Soni wrote:
>>>>
>>>>
>>>> On 2/3/2026 1:40 PM, Manivannan Sadhasivam wrote:
>>>>> On Tue, Feb 03, 2026 at 01:37:12PM +0530, Manivannan Sadhasivam wrote:
>>>>>> The current platform driver design causes probe ordering races with clients
>>>>>> (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 clients non-functional even when ICE should be
>>>>>> gracefully disabled. devm_of_qcom_ice_get() cannot 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 client drivers
>>>>>> as 'qcom,ice' is not considered as a DT 'supplier'. So the client drivers
>>>>>> have no idea of when the ICE driver is going to probe.
>>>>>>
>>>>>> To avoid all this hassle, remove the platform driver support altogether and
>>>>>> just expose the ICE driver as a pure library to client drivers. With this
>>>>>> design, when devm_of_qcom_ice_get() is called, it will check if the ICE
>>>>>> instance is available or not. If not, it will create one based on the ICE
>>>>>> DT node, increase the refcount and return the handle. When the next client
>>>>>> calls the API again, the ICE instance would be available. So this function
>>>>>> will just increment the refcount and return the instance.
>>>>>>
>>>>>> Finally, when the client devices get destroyed, refcount will be
>>>>>> decremented and finally the cleanup will happen once all clients are
>>>>>> destroyed.
>>>>>>
>>>>>> For the clients using the old DT binding of providing the separate 'ice'
>>>>>> register range in their node, this change has no impact.
>>>>>>
>>>>>> Cc: stable@xxxxxxxxxxxxxxx
>>>>>> Cc: Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx>
>>>>>> Reported-by: Sumit Garg <sumit.garg@xxxxxxxxxxxxxxxx>
>>>>>> Fixes: 2afbf43a4aec ("soc: qcom: Make the Qualcomm UFS/SDCC ICE a dedicated driver")
>>>>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
>>>>>> ---
>>>>>>  drivers/soc/qcom/ice.c | 100 ++++++++++++++++-------------------------
>>>>>>  1 file changed, 39 insertions(+), 61 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
>>>>>> index b203bc685cad..b5a9cf8de6e4 100644
>>>>>> --- a/drivers/soc/qcom/ice.c
>>>>>> +++ b/drivers/soc/qcom/ice.c
>>>>>> @@ -107,12 +107,16 @@ struct qcom_ice {
>>>>>>  	struct device *dev;
>>>>>>  	void __iomem *base;
>>>>>>  
>>>>>> +	struct kref refcount;
>>>>>>  	struct clk *core_clk;
>>>>>>  	bool use_hwkm;
>>>>>>  	bool hwkm_init_complete;
>>>>>>  	u8 hwkm_version;
>>>>>>  };
>>>>>>  
>>>>>> +static DEFINE_MUTEX(ice_mutex);
>>>>>> +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);
>>>>>> @@ -599,8 +603,8 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
>>>>>>   * This function will provide an ICE instance either by creating one for the
>>>>>>   * consumer device if its DT node provides the 'ice' reg range and the 'ice'
>>>>>>   * clock (for legacy DT style). On the other hand, if consumer provides a
>>>>>> - * phandle via 'qcom,ice' property to an ICE DT, the ICE instance will already
>>>>>> - * be created and so this function will return that instead.
>>>>>> + * phandle via 'qcom,ice' property to an ICE DT node, then the ICE instance will
>>>>>> + * be created if not already done and will be returned.
>>>>>>   *
>>>>>>   * Return: ICE pointer on success, NULL if there is no ICE data provided by the
>>>>>>   * consumer or ERR_PTR() on error.
>>>>>> @@ -611,11 +615,12 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
>>>>>>  	struct qcom_ice *ice;
>>>>>>  	struct resource *res;
>>>>>>  	void __iomem *base;
>>>>>> -	struct device_link *link;
>>>>>>  
>>>>>>  	if (!dev || !dev->of_node)
>>>>>>  		return ERR_PTR(-ENODEV);
>>>>>>  
>>>>>> +	guard(mutex)(&ice_mutex);
>>>>>> +
>>>>>>  	/*
>>>>>>  	 * In order to support legacy style devicetree bindings, we need
>>>>>>  	 * to create the ICE instance using the consumer device and the reg
>>>>>> @@ -631,6 +636,16 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
>>>>>>  		return qcom_ice_create(&pdev->dev, base);
>>>>>>  	}
>>>>>>  
>>>>>> +	/*
>>>>>> +	 * If the ICE node has been initialized already, just increase the
>>>>>> +	 * refcount and return the handle.
>>>>>> +	 */
>>>>>> +	if (ice_handle) {
>>>>>> +		kref_get(&ice_handle->refcount);
>>>>>> +
>>>>>> +		return ice_handle;
>>>>
>>>> How will this work for a device using both UFS and eMMC storage (one being primary storage
>>>> and other being secondary)? UFS and eMMC will have seperate ICE DT node so returning same
>>>> handle to both clients will not be correct.
>>>>
>>>
>>> I'm not aware of any platforms using separate ICE nodes. All are using shared
>>> node only. Which platform are you referring to?
>>
>> Example talos uses both UFS and eMMC:
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/qcom/talos.dtsi#n699
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/qcom/talos.dtsi#n1398
>>
> 
> These are not using *separate* ICE DT nodes, but just have their own ICE MMIO
> regions. This is already handled in this patch which parses the ICE region first
> if available.
> 
>> For few more targets where eMMC is used along with UFS, the patches to add ICE handle for eMMC is in flight with this patch:
>> https://lore.kernel.org/all/20260217052526.2335759-1-neeraj.soni@xxxxxxxxxxxxxxxx/
>>
> 
> So this adds a new ICE node, but just for eMMC. Are you saying that there will
> be ufs_crypto node as well?
> 
It is alrady there for these targets:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/qcom/kodiak.dtsi#n2514
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/qcom/monaco.dtsi#n2685

> - Mani
> 
Regards
Neeraj