From owner-freebsd-acpi@FreeBSD.ORG Mon Mar 16 15:02: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 D6040DF2 for ; Mon, 16 Mar 2015 15:02: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 997FBE2D for ; Mon, 16 Mar 2015 15:02:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=att.net; s=s1024; t=1426518028; bh=CbAl96ukmF+qQcOUR58a2IvFqR6P1bjv6lLSvPTe5Qo=; h=Date:From:To:CC:Subject:References:In-Reply-To:From:Subject; b=IaJ12VWR7ObZHn2TJtwZJxHc6sF4GR351uJgBLO5fb+LQQTbrsX+q9JfTSf9T2vzmiJoegIn2G35ou2xnYT/kC9qC/SNN+4hMe4B5iYAzLBlY71KzzIYW+zsXp5nhBF6Pcx2vSroxwjKTZyUbDylPYdMmL/peNfz24S1INx6j8A= Received: from [98.138.101.128] by nm8.bullet.mail.ne1.yahoo.com with NNFMP; 16 Mar 2015 15:00:28 -0000 Received: from [98.138.226.30] by tm16.bullet.mail.ne1.yahoo.com with NNFMP; 16 Mar 2015 15:00:28 -0000 Received: from [127.0.0.1] by smtp201.mail.ne1.yahoo.com with NNFMP; 16 Mar 2015 15:00:28 -0000 X-Yahoo-Newman-Id: 182340.16851.bm@smtp201.mail.ne1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: GiGW6AwVM1kY2qJhIN0yWzAEc4TmlI.YuTPGBtWMAj8aHpY 8qfDg6rx.YZulATnucz12i7RHRpHNlfYntdzBBT2nvwFOBKKsdMSu4QDN65X pg6OYjpGCmWO8IAi2nKm6fwOIGUhY9C6lM5g8dU4rFa_vifg2vatsR3DMOiL C1K_dHfbBu5wdF3kHzK_cNAHD4uAHQBZ0yl_M8W4pJeOXeW0JUZscwfRZf2e bH2xe0GDHm6kqB6aO_7VHAvQzsLJoKcalMfbBF_uTOAnxNI3GQgaB4jyXs9t R0r1ojOnZIHP96Bxg1DbkhAJzpwybhDNtk1gcdxNGQ1aNPBYpOo2Hpy_J4FE lcyBUqScy31uX3YLx4s3qorYQC3AQ3kursNOx04vMEeqOZwHpX5Kv5Fc9bJu dkTQycX.OywVvzXTs54BnLfMSHte9r4EXSrhwmcdcWNEQcSTu9gRd1H_RYPk eH1ZLJYG8irikebcsxRICO3.cqQfydWnj9a4hB9.9vl11ca4OlhlLtSuBd7o cW0vqjOQ4CaKOI_TlEl8JahyQvv.e9wKbV7ukg1zcmK4h3LFr X-Yahoo-SMTP: OKD1keCswBBTAmAF1s00hLyKW3wE3YfSK0Eazl6b4VZG4LTqJxg- Message-ID: <5506F00A.3030708@att.net> Date: Mon, 16 Mar 2015 11:00:26 -0400 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. 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> <54F9D7E6.4050807@att.net> <5504FF32.3020202@att.net> <20150317001401.X22641@sola.nimnet.asn.au> In-Reply-To: <20150317001401.X22641@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: Mon, 16 Mar 2015 15:02:58 -0000 On 03/16/2015 09:59 AM, Ian Smith wrote: > On Sat, 14 Mar 2015 23:40:34 -0400, Anthony Jenkins wrote: > > > How about this one? :-) Sorry it's a week late. > > +static unsigned int atrtc_verbose = 0; > +SYSCTL_UINT(_debug, OID_AUTO, atrtc_verbose, CTLFLAG_RWTUN, > + &atrtc_verbose, 0, "AT-RTC Debug Level (0-2)"); > +#define ATRTC_DBG_PRINTF(level, format, ...) \ > + if (atrtc_verbose >= level) printf(format, ##__VA_ARGS__) > + > > A sysctl is good, but it needs to default to 1 if we're ever going to > find out what various vendors are wanting to do with CMOS settings; > anyone annoyed by any extra messages outside boot or suspend/resume > could turn it off - after we've had the opportunity to get a report. I'd actually prefer debug level to be compile-time (there's no real need for it to be runtime), but I really don't want to add a config(5) knob (I can of course). > I can only reiterate part of my message before last, of March 2nd: > > ======= >> +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; That should probably be bytes = (width + 7) >>3; or bytes = (width + 7) / 8; /* Integer division more expensive on x86? */ because width == 1 would result in bytes = 0. > 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? > ======= > > Otherwise (for example) a 2 byte read from 0x0b or 4 byte read from > 0x09-0x0b will read 0x0c (clearing interrupts), or a 2 or 4 byte write > to (say) 0x01 will also write to 0x02 and 0x04 (clobbering the time). Right, this is an (incorrect) hybrid of a few attempts, probably from around the time I lost my SSD and only had a single backup copy of my work to go from. In one revision I had disallowed all multibyte accesses (width > 8) since IMHO it was more consistent/correct with the suggested locking. I wasn't ignoring your suggestion, just making one or a few changes at a time (generally the simpler ones). I /believe/ the ACPI spec disallows requests for width=0. Looking at ACPIspec50.pdf, the smallest OperationRegion that can be defined is 1 byte, and the smallest Field within an OpRegion is 1 bit, and all accesses to an OpRegion occur via Fields (Section 19.5.96 OperationRegion (Declare Operation Region)). Doesn't hurt to check tho. Also OpRegions operations are synchronized, so I at least don't have to worry about AML accessing CMOS all willy-nilly. > Sorry if I'm too much of a stickler for defensive programming .. Oh no problem, I can handle a bit of criticism :-) Anthony > > 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"