Re: [PATCH v3 2/8] media: v4l: subdev: Also return pads array information on stream functions

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Laurent,

On Tue, Oct 24, 2023 at 01:07:29AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Mon, Oct 23, 2023 at 08:44:02PM +0300, Sakari Ailus wrote:
> > There are two sets of functions that return information from sub-device
> > state, one for stream-unaware users and another for stream-aware users.
> > Add support for stream-aware functions to return format, crop and compose
> > information from pad-based array that are functionally equivalent to the
> > old, stream-unaware ones.
> > 
> > Also check state is non-NULL, in order to guard against old drivers
> > potentially calling this with NULL state for active formats or selection
> > rectangles.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-subdev.c | 39 +++++++++++++++++++++++++++
> >  include/media/v4l2-subdev.h           |  9 ++++---
> >  2 files changed, 45 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> > index ee4fe8f33a41..955ee9a6c91f 100644
> > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > @@ -1684,6 +1684,19 @@ v4l2_subdev_state_get_stream_format(struct v4l2_subdev_state *state,
> >  	struct v4l2_subdev_stream_configs *stream_configs;
> >  	unsigned int i;
> >  
> > +	if (WARN_ON(!state))
> > +		return NULL;
> > +
> > +	if (state->pads) {
> > +		if (stream)
> > +			return NULL;
> > +
> > +		if (WARN_ON(pad >= state->sd->entity.num_pads))
> > +			pad = 0;
> 
> Do we want to carry this forward from the existing pad config accessors,
> can't we return NULL instead ?

I also think this should be changed, but again, this should be done in a
separate patch. I'd also postpone it after this set is merged.

> 
> Either way,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

-- 
Regards,

Sakari Ailus