On Monday, September 26, 2011 13:17:00 Guennadi Liakhovetski wrote:
> Hi Hans
>
> Sorry for reviving an almost 2 year old thread, but the topic, discussed
> back then is still relevant (I'll include a complete quote to refresh the
> old discussion):
>
> On Thu, 5 Nov 2009, Hans Verkuil wrote:
>
> > On Friday 23 October 2009 16:25:40 Laurent Pinchart wrote:
> > > Hi everybody,
> > >
> > > while working on device node support for subdevs I ran into an issue with
> > > the
> > > way v4l2 objects are structured.
> > >
> > > We currently have the following structure:
> > >
> > > - video_device represents a device that complies with the V4L1 or V4L2
> > > API.
> > > Every video_device has a corresponding device node.
> > >
> > > - v4l2_device represents a high-level media device that handles
> > > sub-devices.
> > > With the new media controller infrastructure a v4l2_device will have a
> > > device
> > > node as well.
> > >
> > > - v4l2_subdev represents a sub-device. As for v4l2_device's, the new
> > > media
> > > controller infrastructure will give a device node for every sub-device.
> > >
> > > - v4l2_entity is the structure that both v4l2_subdev and video_device
> > > derive
> > > from. Most of the media controller code will deal with entities rather
> > > than
> > > sub-devices or video devices, as most operations (such as discovering the
> > > topology and create links) do not depend on the exact nature of the
> > > entity.
> > > New types of entities could be introduced later.
> > >
> > > Both the video_device and v4l2_subdev structure inherit from v4l2_entity,
> > > so
> > > both of them have a v4l2_entity field. With v4l2_device and v4l2_subdev
> > > now
> > > needing to devices to have device nodes created, the v4l2_device and
> > > v4l2_subdev structure both have a video_device field.
> > >
> > > This isn't clean for two reasons:
> > >
> > > - v4l2_device isn't a v4l2_entity, so it should inherit from a structure
> > > (video_device) that itself inherits from v4l2_entity.
> > >
> > > - v4l2_subdev shouldn't inherit twice from v4l2_entity, once directly and
> > > once
> > > through video_device.
> >
> > I agree.
> >
> > > To fix this I would like to refactor the video_device structure and cut
> > > it in
> > > two pieces. One of them will deal with device node related tasks, being
> > > mostly
> > > V4L1/V4L2 agnostic, and the other will inherit from the first and add
> > > V4L1/V4L2 support (tvnorms/current_norm/ioctl_ops fields from the current
> > > video_device structure), as well as media controller support (inheriting
> > > from
> > > v4l2_entity).
> > >
> > > My plan was to create a video_devnode structure for the low-level device
> > > node
> >
> > Let's call it v4l2_devnode to be consistent with the current naming
> > convention.
> >
> > > related structure, and keeping the video_device name for the higher level
> > > structure. v4l2_device, v4l2_subdev and video_device would then all have
> > > a
> > > video_devnode field.
> > >
> > > While this isn't exactly difficult, it would require changing a lot of
> > > drivers, as some field will be moved from video_device to
> > > video_device::video_devnode. Some of those fields are internal, some of
> > > them
> > > are accessed by drivers while they shouldn't in most cases (the minor
> > > field
> > > for instance), and some are public (name, parent).
> > >
> > > I would like to have your opinion on whether you think this proposal is
> > > acceptable or whether you see a better and cleaner way to restructure the
> > > video device code structures.
> > >
> >
> > I have two issues with this:
> >
> > 1) Is it really necessary to do this now? We are still in the prototyping
> > phase and I think it is probably more efficient right now to hack around
> > this
> > and postpone the real fix (as described above) until we are sure that the mc
> > concept is working correctly.
>
> Here comes my question: is it the right time for this now?;-) I've relaxed
> the problem a bit with this my patch:
>
> http://patchwork.linuxtv.org/patch/7817/
>
> But the problem, described above, when MC _is_ used - that of double
> inheritance - still remains. I really think it should be fixed now.
I'd say, make a proposal for this. I'm not against it and we have enough
experience now to have a good feel for this issue.
I think I would like to put this perhaps in a slightly bigger picture
as well. There are some naming problems as well that I would like to address:
1) v4l2_device is very poorly named. It really represents a root structure from
which to find all other pieces. It does not really represent a device. My
proposal
would be to rename it v4l2_root (alternatives are: v4l2_core, v4l2_top,
v4l2_sys).
Comments?
2) If v4l2_device becomes available as a name, then I would like to rename
video_device to v4l2_device. Originally I attempted to rename video_device to
v4l2_devnode, but that was opposed by Mauro. I gathered from him that he thinks
v4l2_device would be a proper name for this, but of course it is currently in
use.
Comments?
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
