Re: [RFC PATCH 7/8] arm64: dts: ti: k3-am62a7: FPDLink overlays for LI OV2312

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dt: ti: symbol collisions and copy-paste errors in ov2312 overlays
- [Low] dt: ti: incorrect header comment and dead code in ov2312 overlays
--

commit 03b11364fb4760c865618c43b8a6138373b8795b
Author: Jai Luthra <jai.luthra@linux.dev>
Subject: arm64: dts: ti: k3-am62a7: FPDLink overlays for LI OV2312

OV2312 is a raw (4x4 RGB-IR bayer) sensor, the LI-OV2312-FPDLinkIII-110H
module [1] packages it with an FPDLink-III serializer (DS90UB953) for
use with sensor fusion setups using FPDLink-III deserializer boards.

Link: https://www.leopardimaging.com/product/autonomous-camera/ti-fpdlinkiii-cameras/li-ov2312-fpdlinkiii/li-ov2312-fpdlinkiii-110h/ [1]

> diff --git a/arch/arm64/boot/dts/ti/k3-fpdlink-ov2312-0-0.dtso b/arch/arm64/boot/dts/ti/k3-fpdlink-ov2312-0-0.dtso
> new file mode 100644
> index 0000000000000..0829f01a0f903
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-fpdlink-ov2312-0-0.dtso
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * IMX390 FPD-Link 3 Camera Module

[Severity: Low]
Does this description explicitly name the wrong camera module? It looks like
this might be a copy-paste artifact that should be updated to OV2312.

[ ... ]

> +&deserializer_0_ports {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	/* FPDLink RX 0 */
> +	port@0 {
> +		reg = <0>;
> +
> +		ub960_fpd3_1_in: endpoint {

[Severity: Medium]
Does this label use the index 1 despite being for port 0? Also, since the
device tree compiler exports overlay labels to a global symbols node, will
using an identical label across multiple camera overlays cause symbol name
collisions and silent overwrites when loaded simultaneously?

[ ... ]

> +&deserializer_0_links {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	link@0 {
> +		reg = <0>;
> +		i2c-alias = <0x44>;
> +
> +		ti,rx-mode = <3>;
> +
> +		serializer: serializer {

[Severity: Medium]
Is this label too generic? Redefining a generic label like serializer in
every overlay can pollute the global symbol namespace and lead to collisions.

[ ... ]

> +				port@0 {
> +					reg = <0>;
> +					ub953_1_in: endpoint {
> +						data-lanes = <1 2>;
> +						/*clock-noncontinuous;*/

[Severity: Low]
Should this commented-out dead code be removed entirely, or is there a need
to evaluate and enable it?

[ ... ]

> diff --git a/arch/arm64/boot/dts/ti/k3-fpdlink-ov2312-0-1.dtso b/arch/arm64/boot/dts/ti/k3-fpdlink-ov2312-0-1.dtso
> new file mode 100644
> index 0000000000000..06cba8c7397fe
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-fpdlink-ov2312-0-1.dtso
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * IMX390 FPD-Link 3 Camera Module

[Severity: Low]
Is this header comment metadata incorrect here as well?

[ ... ]

> diff --git a/arch/arm64/boot/dts/ti/k3-fpdlink-ov2312-0-2.dtso b/arch/arm64/boot/dts/ti/k3-fpdlink-ov2312-0-2.dtso
> new file mode 100644
> index 0000000000000..7d377dbe5bbfb
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-fpdlink-ov2312-0-2.dtso
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * IMX390 FPD-Link 3 Camera Module
> + * Copyright (c) 2023 Texas Instruments Incorporated - http://www.ti.com/
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +
> +&deserializer_0_ports {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	/* FPDLink RX 2 */
> +	port@2 {
> +		reg = <2>;
> +
> +		ub960_fpd3_1_in: endpoint {

[Severity: Medium]
Is the _1_ suffix incorrectly duplicated for port 2 here? As with the first
overlay, this identical label will clash with the other overlays.

[ ... ]

> diff --git a/arch/arm64/boot/dts/ti/k3-fpdlink-ov2312-0-3.dtso b/arch/arm64/boot/dts/ti/k3-fpdlink-ov2312-0-3.dtso
> new file mode 100644
> index 0000000000000..f74bff5f12b02
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-fpdlink-ov2312-0-3.dtso
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * IMX390 FPD-Link 3 Camera Module
> + * Copyright (c) 2023 Texas Instruments Incorporated - http://www.ti.com/
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +
> +&deserializer_0_ports {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	/* FPDLink RX 3 */
> +	port@3 {
> +		reg = <3>;
> +
> +		ub960_fpd3_1_in: endpoint {

[Severity: Medium]
Is the _1_ suffix incorrectly duplicated for port 3 as well, leading to the
same symbol clash when multiple overlays are loaded?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925133001.2780868-1-r-donadkar@ti.com?part=7