for review: retire 'magic' ivar
Andriy Gapon
avg at icyb.net.ua
Mon Nov 2 17:03:16 UTC 2009
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
More information about the freebsd-acpi
mailing list