From owner-freebsd-acpi@FreeBSD.ORG Sun Mar 1 14:29:22 2015 Return-Path: Delivered-To: freebsd-acpi@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 9FFDAA93 for ; Sun, 1 Mar 2015 14:29:22 +0000 (UTC) Received: from sola.nimnet.asn.au (paqi.nimnet.asn.au [115.70.110.159]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id E88B5635 for ; Sun, 1 Mar 2015 14:29:21 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by sola.nimnet.asn.au (8.14.2/8.14.2) with ESMTP id t21ET6pw051775; Mon, 2 Mar 2015 01:29:06 +1100 (EST) (envelope-from smithi@nimnet.asn.au) Date: Mon, 2 Mar 2015 01:29:06 +1100 (EST) From: Ian Smith To: Anthony Jenkins Subject: Re: [PATCH] ACPI CMOS region support rev. 3 [was Re: No acpi_dell(4)] In-Reply-To: <54F14368.4020807@att.net> Message-ID: <20150302002647.W42658@sola.nimnet.asn.au> References: <20150222180817.GD27984@strugglingcoder.info> <54EB8C21.2080600@att.net> <2401337.2oUs7iAbtB@ralph.baldwin.cx> <54EF3D5D.4010106@att.net> <20150227222203.P38620@sola.nimnet.asn.au> <20150228125857.D1277@besplex.bde.org> <54F14368.4020807@att.net> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; CHARSET=US-ASCII; FORMAT=flowed Content-ID: <20150302002647.R42658@sola.nimnet.asn.au> Cc: freebsd-acpi@freebsd.org X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 01 Mar 2015 14:29:22 -0000 On Fri, 27 Feb 2015 23:26:16 -0500, Anthony Jenkins wrote: > On 02/27/15 21:56, Bruce Evans wrote: > > On Sat, 28 Feb 2015, Ian Smith wrote: I'm going to contract this down to a couple of points, ok fellas? > > > Don't worry about any extra locking; this is going to be used at boot > > > and/or resume we assume; > > Bleah... I hate assumptions like that. So let's log everything at least while this is experimental in head, get lots of people using all sorts of hardware to report any surprises, and find out whenever else such service might be requested? Sure, add a sysctl hw.acpi.cmosaccess.STFU for when we find benign stuff being logged at other times than boot and suspend/resume, but when Jo Blo reports here or in questions@ or laptop@ that her Hidden Dragon Kneetop won't boot / suspend / resume / whatever with these funny lines in /var/log/messages, we'll know what it's about without undertaling the sort of research you had to do to get your HP Envy going, eh? :) % sysctl smithi.STFU=1 # enough already .. > +static ACPI_STATUS > +acpi_rtc_cmos_handler(UINT32 function, ACPI_PHYSICAL_ADDRESS address, > + UINT32 width, UINT64 *value, void *context, void *region_context) > +{ > + struct atrtc_softc *sc; > + > + sc = (struct atrtc_softc *)context; > + if (!value || !sc) > + return AE_BAD_PARAMETER; > + if (width > 32 || (width & 0x07) || address >= 64) > + return AE_BAD_PARAMETER; Width 0 passes, and address 61 width 32 passes. Maybe simpler: int bytes; /* or size, whatever, above */ bytes = width >> 3; and substitute 'bytes' subsequently, and here, perhaps: if (bytes < 1 || bytes > 4 || (width & 0x07) || address > (64 - bytes)) > + if (!acpi_check_rtc_byteaccess(function == ACPI_READ, address)) > + return AE_BAD_PARAMETER; acpi_check_rtc_byteaccess() needs to be called per byte of 1, 2 or 4 bytes - or pass it 'bytes' also, and loop over each of them within? Failing this _really_ needs a printf, as below + 'REJECTED' ono, IMO. > + > + switch (function) { > + case ACPI_READ: > + acpi_cmos_read(address, (UINT8 *)value, width >> 3); > + break; > + case ACPI_WRITE: > + acpi_cmos_write(address, (const UINT8 *)value, > + width >> 3); > + break; > + default: > + return AE_BAD_PARAMETER; > + } > + printf("%s: %-5s%02u address=%04lx value=%08x\n", > + __FUNCTION__, function == ACPI_READ ? "READ" : "WRITE", width >> 3, > + address, *((UINT32 *)value)); > + return AE_OK; > +} cheers, Ian From owner-freebsd-acpi@FreeBSD.ORG Tue Mar 3 16:48:58 2015 Return-Path: Delivered-To: freebsd-acpi@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id AA1CA61D for ; Tue, 3 Mar 2015 16:48:58 +0000 (UTC) Received: from nm8-vm2.bullet.mail.ne1.yahoo.com (nm8-vm2.bullet.mail.ne1.yahoo.com [98.138.90.156]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 6F5616A6 for ; Tue, 3 Mar 2015 16:48:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=att.net; s=s1024; t=1425401151; bh=J3gWCkYp3vCrVca9OQON1FTBaEX756fHr20o8pYTgwg=; h=Date:From:To:CC:Subject:References:In-Reply-To:From:Subject; b=xnimPJO8AxeqWpFAtNIgA3BdWZfmZCG+pA2JDPnekAGiPfk2bBQqlCnRKa2hFbSk4r04PFWE2RDEjVFUvE03YYevmQcRd+sA3hJLn0iuLyPxWRMTcG1qCwEUnt/iZYr+Qq4h2s9PhWapgLeo12P7MnbNOrINfIomRbsIlUfYHn4= Received: from [98.138.226.176] by nm8.bullet.mail.ne1.yahoo.com with NNFMP; 03 Mar 2015 16:45:51 -0000 Received: from [98.138.226.57] by tm11.bullet.mail.ne1.yahoo.com with NNFMP; 03 Mar 2015 16:45:51 -0000 Received: from [127.0.0.1] by smtp208.mail.ne1.yahoo.com with NNFMP; 03 Mar 2015 16:45:51 -0000 X-Yahoo-Newman-Id: 74919.83412.bm@smtp208.mail.ne1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: IO9rT1cVM1kW6vo1TWLgRQZozuKK7hx0r6GPxfjRioZLHUU jAqPO3X4zyrNwjvFEnNNqJ3SAGDzsB7Gjb7Kvygj23UNDvuHiZewzoBdQ3Ij jBVB50h.51DHsqBFMYg_bn3VOQCgBY_jxFIVcmPNoQ78YQb4nwOkj655jtbk cqtbgIzrdQ.wlmWPUhkPqeE9fhRm3eRyJFIl0EmNoIs62sKJ5EM9QIrH5nWT N5EHd7jiHJCmtnZ.vvToI3NjZBKeo4RfzTAr64VXKPpcKeXbtBaRzTg_0C7C j5D7yaS3EpHMhoRL9LS23VgKNFGrObEhYCEmM7SXcXzbLmZryJpM3rNg3frG HVrGjBjt.Vq0P8.y9dA6gRWLswRtVoUPXTeOj5ZMZZcJHASOC9CfhfJ3wbAT 2QjeIXHKRgRWwoDdqLp9k83ag2pMz0UkXXmkaJkkE98oho2tsHmvYkSv6pNj _baVuVqkharvbR4zP79l76dniZDIU24iT2C5wTW1h_Rux3Ue3SUfkKtRpfZ2 woIBP.LIg4gHwcPR4aS4pnm.Jnn.1l.lOkqATxUVP6w6BNliB X-Yahoo-SMTP: OKD1keCswBBTAmAF1s00hLyKW3wE3YfSK0Eazl6b4VZG4LTqJxg- Message-ID: <54F5E53D.1090601@att.net> Date: Tue, 03 Mar 2015 11:45:49 -0500 From: Anthony Jenkins User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Ian Smith Subject: Re: [PATCH] ACPI CMOS region support rev. 3 [was Re: No acpi_dell(4)] References: <20150222180817.GD27984@strugglingcoder.info> <54EB8C21.2080600@att.net> <2401337.2oUs7iAbtB@ralph.baldwin.cx> <54EF3D5D.4010106@att.net> <20150227222203.P38620@sola.nimnet.asn.au> <20150228125857.D1277@besplex.bde.org> <54F14368.4020807@att.net> <20150302002647.W42658@sola.nimnet.asn.au> In-Reply-To: <20150302002647.W42658@sola.nimnet.asn.au> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: freebsd-acpi@freebsd.org X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Mar 2015 16:48:58 -0000 On 03/01/2015 09:29 AM, Ian Smith wrote: > On Fri, 27 Feb 2015 23:26:16 -0500, Anthony Jenkins wrote: > > On 02/27/15 21:56, Bruce Evans wrote: > > > On Sat, 28 Feb 2015, Ian Smith wrote: > > I'm going to contract this down to a couple of points, ok fellas? > > > > > Don't worry about any extra locking; this is going to be used at > boot > > > > and/or resume we assume; > > > Bleah... I hate assumptions like that. > > So let's log everything at least while this is experimental in head, > get lots of people using all sorts of hardware to report any > surprises, and find out whenever else such service might be requested? No problem adding logging. The bit I'm confused about (unless I'm misunderstanding the argument) is why # ugly pseudocode function original_cmos_xfer(): lock(); transfer byte to/from CMOS unlock(); function acpi_cmos_xfer(): foreach byte in num_bytes: call original_cmos_xfer(byte) end foreach is preferable to # ugly pseudocode function acpi_cmos_xfer(): lock(); foreachbyte in num_bytes: transfer byte to/from CMOS end foreach unlock(); function original_cmos_xfer(): call acpi_cmos_xfer(num_bytes = 1); There was no "extra locking", but I did introduce an extra function call which would slow down CMOS accesses to the RTC for some performance timers; I assume that's the main complaint. To me, the former pseudocode is "incorrect" because it allows the possibility of another thread of execution mucking with a multibyte CMOS access by ACPIBIOS. I agree it's /probably/ unlikely tho. > Sure, add a sysctl hw.acpi.cmosaccess.STFU for when we find benign > stuff being logged at other times than boot and suspend/resume, but > when Jo Blo reports here or in questions@ or laptop@ that her Hidden > Dragon Kneetop won't boot / suspend / resume / whatever with these > funny lines in /var/log/messages, we'll know what it's about without > undertaling the sort of research you had to do to get your HP Envy > going, eh? :) Agreed; just saving logging tweaks for last. > > % sysctl smithi.STFU=1 # enough already .. > > > +static ACPI_STATUS > > +acpi_rtc_cmos_handler(UINT32 function, ACPI_PHYSICAL_ADDRESS address, > > + UINT32 width, UINT64 *value, void *context, void *region_context) > > +{ > > + struct atrtc_softc *sc; > > + > > + sc = (struct atrtc_softc *)context; > > + if (!value || !sc) > > + return AE_BAD_PARAMETER; > > + if (width > 32 || (width & 0x07) || address >= 64) > > + return AE_BAD_PARAMETER; > > Width 0 passes, and address 61 width 32 passes. Maybe simpler: > int bytes; /* or size, whatever, above */ > bytes = width >> 3; > and substitute 'bytes' subsequently, and here, perhaps: > > if (bytes < 1 || bytes > 4 || (width & 0x07) || address > (64 - > bytes)) > > > + if (!acpi_check_rtc_byteaccess(function == ACPI_READ, address)) > > + return AE_BAD_PARAMETER; > > acpi_check_rtc_byteaccess() needs to be called per byte of 1, 2 or 4 > bytes - or pass it 'bytes' also, and loop over each of them within? Thought I had done/fixed that... I'll check. > Failing this _really_ needs a printf, as below + 'REJECTED' ono, IMO. Agreed. Thanks, Anthony > > > + > > + switch (function) { > > + case ACPI_READ: > > + acpi_cmos_read(address, (UINT8 *)value, width > >> 3); > > + break; > > + case ACPI_WRITE: > > + acpi_cmos_write(address, (const UINT8 *)value, > > + width >> 3); > > + break; > > + default: > > + return AE_BAD_PARAMETER; > > + } > > + printf("%s: %-5s%02u address=%04lx value=%08x\n", > > + __FUNCTION__, function == ACPI_READ ? "READ" : "WRITE", > width >> 3, > > + address, *((UINT32 *)value)); > > + return AE_OK; > > +} > > cheers, Ian From owner-freebsd-acpi@FreeBSD.ORG Fri Mar 6 11:58:40 2015 Return-Path: Delivered-To: freebsd-acpi@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id B5F70CB for ; Fri, 6 Mar 2015 11:58:40 +0000 (UTC) Received: from sola.nimnet.asn.au (paqi.nimnet.asn.au [115.70.110.159]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id AEA8DDA1 for ; Fri, 6 Mar 2015 11:58:39 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by sola.nimnet.asn.au (8.14.2/8.14.2) with ESMTP id t26BnJ4P087780; Fri, 6 Mar 2015 22:49:20 +1100 (EST) (envelope-from smithi@nimnet.asn.au) Date: Fri, 6 Mar 2015 22:49:19 +1100 (EST) From: Ian Smith To: Anthony Jenkins Subject: Re: [PATCH] ACPI CMOS region support rev. 3 [was Re: No acpi_dell(4)] In-Reply-To: <54F5E53D.1090601@att.net> Message-ID: <20150306025800.U46361@sola.nimnet.asn.au> References: <20150222180817.GD27984@strugglingcoder.info> <54EB8C21.2080600@att.net> <2401337.2oUs7iAbtB@ralph.baldwin.cx> <54EF3D5D.4010106@att.net> <20150227222203.P38620@sola.nimnet.asn.au> <20150228125857.D1277@besplex.bde.org> <54F14368.4020807@att.net> <20150302002647.W42658@sola.nimnet.asn.au> <54F5E53D.1090601@att.net> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Cc: freebsd-acpi@freebsd.org X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Mar 2015 11:58:40 -0000 On Tue, 3 Mar 2015 11:45:49 -0500, Anthony Jenkins wrote: > On 03/01/2015 09:29 AM, Ian Smith wrote: [..] > No problem adding logging. The bit I'm confused about (unless I'm > misunderstanding the argument) is why > > # ugly pseudocode > function original_cmos_xfer(): > lock(); > transfer byte to/from CMOS > unlock(); > > function acpi_cmos_xfer(): > foreach byte in num_bytes: > call original_cmos_xfer(byte) > end foreach > > > is preferable to > > # ugly pseudocode > function acpi_cmos_xfer(): > lock(); > foreachbyte in num_bytes: > transfer byte to/from CMOS > end foreach > unlock(); > > function original_cmos_xfer(): > call acpi_cmos_xfer(num_bytes = 1); > > > There was no "extra locking", but I did introduce an extra function call > which would slow down CMOS accesses to the RTC for some performance > timers; I assume that's the main complaint. And slow down all other callers of rtcin() and writertc(), as well as any timer using the RTC periodic interrupt. Most callers are right here in /sys/x86/isa/atrtc.c; I make it 37 plus another 9 #ifdef DDB. This works for me on a 9.3-R system, not checked on 10.x or head .. smithi@x200:~ % find /usr/src -type f -name "*.[ch]" \ -exec egrep -Hl 'rtcin\(|writertc\(|readrtc\(' {} + | xargs less :/rtcin|writertc|readrtc All in /sys/ of course. Ignoring mips/malta, most callers likely don't care if these calls take twice(?) as long, but to me it seems suboptimal to impede likely more frequently used calls for the sake of a much less frequently used high-level function, that itself is not timing-critical. I know nothing about how much xen/clock.c cares about timing. nvram.c doesn't seem bothered, nor fetching memory size, fdc or vga parameters. On the other hand, interrupt service needs to care a lot about timing. Remember that FreeBSD is running on other than latest kit, including eg 266MHz 5x86 Soekris routers and the like, and smaller embedded systems that may very well need not to have RTC ISRs time-pessimised, at up to 8kHz rates; such are also less likely to have HPETs to use as timers. And for that matter, many older and smaller boards are unlikely to be running ACPI BIOS at all - which brings up another question, below .. > To me, the former > pseudocode is "incorrect" because it allows the possibility of another > thread of execution mucking with a multibyte CMOS access by ACPIBIOS. I > agree it's /probably/ unlikely tho. I've no idea whether that may be possible, or if or how it might matter. But all that's just me, the Time Lords are likely having a good laugh :) Regarding systems without ACPI loaded, or active: what happens when the below AcpiInstallAddressSpaceHandler() call fails, but returns 0? Would not that prevent rtc_start() from running atrtc_start() etc for non-ACPI clock initialisation and registration? I suppose there's a global kernel variable for acpi_is_active ono? > +static int > rtc_start(struct eventtimer *et, sbintime_t first, sbintime_t period) > { > > @@ -245,10 +323,17 @@ > int i; > > sc = device_get_softc(dev); > + sc->acpi_handle = acpi_get_handle(dev); > sc->port_res = bus_alloc_resource(dev, SYS_RES_IOPORT, &sc->port_rid, > IO_RTC, IO_RTC + 1, 2, RF_ACTIVE); > if (sc->port_res == NULL) > device_printf(dev, "Warning: Couldn't map I/O.\n"); > + if (ACPI_FAILURE(AcpiInstallAddressSpaceHandler(sc->acpi_handle, > + ACPI_ADR_SPACE_CMOS, acpi_rtc_cmos_handler, NULL, sc))) > + { > + device_printf(dev, "Error registering ACPI CMOS address space handler.\n"); > + return 0; > + } > atrtc_start(); > clock_register(dev, 1000000); > bzero(&sc->et, sizeof(struct eventtimer)); > @@ -286,6 +371,15 @@ > return(0); > } Might that not matter for detach, as you're not testing for return code? > +static int atrtc_detach(device_t dev) > +{ > + struct atrtc_softc *sc; > + > + sc = device_get_softc(dev); > + AcpiRemoveAddressSpaceHandler(sc->acpi_handle, > ACPI_ADR_SPACE_CMOS, acpi_rtc_cmos_handler); > + return bus_generic_detach(dev); > +} cheers, Ian From owner-freebsd-acpi@FreeBSD.ORG Fri Mar 6 16:40:41 2015 Return-Path: Delivered-To: freebsd-acpi@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 4279E33C for ; Fri, 6 Mar 2015 16:40:41 +0000 (UTC) Received: from nm19-vm4.bullet.mail.gq1.yahoo.com (nm19-vm4.bullet.mail.gq1.yahoo.com [98.136.217.27]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 0E23BFFF for ; Fri, 6 Mar 2015 16:40:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=att.net; s=s1024; t=1425659879; bh=De223/rrxuZ5CKXoqkGsWu4zGLJZkqv4GunBM0nMAxU=; h=Date:From:To:CC:Subject:References:In-Reply-To:From:Subject; b=M8zBt9E5GKK2larX5sCVc+7bfKlB4pO09Rkq39Mfr/dwAx9dvxcSz0EwdJSF41eCqoJ//TR6I8az4T6H0QMl8hp3IJ9NXrrqcvbJgarH4UiFR0OP4QKbik2HYkmtxKJDse7/v7/yLoiYaoOFVZiKvmHG1UljrnIk+PtEXS5JgEA= Received: from [98.137.12.63] by nm19.bullet.mail.gq1.yahoo.com with NNFMP; 06 Mar 2015 16:37:59 -0000 Received: from [98.138.84.37] by tm8.bullet.mail.gq1.yahoo.com with NNFMP; 06 Mar 2015 16:37:59 -0000 Received: from [127.0.0.1] by smtp105.mail.ne1.yahoo.com with NNFMP; 06 Mar 2015 16:37:58 -0000 X-Yahoo-Newman-Id: 929932.61238.bm@smtp105.mail.ne1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: EgsOs7AVM1n9B_Ejb1D6qBoPga6bVV9AgsbMXRIslWKtg4k RdZ0yw1eBjIDDXhPtPVfB4ueeDVYwihuqMJBHCFLEGo1YUItEZjfZpXOlnI7 nf_z5__YaENi1EkiTHzrddsYpNQ0rU.ivYMRVJE0CTLxcBlFq8zXI9l9eHIU 7IPwOcB33feXW2YZr9Uxn52qNBiCQz.DSV4GfMblR2fJIlncNI.Q6VUcALnE _iRY5zPCa3CLpkgN5i0UxUa5yZpZ7GVdInh_cytxBtK8VZKyXkzo8cCBHQJR 9FeP5fAb1FQJVbabsyy7oaHnSU6OLxV.A8.jkZQy94cYrhfv1eIU689a56Iv pWvPZLYGrNIbKlQ5hO5BDrhNQKBimx.73zj0nSCvlF5hyAQbyhk0C_h1wplB my8AYiztOzWK2iGwkwweEqVu.kKcelgzHH7fxWrCb1k59gb7hQExWR1UhW7m IgCinqavLLYNu3pcy7cbafvAybwCcUpDyHnt4xOpDq_V6I0sRW9ombKMn8vH FG5u8gsEdCiIWStCsiFgxuBNQX27Mktu5M9extn8c6gBk0Q-- X-Yahoo-SMTP: OKD1keCswBBTAmAF1s00hLyKW3wE3YfSK0Eazl6b4VZG4LTqJxg- Message-ID: <54F9D7E6.4050807@att.net> Date: Fri, 06 Mar 2015 11:37:58 -0500 From: Anthony Jenkins User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Ian Smith Subject: Re: [PATCH] ACPI CMOS region support rev. 3 [was Re: No acpi_dell(4)] References: <20150222180817.GD27984@strugglingcoder.info> <54EB8C21.2080600@att.net> <2401337.2oUs7iAbtB@ralph.baldwin.cx> <54EF3D5D.4010106@att.net> <20150227222203.P38620@sola.nimnet.asn.au> <20150228125857.D1277@besplex.bde.org> <54F14368.4020807@att.net> <20150302002647.W42658@sola.nimnet.asn.au> <54F5E53D.1090601@att.net> <20150306025800.U46361@sola.nimnet.asn.au> In-Reply-To: <20150306025800.U46361@sola.nimnet.asn.au> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: freebsd-acpi@freebsd.org X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Mar 2015 16:40:41 -0000 On 03/06/2015 06:49 AM, Ian Smith wrote: > On Tue, 3 Mar 2015 11:45:49 -0500, Anthony Jenkins wrote: > > On 03/01/2015 09:29 AM, Ian Smith wrote: > [..] > Regarding systems without ACPI loaded, or active: what happens when the > below AcpiInstallAddressSpaceHandler() call fails, but returns 0? Would > not that prevent rtc_start() from running atrtc_start() etc for non-ACPI > clock initialisation and registration? Good catch, there's technically no reason to bail on rtc_start() if I fail to register the ACPI CMOS handler; it'll just never get called (same as old behaviour). I'll change "Error" to "Warning" and remove the return 0. > I suppose there's a global kernel variable for acpi_is_active ono? > > > +static int > > rtc_start(struct eventtimer *et, sbintime_t first, sbintime_t period) > > { > > > > @@ -245,10 +323,17 @@ > > int i; > > > > sc = device_get_softc(dev); > > + sc->acpi_handle = acpi_get_handle(dev); > > sc->port_res = bus_alloc_resource(dev, SYS_RES_IOPORT, &sc->port_rid, > > IO_RTC, IO_RTC + 1, 2, RF_ACTIVE); > > if (sc->port_res == NULL) > > device_printf(dev, "Warning: Couldn't map I/O.\n"); > > + if (ACPI_FAILURE(AcpiInstallAddressSpaceHandler(sc->acpi_handle, > > + ACPI_ADR_SPACE_CMOS, acpi_rtc_cmos_handler, NULL, sc))) > > + { > > + device_printf(dev, "Error registering ACPI CMOS address space handler.\n"); > > + return 0; > > + } > > atrtc_start(); > > clock_register(dev, 1000000); > > bzero(&sc->et, sizeof(struct eventtimer)); > > @@ -286,6 +371,15 @@ > > return(0); > > } > > Might that not matter for detach, as you're not testing for return code? Yeah I'd prefer to remember the failure to install the handler and conditionally uninstall it in the detach method. I'll fix up the logging and add these suggestions this weekend. Thanks, Anthony > > +static int atrtc_detach(device_t dev) > > +{ > > + struct atrtc_softc *sc; > > + > > + sc = device_get_softc(dev); > > + AcpiRemoveAddressSpaceHandler(sc->acpi_handle, > > ACPI_ADR_SPACE_CMOS, acpi_rtc_cmos_handler); > > + return bus_generic_detach(dev); > > +} > > cheers, Ian > _______________________________________________ > freebsd-acpi@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-acpi > To unsubscribe, send any mail to "freebsd-acpi-unsubscribe@freebsd.org"