[LPIA Hardy LUM] Disable RTC wake alarm to reduce power in Hibernate - attempt #3
Andy Whitcroft
apw at canonical.com
Wed Mar 18 14:38:41 UTC 2009
On Wed, Mar 18, 2009 at 01:37:24PM +0000, Colin Ian King wrote:
> From c625b8b71ba76f2d9b3944dc3eb90fb81bc188bd Mon Sep 17 00:00:00 2001
> From: Colin Ian King <colin.king at canonical.com>
> Date: Wed, 18 Mar 2009 13:25:22 +0000
> Subject: [PATCH] UBUNTU: SAUCE: hpmini: Disable RTC wake alarm to reduce power in Hibernate
> Bug: #337943
>
> Add sysfs control to enable or disable RTC Event Enable (RTC_EN) bit
> of port ACPI PM1a_EVT_BLK + 2 because the BIOS overlooked this
> functionality on the HP Mini. Disabling this during hibernate saves
> several mAh.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
> ubuntu/misc/hpmini.c | 108 +++++++++++++++++++++++++++++++++++++++++--------
> 1 files changed, 90 insertions(+), 18 deletions(-)
>
> diff --git a/ubuntu/misc/hpmini.c b/ubuntu/misc/hpmini.c
> index e3e076f..abca758 100644
> --- a/ubuntu/misc/hpmini.c
> +++ b/ubuntu/misc/hpmini.c
> @@ -32,8 +32,8 @@
>
> /* module and version information */
> #define DRIVER_NAME "HP Mini"
> -#define DRIVER_VERSION "1.00"
> -#define DRIVER_RELEASE_DATE "06-Nov-2008"
> +#define DRIVER_VERSION "1.00ubuntu1"
Better version...
> +#define DRIVER_RELEASE_DATE "18-Mar-2009"
> #define PREFIX DRIVER_NAME ": "
>
> /* Change this to non-zero to enable debug output. */
> @@ -59,6 +59,10 @@ static struct platform_device *hpmini_platform_device;
> * LPC Interface Bridge */
> static u32 gpio_base_address;
>
> +/* the base address of I/O space for PM as read from the ICH7
> + * LPC Interface Bridge */
> +static u32 pm_base_address;
> +
> /* address of 32-bit GPIO base address register on the ICH7
> * LPC Interface Bridge */
> #define GPIO_BASE_ADDRESS_REGISTER 0x48
> @@ -76,6 +80,20 @@ static u32 gpio_base_address;
> #define WIFI_RADIO_ON 0x8 /* GPIO27 */
> #define ETHERNET_ON 0x10 /* GPIO28 */
>
> +/* address of 32-bit PM base address register on the ICH7
> + * LPC Interface Bridge */
> +#define PM_BASE_ADDRESS_REGISTER 0x40
> +
> +/* Only bits 15:7 of the PM base address register are valid. */
> +#define PM_BASE_ADDRESS_MASK 0xff80
> +
> +/* offsets from the I/O base address for specific PM signals */
> +#define PM_RTC_EN_OFFSET 0x2
> +
> +/* RTC Event Enable (RTC_EN) */
> +#define RTC_EN 0x400
> +
> +
> /*
> * sysfs callback functions and files
> */
> @@ -249,29 +267,74 @@ static ssize_t ethernet_set(struct device *dev, struct device_attribute *devattr
> return count;
> }
>
> +static ssize_t rtc_show(struct device *dev,
> + struct device_attribute *devattr, char *buf)
> +{
> + u16 data;
> +
> + data = inw(pm_base_address + PM_RTC_EN_OFFSET);
> +
> + DPRINT("%s: status = 0x%x\n", __func__, data);
> +
> + return snprintf(buf, PAGE_SIZE, "%u\n", (data & RTC_EN) ? 1 : 0);
> +}
> +
> +static ssize_t rtc_set(struct device *dev, struct device_attribute *devattr,
> + const char *buf, size_t count)
> +{
> + unsigned long val;
> + u16 original_data;
> + u16 new_data;
> +
> + val = simple_strtoul(buf, NULL, 10);
Checkpatch suggests strict_strtoul here, any reason to not use that
(safer) function?
> + DPRINT("%s: new value = %lu\n", __func__, val);
> + /* See section 10.8.3.2 of Intel(TM) I/O Controller
> + * Hub 7 (ICH7) Family data sheet */
> + original_data = new_data = inw(pm_base_address + PM_RTC_EN_OFFSET);
> + new_data = original_data;
That looks to be a double assignment to new_data?
> +
> + if (val)
> + new_data |= RTC_EN;
> + else
> + new_data &= ~RTC_EN;
> +
> + if (new_data != original_data)
> + outw(new_data, pm_base_address + PM_RTC_EN_OFFSET);
I might recommend a blank line here.
> + return count;
> +}
> +
> +
> +
> static SENSOR_DEVICE_ATTR(bluetooth, /* name */
> - S_IWUSR | S_IRUGO, /* mode */
> - bluetooth_show, /* show */
> - bluetooth_set, /* store */
> - 0); /* index */
> + S_IWUSR | S_IRUGO, /* mode */
> + bluetooth_show, /* show */
> + bluetooth_set, /* store */
> + 0); /* index */
>
> static SENSOR_DEVICE_ATTR(wifi, /* name */
> - S_IWUSR | S_IRUGO, /* mode */
> - wifi_show, /* show */
> - wifi_set, /* store */
> - 0); /* index */
> + S_IWUSR | S_IRUGO, /* mode */
> + wifi_show, /* show */
> + wifi_set, /* store */
> + 1); /* index */
>
> static SENSOR_DEVICE_ATTR(wwan, /* name */
> - S_IWUSR | S_IRUGO, /* mode */
> - wwan_show, /* show */
> - wwan_set, /* store */
> - 0); /* index */
> + S_IWUSR | S_IRUGO, /* mode */
> + wwan_show, /* show */
> + wwan_set, /* store */
> + 2); /* index */
Ok these are all cleanups right? and should be in the previous patch?
> static SENSOR_DEVICE_ATTR(ethernet, /* name */
> - S_IWUSR | S_IRUGO, /* mode */
> - ethernet_show, /* show */
> - ethernet_set, /* store */
> - 0); /* index */
> + S_IWUSR | S_IRUGO, /* mode */
> + ethernet_show, /* show */
> + ethernet_set, /* store */
> + 3); /* index */
> +
> +static SENSOR_DEVICE_ATTR(rtc, /* name */
> + S_IWUSR | S_IRUGO, /* mode */
> + rtc_show, /* show */
> + rtc_set, /* store */
> + 4); /* index */
>
> static struct attribute *hpmini_attributes[] =
> {
> @@ -279,6 +342,7 @@ static struct attribute *hpmini_attributes[] =
> &sensor_dev_attr_wifi.dev_attr.attr,
> &sensor_dev_attr_wwan.dev_attr.attr,
> &sensor_dev_attr_ethernet.dev_attr.attr,
> + &sensor_dev_attr_rtc.dev_attr.attr,
> NULL
> };
>
> @@ -319,6 +383,14 @@ static int __devinit hpmini_probe(struct platform_device *dev)
>
> DPRINT("base address = 0x%x\n", gpio_base_address);
>
> + pci_read_config_dword(pcidev, PM_BASE_ADDRESS_REGISTER,
> + &pm_base_address);
> + /* Only bits 15:7 of the PM base address register are valid,
> + * so mask off all other bits */
> + pm_base_address &= PM_BASE_ADDRESS_MASK;
> +
> + DPRINT("base address = 0x%x\n", pm_base_address);
> +
> return 0;
> }
-apw
More information about the kernel-team
mailing list