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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Daniel,

On 23.09.2022 01:19, Daniel Scally wrote:
> On 22/09/2022 23:48, Sakari Ailus wrote:
>> 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:
>>> On 21.03.2022 15:51, Laurent Pinchart wrote:
>>>> From: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
>>>>
>>>> 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@xxxxxxxxxxxxxxxx>
>>> 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 :)

Good catch, thanks!

I have no idea why that loop compared the asd->match.fwnode and 
subdev->dev->of_node, maybe it some kind of historical left-over or 
cargo-cult. The asd pointer is already available there and can be used 
to find the proper sensor. This loop works both with the new and old 
version:

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

I will prepare a patch in a few minutes.

>>> 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.
>>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux