Re: [PATCH v1 1/1 RESEND] media: i2c: mt9m114: Add get_fwnode_pad operation for IFP

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

Hi Svyatoslav,

Thank you for the patch.

On Thu, Jun 25, 2026 at 11:21:11AM +0300, Svyatoslav Ryhel wrote:
> Currently, the driver's binding exposes only one endpoint, which maps to
> the IFP subdevice's SOURCE pad. This configuration causes failures for
> many devices using this camera because both the DT binding and the
> one-to-one pad mapping logic map the endpoint to the wrong pad.

Could you please explain what failures this causes ? A brief analysis of
the code seems to indicate the patch will only make a difference when
the DT node has multiple endpoints, and will prevent links to be created
for any endpoint but the first one. I don't think that's desirable.

> Fix this
> by implementing the get_fwnode_pad operation for the IFP, which correctly
> matches the endpoint to the corresponding IFP pad.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/media/i2c/mt9m114.c | 44 ++++++++++++++++++++++++++++---------
>  1 file changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/i2c/mt9m114.c b/drivers/media/i2c/mt9m114.c
> index e395e2d14e97..16c2582551d3 100644
> --- a/drivers/media/i2c/mt9m114.c
> +++ b/drivers/media/i2c/mt9m114.c
> @@ -1020,14 +1020,6 @@ static int mt9m114_stop_streaming(struct mt9m114 *sensor)
>  	return ret;
>  }
>  
> -/* -----------------------------------------------------------------------------
> - * Common Subdev Operations
> - */
> -
> -static const struct media_entity_operations mt9m114_entity_ops = {
> -	.link_validate = v4l2_subdev_link_validate,
> -};
> -
>  /* -----------------------------------------------------------------------------
>   * Pixel Array Control Operations
>   */
> @@ -1381,6 +1373,10 @@ static const struct v4l2_subdev_internal_ops mt9m114_pa_internal_ops = {
>  	.init_state = mt9m114_pa_init_state,
>  };
>  
> +static const struct media_entity_operations mt9m114_pa_entity_ops = {
> +	.link_validate = v4l2_subdev_link_validate,
> +};
> +
>  static int mt9m114_pa_init(struct mt9m114 *sensor)
>  {
>  	struct v4l2_ctrl_handler *hdl = &sensor->pa.hdl;
> @@ -1403,7 +1399,7 @@ static int mt9m114_pa_init(struct mt9m114 *sensor)
>  
>  	/* Initialize the media entity. */
>  	sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
> -	sd->entity.ops = &mt9m114_entity_ops;
> +	sd->entity.ops = &mt9m114_pa_entity_ops;
>  	pads[0].flags = MEDIA_PAD_FL_SOURCE;
>  	ret = media_entity_pads_init(&sd->entity, 1, pads);
>  	if (ret < 0)
> @@ -2092,6 +2088,29 @@ static int mt9m114_ifp_registered(struct v4l2_subdev *sd)
>  	return 0;
>  }
>  
> +/*
> + * The IFP has only one fwnode endpoint, which corresponds to the pad
> + * linked to the PA (PA SINK), while it should be the SOURCE for the
> + * next media device in the pipe.
> + */
> +static int mt9m114_ifp_get_fwnode_pad(struct media_entity *entity,
> +				      struct fwnode_endpoint *endpoint)
> +{
> +	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> +	struct mt9m114 *sensor = ifp_to_mt9m114(sd);
> +	struct fwnode_handle *ifp_port = dev_fwnode(&sensor->client->dev);

This is not a port fwnode. I'd name the variable just fwnode.

> +	struct fwnode_handle *ifp_ep;
> +	int ret;
> +
> +	ifp_ep = fwnode_graph_get_next_endpoint(ifp_port, NULL);
> +
> +	ret = endpoint->local_fwnode == ifp_ep ? 1 : -ENXIO;
> +
> +	fwnode_handle_put(ifp_ep);
> +
> +	return ret;

Let's use the cleanup API:

#include <cleanup.h>

...

	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
	struct mt9m114 *sensor = ifp_to_mt9m114(sd);
	struct fwnode_handle *fwnode = dev_fwnode(&sensor->client->dev);
	struct fwnode_handle *ifp_ep __free(fwnode_handle) =
		fwnode_graph_get_next_endpoint(fwnode, NULL);

	return endpoint->local_fwnode == ifp_ep ? 1 : -ENXIO;

> +}
> +
>  static const struct v4l2_subdev_video_ops mt9m114_ifp_video_ops = {
>  	.s_stream = mt9m114_ifp_s_stream,
>  };
> @@ -2119,6 +2138,11 @@ static const struct v4l2_subdev_internal_ops mt9m114_ifp_internal_ops = {
>  	.unregistered = mt9m114_ifp_unregistered,
>  };
>  
> +static const struct media_entity_operations mt9m114_ifp_entity_ops = {
> +	.link_validate = v4l2_subdev_link_validate,
> +	.get_fwnode_pad = mt9m114_ifp_get_fwnode_pad,
> +};
> +
>  static int mt9m114_ifp_init(struct mt9m114 *sensor)
>  {
>  	struct v4l2_subdev *sd = &sensor->ifp.sd;
> @@ -2136,7 +2160,7 @@ static int mt9m114_ifp_init(struct mt9m114 *sensor)
>  
>  	/* Initialize the media entity. */
>  	sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_ISP;
> -	sd->entity.ops = &mt9m114_entity_ops;
> +	sd->entity.ops = &mt9m114_ifp_entity_ops;
>  	pads[0].flags = MEDIA_PAD_FL_SINK;
>  	pads[1].flags = MEDIA_PAD_FL_SOURCE;
>  	ret = media_entity_pads_init(&sd->entity, 2, pads);

-- 
Regards,

Laurent Pinchart