Hi Hans, On Thu, Oct 26, 2023 at 09:26:40AM +0200, Hans Verkuil wrote: > On 26/10/2023 09:03, Sakari Ailus wrote: > > Rename the sub-devices state information access functions, removing > > "_stream" from them. This makes them shorter and so more convenient to > > use. No other sets of functions will be needed to access this information. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > Reviewed-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx> > > --- > > drivers/media/i2c/ds90ub913.c | 3 +-- > > drivers/media/i2c/ds90ub953.c | 3 +-- > > drivers/media/i2c/ds90ub960.c | 12 ++++----- > > .../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 10 +++---- > > drivers/media/v4l2-core/v4l2-subdev.c | 27 +++++++++---------- > > include/media/v4l2-subdev.h | 19 +++++++------ > > 6 files changed, 34 insertions(+), 40 deletions(-) > > > > diff --git a/drivers/media/i2c/ds90ub913.c b/drivers/media/i2c/ds90ub913.c > > index 8e9ebed09f64..8bb6be956780 100644 > > --- a/drivers/media/i2c/ds90ub913.c > > +++ b/drivers/media/i2c/ds90ub913.c > > @@ -424,8 +424,7 @@ static int ub913_set_fmt(struct v4l2_subdev *sd, > > } > > > > /* Set sink format */ > > - fmt = v4l2_subdev_state_get_stream_format(state, format->pad, > > - format->stream); > > + fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream); > > if (!fmt) > > return -EINVAL; > > If we want shorter names, how about replacing _get_ with _g_? And of course > _set_ with _s_. I would prefer that, but it's up to you. These functions doesn't actually "get" something, but return a pointer to the data. There's thus no corresponding set. I think I'll keep it as-is for now but I think "_get" could as well be removed here. > > Since we are changing function names here, I wonder if this isn't a good > opportunity to straighten out some more confusing names. > > I noticed that in v4l2-subdev.h "fmt" typically refers to v4l2_subdev_format, > while "format" tends to refer to v4l2_mbus_framefmt. > > I find this really confusing, especially if you do not use these functions > regularly. > > My personal preference would be to use "format" for things relating to > v4l2_subdev_format and "ffmt" for v4l2_mbus_framefmt. Or possibly just "fmt" > if people don't like "ffmt". It would signal the type of format a lot better. I'll use ffmt for the name of the function. > > Anyway, I'm just throwing this out here since I think it is confusing. -- Regards, Sakari Ailus
