Re: [PATCH v2 1/3] platform: x86: dell-rbtn: Dell Airplane Mode Switch driver

[Gabriele Mazzotta <gabriele.mzt@gmail.com>]

On Thursday 30 April 2015 09:44:29 Pali Rohár wrote:
> On Thursday 30 April 2015 14:06:27 Alex Hung wrote:
> > Method ABRT is to be used by driver to disable BIOS handling of radio
> > button. So the changes in behaviours observed by Gabriele is expected.
> > I have seen other systems behave the same way.
> > 
> 
> Right, that after that ARBT call operating system get full control over
> radio devices and ACPI/BIOS will not automatically enable/disable them.
> I think this is OK.
> 
> But for that we need also support for manually enable/disable radio
> devices and code for this support is missing. Or do DELLABCE/RBTN acpi
> devices somehow support enabling/disabling it via system/kernel request?
> 
> > I do also see firmware only sends Notify(RBTN, 0x80) and no hard block
> > whether ABRT(1) is called or not.  Thus keycode are the only option on
> > those machines.
> > 
> 
> Key is ok, but we *must* have ability to hard block it via some
> ACPI/WMI/BIOS/FW/etc... call. Otherwise ARBT(1) is no go as users should
> be able to enable/disable their radio devices (bluetooth for powersave)

Does it really matter in the end? As I understand it, radio devices are
off either way.

> > The idea to have an option (kernel parameter) for calling ABRT is
> > great. I can help verify on more machines. Is Gabriele's patch above a
> > final version that I should test?
> > 
> 
> No, I do not think so. This looks like hack or pure design. Kernel
> option could be there, but just for buggy BIOSes (and future changed
> design).
> 
> But now it looks like for correct work is specifying that param
> required -- which is bad.
> 
> Alex, can you ask Dell people how should system turn off e.g bluetooth
> or wifi device if ARTB(1) is called and system/kernel (instead ACPI) is
> expected to turn off/on blueooth (and wifi) devices?
> 
> I think that without this information (and working driver for it) we
> should not enable ARTB(1) or including this driver into kernel tree as
> it will break some existing machines...

I disagree. If we skip ARBT(1), userspace will get confused on laptops
like mine as it would receive two events: an rfkill event (because of
the BIOS shutting down radios) and a keypress (because of Alex's patch).
If we call ARBT(1), userspace will only receive the keypress and
userspace will correctly handle the devices.

That's why I suggested the use of a kernel parameter. It was just a way
to allow users to choose one mode over another when they know they're
both available. It's not something users are required to do.

I believe that in the end what really matters is that when function
keys are pressed something happens, be it the BIOS turning off radio
devices or the kernel telling userspace to do it, but not both.

> Darren, I do not know what is better, but it looks like that some dell
> machines working fine now and some not (since begining). And after
> loading this driver some machines are fixed, but some which worked are
> broken... What do you think as maintainer?