Re: [PATCH v3 2/8] media: uvcvideo: Factor out gpio functions to its own file

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 25-Nov-24 4:10 PM, Ricardo Ribalda wrote:
> On Mon, 25 Nov 2024 at 15:45, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Ricardo,
>>
>> On 12-Nov-24 6:30 PM, Ricardo Ribalda wrote:
>>> This is just a refactor patch, no new functionality is added.
>>>
>>> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
>>
>> I guess this patch may need to change depending on if we want
>> to keep the GPIO handling as a UVC entity or not.
> 
> I have a v4 that removes the virtual uvc entity. Still with that
> approach IMHO this patch makes sense.

Right I was not suggesting dropping it, but atm it is moving
the uvc entity creation around. I was wondering if that
moving around before removing it still makes sense ?

Note I have no preference either way if first moving it
and then dropping it is cleaner, or just easier because
of the history of this patch-set then IMHO either way
is fine.

> I was planning to send it today, I am testing it right now.
> 
>>
>> Laurent what is your take on this, should this stay as
>> a struct uvc_entity; or should the gpio_desc and input_device
>> be stored directly inside struct uvc_device as is done for
>> the snapshot-button input_device?
>>
>>
>> Also de we want a separate input_device for this or do
>> we re-use the snapshot button one ?
>>
>> Since my plan is to open-up the permission on the device with
>> the SW_CAMERA_LENS_COVER to be equal to the /dev/video#
>> permissions sharing has the downside of allowing keylogging
>> of the snapshot button.
> 
> A downside of having 2 devices is that userspace will have to either
> figure out what evdev they want to use or listen to both....

Right, so both would be picked up by the compositor and the
snapshot button is just another multimedia-key then, while
the compositor can use SW_CAMERA_LENS_COVER for its OSD.

The other would be of interest to libcamera.

I'm think we may need a naming convention for the evdev
with SW_CAMERA_LENS_COVER something like "* camera privacy"
or whatever then we can have a udev rule matching on that for
uaccess + libcamera can do a strstr on the name for
"camera privacy" and ignore other evdevs.

Regards,

Hans