[{"id":22104,"web_url":"https://patchwork.libcamera.org/comment/22104/","msgid":"","date":"2022-02-02T16:38:14","subject":"Re: [libcamera-devel] [PATCH v2 6/6] media: v4l2-async: Create\n\tlinks during v4l2_async_match_notify()","submitter":{"id":37,"url":"https://patchwork.libcamera.org/api/people/37/","name":"Sakari Ailus","email":"sakari.ailus@linux.intel.com"},"content":"Hi Daniel,\n\nThanks for the update.\n\nOn Sun, Jan 30, 2022 at 11:58:21PM +0000, Daniel Scally wrote:\n> Upon an async fwnode match, there's some typical behaviour that the\n> notifier and matching subdev will want to do. For example, a notifier\n> representing a sensor matching to an async subdev representing its\n> VCM will want to create an ancillary link to expose that relationship\n> to userspace.\n> \n> To avoid lots of code in individual drivers, try to build these links\n> within v4l2 core.\n> \n> Signed-off-by: Daniel Scally \n> ---\n> Changes since v1:\n> \n> \t- Added #ifdef guards for CONFIG_MEDIA_CONTROLLER\n> \t- Some spelling and nomenclature cleanup (Laurent)\n> \n> Changes since the rfc:\n> \n> \t- None\n> \n> drivers/media/v4l2-core/v4l2-async.c | 56 ++++++++++++++++++++++++++++\n> 1 file changed, 56 insertions(+)\n> \n> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c\n> index 0404267f1ae4..8980534e755e 100644\n> --- a/drivers/media/v4l2-core/v4l2-async.c\n> +++ b/drivers/media/v4l2-core/v4l2-async.c\n> @@ -275,6 +275,50 @@ v4l2_async_nf_try_complete(struct v4l2_async_notifier *notifier)\n> static int\n> v4l2_async_nf_try_all_subdevs(struct v4l2_async_notifier *notifier);\n> \n> +static int\n> +__v4l2_async_create_ancillary_link(struct v4l2_async_notifier *notifier,\n> +\t\t\t\t struct v4l2_subdev *sd)\n> +{\n> +\tstruct media_link *link = NULL;\n> +\n> +#if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)\n> +\n> +\tif (sd->entity.function != MEDIA_ENT_F_LENS &&\n> +\t sd->entity.function != MEDIA_ENT_F_FLASH)\n> +\t\treturn -EINVAL;\n> +\n> +\tlink = media_create_ancillary_link(¬ifier->sd->entity, &sd->entity);\n> +\n> +#endif\n> +\n> +\treturn IS_ERR(link) ? PTR_ERR(link) : 0;\n> +}\n> +\n> +/*\n> + * Create links on behalf of the notifier and subdev, where it's obvious what\n> + * should be done. At the moment, we only support cases where the notifier\n> + * is a camera sensor and the subdev is a lens controller.\n\nI think I'd rather change this so that ancillary links are created for lens\nand flash subdevs, independently of the function of the notifier subdev.\n\nAre there cases where this would go wrong currently, or in the future? I\ncan't think of any right now at least. I guess we could add more\ninformation in the future to convey here if needed.\n\n> + *\n> + * TODO: Create data links if the notifier's function is\n> + * MEDIA_ENT_F_VID_IF_BRIDGE and the subdev's is MEDIA_ENT_F_CAM_SENSOR.\n> + */\n> +static int v4l2_async_try_create_links(struct v4l2_async_notifier *notifier,\n> +\t\t\t\t struct v4l2_subdev *sd)\n\nHow about calling this v4l2_async_create_ancillary_links()?\n\n> +{\n> +#if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)\n> +\n> +\tif (!notifier->sd)\n> +\t\treturn 0;\n> +\n> +\tswitch (notifier->sd->entity.function) {\n> +\tcase MEDIA_ENT_F_CAM_SENSOR:\n> +\t\treturn __v4l2_async_create_ancillary_link(notifier, sd);\n> +\t}\n> +\n> +#endif\n> +\treturn 0;\n> +}\n> +\n> static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,\n> \t\t\t\t struct v4l2_device *v4l2_dev,\n> \t\t\t\t struct v4l2_subdev *sd,\n> @@ -293,6 +337,18 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,\n> \t\treturn ret;\n> \t}\n> \n> +\t/*\n> +\t * Depending of the function of the entities involved, we may want to\n> +\t * create links between them (for example between a sensor and its lens\n> +\t * or between a sensor's source pad and the connected device's sink\n> +\t * pad).\n> +\t */\n> +\tret = v4l2_async_try_create_links(notifier, sd);\n> +\tif (ret) {\n\nThe notifier's bound function has been called already. The unbound function\nneeds to be thus called here, too.\n\n> +\t\tv4l2_device_unregister_subdev(sd);\n> +\t\treturn ret;\n> +\t}\n> +\n> \t/* Remove from the waiting list */\n> \tlist_del(&asd->list);\n> \tsd->asd = asd;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 41706BF415\n\tfor ;\n\tWed, 2 Feb 2022 16:39:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F907609DF;\n\tWed, 2 Feb 2022 17:39:35 +0100 (CET)","from mga11.intel.com (mga11.intel.com [192.55.52.93])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 054A9609B5\n\tfor ;\n\tWed, 2 Feb 2022 17:39:32 +0100 (CET)","from orsmga002.jf.intel.com ([10.7.209.21])\n\tby fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; \n\t02 Feb 2022 08:38:20 -0800","from paasikivi.fi.intel.com ([10.237.72.42])\n\tby orsmga002-auth.jf.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Feb 2022 08:38:17 -0800","from paasikivi.fi.intel.com (localhost [127.0.0.1])\n\tby paasikivi.fi.intel.com (Postfix) with SMTP id BD18820139;\n\tWed, 2 Feb 2022 18:38:14 +0200 (EET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=intel.com header.i=@intel.com\n\theader.b=\"ZMZSzpFY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=intel.com; i=@intel.com; q=dns/txt; s=Intel;\n\tt=1643819973; x=1675355973;\n\th=date:from:to:cc:subject:message-id:references:\n\tmime-version:in-reply-to;\n\tbh=46B9ATcsjaW2tDg2VW5g3uE1sBtK/GhWxh2/eTndAeQ=;\n\tb=ZMZSzpFYdlfi7TKV8CylAJcAYTax+bjOPpWEZmS8IpTFOJn82c8/j/Wo\n\tfBXOwUYS6qp7/BYMdV3//NqDR8OF5tWHyR3xeeKr5y9mbg7L03KvxU8gR\n\tSjQbW3zv8ReyIzbnPUoAx+XXVJiSTP944VkZn80Kh2RAs2xzy6s1T2h6l\n\tGnM0GGrmtRt4+HbqsqiK/GG9SWcETqezYo0YrFsfwACf6bxW1OUDK2ny1\n\tN8ab2fyUKoRiSnCqWEvcxf+JjxqGv3EF5FP/qADH5jBhmcpenGTWKDxIQ\n\tTrhFeuYkozZ0hvZC5NV+E08JSjNayWmz6GuyI4Zg2W4EpkfbF77kASEA1 Q==;","X-IronPort-AV":["E=McAfee;i=\"6200,9189,10246\"; a=\"245553368\"","E=Sophos;i=\"5.88,337,1635231600\"; d=\"scan'208\";a=\"245553368\"","E=Sophos;i=\"5.88,337,1635231600\"; d=\"scan'208\";a=\"497800799\""],"Date":"Wed, 2 Feb 2022 18:38:14 +0200","From":"Sakari Ailus ","To":"Daniel Scally ","Message-ID":"","References":"<20220130235821.48076-1-djrscally@gmail.com>\n\t<20220130235821.48076-7-djrscally@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220130235821.48076-7-djrscally@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2 6/6] media: v4l2-async: Create\n\tlinks during v4l2_async_match_notify()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, linux-media@vger.kernel.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22107,"web_url":"https://patchwork.libcamera.org/comment/22107/","msgid":"","date":"2022-02-02T21:48:56","subject":"Re: [libcamera-devel] [PATCH v2 6/6] media: v4l2-async: Create\n\tlinks during v4l2_async_match_notify()","submitter":{"id":90,"url":"https://patchwork.libcamera.org/api/people/90/","name":"Daniel Scally","email":"djrscally@gmail.com"},"content":"Hi Sakari\n\nOn 02/02/2022 16:38, Sakari Ailus wrote:\n> Hi Daniel,\n>\n> Thanks for the update.\n>\n> On Sun, Jan 30, 2022 at 11:58:21PM +0000, Daniel Scally wrote:\n>> Upon an async fwnode match, there's some typical behaviour that the\n>> notifier and matching subdev will want to do. For example, a notifier\n>> representing a sensor matching to an async subdev representing its\n>> VCM will want to create an ancillary link to expose that relationship\n>> to userspace.\n>>\n>> To avoid lots of code in individual drivers, try to build these links\n>> within v4l2 core.\n>>\n>> Signed-off-by: Daniel Scally \n>> ---\n>> Changes since v1:\n>>\n>> \t- Added #ifdef guards for CONFIG_MEDIA_CONTROLLER\n>> \t- Some spelling and nomenclature cleanup (Laurent)\n>>\n>> Changes since the rfc:\n>>\n>> \t- None\n>>\n>> drivers/media/v4l2-core/v4l2-async.c | 56 ++++++++++++++++++++++++++++\n>> 1 file changed, 56 insertions(+)\n>>\n>> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c\n>> index 0404267f1ae4..8980534e755e 100644\n>> --- a/drivers/media/v4l2-core/v4l2-async.c\n>> +++ b/drivers/media/v4l2-core/v4l2-async.c\n>> @@ -275,6 +275,50 @@ v4l2_async_nf_try_complete(struct v4l2_async_notifier *notifier)\n>> static int\n>> v4l2_async_nf_try_all_subdevs(struct v4l2_async_notifier *notifier);\n>> \n>> +static int\n>> +__v4l2_async_create_ancillary_link(struct v4l2_async_notifier *notifier,\n>> +\t\t\t\t struct v4l2_subdev *sd)\n>> +{\n>> +\tstruct media_link *link = NULL;\n>> +\n>> +#if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)\n>> +\n>> +\tif (sd->entity.function != MEDIA_ENT_F_LENS &&\n>> +\t sd->entity.function != MEDIA_ENT_F_FLASH)\n>> +\t\treturn -EINVAL;\n>> +\n>> +\tlink = media_create_ancillary_link(¬ifier->sd->entity, &sd->entity);\n>> +\n>> +#endif\n>> +\n>> +\treturn IS_ERR(link) ? PTR_ERR(link) : 0;\n>> +}\n>> +\n>> +/*\n>> + * Create links on behalf of the notifier and subdev, where it's obvious what\n>> + * should be done. At the moment, we only support cases where the notifier\n>> + * is a camera sensor and the subdev is a lens controller.\n> I think I'd rather change this so that ancillary links are created for lens\n> and flash subdevs, independently of the function of the notifier subdev.\n>\n> Are there cases where this would go wrong currently, or in the future? I\n> can't think of any right now at least. I guess we could add more\n> information in the future to convey here if needed.\nI don't think doing that would go wrong anyhow...at least not that I\ncould think of at the minute. My plan was to add a new function like\n__v4l2_async_create_data_links() and call that (from\nv4l2_async_try_create_links()) where the function of the notifier subdev\nwas MEDIA_ENT_F_VID_IF_BRIDGE...you think we shouldn't be doing that? Or\njust that it should be separate?\n>> + *\n>> + * TODO: Create data links if the notifier's function is\n>> + * MEDIA_ENT_F_VID_IF_BRIDGE and the subdev's is MEDIA_ENT_F_CAM_SENSOR.\n>> + */\n>> +static int v4l2_async_try_create_links(struct v4l2_async_notifier *notifier,\n>> +\t\t\t\t struct v4l2_subdev *sd)\n> How about calling this v4l2_async_create_ancillary_links()?\n\n\nSure\n\n>\n>> +{\n>> +#if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)\n>> +\n>> +\tif (!notifier->sd)\n>> +\t\treturn 0;\n>> +\n>> +\tswitch (notifier->sd->entity.function) {\n>> +\tcase MEDIA_ENT_F_CAM_SENSOR:\n>> +\t\treturn __v4l2_async_create_ancillary_link(notifier, sd);\n>> +\t}\n>> +\n>> +#endif\n>> +\treturn 0;\n>> +}\n>> +\n>> static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,\n>> \t\t\t\t struct v4l2_device *v4l2_dev,\n>> \t\t\t\t struct v4l2_subdev *sd,\n>> @@ -293,6 +337,18 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,\n>> \t\treturn ret;\n>> \t}\n>> \n>> +\t/*\n>> +\t * Depending of the function of the entities involved, we may want to\n>> +\t * create links between them (for example between a sensor and its lens\n>> +\t * or between a sensor's source pad and the connected device's sink\n>> +\t * pad).\n>> +\t */\n>> +\tret = v4l2_async_try_create_links(notifier, sd);\n>> +\tif (ret) {\n> The notifier's bound function has been called already. The unbound function\n> needs to be thus called here, too.\n\n\nThank you, I'll do that in the next version\n\n>\n>> +\t\tv4l2_device_unregister_subdev(sd);\n>> +\t\treturn ret;\n>> +\t}\n>> +\n>> \t/* Remove from the waiting list */\n>> \tlist_del(&asd->list);\n>> \tsd->asd = asd;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D11F3BDCBF\n\tfor ;\n\tWed, 2 Feb 2022 21:49:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1ABCE609E5;\n\tWed, 2 Feb 2022 22:49:00 +0100 (CET)","from mail-wr1-x42a.google.com (mail-wr1-x42a.google.com\n\t[IPv6:2a00:1450:4864:20::42a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5D347609B5\n\tfor ;\n\tWed, 2 Feb 2022 22:48:58 +0100 (CET)","by mail-wr1-x42a.google.com with SMTP id a13so1050559wrh.9\n\tfor ;\n\tWed, 02 Feb 2022 13:48:58 -0800 (PST)","from [192.168.0.14]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net. [86.13.91.161])\n\tby smtp.gmail.com with ESMTPSA id\n\tz6sm5497243wmf.37.2022.02.02.13.48.56\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tWed, 02 Feb 2022 13:48:57 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"B6+pONyw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=message-id:date:mime-version:user-agent:subject:content-language:to\n\t:cc:references:from:in-reply-to:content-transfer-encoding;\n\tbh=dKrNhjxZnRr3GwmN4wC37geXc+8FzlBlTKhpm5ZdPiA=;\n\tb=B6+pONywolt9Yo5cnElNiTpbB52ukKio34H/oBcksKnyE44krUh/rD+EVfAcOuQMgW\n\tY1G8D/DHqcxXdUo315NNC8tjgKpv9XjO+W2jthjZFmXJrmUX0wKKdl85JW7UhQogi5Au\n\tHqQa1TfTgh/Z56811acw1F3thoLTSYIPor9bb5o9t8stMNOeRNe3dPH0Av76F1vT8xhi\n\tItnFqmu9MesIfD9i5v1+snO1400MZjyR4o47WmzI2Xy2OioY7x/mhnOI+04RAun4D6pt\n\tJJ4OgscsFegmTOGCKWmPJ8mZqpObeluX6kMTyLzYDYWXl3XKpY4kElVwie/ztu5ZY4Ls\n\tjz5g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:message-id:date:mime-version:user-agent:subject\n\t:content-language:to:cc:references:from:in-reply-to\n\t:content-transfer-encoding;\n\tbh=dKrNhjxZnRr3GwmN4wC37geXc+8FzlBlTKhpm5ZdPiA=;\n\tb=GjhoYstuS2OlHAj3PQoMc4QygnA/IT5+PtUIy0Lof5HYSBe1q+o1OM6mCE5IzWZJdC\n\tKIEIaR/n24teJSsFfwaJ+ODsV4yHtHTGTcr/56FwVEv9XcArY+pdjGLjjCMVQZzE37PD\n\ts4vxPY2N3ls9ByrPaN4JLGu7Zca+EQ+qZRYteSgrkpxUsdO7lrpbakBEGLxxUoWu0yD1\n\tS5LthYSC46faos5GecJ21CRP6GU4pbWESNKeEEe2w4eM4Zkvbl86Gb9KvmMgFNbwi0yZ\n\tLuU+K0Hp1sCdjmQ1alftulLpXVgyNJeZ54DPcLLFRRJFF5lWF5cVj4wFhiwn5PdlXIdk\n\tk+rw==","X-Gm-Message-State":"AOAM531wbIHZfaYgWwR9vGCpgzPUPl65lMt/HkgeZXxPnMTeVz/qJSxt\n\taVlsxkKrfQ+fXZx5+SPtevU=","X-Google-Smtp-Source":"ABdhPJwKoeMKgAp8csdr5fFOVxx3u6CBPGynoORELFM56rZyMjvEZAQyu4d6h5m//kK9g3ZATpw2Ug==","X-Received":"by 2002:a5d:5352:: with SMTP id\n\tt18mr17353429wrv.247.1643838537796; \n\tWed, 02 Feb 2022 13:48:57 -0800 (PST)","Message-ID":"","Date":"Wed, 2 Feb 2022 21:48:56 +0000","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.5.0","Content-Language":"en-US","To":"Sakari Ailus ","References":"<20220130235821.48076-1-djrscally@gmail.com>\n\t<20220130235821.48076-7-djrscally@gmail.com>\n\t","From":"Daniel Scally ","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 6/6] media: v4l2-async: Create\n\tlinks during v4l2_async_match_notify()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, linux-media@vger.kernel.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22166,"web_url":"https://patchwork.libcamera.org/comment/22166/","msgid":"<5dbc76de-0802-3410-135e-be8e25d622be@gmail.com>","date":"2022-02-10T23:51:24","subject":"Re: [libcamera-devel] [PATCH v2 6/6] media: v4l2-async: Create\n\tlinks during v4l2_async_match_notify()","submitter":{"id":90,"url":"https://patchwork.libcamera.org/api/people/90/","name":"Daniel Scally","email":"djrscally@gmail.com"},"content":"Hi Sakari\n\nOn 02/02/2022 21:48, Daniel Scally wrote:\n> Hi Sakari\n>\n> On 02/02/2022 16:38, Sakari Ailus wrote:\n>> Hi Daniel,\n>>\n>> Thanks for the update.\n>>\n>> On Sun, Jan 30, 2022 at 11:58:21PM +0000, Daniel Scally wrote:\n>>> Upon an async fwnode match, there's some typical behaviour that the\n>>> notifier and matching subdev will want to do. For example, a notifier\n>>> representing a sensor matching to an async subdev representing its\n>>> VCM will want to create an ancillary link to expose that relationship\n>>> to userspace.\n>>>\n>>> To avoid lots of code in individual drivers, try to build these links\n>>> within v4l2 core.\n>>>\n>>> Signed-off-by: Daniel Scally \n>>> ---\n>>> Changes since v1:\n>>>\n>>> \t- Added #ifdef guards for CONFIG_MEDIA_CONTROLLER\n>>> \t- Some spelling and nomenclature cleanup (Laurent)\n>>>\n>>> Changes since the rfc:\n>>>\n>>> \t- None\n>>>\n>>> drivers/media/v4l2-core/v4l2-async.c | 56 ++++++++++++++++++++++++++++\n>>> 1 file changed, 56 insertions(+)\n>>>\n>>> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c\n>>> index 0404267f1ae4..8980534e755e 100644\n>>> --- a/drivers/media/v4l2-core/v4l2-async.c\n>>> +++ b/drivers/media/v4l2-core/v4l2-async.c\n>>> @@ -275,6 +275,50 @@ v4l2_async_nf_try_complete(struct v4l2_async_notifier *notifier)\n>>> static int\n>>> v4l2_async_nf_try_all_subdevs(struct v4l2_async_notifier *notifier);\n>>> \n>>> +static int\n>>> +__v4l2_async_create_ancillary_link(struct v4l2_async_notifier *notifier,\n>>> +\t\t\t\t struct v4l2_subdev *sd)\n>>> +{\n>>> +\tstruct media_link *link = NULL;\n>>> +\n>>> +#if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)\n>>> +\n>>> +\tif (sd->entity.function != MEDIA_ENT_F_LENS &&\n>>> +\t sd->entity.function != MEDIA_ENT_F_FLASH)\n>>> +\t\treturn -EINVAL;\n>>> +\n>>> +\tlink = media_create_ancillary_link(¬ifier->sd->entity, &sd->entity);\n>>> +\n>>> +#endif\n>>> +\n>>> +\treturn IS_ERR(link) ? PTR_ERR(link) : 0;\n>>> +}\n>>> +\n>>> +/*\n>>> + * Create links on behalf of the notifier and subdev, where it's obvious what\n>>> + * should be done. At the moment, we only support cases where the notifier\n>>> + * is a camera sensor and the subdev is a lens controller.\n>> I think I'd rather change this so that ancillary links are created for lens\n>> and flash subdevs, independently of the function of the notifier subdev.\n>>\n>> Are there cases where this would go wrong currently, or in the future? I\n>> can't think of any right now at least. I guess we could add more\n>> information in the future to convey here if needed.\n> I don't think doing that would go wrong anyhow...at least not that I\n> could think of at the minute. My plan was to add a new function like\n> __v4l2_async_create_data_links() and call that (from\n> v4l2_async_try_create_links()) where the function of the notifier subdev\n> was MEDIA_ENT_F_VID_IF_BRIDGE...you think we shouldn't be doing that? Or\n> just that it should be separate?\n\n\nGentle ping for this part :)\n\n>>> + *\n>>> + * TODO: Create data links if the notifier's function is\n>>> + * MEDIA_ENT_F_VID_IF_BRIDGE and the subdev's is MEDIA_ENT_F_CAM_SENSOR.\n>>> + */\n>>> +static int v4l2_async_try_create_links(struct v4l2_async_notifier *notifier,\n>>> +\t\t\t\t struct v4l2_subdev *sd)\n>> How about calling this v4l2_async_create_ancillary_links()?\n>\n> Sure\n>\n>>> +{\n>>> +#if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)\n>>> +\n>>> +\tif (!notifier->sd)\n>>> +\t\treturn 0;\n>>> +\n>>> +\tswitch (notifier->sd->entity.function) {\n>>> +\tcase MEDIA_ENT_F_CAM_SENSOR:\n>>> +\t\treturn __v4l2_async_create_ancillary_link(notifier, sd);\n>>> +\t}\n>>> +\n>>> +#endif\n>>> +\treturn 0;\n>>> +}\n>>> +\n>>> static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,\n>>> \t\t\t\t struct v4l2_device *v4l2_dev,\n>>> \t\t\t\t struct v4l2_subdev *sd,\n>>> @@ -293,6 +337,18 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,\n>>> \t\treturn ret;\n>>> \t}\n>>> \n>>> +\t/*\n>>> +\t * Depending of the function of the entities involved, we may want to\n>>> +\t * create links between them (for example between a sensor and its lens\n>>> +\t * or between a sensor's source pad and the connected device's sink\n>>> +\t * pad).\n>>> +\t */\n>>> +\tret = v4l2_async_try_create_links(notifier, sd);\n>>> +\tif (ret) {\n>> The notifier's bound function has been called already. The unbound function\n>> needs to be thus called here, too.\n>\n> Thank you, I'll do that in the next version\n>\n>>> +\t\tv4l2_device_unregister_subdev(sd);\n>>> +\t\treturn ret;\n>>> +\t}\n>>> +\n>>> \t/* Remove from the waiting list */\n>>> \tlist_del(&asd->list);\n>>> \tsd->asd = asd;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 59E20BDCBF\n\tfor ;\n\tThu, 10 Feb 2022 23:51:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7D9F2610F4;\n\tFri, 11 Feb 2022 00:51:29 +0100 (CET)","from mail-wr1-x434.google.com (mail-wr1-x434.google.com\n\t[IPv6:2a00:1450:4864:20::434])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5D0BE60E70\n\tfor ;\n\tFri, 11 Feb 2022 00:51:27 +0100 (CET)","by mail-wr1-x434.google.com with SMTP id e3so12401719wra.0\n\tfor ;\n\tThu, 10 Feb 2022 15:51:27 -0800 (PST)","from [192.168.0.14]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net. [86.13.91.161])\n\tby smtp.gmail.com with ESMTPSA id\n\tp8sm13669412wro.106.2022.02.10.15.51.25\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tThu, 10 Feb 2022 15:51:25 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"gjkb7DY0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=message-id:date:mime-version:user-agent:subject:content-language\n\t:from:to:cc:references:in-reply-to:content-transfer-encoding;\n\tbh=gkIX3A/FoIg8Nja/Sur+AvfZK5v27QkBAhmPJ3w3H4M=;\n\tb=gjkb7DY06yaMlZpeP/B/ZDbiVKrrzoA8UvaZX0b+zqP1ppPu7SIeKxcLV2TV8Y61a4\n\tOyjnXydrWQzbh2LJgzDMGWskhqdDpiOZY6AbKWfCjeepb0ch6Pc0X95EQsUhCzUaC3yM\n\tiXxc+frq3meP4zyEL3nGXgvlrfJ0aV1l/HLoPlkhxnZxKeZ62LjFQoMo8I6eJ1wwi3Pj\n\t2bzrtOdE9NrOvOTS8L6mweP2i1bdXjaDi9ZJwMINc0k9pxzzIHdNURcxXd5JBC65dgxa\n\t7T0Znp6v2nnhrPyhz6R0+bzmZxM0OAqSwKK7JrXepwtg48XSVGXkjUOz22XlNVZeTvvZ\n\t3D/g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:message-id:date:mime-version:user-agent:subject\n\t:content-language:from:to:cc:references:in-reply-to\n\t:content-transfer-encoding;\n\tbh=gkIX3A/FoIg8Nja/Sur+AvfZK5v27QkBAhmPJ3w3H4M=;\n\tb=2wdYTDAZGTloyLnfHxMWcxCAayy5sIDvAJULoHDLUi4ZKP0VEWcaSmYAxwp+Ddchr7\n\tPfwKeYZbL00sfg5fOfjgrX4F8jZTWHnckkxbBbtF0Tjgl4Hj+FGxs5hyop6SrwYyhz4K\n\tfnQpdjAKhXOBsGvcz8xphBx4Ihdu7a8LTGc5IctTqmMX4OVCLa6hLwU2BVyEx3XyTHJJ\n\tfaKm99F0alF2v+NYv9Chtb6yatzZZqjMp510Xygdy5nI0LS5Kp2nB3IZhPXDnTSfgIrM\n\tcbZKgG1sFT91izGeVCYu7YOaxu4+kOIW9wMBGegcS4zZGnb89QhJ3MfYiC5M1jhFfLOY\n\tW0UQ==","X-Gm-Message-State":"AOAM531SNJiSzb3C9UPfViQjdXjDyT9GOnewVWdB7cnhKKc6LJhXpChu\n\t5NVfvjE1z7f1Qtv4J96MWd0=","X-Google-Smtp-Source":"ABdhPJySnCheXL9DXTbAoK/BBR9+4B8W6qt2VMG7VFsJrQ13XjldN2yDF+dTrrbCkWsaktccjkBkDw==","X-Received":"by 2002:a05:6000:1545:: with SMTP id\n\t5mr8144555wry.443.1644537086853; \n\tThu, 10 Feb 2022 15:51:26 -0800 (PST)","Message-ID":"<5dbc76de-0802-3410-135e-be8e25d622be@gmail.com>","Date":"Thu, 10 Feb 2022 23:51:24 +0000","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.5.0","Content-Language":"en-US","From":"Daniel Scally ","To":"Sakari Ailus ","References":"<20220130235821.48076-1-djrscally@gmail.com>\n\t<20220130235821.48076-7-djrscally@gmail.com>\n\t\n\t","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 6/6] media: v4l2-async: Create\n\tlinks during v4l2_async_match_notify()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, linux-media@vger.kernel.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22169,"web_url":"https://patchwork.libcamera.org/comment/22169/","msgid":"","date":"2022-02-11T11:26:59","subject":"Re: [libcamera-devel] [PATCH v2 6/6] media: v4l2-async: Create\n\tlinks during v4l2_async_match_notify()","submitter":{"id":37,"url":"https://patchwork.libcamera.org/api/people/37/","name":"Sakari Ailus","email":"sakari.ailus@linux.intel.com"},"content":"Hi Daniel,\n\nThanks for the ping.\n\nOn Wed, Feb 02, 2022 at 09:48:56PM +0000, Daniel Scally wrote:\n> Hi Sakari\n> \n> On 02/02/2022 16:38, Sakari Ailus wrote:\n> > Hi Daniel,\n> >\n> > Thanks for the update.\n> >\n> > On Sun, Jan 30, 2022 at 11:58:21PM +0000, Daniel Scally wrote:\n> >> Upon an async fwnode match, there's some typical behaviour that the\n> >> notifier and matching subdev will want to do. For example, a notifier\n> >> representing a sensor matching to an async subdev representing its\n> >> VCM will want to create an ancillary link to expose that relationship\n> >> to userspace.\n> >>\n> >> To avoid lots of code in individual drivers, try to build these links\n> >> within v4l2 core.\n> >>\n> >> Signed-off-by: Daniel Scally \n> >> ---\n> >> Changes since v1:\n> >>\n> >> \t- Added #ifdef guards for CONFIG_MEDIA_CONTROLLER\n> >> \t- Some spelling and nomenclature cleanup (Laurent)\n> >>\n> >> Changes since the rfc:\n> >>\n> >> \t- None\n> >>\n> >> drivers/media/v4l2-core/v4l2-async.c | 56 ++++++++++++++++++++++++++++\n> >> 1 file changed, 56 insertions(+)\n> >>\n> >> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c\n> >> index 0404267f1ae4..8980534e755e 100644\n> >> --- a/drivers/media/v4l2-core/v4l2-async.c\n> >> +++ b/drivers/media/v4l2-core/v4l2-async.c\n> >> @@ -275,6 +275,50 @@ v4l2_async_nf_try_complete(struct v4l2_async_notifier *notifier)\n> >> static int\n> >> v4l2_async_nf_try_all_subdevs(struct v4l2_async_notifier *notifier);\n> >> \n> >> +static int\n> >> +__v4l2_async_create_ancillary_link(struct v4l2_async_notifier *notifier,\n> >> +\t\t\t\t struct v4l2_subdev *sd)\n> >> +{\n> >> +\tstruct media_link *link = NULL;\n> >> +\n> >> +#if IS_ENABLED(CONFIG_MEDIA_CONTROLLER)\n> >> +\n> >> +\tif (sd->entity.function != MEDIA_ENT_F_LENS &&\n> >> +\t sd->entity.function != MEDIA_ENT_F_FLASH)\n> >> +\t\treturn -EINVAL;\n> >> +\n> >> +\tlink = media_create_ancillary_link(¬ifier->sd->entity, &sd->entity);\n> >> +\n> >> +#endif\n> >> +\n> >> +\treturn IS_ERR(link) ? PTR_ERR(link) : 0;\n> >> +}\n> >> +\n> >> +/*\n> >> + * Create links on behalf of the notifier and subdev, where it's obvious what\n> >> + * should be done. At the moment, we only support cases where the notifier\n> >> + * is a camera sensor and the subdev is a lens controller.\n> > I think I'd rather change this so that ancillary links are created for lens\n> > and flash subdevs, independently of the function of the notifier subdev.\n> >\n> > Are there cases where this would go wrong currently, or in the future? I\n> > can't think of any right now at least. I guess we could add more\n> > information in the future to convey here if needed.\n> I don't think doing that would go wrong anyhow...at least not that I\n> could think of at the minute. My plan was to add a new function like\n> __v4l2_async_create_data_links() and call that (from\n> v4l2_async_try_create_links()) where the function of the notifier subdev\n> was MEDIA_ENT_F_VID_IF_BRIDGE...you think we shouldn't be doing that? Or\n> just that it should be separate?\n\nI'm not sure the function of the subdev should be involved with this.\n\nThe function is mainly used by the user space and different drivers tend to\nuse different functions. Of course there could (and should) be alignment on\nthis, but as you can have only a single function, there are bound to be\ncases where you have to pick one that fits the best but does not entirely\nmatch what the device is.\n\nI see no problem doing this automatically, as long as it does not clash\nwith what drivers create by themselves. The local pad where the data link\nis connected often comes from the driver --- same for the flags btw. --- so\nsome way needs to be provided for the driver to provide this information.\n\nThere's a callback for connecting endpoint with a pad the transmitter\ndrivers use, maybe that could be helpful?","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9C6A2BF415\n\tfor ;\n\tFri, 11 Feb 2022 11:27:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1B66B60201;\n\tFri, 11 Feb 2022 12:27:09 +0100 (CET)","from mga05.intel.com (mga05.intel.com [192.55.52.43])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1EC9E60200\n\tfor ;\n\tFri, 11 Feb 2022 12:27:05 +0100 (CET)","from fmsmga007.fm.intel.com ([10.253.24.52])\n\tby fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; \n\t11 Feb 2022 03:27:04 -0800","from punajuuri.fi.intel.com (HELO paasikivi.fi.intel.com)\n\t([10.237.72.43]) by fmsmga007-auth.fm.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Feb 2022 03:27:02 -0800","from paasikivi.fi.intel.com (localhost [127.0.0.1])\n\tby paasikivi.fi.intel.com (Postfix) with SMTP id F1BA4204FB;\n\tFri, 11 Feb 2022 13:26:59 +0200 (EET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=intel.com header.i=@intel.com\n\theader.b=\"HHmu15G0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=intel.com; i=@intel.com; q=dns/txt; s=Intel;\n\tt=1644578826; x=1676114826;\n\th=date:from:to:cc:subject:message-id:references:\n\tmime-version:in-reply-to;\n\tbh=DZ/aDg0txxoM2NpoFPjszz7fuQTABAltlA0Qlx5n7gs=;\n\tb=HHmu15G0Es40jtVdKaLGiRCeNH2sQyguqpeG+yF4RCDlduhK5S+w+SOo\n\twyYp0d1Q50ZgJ/IHYElAc3yBblz7sSXCBjxJEA8f9iECbMdrkjVh/3OR5\n\t49BE3C8dxpgpfPCGtDg1qUShwZ06GJPwxXiqV7yRGHVqVVqhw1IlHKTL2\n\te0nWXAN0drH5l/7/fmf8YXgtusdq3dSuZb/FPymGIpg/5pVYt5Igo0ywl\n\trVPpuoHXaExmnTly6Y3w2G/Z3Fu1V69u7R2UTitG3sEriMpsMlobwZo/Q\n\tYb17CTnvWfvPi85jT5XIe6elCSsmCEEJ/XynW3vnOUJI5G7C84CPi0Cnl A==;","X-IronPort-AV":["E=McAfee;i=\"6200,9189,10254\"; a=\"336137299\"","E=Sophos;i=\"5.88,360,1635231600\"; d=\"scan'208\";a=\"336137299\"","E=Sophos;i=\"5.88,360,1635231600\"; d=\"scan'208\";a=\"537624782\""],"Date":"Fri, 11 Feb 2022 13:26:59 +0200","From":"Sakari Ailus ","To":"Daniel Scally ","Message-ID":"","References":"<20220130235821.48076-1-djrscally@gmail.com>\n\t<20220130235821.48076-7-djrscally@gmail.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 6/6] media: v4l2-async: Create\n\tlinks during v4l2_async_match_notify()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, linux-media@vger.kernel.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]