[PATCH 0/2] eeepc-laptop updates for 2.6.32-rcX

[PATCH 0/2] eeepc-laptop updates for 2.6.32-rcX

[Corentin Chary]

Hi Len,
Two more patch for eeepc-laptop, the first revert a previous commit:
> a much better patch for this
> (which fixes the problem directly in rt2860sta) arrived in Greg KH's
> staging.current queue late yesterday.

The second really speed up the boot process for Eeepc.
Thanks,

Corentin Chary (1):
  Revert "eeepc-laptop: Prevent a panic when disabling RT2860 wireless
    when associated"

Luca Niccoli (1):
  eeepc-laptop: don't enable camera at startup if it's already on.

 drivers/platform/x86/eeepc-laptop.c |   30 ++++++++++--------------------
 1 files changed, 10 insertions(+), 20 deletions(-)

[PATCH 1/2] Revert "eeepc-laptop: Prevent a panic when disabling RT2860 wireless when associated"

[Corentin Chary]

rt2860sta is fine with the patch as is, but iwl3945 isn't
(eeepc_rfkill_set() needs to call eeepc_rfkill_hotplug(true) – which means
that we're back to causing the rt2860sta panic

This reverts commit b56ab33d68638e6aafdbfc694025e8354a628f49.

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
Signed-off-by: Darren Salt <linux@youmustbejoking.demon.co.uk>
---
 drivers/platform/x86/eeepc-laptop.c |   27 ++++++++-------------------
 1 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
index d379e74..789d6ae 100644
--- a/drivers/platform/x86/eeepc-laptop.c
+++ b/drivers/platform/x86/eeepc-laptop.c
@@ -150,8 +150,6 @@ struct eeepc_hotk {
 /* The actual device the driver binds to */
 static struct eeepc_hotk *ehotk;
 
-static void eeepc_rfkill_hotplug(bool real);
-
 /* Platform device/driver */
 static int eeepc_hotk_thaw(struct device *device);
 static int eeepc_hotk_restore(struct device *device);
@@ -345,16 +343,7 @@ static bool eeepc_wlan_rfkill_blocked(void)
 static int eeepc_rfkill_set(void *data, bool blocked)
 {
 	unsigned long asl = (unsigned long)data;
-	int ret;
-
-	if (asl != CM_ASL_WLAN)
-		return set_acpi(asl, !blocked);
-
-	/* hack to avoid panic with rt2860sta */
-	if (blocked)
-		eeepc_rfkill_hotplug(false);
-	ret = set_acpi(asl, !blocked);
-	return ret;
+	return set_acpi(asl, !blocked);
 }
 
 static const struct rfkill_ops eeepc_rfkill_ops = {
@@ -654,13 +643,13 @@ static int eeepc_get_adapter_status(struct hotplug_slot *hotplug_slot,
 	return 0;
 }
 
-static void eeepc_rfkill_hotplug(bool real)
+static void eeepc_rfkill_hotplug(void)
 {
 	struct pci_dev *dev;
 	struct pci_bus *bus;
-	bool blocked = real ? eeepc_wlan_rfkill_blocked() : true;
+	bool blocked = eeepc_wlan_rfkill_blocked();
 
-	if (real && ehotk->wlan_rfkill)
+	if (ehotk->wlan_rfkill)
 		rfkill_set_sw_state(ehotk->wlan_rfkill, blocked);
 
 	mutex_lock(&ehotk->hotplug_lock);
@@ -703,7 +692,7 @@ static void eeepc_rfkill_notify(acpi_handle handle, u32 event, void *data)
 	if (event != ACPI_NOTIFY_BUS_CHECK)
 		return;
 
-	eeepc_rfkill_hotplug(true);
+	eeepc_rfkill_hotplug();
 }
 
 static void eeepc_hotk_notify(struct acpi_device *device, u32 event)
@@ -861,7 +850,7 @@ static int eeepc_hotk_restore(struct device *device)
 {
 	/* Refresh both wlan rfkill state and pci hotplug */
 	if (ehotk->wlan_rfkill)
-		eeepc_rfkill_hotplug(true);
+		eeepc_rfkill_hotplug();
 
 	if (ehotk->bluetooth_rfkill)
 		rfkill_set_sw_state(ehotk->bluetooth_rfkill,
@@ -1004,7 +993,7 @@ static void eeepc_rfkill_exit(void)
 	 * Refresh pci hotplug in case the rfkill state was changed after
 	 * eeepc_unregister_rfkill_notifier()
 	 */
-	eeepc_rfkill_hotplug(true);
+	eeepc_rfkill_hotplug();
 	if (ehotk->hotplug_slot)
 		pci_hp_deregister(ehotk->hotplug_slot);
 
@@ -1120,7 +1109,7 @@ static int eeepc_rfkill_init(struct device *dev)
 	 * Refresh pci hotplug in case the rfkill state was changed during
 	 * setup.
 	 */
-	eeepc_rfkill_hotplug(true);
+	eeepc_rfkill_hotplug();
 
 exit:
 	if (result && result != -ENODEV)
-- 
1.6.5.rc2

--
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

[PATCH 2/2] eeepc-laptop: don't enable camera at startup if it's already on.

[Corentin Chary]

From: Luca Niccoli <lultimouomo@gmail.com>

Switching the camera takes 500ms, checking if it's on is almost free...
The BIOS remembers the setting through reboots, so there's good chance the
camera is already enabled.

Signed-off-by: Luca Niccoli <lultimouomo@gmail.com>
Cc: Corentin Chary <corentincj@iksaif.net>
Cc: Len Brown <len.brown@intel.com>
Cc: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
Cc: Matthew Garrett <mjg@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/platform/x86/eeepc-laptop.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c
index 789d6ae..4226e53 100644
--- a/drivers/platform/x86/eeepc-laptop.c
+++ b/drivers/platform/x86/eeepc-laptop.c
@@ -356,7 +356,8 @@ static void __devinit eeepc_enable_camera(void)
 	 * If the following call to set_acpi() fails, it's because there's no
 	 * camera so we can ignore the error.
 	 */
-	set_acpi(CM_ASL_CAMERA, 1);
+	if (get_acpi(CM_ASL_CAMERA) == 0)
+		set_acpi(CM_ASL_CAMERA, 1);
 }
 
 /*
-- 
1.6.5.rc2

Re: [PATCH 2/2] eeepc-laptop: don't enable camera at startup if it's already on.

[Andrey Rahmatullin]

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

On Fri, Oct 16, 2009 at 10:22:47PM +0200, Corentin Chary wrote:
> Switching the camera takes 500ms, checking if it's on is almost free...
So if I don't want camera to be enabled, I should wait 500ms on boot to
enable it and then other 500ms to disable?

-- 
WBR, wRAR (ALT Linux Team)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 2/2] eeepc-laptop: don't enable camera at startup if it's already on.

[Alan Jenkins]

On 10/20/09, Andrey Rahmatullin <wrar@altlinux.org> wrote:
> On Fri, Oct 16, 2009 at 10:22:47PM +0200, Corentin Chary wrote:
>> Switching the camera takes 500ms, checking if it's on is almost free...
> So if I don't want camera to be enabled, I should wait 500ms on boot to
> enable it and then other 500ms to disable?

Yup.  The original patch from Pekka was accepted alongside a patch
which enabled auto-suspend for the uvcvideo driver by default.  This
was shown to have the same effect on CPU wakeups as measured by
powertop.

So the question is why do you want to disable it?  Have you measured
the power consumption on the current kernel using a more comprehensive
method than powertop?

Or perhaps you have a different camera chip?  Have you tried enabling
auto-suspend manually?  If it works, perhaps that driver can enable
auto-suspend as well.

Regards
Alan

Re: [PATCH 2/2] eeepc-laptop: don't enable camera at startup if it's already on.

[Andrey Rahmatullin]

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

On Tue, Oct 20, 2009 at 10:04:04AM +0100, Alan Jenkins wrote:
> Yup.  The original patch from Pekka was accepted alongside a patch
> which enabled auto-suspend for the uvcvideo driver by default.  This
> was shown to have the same effect on CPU wakeups as measured by
> powertop.
When the camera is enabled, powertop says "device is active 100% of time".
That was enough for me :)

> Have you measured the power consumption on the current kernel using a
> more comprehensive method than powertop?
No.

> Or perhaps you have a different camera chip?  Have you tried enabling
> auto-suspend manually?  If it works, perhaps that driver can enable
> auto-suspend as well.
/sys/module/usbcore/parameters/autosuspend and
/sys/devices/pci0000:00/0000:00:1d.7/usb5/5-8/power/autosuspend both
contain 1, but powertop still suggests adding usbcore.autosuspend=1.
Though nothing related to USB exists in wakeup causes list.

-- 
WBR, wRAR (ALT Linux Team)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 2/2] eeepc-laptop: don't enable camera at startup if it's already on.

[Alan Jenkins]

On 10/20/09, Andrey Rahmatullin <wrar@altlinux.org> wrote:
> On Tue, Oct 20, 2009 at 10:04:04AM +0100, Alan Jenkins wrote:
>> Yup.  The original patch from Pekka was accepted alongside a patch
>> which enabled auto-suspend for the uvcvideo driver by default.  This
>> was shown to have the same effect on CPU wakeups as measured by
>> powertop.
> When the camera is enabled, powertop says "device is active 100% of time".
> That was enough for me :)

yes but if you are missing the original "enable camera by default"
patch  *which has already been merged* then you are almost certainly
also missing the patch to enable autosuspend on uvcvideo by default.

>> Have you measured the power consumption on the current kernel using a
>> more comprehensive method than powertop?
> No.
>
>> Or perhaps you have a different camera chip?  Have you tried enabling
>> auto-suspend manually?  If it works, perhaps that driver can enable
>> auto-suspend as well.
> /sys/module/usbcore/parameters/autosuspend and
> /sys/devices/pci0000:00/0000:00:1d.7/usb5/5-8/power/autosuspend both
> contain 1, but powertop still suggests adding usbcore.autosuspend=1.
> Though nothing related to USB exists in wakeup causes list.

Yeah, powertop's autosuspend recommendation is a bit outdated.

Here's a link to the original thread.
<http://lkml.indiana.edu/hypermail/linux/kernel/0906.0/02603.html>.
The command I used to manually enable autosuspend was

echo -n auto > /sys/bus/usb/drivers/uvcvideo/*:*/../power/level

obviously this will need adjusting if the driver is different.  (This
is still the same "power" directory at the end, you're just reaching
it through a different path).

I hope this addresses your concern.
Alan

Re: [PATCH 2/2] eeepc-laptop: don't enable camera at startup if it's already on.

[Andrey Rahmatullin]

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

On Tue, Oct 20, 2009 at 10:38:17AM +0100, Alan Jenkins wrote:
> yes but if you are missing the original "enable camera by default"
> patch  *which has already been merged* then you are almost certainly
> also missing the patch to enable autosuspend on uvcvideo by default.
No, I'm not missing it, I'm using .32-rc5

> Yeah, powertop's autosuspend recommendation is a bit outdated.
So, can I safely ignore it and assume all is right when I use latest
kernels?

> Here's a link to the original thread.
> <http://lkml.indiana.edu/hypermail/linux/kernel/0906.0/02603.html>.
OK, I'll read it, thanks.

-- 
WBR, wRAR (ALT Linux Team)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 2/2] eeepc-laptop: don't enable camera at startup if it's already on.

[Alan Jenkins]

On 10/20/09, Andrey Rahmatullin <wrar@altlinux.org> wrote:
> On Tue, Oct 20, 2009 at 10:38:17AM +0100, Alan Jenkins wrote:
>> yes but if you are missing the original "enable camera by default"
>> patch  *which has already been merged* then you are almost certainly
>> also missing the patch to enable autosuspend on uvcvideo by default.
> No, I'm not missing it, I'm using .32-rc5
>
>> Yeah, powertop's autosuspend recommendation is a bit outdated.
> So, can I safely ignore it and assume all is right when I use latest
> kernels?

Sorry, I assumed this was the case but I didn't check properly.  As
Luca observes it's not been merged yet.  <google>.

The patch exists in Fedora
<http://osdir.com/ml/fedora-extras-commits/2009-07/msg06079.html>.

with the aim of beta-testing it
<http://lkml.indiana.edu/hypermail/linux/kernel/0909.1/03142.html>.
It's fair to say there's a degree of risk to it.  UVC is a "standard"
which presumably has various implementations with their own potential
bugs.

In the mean time - the thread I linked to includes a helpful note from
Xandros.  They measured that enabling the camera on the 901 model cost
~3% of overall power.

I guess the argument is that it's better to waste a little power,
rather than make life hard for people who install Linux themselves
:-/.  After all these devices are mainly shipped with Windows, and the
original pre-installed Linux (which hacked around this in video
applications) is not seriously maintained.  The problem is that when
the pre-installed OS uses the hack, the camera will be disabled in the
BIOS.  When you install a generic version of Linux without hacked
applications, you either have eeepc-laptop enable the camera, or the
user has to learn how to do it themself.

I don't have a strong opinion myself, so long as it's fixed properly
in the long term.

Alan

Re: [PATCH 2/2] eeepc-laptop: don't enable camera at startup if it's already on.

[Luca Niccoli]

2009/10/20 Alan Jenkins <sourcejedi.lkml@googlemail.com>:

> I guess the argument is that it's better to waste a little power,
> rather than make life hard for people who install Linux themselves
> :-/.  After all these devices are mainly shipped with Windows, and the
> original pre-installed Linux (which hacked around this in video
> applications) is not seriously maintained.  The problem is that when
> the pre-installed OS uses the hack, the camera will be disabled in the
> BIOS.  When you install a generic version of Linux without hacked
> applications, you either have eeepc-laptop enable the camera, or the
> user has to learn how to do it themself.
>
> I don't have a strong opinion myself, so long as it's fixed properly
> in the long term.

For the short term, I guess that distributions that ship some
eee-related package could make it include an udev rule - I'm going to
propose this in Debian.
For the long term:
if having blanket-enabled autosuspend is shown to rarely raise
problems, maybe a blacklist could be added in default udev rules?

Luca
--
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

Re: [PATCH 2/2] eeepc-laptop: don't enable camera at startup if it's already on.

[Luca Niccoli]

2009/10/20 Alan Jenkins <sourcejedi.lkml@googlemail.com>:

> So the question is why do you want to disable it?  Have you measured
> the power consumption on the current kernel using a more comprehensive
> method than powertop?

Actually, on my eee 901, linux 2.6.31, udev 146,
I have /sys/bus/usb/drivers/uvcvideo/*:*/../power/level = on
If I manually set level to auto, I save 0.2W
How's that supposed to work? Is it kernel's or udev responsibility to
set the autosuspend?
Cheers,

Luca
--
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

Re: [PATCH 2/2] eeepc-laptop: don't enable camera at startup if it's already on.

[Len Brown]

applied

thanks,
Len Brown, Intel Open Source Technology Center

Re: [PATCH 1/2] Revert "eeepc-laptop: Prevent a panic when disabling RT2860 wireless when associated"

[Len Brown]

applied

thanks,
Len Brown, Intel Open Source Technology Center