[PATCH] media: rkisp1: Don't create data links for non-sensor subdevs

[Daniel Scally]

Hi Jacopo

On 07/06/2022 17:41, Jacopo Mondi wrote:
> Hi Dan
>
> On Mon, Jun 06, 2022 at 11:51:49PM +0100, Daniel Scally wrote:
>> With the introduction of ancillary links, not all subdevs linked to
>> the ISP's v4l2_dev necessarily represent sensors / bridges. Check the
>> function for the subdevs and skip any that represent lens or flash
>> controllers before creating data links.
>>
>> Signed-off-by: Daniel Scally <djrscally at gmail.com>
>> ---
>>
>> This should fix the issues that have been noticed, but perhaps a new flag like
>> MEDIA_ENT_FL_HAS_SOURCE or something would be a better way to denote subdevs
>> that need data links?
>>
> I agree this a bit fragile...
>
> I noticed ancillary links are only created for subdev notifiers,
> which have a populated 'sd' and consequentially an entity. Could an
> helper that walks the links of the notifier's subdev links and checks
> if the subdev at hand is already linked, help ? Maybe with an optional
> set of link flags to match on ?


Or maybe just check if the subdev's notifier is the same as the rkisp1's
notifier? Like:


if(sd->notifier!= &rkisp1->notifier)
continue
That's a bit less clunky than both other solutions I think
>
>
>>  drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
>> index 3f5cfa7eb937..e90f0216cb06 100644
>> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
>> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
>> @@ -134,6 +134,10 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1)
>>  		    sd == &rkisp1->resizer_devs[RKISP1_SELFPATH].sd)
>>  			continue;
>>
>> +		if (sd->entity.function == MEDIA_ENT_F_LENS ||
>> +		    sd->entity.function == MEDIA_ENT_F_FLASH)
>> +			continue;
>> +
>>  		ret = media_entity_get_fwnode_pad(&sd->entity, sd->fwnode,
>>  						  MEDIA_PAD_FL_SOURCE);
>>  		if (ret < 0) {
>> --
>> 2.25.1
>>