2012-10-11 10:59:13

by Trepak Vilmos

[permalink] [raw]
Subject: [PATCH] Platform: hp-wmi: add rfkill support for integrated GPS

Add rfkill support for the GPS radio found in HP laptops (HP Elitebook 2170p and the like)
using the Ericsson F5321/H5321 Mobile Broadband Module.
Signed-off-by: Viliam Trepák <[email protected]>
---
After spending hours trying to get GPS working on my laptop, poring over AT command set
documentations and brute-forcing AT command parameters, I found this in dmesg:

> hp_wmi: unknown device type 0x3

Some copy&pasting later I could switch the GPS radio on and minicom started spewing out
NMEA sentences...

Tested on the aforementioned laptop, with kernel 3.5.6. Applies cleanly to 3.6.1 as well.

--- linux-3.6.1/drivers/platform/x86/hp-wmi.c.orig 2012-10-07 17:41:28.000000000 +0200
+++ linux-3.6.1/drivers/platform/x86/hp-wmi.c 2012-10-11 10:51:17.469934685 +0200
@@ -60,6 +60,7 @@ enum hp_wmi_radio {
HPWMI_WIFI = 0,
HPWMI_BLUETOOTH = 1,
HPWMI_WWAN = 2,
+ HPWMI_GPS = 3,
};

