From owner-freebsd-acpi@FreeBSD.ORG Tue May 22 16:30:08 2007 Return-Path: X-Original-To: freebsd-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 75AD416A46D for ; Tue, 22 May 2007 16:30:08 +0000 (UTC) (envelope-from takeharu1219@ybb.ne.jp) Received: from ybbsmtp10.mail.ogk.yahoo.co.jp (ybbsmtp10.mail.ogk.yahoo.co.jp [124.83.153.130]) by mx1.freebsd.org (Postfix) with SMTP id EF87F13C469 for ; Tue, 22 May 2007 16:30:07 +0000 (UTC) (envelope-from takeharu1219@ybb.ne.jp) Received: (qmail 74057 invoked by alias); 22 May 2007 16:03:25 -0000 DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=ybb20050223; d=ybb.ne.jp; b=kGpDFm5US28CcUFcVBjD4GPzBMtMcCUKeV6K/qtAcj30zTukCaXuf3NeOOGQYG4YhoIVua9msnRk6JcrWLoAdtjSuqc32yDEUARf+2GUSqGMyuQIcp1IWjFOR4wVwVep ; Received: from unknown (HELO ?127.0.0.1?) (takeharu1219@219.35.170.86 with plain) by ybbsmtp10.mail.ogk.yahoo.co.jp with SMTP; 22 May 2007 16:03:24 -0000 X-Apparently-From: Message-ID: <4653144C.2020601@ybb.ne.jp> Date: Wed, 23 May 2007 01:03:24 +0900 From: Takeharu KATO User-Agent: Thunderbird 1.5.0.9 (Windows/20061207) MIME-Version: 1.0 To: Marius Nuennerich References: <200705211400.l4LE0CE0057099@freefall.freebsd.org> In-Reply-To: <200705211400.l4LE0CE0057099@freefall.freebsd.org> Content-Type: text/plain; charset=ISO-2022-JP Content-Transfer-Encoding: 7bit Cc: freebsd-acpi@FreeBSD.org Subject: Re: kern/112544: [acpi] [patch] Add High Precision Event Timer Driver for userland timer facility 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, 22 May 2007 16:30:08 -0000 Hi Thank you for your review, I'll fix following issue. Marius Nuennerich wrote: > I haven't read the whole patch yet. > To me line 164 (of the first patch file) seems bogus. It tries to do > the right thing (read-modify-write) but actually does not. > HPET_OFFSET_ENABLE is the offset in the hpet datastructure. > > Should be like this: > val = bus_read_4(sc->mem_res, HPET_OFFSET_ENABLE); > bus_write_4(sc->mem_res, HPET_OFFSET_ENABLE, val | 1); > > And 1 should better HPET_ENABLE_ENABLE. > > Line 133 of the same patch does not even try to do a read-modify-write. > However the documentation is clear that it should. > > This all applies to the driver in -current (1.8) as well. >