Re: [PATCH v2 2/4] media: uvcvideo: Do not set an async control owned by other fh

[Hans de Goede]

Hi,

On 3-Dec-24 8:32 PM, Laurent Pinchart wrote:
> On Mon, Dec 02, 2024 at 02:29:24PM +0100, Ricardo Ribalda wrote:
>> On Mon, 2 Dec 2024 at 13:19, Hans de Goede wrote:
>>> On 2-Dec-24 11:50 AM, Ricardo Ribalda wrote:
>>>> On Mon, 2 Dec 2024 at 11:27, Hans de Goede wrote:

<snip>

>>>>> Note that if we simply return -EBUSY on set until acked by a status
>>>>> event we also avoid the issue of ctrl->handle getting overwritten,
>>>>> but that relies on reliable status events; or requires timeout handling.
>>>>>
>>>>> 3. I agree with Ricardo that a timeout based approach for cameras which
>>>>> to not properly send status events for async ctrls is going to be
>>>>> problematic. Things like pan/tilt homing can take multiple seconds which
>>>>> is really long to use as a timeout if we plan to return -EBUSY until
>>>>> the timeout triggers. I think it would be better to just rely on
>>>>> the hardware sending a stall, or it accepting and correctly handling
>>>>> a new CUR_SET command while the previous one is still being processed.
>>>>>
>>>>> I guess we can track if the hw does send status events when async ctrls
>>>>> complete and then do the -EBUSY thing without going out to the hw after
>>>>> the first time an async ctrl has been acked by a status event.
> 
> That sounds quite complex, and wouldn't guard against the status event
> being occasionally lost. I'm more concerned about devices that only
> occasionally mess up sending the status event, not devices that never
> send it.

I did wonder if we would see devices where the status event is
occasionally lost.

I think that patches 1-4 of "[PATCH v6 0/5] media: uvcvideo: Two +1 fixes for
async controls" solves the issue at hand nicely, so lets go with that.

And I hereby withdraw my proposal for a per ctrl flag to track if
we get status events for that control, because as you say that will
not work in the case where the status events are missing some of
the time (rather then the status events simply being never send).

Regards,

Hans