Hi Hans,

On Mon, Aug 22, 2016 at 02:01:44PM +0200, Hans Verkuil wrote:
> On 08/19/2016 12:23 PM, Sakari Ailus wrote:
> > From: Sakari Ailus <sakari.ai...@iki.fi>
> > 
> > Allow allocating the media device dynamically. As the struct media_device
> > embeds struct media_devnode, the lifetime of that object is that same than
> > that of the media_device.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> > ---
> >  drivers/media/media-device.c | 22 ++++++++++++++++++++++
> >  include/media/media-device.h | 38 ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 60 insertions(+)
> > 
> > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> > index a431775..5a86d36 100644
> > --- a/drivers/media/media-device.c
> > +++ b/drivers/media/media-device.c
> > @@ -544,7 +544,15 @@ static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
> >  
> >  static void media_device_release(struct media_devnode *devnode)
> >  {
> > +   struct media_device *mdev = to_media_device(devnode);
> > +
> > +   ida_destroy(&mdev->entity_internal_idx);
> > +   mdev->entity_internal_idx_max = 0;
> > +   media_entity_graph_walk_cleanup(&mdev->pm_count_walk);
> > +   mutex_destroy(&mdev->graph_mutex);
> >     dev_dbg(devnode->parent, "Media device released\n");
> > +
> > +   kfree(mdev);
> 
> Doesn't this break bisect? mdev is now freed, but media_device_alloc isn't
> called yet.
> 
> That doesn't seem right.

You're right.

media_device_release() actually may only be called for drivers that use
media_device_init(). I'll fix that.

> 
> Regards,
> 
>       Hans
> 
> >  }
> >  
> >  /**
> > @@ -691,6 +699,20 @@ void media_device_init(struct media_device *mdev)
> >  }
> >  EXPORT_SYMBOL_GPL(media_device_init);
> >  
> > +struct media_device *media_device_alloc(void)
> > +{
> > +   struct media_device *mdev;
> > +
> > +   mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
> > +   if (!mdev)
> > +           return NULL;
> > +
> > +   media_device_init(mdev);
> > +
> > +   return mdev;
> > +}
> > +EXPORT_SYMBOL_GPL(media_device_alloc);
> > +
> >  void media_device_cleanup(struct media_device *mdev)
> >  {
> >     ida_destroy(&mdev->entity_internal_idx);
> > diff --git a/include/media/media-device.h b/include/media/media-device.h
> > index 4eee613..d1d45ab 100644
> > --- a/include/media/media-device.h
> > +++ b/include/media/media-device.h
> > @@ -197,6 +197,42 @@ static inline __must_check int media_entity_enum_init(
> >  void media_device_init(struct media_device *mdev);
> >  
> >  /**
> > + * media_device_alloc() - Allocate and initialise a media device
> > + *
> > + * Allocate and initialise a media device. Returns a media device.
> > + * The media device is refcounted, and this function returns a media
> > + * device the refcount of which is one (1).
> > + *
> > + * References are taken and given using media_device_get() and
> > + * media_device_put().
> > + */
> > +struct media_device *media_device_alloc(void);
> > +
> > +/**
> > + * media_device_get() - Get a reference to a media device
> > + *
> > + * mdev: media device
> > + */
> > +#define media_device_get(mdev)                                             
> > \
> > +   do {                                                            \
> > +           dev_dbg((mdev)->dev, "%s: get media device %s\n",       \
> > +                   __func__, (mdev)->bus_info);                    \
> > +           get_device(&(mdev)->devnode.dev);                       \
> > +   } while (0)
> > +
> > +/**
> > + * media_device_put() - Put a reference to a media device
> > + *
> > + * mdev: media device
> > + */
> > +#define media_device_put(mdev)                                             
> > \
> > +   do {                                                            \
> > +           dev_dbg((mdev)->dev, "%s: put media device %s\n",       \
> > +                   __func__, (mdev)->bus_info);                    \
> > +           put_device(&(mdev)->devnode.dev);                       \
> > +   } while (0)
> > +
> > +/**
> >   * media_device_cleanup() - Cleanups a media device element
> >   *
> >   * @mdev:  pointer to struct &media_device
> > @@ -425,6 +461,8 @@ void __media_device_usb_init(struct media_device *mdev,
> >                          const char *driver_name);
> >  
> >  #else
> > +#define media_device_get(mdev) do { } while (0)
> > +#define media_device_put(mdev) do { } while (0)
> >  static inline int media_device_register(struct media_device *mdev)
> >  {
> >     return 0;
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi     XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to