Date: Fri, 29 Sep 2000 18:23:11 +0900 From: Mitsuru IWASAKI <iwasaki@jp.FreeBSD.org> To: msmith@freebsd.org Cc: iwasaki@jp.FreeBSD.org, takawata@shidahara1.planet.sci.kobe-u.ac.jp, haro@tk.kubota.co.jp, current@freebsd.org, acpi-jp@jp.FreeBSD.org Subject: Re: My system hang with ACPI kernel thread Message-ID: <20000929182311Q.iwasaki@jp.FreeBSD.org> In-Reply-To: <200009281819.e8SIJhA01660@mass.osd.bsdi.com> References: <20000929023628R.iwasaki@jp.FreeBSD.org> <200009281819.e8SIJhA01660@mass.osd.bsdi.com> <200009290521.e8T5LUA03595@mass.osd.bsdi.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> > > > Currently kernel thread seems broken, so mallocing storage in > > > > acpi_queue_event() never be freed. I think number of events at a > > > > point of tme is limited and we can have static storage for the events. > > > > The implementaion of sys/i386/apm/apm.c:apm_record_event() (it's for apmd) > > > > would be a good example. > > > > > > I have a megapatch for acpi.c that I am nearly ready to commit which > > > converts it to use bus resources for all I/O allocations. I'll fix this > > > in there, if you like. > > > > I'm interested in your patch. Can I see it? > > Sure. I'm not 100% sure it's going to work right, so please feel free > to tell me I've broken something... I've just tried the patch, GREAT! it seems working for me perfectly w/o any functional changes, much better than before. I'll do testing more. I have some comments; # most of them is not related with your patch :-) but I'd like to # consult with you. Can we separate the code which uses FreeBSD specific APIs and structs into a other file or arrange them at one place? Because I'm going to merge NetBSD porting effort, I want to avoid having too many #ifdef __FreeBSD__. The patch is available at http://www.imou.to/~AoiMoe/UNIX-at-Random/acpi/acpi-freebsd-netbsd-changes-2000-09-24.diff.gz Because of above reason, /* * Debugging, console output * * XXX this should really be using device_printf */ extern int acpi_debug; #define ACPI_DEVPRINTF(args...) printf("acpi0: " args) I don't use device_printf() here. # Also we don't have more than 2 instances of acpi :-) static void acpi_trans_sleeping_state(acpi_softc_t *sc, u_int8_t state) : /* XXX should be MI */ /* XXX should always be called with interrupts enabled! */ ef = read_eflags(); disable_intr(); for this, I referred the comments in ACPI spec 7.5.2. // Required environment: Executing on the system boot // processor. All other processors stopped. Interrupts <-- // disabled. All Power Resources (and devices) are in // corresponding device state to support NewState. I don't know much about IA64, I'm not sure {read|write}_eflags() inline functions will be available on IA64. Should we replace them with save_intr() and restore_intr() ? because seems more general. Also we need to consider SMP. There is no hack for it so far. # I think APM BIOS Call need to be executed on the system boot # processor too. acpi_soft_off(void *data, int howto) : /* XXX Disable GPE intrrupt,or power on again in some machine */ acpi_io_gpe0_enable(sc, ACPI_REGISTER_OUTPUT, &vala); acpi_io_gpe1_enable(sc, ACPI_REGISTER_OUTPUT, &vala); This still give no effect on my PORTEGE. I think this should be done in earlier phase. Maybe we'd better to have acpi_disable_events() and call this at shutdown_pre_sync (or some such) for shutdown -p, also call this in acpi_set_sleeping_state() for power button/acpiconf -s 5. acpi_intr(void *data) : #if 0 /* Clear interrupt status */ val = enable; /* XXX */ acpi_io_gpe0_status(sc, ACPI_REGISTER_OUTPUT, &val); /* Re-enable interrupt */ acpi_io_gpe0_enable(sc, ACPI_REGISTER_OUTPUT, &enable); acpi_debug = 0; /* Shut up again */ #endif GPE1 too, right? :-) acpi_attach(device_t dev) : sc->iores[i].rsc = bus_alloc_resource(dev, SYS_RES_IOPORT, &sc->iores[i].rid, 0, ~0, 1, RF_ACTIVE); ^^ I didn't understand clearly for long time, but this give us more flexibility w/o problems if we call bus_set_resource() and set count correctly, right? #ifndef ACPI_NO_ENABLE_ON_BOOT acpi_enable_disable(sc, 1); acpi_enable_events(sc); acpi_intr((void *)sc); #endif Should we remove them and have acpi_enalbe="YES" in /etc/rc.conf just line APM ? And last thing, how about header file name and location? some poeple suggested that /sys/dev/acpi/acpi.h should be renamed to acpivar.h. And /sys/sys/acpi.h should be moved to /sys/dev/acpi.h (installed in /usr/include/dev/acpi/acpi.h). We didn't care and are not interested in this matter at all so far, but it seems quite reasonable for me and doesn't hurt anything. And *real* last thing :-) msmith> the code in machdep.c. Everything will move to acpi_machdep.c I'll also msmith> be deleting <machine/acpi.h>, as it's not necessary (and never was). I think better to wait deleting until IA64 porting. I'm not sure if this file really unnecessary or not. Thanks! To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20000929182311Q.iwasaki>