Re: [PATCH 1/5] media: i2c: ov5675: Wait for endpoint

[Hans de Goede <hansg@kernel.org>]

Hi,

Thank you for your patches for this, cool to see someone working
on supporting this!

On 10-Mar-26 13:44, Antti Laakso wrote:
> Defer probe if endpoint is not yet available. And do it before acquiring
> clock, gpio and regulators.
> 
> Signed-off-by: Antti Laakso <antti.laakso@linux.intel.com>
> ---
>  drivers/media/i2c/ov5675.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5675.c b/drivers/media/i2c/ov5675.c
> index ea26df328189..81916fa8c117 100644
> --- a/drivers/media/i2c/ov5675.c
> +++ b/drivers/media/i2c/ov5675.c
> @@ -1181,6 +1181,13 @@ static int ov5675_get_hwcfg(struct ov5675 *ov5675)
>  	if (!fwnode)
>  		return -ENXIO;
>  
> +	ep = fwnode_graph_get_endpoint_by_id(fwnode, 0, 0,
> +					     FWNODE_GRAPH_ENDPOINT_NEXT);
> +	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
> +	fwnode_handle_put(ep);
> +	if (ret)
> +		return ret;

This should be:

		return dev_err_probe(dev, ret, "failed to parse endpoint\n");


> +
>  	ov5675->xvclk = devm_v4l2_sensor_clk_get(dev, NULL);
>  	if (IS_ERR(ov5675->xvclk))

and here and in all other error-exit paths you now need to do:

	if (IS_ERR(ov5675->xvclk)) {
		ret = dev_err_probe(dev, PTR_ERR(ov5675->xvclk), "failed to get xvclk\n");
		goto check_hwcfg_error;
	}

So that the v4l2_fwnode_endpoint_free(&bus_cfg); get called.
Note in this case I also dropped the %pe print because dev_err_probe()
already prints the errno value from its second argument.

You will need to add a similar ret = ...; goto check_hwcfg_error;
to all other error-exits below this point.

Regards,

Hans




>  		return dev_err_probe(dev, PTR_ERR(ov5675->xvclk),

> @@ -1210,15 +1217,6 @@ static int ov5675_get_hwcfg(struct ov5675 *ov5675)
>  	if (ret)
>  		return ret;
>  
> -	ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
> -	if (!ep)
> -		return -ENXIO;
> -
> -	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
> -	fwnode_handle_put(ep);
> -	if (ret)
> -		return ret;
> -
>  	if (bus_cfg.bus.mipi_csi2.num_data_lanes != OV5675_DATA_LANES) {
>  		dev_err(dev, "number of CSI2 data lanes %d is not supported",
>  			bus_cfg.bus.mipi_csi2.num_data_lanes);