Re: [PATCH v4 3/5] soc: qcom: ice: Return proper error codes from devm_of_qcom_ice_get() instead of NULL

[Manivannan Sadhasivam]

On Fri, Mar 06, 2026 at 02:30:02PM +0530, Sumit Garg wrote:
> On Fri, Mar 6, 2026 at 2:17 PM Sumit Garg <sumit.garg@xxxxxxxxxxxxxxxx> wrote:
> >
> > Hey Mani,
> >
> > On Mon, Mar 2, 2026 at 6:30 PM Manivannan Sadhasivam via B4 Relay
> > <devnull+manivannan.sadhasivam.oss.qualcomm.com@xxxxxxxxxx> wrote:
> > >
> > > From: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> > >
> > > devm_of_qcom_ice_get() currently returns NULL if ICE SCM is not available
> > > or "qcom,ice" property is not found in DT. But this confuses the clients
> > > since NULL doesn't convey the reason for failure. So return proper error
> > > codes instead of NULL.
> > >
> > > Reported-by: Sumit Garg <sumit.garg@xxxxxxxxxxxxxxxx>
> > > Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> > > ---
> > >  drivers/soc/qcom/ice.c | 9 ++++-----
> > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> > > index 833d23dc7b06..d1efc676b63c 100644
> > > --- a/drivers/soc/qcom/ice.c
> > > +++ b/drivers/soc/qcom/ice.c
> > > @@ -561,7 +561,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);
> > >         }
> >
> > With this patch-set on top of v7.0-rc2, I still see UFS probe failing
> > when ICE isn't supported with OP-TEE as follows:
> >
> > [    5.401558] qcom-ice 1d88000.crypto: ICE SCM interface not found
> > [    5.419482] qcom-ice 1d88000.crypto: probe with driver qcom-ice
> > failed with error -95
> > <snip>
> > [   18.662977] ufshcd-qcom 1d84000.ufshc: freq-table-hz property not specified
> > [   18.670193] ufshcd-qcom 1d84000.ufshc: ufshcd_populate_vreg: Unable
> > to find vdd-hba-supply regulator, assuming enabled
> > [   18.737665] platform 1d84000.ufshc: deferred probe pending:
> > ufshcd-qcom: ufshcd_pltfrm_init() failed
> > [   18.747141] platform 3370000.codec: deferred probe pending:
> > platform: wait for supplier /soc@0/pinctrl@33c0000/dmic23-data-state
> >
> > Maybe it's the "qcom-ice" driver failure leading to this deferred
> > probe problem again.
> >

Urgh... I completely forgot that the driver core removes the drvdata during
probe error >.<

> 
> Following diff on top of your patchset allows the UFS driver to probe
> successfully without ICE support. I suppose just setting the drvdata
> should be sufficient.
> 
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index d1efc676b63c..a86980647097 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -734,12 +734,6 @@ static int qcom_ice_probe(struct platform_device *pdev)
>         }
> 
>         engine = qcom_ice_create(&pdev->dev, base);
> -       if (IS_ERR(engine)) {
> -               /* Store the error pointer for devm_of_qcom_ice_get() */
> -               platform_set_drvdata(pdev, engine);
> -               return PTR_ERR(engine);
> -       }
> -

No, this will indicate probe success which we do not want and is not safe all
the time. Like what if qcom_ice_create() returned -EPROBE_DEFER.

Let me just store the ice_handle in a global xarray with key based on node
phandle instead of drvdata. This will ensure that the pointer stays till the
driver is loaded.

- Mani

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