linux-kernel - Re: [PATCH][v2] asus-rbtn: new driver for asus radio button for Windows 8

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150630161710.GA61463@vmdeb7>
Date:	Tue, 30 Jun 2015 09:17:10 -0700
From:	Darren Hart <dvhart@...radead.org>
To:	Pali Rohár <pali.rohar@...il.com>
Cc:	Alex Hung <alex.hung@...onical.com>,
	Corentin Chary <corentin.chary@...il.com>,
	"platform-driver-x86@...r.kernel.org" 
	<platform-driver-x86@...r.kernel.org>,
	acpi4asus-user@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH][v2] asus-rbtn: new driver for asus radio button for
 Windows 8

On Mon, Jun 29, 2015 at 02:29:53PM +0200, Pali Rohár wrote:
> On Friday 26 June 2015 23:24:10 Alex Hung wrote:
> > On Fri, Jun 26, 2015 at 10:56 PM, Pali Rohár <pali.rohar@...il.com> wrote:
> > > Hi!
> > >
> > > On Wednesday 24 June 2015 10:57:51 Alex Hung wrote:
> > >> ASUS introduced a new approach to handle wireless hotkey
> > >> since Windows 8.  When the hotkey is pressed, BIOS generates
> > >> a notification 0x88 to a new ACPI device, ATK4001.  This
> > >> new driver not only translates the notification to KEY_RFKILL
> > >> but also toggles its LED accordingly.
> > >>
> > >> Signed-off-by: Alex Hung <alex.hung@...onical.com>
> > >
> > > ...
> > >
> > >> +static int asus_radio_led_set(bool blocked)
> > >> +{
> > >> +     acpi_status status;
> > >> +     union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> > >> +     struct acpi_object_list args = { 1, &arg0 };
> > >> +     unsigned long long output;
> > >> +
> > >> +     arg0.integer.value = blocked;
> > >> +     status = acpi_evaluate_integer(asus_rbtn_device->handle, "HSWC",
> > >> +                                    &args, &output);
> > >
> > > What is this ACPI call doing? Just set LED control? Or something more?
> > >
> > >> +     if (!ACPI_SUCCESS(status) || output == 0) {
> > >> +             pr_err("fail to change wireless LED.\n");
> > >> +             return -EINVAL;
> > >> +     }
> > >> +
> > >> +     return 0;
> > >> +}
> > >> +
> > >> +static int asus_rfkill_set(void *data, bool blocked)
> > >> +{
> > >> +     radio_led_state = blocked ? 0 : 1;
> > >> +
> > >> +     return asus_radio_led_set(radio_led_state);
> > >> +}
> > >
> > > In my opinion this is not good idea that "rfkill block" call from
> > > userspace just change LED on/off state and nothing more...
> > >
> > > If above ACPI call just change LED, then should not be this in LED
> > > subsystem instead rfkill one? Or why do you prefer to use rfkill
> > > interface instead led?
> > 
> > It indeed controls LED only at the moment.  My intention was to have
> > have everything work without the need to modify any userspace
> > applications.  Current it is 1) aus-rbtn issues KEY_RFKILL 2) an
> > userspace application changes rfkill states, and 3) both radio and LED
> > work.  It will also work when a user enable/disable wireless devices
> > on a user application which uses rfkill interface.
> > 
> > Come to think about it now, I may have to handle LED with WLAN and BT
> > but I will have to find a system with both devices later.
> > 
> > I am not too familiar with userspace applications v.s. LED.  Is it
> > possible to do the same (i.e. without touching userspace)? I think
> > rfkill is good interface to handle whatever needs doing when changing
> > wireless states, such as LED controls.  However, if other approach can
> > meet the need I am happy to investigate.
> > 
> 
> There are triggers for led which automatically enable/disable led. I
> think that configuring default wifi/bluetooth trigger for that new led
> could work...
> 

I agree with Pali. If all we're doing is changing LED state, this sounds like a
job for a LEDs trigger.

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/