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