enum hp_wmi_event_ids {
@@ -147,6 +148,7 @@ static struct platform_device *hp_wmi_pl
static struct rfkill *wifi_rfkill;
static struct rfkill *bluetooth_rfkill;
static struct rfkill *wwan_rfkill;
+static struct rfkill *gps_rfkill;

struct rfkill2_device {
u8 id;
@@ -543,6 +545,10 @@ static void hp_wmi_notify(u32 value, voi
rfkill_set_states(wwan_rfkill,
hp_wmi_get_sw_state(HPWMI_WWAN),
hp_wmi_get_hw_state(HPWMI_WWAN));
+ if (gps_rfkill)
+ rfkill_set_states(gps_rfkill,
+ hp_wmi_get_sw_state(HPWMI_GPS),
+ hp_wmi_get_hw_state(HPWMI_GPS));
break;
case HPWMI_CPU_BATTERY_THROTTLE:
pr_info("Unimplemented CPU throttle because of 3 Cell battery event detected\n");
@@ -663,6 +669,24 @@ static int __devinit hp_wmi_rfkill_setup
goto register_bluetooth_error;
}

+ if (wireless & 0x3) {
+ gps_rfkill = rfkill_alloc("hp-gps", &device->dev,
+ RFKILL_TYPE_GPS,
+ &hp_wmi_rfkill_ops,
+ (void *) HPWMI_GPS);
+ if (!gps_rfkill) {
+ err = -ENOMEM;
+ goto register_bluetooth_error;
+ }
+ rfkill_init_sw_state(gps_rfkill,
+ hp_wmi_get_sw_state(HPWMI_GPS));
+ rfkill_set_hw_state(bluetooth_rfkill,
+ hp_wmi_get_hw_state(HPWMI_GPS));
+ err = rfkill_register(gps_rfkill);
+ if (err)
+ goto register_gps_error;
+ }
+
if (wireless & 0x4) {
wwan_rfkill = rfkill_alloc("hp-wwan", &device->dev,
RFKILL_TYPE_WWAN,
@@ -670,7 +694,7 @@ static int __devinit hp_wmi_rfkill_setup
(void *) HPWMI_WWAN);
if (!wwan_rfkill) {
err = -ENOMEM;
- goto register_bluetooth_error;
+ goto register_gps_error;
}
rfkill_init_sw_state(wwan_rfkill,
hp_wmi_get_sw_state(HPWMI_WWAN));
@@ -685,6 +709,11 @@ static int __devinit hp_wmi_rfkill_setup
register_wwan_err:
rfkill_destroy(wwan_rfkill);
wwan_rfkill = NULL;
+ if (gps_rfkill)
+ rfkill_unregister(gps_rfkill);
+register_gps_error:
+ rfkill_destroy(gps_rfkill);
+ gps_rfkill = NULL;
if (bluetooth_rfkill)
rfkill_unregister(bluetooth_rfkill);
register_bluetooth_error:
@@ -729,6 +758,10 @@ static int __devinit hp_wmi_rfkill2_setu
type = RFKILL_TYPE_WWAN;
name = "hp-wwan";
break;
+ case HPWMI_GPS:
+ type = RFKILL_TYPE_GPS;
+ name = "hp-gps";
+ break;
default:
pr_warn("unknown device type 0x%x\n",
state.device[i].radio_type);
@@ -786,6 +819,7 @@ static int __devinit hp_wmi_bios_setup(s
wifi_rfkill = NULL;
bluetooth_rfkill = NULL;
wwan_rfkill = NULL;
+ gps_rfkill = NULL;
rfkill2_count = 0;

if (hp_wmi_rfkill_setup(device))
@@ -835,6 +869,10 @@ static int __exit hp_wmi_bios_remove(str
rfkill_unregister(wwan_rfkill);
rfkill_destroy(wwan_rfkill);
}
+ if (gps_rfkill) {
+ rfkill_unregister(gps_rfkill);
+ rfkill_destroy(gps_rfkill);
+ }

return 0;
}
@@ -870,6 +908,10 @@ static int hp_wmi_resume_handler(struct
rfkill_set_states(wwan_rfkill,
hp_wmi_get_sw_state(HPWMI_WWAN),
hp_wmi_get_hw_state(HPWMI_WWAN));
+ if (gps_rfkill)
+ rfkill_set_states(gps_rfkill,
+ hp_wmi_get_sw_state(HPWMI_GPS),
+ hp_wmi_get_hw_state(HPWMI_GPS));

return 0;
}


2012-10-11 12:34:27

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH] Platform: hp-wmi: add rfkill support for integrated GPS

On Thu, Oct 11, 2012 at 12:51:00PM +0200, Trep?k Vilmos wrote:
> @@ -663,6 +669,24 @@ static int __devinit hp_wmi_rfkill_setup
> goto register_bluetooth_error;
> }
>
> + if (wireless & 0x3) {
> + gps_rfkill = rfkill_alloc("hp-gps", &device->dev,
> + RFKILL_TYPE_GPS,
> + &hp_wmi_rfkill_ops,
> + (void *) HPWMI_GPS);

I don't think this bit is right - in this codepath, wireless is a mask,
so 0x3 would mean wifi and bluetooth, not gps. I'd expect gps to be 0x8.
The rest looks good, though.


--
Matthew Garrett | [email protected]

2012-10-11 13:15:50

by Trepak Vilmos

[permalink] [raw]
Subject: Re: [PATCH] Platform: hp-wmi: add rfkill support for integrated GPS

On Thu, 2012-10-11 at 13:34 +0100, Matthew Garrett wrote:
> On Thu, Oct 11, 2012 at 12:51:00PM +0200, Trepák Vilmos wrote:
> > @@ -663,6 +669,24 @@ static int __devinit hp_wmi_rfkill_setup
> > goto register_bluetooth_error;
> > }
> >
> > + if (wireless & 0x3) {
> > + gps_rfkill = rfkill_alloc("hp-gps", &device->dev,
> > + RFKILL_TYPE_GPS,
> > + &hp_wmi_rfkill_ops,
> > + (void *) HPWMI_GPS);
>
> I don't think this bit is right - in this codepath, wireless is a mask,
> so 0x3 would mean wifi and bluetooth, not gps. I'd expect gps to be 0x8.

And you'd expect right. Re-tested with 0x8, it works.

>
> The rest looks good, though.
>

Of course it does, it's just copy & paste & replace... I don't even
think I deserve credit for it.

Is it worth it to resubmit the patch for a 1-byte change or will you
just take care of it yourself? If it's easier for you, I'll resubmit.


--
Trepák Vilmos <[email protected]>

2012-10-11 13:18:10

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH] Platform: hp-wmi: add rfkill support for integrated GPS

On Thu, Oct 11, 2012 at 03:15:47PM +0200, Trep?k Vilmos wrote:
> On Thu, 2012-10-11 at 13:34 +0100, Matthew Garrett wrote:
> > I don't think this bit is right - in this codepath, wireless is a mask,
> > so 0x3 would mean wifi and bluetooth, not gps. I'd expect gps to be 0x8.
>
> And you'd expect right. Re-tested with 0x8, it works.

Ok.

> >
> > The rest looks good, though.
> >
>
> Of course it does, it's just copy & paste & replace... I don't even
> think I deserve credit for it.
>
> Is it worth it to resubmit the patch for a 1-byte change or will you
> just take care of it yourself? If it's easier for you, I'll resubmit.

No worries, I'll fix it up. Thanks!

--
Matthew Garrett | [email protected]