Hi Mauro, On Mon, 2021-02-08 at 13:54 +0100, Mauro Carvalho Chehab wrote: > Em Mon, 8 Feb 2021 12:32:44 +0200 > Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> escreveu: > > > Hi Mauro, > > > > Thanks for the review. > > > > On Sat, Feb 06, 2021 at 09:15:46AM +0100, Mauro Carvalho Chehab wrote: > > > Em Tue, 2 Feb 2021 15:56:09 +0200 > > > Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> escreveu: > > > > > > > From: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx> > > > > > > > > Most -if not all- use-cases are expected to be covered by one of: > > > > v4l2_async_notifier_add_fwnode_subdev, > > > > v4l2_async_notifier_add_fwnode_remote_subdev or > > > > v4l2_async_notifier_add_i2c_subdev. > > > > > > Actually, all cases outside V4L2 core: > > > > > > $ git grep v4l2_async_notifier_add_subdev > > > Documentation/driver-api/media/v4l2-subdev.rst:using the :c:func:`v4l2_async_notifier_add_subdev` call. This function > > > drivers/media/v4l2-core/v4l2-async.c:int v4l2_async_notifier_add_subdev(struct v4l2_async_notifier *notifier, > > > drivers/media/v4l2-core/v4l2-async.c:EXPORT_SYMBOL_GPL(v4l2_async_notifier_add_subdev); > > > drivers/media/v4l2-core/v4l2-async.c: ret = v4l2_async_notifier_add_subdev(notifier, asd); > > > drivers/media/v4l2-core/v4l2-async.c: ret = v4l2_async_notifier_add_subdev(notifier, asd); > > > drivers/media/v4l2-core/v4l2-fwnode.c: ret = v4l2_async_notifier_add_subdev(notifier, asd); > > > include/media/v4l2-async.h: * before the first call to @v4l2_async_notifier_add_subdev. > > > include/media/v4l2-async.h: * v4l2_async_notifier_add_subdev - Add an async subdev to the > > > include/media/v4l2-async.h:int v4l2_async_notifier_add_subdev(struct v4l2_async_notifier *notifier, > > > include/media/v4l2-async.h: * @v4l2_async_notifier_add_subdev, > > > > > > > > > > > > > > We'd like to discourage drivers from using v4l2_async_notifier_add_subdev, > > > > so rename it as __v4l2_async_notifier_add_subdev. This is > > > > typically a good hint for drivers to avoid using the function. > > > > > > IMO, the best here would be to create a header file: > > > > > > drivers/media/v4l2-core/v4l2-async-priv.h > > > > > > and move v4l2_async_notifier_add_subdev from include/media/v4l2-async.h. > > > > > > This will warrant that only the V4L2 core would have access to it. > > > Also, it is a lot better than adding something like this: > > > > It'd be the first header in the directory. I suppose there are other > > functions that could have the prototype there. > > > > I'll still see if there could be other options for this. > > > > The topic of split into modules of v4l2-async and v4l2-fwnode was also > > discussed recently, and it's obviously related to this. The two are > > virtually always used together and so would make sense to be in the same > > module. In practice this would mean moving v4l2-async out of videodev2. The > > module wouldn't be large but the vast majority of regular laptop and > > desktop PC users are having this in memory needlessly. > > IMO, splitting kAPI headers from v4l-core internal usage is a good > idea for the things that should be used only inside the core. > > The RC core has one such header: > > drivers/media/rc/rc-core-priv.h > > Several DVB frontend and tuner drivers also have things like that. > This solution looks like the way to go. Thanks for bringing it up. Specifically for v4l2-async, we were considering reorganizing the code, perhaps removing the need for a private/core API. We'll see how that goes. Thanks, Ezequiel
