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

[Eric Biggers]

On Tue, Feb 10, 2026 at 12:26:50PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
>  drivers/soc/qcom/ice.c | 118 ++++++++++++++++++-------------------------------
>  1 file changed, 44 insertions(+), 74 deletions(-)

I don't yet know enough to be confident that this is the correct fix,
but there are a few things I noticed that look like bugs:

> +static DEFINE_MUTEX(ice_mutex);
> +struct qcom_ice *ice_handle;

ice_handle is used only in this file, so it should be static

> @@ -643,41 +645,42 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
[...]
> +	ice = qcom_ice_create(&pdev->dev, base);
> +	if (IS_ERR(ice)) {
>  		platform_device_put(pdev);
> -		ice = ERR_PTR(-EINVAL);
> +		return ice_handle;
>  	}

This error path returns NULL, where this patch seems to have been
intended to remove NULL as a possible return value.

> -static void qcom_ice_put(const struct qcom_ice *ice)
> +static void qcom_ice_put(struct kref *kref)
>  {
> -	struct platform_device *pdev = to_platform_device(ice->dev);
> -
> -	if (!platform_get_resource_byname(pdev, IORESOURCE_MEM, "ice"))
> -		platform_device_put(pdev);
> +	platform_device_put(to_platform_device(ice_handle->dev));
> +	ice_handle = NULL;
>  }

Elsewhere ice_handle is protected by ice_mutex, but this seems to modify
it without holding the mutex.

I'm also wondering what happens if all consumer devices are removed.
platform_device_put() gets executed on the ICE platform_device for each
one, but does that actually drop the last reference and cause the
resources allocated with devm_*() to be freed?  On do they stick around
until/unless the ICE device is actually removed as well?

>  static void devm_of_qcom_ice_put(struct device *dev, void *res)
>  {
> -	qcom_ice_put(*(struct qcom_ice **)res);
> +	const struct qcom_ice *ice = *(struct qcom_ice **)res;
> +	struct platform_device *pdev = to_platform_device(ice->dev);
> +
> +	if (!platform_get_resource_byname(pdev, IORESOURCE_MEM, "ice"))
> +		kref_put(&ice_handle->refcount, qcom_ice_put);
>  }

Above probably should use the ice local variable, not ice_handle.

- Eric