[v1,1/2] media: v4l2-subdev: Drop unreacheable warning

Message ID 20231113101718.6098-2-laurent.pinchart+renesas@ideasonboard.com (mailing list archive)
State Accepted
Headers
Series media: v4l2-subdev: Fix link validation issue |

Commit Message

Laurent Pinchart Nov. 13, 2023, 10:17 a.m. UTC
  The v4l2_subdev_link_validate_get_format() function warns if the pad
given as argument belongs to a non-subdev entity. This can't happen, as
the function is called from v4l2_subdev_link_validate() only (indirectly
through v4l2_subdev_link_validate_locked()), and that function ensures
that both ends of the link are subdevs.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 8 --------
 1 file changed, 8 deletions(-)
  

Comments

Sakari Ailus Nov. 13, 2023, 10:42 a.m. UTC | #1
Hi Laurent,

On Mon, Nov 13, 2023 at 12:17:17PM +0200, Laurent Pinchart wrote:
> The v4l2_subdev_link_validate_get_format() function warns if the pad
> given as argument belongs to a non-subdev entity. This can't happen, as
> the function is called from v4l2_subdev_link_validate() only (indirectly
> through v4l2_subdev_link_validate_locked()), and that function ensures
> that both ends of the link are subdevs.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index be86b906c985..67d43206ce32 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -1184,14 +1184,6 @@ v4l2_subdev_link_validate_get_format(struct media_pad *pad, u32 stream,
>  	struct v4l2_subdev *sd;
>  	int ret;
>  
> -	if (!is_media_entity_v4l2_subdev(pad->entity)) {
> -		WARN(pad->entity->function != MEDIA_ENT_F_IO_V4L,
> -		     "Driver bug! Wrong media entity type 0x%08x, entity %s\n",
> -		     pad->entity->function, pad->entity->name);
> -
> -		return -EINVAL;
> -	}
> -
>  	sd = media_entity_to_v4l2_subdev(pad->entity);

It'd be good to check sd is non-NULL here, although pad presumably is
always a pad of a sub-device entity.

>  
>  	fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;
  
Laurent Pinchart Nov. 13, 2023, 10:50 a.m. UTC | #2
On Mon, Nov 13, 2023 at 10:42:16AM +0000, Sakari Ailus wrote:
> Hi Laurent,
> 
> On Mon, Nov 13, 2023 at 12:17:17PM +0200, Laurent Pinchart wrote:
> > The v4l2_subdev_link_validate_get_format() function warns if the pad
> > given as argument belongs to a non-subdev entity. This can't happen, as
> > the function is called from v4l2_subdev_link_validate() only (indirectly
> > through v4l2_subdev_link_validate_locked()), and that function ensures
> > that both ends of the link are subdevs.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-subdev.c | 8 --------
> >  1 file changed, 8 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > index be86b906c985..67d43206ce32 100644
> > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > @@ -1184,14 +1184,6 @@ v4l2_subdev_link_validate_get_format(struct media_pad *pad, u32 stream,
> >  	struct v4l2_subdev *sd;
> >  	int ret;
> >  
> > -	if (!is_media_entity_v4l2_subdev(pad->entity)) {
> > -		WARN(pad->entity->function != MEDIA_ENT_F_IO_V4L,
> > -		     "Driver bug! Wrong media entity type 0x%08x, entity %s\n",
> > -		     pad->entity->function, pad->entity->name);
> > -
> > -		return -EINVAL;
> > -	}
> > -
> >  	sd = media_entity_to_v4l2_subdev(pad->entity);
> 
> It'd be good to check sd is non-NULL here, although pad presumably is
> always a pad of a sub-device entity.

It's guaranteed by the caller. Is there a specific reason you think a
check would be good ?

> >  
> >  	fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;
  
Tomi Valkeinen Nov. 13, 2023, 11:41 a.m. UTC | #3
On 13/11/2023 12:17, Laurent Pinchart wrote:
> The v4l2_subdev_link_validate_get_format() function warns if the pad
> given as argument belongs to a non-subdev entity. This can't happen, as
> the function is called from v4l2_subdev_link_validate() only (indirectly
> through v4l2_subdev_link_validate_locked()), and that function ensures
> that both ends of the link are subdevs.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>   drivers/media/v4l2-core/v4l2-subdev.c | 8 --------
>   1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index be86b906c985..67d43206ce32 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -1184,14 +1184,6 @@ v4l2_subdev_link_validate_get_format(struct media_pad *pad, u32 stream,
>   	struct v4l2_subdev *sd;
>   	int ret;
>   
> -	if (!is_media_entity_v4l2_subdev(pad->entity)) {
> -		WARN(pad->entity->function != MEDIA_ENT_F_IO_V4L,
> -		     "Driver bug! Wrong media entity type 0x%08x, entity %s\n",
> -		     pad->entity->function, pad->entity->name);
> -
> -		return -EINVAL;
> -	}
> -
>   	sd = media_entity_to_v4l2_subdev(pad->entity);
>   
>   	fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;

At some point I also wondered why we need to check for non-subdev 
entity. But, it was there already, so I left it...

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi
  
Laurent Pinchart June 18, 2024, 3:14 p.m. UTC | #4
Hi Sakari,

On Mon, Nov 13, 2023 at 12:50:16PM +0200, Laurent Pinchart wrote:
> On Mon, Nov 13, 2023 at 10:42:16AM +0000, Sakari Ailus wrote:
> > On Mon, Nov 13, 2023 at 12:17:17PM +0200, Laurent Pinchart wrote:
> > > The v4l2_subdev_link_validate_get_format() function warns if the pad
> > > given as argument belongs to a non-subdev entity. This can't happen, as
> > > the function is called from v4l2_subdev_link_validate() only (indirectly
> > > through v4l2_subdev_link_validate_locked()), and that function ensures
> > > that both ends of the link are subdevs.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > >  drivers/media/v4l2-core/v4l2-subdev.c | 8 --------
> > >  1 file changed, 8 deletions(-)
> > > 
> > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > > index be86b906c985..67d43206ce32 100644
> > > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > > @@ -1184,14 +1184,6 @@ v4l2_subdev_link_validate_get_format(struct media_pad *pad, u32 stream,
> > >  	struct v4l2_subdev *sd;
> > >  	int ret;
> > >  
> > > -	if (!is_media_entity_v4l2_subdev(pad->entity)) {
> > > -		WARN(pad->entity->function != MEDIA_ENT_F_IO_V4L,
> > > -		     "Driver bug! Wrong media entity type 0x%08x, entity %s\n",
> > > -		     pad->entity->function, pad->entity->name);
> > > -
> > > -		return -EINVAL;
> > > -	}
> > > -
> > >  	sd = media_entity_to_v4l2_subdev(pad->entity);
> > 
> > It'd be good to check sd is non-NULL here, although pad presumably is
> > always a pad of a sub-device entity.
> 
> It's guaranteed by the caller. Is there a specific reason you think a
> check would be good ?

Would you still want a check ?

> > >  
> > >  	fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;
  

Patch

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index be86b906c985..67d43206ce32 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -1184,14 +1184,6 @@  v4l2_subdev_link_validate_get_format(struct media_pad *pad, u32 stream,
 	struct v4l2_subdev *sd;
 	int ret;
 
-	if (!is_media_entity_v4l2_subdev(pad->entity)) {
-		WARN(pad->entity->function != MEDIA_ENT_F_IO_V4L,
-		     "Driver bug! Wrong media entity type 0x%08x, entity %s\n",
-		     pad->entity->function, pad->entity->name);
-
-		return -EINVAL;
-	}
-
 	sd = media_entity_to_v4l2_subdev(pad->entity);
 
 	fmt->which = V4L2_SUBDEV_FORMAT_ACTIVE;