* [PATCH] soc: qcom: ice: Avoid probe deferring for un-supported ICE feature
@ 2026-01-30 9:11 Sumit Garg
2026-01-30 9:34 ` Konrad Dybcio
0 siblings, 1 reply; 11+ messages in thread
From: Sumit Garg @ 2026-01-30 9:11 UTC (permalink / raw)
To: linux-arm-msm
Cc: andersson, konradybcio, abelvesa, mani, linux-kernel, Sumit Garg
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 avoid 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>
---
drivers/soc/qcom/ice.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index b203bc685cad..ab9586b8caf5 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -652,7 +652,7 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
dev_err(dev, "Cannot get ice instance from %s\n",
dev_name(&pdev->dev));
platform_device_put(pdev);
- return ERR_PTR(-EPROBE_DEFER);
+ return NULL;
}
link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_SUPPLIER);
--
2.51.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] soc: qcom: ice: Avoid probe deferring for un-supported ICE feature 2026-01-30 9:11 [PATCH] soc: qcom: ice: Avoid probe deferring for un-supported ICE feature Sumit Garg @ 2026-01-30 9:34 ` Konrad Dybcio 2026-01-30 9:52 ` Sumit Garg 0 siblings, 1 reply; 11+ messages in thread From: Konrad Dybcio @ 2026-01-30 9:34 UTC (permalink / raw) To: Sumit Garg, linux-arm-msm Cc: andersson, konradybcio, abelvesa, mani, linux-kernel, Sumit Garg On 1/30/26 10:11 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 avoid 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> > --- > drivers/soc/qcom/ice.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c > index b203bc685cad..ab9586b8caf5 100644 > --- a/drivers/soc/qcom/ice.c > +++ b/drivers/soc/qcom/ice.c > @@ -652,7 +652,7 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev) > dev_err(dev, "Cannot get ice instance from %s\n", > dev_name(&pdev->dev)); > platform_device_put(pdev); > - return ERR_PTR(-EPROBE_DEFER); > + return NULL; Wouldn't this wreck the "actually need to defer" case, i.e. qcom,ufs probes first calls devm_of_qcom_ice_get() "ice" reg is missing, non-legacy case qcom,ice is absent (not yet probed) return NULL ... qcom,ice probes ? Konrad ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] soc: qcom: ice: Avoid probe deferring for un-supported ICE feature 2026-01-30 9:34 ` Konrad Dybcio @ 2026-01-30 9:52 ` Sumit Garg 2026-01-30 9:59 ` Konrad Dybcio 0 siblings, 1 reply; 11+ messages in thread From: Sumit Garg @ 2026-01-30 9:52 UTC (permalink / raw) To: Konrad Dybcio Cc: linux-arm-msm, andersson, konradybcio, abelvesa, mani, linux-kernel, Sumit Garg On Fri, Jan 30, 2026 at 10:34:26AM +0100, Konrad Dybcio wrote: > On 1/30/26 10:11 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 avoid 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> > > --- > > drivers/soc/qcom/ice.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c > > index b203bc685cad..ab9586b8caf5 100644 > > --- a/drivers/soc/qcom/ice.c > > +++ b/drivers/soc/qcom/ice.c > > @@ -652,7 +652,7 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev) > > dev_err(dev, "Cannot get ice instance from %s\n", > > dev_name(&pdev->dev)); > > platform_device_put(pdev); > > - return ERR_PTR(-EPROBE_DEFER); > > + return NULL; > > Wouldn't this wreck the "actually need to defer" case, i.e. > > > qcom,ufs probes first > calls devm_of_qcom_ice_get() > "ice" reg is missing, non-legacy case > qcom,ice is absent (not yet probed) > return NULL > > ... > > qcom,ice probes > > ? Since qcom,ufs depends on qcom,ice via a phandle, so isn't the probe orderering automatically taken care off? Or that isn't the case here? -Sumit ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] soc: qcom: ice: Avoid probe deferring for un-supported ICE feature 2026-01-30 9:52 ` Sumit Garg @ 2026-01-30 9:59 ` Konrad Dybcio 2026-01-30 11:36 ` Sumit Garg 2026-02-02 10:55 ` Manivannan Sadhasivam 0 siblings, 2 replies; 11+ messages in thread From: Konrad Dybcio @ 2026-01-30 9:59 UTC (permalink / raw) To: Sumit Garg, Rob Herring Cc: linux-arm-msm, andersson, konradybcio, abelvesa, mani, linux-kernel, Sumit Garg On 1/30/26 10:52 AM, Sumit Garg wrote: > On Fri, Jan 30, 2026 at 10:34:26AM +0100, Konrad Dybcio wrote: >> On 1/30/26 10:11 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 avoid 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> >>> --- >>> drivers/soc/qcom/ice.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c >>> index b203bc685cad..ab9586b8caf5 100644 >>> --- a/drivers/soc/qcom/ice.c >>> +++ b/drivers/soc/qcom/ice.c >>> @@ -652,7 +652,7 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev) >>> dev_err(dev, "Cannot get ice instance from %s\n", >>> dev_name(&pdev->dev)); >>> platform_device_put(pdev); >>> - return ERR_PTR(-EPROBE_DEFER); >>> + return NULL; >> >> Wouldn't this wreck the "actually need to defer" case, i.e. >> >> >> qcom,ufs probes first >> calls devm_of_qcom_ice_get() >> "ice" reg is missing, non-legacy case >> qcom,ice is absent (not yet probed) >> return NULL >> >> ... >> >> qcom,ice probes >> >> ? > > Since qcom,ufs depends on qcom,ice via a phandle, so isn't the probe > orderering automatically taken care off? Or that isn't the case here? No, that's guaranteed by devlink only with certain properties. In this case though, I think it could make sense to add it to the "suppliers" list in drivers/of/property.c. I don't know if vendors adding their custom properties there is a pattern that +Rob will be happy about though.. Konrad ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] soc: qcom: ice: Avoid probe deferring for un-supported ICE feature 2026-01-30 9:59 ` Konrad Dybcio @ 2026-01-30 11:36 ` Sumit Garg 2026-01-30 11:55 ` Konrad Dybcio 2026-02-02 10:55 ` Manivannan Sadhasivam 1 sibling, 1 reply; 11+ messages in thread From: Sumit Garg @ 2026-01-30 11:36 UTC (permalink / raw) To: Konrad Dybcio Cc: Rob Herring, linux-arm-msm, andersson, konradybcio, abelvesa, mani, linux-kernel, Sumit Garg On Fri, Jan 30, 2026 at 10:59:18AM +0100, Konrad Dybcio wrote: > On 1/30/26 10:52 AM, Sumit Garg wrote: > > On Fri, Jan 30, 2026 at 10:34:26AM +0100, Konrad Dybcio wrote: > >> On 1/30/26 10:11 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 avoid 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> > >>> --- > >>> drivers/soc/qcom/ice.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c > >>> index b203bc685cad..ab9586b8caf5 100644 > >>> --- a/drivers/soc/qcom/ice.c > >>> +++ b/drivers/soc/qcom/ice.c > >>> @@ -652,7 +652,7 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev) > >>> dev_err(dev, "Cannot get ice instance from %s\n", > >>> dev_name(&pdev->dev)); > >>> platform_device_put(pdev); > >>> - return ERR_PTR(-EPROBE_DEFER); > >>> + return NULL; > >> > >> Wouldn't this wreck the "actually need to defer" case, i.e. > >> > >> > >> qcom,ufs probes first > >> calls devm_of_qcom_ice_get() > >> "ice" reg is missing, non-legacy case > >> qcom,ice is absent (not yet probed) > >> return NULL > >> > >> ... > >> > >> qcom,ice probes > >> > >> ? > > > > Since qcom,ufs depends on qcom,ice via a phandle, so isn't the probe > > orderering automatically taken care off? Or that isn't the case here? > > No, that's guaranteed by devlink only with certain properties. Okay I see. The other alternate solution I can come up is following to keep the deferred probing intact. Let me know your thoughts on this: diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c index ab9586b8caf5..76bf9f94fbaf 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(-ENODEV); } 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 NULL; + if (PTR_ERR(ice) == -ENODEV) + 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) != -ENODEV) return PTR_ERR(engine); platform_set_drvdata(pdev, engine); > In this case though, I think it could make sense to add it to the > "suppliers" list in drivers/of/property.c. > > I don't know if vendors adding their custom properties there is a > pattern that +Rob will be happy about though.. Not sure if that's a shorter path as I would like to see fix for this issue backported as well. Aside, not sure how much stable ICE feature itself is as I got following crash with QLI boot firmware on Kodiak: [ 5.172970] SError Interrupt on CPU6, code 0x00000000be000000 -- SError [ 5.172986] CPU: 6 UID: 0 PID: 241 Comm: (udev-worker) Tainted: G M 6.19.0-rc5-next-20260115-gc1a0fee87a05 #9 PREEMPT [ 5.172996] Tainted: [M]=MACHINE_CHECK [ 5.172999] Hardware name: Qualcomm Technologies, Inc. Robotics RB3gen2 (DT) [ 5.173003] pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 5.173010] pc : qcom_ice_create.part.0+0x6c/0x24c [qcom_ice] [ 5.173024] lr : qcom_ice_create.part.0+0xe4/0x24c [qcom_ice] [ 5.173035] sp : ffff8000810fb860 [ 5.173037] x29: ffff8000810fb860 x28: ffff8000810fbcc0 x27: ffffb38718304084 [ 5.173046] x26: 0000000000000000 x25: ffff8000810fbcc0 x24: ffff0000832ff068 [ 5.173052] x23: 000000000000003d x22: 0000000000000000 x21: ffff00008144fc10 [ 5.173058] x20: ffff0000832ff380 x19: ffff8000815e8000 x18: 000000000000000a [ 5.173063] x17: 000000000404006f x16: ffffb38728601480 x15: ffffb38729fbefc8 [ 5.173068] x14: ffff8000815f1000 x13: ffffb3872a0ab208 x12: ffffb3872a0ab000 [ 5.173073] x11: 0000000000008000 x10: 0000000000159ff8 x9 : 0000000000000003 [ 5.173079] x8 : ffff00008083ccc0 x7 : 0000000000000000 x6 : 0000000000077064 [ 5.173084] x5 : ffff8000810fb744 x4 : 0000000000000000 x3 : ffff0000832ff480 [ 5.173089] x2 : ffff0000832ff540 x1 : ffff00008e0c8000 x0 : ffff00008083cc60 [ 5.173096] Kernel panic - not syncing: Asynchronous SError Interrupt [ 5.173100] CPU: 6 UID: 0 PID: 241 Comm: (udev-worker) Tainted: G M 6.19.0-rc5-next-20260115-gc1a0fee87a05 #9 PREEMPT [ 5.173107] Tainted: [M]=MACHINE_CHECK [ 5.173109] Hardware name: Qualcomm Technologies, Inc. Robotics RB3gen2 (DT) [ 5.173111] Call trace: [ 5.173113] show_stack+0x18/0x30 (C) [ 5.173125] dump_stack_lvl+0x60/0x80 [ 5.173132] dump_stack+0x18/0x24 [ 5.173137] vpanic+0xf8/0x2dc [ 5.173143] abort+0x0/0x4 [ 5.173149] add_taint+0x0/0xbc [ 5.173154] arm64_serror_panic+0x70/0x80 [ 5.173161] arm64_is_fatal_ras_serror+0x34/0xac [ 5.173167] do_serror+0x58/0x74 [ 5.173173] el1h_64_error_handler+0x38/0x60 [ 5.173184] el1h_64_error+0x6c/0x70 [ 5.173189] qcom_ice_create.part.0+0x6c/0x24c [qcom_ice] (P) [ 5.173196] qcom_ice_probe+0x58/0xa0 [qcom_ice] [ 5.173202] platform_probe+0x5c/0xa4 [ 5.173208] really_probe+0xbc/0x2ac [ 5.173217] __driver_probe_device+0x78/0x118 [ 5.173225] driver_probe_device+0x3c/0x178 [ 5.173233] __driver_attach+0x90/0x184 [ 5.173241] bus_for_each_dev+0x7c/0xdc [ 5.173248] driver_attach+0x24/0x3c [ 5.173256] bus_add_driver+0xe4/0x20c [ 5.173263] driver_register+0x68/0x130 [ 5.173271] __platform_driver_register+0x20/0x2c [ 5.173276] qcom_ice_driver_init+0x24/0x1000 [qcom_ice] [ 5.173283] do_one_initcall+0x60/0x1d4 [ 5.173289] do_init_module+0x54/0x240 [ 5.173296] load_module+0x16fc/0x1c60 [ 5.173302] init_module_from_file+0xd8/0xf0 [ 5.173309] __arm64_sys_finit_module+0x1b8/0x334 [ 5.173315] invoke_syscall.constprop.0+0x40/0xf0 [ 5.173320] el0_svc_common.constprop.0+0x38/0xd8 [ 5.173325] do_el0_svc+0x1c/0x28 [ 5.173329] el0_svc+0x34/0x104 [ 5.173337] el0t_64_sync_handler+0xa0/0xe4 [ 5.173346] el0t_64_sync+0x198/0x19c [ 5.173352] SMP: stopping secondary CPUs [ 5.174472] Kernel Offset: 0x3386a7e00000 from 0xffff800080000000 [ 5.174475] PHYS_OFFSET: 0xfff1000080000000 [ 5.174478] CPU features: 0x600000,000b8001,4008c501,0400720b [ 5.174482] Memory Limit: none [ 5.175510] Calling EFI runtime from NMI context [ 5.177177] Calling EFI runtime from NMI context [ 5.178490] Calling EFI runtime from NMI context [ 5.179729] Calling EFI runtime from NMI context [ 5.180928] Calling EFI runtime from NMI context [ 5.182074] Calling EFI runtime from NMI context [ 5.183218] Calling EFI runtime from NMI context [ 5.184319] Calling EFI runtime from NMI context [ 5.185387] Calling EFI runtime from NMI context [ 5.186404] Calling EFI runtime from NMI context [ 5.569397] ---[ end Kernel panic - not syncing: Asynchronous SError Interrupt ]--- -Sumit ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] soc: qcom: ice: Avoid probe deferring for un-supported ICE feature 2026-01-30 11:36 ` Sumit Garg @ 2026-01-30 11:55 ` Konrad Dybcio 2026-01-30 12:29 ` Sumit Garg 0 siblings, 1 reply; 11+ messages in thread From: Konrad Dybcio @ 2026-01-30 11:55 UTC (permalink / raw) To: Sumit Garg Cc: Rob Herring, linux-arm-msm, andersson, konradybcio, abelvesa, mani, linux-kernel, Sumit Garg On 1/30/26 12:36 PM, Sumit Garg wrote: > On Fri, Jan 30, 2026 at 10:59:18AM +0100, Konrad Dybcio wrote: >> On 1/30/26 10:52 AM, Sumit Garg wrote: >>> On Fri, Jan 30, 2026 at 10:34:26AM +0100, Konrad Dybcio wrote: >>>> On 1/30/26 10:11 AM, Sumit Garg wrote: >>>>> From: Sumit Garg <sumit.garg@oss.qualcomm.com> [...] >>> Since qcom,ufs depends on qcom,ice via a phandle, so isn't the probe >>> orderering automatically taken care off? Or that isn't the case here? >> >> No, that's guaranteed by devlink only with certain properties. > > Okay I see. The other alternate solution I can come up is following to > keep the deferred probing intact. Let me know your thoughts on this: > > diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c > index ab9586b8caf5..76bf9f94fbaf 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(-ENODEV); > } > > 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 NULL; > + if (PTR_ERR(ice) == -ENODEV) > + 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) != -ENODEV) > return PTR_ERR(engine); > > platform_set_drvdata(pdev, engine); This looks more robust. Although the UFS and MMC drivers today check for EOPNOTSUPP, so perhaps throwing that would be even better > >> In this case though, I think it could make sense to add it to the >> "suppliers" list in drivers/of/property.c. >> >> I don't know if vendors adding their custom properties there is a >> pattern that +Rob will be happy about though.. > > Not sure if that's a shorter path as I would like to see fix for this > issue backported as well. > > Aside, not sure how much stable ICE feature itself is as I got following > crash with QLI boot firmware on Kodiak: > > [ 5.172970] SError Interrupt on CPU6, code 0x00000000be000000 -- SError > [ 5.172986] CPU: 6 UID: 0 PID: 241 Comm: (udev-worker) Tainted: G M 6.19.0-rc5-next-20260115-gc1a0fee87a05 #9 PREEMPT > [ 5.172996] Tainted: [M]=MACHINE_CHECK > [ 5.172999] Hardware name: Qualcomm Technologies, Inc. Robotics RB3gen2 (DT) > [ 5.173003] pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > [ 5.173010] pc : qcom_ice_create.part.0+0x6c/0x24c [qcom_ice] > [ 5.173024] lr : qcom_ice_create.part.0+0xe4/0x24c [qcom_ice] Could you please decode the pc value with ./scripts/faddr2line? My compiler produces different output ./scripts/faddr2line vmlinux(or path to .ko) <symbol_name> Konrad ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] soc: qcom: ice: Avoid probe deferring for un-supported ICE feature 2026-01-30 11:55 ` Konrad Dybcio @ 2026-01-30 12:29 ` Sumit Garg 2026-01-30 12:36 ` Konrad Dybcio 0 siblings, 1 reply; 11+ messages in thread From: Sumit Garg @ 2026-01-30 12:29 UTC (permalink / raw) To: Konrad Dybcio Cc: Rob Herring, linux-arm-msm, andersson, konradybcio, abelvesa, mani, linux-kernel, Sumit Garg On Fri, Jan 30, 2026 at 12:55:53PM +0100, Konrad Dybcio wrote: > On 1/30/26 12:36 PM, Sumit Garg wrote: > > On Fri, Jan 30, 2026 at 10:59:18AM +0100, Konrad Dybcio wrote: > >> On 1/30/26 10:52 AM, Sumit Garg wrote: > >>> On Fri, Jan 30, 2026 at 10:34:26AM +0100, Konrad Dybcio wrote: > >>>> On 1/30/26 10:11 AM, Sumit Garg wrote: > >>>>> From: Sumit Garg <sumit.garg@oss.qualcomm.com> > > [...] > > >>> Since qcom,ufs depends on qcom,ice via a phandle, so isn't the probe > >>> orderering automatically taken care off? Or that isn't the case here? > >> > >> No, that's guaranteed by devlink only with certain properties. > > > > Okay I see. The other alternate solution I can come up is following to > > keep the deferred probing intact. Let me know your thoughts on this: > > > > diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c > > index ab9586b8caf5..76bf9f94fbaf 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(-ENODEV); > > } > > > > 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 NULL; > > + if (PTR_ERR(ice) == -ENODEV) > > + 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) != -ENODEV) > > return PTR_ERR(engine); > > > > platform_set_drvdata(pdev, engine); > > This looks more robust. Although the UFS and MMC drivers today check > for EOPNOTSUPP, so perhaps throwing that would be even better Sure, I can use that error code instead. > > > > >> In this case though, I think it could make sense to add it to the > >> "suppliers" list in drivers/of/property.c. > >> > >> I don't know if vendors adding their custom properties there is a > >> pattern that +Rob will be happy about though.. > > > > Not sure if that's a shorter path as I would like to see fix for this > > issue backported as well. > > > > Aside, not sure how much stable ICE feature itself is as I got following > > crash with QLI boot firmware on Kodiak: > > > > [ 5.172970] SError Interrupt on CPU6, code 0x00000000be000000 -- SError > > [ 5.172986] CPU: 6 UID: 0 PID: 241 Comm: (udev-worker) Tainted: G M 6.19.0-rc5-next-20260115-gc1a0fee87a05 #9 PREEMPT > > [ 5.172996] Tainted: [M]=MACHINE_CHECK > > [ 5.172999] Hardware name: Qualcomm Technologies, Inc. Robotics RB3gen2 (DT) > > [ 5.173003] pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > > [ 5.173010] pc : qcom_ice_create.part.0+0x6c/0x24c [qcom_ice] > > [ 5.173024] lr : qcom_ice_create.part.0+0xe4/0x24c [qcom_ice] > > Could you please decode the pc value with ./scripts/faddr2line? > My compiler produces different output > > ./scripts/faddr2line vmlinux(or path to .ko) <symbol_name> > This points at: static bool qcom_ice_check_supported(struct qcom_ice *ice) { --> u32 regval = qcom_ice_readl(ice, QCOM_ICE_REG_VERSION); $ ./scripts/faddr2line ./drivers/soc/qcom/qcom_ice.ko qcom_ice_create.part.0+0x6c/0x24c qcom_ice_create.part.0+0x6c/0x24c: readl at /home/sumit/build/upstream/linux/./include/asm-generic/io.h:232 (discriminator 1) (inlined by) qcom_ice_check_supported at /home/sumit/build/upstream/linux/drivers/soc/qcom/ice.c:118 (discriminator 1) (inlined by) qcom_ice_create at /home/sumit/build/upstream/linux/drivers/soc/qcom/ice.c:587 (discriminator 1) To me it looks like an issue related to access control policy. Note that it's the Gunyah based stack only where this issue is seen. -Sumit ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] soc: qcom: ice: Avoid probe deferring for un-supported ICE feature 2026-01-30 12:29 ` Sumit Garg @ 2026-01-30 12:36 ` Konrad Dybcio 2026-02-02 8:32 ` Sumit Garg 0 siblings, 1 reply; 11+ messages in thread From: Konrad Dybcio @ 2026-01-30 12:36 UTC (permalink / raw) To: Sumit Garg Cc: Rob Herring, linux-arm-msm, andersson, konradybcio, abelvesa, mani, linux-kernel, Sumit Garg On 1/30/26 1:29 PM, Sumit Garg wrote: > On Fri, Jan 30, 2026 at 12:55:53PM +0100, Konrad Dybcio wrote: >> On 1/30/26 12:36 PM, Sumit Garg wrote: >>> On Fri, Jan 30, 2026 at 10:59:18AM +0100, Konrad Dybcio wrote: >>>> On 1/30/26 10:52 AM, Sumit Garg wrote: >>>>> On Fri, Jan 30, 2026 at 10:34:26AM +0100, Konrad Dybcio wrote: >>>>>> On 1/30/26 10:11 AM, Sumit Garg wrote: >>>>>>> From: Sumit Garg <sumit.garg@oss.qualcomm.com> >> >> [...] >> >>>>> Since qcom,ufs depends on qcom,ice via a phandle, so isn't the probe >>>>> orderering automatically taken care off? Or that isn't the case here? >>>> >>>> No, that's guaranteed by devlink only with certain properties. >>> >>> Okay I see. The other alternate solution I can come up is following to >>> keep the deferred probing intact. Let me know your thoughts on this: >>> >>> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c >>> index ab9586b8caf5..76bf9f94fbaf 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(-ENODEV); >>> } >>> >>> 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 NULL; >>> + if (PTR_ERR(ice) == -ENODEV) >>> + 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) != -ENODEV) >>> return PTR_ERR(engine); >>> >>> platform_set_drvdata(pdev, engine); >> >> This looks more robust. Although the UFS and MMC drivers today check >> for EOPNOTSUPP, so perhaps throwing that would be even better > > Sure, I can use that error code instead. > >> >>> >>>> In this case though, I think it could make sense to add it to the >>>> "suppliers" list in drivers/of/property.c. >>>> >>>> I don't know if vendors adding their custom properties there is a >>>> pattern that +Rob will be happy about though.. >>> >>> Not sure if that's a shorter path as I would like to see fix for this >>> issue backported as well. >>> >>> Aside, not sure how much stable ICE feature itself is as I got following >>> crash with QLI boot firmware on Kodiak: >>> >>> [ 5.172970] SError Interrupt on CPU6, code 0x00000000be000000 -- SError >>> [ 5.172986] CPU: 6 UID: 0 PID: 241 Comm: (udev-worker) Tainted: G M 6.19.0-rc5-next-20260115-gc1a0fee87a05 #9 PREEMPT >>> [ 5.172996] Tainted: [M]=MACHINE_CHECK >>> [ 5.172999] Hardware name: Qualcomm Technologies, Inc. Robotics RB3gen2 (DT) >>> [ 5.173003] pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) >>> [ 5.173010] pc : qcom_ice_create.part.0+0x6c/0x24c [qcom_ice] >>> [ 5.173024] lr : qcom_ice_create.part.0+0xe4/0x24c [qcom_ice] >> >> Could you please decode the pc value with ./scripts/faddr2line? >> My compiler produces different output >> >> ./scripts/faddr2line vmlinux(or path to .ko) <symbol_name> >> > > This points at: > > static bool qcom_ice_check_supported(struct qcom_ice *ice) > { > --> u32 regval = qcom_ice_readl(ice, QCOM_ICE_REG_VERSION); > > $ ./scripts/faddr2line ./drivers/soc/qcom/qcom_ice.ko qcom_ice_create.part.0+0x6c/0x24c > qcom_ice_create.part.0+0x6c/0x24c: > readl at /home/sumit/build/upstream/linux/./include/asm-generic/io.h:232 (discriminator 1) > (inlined by) qcom_ice_check_supported at /home/sumit/build/upstream/linux/drivers/soc/qcom/ice.c:118 (discriminator 1) > (inlined by) qcom_ice_create at /home/sumit/build/upstream/linux/drivers/soc/qcom/ice.c:587 (discriminator 1) > > To me it looks like an issue related to access control policy. Note that > it's the Gunyah based stack only where this issue is seen. Please first check if that's not just a lack of power, because apparently we never guaranteed that would be present, see: https://lore.kernel.org/linux-arm-msm/20260123-qcom_ice_power_and_clk_vote-v1-0-e9059776f85c@qti.qualcomm.com/ https://lore.kernel.org/linux-arm-msm/20260123-enable-ufs-ice-clock-scaling-v3-0-d0d8532abd98@oss.qualcomm.com/ Konrad ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] soc: qcom: ice: Avoid probe deferring for un-supported ICE feature 2026-01-30 12:36 ` Konrad Dybcio @ 2026-02-02 8:32 ` Sumit Garg 2026-02-02 10:51 ` Manivannan Sadhasivam 0 siblings, 1 reply; 11+ messages in thread From: Sumit Garg @ 2026-02-02 8:32 UTC (permalink / raw) To: Konrad Dybcio Cc: Rob Herring, linux-arm-msm, andersson, konradybcio, abelvesa, mani, linux-kernel, Sumit Garg On Fri, Jan 30, 2026 at 01:36:28PM +0100, Konrad Dybcio wrote: > On 1/30/26 1:29 PM, Sumit Garg wrote: > > On Fri, Jan 30, 2026 at 12:55:53PM +0100, Konrad Dybcio wrote: > >> On 1/30/26 12:36 PM, Sumit Garg wrote: > >>> On Fri, Jan 30, 2026 at 10:59:18AM +0100, Konrad Dybcio wrote: > >>>> On 1/30/26 10:52 AM, Sumit Garg wrote: > >>>>> On Fri, Jan 30, 2026 at 10:34:26AM +0100, Konrad Dybcio wrote: > >>>>>> On 1/30/26 10:11 AM, Sumit Garg wrote: > >>>>>>> From: Sumit Garg <sumit.garg@oss.qualcomm.com> > >> > >> [...] > >> > >>>>> Since qcom,ufs depends on qcom,ice via a phandle, so isn't the probe > >>>>> orderering automatically taken care off? Or that isn't the case here? > >>>> > >>>> No, that's guaranteed by devlink only with certain properties. > >>> > >>> Okay I see. The other alternate solution I can come up is following to > >>> keep the deferred probing intact. Let me know your thoughts on this: > >>> > >>> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c > >>> index ab9586b8caf5..76bf9f94fbaf 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(-ENODEV); > >>> } > >>> > >>> 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 NULL; > >>> + if (PTR_ERR(ice) == -ENODEV) > >>> + 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) != -ENODEV) > >>> return PTR_ERR(engine); > >>> > >>> platform_set_drvdata(pdev, engine); > >> > >> This looks more robust. Although the UFS and MMC drivers today check > >> for EOPNOTSUPP, so perhaps throwing that would be even better > > > > Sure, I can use that error code instead. > > > >> > >>> > >>>> In this case though, I think it could make sense to add it to the > >>>> "suppliers" list in drivers/of/property.c. > >>>> > >>>> I don't know if vendors adding their custom properties there is a > >>>> pattern that +Rob will be happy about though.. > >>> > >>> Not sure if that's a shorter path as I would like to see fix for this > >>> issue backported as well. > >>> > >>> Aside, not sure how much stable ICE feature itself is as I got following > >>> crash with QLI boot firmware on Kodiak: > >>> > >>> [ 5.172970] SError Interrupt on CPU6, code 0x00000000be000000 -- SError > >>> [ 5.172986] CPU: 6 UID: 0 PID: 241 Comm: (udev-worker) Tainted: G M 6.19.0-rc5-next-20260115-gc1a0fee87a05 #9 PREEMPT > >>> [ 5.172996] Tainted: [M]=MACHINE_CHECK > >>> [ 5.172999] Hardware name: Qualcomm Technologies, Inc. Robotics RB3gen2 (DT) > >>> [ 5.173003] pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > >>> [ 5.173010] pc : qcom_ice_create.part.0+0x6c/0x24c [qcom_ice] > >>> [ 5.173024] lr : qcom_ice_create.part.0+0xe4/0x24c [qcom_ice] > >> > >> Could you please decode the pc value with ./scripts/faddr2line? > >> My compiler produces different output > >> > >> ./scripts/faddr2line vmlinux(or path to .ko) <symbol_name> > >> > > > > This points at: > > > > static bool qcom_ice_check_supported(struct qcom_ice *ice) > > { > > --> u32 regval = qcom_ice_readl(ice, QCOM_ICE_REG_VERSION); > > > > $ ./scripts/faddr2line ./drivers/soc/qcom/qcom_ice.ko qcom_ice_create.part.0+0x6c/0x24c > > qcom_ice_create.part.0+0x6c/0x24c: > > readl at /home/sumit/build/upstream/linux/./include/asm-generic/io.h:232 (discriminator 1) > > (inlined by) qcom_ice_check_supported at /home/sumit/build/upstream/linux/drivers/soc/qcom/ice.c:118 (discriminator 1) > > (inlined by) qcom_ice_create at /home/sumit/build/upstream/linux/drivers/soc/qcom/ice.c:587 (discriminator 1) > > > > To me it looks like an issue related to access control policy. Note that > > it's the Gunyah based stack only where this issue is seen. > > Please first check if that's not just a lack of power, because apparently > we never guaranteed that would be present, see: > > https://lore.kernel.org/linux-arm-msm/20260123-qcom_ice_power_and_clk_vote-v1-0-e9059776f85c@qti.qualcomm.com/ > > https://lore.kernel.org/linux-arm-msm/20260123-enable-ufs-ice-clock-scaling-v3-0-d0d8532abd98@oss.qualcomm.com/ Thanks for these references. TBH, if there are some known issues with ICE then it shouldn't be enabled in the common defconfig. BTW, it looks like Mani is already looking into proper fix for this issue as per offline communication. -Sumit ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] soc: qcom: ice: Avoid probe deferring for un-supported ICE feature 2026-02-02 8:32 ` Sumit Garg @ 2026-02-02 10:51 ` Manivannan Sadhasivam 0 siblings, 0 replies; 11+ messages in thread From: Manivannan Sadhasivam @ 2026-02-02 10:51 UTC (permalink / raw) To: Sumit Garg Cc: Konrad Dybcio, Rob Herring, linux-arm-msm, andersson, konradybcio, abelvesa, linux-kernel, Sumit Garg On Mon, Feb 02, 2026 at 02:02:28PM +0530, Sumit Garg wrote: > On Fri, Jan 30, 2026 at 01:36:28PM +0100, Konrad Dybcio wrote: > > On 1/30/26 1:29 PM, Sumit Garg wrote: > > > On Fri, Jan 30, 2026 at 12:55:53PM +0100, Konrad Dybcio wrote: > > >> On 1/30/26 12:36 PM, Sumit Garg wrote: > > >>> On Fri, Jan 30, 2026 at 10:59:18AM +0100, Konrad Dybcio wrote: > > >>>> On 1/30/26 10:52 AM, Sumit Garg wrote: > > >>>>> On Fri, Jan 30, 2026 at 10:34:26AM +0100, Konrad Dybcio wrote: > > >>>>>> On 1/30/26 10:11 AM, Sumit Garg wrote: > > >>>>>>> From: Sumit Garg <sumit.garg@oss.qualcomm.com> > > >> > > >> [...] > > >> > > >>>>> Since qcom,ufs depends on qcom,ice via a phandle, so isn't the probe > > >>>>> orderering automatically taken care off? Or that isn't the case here? > > >>>> > > >>>> No, that's guaranteed by devlink only with certain properties. > > >>> > > >>> Okay I see. The other alternate solution I can come up is following to > > >>> keep the deferred probing intact. Let me know your thoughts on this: > > >>> > > >>> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c > > >>> index ab9586b8caf5..76bf9f94fbaf 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(-ENODEV); > > >>> } > > >>> > > >>> 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 NULL; > > >>> + if (PTR_ERR(ice) == -ENODEV) > > >>> + 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) != -ENODEV) > > >>> return PTR_ERR(engine); > > >>> > > >>> platform_set_drvdata(pdev, engine); > > >> > > >> This looks more robust. Although the UFS and MMC drivers today check > > >> for EOPNOTSUPP, so perhaps throwing that would be even better > > > > > > Sure, I can use that error code instead. > > > > > >> > > >>> > > >>>> In this case though, I think it could make sense to add it to the > > >>>> "suppliers" list in drivers/of/property.c. > > >>>> > > >>>> I don't know if vendors adding their custom properties there is a > > >>>> pattern that +Rob will be happy about though.. > > >>> > > >>> Not sure if that's a shorter path as I would like to see fix for this > > >>> issue backported as well. > > >>> > > >>> Aside, not sure how much stable ICE feature itself is as I got following > > >>> crash with QLI boot firmware on Kodiak: > > >>> > > >>> [ 5.172970] SError Interrupt on CPU6, code 0x00000000be000000 -- SError > > >>> [ 5.172986] CPU: 6 UID: 0 PID: 241 Comm: (udev-worker) Tainted: G M 6.19.0-rc5-next-20260115-gc1a0fee87a05 #9 PREEMPT > > >>> [ 5.172996] Tainted: [M]=MACHINE_CHECK > > >>> [ 5.172999] Hardware name: Qualcomm Technologies, Inc. Robotics RB3gen2 (DT) > > >>> [ 5.173003] pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > > >>> [ 5.173010] pc : qcom_ice_create.part.0+0x6c/0x24c [qcom_ice] > > >>> [ 5.173024] lr : qcom_ice_create.part.0+0xe4/0x24c [qcom_ice] > > >> > > >> Could you please decode the pc value with ./scripts/faddr2line? > > >> My compiler produces different output > > >> > > >> ./scripts/faddr2line vmlinux(or path to .ko) <symbol_name> > > >> > > > > > > This points at: > > > > > > static bool qcom_ice_check_supported(struct qcom_ice *ice) > > > { > > > --> u32 regval = qcom_ice_readl(ice, QCOM_ICE_REG_VERSION); > > > > > > $ ./scripts/faddr2line ./drivers/soc/qcom/qcom_ice.ko qcom_ice_create.part.0+0x6c/0x24c > > > qcom_ice_create.part.0+0x6c/0x24c: > > > readl at /home/sumit/build/upstream/linux/./include/asm-generic/io.h:232 (discriminator 1) > > > (inlined by) qcom_ice_check_supported at /home/sumit/build/upstream/linux/drivers/soc/qcom/ice.c:118 (discriminator 1) > > > (inlined by) qcom_ice_create at /home/sumit/build/upstream/linux/drivers/soc/qcom/ice.c:587 (discriminator 1) > > > > > > To me it looks like an issue related to access control policy. Note that > > > it's the Gunyah based stack only where this issue is seen. > > > > Please first check if that's not just a lack of power, because apparently > > we never guaranteed that would be present, see: > > > > https://lore.kernel.org/linux-arm-msm/20260123-qcom_ice_power_and_clk_vote-v1-0-e9059776f85c@qti.qualcomm.com/ > > > > https://lore.kernel.org/linux-arm-msm/20260123-enable-ufs-ice-clock-scaling-v3-0-d0d8532abd98@oss.qualcomm.com/ > > Thanks for these references. TBH, if there are some known issues with > ICE then it shouldn't be enabled in the common defconfig. BTW, it looks > like Mani is already looking into proper fix for this issue as per > offline communication. > The existing ICE based platform driver probe and of_qcom_ice_get() are quite racy. I'm working on a patch that removes registering the ICE driver as a platform driver and moves qcom_ice_create() to of_qcom_ice_get() with proper reference counting. Will post the patch soon. - Mani -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] soc: qcom: ice: Avoid probe deferring for un-supported ICE feature 2026-01-30 9:59 ` Konrad Dybcio 2026-01-30 11:36 ` Sumit Garg @ 2026-02-02 10:55 ` Manivannan Sadhasivam 1 sibling, 0 replies; 11+ messages in thread From: Manivannan Sadhasivam @ 2026-02-02 10:55 UTC (permalink / raw) To: Konrad Dybcio Cc: Sumit Garg, Rob Herring, linux-arm-msm, andersson, konradybcio, abelvesa, linux-kernel, Sumit Garg On Fri, Jan 30, 2026 at 10:59:18AM +0100, Konrad Dybcio wrote: > On 1/30/26 10:52 AM, Sumit Garg wrote: > > On Fri, Jan 30, 2026 at 10:34:26AM +0100, Konrad Dybcio wrote: > >> On 1/30/26 10:11 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 avoid 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> > >>> --- > >>> drivers/soc/qcom/ice.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c > >>> index b203bc685cad..ab9586b8caf5 100644 > >>> --- a/drivers/soc/qcom/ice.c > >>> +++ b/drivers/soc/qcom/ice.c > >>> @@ -652,7 +652,7 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev) > >>> dev_err(dev, "Cannot get ice instance from %s\n", > >>> dev_name(&pdev->dev)); > >>> platform_device_put(pdev); > >>> - return ERR_PTR(-EPROBE_DEFER); > >>> + return NULL; > >> > >> Wouldn't this wreck the "actually need to defer" case, i.e. > >> > >> > >> qcom,ufs probes first > >> calls devm_of_qcom_ice_get() > >> "ice" reg is missing, non-legacy case > >> qcom,ice is absent (not yet probed) > >> return NULL > >> > >> ... > >> > >> qcom,ice probes > >> > >> ? > > > > Since qcom,ufs depends on qcom,ice via a phandle, so isn't the probe > > orderering automatically taken care off? Or that isn't the case here? > > No, that's guaranteed by devlink only with certain properties. > > In this case though, I think it could make sense to add it to the > "suppliers" list in drivers/of/property.c. > > I don't know if vendors adding their custom properties there is a > pattern that +Rob will be happy about though.. > Yeah. I initially considered adding 'qcom,ice' as a supplier, but was not sure of its reception. So decided not to pursue this path. I think there is no point in exposing the ICE driver as a platform driver and allow it to race against the of_qcom_ice_get() API. - Mani -- மணிவண்ணன் சதாசிவம் ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-02-02 10:55 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-01-30 9:11 [PATCH] soc: qcom: ice: Avoid probe deferring for un-supported ICE feature Sumit Garg 2026-01-30 9:34 ` Konrad Dybcio 2026-01-30 9:52 ` Sumit Garg 2026-01-30 9:59 ` Konrad Dybcio 2026-01-30 11:36 ` Sumit Garg 2026-01-30 11:55 ` Konrad Dybcio 2026-01-30 12:29 ` Sumit Garg 2026-01-30 12:36 ` Konrad Dybcio 2026-02-02 8:32 ` Sumit Garg 2026-02-02 10:51 ` Manivannan Sadhasivam 2026-02-02 10:55 ` Manivannan Sadhasivam
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®