mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind
@ 2026-10-08 13:26 D. Manresa
  2026-10-08 13:26 ` [PATCH v3 1/2] media: ipu-bridge: don't reference the module image from software nodes D. Manresa
  2026-10-08 13:26 ` [PATCH v3 2/2] media: ipu-bridge: reuse the software nodes on rebind D. Manresa
  0 siblings, 2 replies; 5+ messages in thread
From: D. Manresa @ 2026-10-08 13:26 UTC (permalink / raw)
  To: Sakari Ailus, Hans de Goede, Daniel Scally
  Cc: Mauro Carvalho Chehab, Fernando Rimoli, linux-media, linux-kernel

Hello,

v3 is a rebase of the v2 series onto media/next; no functional change.

1/2 no longer applied there: the endpoint properties are now written
through the IPU_BRIDGE_NEXT_PROPERTY() indices and a local names
pointer, so the link-frequencies hunk had to be rewritten against that
shape. 2/2 applied unchanged.

While rebasing I noticed that commit a20c843c9bc4 ("media: ipu-bridge:
Keep the clock-noncontinuous property name out of rodata") already
fixed the third reference of this kind, the one I had reported
separately. The two remaining ones this patch moves - the
link-frequencies *values* and the "lens-focus" property *name* - are
disjoint from it, and 1/2 now says so.

Testing: the v1/v2 testing stands, 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, twice in a row, with all three
cameras streaming after each rebind. The rebased files here are
compile-tested only (gcc-13, no new warnings): that machine is not
running a media/next kernel, and it is about to leave my hands, so I
cannot promise a fresh run on it. Other Surface Pro 7+, Pro 8 and Go 4
owners are testing this hardware in
https://github.com/linux-surface/linux-surface/pull/2252 if a retest on
next is wanted before applying.

Changes since v2:
- rebased onto media/next (1/2 rewritten for the new property
  indexing); no functional change;
- 1/2 commit message notes a20c843c9bc4 and what is left after it.

v2: https://lore.kernel.org/linux-media/20260831140304.45940-1-dmanresa@gmail.com/

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.

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] 5+ messages in thread

* [PATCH v3 1/2] media: ipu-bridge: don't reference the module image from software nodes
  2026-10-08 13:26 [PATCH v3 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind D. Manresa
@ 2026-10-08 13:26 ` D. Manresa
  2026-10-09  9:17   ` Sakari Ailus
  2026-10-08 13:26 ` [PATCH v3 2/2] media: ipu-bridge: reuse the software nodes on rebind D. Manresa
  1 sibling, 1 reply; 5+ messages in thread
From: D. Manresa @ 2026-10-08 13:26 UTC (permalink / raw)
  To: Sakari Ailus, Hans de Goede, Daniel Scally
  Cc: Mauro Carvalho Chehab, Fernando Rimoli, linux-media, linux-kernel

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". Commit a20c843c9bc4
("media: ipu-bridge: Keep the clock-noncontinuous property name out of
rodata") moved the "clock-noncontinuous" property name there as well.

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 3739c4a..b9779c3 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -623,7 +623,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[IPU_BRIDGE_NEXT_PROPERTY(i, EP_BUS_TYPE)] =
@@ -636,11 +637,17 @@ 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) {
+		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[IPU_BRIDGE_NEXT_PROPERTY(i, EP_LINK_FREQUENCIES)] =
 			PROPERTY_ENTRY_U64_ARRAY_LEN(names->link_frequencies,
-						     cfg->link_freqs,
+						     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)] =
@@ -1112,6 +1119,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->pci_id = dev_is_pci(dev) ? to_pci_dev(dev)->device : 0;
diff --git a/include/media/ipu-bridge.h b/include/media/ipu-bridge.h
index 3ef94c2..a49f37f 100644
--- a/include/media/ipu-bridge.h
+++ b/include/media/ipu-bridge.h
@@ -203,6 +203,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	[flat|nested] 5+ messages in thread

* [PATCH v3 2/2] media: ipu-bridge: reuse the software nodes on rebind
  2026-10-08 13:26 [PATCH v3 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind D. Manresa
  2026-10-08 13:26 ` [PATCH v3 1/2] media: ipu-bridge: don't reference the module image from software nodes D. Manresa
@ 2026-10-08 13:26 ` D. Manresa
  2026-10-09  9:20   ` Sakari Ailus
  1 sibling, 1 reply; 5+ messages in thread
From: D. Manresa @ 2026-10-08 13:26 UTC (permalink / raw)
  To: Sakari Ailus, Hans de Goede, Daniel Scally
  Cc: Mauro Carvalho Chehab, Fernando Rimoli, linux-media, linux-kernel

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 b9779c3..f0fadc0 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -1099,6 +1099,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;
@@ -1109,6 +1110,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	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/2] media: ipu-bridge: don't reference the module image from software nodes
  2026-10-08 13:26 ` [PATCH v3 1/2] media: ipu-bridge: don't reference the module image from software nodes D. Manresa
@ 2026-10-09  9:17   ` Sakari Ailus
  0 siblings, 0 replies; 5+ messages in thread
