From owner-freebsd-acpi@FreeBSD.ORG Tue May 15 07:28:17 2007 Return-Path: X-Original-To: acpi@freebsd.org Delivered-To: freebsd-acpi@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 0B6FF16A400 for ; Tue, 15 May 2007 07:28:17 +0000 (UTC) (envelope-from nate@root.org) Received: from root.org (root.org [67.118.192.226]) by mx1.freebsd.org (Postfix) with ESMTP id D616813C487 for ; Tue, 15 May 2007 07:28:16 +0000 (UTC) (envelope-from nate@root.org) Received: (qmail 6201 invoked from network); 15 May 2007 07:28:19 -0000 Received: from ppp-71-139-42-13.dsl.snfc21.pacbell.net (HELO ?10.0.5.18?) (nate-mail@71.139.42.13) by root.org with ESMTPA; 15 May 2007 07:28:19 -0000 Message-ID: <46496103.3000407@root.org> Date: Tue, 15 May 2007 00:28:03 -0700 From: Nate Lawson User-Agent: Thunderbird 2.0.0.0 (X11/20070424) MIME-Version: 1.0 To: Takanori Watanabe References: <200705150522.l4F5MNXL022699@sana.init-main.com> In-Reply-To: <200705150522.l4F5MNXL022699@sana.init-main.com> X-Enigmail-Version: 0.95.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: acpi@freebsd.org Subject: Re: acpi_hpet not found 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: Tue, 15 May 2007 07:28:17 -0000 Takanori Watanabe wrote: > In message <200705150309.l4F39Z9d021438@sana.init-main.com>, Takanori Watanabe > さんいわく: >> In message <4648D460.9040400@root.org>, Nate Lawson wrote: >>> >>> But he has no Device of type PNP0103. So we need to implement >>> table-based HPET probing, not just AML PNP probing. This is similar to >>> how we handle ECDT for the embedded controller. >>> >>> When we do that, we also need to work around the fact that the address >>> bit width is wrong (0). >> I have patch for this on RELENG_6. Thanks for writing this. Comments below. > And I have it for CURRENT.(Untested) > > > Index: acpi.c > =================================================================== > RCS file: /home/ncvs/src/sys/dev/acpica/acpi.c,v > retrieving revision 1.235 > diff -u -r1.235 acpi.c > - --- acpi.c 25 Apr 2007 16:22:18 -0000 1.235 > +++ acpi.c 30 Apr 2007 15:58:34 -0000 > @@ -503,6 +503,7 @@ > * a problem but should be addressed eventually. > */ > acpi_ec_ecdt_probe(dev); > + acpi_hpet_table_probe(dev); > > /* Bring device objects and regions online. */ > if (ACPI_FAILURE(status = AcpiInitializeObjects(flags))) { > Index: acpi_hpet.c > =================================================================== > RCS file: /home/ncvs/src/sys/dev/acpica/acpi_hpet.c,v > retrieving revision 1.6 > diff -u -r1.6 acpi_hpet.c > - --- acpi_hpet.c 28 Mar 2007 22:28:48 -0000 1.6 > +++ acpi_hpet.c 6 Apr 2007 04:10:51 -0000 > @@ -41,6 +41,8 @@ > > ACPI_SERIAL_DECL(hpet, "ACPI HPET support"); > > +static devclass_t acpi_hpet_devclass; > + > /* ACPI CA debugging */ > #define _COMPONENT ACPI_TIMER > ACPI_MODULE_NAME("HPET") > @@ -77,17 +79,52 @@ > sc = tc->tc_priv; > return (bus_read_4(sc->mem_res, HPET_OFFSET_VALUE)); > } > +#define DEV_HPET(x) (acpi_get_magic(x) == (int)&acpi_hpet_devclass) add newline here. > +void > +acpi_hpet_table_probe(device_t parent) > +{ > + ACPI_TABLE_HPET *hpet; > + ACPI_TABLE_HEADER *hdr; > + ACPI_STATUS status; > + device_t child; > + > + /*Currently, id and minimam clock tick info. is discarded.*/ > + > + status = AcpiGetTable(ACPI_SIG_HPET, 1, (ACPI_TABLE_HEADER **)&hdr); > + remove newline here. > + if (ACPI_FAILURE(status)) > + return; > + > + hpet = (ACPI_TABLE_HPET *) hdr; > + child = BUS_ADD_CHILD(parent, 0, "acpi_hpet", hpet->Sequence); Perhaps we should use just a fixed unit 0 instead of hpet->Sequence? Is there ever going to be more than one timer? Also, I worry that some bios tables might get this value wrong. It might be best to use hpet->Sequence and printf a warning if it != 0. > + if (child == NULL) { > + printf("%s: can't add child\n", __func__); > + return; > + } > + > + acpi_set_magic(child, (int)&acpi_hpet_devclass); > + bus_set_resource(child, SYS_RES_MEMORY, 0, hpet->Address.Address, HPET_MEM_WIDTH); > + if(device_probe_and_attach(child) != 0) > + device_delete_child(parent, child); > +} > > static int > acpi_hpet_probe(device_t dev) > { > ACPI_FUNCTION_TRACE((char *)(uintptr_t) __func__); > - - > - - if (acpi_disabled("hpet") || > - - ACPI_ID_PROBE(device_get_parent(dev), dev, hpet_ids) == NULL || > - - device_get_unit(dev) != 0) > + /*First of all, hpet hpet should not disabled.*/ > + if(acpi_disabled("hpet")) > + return ENXIO; > + > + /*Already knows */ > + if(DEV_HPET(dev)){ > + return 0; > + } The above code does not set the device description string (below) when it returns. It would be better to combine this with the below if()... if (!DEV_HPET(dev) && (ACPI_ID_PROBE... > + if (ACPI_ID_PROBE(device_get_parent(dev), dev, hpet_ids) == NULL || > + devclass_get_device(acpi_hpet_devclass, device_get_unit(dev))) > return (ENXIO); > - - > + > device_set_desc(dev, "High Precision Event Timer"); > return (0); > } > @@ -211,7 +248,6 @@ > sizeof(struct acpi_hpet_softc), > }; > > - -static devclass_t acpi_hpet_devclass; > > DRIVER_MODULE(acpi_hpet, acpi, acpi_hpet_driver, acpi_hpet_devclass, 0, 0); > MODULE_DEPEND(acpi_hpet, acpi, 1, 1, 1); > Index: acpivar.h > =================================================================== > RCS file: /home/ncvs/src/sys/dev/acpica/acpivar.h,v > retrieving revision 1.104 > diff -u -r1.104 acpivar.h > - --- acpivar.h 22 Mar 2007 18:16:40 -0000 1.104 > +++ acpivar.h 28 Mar 2007 15:44:43 -0000 > @@ -407,6 +407,8 @@ > > /* Embedded controller. */ > void acpi_ec_ecdt_probe(device_t); > +/* HPET table probe*/ > +void acpi_hpet_table_probe(device_t); > > /* AC adapter interface. */ > int acpi_acad_get_acline(int *); -- Nate