[PATCH] staging: media: tegra-video: vi: fix probe failure on skipped last port

[PATCH] staging: media: tegra-video: vi: fix probe failure on skipped last port

[Hao-Qun Huang]

tegra_vi_channels_alloc() iterates over port nodes and skips those
whose reg property cannot be read or whose remote endpoint fails
v4l2_fwnode_endpoint_parse(), leaving the negative result of the
failed call in ret. If that happens on the last port node, the loop
ends with ret still negative and tegra_vi_init() fails the whole VI
probe.

The same defective port earlier in the ports node is skipped silently,
so probing succeeds or fails depending on the order of the port nodes.
The CSI equivalent, tegra_csi_channels_alloc(), returns 0
unconditionally after its loop and does not have this problem.

Use a separate variable for the per-port checks so that only fatal
errors end up in ret.

Fixes: 1ebaeb09830f ("media: tegra-video: Add support for external sensor capture")
Fixes: 2ac4035a78c9 ("media: tegra-video: Add support for x8 captures with gang ports")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Hao-Qun Huang <alvinhuang0603@gmail.com>
---
diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c
index 456134a9e8cf..f461e117305e 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -1257,6 +1257,7 @@ static int tegra_vi_channels_alloc(struct tegra_vi *vi)
 	struct device_node *parent;
 	struct v4l2_fwnode_endpoint v4l2_ep = { .bus_type = 0 };
 	unsigned int lanes;
+	int err;
 	int ret = 0;
 
 	ports = of_get_child_by_name(node, "ports");
@@ -1267,8 +1268,8 @@ static int tegra_vi_channels_alloc(struct tegra_vi *vi)
 		if (!of_node_name_eq(port, "port"))
 			continue;
 
-		ret = of_property_read_u32(port, "reg", &port_num);
-		if (ret < 0)
+		err = of_property_read_u32(port, "reg", &port_num);
+		if (err < 0)
 			continue;
 
 		if (port_num > vi->soc->vi_max_channels) {
@@ -1289,10 +1290,10 @@ static int tegra_vi_channels_alloc(struct tegra_vi *vi)
 
 		ep = of_graph_get_endpoint_by_regs(parent, 0, 0);
 		of_node_put(parent);
-		ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep),
+		err = v4l2_fwnode_endpoint_parse(of_fwnode_handle(ep),
 						 &v4l2_ep);
 		of_node_put(ep);
-		if (ret)
+		if (err)
 			continue;
 
 		lanes = v4l2_ep.bus.mipi_csi2.num_data_lanes;

Re: [PATCH] staging: media: tegra-video: vi: fix probe failure on skipped last port

[Luca Ceresoli]

Hello,

On Tue Jul 7, 2026 at 5:24 PM CEST, Hao-Qun Huang wrote:
> tegra_vi_channels_alloc() iterates over port nodes and skips those
> whose reg property cannot be read or whose remote endpoint fails
> v4l2_fwnode_endpoint_parse(), leaving the negative result of the
> failed call in ret. If that happens on the last port node, the loop
> ends with ret still negative and tegra_vi_init() fails the whole VI
> probe.
>
> The same defective port earlier in the ports node is skipped silently,
> so probing succeeds or fails depending on the order of the port nodes.
> The CSI equivalent, tegra_csi_channels_alloc(), returns 0
> unconditionally after its loop and does not have this problem.
>
> Use a separate variable for the per-port checks so that only fatal
> errors end up in ret.
>
> Fixes: 1ebaeb09830f ("media: tegra-video: Add support for external sensor capture")
> Fixes: 2ac4035a78c9 ("media: tegra-video: Add support for x8 captures with gang ports")
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Hao-Qun Huang <alvinhuang0603@gmail.com>

Do you have a real use case where ths failure happens?

Also, mixing two different return values may solve a bug but makes for more
intricated. I'd rather second a fix based on moving the whole foreach loop
body into a subfunction, and make the foreach loop body as simple as
calling the subfunction and handling returned errors. This would fix the
code making it more readbale.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] staging: media: tegra-video: vi: fix probe failure on skipped last port

[Dan Carpenter]

