Re: [PATCH v4 24/29] media: v4l2-subdev: Introduce v4l2_subdev_get_frame_desc()

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

On Wed, Apr 22, 2026 at 01:02:32PM +0300, Sakari Ailus wrote:
> On Wed, Apr 22, 2026 at 12:02:56PM +0300, Laurent Pinchart wrote:
> > > > > > > > > +int v4l2_subdev_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> > > > > > > > > +			       struct v4l2_mbus_frame_desc *desc)
> > > > > > > > > +{
> > > > > > > > > +	struct v4l2_subdev_format subdev_fmt = {
> > > > > > > > > +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> > > > > > > > > +		.pad = pad,
> > > > > > > > > +	};
> > > > > > > > > +	int ret;
> > > > > > > > > +
> > > > > > > > > +	if (v4l2_subdev_has_op(sd, pad, get_frame_desc)) {
> > > > > > > > > +		unsigned int type = desc->type;
> > > > > > > > > +
> > > > > > > > > +		ret = v4l2_subdev_call(sd, pad, get_frame_desc, pad, desc);
> > > > > > > > > +
> > > > > > > > > +		if (desc->type != type)
> > > > > > > > > +			return -EINVAL;
> > > > > > 
> > > > > > I'd add a dev_err() here. There are .get_frame_desc() callers that check
> > > > > > if the returned type matches what they expect and log an error
> > > > > > otherwise. When using this helper the check can't be performed in the
> > > > > > callera any more, leading to possibly hard to debug issues if no message
> > > > > > is printed.
> > > > > 
> > > > > dev_err_once()?
> > > > 
> > > > Is there a need to limit it to printing the message once only ? It will
> > > > only occur if an incompatible source is connected, which shouldn't
> > > > happen in normal circumstances.
> > > 
> > > Yes, but still enables filling logs with that message. A single one in this
> > > case should be enough.
> > 
> > Is it user-triggerable without a serious bug in drivers ?
> 
> No. But one message still tells about the problem, doesn't it?

Yes, but it then means a reboot is necessary during development every
time this error occurs. It also means that if two errors originate from
different callers only one message will be printed, which is also not
optimal for development.

If this error was triggerable by userspace in normal circumstances then
I'd still avoid _once() but go for dev_dbg().

-- 
Regards,

Laurent Pinchart