[PATCH] ACPI CMOS region support rev. 4

Ian Smith smithi at nimnet.asn.au
Mon Mar 16 13:59:22 UTC 2015


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 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;
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).

Sorry if I'm too much of a stickler for defensive programming ..

cheers, Ian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: atrtc_c_rev4.diff
Type: text/x-patch
Size: 5565 bytes
Desc: 
URL: <http://lists.freebsd.org/pipermail/freebsd-acpi/attachments/20150317/6d611821/attachment.bin>


More information about the freebsd-acpi mailing list