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. Bob > -----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 > > > > ��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
