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