From: Sakari Ailus @ 2026-10-09  9:17 UTC (permalink / raw)
  To: D. Manresa
  Cc: Hans de Goede, Daniel Scally, Mauro Carvalho Chehab,
	Fernando Rimoli, linux-media, linux-kernel

Hi D.,

Thanks for the update.

On Thu, Oct 08, 2026 at 03:26:13PM +0200, D. Manresa wrote:
> 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". Commit a20c843c9bc4
> ("media: ipu-bridge: Keep the clock-noncontinuous property name out of
> rodata") moved the "clock-noncontinuous" property name there as well.
> 
> 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).

This commit message is exceedingly long considering what the patch does.
Please remove non-essential information in it.

> 
> 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 3739c4a..b9779c3 100644
> --- a/drivers/media/pci/intel/ipu-bridge.c
> +++ b/drivers/media/pci/intel/ipu-bridge.c
> @@ -623,7 +623,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[IPU_BRIDGE_NEXT_PROPERTY(i, EP_BUS_TYPE)] =
> @@ -636,11 +637,17 @@ 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) {
> +		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[IPU_BRIDGE_NEXT_PROPERTY(i, EP_LINK_FREQUENCIES)] =
>  			PROPERTY_ENTRY_U64_ARRAY_LEN(names->link_frequencies,
> -						     cfg->link_freqs,
> +						     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)] =
> @@ -1112,6 +1119,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->pci_id = dev_is_pci(dev) ? to_pci_dev(dev)->device : 0;
> diff --git a/include/media/ipu-bridge.h b/include/media/ipu-bridge.h
> index 3ef94c2..a49f37f 100644
> --- a/include/media/ipu-bridge.h
> +++ b/include/media/ipu-bridge.h
> @@ -203,6 +203,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];

Can you move this to struct ipu_sensor?

>  	unsigned int n_sensors;
>  	struct ipu_sensor sensors[IPU_MAX_PORTS];
>  };

-- 
Regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 2/2] media: ipu-bridge: reuse the software nodes on rebind
  2026-10-08 13:26 ` [PATCH v3 2/2] media: ipu-bridge: reuse the software nodes on rebind D. Manresa
@ 2026-10-09  9:20   ` Sakari Ailus
  0 siblings, 0 replies; 5+ messages in thread
From: Sakari Ailus @ 2026-10-09  9:20 UTC (permalink / raw)
  To: D. Manresa
  Cc: Hans de Goede, Daniel Scally, Mauro Carvalho Chehab,
	Fernando Rimoli, linux-media, linux-kernel

Hi D.,

On Thu, Oct 08, 2026 at 03:26:14PM +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.

Same for this comment, please clean it up.

> 
> 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 b9779c3..f0fadc0 100644
> --- a/drivers/media/pci/intel/ipu-bridge.c
> +++ b/drivers/media/pci/intel/ipu-bridge.c
> @@ -1099,6 +1099,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;
> @@ -1109,6 +1110,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.
> +	 */

This, too.

> +	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.
> +		 */

Rather than this, it'd be more useful to say why, if you are to add a
comment.

> +		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] 5+ messages in thread

end of thread, other threads:[~2026-10-09  9:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-08 13:26 [PATCH v3 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind D. Manresa
2026-10-08 13:26 ` [PATCH v3 1/2] media: ipu-bridge: don't reference the module image from software nodes D. Manresa
2026-10-09  9:17   ` Sakari Ailus
2026-10-08 13:26 ` [PATCH v3 2/2] media: ipu-bridge: reuse the software nodes on rebind D. Manresa
2026-10-09  9:20   ` Sakari Ailus

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®