[PATCH v4 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind

[PATCH v4 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind

[D. Manresa]

Hello,

v4 addresses Sakari's review of v3. Both commit messages are much
shorter, the in-code comments are trimmed, and the data moved in 1/2
now lives in struct ipu_sensor as asked:

- the "lens-focus" name goes into struct ipu_property_names, next to
  clock_noncontinuous, so it is copied per sensor like every other
  property name and ipu_bridge_init() needs no strscpy() for it;
- link_freqs becomes a per-sensor array, which also drops the
  sensor - bridge->sensors indexing v3 needed.

No functional change against v3.

Testing: unchanged from v1/v2, on a Surface Pro 7+ (IPU6, OV5693 +
OV8865 + OV7251) running the equivalent change on 6.19 - the PCI remove
-> module unload -> rescan -> modprobe sequence, fatal today (-EEXIST),
completes cleanly with the series, with all three cameras streaming
after each rebind. The patches as posted are compile-tested only; that
machine is not on a media/next kernel and is leaving my hands, so I
cannot promise a fresh run on it.

Changes since v3:
- 1/2 and 2/2: commit messages cut down;
- 1/2: lens_focus moved into struct ipu_property_names and link_freqs
  into struct ipu_sensor;
- 2/2: reuse-path comment shortened, and the fwnode_handle_put()
  comment now says why the reference is dropped rather than what the
  call does.

Changes since v2:
- rebased onto media/next (1/2 rewritten for the new property
  indexing).

Changes since v1:
- shortened the 2/2 commit message;
- dev_info() -> dev_dbg() on the reuse path;
- Assisted-by: tag per Documentation/process/coding-assistants.rst.

v3: https://lore.kernel.org/linux-media/20261008132614.2456716-1-dmanresa@gmail.com/

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 | 29 +++++++++++++++++++++++++---
 include/media/ipu-bridge.h           |  6 ++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH v4 1/2] media: ipu-bridge: don't reference the module image from software nodes

[D. Manresa]

The software nodes the bridge registers are deliberately never
unregistered, so that a later rebind can reuse them. Nothing reachable
from them may therefore point into the module image, but two properties
still do: the "link-frequencies" values point at cfg->link_freqs in
ipu_supported_sensors[], and the "lens-focus" property name is a string
literal.

Both dangle once the module is unloaded. Re-probing sensor drivers then
read poisoned link frequencies from the surviving nodes and fail:

  ov5693: supported link freq 419200000 not found

Copy both into struct ipu_sensor, where the other property names and
values already live.

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 | 12 +++++++++---
 include/media/ipu-bridge.h           |  6 ++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 3739c4a..aea4699 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -290,6 +290,7 @@ static const struct ipu_property_names prop_names = {
 	.remote_endpoint = "remote-endpoint",
 	.link_frequencies = "link-frequencies",
 	.clock_noncontinuous = "clock-noncontinuous",
+	.lens_focus = "lens-focus",
 };
 
 static const char * const ipu_vcm_types[] = {
@@ -623,7 +624,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(names->lens_focus,
+						 sensor->vcm_ref);
 	}
 
 	sensor->ep_properties[IPU_BRIDGE_NEXT_PROPERTY(i, EP_BUS_TYPE)] =
@@ -636,11 +638,15 @@ static void ipu_bridge_create_fwnode_properties(
 		PROPERTY_ENTRY_REF_ARRAY(names->remote_endpoint,
 					 sensor->local_ref);
 
-	if (cfg->nr_link_freqs > 0)
+	if (cfg->nr_link_freqs > 0) {
+		memcpy(sensor->link_freqs, cfg->link_freqs,
+		       cfg->nr_link_freqs * sizeof(*sensor->link_freqs));
+
 		sensor->ep_properties[IPU_BRIDGE_NEXT_PROPERTY(i, EP_LINK_FREQUENCIES)] =
 			PROPERTY_ENTRY_U64_ARRAY_LEN(names->link_frequencies,
-						     cfg->link_freqs,
+						     sensor->link_freqs,
 						     cfg->nr_link_freqs);
+	}
 
 	if (cfg->flags & IPU_BR_FL_CSI2_CLK_NONCONTINUOUS)
 		sensor->ep_properties[IPU_BRIDGE_NEXT_PROPERTY(i, EP_CLOCK_NONCONTINUOUS)] =
diff --git a/include/media/ipu-bridge.h b/include/media/ipu-bridge.h
index 3ef94c2..7c89fb6 100644
--- a/include/media/ipu-bridge.h
+++ b/include/media/ipu-bridge.h
@@ -136,6 +136,7 @@ struct ipu_property_names {
 	char remote_endpoint[16];
 	char link_frequencies[17];
 	char clock_noncontinuous[20];
+	char lens_focus[11];
 };
 
 struct ipu_node_names {
@@ -178,6 +179,11 @@ struct ipu_sensor {
 	const char *vcm_type;
 
 	struct ipu_property_names prop_names;
+	/*
+	 * The registered software nodes outlive the module, so the property
+	 * values they point at must not live in it.
+	 */
+	u64 link_freqs[MAX_NUM_LINK_FREQS];
 	struct property_entry ep_properties[IPU_BRIDGE_EP_NUM_ENTRIES];
 	struct property_entry dev_properties[5];
 	struct property_entry ipu_properties[3];
-- 
2.43.0

[PATCH v4 2/2] media: ipu-bridge: reuse the software nodes on rebind

[D. Manresa]

The software nodes are deliberately never unregistered so that a rebind
can reuse them, but ipu_bridge_init() only takes that path when the IPU
kept its secondary fwnode link. Removing and re-scanning the IPU PCI
device clears that link, so the next probe tries to register the IPU HID
node again and fails:

  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.

Look the registered node up instead and restore the device's secondary
fwnode.

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 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index aea4699..a77ea60 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -1098,6 +1098,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;
@@ -1108,6 +1109,22 @@ int ipu_bridge_init(struct device *dev,
 	if (!ipu_bridge_check_fwnode_graph(dev_fwnode(dev)))
 		return 0;
 
+	/*
+	 * The nodes registered by a previous bind are still there, but the
+	 * PCI remove/rescan cycle dropped the IPU's secondary fwnode link to
+	 * them. Registering them again would fail with -EEXIST, so reuse
+	 * them and restore just that 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 is never unregistered, no reference is needed. */
+		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