On 12/06/18 00:13, Steve Longerbeam wrote: > > > On 12/5/18 10:50 AM, Hans Verkuil wrote: >> On 12/05/2018 02:20 AM, Steve Longerbeam wrote: >>> Hi Hans, Philipp, >>> >>> One comment on my side... >>> >>> On 12/3/18 7:21 AM, Hans Verkuil wrote: >>>> <snip> >>>>> +void imx_media_mem2mem_device_unregister(struct imx_media_video_dev *vdev) >>>>> +{ >>>>> + struct mem2mem_priv *priv = to_mem2mem_priv(vdev); >>>>> + struct video_device *vfd = priv->vdev.vfd; >>>>> + >>>>> + mutex_lock(&priv->mutex); >>>>> + >>>>> + if (video_is_registered(vfd)) { >>>>> + video_unregister_device(vfd); >>>>> + media_entity_cleanup(&vfd->entity); >>>> Is this needed? >>>> >>>> If this is to be part of the media controller, then I expect to see a call >>>> to v4l2_m2m_register_media_controller() somewhere. >>>> >>> Yes, I agree there should be a call to >>> v4l2_m2m_register_media_controller(). This driver does not connect with >>> any of the imx-media entities, but calling it will at least make the >>> mem2mem output/capture device entities (and processing entity) visible >>> in the media graph. >>> >>> Philipp, can you pick/squash the following from my media-tree github fork? >>> >>> 6fa05f5170 ("media: imx: mem2mem: Add missing media-device header") >>> d355bf8b15 ("media: imx: Add missing unregister and remove of mem2mem >>> device") >>> 6787a50cdc ("media: imx: mem2mem: Register with media control") >>> >>> Steve >>> >> Why is this driver part of the imx driver? Since it doesn't connect with >> any of the imx-media entities, doesn't that mean that this is really a >> stand-alone driver? > > It is basically a stand-alone m2m driver, but it makes use of some > imx-media utility functions like imx_media_enum_format(). Also making it > a true stand-alone driver would require creating a second /dev/mediaN > device. If it is standalone, is it reused in newer iMX versions? (7 or 8) And if it is just a regular m2m device, then it doesn't need to create a media device either (doesn't hurt, but it is not required). Regards, Hans > > Steve >