Hi Fabio,
On 3/28/24 6:13 AM, Fabio Estevam wrote:
> From: Fabio Estevam <festevam@xxxxxxx>
>
> 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@xxxxxxx>
> ---
> drivers/media/i2c/ov2680.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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 you need this then that suggests that bus_cfg.nr_of_link_frequencies != 0
otherwise this patch will not make a difference. So that suggests that
patch 1/2 is not necessary ?
And if bus_cfg.nr_of_link_frequencies != 0 and we break then:
>
> if (bus_cfg.nr_of_link_frequencies == 0 ||
> bus_cfg.nr_of_link_frequencies == i) {
This will never be true (neither condition is true) so we will continue
with a clean exit of the function. Except that that clean exit
does "return ret" and I see there may some paths where that is not 0
even though we are doing a clean exit.
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 ?
Regards,
Hans
p.s.
Your early return 0 in this patch also leaks the bus_cfg.
