diff mbox

sony-laptop: support rfkill via ACPI interfaces

Message ID 20090319212123.GA24700@srcf.ucam.org (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Matthew Garrett March 19, 2009, 9:21 p.m. UTC
Enable events on all Vaio's with the new-style ACPI interface, and use
it to support rfkill where available.
    
Signed-off-by: Matthew Garrett <mjg@redhat.com>

---

Mattia, I didn't want to mess with the DMI stuff, but I suspect we can 
probably drop the setup callbacks there and just enable them on all 
hardware that supports these methods. We possibly also want to be 
calling the ECON method on the SNC since some codepaths in the tables 
seem to depend on them - but I'm also worried to a certain extent on how 
much that might change driver interactions with some machines.

Comments

Norbert Preining March 19, 2009, 9:34 p.m. UTC | #1
HI Matthew,

On Do, 19 Mär 2009, Matthew Garrett wrote:
> Enable events on all Vaio's with the new-style ACPI interface, and use
> it to support rfkill where available.

Thanks for these patches, do they enable wwan activation on Vaio Z
series, too?

Currently the stock kernel sony-laptop with all patches floating around
on lkml did not make the wwan interface show up from the turned off
state. The only way to do that is using the 0.6 version for Vaio Z from
http://www.basyskom.org/~eva/log_installation_vaio_z21vnx.html

Thanks and all the best

Norbert

-------------------------------------------------------------------------------
Dr. Norbert Preining <preining@logic.at>        Vienna University of Technology
Debian Developer <preining@debian.org>                         Debian TeX Group
gpg DSA: 0x09C5B094      fp: 14DF 2E6C 0307 BE6D AD76  A9C0 D2BF 4AA3 09C5 B094
-------------------------------------------------------------------------------
BAUGHURST
That kind of large fierce ugly woman who owns a small fierce ugly dog.
			--- Douglas Adams, The Meaning of Liff
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthew Garrett March 19, 2009, 9:44 p.m. UTC | #2
On Thu, Mar 19, 2009 at 10:34:28PM +0100, Norbert Preining wrote:
> HI Matthew,
> 
> On Do, 19 Mär 2009, Matthew Garrett wrote:
> > Enable events on all Vaio's with the new-style ACPI interface, and use
> > it to support rfkill where available.
> 
> Thanks for these patches, do they enable wwan activation on Vaio Z
> series, too?

I expect so.

> Currently the stock kernel sony-laptop with all patches floating around
> on lkml did not make the wwan interface show up from the turned off
> state. The only way to do that is using the 0.6 version for Vaio Z from
> http://www.basyskom.org/~eva/log_installation_vaio_z21vnx.html

Sigh. I could have saved some amount of time if these patches had 
actually been sent to the mailing list. They're functionally equivalent, 
except mine integrate with the rfkill class correctly and check whether 
the hardware is present before creating the rfkill device.
Norbert Preining March 19, 2009, 9:49 p.m. UTC | #3
On Do, 19 Mär 2009, Matthew Garrett wrote:
> Sigh. I could have saved some amount of time if these patches had 
> actually been sent to the mailing list. They're functionally equivalent, 

Umpf, bad.

> except mine integrate with the rfkill class correctly and check whether 
> the hardware is present before creating the rfkill device.

I found that out only recently that there is this module floating
around.

Ok, I will give your patches a testing spin on my Vaio Z11.

One question: Should I apply the patches Mattia posted some time ago 
(9 of them if I remember correctly) and your patch on top of them, or
apply your patch to current -rc8?

Best wishes

Norbert

-------------------------------------------------------------------------------
Dr. Norbert Preining <preining@logic.at>        Vienna University of Technology
Debian Developer <preining@debian.org>                         Debian TeX Group
gpg DSA: 0x09C5B094      fp: 14DF 2E6C 0307 BE6D AD76  A9C0 D2BF 4AA3 09C5 B094
-------------------------------------------------------------------------------
DITHERINGTON (n)
Sudden access to panic experienced by one who realises that he is
being drawn inexorably into a clabby (q.v.) conversation, i.e. one he
has no hope of enjoying, benefiting from or understanding.
			--- Douglas Adams, The Meaning of Liff
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthew Garrett March 19, 2009, 9:56 p.m. UTC | #4
On Thu, Mar 19, 2009 at 10:49:14PM +0100, Norbert Preining wrote:

> One question: Should I apply the patches Mattia posted some time ago 
> (9 of them if I remember correctly) and your patch on top of them, or
> apply your patch to current -rc8?

Mine doesn't depend on any of Mattia's patches, but should work with 
them.
Norbert Preining March 19, 2009, 10:15 p.m. UTC | #5
Hi Matthew,

On Do, 19 Mär 2009, Matthew Garrett wrote:
> > > Enable events on all Vaio's with the new-style ACPI interface, and use
> > > it to support rfkill where available.
> > 
> > Thanks for these patches, do they enable wwan activation on Vaio Z
> > series, too?
> 
> I expect so.

Ok, here some first test result:
kernel 2.6.29-rc8 plus only your patch.

Nice.

Now I have loads of rfkill switches:
/sys/class/rfkill# for i in rfkill*/name ; do echo $i; cat $i; done
rfkill10/name
5100AGN
rfkill11/name
sony-wifi
rfkill12/name
sony-bluetooth
rfkill13/name
sony-wwan
rfkill15/name
hso-0


The interesting thing is that the last one hso-0 does not do anything
when echoing 0/1 onto the state, while echoing 0/1 onto rfkill13/state
the device is actually dis/enabled.

Fine.

Especially since I can now also turn on and off the bluetooth device.
Great.

Thanks a lot!

Best wishes

Norbert

-------------------------------------------------------------------------------
Dr. Norbert Preining <preining@logic.at>        Vienna University of Technology
Debian Developer <preining@debian.org>                         Debian TeX Group
gpg DSA: 0x09C5B094      fp: 14DF 2E6C 0307 BE6D AD76  A9C0 D2BF 4AA3 09C5 B094
-------------------------------------------------------------------------------
CLACKMANNAN (n.)
The sound made by knocking over an elephant's-foot umbrella stand full
of walking sticks. Hence name for a particular kind of disco drum
riff.
			--- Douglas Adams, The Meaning of Liff
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Norbert Preining March 20, 2009, 12:28 a.m. UTC | #6
On Do, 19 Mär 2009, Norbert Preining wrote:
> Ok, here some first test result:
> kernel 2.6.29-rc8 plus only your patch.

Another result, but this time a bit negative: Your patch does not (well
I know) incorporate the stuff turning off the secondary nvidia card on
the device having Stamina-Speed switch. 

Outcome of that is that the power consumption is much higher with your
modules about 8-10W (measured with powertop). So I have to go back to
the other module.

Best wishes

Norbert

-------------------------------------------------------------------------------
Dr. Norbert Preining <preining@logic.at>        Vienna University of Technology
Debian Developer <preining@debian.org>                         Debian TeX Group
gpg DSA: 0x09C5B094      fp: 14DF 2E6C 0307 BE6D AD76  A9C0 D2BF 4AA3 09C5 B094
-------------------------------------------------------------------------------
`That young girl is one of the least benightedly
unintelligent organic life forms it has been my profound
lack of pleasure not to be able to avoid meeting.'
                 --- Marvin's first ever compliment about anybody.
                 --- Douglas Adams, The Hitchhikers Guide to the Galaxy
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthew Garrett March 20, 2009, 12:38 a.m. UTC | #7
On Fri, Mar 20, 2009 at 01:28:27AM +0100, Norbert Preining wrote:
> On Do, 19 Mär 2009, Norbert Preining wrote:
> > Ok, here some first test result:
> > kernel 2.6.29-rc8 plus only your patch.
> 
> Another result, but this time a bit negative: Your patch does not (well
> I know) incorporate the stuff turning off the secondary nvidia card on
> the device having Stamina-Speed switch. 

As far as we can tell, the handling of the handover is consistent across 
all dual-GPU nvidia laptops[1]. The right place for support to end up is 
in the kernel component of the nouveau drivers.

[1] Possibly not the Macs
Norbert Preining March 20, 2009, 12:40 a.m. UTC | #8
On Fr, 20 Mär 2009, Matthew Garrett wrote:
> As far as we can tell, the handling of the handover is consistent across 
> all dual-GPU nvidia laptops[1]. The right place for support to end up is 
> in the kernel component of the nouveau drivers.

Whatever the nouveau drivers are? You mean those based on the kernel
mode swching? Unfortunately that is far from prime time, and we need
power saving now.

I will take a look over the weekend maybe I can add a patch ontop of
yours that only does that, taken from the code in the special vaio-Z
sony-laptop module.

Best wishes

Norbert

-------------------------------------------------------------------------------
Dr. Norbert Preining <preining@logic.at>        Vienna University of Technology
Debian Developer <preining@debian.org>                         Debian TeX Group
gpg DSA: 0x09C5B094      fp: 14DF 2E6C 0307 BE6D AD76  A9C0 D2BF 4AA3 09C5 B094
-------------------------------------------------------------------------------
BODMIN
The irrational and inevitable discrepancy between the amount pooled
and the amount needed when a large group of people try to pay a bill
together after a meal.
			--- Douglas Adams, The Meaning of Liff
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mattia Dongili March 20, 2009, 8:52 a.m. UTC | #9
On Thu, Mar 19, 2009 at 09:21:23PM +0000, Matthew Garrett wrote:
> Enable events on all Vaio's with the new-style ACPI interface, and use
> it to support rfkill where available.
>     
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> 
> ---
> 
> Mattia, I didn't want to mess with the DMI stuff, but I suspect we can 
> probably drop the setup callbacks there and just enable them on all 
> hardware that supports these methods. We possibly also want to be 

I'm more of the idea to provide a module option to force the setup
callback if the module is not in the DMI list.
Although for now all of the models that have SN07 and friends seem to
benefit from throwing some magic numbers at them.

> calling the ECON method on the SNC since some codepaths in the tables 
> seem to depend on them - but I'm also worried to a certain extent on how 
> much that might change driver interactions with some machines.

My understanding about ECON is that it is always enabled if the embedded
controller is enabled. The SPIC device has the same kind of dependency
and as far as I could see ECON is always 1. So I don't think it makes
much of a difference.

A couple of comments below.

Thanks

> diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
> index 537959d..c57f54c 100644
> --- a/drivers/platform/x86/sony-laptop.c
> +++ b/drivers/platform/x86/sony-laptop.c
> @@ -64,6 +64,7 @@
>  #include <asm/uaccess.h>
>  #include <linux/sonypi.h>
>  #include <linux/sony-laptop.h>
> +#include <linux/rfkill.h>
>  #ifdef CONFIG_SONYPI_COMPAT
>  #include <linux/poll.h>
>  #include <linux/miscdevice.h>
> @@ -143,6 +144,11 @@ struct sony_laptop_keypress {
>  	int key;
>  };
>  
> +static struct rfkill *sony_wifi_rfkill;
> +static struct rfkill *sony_bluetooth_rfkill;
> +static struct rfkill *sony_wwan_rfkill;
> +static struct rfkill *sony_wimax_rfkill;
> +
>  /* Correspondance table between sonypi events
>   * and input layer indexes in the keymap
>   */
> @@ -981,6 +987,145 @@ static int sony_nc_resume(struct acpi_device *device)
>  	return 0;
>  }
>  
> +static void sony_rfkill_cleanup(void)
> +{
> +	if (sony_wifi_rfkill)
> +		rfkill_unregister(sony_wifi_rfkill);
> +	if (sony_bluetooth_rfkill)
> +		rfkill_unregister(sony_bluetooth_rfkill);
> +	if (sony_wwan_rfkill)
> +		rfkill_unregister(sony_wwan_rfkill);
> +	if (sony_wimax_rfkill)
> +		rfkill_unregister(sony_wimax_rfkill);
> +}
> +
> +static int sony_nc_rfkill_get(void *data, enum rfkill_state *state)
> +{
> +	int result;
> +
> +	acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0x3 | ((long) data << 8),
> +			 &result);
> +	if (result & 0xf)
> +		*state = RFKILL_STATE_UNBLOCKED;
> +	else
> +		*state = RFKILL_STATE_SOFT_BLOCKED;
> +	return 0;
> +}
> +
> +static int sony_nc_rfkill_set(void *data, enum rfkill_state state)
> +{
> +	int result;
> +	int call = 0x3 | (((long) data + 1) << 8);
> +
> +	if (state == RFKILL_STATE_UNBLOCKED)
> +		call |= 0xff0000;
> +
> +	return acpi_callsetfunc(sony_nc_acpi_handle, "SN07", call, &result);
> +}
> +
> +static int sony_nc_setup_wifi_rfkill(struct acpi_device *device)
> +{
> +	int err = 0;
> +
> +	sony_wifi_rfkill = rfkill_allocate(&device->dev, RFKILL_TYPE_WLAN);
> +	if (!sony_wifi_rfkill)
> +		return -1;
> +	sony_wifi_rfkill->name = "sony-wifi";
> +	sony_wifi_rfkill->toggle_radio = sony_nc_rfkill_set;
> +	sony_wifi_rfkill->get_state = sony_nc_rfkill_get;
> +	sony_wifi_rfkill->user_claim_unsupported = 1;
> +	sony_wifi_rfkill->data = (void *)3;
> +	err = rfkill_register(sony_wifi_rfkill);
> +	if (err)
> +		rfkill_unregister(sony_wifi_rfkill);

do we really need to unregister if registering failed?
Looking at rfkill_{un,}register this seems unnecessary while an
rfkill_free seems more appropriate.
The same applies for the other rfkill setup functions.

> +	return err;
> +}
> +
> +static int sony_nc_setup_bluetooth_rfkill(struct acpi_device *device)
> +{
> +	int err = 0;
> +
> +	sony_bluetooth_rfkill = rfkill_allocate(&device->dev,
> +						RFKILL_TYPE_BLUETOOTH);
> +	if (!sony_bluetooth_rfkill)
> +		return -1;
> +	sony_bluetooth_rfkill->name = "sony-bluetooth";
> +	sony_bluetooth_rfkill->toggle_radio = sony_nc_rfkill_set;
> +	sony_bluetooth_rfkill->get_state = sony_nc_rfkill_get;
> +	sony_bluetooth_rfkill->user_claim_unsupported = 1;
> +	sony_bluetooth_rfkill->data = (void *)5;
> +	err = rfkill_register(sony_bluetooth_rfkill);
> +	if (err)
> +		rfkill_unregister(sony_bluetooth_rfkill);



> +	return err;
> +}
> +
> +static int sony_nc_setup_wwan_rfkill(struct acpi_device *device)
> +{
> +	int err = 0;
> +
> +	sony_wwan_rfkill = rfkill_allocate(&device->dev, RFKILL_TYPE_WWAN);
> +	if (!sony_wwan_rfkill)
> +		return -1;
> +	sony_wwan_rfkill->name = "sony-wwan";
> +	sony_wwan_rfkill->toggle_radio = sony_nc_rfkill_set;
> +	sony_wwan_rfkill->get_state = sony_nc_rfkill_get;
> +	sony_wwan_rfkill->user_claim_unsupported = 1;
> +	sony_wwan_rfkill->data = (void *)7;
> +	err = rfkill_register(sony_wwan_rfkill);
> +	if (err)
> +		rfkill_unregister(sony_wwan_rfkill);
> +	return err;
> +}
> +
> +static int sony_nc_setup_wimax_rfkill(struct acpi_device *device)
> +{
> +	int err = 0;
> +
> +	sony_wimax_rfkill = rfkill_allocate(&device->dev, RFKILL_TYPE_WIMAX);
> +	if (!sony_wimax_rfkill)
> +		return -1;
> +	sony_wimax_rfkill->name = "sony-wimax";
> +	sony_wimax_rfkill->toggle_radio = sony_nc_rfkill_set;
> +	sony_wimax_rfkill->get_state = sony_nc_rfkill_get;
> +	sony_wimax_rfkill->user_claim_unsupported = 1;
> +	sony_wimax_rfkill->data = (void *)9;
> +	err = rfkill_register(sony_wimax_rfkill);
> +	if (err)
> +		rfkill_unregister(sony_wimax_rfkill);
> +	return err;
> +}
> +
> +static int sony_nc_function_setup(struct acpi_device *device)
> +{
> +	int result;
> +
> +	/* Enable all events */
> +	acpi_callsetfunc(sony_nc_acpi_handle, "SN02", 0xffff, &result);
> +
> +	/* Setup hotkey decoding */
> +	acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0x2, &result);
> +
> +	/* Eaable hotkey event generation */
> +	acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0, &result);
> +
> +	/* Set BCHA, whatever /that/ does */
> +	acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0x101, &result);
> +
> +	acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0xb03, &result);

hummm, this is very similar to the callback setup executed when matching
the snc dmi list.
On which vaio model did you get this numbers? Did you find the other
initialization path (the one dependent on the DMI list) any useful on
that model? i.e.: do you need both?

> +	if (result & 0x1)
> +		sony_nc_setup_wifi_rfkill(device);
> +	if (result & 0x2)
> +		sony_nc_setup_bluetooth_rfkill(device);
> +	if (result & 0x1c)
> +		sony_nc_setup_wwan_rfkill(device);
> +	if (result & 0x20)
> +		sony_nc_setup_wimax_rfkill(device);
> +
> +	return 0;
> +}
> +
>  static int sony_nc_add(struct acpi_device *device)
>  {
>  	acpi_status status;
> @@ -1024,6 +1169,12 @@ static int sony_nc_add(struct acpi_device *device)
>  			dprintk("_INI Method failed\n");
>  	}
>  
> +	if (ACPI_SUCCESS(acpi_get_handle(sony_nc_acpi_handle, "SN07",
> +					 &handle))) {
> +		dprintk("Doing SNC setup\n");
> +		sony_nc_function_setup(device);
> +	}
> +
>  	/* setup input devices and helper fifo */
>  	result = sony_laptop_setup_input(device);
>  	if (result) {
> @@ -1131,6 +1282,7 @@ static int sony_nc_add(struct acpi_device *device)
>  	sony_laptop_remove_input();
>  
>        outwalk:
> +	sony_rfkill_cleanup();
>  	return result;
>  }
>  
> @@ -1156,6 +1308,7 @@ static int sony_nc_remove(struct acpi_device *device, int type)
>  
>  	sony_pf_remove();
>  	sony_laptop_remove_input();
> +	sony_rfkill_cleanup();
>  	dprintk(SONY_NC_DRIVER_NAME " removed.\n");
>  
>  	return 0;
> 
> -- 
> Matthew Garrett | mjg59@srcf.ucam.org
>
Matthew Garrett March 20, 2009, 2 p.m. UTC | #10
On Fri, Mar 20, 2009 at 05:52:14PM +0900, Mattia Dongili wrote:

> I'm more of the idea to provide a module option to force the setup
> callback if the module is not in the DMI list.
> Although for now all of the models that have SN07 and friends seem to
> benefit from throwing some magic numbers at them.

I suspect that this is how new machines expect to be controlled.

> > calling the ECON method on the SNC since some codepaths in the tables 
> > seem to depend on them - but I'm also worried to a certain extent on how 
> > much that might change driver interactions with some machines.
> 
> My understanding about ECON is that it is always enabled if the embedded
> controller is enabled. The SPIC device has the same kind of dependency
> and as far as I could see ECON is always 1. So I don't think it makes
> much of a difference.

I had one machine where ECON seemed to need to be called explicitly, but 
I can't remember the details now. Calling it probably wouldn't hurt 
anything.

> do we really need to unregister if registering failed?
> Looking at rfkill_{un,}register this seems unnecessary while an
> rfkill_free seems more appropriate.
> The same applies for the other rfkill setup functions.

Yeah, I'll fix that up.

> > +	acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0x101, &result);
> > +
> > +	acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0xb03, &result);
> 
> hummm, this is very similar to the callback setup executed when matching
> the snc dmi list.
> On which vaio model did you get this numbers? Did you find the other
> initialization path (the one dependent on the DMI list) any useful on
> that model? i.e.: do you need both?

The numbers correspond to enabling all events. I couldn't think of any 
reason why we'd only want to enable a subset. The current nc setup code 
seems to enable some events and then disable them again, which I don't 
really understand.
Mattia Dongili March 21, 2009, 4 a.m. UTC | #11
On Fri, Mar 20, 2009 at 02:00:04PM +0000, Matthew Garrett wrote:
> On Fri, Mar 20, 2009 at 05:52:14PM +0900, Mattia Dongili wrote:
> 
> > I'm more of the idea to provide a module option to force the setup
> > callback if the module is not in the DMI list.
> > Although for now all of the models that have SN07 and friends seem to
> > benefit from throwing some magic numbers at them.
> 
> I suspect that this is how new machines expect to be controlled.

agreed, but I need to figure out if the initialization sequence is
really the same for those new TT/Z models and the olders one (see also
below).

> > > calling the ECON method on the SNC since some codepaths in the tables 
> > > seem to depend on them - but I'm also worried to a certain extent on how 
> > > much that might change driver interactions with some machines.
> > 
> > My understanding about ECON is that it is always enabled if the embedded
> > controller is enabled. The SPIC device has the same kind of dependency
> > and as far as I could see ECON is always 1. So I don't think it makes
> > much of a difference.
> 
> I had one machine where ECON seemed to need to be called explicitly, but 
> I can't remember the details now. Calling it probably wouldn't hurt 
> anything.

seems to be a TT and Z specific thing though. The DSDT on other models
doesn't provide the ECON method.

...
> > > +	acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0x101, &result);
> > > +
> > > +	acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0xb03, &result);
> > 
> > hummm, this is very similar to the callback setup executed when matching
> > the snc dmi list.
> > On which vaio model did you get this numbers? Did you find the other
> > initialization path (the one dependent on the DMI list) any useful on
> > that model? i.e.: do you need both?
> 
> The numbers correspond to enabling all events. I couldn't think of any 
> reason why we'd only want to enable a subset. The current nc setup code 
> seems to enable some events and then disable them again, which I don't 
> really understand.

Well, the current sequence was taken from a trace in windows on a Vaio C
Type, then it demonstrated to be helpful on other models as well.
The SN07[1] method is very different from the Z and TT type to the AR, C,
FE, FZ and N so I'm starting to suspect that we're just seeing a new
generation of SNC based models. I'll see if some users with older models
can give the new sequence a go.

In the meantime can we make your sony_nc_function_setup less invasive
and depend on the DMI to match?

[1]: more dsdt tables here http://www.kamineko.org/dsdt-vaio/
Matthew Garrett March 21, 2009, 4:35 a.m. UTC | #12
On Sat, Mar 21, 2009 at 01:00:10PM +0900, Mattia Dongili wrote:
> On Fri, Mar 20, 2009 at 02:00:04PM +0000, Matthew Garrett wrote:
> > I had one machine where ECON seemed to need to be called explicitly, but 
> > I can't remember the details now. Calling it probably wouldn't hurt 
> > anything.
> 
> seems to be a TT and Z specific thing though. The DSDT on other models
> doesn't provide the ECON method.

Yeah. As I said, I don't think there's any harm in causing it - I think 
I was getting more promising results from hotkey events in the Z when I 
called ECON, but I don't have access to that machine right now and never 
got it finished off.

> > The numbers correspond to enabling all events. I couldn't think of any 
> > reason why we'd only want to enable a subset. The current nc setup code 
> > seems to enable some events and then disable them again, which I don't 
> > really understand.
> 
> Well, the current sequence was taken from a trace in windows on a Vaio C
> Type, then it demonstrated to be helpful on other models as well.
> The SN07[1] method is very different from the Z and TT type to the AR, C,
> FE, FZ and N so I'm starting to suspect that we're just seeing a new
> generation of SNC based models. I'll see if some users with older models
> can give the new sequence a go.

Looking through, the implementation seems quite different but the 
functionality seems the same - the newer machines seem to return values 
directly, whereas older ones tended to trap into SMM. The wireless 
control (at least, the enumeration call I make) seems to be a noop on 
these older machines. It /looks/ like we can probably get some sort of 
versioning information about the interface by calling SN00. I think that 
would probably be a better approach than using DMI for this.

I've put this into rawhide, so I suspect we'll hear complaints if it 
breaks things for anybody.
Mattia Dongili March 21, 2009, 6:32 a.m. UTC | #13
On Sat, Mar 21, 2009 at 04:35:13AM +0000, Matthew Garrett wrote:
> On Sat, Mar 21, 2009 at 01:00:10PM +0900, Mattia Dongili wrote:
> > On Fri, Mar 20, 2009 at 02:00:04PM +0000, Matthew Garrett wrote:
> > > I had one machine where ECON seemed to need to be called explicitly, but 
> > > I can't remember the details now. Calling it probably wouldn't hurt 
> > > anything.
> > 
> > seems to be a TT and Z specific thing though. The DSDT on other models
> > doesn't provide the ECON method.
> 
> Yeah. As I said, I don't think there's any harm in causing it - I think 
> I was getting more promising results from hotkey events in the Z when I 
> called ECON, but I don't have access to that machine right now and never 
> got it finished off.

Sounds reasonable.

> > > The numbers correspond to enabling all events. I couldn't think of any 
> > > reason why we'd only want to enable a subset. The current nc setup code 
> > > seems to enable some events and then disable them again, which I don't 
> > > really understand.
> > 
> > Well, the current sequence was taken from a trace in windows on a Vaio C
> > Type, then it demonstrated to be helpful on other models as well.
> > The SN07[1] method is very different from the Z and TT type to the AR, C,
> > FE, FZ and N so I'm starting to suspect that we're just seeing a new
> > generation of SNC based models. I'll see if some users with older models
> > can give the new sequence a go.
> 
> Looking through, the implementation seems quite different but the 
> functionality seems the same - the newer machines seem to return values 
> directly, whereas older ones tended to trap into SMM. The wireless 
> control (at least, the enumeration call I make) seems to be a noop on 
> these older machines. It /looks/ like we can probably get some sort of 
> versioning information about the interface by calling SN00. I think that 
> would probably be a better approach than using DMI for this.

sure, that DMI whitelist is already annoying in its current shape.
Getting the ACPI tables to tell us what SNC version we are looking at
would be so much better.
I just grepped the DSDTs I have here and this is what I got:
DSDT.c1s.dsl:                    Name (SNI4, 0x344A0001)
DSDT.c71bw.dsl:                    Name (SNI4, 0x344A0001)
DSDT.fe21b.dsl:                    Name (SNI4, 0x334A0000)
DSDT.fe31m.dsl:                    Name (SNI4, 0x334A0000)
DSDT.fe41z.dsl:                    Name (SNI4, 0x334A0000)
DSDT.fe830fe.dsl:                    Name (SNI4, 0x334A0000)
DSDT.fz15.dsl:                    Name (SNI4, 0x374A0000)
DSDT.fz180e.dsl:                    Name (SNI4, 0x374A0000)
DSDT.n370e.dsl:                    Name (SNI4, 0x344A0001)
DSDT.tt11lnb.dsl:                        Store (0x344D0000, Index (CFGI, 0x04))
DSDT.z11awn.dsl:                        Store (0x334D0000, Index (CFGI, 0x04))
DSDT.z11vn.dsl:                        Store (0x334D0000, Index (CFGI, 0x04))
DSDT.z26gn.dsl:                        Store (0x334D0000, Index (CFGI, 0x04))
DSDT.z90s.dsl:                        Store (0x334D0000, Index (CFGI, 0x04))
VGN-AR31S-R0200J6.dsl:                    Name (SNI4, 0x364A0000)
VGN-AR370E-R0200J6.dsl:                    Name (SNI4, 0x364A0000)
VGN-C1S.dsl:                    Name (SNI4, 0x344A0001)
VGN-C1ZB-R0034J4.dsl:                    Name (SNI4, 0x344A0001)
VGN-C240E-R0080J4.dsl:                    Name (SNI4, 0x344A0001)
VGN-C2S-R0080J4.dsl:                    Name (SNI4, 0x344A0001)
VGN-C2Z-R0080J4.dsl:                    Name (SNI4, 0x344A0001)
VGN-FE11H-R0072J3.dsl:                    Name (SNI4, 0x334A0000)
VGN-FE11H-R0074J3.dsl:                    Name (SNI4, 0x334A0000)
VGN-FE11M-R0172J3.dsl:                    Name (SNI4, 0x334A0000)
VGN-FE21M-R0130J3.dsl:                    Name (SNI4, 0x334A0000)
VGN-FE31M.dsl:                    Name (SNI4, 0x334A0000)
VGN-FE41E-R0190J3.dsl:                    Name (SNI4, 0x334A0000)
VGN-FE41M-R0190J3.dsl:                    Name (SNI4, 0x334A0000)
VGN-FE41Z-R0200J3.dsl:                    Name (SNI4, 0x334A0000)
VGN-FE550G-R0074J3.dsl:                    Name (SNI4, 0x334A0000)
VGN-FE590P-R0072J3.dsl:                    Name (SNI4, 0x334A0000)
VGN-FE660G-R0133J3.dsl:                    Name (SNI4, 0x334A0000)
VGN-FE770G-R0173J3.dsl:                    Name (SNI4, 0x334A0000)
VGN-FE830.dsl:                    Name (SNI4, 0x334A0000)
VGN-FE880EH-R0200J3.dsl:                    Name (SNI4, 0x334A0000)
VGN-N130G-R0020J4.dsl:                    Name (SNI4, 0x344A0001)
VGN-N230E-R0070J4.dsl:                    Name (SNI4, 0x344A0001)

SNI4 or CFGI+0x04 is the only differing number and is returned with
SN00(4).

> I've put this into rawhide, so I suspect we'll hear complaints if it 
> breaks things for anybody.

want to try to push your patch to mainline or would you prefer to wait?
IMO pushing it and eventually fixing support for 0x344a000[10] models is
fine. After all your snc_setup code could be easily plugged into the DMI
list for the time being and Z and TT users would be happy.
Matthew Garrett March 21, 2009, 2:06 p.m. UTC | #14
On Sat, Mar 21, 2009 at 03:32:30PM +0900, Mattia Dongili wrote:

> sure, that DMI whitelist is already annoying in its current shape.
> Getting the ACPI tables to tell us what SNC version we are looking at
> would be so much better.
> I just grepped the DSDTs I have here and this is what I got:
> DSDT.c1s.dsl:                    Name (SNI4, 0x344A0001)
> DSDT.c71bw.dsl:                    Name (SNI4, 0x344A0001)

(snip)

The second byte seems to be the most consistent here - 0x4a for the 
models currently handled by the nc code, 0x4d for the tt and z, 0x55 for 
the p. I've no idea whether this is a monotonically increasing version 
string or a bitmask of supported functionality, but it ought to be 
enough to key off to begin with.

> want to try to push your patch to mainline or would you prefer to wait?
> IMO pushing it and eventually fixing support for 0x344a000[10] models is
> fine. After all your snc_setup code could be easily plugged into the DMI
> list for the time being and Z and TT users would be happy.

Might as well push it and see what happens?
Mattia Dongili March 21, 2009, 2:37 p.m. UTC | #15
On Sat, Mar 21, 2009 at 02:06:54PM +0000, Matthew Garrett wrote:
> On Sat, Mar 21, 2009 at 03:32:30PM +0900, Mattia Dongili wrote:
> 
> > sure, that DMI whitelist is already annoying in its current shape.
> > Getting the ACPI tables to tell us what SNC version we are looking at
> > would be so much better.
> > I just grepped the DSDTs I have here and this is what I got:
> > DSDT.c1s.dsl:                    Name (SNI4, 0x344A0001)
> > DSDT.c71bw.dsl:                    Name (SNI4, 0x344A0001)
> 
> (snip)
> 
> The second byte seems to be the most consistent here - 0x4a for the 
> models currently handled by the nc code, 0x4d for the tt and z, 0x55 for 
> the p. I've no idea whether this is a monotonically increasing version 
> string or a bitmask of supported functionality, but it ought to be 
> enough to key off to begin with.

Oh, do you have a vaio type P DSDT to share?

> > want to try to push your patch to mainline or would you prefer to wait?
> > IMO pushing it and eventually fixing support for 0x344a000[10] models is
> > fine. After all your snc_setup code could be easily plugged into the DMI
> > list for the time being and Z and TT users would be happy.
> 
> Might as well push it and see what happens?

Sure, if you're ok I'll apply the patch you sent just changing the
rfkill_free and send the whole patch-set to Len tomorrow morning (JST).

cheers
Matthew Garrett March 21, 2009, 2:55 p.m. UTC | #16
On Sat, Mar 21, 2009 at 11:37:18PM +0900, Mattia Dongili wrote:

> Oh, do you have a vaio type P DSDT to share?

Attached. I'm now fairly sure that the second byte returned by SN00(4) 
is the gross version, with the leading byte being a subversion(!?). 
Machines with 0x4a in the second byte have the following set of methods 
(as called by SN07):

   0x33:  0x34:  0x36:  0x37:
0, 0x0101 0x0101 0x0101 0x0101
1, 0x0102 0x0102 0x0102 0x0102
2, 0x0100 0x0100 0x0100 0x0100
3, 0x0104        0x0104
4, 0x0107 0x010F 0x0107 0x010F
5,               0x0106 0x0106
6,               0x0000 0x0113
7,        0x0111        0x010A
C,               0x010C
D,               0x010D 0x010D
E, 0x0105 0x0105 0x0105 0x0105
F, 0x0103 0x0103 0x0103

They're indexed by the leading byte. Machines with 0x4d as the second 
byte have:

0, 0x0100
1, 0x0101
2, 0x0113
3, 0x0124
4, 0x0126
5, 0x0125
6, 0x011D
7, 0x0121
8, 0x0105
9, 0x0114
a, 0x0119
b, 0x0122
c, 0x0128
d, 0x0115
e, 0x0131
f, 0x12f

Machines with 0x55 as the second byte (only the P, as far as I can 
tell):

0, 0x100
1, 0x101
2, 0x113
3, 0x124
4, 0x132
5, 0x125
6, 0x11d
7, 0x121
8, 0x105
9, 0x114
a, 0x119
b, 0x122
c, 0x130
d, 0x115
e, 0x131

This actually makes me think I've been taking the wrong approach. I'm 
now guessing that the numbers returned by SN00 in the 20-2f range are 
method signatures - 0x100 is hotkeys, 0x124 is rfkill. This fits with 
the fact that SN07(0) looks to return hotkey values on the Z, whereas 
that's SN07(2) on the older hardware. Both have 0x100 returned by 
SN00(0x20). I think if we rewrite the code on this basis we can merge 
the functionality. I'll try to do that tomorrow and also (with luck) get 
the hotkey stuff on the Z back up.

> Sure, if you're ok I'll apply the patch you sent just changing the
> rfkill_free and send the whole patch-set to Len tomorrow morning (JST).

Mind waiting until I've tried this rewrite? It'd be nice to know what a 
bunch more of these methods do, but I guess that's less urgent.
Matthew Garrett March 21, 2009, 3:10 p.m. UTC | #17
On Sat, Mar 21, 2009 at 02:55:02PM +0000, Matthew Garrett wrote:

>    0x33:  0x34:  0x36:  0x37:
> 0, 0x0101 0x0101 0x0101 0x0101
> 1, 0x0102 0x0102 0x0102 0x0102
> 2, 0x0100 0x0100 0x0100 0x0100
> 3, 0x0104        0x0104
> 4, 0x0107 0x010F 0x0107 0x010F
> 5,               0x0106 0x0106
> 6,               0x0000 0x0113
> 7,        0x0111        0x010A
> C,               0x010C
> D,               0x010D 0x010D
> E, 0x0105 0x0105 0x0105 0x0105
> F, 0x0103 0x0103 0x0103

Actually slightly more complicated than that - some are only stored on 
certain OS versions. Here's a more complete table.

   0x33:  0x34:  0x36:  0x37:

0, 0x0101 0x0101 0x0101 0x0101
1, 0x0102 0x0102 0x0102 0x0102
2, 0x0100 0x0100 0x0100 0x0100
3, 0x0104 0x0113 0x0104
4, 0x0107 0x010F 0x0107 0x010F
5,               0x0106 0x0106
6,               0x0113 0x0113
7,        0x0111        0x010A
C,               0x010C
D,               0x010D 0x010D
E, 0x0105 0x0105 0x0105 0x0105
F, 0x0103 0x0103 0x0103

The way that 0x113 moves around leaves me pretty sure that calling SN07 
without checking what the function signature is is not a good idea. I'll 
rewrite my patch.
Matthias Welwarsky March 21, 2009, 7:15 p.m. UTC | #18
On Saturday 21 March 2009 16:10:37 Matthew Garrett wrote:
> On Sat, Mar 21, 2009 at 02:55:02PM +0000, Matthew Garrett wrote:
> >    0x33:  0x34:  0x36:  0x37:
> > 0, 0x0101 0x0101 0x0101 0x0101
> > 1, 0x0102 0x0102 0x0102 0x0102
> > 2, 0x0100 0x0100 0x0100 0x0100
> > 3, 0x0104        0x0104
> > 4, 0x0107 0x010F 0x0107 0x010F
> > 5,               0x0106 0x0106
> > 6,               0x0000 0x0113
> > 7,        0x0111        0x010A
> > C,               0x010C
> > D,               0x010D 0x010D
> > E, 0x0105 0x0105 0x0105 0x0105
> > F, 0x0103 0x0103 0x0103
>
> Actually slightly more complicated than that - some are only stored on
> certain OS versions. Here's a more complete table.
>
>    0x33:  0x34:  0x36:  0x37:
>
> 0, 0x0101 0x0101 0x0101 0x0101
> 1, 0x0102 0x0102 0x0102 0x0102
> 2, 0x0100 0x0100 0x0100 0x0100
> 3, 0x0104 0x0113 0x0104
> 4, 0x0107 0x010F 0x0107 0x010F
> 5,               0x0106 0x0106
> 6,               0x0113 0x0113
> 7,        0x0111        0x010A
> C,               0x010C
> D,               0x010D 0x010D
> E, 0x0105 0x0105 0x0105 0x0105
> F, 0x0103 0x0103 0x0103
>
> The way that 0x113 moves around leaves me pretty sure that calling SN07
> without checking what the function signature is is not a good idea. I'll
> rewrite my patch.

If that's really a function signature, why are some of the slots empty? If 
you're supposed to read the CFGI first via SN00, that would not make much 
sense. However, it makes sense if the function is encoded by the slot and a 
hole (i.e. Zero) means that the function is not supported or disabled.

If you're right though, it means that "0x124" on all new-style models (those 
with SN07 method) must always operate the killswitch. I might be able to find 
that out from the decompiled DSDT. Right now I only checked the Z series DSDTs 
(Z11 and Z21) and of course they are very similar. But I would be very 
interested to see the code of  functions called for the other "slot 3" 
methods, like 0x0104 and 0x0113. If you had some DSDTs to share...

regards,
	matthias

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mattia Dongili March 22, 2009, 2:38 a.m. UTC | #19
On Sat, Mar 21, 2009 at 03:10:37PM +0000, Matthew Garrett wrote:
> On Sat, Mar 21, 2009 at 02:55:02PM +0000, Matthew Garrett wrote:
> 
> >    0x33:  0x34:  0x36:  0x37:
> > 0, 0x0101 0x0101 0x0101 0x0101
> > 1, 0x0102 0x0102 0x0102 0x0102
> > 2, 0x0100 0x0100 0x0100 0x0100
> > 3, 0x0104        0x0104
> > 4, 0x0107 0x010F 0x0107 0x010F
> > 5,               0x0106 0x0106
> > 6,               0x0000 0x0113
> > 7,        0x0111        0x010A
> > C,               0x010C
> > D,               0x010D 0x010D
> > E, 0x0105 0x0105 0x0105 0x0105
> > F, 0x0103 0x0103 0x0103
> 
> Actually slightly more complicated than that - some are only stored on 
> certain OS versions. Here's a more complete table.
> 
>    0x33:  0x34:  0x36:  0x37:
> 
> 0, 0x0101 0x0101 0x0101 0x0101
> 1, 0x0102 0x0102 0x0102 0x0102
> 2, 0x0100 0x0100 0x0100 0x0100
> 3, 0x0104 0x0113 0x0104
> 4, 0x0107 0x010F 0x0107 0x010F
> 5,               0x0106 0x0106
> 6,               0x0113 0x0113
> 7,        0x0111        0x010A
> C,               0x010C
> D,               0x010D 0x010D
> E, 0x0105 0x0105 0x0105 0x0105
> F, 0x0103 0x0103 0x0103
> 
> The way that 0x113 moves around leaves me pretty sure that calling SN07 
> without checking what the function signature is is not a good idea. I'll 
> rewrite my patch.

Ok no rush, I'll wait until you're done before submitting the batch for
.30.

PS: you forgot the type P DSDT attachment. ;)
Matthew Garrett March 22, 2009, 1:33 p.m. UTC | #20
On Sat, Mar 21, 2009 at 08:15:00PM +0100, Matthias Welwarsky wrote:
> On Saturday 21 March 2009 16:10:37 Matthew Garrett wrote:
> > The way that 0x113 moves around leaves me pretty sure that calling SN07
> > without checking what the function signature is is not a good idea. I'll
> > rewrite my patch.
> 
> If that's really a function signature, why are some of the slots empty? If 
> you're supposed to read the CFGI first via SN00, that would not make much 
> sense. However, it makes sense if the function is encoded by the slot and a 
> hole (i.e. Zero) means that the function is not supported or disabled.

I thought that at first, but 0x113 is the same code on the machines I 
checked even though it moves around. 0x100 appears to be "Get hotkey 
event" on the newer hardware even though it's not in slot 2.

> If you're right though, it means that "0x124" on all new-style models (those 
> with SN07 method) must always operate the killswitch. I might be able to find 
> that out from the decompiled DSDT. Right now I only checked the Z series DSDTs 
> (Z11 and Z21) and of course they are very similar. But I would be very 
> interested to see the code of  functions called for the other "slot 3" 
> methods, like 0x0104 and 0x0113. If you had some DSDTs to share...

The P, TT and Z all seem to have 0x124 as killswitch - the 0x113 and 
0x104 methods don't seem to be killswitch related, as far as I can tell. 
Mattia linked to a set of DSDTs earlier in the thread.
diff mbox

Patch

diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index 537959d..c57f54c 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -64,6 +64,7 @@ 
 #include <asm/uaccess.h>
 #include <linux/sonypi.h>
 #include <linux/sony-laptop.h>
+#include <linux/rfkill.h>
 #ifdef CONFIG_SONYPI_COMPAT
 #include <linux/poll.h>
 #include <linux/miscdevice.h>
@@ -143,6 +144,11 @@  struct sony_laptop_keypress {
 	int key;
 };
 
+static struct rfkill *sony_wifi_rfkill;
+static struct rfkill *sony_bluetooth_rfkill;
+static struct rfkill *sony_wwan_rfkill;
+static struct rfkill *sony_wimax_rfkill;
+
 /* Correspondance table between sonypi events
  * and input layer indexes in the keymap
  */
@@ -981,6 +987,145 @@  static int sony_nc_resume(struct acpi_device *device)
 	return 0;
 }
 
+static void sony_rfkill_cleanup(void)
+{
+	if (sony_wifi_rfkill)
+		rfkill_unregister(sony_wifi_rfkill);
+	if (sony_bluetooth_rfkill)
+		rfkill_unregister(sony_bluetooth_rfkill);
+	if (sony_wwan_rfkill)
+		rfkill_unregister(sony_wwan_rfkill);
+	if (sony_wimax_rfkill)
+		rfkill_unregister(sony_wimax_rfkill);
+}
+
+static int sony_nc_rfkill_get(void *data, enum rfkill_state *state)
+{
+	int result;
+
+	acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0x3 | ((long) data << 8),
+			 &result);
+	if (result & 0xf)
+		*state = RFKILL_STATE_UNBLOCKED;
+	else
+		*state = RFKILL_STATE_SOFT_BLOCKED;
+	return 0;
+}
+
+static int sony_nc_rfkill_set(void *data, enum rfkill_state state)
+{
+	int result;
+	int call = 0x3 | (((long) data + 1) << 8);
+
+	if (state == RFKILL_STATE_UNBLOCKED)
+		call |= 0xff0000;
+
+	return acpi_callsetfunc(sony_nc_acpi_handle, "SN07", call, &result);
+}
+
+static int sony_nc_setup_wifi_rfkill(struct acpi_device *device)
+{
+	int err = 0;
+
+	sony_wifi_rfkill = rfkill_allocate(&device->dev, RFKILL_TYPE_WLAN);
+	if (!sony_wifi_rfkill)
+		return -1;
+	sony_wifi_rfkill->name = "sony-wifi";
+	sony_wifi_rfkill->toggle_radio = sony_nc_rfkill_set;
+	sony_wifi_rfkill->get_state = sony_nc_rfkill_get;
+	sony_wifi_rfkill->user_claim_unsupported = 1;
+	sony_wifi_rfkill->data = (void *)3;
+	err = rfkill_register(sony_wifi_rfkill);
+	if (err)
+		rfkill_unregister(sony_wifi_rfkill);
+	return err;
+}
+
+static int sony_nc_setup_bluetooth_rfkill(struct acpi_device *device)
+{
+	int err = 0;
+
+	sony_bluetooth_rfkill = rfkill_allocate(&device->dev,
+						RFKILL_TYPE_BLUETOOTH);
+	if (!sony_bluetooth_rfkill)
+		return -1;
+	sony_bluetooth_rfkill->name = "sony-bluetooth";
+	sony_bluetooth_rfkill->toggle_radio = sony_nc_rfkill_set;
+	sony_bluetooth_rfkill->get_state = sony_nc_rfkill_get;
+	sony_bluetooth_rfkill->user_claim_unsupported = 1;
+	sony_bluetooth_rfkill->data = (void *)5;
+	err = rfkill_register(sony_bluetooth_rfkill);
+	if (err)
+		rfkill_unregister(sony_bluetooth_rfkill);
+	return err;
+}
+
+static int sony_nc_setup_wwan_rfkill(struct acpi_device *device)
+{
+	int err = 0;
+
+	sony_wwan_rfkill = rfkill_allocate(&device->dev, RFKILL_TYPE_WWAN);
+	if (!sony_wwan_rfkill)
+		return -1;
+	sony_wwan_rfkill->name = "sony-wwan";
+	sony_wwan_rfkill->toggle_radio = sony_nc_rfkill_set;
+	sony_wwan_rfkill->get_state = sony_nc_rfkill_get;
+	sony_wwan_rfkill->user_claim_unsupported = 1;
+	sony_wwan_rfkill->data = (void *)7;
+	err = rfkill_register(sony_wwan_rfkill);
+	if (err)
+		rfkill_unregister(sony_wwan_rfkill);
+	return err;
+}
+
+static int sony_nc_setup_wimax_rfkill(struct acpi_device *device)
+{
+	int err = 0;
+
+	sony_wimax_rfkill = rfkill_allocate(&device->dev, RFKILL_TYPE_WIMAX);
+	if (!sony_wimax_rfkill)
+		return -1;
+	sony_wimax_rfkill->name = "sony-wimax";
+	sony_wimax_rfkill->toggle_radio = sony_nc_rfkill_set;
+	sony_wimax_rfkill->get_state = sony_nc_rfkill_get;
+	sony_wimax_rfkill->user_claim_unsupported = 1;
+	sony_wimax_rfkill->data = (void *)9;
+	err = rfkill_register(sony_wimax_rfkill);
+	if (err)
+		rfkill_unregister(sony_wimax_rfkill);
+	return err;
+}
+
+static int sony_nc_function_setup(struct acpi_device *device)
+{
+	int result;
+
+	/* Enable all events */
+	acpi_callsetfunc(sony_nc_acpi_handle, "SN02", 0xffff, &result);
+
+	/* Setup hotkey decoding */
+	acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0x2, &result);
+
+	/* Eaable hotkey event generation */
+	acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0, &result);
+
+	/* Set BCHA, whatever /that/ does */
+	acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0x101, &result);
+
+	acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0xb03, &result);
+
+	if (result & 0x1)
+		sony_nc_setup_wifi_rfkill(device);
+	if (result & 0x2)
+		sony_nc_setup_bluetooth_rfkill(device);
+	if (result & 0x1c)
+		sony_nc_setup_wwan_rfkill(device);
+	if (result & 0x20)
+		sony_nc_setup_wimax_rfkill(device);
+
+	return 0;
+}
+
 static int sony_nc_add(struct acpi_device *device)
 {
 	acpi_status status;
@@ -1024,6 +1169,12 @@  static int sony_nc_add(struct acpi_device *device)
 			dprintk("_INI Method failed\n");
 	}
 
+	if (ACPI_SUCCESS(acpi_get_handle(sony_nc_acpi_handle, "SN07",
+					 &handle))) {
+		dprintk("Doing SNC setup\n");
+		sony_nc_function_setup(device);
+	}
+
 	/* setup input devices and helper fifo */
 	result = sony_laptop_setup_input(device);
 	if (result) {
@@ -1131,6 +1282,7 @@  static int sony_nc_add(struct acpi_device *device)
 	sony_laptop_remove_input();
 
       outwalk:
+	sony_rfkill_cleanup();
 	return result;
 }
 
@@ -1156,6 +1308,7 @@  static int sony_nc_remove(struct acpi_device *device, int type)
 
 	sony_pf_remove();
 	sony_laptop_remove_input();
+	sony_rfkill_cleanup();
 	dprintk(SONY_NC_DRIVER_NAME " removed.\n");
 
 	return 0;