Date: Mon, 2 Mar 2015 01:29:06 +1100 (EST) From: Ian Smith <smithi@nimnet.asn.au> To: Anthony Jenkins <Anthony.B.Jenkins@att.net> Cc: freebsd-acpi@freebsd.org Subject: Re: [PATCH] ACPI CMOS region support rev. 3 [was Re: No acpi_dell(4)] Message-ID: <20150302002647.W42658@sola.nimnet.asn.au> In-Reply-To: <54F14368.4020807@att.net> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150302002647.W42658>