Hi Dan,
cc-ing Kieran which has proposed to discuss about this during media
summit
On Fri, May 06, 2022 at 10:15:55PM +0100, Daniel Scally wrote:
> Creating links and registering subdev nodes during the .complete()
> callback has the unfortunate effect of preventing all cameras that
> connect to a notifier from working if any one of their drivers fails
> to probe. Moving the functionality from .complete() to .bound() allows
> those camera sensor drivers that did probe correctly to work regardless.
>
> Signed-off-by: Daniel Scally <djrscally@xxxxxxxxx>
> ---
>
> This results in v4l2_device_register_subdev_nodes() being called multiple times
> but since it's a no-op where the devnode exists already, I think that it's ok.
>
> drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 65 +++++++------------
> 1 file changed, 23 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> index 93cc0577b6db..6a1bcb20725d 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> @@ -1387,7 +1387,10 @@ static int cio2_notifier_bound(struct v4l2_async_notifier *notifier,
> {
> struct cio2_device *cio2 = to_cio2_device(notifier);
> struct sensor_async_subdev *s_asd = to_sensor_asd(asd);
> + struct device *dev = &cio2->pci_dev->dev;
> struct cio2_queue *q;
> + unsigned int pad;
> + int ret;
>
> if (cio2->queue[s_asd->csi2.port].sensor)
> return -EBUSY;
> @@ -1398,7 +1401,26 @@ static int cio2_notifier_bound(struct v4l2_async_notifier *notifier,
> q->sensor = sd;
> q->csi_rx_base = cio2->base + CIO2_REG_PIPE_BASE(q->csi2.port);
>
> - return 0;
> + for (pad = 0; pad < q->sensor->entity.num_pads; pad++)
> + if (q->sensor->entity.pads[pad].flags &
> + MEDIA_PAD_FL_SOURCE)
> + break;
> +
> + if (pad == q->sensor->entity.num_pads) {
> + dev_err(dev, "failed to find src pad for %s\n",
> + q->sensor->name);
> + return -ENXIO;
> + }
> +
> + ret = media_create_pad_link(&q->sensor->entity, pad, &q->subdev.entity,
> + CIO2_PAD_SINK, 0);
> + if (ret) {
> + dev_err(dev, "failed to create link for %s\n",
> + q->sensor->name);
> + return ret;
> + }
> +
> + return v4l2_device_register_subdev_nodes(&cio2->v4l2_dev);
> }
>
> /* The .unbind callback */
> @@ -1412,50 +1434,9 @@ static void cio2_notifier_unbind(struct v4l2_async_notifier *notifier,
> cio2->queue[s_asd->csi2.port].sensor = NULL;
> }
>
> -/* .complete() is called after all subdevices have been located */
> -static int cio2_notifier_complete(struct v4l2_async_notifier *notifier)
> -{
> - struct cio2_device *cio2 = to_cio2_device(notifier);
> - struct device *dev = &cio2->pci_dev->dev;
> - struct sensor_async_subdev *s_asd;
> - struct v4l2_async_subdev *asd;
> - struct cio2_queue *q;
> - unsigned int pad;
> - int ret;
> -
> - list_for_each_entry(asd, &cio2->notifier.asd_list, asd_list) {
> - s_asd = to_sensor_asd(asd);
> - q = &cio2->queue[s_asd->csi2.port];
> -
> - for (pad = 0; pad < q->sensor->entity.num_pads; pad++)
> - if (q->sensor->entity.pads[pad].flags &
> - MEDIA_PAD_FL_SOURCE)
> - break;
> -
> - if (pad == q->sensor->entity.num_pads) {
> - dev_err(dev, "failed to find src pad for %s\n",
> - q->sensor->name);
> - return -ENXIO;
> - }
> -
> - ret = media_create_pad_link(
> - &q->sensor->entity, pad,
> - &q->subdev.entity, CIO2_PAD_SINK,
> - 0);
> - if (ret) {
> - dev_err(dev, "failed to create link for %s\n",
> - q->sensor->name);
> - return ret;
> - }
> - }
> -
> - return v4l2_device_register_subdev_nodes(&cio2->v4l2_dev);
> -}
> -
> static const struct v4l2_async_notifier_operations cio2_async_ops = {
> .bound = cio2_notifier_bound,
> .unbind = cio2_notifier_unbind,
> - .complete = cio2_notifier_complete,
> };
>
> static int cio2_parse_firmware(struct cio2_device *cio2)
> --
> 2.25.1
>
