* ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties
@ 2026-08-27 23:26 D. Manresa
2026-08-28 15:33 ` Sakari Ailus
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: D. Manresa @ 2026-08-27 23:26 UTC (permalink / raw)
To: Sakari Ailus, Bingbu Cao, Dan Scally
Cc: D . Manresa, Tianshu Qiu, Mauro Carvalho Chehab, linux-media,
linux-kernel
Hi,
ipu-bridge registers software nodes at probe time but has no teardown
path at all: nothing unregisters them when the IPU device goes away.
After a PCI remove/rescan of the IPU6, re-probe fails with -EEXIST,
the orphaned nodes stay in /sys/kernel/software_nodes/, and once the
module has been unloaded their properties reference freed module
memory, so sensor drivers that re-probe read garbage. There is no
userspace interface to remove software nodes; only a reboot recovers.
Hardware / kernel
-----------------
- Microsoft Surface Pro 7+, IPU6 Tiger Lake (PCI 8086:9a19, 0000:00:05.0)
- sensors OV5693 (INT33BE), OV8865 (INT347A, dw9719 VCM), OV7251
(INT347E)
- linux-surface 6.19.8-surface-3 (base v6.19.8). The linux-surface
patchset only adds entries to ipu_supported_sensors[]; the
registration/teardown paths discussed here are unmodified mainline.
Line references are against mainline v6.19.
Analysis
--------
ipu_bridge_init() (drivers/media/pci/intel/ipu-bridge.c:837), called
from ipu6_isys_init() (drivers/media/pci/intel/ipu6/ipu6.c:378) during
probe:
- kzallocs struct ipu_bridge (ipu-bridge.c:854); on success it is
intentionally never freed -- the comment at ipu-bridge.c:870 keeps
data in the struct "so that it survives if the module is unloaded
along with the rest of the struct";
- registers the IPU HID node "INT343E" (IPU_HID,
include/media/ipu-bridge.h:11) via software_node_register()
(ipu-bridge.c:864);
- registers one node group per sensor via
software_node_register_node_group() in ipu_bridge_connect_sensor()
(ipu-bridge.c:729);
- points the sensor ACPI fwnode at the swnodes: primary->secondary =
fwnode (ipu-bridge.c:743), and set_secondary_fwnode(dev, fwnode) for
the IPU itself (ipu-bridge.c:893).
The only unregister calls in the file are error paths inside init
itself: ipu_bridge_unregister_sensors() (static, ipu-bridge.c:681,
called at 784 and 898) and software_node_unregister(
&bridge->ipu_hid_node) (ipu-bridge.c:900). No teardown function is
exported, and nothing on the ipu6 remove path unregisters the nodes or
clears the secondary fwnodes.
Because the swnodes and their property_entry arrays live in the leaked
heap struct they survive unbind, but they are not self-contained:
- the property name strings are pointers into the module image
(sensor->prop_names = prop_names at ipu-bridge.c:367; static const
prop_names at ipu-bridge.c:124);
- the "link-frequencies" values point at cfg->link_freqs inside the
module-rodata table ipu_supported_sensors[] (property built at
ipu-bridge.c:433, table at ipu-bridge.c:51).
So after ipu_bridge.ko is unloaded, the still-registered nodes carry
dangling pointers, and the sensors' ACPI fwnodes still have
fwnode->secondary aimed at them.
Observed behaviour (live, 2026-08-27)
-------------------------------------
Attempting to recover a wedged ISYS by PCI remove/rescan:
echo 1 > /sys/bus/pci/devices/0000:00:05.0/remove
modprobe -r intel_ipu6_isys intel_ipu6 # ipu_bridge unloads too
echo 1 > /sys/bus/pci/rescan
modprobe intel_ipu6
dmesg (verbatim, trimmed):
sysfs: cannot create duplicate filename
'/kernel/software_nodes/INT343E'
Call Trace:
sysfs_warn_dup+0x8b/0xb0
...
swnode_register+0x165/0x240
software_node_register+0xd2/0x120
ipu_bridge_init+0x192/0xeb0 [ipu_bridge]
ipu6_pci_probe+0x417/0xbe0 [intel_ipu6]
...
kobject: kobject_add_internal failed for INT343E with -EEXIST, don't
try to register things with the same name in the same directory.
intel-ipu6 0000:00:05.0: Failed to register the IPU HID node
intel-ipu6 0000:00:05.0: error -EEXIST: IPU6 bridge init failed
intel-ipu6 0000:00:05.0: probe with driver intel-ipu6 failed with
error -17
The sensor drivers, re-probing against the stale secondary fwnodes,
then read poisoned link-frequencies (verbatim):
ov5693 i2c-INT33BE:00: supported link freq 419200000ll not found
ov5693 i2c-INT33BE:00: probe with driver ov5693 failed with error -22
ov7251 i2c-INT347E:00: error -EINVAL: no supported link freq found
ov7251 i2c-INT347E:00: probe with driver ov7251 failed with error -22
ov8865 i2c-INT347A:00: failed to find 360000000 clk rate in endpoint
link-frequencies
(The ov5693/ov7251 on this machine carry local patches, so the exact
wording of their two lines may differ from pure mainline; the ov8865
line is the unmodified mainline driver, and the values being missed --
419200000, 360000000 -- are exactly what the bridge publishes for
these sensors at ipu-bridge.c:61 and :69, so the properties no longer
return their original contents.)
/sys/kernel/software_nodes/ afterwards still lists INT343E, INT33BE-4,
INT347A-1, INT347E-5 and dw9719-1. There is no userspace interface to
unregister a software node, so the system cannot be recovered without
a reboot.
Reproducer
----------
On any IPU6 machine handled by ipu-bridge:
1. boot normally, wait for "Connected N cameras"
2. echo 1 > /sys/bus/pci/devices/0000:00:05.0/remove
3. modprobe -r intel_ipu6_isys intel_ipu6
4. echo 1 > /sys/bus/pci/rescan
5. modprobe intel_ipu6
Step 5 (or the automatic probe at step 4) fails with the -EEXIST trace
above and the cameras are gone until reboot.
Expected: either an idempotent/refcounted ipu_bridge_init() that
reuses already-registered nodes across rebinds, or a real teardown
that unregisters the node groups and clears the secondary fwnodes on
unbind (at which point the deliberate struct leak could go as well).
Workaround: none other than not touching the PCI device; documented as
an operational trap in
https://github.com/dmanresa-saes/surface-ipu6-cameras
Happy to test patches on this hardware.
This report was drafted with AI assistance (Anthropic Claude) and
verified on the actual hardware by the undersigned.
D. Manresa <dmanresa@gmail.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties
2026-08-27 23:26 ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties D. Manresa
@ 2026-08-28 15:33 ` Sakari Ailus
2026-08-28 20:54 ` D. Manresa
2026-08-31 9:03 ` Sakari Ailus
2026-08-31 9:42 ` [PATCH 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind D. Manresa
2 siblings, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2026-08-28 15:33 UTC (permalink / raw)
To: D. Manresa
Cc: Bingbu Cao, Dan Scally, Tianshu Qiu, Mauro Carvalho Chehab,
linux-media, linux-kernel
Hi D.,
On Fri, Aug 28, 2026 at 01:26:36AM +0200, D. Manresa wrote:
> Hi,
>
> ipu-bridge registers software nodes at probe time but has no teardown
> path at all: nothing unregisters them when the IPU device goes away.
> After a PCI remove/rescan of the IPU6, re-probe fails with -EEXIST,
> the orphaned nodes stay in /sys/kernel/software_nodes/, and once the
> module has been unloaded their properties reference freed module
> memory, so sensor drivers that re-probe read garbage. There is no
> userspace interface to remove software nodes; only a reboot recovers.
>
> Hardware / kernel
> -----------------
> - Microsoft Surface Pro 7+, IPU6 Tiger Lake (PCI 8086:9a19, 0000:00:05.0)
> - sensors OV5693 (INT33BE), OV8865 (INT347A, dw9719 VCM), OV7251
> (INT347E)
> - linux-surface 6.19.8-surface-3 (base v6.19.8). The linux-surface
> patchset only adds entries to ipu_supported_sensors[]; the
> registration/teardown paths discussed here are unmodified mainline.
> Line references are against mainline v6.19.
The regulator string length has been increased to 6 since and I believe
with <20260729-sp7plus-int3472-v2-1-cdfaf97ac3ad@berg.pm> (on LMML) the
camera might work streaming-wise at least. There might be Bayer order
issues though, there was another patchset
(<20260729-sp7plus-ov-flips-v2-0-91884b81a8f5@berg.pm>) addressing those.
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties
2026-08-28 15:33 ` Sakari Ailus
@ 2026-08-28 20:54 ` D. Manresa
2026-08-30 12:40 ` johannes.goede
0 siblings, 1 reply; 17+ messages in thread
From: D. Manresa @ 2026-08-28 20:54 UTC (permalink / raw)
To: Sakari Ailus
Cc: D . Manresa, Dan Scally, Mauro Carvalho Chehab, linux-media,
linux-kernel
Hi Sakari,
On Fri, Aug 28, 2026 at 06:33:16PM +0300, Sakari Ailus wrote:
> The regulator string length has been increased to 6 since and I believe
> with <20260729-sp7plus-int3472-v2-1-cdfaf97ac3ad@berg.pm> (on LMML) the
> camera might work streaming-wise at least. There might be Bayer order
> issues though, there was another patchset
> (<20260729-sp7plus-ov-flips-v2-0-91884b81a8f5@berg.pm>) addressing those.
Thanks for the pointers -- good to know about the string length fix, and
we are aware of Jakob's two series: we have adopted his POWER1 approach
in our local tree and have offered him Tested-by on this hardware to
help both series move.
This report is orthogonal to the sensor bring-up, though: all three
sensors here are fully powered and streaming (the cameras work
end-to-end, hardware ISP included). The issue is that ipu-bridge has no
teardown -- after any unbind of the IPU PCI device its software nodes
stay registered, so a rescan can never re-probe (-EEXIST), and once the
module is gone the node properties dangle into freed module memory. It
reproduces on any ipu-bridge machine regardless of sensor state.
Would a fix be welcome, and if so, which direction do you prefer:
a real teardown on unbind (unregister the node groups, clear the
secondary fwnodes, drop the deliberate struct leak), or making
ipu_bridge_init() idempotent so a rebind reuses the already-registered
nodes? Happy to write and test either on this hardware.
Regards,
D. Manresa
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties
2026-08-28 20:54 ` D. Manresa
@ 2026-08-30 12:40 ` johannes.goede
2026-08-31 8:02 ` Sakari Ailus
0 siblings, 1 reply; 17+ messages in thread
From: johannes.goede @ 2026-08-30 12:40 UTC (permalink / raw)
To: D. Manresa, Sakari Ailus
Cc: Dan Scally, Mauro Carvalho Chehab, linux-media, linux-kernel
Hi,
On 28-Aug-26 22:54, D. Manresa wrote:
> Hi Sakari,
>
> On Fri, Aug 28, 2026 at 06:33:16PM +0300, Sakari Ailus wrote:
>> The regulator string length has been increased to 6 since and I believe
>> with <20260729-sp7plus-int3472-v2-1-cdfaf97ac3ad@berg.pm> (on LMML) the
>> camera might work streaming-wise at least. There might be Bayer order
>> issues though, there was another patchset
>> (<20260729-sp7plus-ov-flips-v2-0-91884b81a8f5@berg.pm>) addressing those.
>
> Thanks for the pointers -- good to know about the string length fix, and
> we are aware of Jakob's two series: we have adopted his POWER1 approach
> in our local tree and have offered him Tested-by on this hardware to
> help both series move.
>
> This report is orthogonal to the sensor bring-up, though: all three
> sensors here are fully powered and streaming (the cameras work
> end-to-end, hardware ISP included). The issue is that ipu-bridge has no
> teardown -- after any unbind of the IPU PCI device its software nodes
> stay registered, so a rescan can never re-probe (-EEXIST), and once the
> module is gone the node properties dangle into freed module memory. It
> reproduces on any ipu-bridge machine regardless of sensor state.
>
> Would a fix be welcome, and if so, which direction do you prefer:
> a real teardown on unbind (unregister the node groups, clear the
> secondary fwnodes, drop the deliberate struct leak), or making
> ipu_bridge_init() idempotent so a rebind reuses the already-registered
> nodes? Happy to write and test either on this hardware.
Not Sakari, but IIRC the goal has always been for the nodes to
stick around (be leaked) since other drivers may still reference
them when the module goes away and then a rebind should use
the already-registered nodes,
Which is why all of the swnodes are dynamically allocated and e.g.
strings a strdup-ed and things are never freed. If there are pointers
in the swnodes to things which go away on module unload then that is
a bug which should fixed.
Regards,
Hans
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties
2026-08-30 12:40 ` johannes.goede
@ 2026-08-31 8:02 ` Sakari Ailus
2026-08-31 10:23 ` D. Manresa
0 siblings, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2026-08-31 8:02 UTC (permalink / raw)
To: johannes.goede
Cc: D. Manresa, Dan Scally, Mauro Carvalho Chehab, linux-media,
linux-kernel
Hi Hans, D.,
On Sun, Aug 30, 2026 at 02:40:52PM +0200, johannes.goede@oss.qualcomm.com wrote:
> Hi,
>
> On 28-Aug-26 22:54, D. Manresa wrote:
> > Hi Sakari,
> >
> > On Fri, Aug 28, 2026 at 06:33:16PM +0300, Sakari Ailus wrote:
> >> The regulator string length has been increased to 6 since and I believe
> >> with <20260729-sp7plus-int3472-v2-1-cdfaf97ac3ad@berg.pm> (on LMML) the
> >> camera might work streaming-wise at least. There might be Bayer order
> >> issues though, there was another patchset
> >> (<20260729-sp7plus-ov-flips-v2-0-91884b81a8f5@berg.pm>) addressing those.
> >
> > Thanks for the pointers -- good to know about the string length fix, and
> > we are aware of Jakob's two series: we have adopted his POWER1 approach
> > in our local tree and have offered him Tested-by on this hardware to
> > help both series move.
> >
> > This report is orthogonal to the sensor bring-up, though: all three
> > sensors here are fully powered and streaming (the cameras work
> > end-to-end, hardware ISP included). The issue is that ipu-bridge has no
> > teardown -- after any unbind of the IPU PCI device its software nodes
> > stay registered, so a rescan can never re-probe (-EEXIST), and once the
> > module is gone the node properties dangle into freed module memory. It
> > reproduces on any ipu-bridge machine regardless of sensor state.
> >
> > Would a fix be welcome, and if so, which direction do you prefer:
> > a real teardown on unbind (unregister the node groups, clear the
> > secondary fwnodes, drop the deliberate struct leak), or making
> > ipu_bridge_init() idempotent so a rebind reuses the already-registered
> > nodes? Happy to write and test either on this hardware.
>
> Not Sakari, but IIRC the goal has always been for the nodes to
> stick around (be leaked) since other drivers may still reference
> them when the module goes away and then a rebind should use
> the already-registered nodes,
>
> Which is why all of the swnodes are dynamically allocated and e.g.
> strings a strdup-ed and things are never freed. If there are pointers
> in the swnodes to things which go away on module unload then that is
> a bug which should fixed.
Indeed. Software nodes do support refcounting so in principle it should be
possible to remove the software nodes, given no driver is holding a
reference to them.
Software nodes, like OF and ACPI nodes, can reference other nodes and this
is the case with the graph data structure. As the remote-endpoint
properties refer to the other nodes, there should be a circular dependency
there, and current framework implementation really doesn't allow removing
the other node, whether or not a reference to the remote node is actually
acquired right now.
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties
2026-08-27 23:26 ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties D. Manresa
2026-08-28 15:33 ` Sakari Ailus
@ 2026-08-31 9:03 ` Sakari Ailus
2026-08-31 9:42 ` [PATCH 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind D. Manresa
2 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2026-08-31 9:03 UTC (permalink / raw)
To: D. Manresa
Cc: Bingbu Cao, Dan Scally, Tianshu Qiu, Mauro Carvalho Chehab,
linux-media, linux-kernel
Hi D.,
On Fri, Aug 28, 2026 at 01:26:36AM +0200, D. Manresa wrote:
> Observed behaviour (live, 2026-08-27)
> -------------------------------------
> Attempting to recover a wedged ISYS by PCI remove/rescan:
>
> echo 1 > /sys/bus/pci/devices/0000:00:05.0/remove
> modprobe -r intel_ipu6_isys intel_ipu6 # ipu_bridge unloads too
> echo 1 > /sys/bus/pci/rescan
> modprobe intel_ipu6
I recall unbinding the ipu6 driver successfully in the past. Do you ensure
above all sub-device drivers have been unbound first? I guess the V4L2
framework nor the ipu6 driver necessarily ensure that right now.
In general, unloading and loading modules may work, and in this case there
are lifetime issues to be addressed (MC device, as well as V4L2 sub-device
nodes), too, but for testing purposes this should just work.
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind
2026-08-27 23:26 ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties D. Manresa
2026-08-28 15:33 ` Sakari Ailus
2026-08-31 9:03 ` Sakari Ailus
@ 2026-08-31 9:42 ` D. Manresa
2026-08-31 9:42 ` [PATCH 1/2] media: ipu-bridge: don't reference the module image from software nodes D. Manresa
` (2 more replies)
2 siblings, 3 replies; 17+ messages in thread
From: D. Manresa @ 2026-08-31 9:42 UTC (permalink / raw)
To: Sakari Ailus, Hans de Goede, Daniel Scally
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, D . Manresa
Hello,
This implements what was agreed in the "ipu-bridge: software nodes are
never unregistered" thread [1]: the software nodes are deliberately
leaked and cannot be removed (circular remote-endpoint references), so
instead of a teardown, make the two halves of the intended design work -
the nodes must actually survive module unload, and a rebind must reuse
them instead of failing.
[1/2] makes the registered properties self-contained in the never-freed
bridge allocation. One correction to my original report: the property
name strings I pointed at (prop_names) were never a problem - struct
ipu_property_names holds char arrays, so those are already copied. The
real module-image references were the "link-frequencies" property
values (pointing into the ipu_supported_sensors[] rodata) and the
"lens-focus" property name string literal. The link-frequencies one is
directly observable on hardware: with the creator module unloaded, a
re-probing sensor reads poisoned frequencies ("supported link freq
419200000ll not found"); reloading the module - which puts identical
rodata back at the same address - makes the same probe succeed again.
[2/2] adds the reuse path to ipu_bridge_init(): if the IPU HID node is
already registered, point the IPU's secondary fwnode at it and return.
The sensors' ACPI fwnodes keep their secondary pointers from the first
bind (nothing clears them), and with [1/2] the nodes they point at are
still valid.
Tested on a Surface Pro 7+ (IPU6 Tiger Lake, OV5693 + OV8865 + OV7251):
the PCI remove -> module unload -> rescan -> modprobe sequence from the
report, which today is fatal until reboot (-EEXIST), completes cleanly
with this series - "Reusing the previously registered software nodes" -
twice in a row, with all three cameras streaming after each rebind. Also
re-verified per Sakari's question that the failure is identical when all
sensor sub-device drivers are unbound before the PCI remove (answered
with the data in [1]).
The series was developed with the assistance of an AI tool (Claude) and
verified on the hardware described above.
Thanks,
D. Manresa
D. Manresa (2):
media: ipu-bridge: don't reference the module image from the software
nodes
media: ipu-bridge: reuse the software nodes on rebind
drivers/media/pci/intel/ipu-bridge.c | 38 ++++++++++++++++++++++++++---
include/media/ipu-bridge.h | 9 +++++++++
2 files changed, 44 insertions(+), 3 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] media: ipu-bridge: don't reference the module image from software nodes
2026-08-31 9:42 ` [PATCH 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind D. Manresa
@ 2026-08-31 9:42 ` D. Manresa
2026-08-31 9:42 ` [PATCH 2/2] media: ipu-bridge: reuse the software nodes on rebind D. Manresa
2026-08-31 14:03 ` [PATCH v2 0/2] media: ipu-bridge: survive module unload and " D. Manresa
2 siblings, 0 replies; 17+ messages in thread
From: D. Manresa @ 2026-08-31 9:42 UTC (permalink / raw)
To: Sakari Ailus, Hans de Goede, Daniel Scally
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, D . Manresa
The software nodes registered by ipu_bridge_init() are deliberately
never unregistered: sensor drivers and the fwnode graph keep references
to them, so they are left registered when the ipu-bridge module is
unloaded and a later rebind is intended to reuse the already registered
nodes.
For that to work, nothing reachable from the registered nodes may point
into the ipu-bridge module image. Most of the data already lives in the
dedicated, never freed, struct ipu_bridge allocation: the property name
strings in struct ipu_property_names are character arrays copied into
the per-sensor struct, the node name strings are likewise character
arrays inside the struct, and the data-lanes array is a struct
ipu_bridge member precisely so that "it survives if the module is
unloaded along with the rest of the struct".
Two references into the module image remain, though:
1. The values of the "link-frequencies" endpoint property point at
cfg->link_freqs inside the const ipu_supported_sensors[] table in
module rodata.
2. The name of the "lens-focus" device property is a string literal in
module rodata.
Both dangle as soon as the module is unloaded, while the properties
that carry them stay registered and readable. In practice, after
unloading and reloading the IPU modules on a Surface Pro 7+ (IPU6,
ov8865 + ov5693 + ov7251), re-probing sensor drivers read poisoned
link-frequencies from the surviving nodes and fail to probe:
ov8865: failed to find 360000000 clk rate in endpoint link-frequencies
ov5693: supported link freq 419200000 not found
where 419200000/360000000 are exactly the values the bridge had
originally published for those sensors, i.e. the properties no longer
return their original contents. Depending on what happens to the freed
module mapping, reading the properties can also fault. Similarly, a VCM
lookup through the "lens-focus" reference can no longer match (or
faults) once the property's name pointer is dangling.
Copy the link frequencies and the "lens-focus" property name into
struct ipu_bridge, next to the data-lanes array kept there for the same
reason, and make the registered properties point at those copies, so
the nodes survive module unload intact. These were the only remaining
references from the registered nodes into the module image (the
sensor->vcm_type pointer into ipu_vcm_types[] is only dereferenced
during ipu_bridge_init() itself and is not reachable from the nodes).
Developed with the assistance of an AI tool (Claude)
Fixes: 803abec64ef9 ("media: ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver")
Fixes: 68b9bcc8a534 ("media: ipu3-cio2: Add support for instantiating i2c-clients for VCMs")
Signed-off-by: D. Manresa <dmanresa@gmail.com>
---
drivers/media/pci/intel/ipu-bridge.c | 14 +++++++++++---
include/media/ipu-bridge.h | 9 +++++++++
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 1bb3a3e..4de42ed 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -503,7 +503,8 @@ static void ipu_bridge_create_fwnode_properties(
sensor->vcm_ref[0] =
SOFTWARE_NODE_REFERENCE(&sensor->swnodes[SWNODE_VCM]);
sensor->dev_properties[3] =
- PROPERTY_ENTRY_REF_ARRAY("lens-focus", sensor->vcm_ref);
+ PROPERTY_ENTRY_REF_ARRAY(bridge->lens_focus,
+ sensor->vcm_ref);
}
sensor->ep_properties[0] = PROPERTY_ENTRY_U32(
@@ -516,11 +517,17 @@ static void ipu_bridge_create_fwnode_properties(
sensor->prop_names.remote_endpoint,
sensor->local_ref);
- if (cfg->nr_link_freqs > 0)
+ if (cfg->nr_link_freqs > 0) {
+ u64 *link_freqs = bridge->link_freqs[sensor - bridge->sensors];
+
+ memcpy(link_freqs, cfg->link_freqs,
+ cfg->nr_link_freqs * sizeof(*link_freqs));
+
sensor->ep_properties[3] = PROPERTY_ENTRY_U64_ARRAY_LEN(
sensor->prop_names.link_frequencies,
- cfg->link_freqs,
+ link_freqs,
cfg->nr_link_freqs);
+ }
sensor->ipu_properties[0] = PROPERTY_ENTRY_U32_ARRAY_LEN(
sensor->prop_names.data_lanes,
@@ -943,6 +950,7 @@ int ipu_bridge_init(struct device *dev,
strscpy(bridge->ipu_node_name, IPU_HID,
sizeof(bridge->ipu_node_name));
+ strscpy(bridge->lens_focus, "lens-focus", sizeof(bridge->lens_focus));
bridge->ipu_hid_node.name = bridge->ipu_node_name;
bridge->dev = dev;
bridge->parse_sensor_fwnode = parse_sensor_fwnode;
diff --git a/include/media/ipu-bridge.h b/include/media/ipu-bridge.h
index 16fac76..4e91ec3 100644
--- a/include/media/ipu-bridge.h
+++ b/include/media/ipu-bridge.h
@@ -164,6 +164,15 @@ struct ipu_bridge {
char ipu_node_name[ACPI_ID_LEN];
struct software_node ipu_hid_node;
u32 data_lanes[4];
+ /*
+ * The software nodes registered by the bridge are deliberately never
+ * unregistered (see ipu_bridge_init()), so every string and array
+ * they reference must live in this never freed struct rather than in
+ * the module image, so that the nodes stay intact if the module is
+ * unloaded.
+ */
+ char lens_focus[sizeof("lens-focus")];
+ u64 link_freqs[IPU_MAX_PORTS][MAX_NUM_LINK_FREQS];
unsigned int n_sensors;
struct ipu_sensor sensors[IPU_MAX_PORTS];
};
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] media: ipu-bridge: reuse the software nodes on rebind
2026-08-31 9:42 ` [PATCH 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind D. Manresa
2026-08-31 9:42 ` [PATCH 1/2] media: ipu-bridge: don't reference the module image from software nodes D. Manresa
@ 2026-08-31 9:42 ` D. Manresa
2026-08-31 12:11 ` Sakari Ailus
2026-08-31 14:03 ` [PATCH v2 0/2] media: ipu-bridge: survive module unload and " D. Manresa
2 siblings, 1 reply; 17+ messages in thread
From: D. Manresa @ 2026-08-31 9:42 UTC (permalink / raw)
To: Sakari Ailus, Hans de Goede, Daniel Scally
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, D . Manresa
The software nodes registered by ipu_bridge_init() are deliberately
never unregistered, and the intended design is for a rebind to reuse
the already registered nodes. That reuse path however only exists for
the case where the IPU device kept its secondary fwnode link, which the
fwnode graph check at the top of ipu_bridge_init() detects: then the
function returns early. When the link is gone, ipu_bridge_init()
unconditionally registers the IPU HID software node again, which fails
with -EEXIST on the sysfs name (the node from the previous bind is
still registered) and the IPU driver fails to probe.
That is exactly what happens when the IPU PCI device is removed and
re-scanned: device_del() unsets the ACPI companion, and
set_primary_fwnode(dev, NULL) then clears the ACPI fwnode's ->secondary
pointer, so the fwnode graph check on the next probe finds no endpoints
and falls through to registration. Observed on a Surface Pro 7+ (IPU6):
echo 1 > /sys/bus/pci/devices/0000:00:05.0/remove
modprobe -r intel_ipu6_isys intel_ipu6 # ipu-bridge unloads too
echo 1 > /sys/bus/pci/rescan
modprobe intel_ipu6
sysfs: cannot create duplicate filename '/kernel/software_nodes/INT343E'
intel-ipu6 0000:00:05.0: Failed to register the IPU HID node
intel-ipu6: probe of 0000:00:05.0 failed with error -17
after which the cameras are unusable until reboot.
Add the missing reuse path: if the IPU software node is already
registered, look it up with software_node_find_by_name(), point the
device's secondary fwnode at it and return success. Restoring the IPU's
secondary fwnode is all a rebind needs: the sensors' ACPI fwnodes still
carry their secondary fwnode pointers from the first bind (the sensor
devices are not removed by an IPU unbind, so nothing clears those), and
the IVSC and VCM links likewise live on devices that survive an IPU
rebind. The previous commit made the registered nodes self-contained in
the never freed bridge allocation, so their properties are still valid
here. The IVSC readiness check is intentionally skipped on this path,
as the IVSC links were already established by the first bind.
software_node_find_by_name() takes a reference on the node it returns;
drop it right away since the node is kept alive by its never dropped
registration, matching the reference handling of the initial-bind path.
Developed with the assistance of an AI tool (Claude)
Fixes: 803abec64ef9 ("media: ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver")
Signed-off-by: D. Manresa <dmanresa@gmail.com>
---
drivers/media/pci/intel/ipu-bridge.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 4de42ed..6fa1c3c 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -930,6 +930,7 @@ static DEFINE_MUTEX(ipu_bridge_mutex);
int ipu_bridge_init(struct device *dev,
ipu_parse_sensor_fwnode_t parse_sensor_fwnode)
{
+ const struct software_node *ipu_node;
struct fwnode_handle *fwnode;
struct ipu_bridge *bridge;
unsigned int i;
@@ -940,6 +941,29 @@ int ipu_bridge_init(struct device *dev,
if (!ipu_bridge_check_fwnode_graph(dev_fwnode(dev)))
return 0;
+ /*
+ * The software nodes registered by a previous ipu_bridge_init() call
+ * are deliberately kept registered when the module is unloaded, and
+ * the sensors' ACPI fwnodes still have them as their secondary
+ * fwnodes. If the IPU software node is already registered this is a
+ * rebind, e.g. after the PCI device was removed and re-scanned,
+ * which drops the IPU's secondary fwnode link. Registering the nodes
+ * again would fail with -EEXIST, so instead reuse them and just
+ * restore the IPU's secondary fwnode link.
+ */
+ ipu_node = software_node_find_by_name(NULL, IPU_HID);
+ if (ipu_node) {
+ fwnode = software_node_fwnode(ipu_node);
+ set_secondary_fwnode(dev, fwnode);
+ /*
+ * The node stays registered, it does not need the reference
+ * software_node_find_by_name() took to stay alive.
+ */
+ fwnode_handle_put(fwnode);
+ dev_info(dev, "Reusing the previously registered software nodes\n");
+ return 0;
+ }
+
if (!ipu_bridge_ivsc_is_ready())
return dev_err_probe(dev, -EPROBE_DEFER,
"waiting for IVSC to become ready\n");
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties
2026-08-31 8:02 ` Sakari Ailus
@ 2026-08-31 10:23 ` D. Manresa
2026-08-31 11:36 ` Sakari Ailus
0 siblings, 1 reply; 17+ messages in thread
From: D. Manresa @ 2026-08-31 10:23 UTC (permalink / raw)
To: Sakari Ailus
Cc: Hans de Goede, Daniel Scally, Mauro Carvalho Chehab, linux-media,
linux-kernel, D . Manresa
[Resending with the lists on Cc - the first copy of this reply went out
to the people only, due to the same mail tooling error on my side that
Hans just caught on the int3472 patch. Fixed now; apologies for the
duplicate, Sakari and Hans.]
On Sun, 31 Aug 2026, Sakari Ailus wrote:
> I recall unbinding the ipu6 driver successfully in the past. Do you ensure
> above all sub-device drivers have been unbound first? I guess the V4L2
> framework nor the ipu6 driver necessarily ensure that right now.
Measured it, since the machine reproduces this in a minute: unbinding all
three sensor sub-device drivers first (ov5693, ov8865, ov7251 - each
confirmed unbound via sysfs; the VCM client had no driver bound) and then
running the same PCI remove -> module unload -> rescan -> modprobe sequence
fails identically, byte for byte:
sysfs: cannot create duplicate filename '/kernel/software_nodes/INT343E'
software_node_register+0xd2/0x120
ipu_bridge_init+0x192/0xeb0 [ipu_bridge]
ipu6_pci_probe+0x417/0xbe0 [intel_ipu6]
kobject: kobject_add_internal failed for INT343E with -EEXIST, ...
intel-ipu6 0000:00:05.0: error -EEXIST: IPU6 bridge init failed
Which makes sense: unbinding the sensors neither unregisters the bridge's
software nodes nor clears their ACPI fwnode->secondary pointers, and the
-EEXIST happens at the IPU HID node registration, before any per-sensor code
runs. A plain module unload/reload without the PCI remove does work, as you
say - the device keeps its secondary fwnode, so the graph is still wired -
but any path that goes through device_del() (which clears the secondary via
set_primary_fwnode(dev, NULL)) ends at the -EEXIST.
While re-testing this I also got a clean confirmation of the dangling
link-frequencies: with the creator module unloaded, rebinding ov5693 against
the surviving nodes fails with "supported link freq 419200000ll not found"
(-22), and the same rebind succeeds the moment the module is loaded again -
identical rodata back at the same address under the stale pointer.
Hans: thanks for the quick ack on the split. Series sent as
[PATCH 0/2] media: ipu-bridge: survive module unload and reuse the
software nodes on rebind
threaded to this report - with one correction to my point 2a folded into the
commit message of 1/2: the property *name* strings in prop_names were never a
problem (char arrays, already copied); the real module-image references were
the link-frequencies values and the "lens-focus" property name literal.
D. Manresa <dmanresa@gmail.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties
2026-08-31 10:23 ` D. Manresa
@ 2026-08-31 11:36 ` Sakari Ailus
0 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2026-08-31 11:36 UTC (permalink / raw)
To: D. Manresa
Cc: Hans de Goede, Daniel Scally, Mauro Carvalho Chehab, linux-media,
linux-kernel
Hi D.,
On Mon, Aug 31, 2026 at 12:23:28PM +0200, D. Manresa wrote:
> [Resending with the lists on Cc - the first copy of this reply went out
> to the people only, due to the same mail tooling error on my side that
> Hans just caught on the int3472 patch. Fixed now; apologies for the
> duplicate, Sakari and Hans.]
>
> On Sun, 31 Aug 2026, Sakari Ailus wrote:
> > I recall unbinding the ipu6 driver successfully in the past. Do you ensure
> > above all sub-device drivers have been unbound first? I guess the V4L2
In fact the sub-device drivers aren't meant to go anywhere whilst the
sub-devices remain registered.
> > framework nor the ipu6 driver necessarily ensure that right now.
>
> Measured it, since the machine reproduces this in a minute: unbinding all
> three sensor sub-device drivers first (ov5693, ov8865, ov7251 - each
> confirmed unbound via sysfs; the VCM client had no driver bound) and then
> running the same PCI remove -> module unload -> rescan -> modprobe sequence
> fails identically, byte for byte:
>
> sysfs: cannot create duplicate filename '/kernel/software_nodes/INT343E'
> software_node_register+0xd2/0x120
> ipu_bridge_init+0x192/0xeb0 [ipu_bridge]
> ipu6_pci_probe+0x417/0xbe0 [intel_ipu6]
> kobject: kobject_add_internal failed for INT343E with -EEXIST, ...
> intel-ipu6 0000:00:05.0: error -EEXIST: IPU6 bridge init failed
>
> Which makes sense: unbinding the sensors neither unregisters the bridge's
> software nodes nor clears their ACPI fwnode->secondary pointers, and the
> -EEXIST happens at the IPU HID node registration, before any per-sensor code
> runs. A plain module unload/reload without the PCI remove does work, as you
> say - the device keeps its secondary fwnode, so the graph is still wired -
> but any path that goes through device_del() (which clears the secondary via
> set_primary_fwnode(dev, NULL)) ends at the -EEXIST.
Indeed.
>
> While re-testing this I also got a clean confirmation of the dangling
> link-frequencies: with the creator module unloaded, rebinding ov5693 against
> the surviving nodes fails with "supported link freq 419200000ll not found"
> (-22), and the same rebind succeeds the moment the module is loaded again -
> identical rodata back at the same address under the stale pointer.
>
> Hans: thanks for the quick ack on the split. Series sent as
>
> [PATCH 0/2] media: ipu-bridge: survive module unload and reuse the
> software nodes on rebind
>
> threaded to this report - with one correction to my point 2a folded into the
> commit message of 1/2: the property *name* strings in prop_names were never a
> problem (char arrays, already copied); the real module-image references were
> the link-frequencies values and the "lens-focus" property name literal.
>
> D. Manresa <dmanresa@gmail.com>
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] media: ipu-bridge: reuse the software nodes on rebind
2026-08-31 9:42 ` [PATCH 2/2] media: ipu-bridge: reuse the software nodes on rebind D. Manresa
@ 2026-08-31 12:11 ` Sakari Ailus
0 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2026-08-31 12:11 UTC (permalink / raw)
To: D. Manresa
Cc: Hans de Goede, Daniel Scally, Mauro Carvalho Chehab, linux-media,
linux-kernel
Hi D.,
On Mon, Aug 31, 2026 at 11:42:56AM +0200, D. Manresa wrote:
> The software nodes registered by ipu_bridge_init() are deliberately
> never unregistered, and the intended design is for a rebind to reuse
> the already registered nodes. That reuse path however only exists for
> the case where the IPU device kept its secondary fwnode link, which the
> fwnode graph check at the top of ipu_bridge_init() detects: then the
> function returns early. When the link is gone, ipu_bridge_init()
> unconditionally registers the IPU HID software node again, which fails
> with -EEXIST on the sysfs name (the node from the previous bind is
> still registered) and the IPU driver fails to probe.
>
> That is exactly what happens when the IPU PCI device is removed and
> re-scanned: device_del() unsets the ACPI companion, and
> set_primary_fwnode(dev, NULL) then clears the ACPI fwnode's ->secondary
> pointer, so the fwnode graph check on the next probe finds no endpoints
> and falls through to registration. Observed on a Surface Pro 7+ (IPU6):
>
> echo 1 > /sys/bus/pci/devices/0000:00:05.0/remove
> modprobe -r intel_ipu6_isys intel_ipu6 # ipu-bridge unloads too
> echo 1 > /sys/bus/pci/rescan
> modprobe intel_ipu6
>
> sysfs: cannot create duplicate filename '/kernel/software_nodes/INT343E'
> intel-ipu6 0000:00:05.0: Failed to register the IPU HID node
> intel-ipu6: probe of 0000:00:05.0 failed with error -17
>
> after which the cameras are unusable until reboot.
>
> Add the missing reuse path: if the IPU software node is already
> registered, look it up with software_node_find_by_name(), point the
> device's secondary fwnode at it and return success. Restoring the IPU's
> secondary fwnode is all a rebind needs: the sensors' ACPI fwnodes still
> carry their secondary fwnode pointers from the first bind (the sensor
> devices are not removed by an IPU unbind, so nothing clears those), and
> the IVSC and VCM links likewise live on devices that survive an IPU
> rebind. The previous commit made the registered nodes self-contained in
I'd refer to the patch by a name, but I don't think you really need that
reference here.
> the never freed bridge allocation, so their properties are still valid
> here. The IVSC readiness check is intentionally skipped on this path,
> as the IVSC links were already established by the first bind.
I'd say this is a bit too elaborate for a commit message. Please shorten
it.
>
> software_node_find_by_name() takes a reference on the node it returns;
> drop it right away since the node is kept alive by its never dropped
> registration, matching the reference handling of the initial-bind path.
>
> Developed with the assistance of an AI tool (Claude)
Please use Assisted-by: tag, see
Documentation/process/coding-assistants.rst .
>
> Fixes: 803abec64ef9 ("media: ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver")
> Signed-off-by: D. Manresa <dmanresa@gmail.com>
> ---
> drivers/media/pci/intel/ipu-bridge.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
> index 4de42ed..6fa1c3c 100644
> --- a/drivers/media/pci/intel/ipu-bridge.c
> +++ b/drivers/media/pci/intel/ipu-bridge.c
> @@ -930,6 +930,7 @@ static DEFINE_MUTEX(ipu_bridge_mutex);
> int ipu_bridge_init(struct device *dev,
> ipu_parse_sensor_fwnode_t parse_sensor_fwnode)
> {
> + const struct software_node *ipu_node;
> struct fwnode_handle *fwnode;
> struct ipu_bridge *bridge;
> unsigned int i;
> @@ -940,6 +941,29 @@ int ipu_bridge_init(struct device *dev,
> if (!ipu_bridge_check_fwnode_graph(dev_fwnode(dev)))
> return 0;
>
> + /*
> + * The software nodes registered by a previous ipu_bridge_init() call
> + * are deliberately kept registered when the module is unloaded, and
> + * the sensors' ACPI fwnodes still have them as their secondary
> + * fwnodes. If the IPU software node is already registered this is a
> + * rebind, e.g. after the PCI device was removed and re-scanned,
> + * which drops the IPU's secondary fwnode link. Registering the nodes
> + * again would fail with -EEXIST, so instead reuse them and just
> + * restore the IPU's secondary fwnode link.
> + */
> + ipu_node = software_node_find_by_name(NULL, IPU_HID);
> + if (ipu_node) {
> + fwnode = software_node_fwnode(ipu_node);
> + set_secondary_fwnode(dev, fwnode);
> + /*
> + * The node stays registered, it does not need the reference
> + * software_node_find_by_name() took to stay alive.
> + */
> + fwnode_handle_put(fwnode);
> + dev_info(dev, "Reusing the previously registered software nodes\n");
I think dev_dbg() should suffice here.
> + return 0;
> + }
> +
> if (!ipu_bridge_ivsc_is_ready())
> return dev_err_probe(dev, -EPROBE_DEFER,
> "waiting for IVSC to become ready\n");
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind
2026-08-31 9:42 ` [PATCH 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind D. Manresa
2026-08-31 9:42 ` [PATCH 1/2] media: ipu-bridge: don't reference the module image from software nodes D. Manresa
2026-08-31 9:42 ` [PATCH 2/2] media: ipu-bridge: reuse the software nodes on rebind D. Manresa
@ 2026-08-31 14:03 ` D. Manresa
2026-08-31 14:03 ` [PATCH v2 1/2] media: ipu-bridge: don't reference the module image from software nodes D. Manresa
2026-08-31 14:03 ` [PATCH v2 2/2] media: ipu-bridge: reuse the software nodes on rebind D. Manresa
2 siblings, 2 replies; 17+ messages in thread
From: D. Manresa @ 2026-08-31 14:03 UTC (permalink / raw)
To: Sakari Ailus, Hans de Goede, Daniel Scally
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, D . Manresa
Hello,
v2 of the ipu-bridge rebind series, addressing Sakari's review of v2/2:
- shortened the 2/2 commit message (dropped the cross-reference to 1/2
and the IVSC/VCM elaboration);
- dev_info() -> dev_dbg() on the reuse path;
- replaced the free-text AI note with the Assisted-by: tag per
Documentation/process/coding-assistants.rst, in both patches.
No code changes other than the log level. The v1 testing stands: on a
Surface Pro 7+ (IPU6, OV5693 + OV8865 + OV7251) the PCI remove ->
module unload -> rescan -> modprobe sequence, fatal today (-EEXIST),
completes cleanly with the series - twice in a row - with all three
cameras streaming after each rebind. (The same sequence with all sensor
sub-device drivers unbound first fails identically without the series,
as reported in the parent thread.)
Thanks,
D. Manresa
D. Manresa (2):
media: ipu-bridge: don't reference the module image from software
nodes
media: ipu-bridge: reuse the software nodes on rebind
drivers/media/pci/intel/ipu-bridge.c | 38 ++++++++++++++++++++++++++---
include/media/ipu-bridge.h | 9 +++++++++
2 files changed, 44 insertions(+), 3 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/2] media: ipu-bridge: don't reference the module image from software nodes
2026-08-31 14:03 ` [PATCH v2 0/2] media: ipu-bridge: survive module unload and " D. Manresa
@ 2026-08-31 14:03 ` D. Manresa
2026-08-31 14:03 ` [PATCH v2 2/2] media: ipu-bridge: reuse the software nodes on rebind D. Manresa
1 sibling, 0 replies; 17+ messages in thread
From: D. Manresa @ 2026-08-31 14:03 UTC (permalink / raw)
To: Sakari Ailus, Hans de Goede, Daniel Scally
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, D . Manresa
The software nodes registered by ipu_bridge_init() are deliberately
never unregistered: sensor drivers and the fwnode graph keep references
to them, so they are left registered when the ipu-bridge module is
unloaded and a later rebind is intended to reuse the already registered
nodes.
For that to work, nothing reachable from the registered nodes may point
into the ipu-bridge module image. Most of the data already lives in the
dedicated, never freed, struct ipu_bridge allocation: the property name
strings in struct ipu_property_names are character arrays copied into
the per-sensor struct, the node name strings are likewise character
arrays inside the struct, and the data-lanes array is a struct
ipu_bridge member precisely so that "it survives if the module is
unloaded along with the rest of the struct".
Two references into the module image remain, though:
1. The values of the "link-frequencies" endpoint property point at
cfg->link_freqs inside the const ipu_supported_sensors[] table in
module rodata.
2. The name of the "lens-focus" device property is a string literal in
module rodata.
Both dangle as soon as the module is unloaded, while the properties
that carry them stay registered and readable. In practice, after
unloading and reloading the IPU modules on a Surface Pro 7+ (IPU6,
ov8865 + ov5693 + ov7251), re-probing sensor drivers read poisoned
link-frequencies from the surviving nodes and fail to probe:
ov8865: failed to find 360000000 clk rate in endpoint link-frequencies
ov5693: supported link freq 419200000 not found
where 419200000/360000000 are exactly the values the bridge had
originally published for those sensors, i.e. the properties no longer
return their original contents. Depending on what happens to the freed
module mapping, reading the properties can also fault. Similarly, a VCM
lookup through the "lens-focus" reference can no longer match (or
faults) once the property's name pointer is dangling.
Copy the link frequencies and the "lens-focus" property name into
struct ipu_bridge, next to the data-lanes array kept there for the same
reason, and make the registered properties point at those copies, so
the nodes survive module unload intact. These were the only remaining
references from the registered nodes into the module image (the
sensor->vcm_type pointer into ipu_vcm_types[] is only dereferenced
during ipu_bridge_init() itself and is not reachable from the nodes).
Assisted-by: LLM
Fixes: 803abec64ef9 ("media: ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver")
Fixes: 68b9bcc8a534 ("media: ipu3-cio2: Add support for instantiating i2c-clients for VCMs")
Signed-off-by: D. Manresa <dmanresa@gmail.com>
---
drivers/media/pci/intel/ipu-bridge.c | 14 +++++++++++---
include/media/ipu-bridge.h | 9 +++++++++
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 1bb3a3e..4de42ed 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -503,7 +503,8 @@ static void ipu_bridge_create_fwnode_properties(
sensor->vcm_ref[0] =
SOFTWARE_NODE_REFERENCE(&sensor->swnodes[SWNODE_VCM]);
sensor->dev_properties[3] =
- PROPERTY_ENTRY_REF_ARRAY("lens-focus", sensor->vcm_ref);
+ PROPERTY_ENTRY_REF_ARRAY(bridge->lens_focus,
+ sensor->vcm_ref);
}
sensor->ep_properties[0] = PROPERTY_ENTRY_U32(
@@ -516,11 +517,17 @@ static void ipu_bridge_create_fwnode_properties(
sensor->prop_names.remote_endpoint,
sensor->local_ref);
- if (cfg->nr_link_freqs > 0)
+ if (cfg->nr_link_freqs > 0) {
+ u64 *link_freqs = bridge->link_freqs[sensor - bridge->sensors];
+
+ memcpy(link_freqs, cfg->link_freqs,
+ cfg->nr_link_freqs * sizeof(*link_freqs));
+
sensor->ep_properties[3] = PROPERTY_ENTRY_U64_ARRAY_LEN(
sensor->prop_names.link_frequencies,
- cfg->link_freqs,
+ link_freqs,
cfg->nr_link_freqs);
+ }
sensor->ipu_properties[0] = PROPERTY_ENTRY_U32_ARRAY_LEN(
sensor->prop_names.data_lanes,
@@ -943,6 +950,7 @@ int ipu_bridge_init(struct device *dev,
strscpy(bridge->ipu_node_name, IPU_HID,
sizeof(bridge->ipu_node_name));
+ strscpy(bridge->lens_focus, "lens-focus", sizeof(bridge->lens_focus));
bridge->ipu_hid_node.name = bridge->ipu_node_name;
bridge->dev = dev;
bridge->parse_sensor_fwnode = parse_sensor_fwnode;
diff --git a/include/media/ipu-bridge.h b/include/media/ipu-bridge.h
index 16fac76..4e91ec3 100644
--- a/include/media/ipu-bridge.h
+++ b/include/media/ipu-bridge.h
@@ -164,6 +164,15 @@ struct ipu_bridge {
char ipu_node_name[ACPI_ID_LEN];
struct software_node ipu_hid_node;
u32 data_lanes[4];
+ /*
+ * The software nodes registered by the bridge are deliberately never
+ * unregistered (see ipu_bridge_init()), so every string and array
+ * they reference must live in this never freed struct rather than in
+ * the module image, so that the nodes stay intact if the module is
+ * unloaded.
+ */
+ char lens_focus[sizeof("lens-focus")];
+ u64 link_freqs[IPU_MAX_PORTS][MAX_NUM_LINK_FREQS];
unsigned int n_sensors;
struct ipu_sensor sensors[IPU_MAX_PORTS];
};
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 2/2] media: ipu-bridge: reuse the software nodes on rebind
2026-08-31 14:03 ` [PATCH v2 0/2] media: ipu-bridge: survive module unload and " D. Manresa
2026-08-31 14:03 ` [PATCH v2 1/2] media: ipu-bridge: don't reference the module image from software nodes D. Manresa
@ 2026-08-31 14:03 ` D. Manresa
2026-09-01 10:47 ` Sakari Ailus
1 sibling, 1 reply; 17+ messages in thread
From: D. Manresa @ 2026-08-31 14:03 UTC (permalink / raw)
To: Sakari Ailus, Hans de Goede, Daniel Scally
Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, D . Manresa
The software nodes registered by ipu_bridge_init() are deliberately
never unregistered, and the intended design is for a rebind to reuse
the already registered nodes. That reuse path however only exists for
the case where the IPU device kept its secondary fwnode link, which the
fwnode graph check at the top of ipu_bridge_init() detects: then the
function returns early. When the link is gone, ipu_bridge_init()
unconditionally registers the IPU HID software node again, which fails
with -EEXIST on the sysfs name (the node from the previous bind is
still registered) and the IPU driver fails to probe.
That is exactly what happens when the IPU PCI device is removed and
re-scanned: device_del() unsets the ACPI companion, and
set_primary_fwnode(dev, NULL) then clears the ACPI fwnode's ->secondary
pointer, so the fwnode graph check on the next probe finds no endpoints
and falls through to registration. Observed on a Surface Pro 7+ (IPU6):
echo 1 > /sys/bus/pci/devices/0000:00:05.0/remove
modprobe -r intel_ipu6_isys intel_ipu6 # ipu-bridge unloads too
echo 1 > /sys/bus/pci/rescan
modprobe intel_ipu6
sysfs: cannot create duplicate filename '/kernel/software_nodes/INT343E'
intel-ipu6 0000:00:05.0: Failed to register the IPU HID node
intel-ipu6: probe of 0000:00:05.0 failed with error -17
after which the cameras are unusable until reboot.
Add the missing reuse path: if the IPU software node is already
registered, look it up with software_node_find_by_name(), point the
device's secondary fwnode at it and return success. That is all a
rebind needs: the sensor, IVSC and VCM links live on devices that
survive an IPU unbind, so nothing has cleared those.
software_node_find_by_name() takes a reference on the node it returns;
drop it right away since the node is kept alive by its never dropped
registration.
Assisted-by: LLM
Fixes: 803abec64ef9 ("media: ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver")
Signed-off-by: D. Manresa <dmanresa@gmail.com>
---
drivers/media/pci/intel/ipu-bridge.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 4de42ed..6fa1c3c 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -930,6 +930,7 @@ static DEFINE_MUTEX(ipu_bridge_mutex);
int ipu_bridge_init(struct device *dev,
ipu_parse_sensor_fwnode_t parse_sensor_fwnode)
{
+ const struct software_node *ipu_node;
struct fwnode_handle *fwnode;
struct ipu_bridge *bridge;
unsigned int i;
@@ -940,6 +941,29 @@ int ipu_bridge_init(struct device *dev,
if (!ipu_bridge_check_fwnode_graph(dev_fwnode(dev)))
return 0;
+ /*
+ * The software nodes registered by a previous ipu_bridge_init() call
+ * are deliberately kept registered when the module is unloaded, and
+ * the sensors' ACPI fwnodes still have them as their secondary
+ * fwnodes. If the IPU software node is already registered this is a
+ * rebind, e.g. after the PCI device was removed and re-scanned,
+ * which drops the IPU's secondary fwnode link. Registering the nodes
+ * again would fail with -EEXIST, so instead reuse them and just
+ * restore the IPU's secondary fwnode link.
+ */
+ ipu_node = software_node_find_by_name(NULL, IPU_HID);
+ if (ipu_node) {
+ fwnode = software_node_fwnode(ipu_node);
+ set_secondary_fwnode(dev, fwnode);
+ /*
+ * The node stays registered, it does not need the reference
+ * software_node_find_by_name() took to stay alive.
+ */
+ fwnode_handle_put(fwnode);
+ dev_dbg(dev, "Reusing the previously registered software nodes\n");
+ return 0;
+ }
+
if (!ipu_bridge_ivsc_is_ready())
return dev_err_probe(dev, -EPROBE_DEFER,
"waiting for IVSC to become ready\n");
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] media: ipu-bridge: reuse the software nodes on rebind
2026-08-31 14:03 ` [PATCH v2 2/2] media: ipu-bridge: reuse the software nodes on rebind D. Manresa
@ 2026-09-01 10:47 ` Sakari Ailus
2026-09-01 11:25 ` D. Manresa
0 siblings, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2026-09-01 10:47 UTC (permalink / raw)
To: D. Manresa
Cc: Hans de Goede, Daniel Scally, Mauro Carvalho Chehab, linux-media,
linux-kernel
Hi D.,
On Mon, Aug 31, 2026 at 04:03:03PM +0200, D. Manresa wrote:
> The software nodes registered by ipu_bridge_init() are deliberately
> never unregistered, and the intended design is for a rebind to reuse
> the already registered nodes. That reuse path however only exists for
> the case where the IPU device kept its secondary fwnode link, which the
> fwnode graph check at the top of ipu_bridge_init() detects: then the
> function returns early. When the link is gone, ipu_bridge_init()
> unconditionally registers the IPU HID software node again, which fails
> with -EEXIST on the sysfs name (the node from the previous bind is
> still registered) and the IPU driver fails to probe.
>
> That is exactly what happens when the IPU PCI device is removed and
> re-scanned: device_del() unsets the ACPI companion, and
> set_primary_fwnode(dev, NULL) then clears the ACPI fwnode's ->secondary
> pointer, so the fwnode graph check on the next probe finds no endpoints
> and falls through to registration. Observed on a Surface Pro 7+ (IPU6):
>
> echo 1 > /sys/bus/pci/devices/0000:00:05.0/remove
> modprobe -r intel_ipu6_isys intel_ipu6 # ipu-bridge unloads too
> echo 1 > /sys/bus/pci/rescan
> modprobe intel_ipu6
>
> sysfs: cannot create duplicate filename '/kernel/software_nodes/INT343E'
> intel-ipu6 0000:00:05.0: Failed to register the IPU HID node
> intel-ipu6: probe of 0000:00:05.0 failed with error -17
>
> after which the cameras are unusable until reboot.
>
> Add the missing reuse path: if the IPU software node is already
> registered, look it up with software_node_find_by_name(), point the
> device's secondary fwnode at it and return success. That is all a
> rebind needs: the sensor, IVSC and VCM links live on devices that
> survive an IPU unbind, so nothing has cleared those.
>
> software_node_find_by_name() takes a reference on the node it returns;
> drop it right away since the node is kept alive by its never dropped
> registration.
>
> Assisted-by: LLM
Which one?
There's also an extra newline here. (No need to resend if you just provide
the info.)
>
> Fixes: 803abec64ef9 ("media: ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver")
> Signed-off-by: D. Manresa <dmanresa@gmail.com>
> ---
> drivers/media/pci/intel/ipu-bridge.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
> index 4de42ed..6fa1c3c 100644
> --- a/drivers/media/pci/intel/ipu-bridge.c
> +++ b/drivers/media/pci/intel/ipu-bridge.c
> @@ -930,6 +930,7 @@ static DEFINE_MUTEX(ipu_bridge_mutex);
> int ipu_bridge_init(struct device *dev,
> ipu_parse_sensor_fwnode_t parse_sensor_fwnode)
> {
> + const struct software_node *ipu_node;
> struct fwnode_handle *fwnode;
> struct ipu_bridge *bridge;
> unsigned int i;
> @@ -940,6 +941,29 @@ int ipu_bridge_init(struct device *dev,
> if (!ipu_bridge_check_fwnode_graph(dev_fwnode(dev)))
> return 0;
>
> + /*
> + * The software nodes registered by a previous ipu_bridge_init() call
> + * are deliberately kept registered when the module is unloaded, and
> + * the sensors' ACPI fwnodes still have them as their secondary
> + * fwnodes. If the IPU software node is already registered this is a
> + * rebind, e.g. after the PCI device was removed and re-scanned,
> + * which drops the IPU's secondary fwnode link. Registering the nodes
> + * again would fail with -EEXIST, so instead reuse them and just
> + * restore the IPU's secondary fwnode link.
> + */
> + ipu_node = software_node_find_by_name(NULL, IPU_HID);
> + if (ipu_node) {
> + fwnode = software_node_fwnode(ipu_node);
> + set_secondary_fwnode(dev, fwnode);
> + /*
> + * The node stays registered, it does not need the reference
> + * software_node_find_by_name() took to stay alive.
> + */
> + fwnode_handle_put(fwnode);
> + dev_dbg(dev, "Reusing the previously registered software nodes\n");
> + return 0;
> + }
> +
> if (!ipu_bridge_ivsc_is_ready())
> return dev_err_probe(dev, -EPROBE_DEFER,
> "waiting for IVSC to become ready\n");
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] media: ipu-bridge: reuse the software nodes on rebind
2026-09-01 10:47 ` Sakari Ailus
@ 2026-09-01 11:25 ` D. Manresa
0 siblings, 0 replies; 17+ messages in thread
From: D. Manresa @ 2026-09-01 11:25 UTC (permalink / raw)
To: Sakari Ailus
Cc: Hans de Goede, Daniel Scally, Mauro Carvalho Chehab, linux-media,
linux-kernel, D . Manresa
On Mon, 1 Sep 2026, Sakari Ailus wrote:
> > Assisted-by: LLM
>
> Which one?
Anthropic Claude (model: Fable 5), for both patches in the series - it did
the code, the commit messages and the failure analysis, directed and
verified on hardware by me. So:
Assisted-by: LLM (Anthropic Claude, Fable 5)
if that form works for you - coding-assistants.rst only shows the generic
"Assisted-by: LLM [tools]" shape, so happy to adjust to whatever spelling
you prefer.
> There's also an extra newline here. (No need to resend if you just provide
> the info.)
An editing artifact - the trailer block should be contiguous, no blank line
between Assisted-by and Fixes. If you can fix both up when applying, great;
otherwise say the word and I will send a v3 with the corrected trailers.
Thanks,
D. Manresa <dmanresa@gmail.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-09-01 11:25 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-27 23:26 ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties D. Manresa
2026-08-28 15:33 ` Sakari Ailus
2026-08-28 20:54 ` D. Manresa
2026-08-30 12:40 ` johannes.goede
2026-08-31 8:02 ` Sakari Ailus
2026-08-31 10:23 ` D. Manresa
2026-08-31 11:36 ` Sakari Ailus
2026-08-31 9:03 ` Sakari Ailus
2026-08-31 9:42 ` [PATCH 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind D. Manresa
2026-08-31 9:42 ` [PATCH 1/2] media: ipu-bridge: don't reference the module image from software nodes D. Manresa
2026-08-31 9:42 ` [PATCH 2/2] media: ipu-bridge: reuse the software nodes on rebind D. Manresa
2026-08-31 12:11 ` Sakari Ailus
2026-08-31 14:03 ` [PATCH v2 0/2] media: ipu-bridge: survive module unload and " D. Manresa
2026-08-31 14:03 ` [PATCH v2 1/2] media: ipu-bridge: don't reference the module image from software nodes D. Manresa
2026-08-31 14:03 ` [PATCH v2 2/2] media: ipu-bridge: reuse the software nodes on rebind D. Manresa
2026-09-01 10:47 ` Sakari Ailus
2026-09-01 11:25 ` D. Manresa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox