Received-SPF: pass (google.com: domain of linux-kernel+bounces-93826-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <829d07c8-b602-4463-9f8e-dc8eb73aa077@redhat.com>
Date: Wed, 6 Mar 2024 12:01:19 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4] platform/x86: add lenovo generic wmi driver
Content-Language: en-US, nl
To: =?UTF-8?B?6Im+6LaF?= <aichao@kylinos.cn>,
 =?UTF-8?Q?Thomas_Wei=C3=9Fschuh?= <thomas.weissschuh@linutronix.de>,
 W_Armin <W_Armin@gmx.de>
Cc: "ilpo.jarvinen" <ilpo.jarvinen@linux.intel.com>,
 linux-kernel <linux-kernel@vger.kernel.org>,
 platform-driver-x86 <platform-driver-x86@vger.kernel.org>
References: <2o0aznm5pjb-2o0c9lfkrd4@nsmail7.0.0--kylin--1>
From: Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <2o0aznm5pjb-2o0c9lfkrd4@nsmail7.0.0--kylin--1>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Hi,

On 3/6/24 09:17, 艾超 wrote:
> Hi
> 
>  
> 
>> I would be interested on which devices this driver was tested and is
>> expected to work with.
> 
>  
> 
> Lenovo A70，it is a Computer integrated machine.
> 
> 
>>> This looks similar to a switch.
>>> Would it be more useful for the user to report a standard switch instead
>>> of a key event which needs to be correlated with the sysfs file?
> 
>> I agree, maybe SW_CAMERA_LENS_COVER might be the right thing to use here,
>> if those camera states (open/closed) are meant to symbolize camera shutter states.
> 
>> In such a case the initial switch state has to be retrieved, or else the input device
>> cannot be registered until the first event is received (similar how the hp-wmi driver
>> handles SW_CAMERA_LENS_COVER events).
> 
>> Ai Chao, can you tell us if those two camera states are meant to act like a switch (camera switched off,
>> camera switched on) or meant to act like a key (camera button pressed, camera button released)?
> 
>  
> 
> The camera button is like a switch.  I can used SW_CAMERA_LENS_COVER to report input event , but the OS
> 
>  can't to show camera OSD. OS need a key value  map to the camera OSD.

Please use SW_CAMERA_LENS_COVER in the next version, that really is the correct
thing to do here, so that you not only report the event of the camera state
changing, but also the actual camera state (on/off) to userspace.

This will also allow you to remove the custom sysfs atribute since you
are now correctly reporting the value to userspace through the evdev event.

As for userspace not responding to SW_CAMERA_LENS_COVER, this is because
SW_CAMERA_LENS_COVER is relatively new and we need to add support to
userspace for this. SW_CAMERA_LENS_COVER is already used on whole a bunch of
Dell laptops for this, so we need to do this anyways.

Please file an issue against GNOME, lets say against gnome-settings-daemon
for this.

Regards,

Hans