| Message ID | 20220614191127.3420492-17-paul.elder@ideasonboard.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: rkisp1: Cleanups and add support for i.MX8MP | expand |
+ Sakari and Hans This patch is needed by this series, but I think it could also be reviewed and merged standalone already. On Wed, Jun 15, 2022 at 04:10:48AM +0900, Paul Elder wrote: > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Drivers typically extend the v4l2_async_subdev structure by embedding it > in a driver-specific structure, to store per-subdev custom data. The > v4l2_async_subdev instances are freed by the v4l2-async framework, which > makes this mechanism cumbersome to use safely when custom data needs > special treatment to be destroyed (such as freeing additional memory, or > releasing references to kernel objects). > > To ease this, add a .destroy() operation to the > v4l2_async_notifier_operations structure. The operation is called right > before the v4l2_async_subdev is freed, giving drivers a chance to > destroy data if needed. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > Documentation/driver-api/media/v4l2-subdev.rst | 6 ++++++ > drivers/media/v4l2-core/v4l2-async.c | 10 ++++++++++ > include/media/v4l2-async.h | 2 ++ > 3 files changed, 18 insertions(+) > > diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst > index cf3b52bdbfb9..6f8d79926aa5 100644 > --- a/Documentation/driver-api/media/v4l2-subdev.rst > +++ b/Documentation/driver-api/media/v4l2-subdev.rst > @@ -243,6 +243,12 @@ notifier callback is called. After all subdevices have been located the > .complete() callback is called. When a subdevice is removed from the > system the .unbind() method is called. All three callbacks are optional. > > +Drivers can store any type of custom data in their driver-specific > +:c:type:`v4l2_async_subdev` wrapper. If any of that data requires special > +handling when the structure is freed, drivers must implement the ``.destroy()`` > +notifier callback. The framework will call it right before freeing the > +:c:type:`v4l2_async_subdev`. > + > Calling subdev operations > ~~~~~~~~~~~~~~~~~~~~~~~~~ > > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c > index c6995718237a..735dede624b8 100644 > --- a/drivers/media/v4l2-core/v4l2-async.c > +++ b/drivers/media/v4l2-core/v4l2-async.c > @@ -52,6 +52,15 @@ static int v4l2_async_nf_call_complete(struct v4l2_async_notifier *n) > return n->ops->complete(n); > } > > +static void v4l2_async_nf_call_destroy(struct v4l2_async_notifier *n, > + struct v4l2_async_subdev *asd) > +{ > + if (!n->ops || !n->ops->destroy) > + return; > + > + n->ops->destroy(asd); > +} > + > static bool match_i2c(struct v4l2_async_notifier *notifier, > struct v4l2_subdev *sd, struct v4l2_async_subdev *asd) > { > @@ -626,6 +635,7 @@ static void __v4l2_async_nf_cleanup(struct v4l2_async_notifier *notifier) > } > > list_del(&asd->asd_list); > + v4l2_async_nf_call_destroy(notifier, asd); > kfree(asd); > } > } > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h > index 13ff3ad948f4..25eb1d138c06 100644 > --- a/include/media/v4l2-async.h > +++ b/include/media/v4l2-async.h > @@ -81,6 +81,7 @@ struct v4l2_async_subdev { > * @complete: All subdevices have been probed successfully. The complete > * callback is only executed for the root notifier. > * @unbind: a subdevice is leaving > + * @destroy: the asd is about to be freed > */ > struct v4l2_async_notifier_operations { > int (*bound)(struct v4l2_async_notifier *notifier, > @@ -90,6 +91,7 @@ struct v4l2_async_notifier_operations { > void (*unbind)(struct v4l2_async_notifier *notifier, > struct v4l2_subdev *subdev, > struct v4l2_async_subdev *asd); > + void (*destroy)(struct v4l2_async_subdev *asd); > }; > > /**
On 6/14/22 21:10, Paul Elder wrote: > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Drivers typically extend the v4l2_async_subdev structure by embedding it > in a driver-specific structure, to store per-subdev custom data. The > v4l2_async_subdev instances are freed by the v4l2-async framework, which > makes this mechanism cumbersome to use safely when custom data needs > special treatment to be destroyed (such as freeing additional memory, or > releasing references to kernel objects). > > To ease this, add a .destroy() operation to the > v4l2_async_notifier_operations structure. The operation is called right > before the v4l2_async_subdev is freed, giving drivers a chance to > destroy data if needed. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Looks good! Regards, Hans > --- > Documentation/driver-api/media/v4l2-subdev.rst | 6 ++++++ > drivers/media/v4l2-core/v4l2-async.c | 10 ++++++++++ > include/media/v4l2-async.h | 2 ++ > 3 files changed, 18 insertions(+) > > diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst > index cf3b52bdbfb9..6f8d79926aa5 100644 > --- a/Documentation/driver-api/media/v4l2-subdev.rst > +++ b/Documentation/driver-api/media/v4l2-subdev.rst > @@ -243,6 +243,12 @@ notifier callback is called. After all subdevices have been located the > .complete() callback is called. When a subdevice is removed from the > system the .unbind() method is called. All three callbacks are optional. > > +Drivers can store any type of custom data in their driver-specific > +:c:type:`v4l2_async_subdev` wrapper. If any of that data requires special > +handling when the structure is freed, drivers must implement the ``.destroy()`` > +notifier callback. The framework will call it right before freeing the > +:c:type:`v4l2_async_subdev`. > + > Calling subdev operations > ~~~~~~~~~~~~~~~~~~~~~~~~~ > > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c > index c6995718237a..735dede624b8 100644 > --- a/drivers/media/v4l2-core/v4l2-async.c > +++ b/drivers/media/v4l2-core/v4l2-async.c > @@ -52,6 +52,15 @@ static int v4l2_async_nf_call_complete(struct v4l2_async_notifier *n) > return n->ops->complete(n); > } > > +static void v4l2_async_nf_call_destroy(struct v4l2_async_notifier *n, > + struct v4l2_async_subdev *asd) > +{ > + if (!n->ops || !n->ops->destroy) > + return; > + > + n->ops->destroy(asd); > +} > + > static bool match_i2c(struct v4l2_async_notifier *notifier, > struct v4l2_subdev *sd, struct v4l2_async_subdev *asd) > { > @@ -626,6 +635,7 @@ static void __v4l2_async_nf_cleanup(struct v4l2_async_notifier *notifier) > } > > list_del(&asd->asd_list); > + v4l2_async_nf_call_destroy(notifier, asd); > kfree(asd); > } > } > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h > index 13ff3ad948f4..25eb1d138c06 100644 > --- a/include/media/v4l2-async.h > +++ b/include/media/v4l2-async.h > @@ -81,6 +81,7 @@ struct v4l2_async_subdev { > * @complete: All subdevices have been probed successfully. The complete > * callback is only executed for the root notifier. > * @unbind: a subdevice is leaving > + * @destroy: the asd is about to be freed > */ > struct v4l2_async_notifier_operations { > int (*bound)(struct v4l2_async_notifier *notifier, > @@ -90,6 +91,7 @@ struct v4l2_async_notifier_operations { > void (*unbind)(struct v4l2_async_notifier *notifier, > struct v4l2_subdev *subdev, > struct v4l2_async_subdev *asd); > + void (*destroy)(struct v4l2_async_subdev *asd); > }; > > /**
diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst index cf3b52bdbfb9..6f8d79926aa5 100644 --- a/Documentation/driver-api/media/v4l2-subdev.rst +++ b/Documentation/driver-api/media/v4l2-subdev.rst @@ -243,6 +243,12 @@ notifier callback is called. After all subdevices have been located the .complete() callback is called. When a subdevice is removed from the system the .unbind() method is called. All three callbacks are optional. +Drivers can store any type of custom data in their driver-specific +:c:type:`v4l2_async_subdev` wrapper. If any of that data requires special +handling when the structure is freed, drivers must implement the ``.destroy()`` +notifier callback. The framework will call it right before freeing the +:c:type:`v4l2_async_subdev`. + Calling subdev operations ~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index c6995718237a..735dede624b8 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -52,6 +52,15 @@ static int v4l2_async_nf_call_complete(struct v4l2_async_notifier *n) return n->ops->complete(n); } +static void v4l2_async_nf_call_destroy(struct v4l2_async_notifier *n, + struct v4l2_async_subdev *asd) +{ + if (!n->ops || !n->ops->destroy) + return; + + n->ops->destroy(asd); +} + static bool match_i2c(struct v4l2_async_notifier *notifier, struct v4l2_subdev *sd, struct v4l2_async_subdev *asd) { @@ -626,6 +635,7 @@ static void __v4l2_async_nf_cleanup(struct v4l2_async_notifier *notifier) } list_del(&asd->asd_list); + v4l2_async_nf_call_destroy(notifier, asd); kfree(asd); } } diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h index 13ff3ad948f4..25eb1d138c06 100644 --- a/include/media/v4l2-async.h +++ b/include/media/v4l2-async.h @@ -81,6 +81,7 @@ struct v4l2_async_subdev { * @complete: All subdevices have been probed successfully. The complete * callback is only executed for the root notifier. * @unbind: a subdevice is leaving + * @destroy: the asd is about to be freed */ struct v4l2_async_notifier_operations { int (*bound)(struct v4l2_async_notifier *notifier, @@ -90,6 +91,7 @@ struct v4l2_async_notifier_operations { void (*unbind)(struct v4l2_async_notifier *notifier, struct v4l2_subdev *subdev, struct v4l2_async_subdev *asd); + void (*destroy)(struct v4l2_async_subdev *asd); }; /**