Re: [PATCH 2/3] platform/x86: dell-wmi: add new keymap type 0x0012

[Sebastian Reichel <sebastian.reichel@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 3620 bytes --]

Hi,

On Tue, Jun 09, 2020 at 06:59:13PM +0200, Hans de Goede wrote:
> Hi Sebastian,
> 
> On 6/9/20 6:45 PM, Sebastian Reichel wrote:
> > Hi,
> > 
> > On Tue, Jun 09, 2020 at 05:49:38PM +0200, Pali Rohár wrote:
> > > On Monday 08 June 2020 20:36:58 Mario.Limonciello@dell.com wrote:
> > > > Can you please comment here how you would like to see events like this should come
> > > > through to userspace?
> > > > 
> > > > * Wrong power adapter (you have X and should have Y)
> > > > * You have plugged a dock into the wrong port
> > > > * Fn-lock mode
> > > 
> > > In my opinion, Fn-lock mode is related to input subsystem and should be
> > > probably reported via input device. For a user, fn-lock is similar like
> > > caps-lock, scroll-lock or num-lock. Also fn-lock is provided by other
> > > laptops and therefore I would expect that kernel provide fn-lock state
> > > for all laptops (thinkpad / latitude / ...) via same interface. And not
> > > via dell-specific interface or general-vendor-message interface.
> > > 
> > > Wrong power adapter sounds like something related to power subsystem.
> > > Adding Sebastian to the loop, maybe he can provide some useful ideas
> > > about it.
> > 
> > I'm missing a bit of context. I suppose this is about connecting a
> > non-genuine power adapter rejected by the embedded controller?
> > Support for that should be hooked into 'drivers/acpi/ac.c' (note:
> > so far there is no standard power-supply class property for this).
> > Also printing a warning to dmesg seems sensible.
> 
> This is not so much about non-genuine as about the adapter having
> the wrong wattage. E.g. plugging a 65W adapter in a laptop which
> has a 45W CPU + a 35W discrete GPU will not allow the laptop to
> really charge while it is in use.

Ok. So how much information is available over WMI? Exposing the
max. input power via the power-supply API makes sense in any case.

> One issue I see with doing this in the power_supply class is that
> the charger is represented by the standard ACPI AC adapter stuff,
> which does not have this info. This sort of extra info comes from
> WMI. Now we could have the WMI driver register a second power_supply
> device, but that means having 2 power_supply devices representing
> the 1 AC adapter which does not feel right.

I agree. WMI and ACPI information need to be merged and exposed
as one device to userspace. It's not the first time we have this
kind of requirement, but so far it was about merging battery info
from two places. Unfortunately no code has been written so far to
support this.

> I was myself actually thinking more along the lines of adding a
> new mechanism to the kernel where the kernel can send messages
> to userspace (either with some special tag inside dmesg, or through
> a new mechanism) indication that the message should be shown
> as a notification (dialog/bubble/whatever) inside the UI.
>
> This could be useful for this adapter case, but e.g. also for
> pluging a thunderbolt device into a non thunderbolt capable
> Type-C port, a superspeed USB device into a USB-2 only USB
> port and probably other cases.
> 
> Rather then inventing separate userspace APIs for all these
> cases having a general notification mechanism might be
> quite useful for this (as long as the kernel does not
> over use it).

I don't think this is a good idea. It brings all kind of
localization problems. Also the information is not machine
parsable. It looks more like a hack to get things working
quickly by avoiding using/designing proper APIs.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]