From owner-freebsd-acpi@FreeBSD.ORG Mon Nov 2 17:03:16 2009 Return-Path: Delivered-To: freebsd-acpi@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 509A2106566B for ; Mon, 2 Nov 2009 17:03:16 +0000 (UTC) (envelope-from avg@icyb.net.ua) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 9F5B58FC08 for ; Mon, 2 Nov 2009 17:03:15 +0000 (UTC) Received: from odyssey.starpoint.kiev.ua (alpha-e.starpoint.kiev.ua [212.40.38.101]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id TAA23185 for ; Mon, 02 Nov 2009 19:03:13 +0200 (EET) (envelope-from avg@icyb.net.ua) Message-ID: <4AEF10D0.7020908@icyb.net.ua> Date: Mon, 02 Nov 2009 19:03:12 +0200 From: Andriy Gapon User-Agent: Thunderbird 2.0.0.23 (X11/20090825) MIME-Version: 1.0 To: freebsd-acpi@FreeBSD.org X-Enigmail-Version: 0.95.7 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Subject: for review: retire 'magic' ivar X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Nov 2009 17:03:16 -0000 ACPI bus defines two ivars with similar purposes - magic and private. It seems that this is redundant. Please review the following patch that removes 'magic' ivar. magic is used in three places, please see below for how it is replaced. 1. acpi_cpu magic is used was storing cpu id. Using integer type for that is, of course, more natural, but 'private' (void *) can be used without any problems. 2. acpi_hpet magic is used to distinguish between a device explicitly created in identify and devices auto-created by bus enumeration. The same could be done by examining 'handle' ivar, because bus enumeration always sets handle to non-NULL, but identify doesn't set it. Please note that there is a buglet in current code that doesn't have any practical consequences: magic is set to address of a variable that holds acpi_hpet_devclass, not to the devclass itself. Thus, if hpet driver were ever unloaded and reloaded it wouldn't recognize its device created on the first identify run. 3. acpi_ec magic is used in this driver similarly to acpi_hpet case. Big difference though, is that acpi_ec identify routine also sets handle ivar of explicitly created device. But it also sets private ivar. So we make use of that fact instead: private is set for explicitly created device, while it is not set for auto-created devices. The same buglet with no consequences is also found here. diff --git a/sys/dev/acpica/acpi.c b/sys/dev/acpica/acpi.c index 525824a..4ed7078 100644 --- a/sys/dev/acpica/acpi.c +++ b/sys/dev/acpica/acpi.c @@ -900,9 +900,6 @@ acpi_read_ivar(device_t dev, device_t child, int index, uintptr_t *result) case ACPI_IVAR_HANDLE: *(ACPI_HANDLE *)result = ad->ad_handle; break; - case ACPI_IVAR_MAGIC: - *(uintptr_t *)result = ad->ad_magic; - break; case ACPI_IVAR_PRIVATE: *(void **)result = ad->ad_private; break; @@ -938,9 +935,6 @@ acpi_write_ivar(device_t dev, device_t child, int index, uintptr_t value) case ACPI_IVAR_HANDLE: ad->ad_handle = (ACPI_HANDLE)value; break; - case ACPI_IVAR_MAGIC: - ad->ad_magic = (uintptr_t)value; - break; case ACPI_IVAR_PRIVATE: ad->ad_private = (void *)value; break; diff --git a/sys/dev/acpica/acpi_cpu.c b/sys/dev/acpica/acpi_cpu.c index 8fe9de6..c16dcb1 100644 --- a/sys/dev/acpica/acpi_cpu.c +++ b/sys/dev/acpica/acpi_cpu.c @@ -255,7 +255,7 @@ acpi_cpu_probe(device_t dev) /* Mark this processor as in-use and save our derived id for attach. */ cpu_softc[cpu_id] = (void *)1; - acpi_set_magic(dev, cpu_id); + acpi_set_private(dev, (void*)(intptr_t)cpu_id); device_set_desc(dev, "ACPI CPU"); return (0); @@ -286,7 +286,7 @@ acpi_cpu_attach(device_t dev) sc = device_get_softc(dev); sc->cpu_dev = dev; sc->cpu_handle = acpi_get_handle(dev); - cpu_id = acpi_get_magic(dev); + cpu_id = (int)(intptr_t)acpi_get_private(dev); cpu_softc[cpu_id] = sc; pcpu_data = pcpu_find(cpu_id); pcpu_data->pc_device = dev; diff --git a/sys/dev/acpica/acpi_ec.c b/sys/dev/acpica/acpi_ec.c index a5a81dc..b339ba1 100644 --- a/sys/dev/acpica/acpi_ec.c +++ b/sys/dev/acpica/acpi_ec.c @@ -129,9 +129,6 @@ struct acpi_ec_params { int uid; }; -/* Indicate that this device has already been probed via ECDT. */ -#define DEV_ECDT(x) (acpi_get_magic(x) == (uintptr_t)&acpi_ec_devclass) - /* * Driver softc. */ @@ -332,7 +329,6 @@ acpi_ec_ecdt_probe(device_t parent) params->uid = ecdt->Uid; acpi_GetInteger(h, "_GLK", ¶ms->glk); acpi_set_private(child, params); - acpi_set_magic(child, (uintptr_t)&acpi_ec_devclass); /* Finish the attach process. */ if (device_probe_and_attach(child) != 0) @@ -348,6 +344,7 @@ acpi_ec_probe(device_t dev) ACPI_STATUS status; device_t peer; char desc[64]; + int ecdt; int ret; struct acpi_ec_params *params; static char *ec_ids[] = { "PNP0C09", NULL }; @@ -362,11 +359,12 @@ acpi_ec_probe(device_t dev) * duplicate probe. */ ret = ENXIO; - params = NULL; + ecdt = 0; buf.Pointer = NULL; buf.Length = ACPI_ALLOCATE_BUFFER; - if (DEV_ECDT(dev)) { - params = acpi_get_private(dev); + params = acpi_get_private(dev); + if (params != NULL) { + ecdt = 1; ret = 0; } else if (!acpi_disabled("ec") && ACPI_ID_PROBE(device_get_parent(dev), dev, ec_ids)) { @@ -439,7 +437,7 @@ out: if (ret == 0) { snprintf(desc, sizeof(desc), "Embedded Controller: GPE %#x%s%s", params->gpe_bit, (params->glk) ? ", GLK" : "", - DEV_ECDT(dev) ? ", ECDT" : ""); + ecdt ? ", ECDT" : ""); device_set_desc_copy(dev, desc); } diff --git a/sys/dev/acpica/acpi_hpet.c b/sys/dev/acpica/acpi_hpet.c index ac28031..875ef07 100644 --- a/sys/dev/acpica/acpi_hpet.c +++ b/sys/dev/acpica/acpi_hpet.c @@ -66,8 +66,6 @@ static void acpi_hpet_test(struct acpi_hpet_softc *sc); static char *hpet_ids[] = { "PNP0103", NULL }; -#define DEV_HPET(x) (acpi_get_magic(x) == (uintptr_t)&acpi_hpet_devclass) - struct timecounter hpet_timecounter = { .tc_get_timecount = hpet_get_timecount, .tc_counter_mask = ~0u, @@ -153,8 +151,6 @@ acpi_hpet_identify(driver_t *driver, device_t parent) return; } - /* Record a magic value so we can detect this device later. */ - acpi_set_magic(child, (uintptr_t)&acpi_hpet_devclass); bus_set_resource(child, SYS_RES_MEMORY, 0, hpet->Address.Address, HPET_MEM_WIDTH); } @@ -166,7 +162,7 @@ acpi_hpet_probe(device_t dev) if (acpi_disabled("hpet")) return (ENXIO); - if (!DEV_HPET(dev) && + if (acpi_get_handle(dev) != NULL && (ACPI_ID_PROBE(device_get_parent(dev), dev, hpet_ids) == NULL || device_get_unit(dev) != 0)) return (ENXIO); diff --git a/sys/dev/acpica/acpivar.h b/sys/dev/acpica/acpivar.h index f4d27e4..abfaa8b 100644 --- a/sys/dev/acpica/acpivar.h +++ b/sys/dev/acpica/acpivar.h @@ -88,7 +88,6 @@ struct acpi_softc { struct acpi_device { /* ACPI ivars */ ACPI_HANDLE ad_handle; - uintptr_t ad_magic; void *ad_private; int ad_flags; @@ -224,7 +223,7 @@ extern int acpi_quirks; * attach to ACPI. */ #define ACPI_IVAR_HANDLE 0x100 -#define ACPI_IVAR_MAGIC 0x101 +#define ACPI_IVAR_UNUSED 0x101 /* Unused/reserved. */ #define ACPI_IVAR_PRIVATE 0x102 #define ACPI_IVAR_FLAGS 0x103 @@ -250,7 +249,6 @@ } __ACPI_BUS_ACCESSOR(acpi, handle, ACPI, HANDLE, ACPI_HANDLE) -__ACPI_BUS_ACCESSOR(acpi, magic, ACPI, MAGIC, uintptr_t) __ACPI_BUS_ACCESSOR(acpi, private, ACPI, PRIVATE, void *) __ACPI_BUS_ACCESSOR(acpi, flags, ACPI, FLAGS, int) -- Andriy Gapon