Re: [PATCH] media: v4l2-async: Use endpoints in __v4l2_async_nf_add_fwnode_remote()

[Daniel Scally <djrscally@gmail.com>]

Hi all

On 22/09/2022 23:48, Sakari Ailus wrote:
> Hi, Marek!
>
> Good to hear from you! And it's so nice you're testing the Samsung Exynos
> drivers! :-)
>
> On Thu, Sep 22, 2022 at 04:06:58PM +0200, Marek Szyprowski wrote:
>> Hi Laurent,
>>
>> On 21.03.2022 15:51, Laurent Pinchart wrote:
>>> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>>
>>> Matching on device fwnode handles is deprecated in favour of endpoint
>>> fwnode handles. Switch the __v4l2_async_nf_add_fwnode_remote() function
>>> to use the latter. The match code handles backward compatibility by
>>> falling by to the device fwnode handle, so this shouldn't introduce any
>>> regression.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> I love the last sentence of the patch description. :)
>>
>> Unfortunately, recently I found that this patch breaks Exynos4 IS (FIMC)
>> driver operation on Trats2 board (exynos4412-trats2.dts). After merging
>> this patch I see the following errors related to the camera sensors:
>>
>> [   16.038705] S5C73M3: S5C73M3 SPI probed successfully
>> [   16.097399] S5C73M3: Sensor type: CML0801-M017, FW version: GDFD01
>> [   16.106842] S5C73M3 0-003c: Consider updating driver S5C73M3 to match
>> on endpoints
>> [   16.298323] S5C73M3: probe of 0-003c failed with error -22
>> [   16.343173] S5K6A3 15-0010: Consider updating driver S5K6A3 to match
>> on endpoints
>> [   16.434968] S5K6A3: probe of 15-0010 failed with error -22
> Have you checked what exactly caused the probe to fail? Laurent's patch
> changes how matching works but if that fails, the result should be a bunch
> of waiting async sub-devices and notifier(s), not a failure on probe.


Might be some other effects too...I can't test it but in 
subdev_notifier_bound() in 
drivers/media/platform/samsung/exynos4-is/media-dev.c there's the 
following check:


     /* Find platform data for this sensor subdev */
     for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++)
         if (fmd->sensor[i].asd &&
             fmd->sensor[i].asd->match.fwnode ==
             of_fwnode_handle(subdev->dev->of_node))
             si = &fmd->sensor[i];

     if (si == NULL)
         return -EINVAL;


And I think following that patch of Laurent's asd->match.fwnode will 
never be the dev->of_node anymore, because it's now the endpoint that's 
assigned as asd->match.fwnode rather than that of the device. That will 
return -EINVAL for the notifier's .bound() callback...I'm not sure if 
that would cause the whole probe to fail, but thought it might be worth 
mentioning :)

>
>> I'm a bit puzzled, because I don't see anything related to endpoint
>> matching in the sensor drivers. Instead I only found that
>> v4l2_async_nf_add_fwnode_remote() function (which is modified by this
>> patch) is called from the
>> drivers/media/platform/samsung/exynos4-is/media-dev.c code. Do you have
>> any idea what is broken after this change? Could you help fixing this issue?
> You can assign the endpoint node to subdev->fwnode instead of the device
> fwnode. No regular sensor driver appears to be doing that though.
>