On Thu, Oct 26, 2023 at 08:25:04AM +0000, Sakari Ailus wrote: > 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. I would avoid this, it will make things even more confusing by adding yet another naming scheme. If we want to clean things up (and I think we should), then it should be done on top, and span the entire subsystem. > > Anyway, I'm just throwing this out here since I think it is confusing. -- Regards, Laurent Pinchart
