On Thursday, January 17, 2013 05:12:24 AM Moore, Robert wrote: > It is correct that ACPICA does not install a default handler for the SystemCMOS address space. > > However, it is conceivable that we could add one. It is a simple thing to do. I think that would be better than adding it the way this patch does. :-) Alternatively, we can just add an empty handler during the Linux-specific initialization. Thanks, Rafael > > -----Original Message----- > > From: Alex Hung [mailto:alex.hung@xxxxxxxxxxxxx] > > Sent: Wednesday, January 16, 2013 7:45 PM > > To: Rafael J. Wysocki > > Cc: Moore, Robert; lenb@xxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx > > Subject: Re: [PATCH] ACPI: add a dummy ACPI RTC driver to handle BIOS's > > accesses to registers delcared in OperationRegion. > > > > On 01/17/2013 07:02 AM, Rafael J. Wysocki wrote: > > > On Wednesday, January 16, 2013 11:15:25 AM Alex Hung wrote: > > >> This is to fix acpica returns an error and terminate AML execution as > > >> soon as BIOS tries to access ACPI's RTC registers. > > >> > > >> BugLink: http://launchpad.net/bugs/1065066 > > > > > > Can you please describe the problem in more detail in the changelog > > itself? > > > Providing a link to the bug report doesn't explain why you think this > > > fix is the best possible one. Which quite frankly I'm not sure of. > > > > > > > Hi, > > > > Sure. > > > > It is found that BIOS declared a RTC device with an OperationRegion as > > below (1). When the BIOS executes accesses to the registers such as in > > Method _Q33 in an EC (2), the kernel outputs messages (3) because there is > > no handler for this OperationRegion. The patch is to create a handler to > > avoid the errors in AML's execution. > > > > (1) > > Device (RTC) > > { > > Name (_HID, EisaId ("PNP0B00")) > > ... > > OperationRegion (CMS0, SystemCMOS, Zero, 0x40) > > Field (CMS0, ByteAcc, NoLock, Preserve) > > { > > RTSE, 8, > > Offset (0x02), > > RTMN, 8, > > Offset (0x04), > > RTHR, 8, > > Offset (0x06), > > RTDY, 8, > > RTDE, 8 > > } > > } > > > > (2) > > Method (_Q33, 0, NotSerialized) > > { > > Store (^^RTC.RTMN, Local0) > > FromBCD (Local0, Local0) > > Store (^^RTC.RTHR, Local1) > > FromBCD (Local1, Local1) > > Store (^^RTC.RTDY, Local2) > > Store (^^RTC.RTSE, Local3) > > ... > > } > > > > (3) > > [ 5553.247507] ACPI Error: No handler for Region [CMS0] > > (ffff88023503b2d0) [SystemCMOS] (20121018/evregion-376) > > [ 5553.247526] ACPI Error: Region SystemCMOS (ID=5) has no handler > > (20121018/exfldio-305) > > [ 5553.247545] ACPI Error: Method parse/execution failed > > [\_SB_.PCI0.LPCB.EC0_._Q33] (Node ffff8802350609d8), AE_NOT_EXIST > > (20121018/psparse-537) > > [ 5553.247603] Failed to execute _Q33 > > > > > > >> Signed-off-by: Alex Hung <alex.hung@xxxxxxxxxxxxx> > > >> --- > > >> drivers/acpi/Kconfig | 9 +++ > > >> drivers/acpi/Makefile | 1 + > > >> drivers/acpi/acpi_rtc.c | 140 > > +++++++++++++++++++++++++++++++++++++++++++++++ > > >> 3 files changed, 150 insertions(+) > > >> create mode 100644 drivers/acpi/acpi_rtc.c > > >> > > >> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig > > >> index 38c5078..fb90397 100644 > > >> --- a/drivers/acpi/Kconfig > > >> +++ b/drivers/acpi/Kconfig > > >> @@ -181,6 +181,15 @@ config ACPI_DOCK > > >> This driver supports ACPI-controlled docking stations and > > removable > > >> drive bays such as the IBM Ultrabay and the Dell Module Bay. > > >> > > >> +config ACPI_RTC > > >> + tristate "RTC" > > >> + default m > > >> + help > > >> + This driver supports an ACPI RTC device. It enables BIOS to read > > and > > >> + to write ACPI RTC registers declared in an OperationRegion with > > >> + RegionSpace as SYSTEMCMOS. This is required if BIOS needs to > > access > > >> + RTC registers during run-time such as a number of HP laptops. > > >> + > > >> config ACPI_I2C > > >> def_tristate I2C > > >> depends on I2C > > >> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile > > >> index 2a4502b..383c62b 100644 > > >> --- a/drivers/acpi/Makefile > > >> +++ b/drivers/acpi/Makefile > > >> @@ -71,6 +71,7 @@ obj-$(CONFIG_ACPI_EC_DEBUGFS) += ec_sys.o > > >> obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o > > >> obj-$(CONFIG_ACPI_BGRT) += bgrt.o > > >> obj-$(CONFIG_ACPI_I2C) += acpi_i2c.o > > >> +obj-$(CONFIG_ACPI_RTC) += acpi_rtc.o > > >> > > >> # processor has its own "processor." module_param namespace > > >> processor-y := processor_driver.o processor_throttling.o > > >> diff --git a/drivers/acpi/acpi_rtc.c b/drivers/acpi/acpi_rtc.c > > >> new file mode 100644 > > >> index 0000000..0ddcfc8 > > >> --- /dev/null > > >> +++ b/drivers/acpi/acpi_rtc.c > > >> @@ -0,0 +1,140 @@ > > >> +/* > > >> + * acpi_rtc - ACPI RTC Driver > > >> + * > > >> + * Copyright (C) 2013 Alex Hung <alex.hung@xxxxxxxxxxxxx> > > >> + * > > >> + * > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > >> + * > > >> + * This program is free software; you can redistribute it and/or > > modify > > >> + * it under the terms of the GNU General Public License as published > > by > > >> + * the Free Software Foundation; either version 2 of the License, or > > (at > > >> + * your option) any later version. > > >> + * > > >> + * This program is distributed in the hope that it will be useful, > > but > > >> + * WITHOUT ANY WARRANTY; without even the implied warranty of > > >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > >> + * General Public License for more details. > > >> + * > > >> + * You should have received a copy of the GNU General Public License > > along > > >> + * with this program; if not, write to the Free Software Foundation, > > Inc., > > >> + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. > > >> + * > > >> + * > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > >> + */ > > >> + > > >> +#include <linux/kernel.h> > > >> +#include <linux/module.h> > > >> +#include <linux/init.h> > > >> +#include <linux/pci.h> > > >> +#include <linux/acpi.h> > > >> +#include <acpi/acpi_bus.h> > > >> +#include <acpi/acpi_drivers.h> > > >> +#include <linux/miscdevice.h> > > >> + > > >> +#define pr_fmt(fmt) "acpi_rtc: " fmt > > >> + > > >> +MODULE_LICENSE("GPL"); > > >> +MODULE_ALIAS("acpi*:PNP0B00:*"); > > >> + > > >> +static const struct acpi_device_id rtc_device_ids[] = { > > >> + {"PNP0B00", 0}, > > >> + {"", 0}, > > >> +}; > > >> + > > >> +static acpi_status acpi_acpi_handle_locate_callback(acpi_handle > > handle, > > >> + u32 level, void *context, void **return_value) > > >> +{ > > >> + struct acpi_device *dev = context; > > >> + dev->handle = handle; > > >> + > > >> + return AE_CTRL_TERMINATE; > > >> +} > > >> + > > >> +static acpi_status > > >> +acpi_rtc_space_handler(u32 function, acpi_physical_address address, > > >> + u32 bits, u64 *value64, > > >> + void *handler_context, void *region_context) > > >> +{ > > >> + return AE_OK; > > >> +} > > >> + > > >> +static int rtc_install_handlers(struct acpi_device *rtc_dev) > > >> +{ > > >> + acpi_status status; > > >> + status = acpi_install_address_space_handler(rtc_dev->handle, > > >> + ACPI_ADR_SPACE_CMOS, > > >> + &acpi_rtc_space_handler, > > >> + NULL, rtc_dev); > > >> + if (ACPI_FAILURE(status)) { > > >> + pr_info("Fail to install ACPI RTC handler\n"); > > >> + return AE_ERROR; > > >> + } > > >> + > > >> + return 0; > > >> +} > > >> + > > >> +static int acpi_rtc_add(struct acpi_device *device) > > >> +{ > > >> + int ret; > > >> + acpi_status status; > > >> + acpi_handle rtc_dev; > > >> + > > >> + status = acpi_get_devices(rtc_device_ids[0].id, > > >> + acpi_acpi_handle_locate_callback, > > >> + &rtc_dev, &rtc_dev); > > >> + if (!ACPI_SUCCESS(status)) > > >> + return AE_NOT_FOUND; > > >> + > > >> + ret = rtc_install_handlers((struct acpi_device *) &rtc_dev); > > >> + > > >> + return ret; > > >> +} > > >> + > > >> +static int acpi_rtc_remove(struct acpi_device *device, int type) > > >> +{ > > >> + acpi_status status; > > >> + status = acpi_remove_address_space_handler(device->handle, > > >> + ACPI_ADR_SPACE_CMOS, > > >> + &acpi_rtc_space_handler); > > >> + if (!ACPI_SUCCESS(status)) > > >> + return AE_ERROR; > > >> + > > >> + return AE_OK; > > >> +} > > >> + > > >> +static struct acpi_driver acpi_rtc_driver = { > > >> + .name = "rtc", > > >> + .class = "ACPI_RTC_CLASS", > > >> + .ids = rtc_device_ids, > > >> + .ops = { > > >> + .add = acpi_rtc_add, > > >> + .remove = acpi_rtc_remove, > > >> + }, > > >> +}; > > >> + > > >> +static int __init rtc_init(void) > > >> +{ > > >> + int err; > > >> + > > >> + pr_info("Initializing ACPI RTC module\n"); > > >> + err = acpi_bus_register_driver(&acpi_rtc_driver); > > >> + if (err) { > > >> + pr_err("Unable to register acpi driver.\n"); > > >> + goto error_acpi_register; > > >> + } > > >> + > > >> + return 0; > > >> + > > >> +error_acpi_register: > > >> + > > >> + return err; > > >> +} > > >> + > > >> +static void __exit rtc_exit(void) > > >> +{ > > >> + pr_info("Exiting ACPI RTC module\n"); > > >> + acpi_bus_unregister_driver(&acpi_rtc_driver); > > >> +} > > >> + > > >> +module_init(rtc_init); > > >> +module_exit(rtc_exit); > > > > > > So all it does is to install an empty address space handler for the CMOS > > > address space, right? > > > > > > Bob, I wonder what you think about that? > > > > > > Rafael > > > > > > > > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
