diff mbox series

[2/2] media: ov2680: Report success on link-frequency match

Message ID 20240328051320.2428125-2-festevam@gmail.com
State New
Headers show
Series [1/2] media: ov2680: Allow probing if link-frequencies is absent | expand

Commit Message

Fabio Estevam March 28, 2024, 5:13 a.m. UTC 
From: Fabio Estevam <festevam@denx.de>

When passing the correct 'link-frequencies' in the DT, the
driver should report success on the match case:

port {
	ov2680_to_mipi: endpoint {
		remote-endpoint = <&mipi_from_sensor>;
		clock-lanes = <0>;
		data-lanes = <1>;
		link-frequencies = /bits/ 64 <330000000>;
	};
};

However, this does not happen and the probe fails like this:

ov2680 1-0036: probe with driver ov2680 failed with error -22

Fix it by returning success upon link-frequency match.

Also tested passing a wrong link-frequencies value in th DT and
confirmed that the driver correctly rejects it.

Fixes: 63b0cd30b78e ("media: ov2680: Add bus-cfg / endpoint property verification")
Signed-off-by: Fabio Estevam <festevam@denx.de>
---
 drivers/media/i2c/ov2680.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Fabio Estevam March 28, 2024, 2:26 p.m. UTC | #1 
Hi Hans,

[Adding DT folks]

On Thu, Mar 28, 2024 at 8:27 AM Hans de Goede <hdegoede@redhat.com> wrote:

> I think that what is necessary for your case with fixed dts file is:
>
> diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
> index bcd031882a37..5c789b5a4bfb 100644
> --- a/drivers/media/i2c/ov2680.c
> +++ b/drivers/media/i2c/ov2680.c
> @@ -1179,6 +1179,8 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor)
>                 goto out_free_bus_cfg;
>         }
>
> +       ret = 0;
> +
>  out_free_bus_cfg:
>         v4l2_fwnode_endpoint_free(&bus_cfg);
>         return ret;
>
> and that then replaces both your patches, can you give this a try ?

This works fine if I pass link-frequencies in the dts, thanks.

--- a/arch/arm/boot/dts/nxp/imx/imx7s-warp.dts
+++ b/arch/arm/boot/dts/nxp/imx/imx7s-warp.dts
@@ -210,6 +210,7 @@ ov2680_to_mipi: endpoint {
                                remote-endpoint = <&mipi_from_sensor>;
                                clock-lanes = <0>;
                                data-lanes = <1>;
+                               link-frequencies = /bits/ 64 <340000000>;
                        };
                };
        };

Can we allow the probe to succeed even if 'link frequencies' is absent?

That was my goal on patch 1/2: to keep existing dtb's functional.

Otherwise, the old dtb's without 'link-frequencies' will be broken and I'm not
sure if the DT folks will accept a patch passing link-frequencies to
imx7s-warp.dts
as a fix to be backported to 6.6.

ovti,ov2680.yaml will also need to be changed to include 'link-frequencies' as
a required property.

Thoughts?
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
index f611ce3a749c..37c21749dc14 100644
--- a/drivers/media/i2c/ov2680.c
+++ b/drivers/media/i2c/ov2680.c
@@ -1128,7 +1128,7 @@  static int ov2680_parse_dt(struct ov2680_dev *sensor)
 
 	for (i = 0; i < bus_cfg.nr_of_link_frequencies; i++)
 		if (bus_cfg.link_frequencies[i] == sensor->link_freq[0])
-			break;
+			return 0;
 
 	if (bus_cfg.nr_of_link_frequencies == 0 ||
 	    bus_cfg.nr_of_link_frequencies == i) {