On Mon, Jul 27, 2026 at 01:10:07PM +0200, Luca Ceresoli wrote:
> Hello,
> 
> On Tue Jul 7, 2026 at 5:24 PM CEST, Hao-Qun Huang wrote:
> > tegra_vi_channels_alloc() iterates over port nodes and skips those
> > whose reg property cannot be read or whose remote endpoint fails
> > v4l2_fwnode_endpoint_parse(), leaving the negative result of the
> > failed call in ret. If that happens on the last port node, the loop
> > ends with ret still negative and tegra_vi_init() fails the whole VI
> > probe.
> >
> > The same defective port earlier in the ports node is skipped silently,
> > so probing succeeds or fails depending on the order of the port nodes.
> > The CSI equivalent, tegra_csi_channels_alloc(), returns 0
> > unconditionally after its loop and does not have this problem.
> >
> > Use a separate variable for the per-port checks so that only fatal
> > errors end up in ret.
> >
> > Fixes: 1ebaeb09830f ("media: tegra-video: Add support for external sensor capture")
> > Fixes: 2ac4035a78c9 ("media: tegra-video: Add support for x8 captures with gang ports")
> > Assisted-by: Claude:claude-fable-5
> > Signed-off-by: Hao-Qun Huang <alvinhuang0603@gmail.com>
> 
> Do you have a real use case where ths failure happens?
> 
> Also, mixing two different return values may solve a bug but makes for more
> intricated. I'd rather second a fix based on moving the whole foreach loop
> body into a subfunction, and make the foreach loop body as simple as
> calling the subfunction and handling returned errors. This would fix the
> code making it more readbale.

My instinct is that the continues should just be exits...  Why are we
accepting broken device trees?

regards,
dan carpenter

Re: [PATCH] staging: media: tegra-video: vi: fix probe failure on skipped last port

[Hao-Qun Huang]

On Mon Jul 27, 2026 at 1:10 PM CEST, Luca Ceresoli wrote:
> Do you have a real use case where ths failure happens?

No, I found it by reading the code.  I went back and checked the in-tree
tegra DTs that have vi ports: lg-x3 (p880), nexus7-grouper and
asus-transformer all link the port through to a CSI channel endpoint with
data-lanes, which parses fine, and asus-tf600t has no remote-endpoint so
it skips on the !parent path, which leaves ret alone.  So no in-tree board
hits this.

> Also, mixing two different return values may solve a bug but makes for more
> intricated. I'd rather second a fix based on moving the whole foreach loop
> body into a subfunction, and make the foreach loop body as simple as
> calling the subfunction and handling returned errors. This would fix the
> code making it more readbale.

Agreed.  This one was already committed to media.git/next on Jul 18
though, so I would rather send the subfunction split as a cleanup on top
than respin it, which also keeps the stable backport small.  I have that
version building here and can post it.

If you would rather have the committed patch dropped and replaced by a v2
with the refactor, say so and I will send that instead.

Thanks,
Hao-Qun

Re: [PATCH] staging: media: tegra-video: vi: fix probe failure on skipped last port

[Luca Ceresoli]

Hello Hao-Qun,

On Mon Jul 27, 2026 at 2:43 PM CEST, Hao-Qun Huang wrote:
> On Mon Jul 27, 2026 at 1:10 PM CEST, Luca Ceresoli wrote:
>> Do you have a real use case where ths failure happens?
>
> No, I found it by reading the code.  I went back and checked the in-tree
> tegra DTs that have vi ports: lg-x3 (p880), nexus7-grouper and
> asus-transformer all link the port through to a CSI channel endpoint with
> data-lanes, which parses fine, and asus-tf600t has no remote-endpoint so
> it skips on the !parent path, which leaves ret alone.  So no in-tree board
> hits this.
>
>> Also, mixing two different return values may solve a bug but makes for more
>> intricated. I'd rather second a fix based on moving the whole foreach loop
>> body into a subfunction, and make the foreach loop body as simple as
>> calling the subfunction and handling returned errors. This would fix the
>> code making it more readbale.
>
> Agreed.  This one was already committed to media.git/next on Jul 18
> though,

Ah, I missed that, I got no e-mail notification about the patch being
applied.

> so I would rather send the subfunction split as a cleanup on top
> than respin it, which also keeps the stable backport small.  I have that
> version building here and can post it.

Yes, I'd appreciate that.

> If you would rather have the committed patch dropped and replaced by a v2
> with the refactor, say so and I will send that instead.

No need, thanks.

Best regards,
Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com