Re: [PATCH v3 14/15] media: mt9m114: Return -EPROBE_DEFER if no endpoint is found

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Wed, Dec 24, 2025 at 01:12:28PM +0100, Hans de Goede wrote:
> On 2-Jul-25 02:53, Laurent Pinchart wrote:
> > On Sun, Jun 29, 2025 at 10:56:24PM +0200, Hans de Goede wrote:
> >> With IPU# bridges, endpoints may only be created when the IPU bridge is
> >> initialized. This may happen after the sensor driver's first probe().
> >>
> >> Signed-off-by: Hans de Goede <hansg@kernel.org>
> >> ---
> >>  drivers/media/i2c/mt9m114.c | 11 +++++++----
> >>  1 file changed, 7 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/media/i2c/mt9m114.c b/drivers/media/i2c/mt9m114.c
> >> index ec5e9ce24d1c..5e759a23e6cc 100644
> >> --- a/drivers/media/i2c/mt9m114.c
> >> +++ b/drivers/media/i2c/mt9m114.c
> >> @@ -2448,11 +2448,14 @@ static int mt9m114_parse_dt(struct mt9m114 *sensor)
> >>  	struct fwnode_handle *ep;
> >>  	int ret;
> >>  
> >> +	/*
> >> +	 * Sometimes the fwnode graph is initialized by the bridge driver,
> >> +	 * wait for this.
> >> +	 */
> > 
> > I'm still not thrilled by this, but there's no real alternative for the
> > time being. Still, as Sakari mentioned, the IPU bridge code should at
> > some point be moved to the ACPI framework, so let's record here that the
> > sensor driver should then be updated:
> > 
> > 	/*
> > 	 * On ACPI systems the fwnode graph can be initialized by a bridge
> > 	 * driver, which may not have probed yet. Wait for this.
> > 	 *
> > 	 * TODO: Return an error once bridge driver code will have moved
> > 	 * to the ACPI core.
> > 	 */
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > I wouldn't like to see this being replicated in lots of sensor drivers
> > though.
> 
> I understand. With the new __devm_v4l2_sensor_clk_get() helper we could
> do something like this:
> 
> From f7b13937af5f706bce3eed87c67a8f484f049c6a Mon Sep 17 00:00:00 2001
> From: Hans de Goede <johannes.goede@oss.qualcomm.com>
> Date: Wed, 24 Dec 2025 13:04:09 +0100
> Subject: [PATCH] media: v4l2-common: sensor_clk_get(): Wait for endpoint
>  fwnode to show up
> 
> On ACPI systems the fwnode graph can be initialized by a bridge driver,
> which may not have probed yet.
> 
> Currently all sensor drivers which are used on ACPI platforms need to wait
> for this themselves.
> 
> Add a check for the endpoint fwnode being present to
> the devm_v4l2_sensor_clk_get() helper, this allows sensor drivers to drop
> the check for this themselves as long as they call the helper before doing
> any endpoint fwnode parsing.
> 
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 554c591e1113..b68b5567a508 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -743,11 +743,27 @@ struct clk *__devm_v4l2_sensor_clk_get(struct device *dev, const char *id,
>  {
>  	bool of_node = is_of_node(dev_fwnode(dev));
>  	const char *clk_id __free(kfree) = NULL;
> +	struct fwnode_handle *ep;
>  	struct clk_hw *clk_hw;
>  	struct clk *clk;
>  	u32 rate = clk_rate;
>  	int ret = 0;
>  
> +	/*
> +	 * On ACPI systems the fwnode graph can be initialized by a bridge
> +	 * driver, which may not have probed yet. The bridge driver also sets
> +	 * the clock-frequency property which is used below. Wait for this.
> +	 *
> +	 * TODO: Return an error once bridge driver code will have moved
> +	 * to the ACPI core.
> +	 */
> +	ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
> +	if (!ep)
> +		return dev_err_probe(dev, -EPROBE_DEFER,
> +				     "waiting for fwnode graph endpoint\n");
> +
> +	fwnode_handle_put(ep);
> +
>  	clk = devm_clk_get_optional(dev, id);
>  	if (IS_ERR(clk))
>  		return clk;
> and then in e.g. the mt9m114.c code move the devm_v4l2_sensor_clk_get()
> call to above mt9m114_parse_dt() and then the handling for this can be
> dropped from the mt9m114.c code (and the same for other sensor drivers
> with a similar check).
> 
> The downside of this is that it makes having an endpoint fwnode available
> mandatory for all sensor drivers which use the devm_v4l2_sensor_clk_get()
> helper, but AFAICT having an endpoint fwnode is mandatory for all sensor
> drivers anyways, so this should not be an issue ?

Yes I think that would work, and would avoid copying the hack in
multiple drivers. Sakari, is this OK with you ?

-- 
Regards,

Laurent Pinchart