Re: [PATCH] media: qcom: camss: fix error path on configuration of power domains

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/7/24 02:30, Bryan O'Donoghue wrote:
On 07/08/2024 00:27, Vladimir Zapolskiy wrote:
Hi Bryan.

On 8/7/24 02:15, Bryan O'Donoghue wrote:
On 06/08/2024 23:12, Vladimir Zapolskiy wrote:
There is a chance to meet runtime issues during configuration of CAMSS
power domains, because on the error path dev_pm_domain_detach() is
unexpectedly called with NULL or error pointer.

Fixes: 23aa4f0cd327 ("media: qcom: camss: Move VFE power-domain
specifics into vfe.c")
Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@xxxxxxxxxx>

Have you tested this with and without named power domains in your dts ?
The logic here is complex to support both the legacy non-named case and
the updated named required case.

The problem and the fix are pretty straightforward, if you notice any
issues
with it, please let me know.

As it's said in the commit description the problem is unrelated to
named/not named
power domains, I tested the fix only on a platform without
"power-domain-names"
property in camss device tree node.

Could you also provide a backtrace of a failing camss_configure_pd() for
the commit log.

Sure, I believe anyone can get a backtrace simply by disabling camcc at
build time,
so that camss power domain supplies disappear:

Ah OK, that's how, proof positive if its not tested, its not working,
I've extensively tested both named and non-named pds but, yep never with
camcc switched off.


[   13.541205] Unable to handle kernel NULL pointer dereference at
virtual address 00000000000001a2
[   13.550224] Mem abort info:
[   13.553110]   ESR = 0x0000000096000004
[   13.556975]   EC = 0x25: DABT (current EL), IL = 32 bits
[   13.562438]   SET = 0, FnV = 0
[   13.565580]   EA = 0, S1PTW = 0
[   13.568813]   FSC = 0x04: level 0 translation fault
[   13.573824] Data abort info:
[   13.576787]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[   13.582424]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[   13.587614]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[   13.593074] user pgtable: 4k pages, 48-bit VAs, pgdp=000000088a55a000
[   13.599693] [00000000000001a2] pgd=0000000000000000,
p4d=0000000000000000
[   13.606666] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
[   13.613104] Modules linked in:

<snip>

[   13.632753] Workqueue: events_unbound deferred_probe_work_func
[   13.638776] pstate: 23400009 (nzCv daif +PAN -UAO +TCO +DIT -SSBS
BTYPE=--)
[   13.645926] pc : dev_pm_domain_detach+0x8/0x48
[   13.650521] lr : camss_probe+0x374/0x9c0
[   13.654577] sp : ffff800086ec3ab0
[   13.657985] x29: ffff800086ec3ab0 x28: ffff8000855079c0 x27:
ffff800085507000
[   13.665329] x26: ffff000800c4b040 x25: 0000000000000000 x24:
ffff00080aa72c20
[   13.672659] x23: ffff800083060588 x22: ffff000801397010 x21:
ffff800083060588
[   13.679989] x20: 00000000ffffff92 x19: ffff00080aa72880 x18:
ffffffffffffffff
[   13.687318] x17: 6e6570656420676e x16: 69726f6e6769202c x15:
0000000000000000
[   13.694648] x14: 000000000000003e x13: 0000000000000000 x12:
0000000000000000
[   13.701988] x11: ffff00080b350460 x10: ffff00080b350248 x9 :
ffff800081ddcde4
[   13.709318] x8 : ffff00080b350270 x7 : 0000000000000001 x6 :
8000003ff0000000
[   13.716658] x5 : ffff00080149a300 x4 : ffff000a72592b70 x3 :
0000000000076404
[   13.723998] x2 : 0000000000000000 x1 : 0000000000000001 x0 :
ffffffffffffff92
[   13.731338] Call trace:
[   13.733865]  dev_pm_domain_detach+0x8/0x48
[   13.738081]  platform_probe+0x70/0xf0
[   13.741864]  really_probe+0xc4/0x2a8
[   13.745556]  __driver_probe_device+0x80/0x140
[   13.750045]  driver_probe_device+0x48/0x170
[   13.754355]  __device_attach_driver+0xc0/0x148
[   13.758937]  bus_for_each_drv+0x88/0xf0
[   13.762894]  __device_attach+0xb0/0x1c0
[   13.766852]  device_initial_probe+0x1c/0x30
[   13.771165]  bus_probe_device+0xb4/0xc0
[   13.775124]  deferred_probe_work_func+0x90/0xd0
[   13.779787]  process_one_work+0x164/0x3e0
[   13.783920]  worker_thread+0x310/0x420
[   13.787777]  kthread+0x120/0x130
[   13.791123]  ret_from_fork+0x10/0x20
[   13.794821] Code: 828a2cb8 ffff8000 aa1e03e9 d503201f (f9410802)
[   13.801088] ---[ end trace 0000000000000000 ]---

I'd be obliged if you could add to your commit log and verify everything
works for you with both named and unnamed power-domains.


No objections to resend the change with an updated commit message, since
it raised a question, I can add information about a method how to reproduce
the bug.

However I would like to know your opinion about the change itself, are there
any noticeable issues? Thank you in advance!

--
Best wishes,
Vladimir




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux