| Message ID | 20240610100530.1107771-9-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Media device lifetime management | expand |
On 10/06/2024 12:05, Sakari Ailus wrote: > cdev_device_del() is the right function to remove a device when > cdev_device_add() succeeds. If it does not, however, put_device() needs to > be used instead. Fix this. Hmm, this too is due to a revert patch (03/26) removing something that needs to be reinstated. Wouldn't it be better to fold this into 04/26, with a comment in the commit log of that commit? The problem with this commit log is that it suggests that it fixes a bug, when really it just corrects something introduced by a revert. Regards, Hans > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > --- > drivers/media/mc/mc-devnode.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c > index 2e33c2007f08..707593d127a7 100644 > --- a/drivers/media/mc/mc-devnode.c > +++ b/drivers/media/mc/mc-devnode.c > @@ -252,9 +252,9 @@ int __must_check media_devnode_register(struct media_devnode *devnode, > > cdev_add_error: > mutex_lock(&media_devnode_lock); > - cdev_device_del(&devnode->cdev, &devnode->dev); > clear_bit(devnode->minor, media_devnode_nums); > mutex_unlock(&media_devnode_lock); > + put_device(&devnode->dev); > > return ret; > }
Hi Hans, On Mon, Jun 17, 2024 at 11:13:57AM +0200, Hans Verkuil wrote: > On 10/06/2024 12:05, Sakari Ailus wrote: > > cdev_device_del() is the right function to remove a device when > > cdev_device_add() succeeds. If it does not, however, put_device() needs to > > be used instead. Fix this. > > Hmm, this too is due to a revert patch (03/26) removing something that needs > to be reinstated. > > Wouldn't it be better to fold this into 04/26, with a comment in the commit > log of that commit? I agree, I'll fold it there. > > The problem with this commit log is that it suggests that it fixes a bug, > when really it just corrects something introduced by a revert.
diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c index 2e33c2007f08..707593d127a7 100644 --- a/drivers/media/mc/mc-devnode.c +++ b/drivers/media/mc/mc-devnode.c @@ -252,9 +252,9 @@ int __must_check media_devnode_register(struct media_devnode *devnode, cdev_add_error: mutex_lock(&media_devnode_lock); - cdev_device_del(&devnode->cdev, &devnode->dev); clear_bit(devnode->minor, media_devnode_nums); mutex_unlock(&media_devnode_lock); + put_device(&devnode->dev); return ret; }