Hi Niklas,
On Mon, Jul 17, 2017 at 6:59 PM, Niklas Söderlund
<niklas.soderlund+renesas@xxxxxxxxxxxx> wrote:
> Add a subdevice specific notifier which can be used by a subdevice
> driver to compliment the master device notifier to extend the subdevice
> discovery.
>
> The master device registers the subdevices closest to itself in its
> notifier while the subdevice(s) register notifiers for their closest
> neighboring devices. Subdevice drivers configures a notifier at probe
> time which are registered by the v4l2-async framework once the subdevice
> itself is register, since it's only at this point the v4l2_dev is
> available to the subnotifier.
>
> Using this incremental approach two problems can be solved:
>
> 1. The master device no longer has to care how many devices exist in
> the pipeline. It only needs to care about its closest subdevice and
> arbitrary long pipelines can be created without having to adapt the
> master device for each case.
>
> 2. Subdevices which are represented as a single DT node but register
> more than one subdevice can use this to improve the pipeline
> discovery, since the subdevice driver is the only one who knows which
> of its subdevices is linked with which subdevice of a neighboring DT
> node.
>
> To allow subdevices to provide its own list of subdevices to the
> v4l2-async framework v4l2_async_subdev_register_notifier() is added.
> This new function must be called before the subdevice itself is
> registered with the v4l2-async framework using
> v4l2_async_register_subdev().
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
Thanks for your patch!
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -217,6 +293,12 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
> "Failed to allocate device cache!\n");
> }
>
> + subdev = kvmalloc_array(n_subdev, sizeof(*subdev), GFP_KERNEL);
> + if (!dev) {
if (!subdev) {
(noticed accidentally[*] :-)
> + dev_err(notifier->v4l2_dev->dev,
> + "Failed to allocate subdevice cache!\n");
> + }
> +
> mutex_lock(&list_lock);
>
> list_del(¬ifier->list);
> @@ -224,6 +306,8 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
> list_for_each_entry_safe(sd, tmp, ¬ifier->done, async_list) {
> if (dev)
> dev[count] = get_device(sd->dev);
> + if (subdev)
> + subdev[count] = sd;
I don't like these "memory allocation fails, let's continue but check the
pointer first"-games.
Why not abort when the dev/subdev arrays cannot be allocated? It's not
like the system is in a good state anyway.
kmalloc() may have printed a big fat warning and invoked the OOM-killer
already.
[*] while checking if you perhaps removed the "dev" games in a later patch.
No, you added another one :-(
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
