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
Index: sys/x86/isa/atrtc.c
===================================================================
--- sys/x86/isa/atrtc.c	(revision 279957)
+++ sys/x86/isa/atrtc.c	(working copy)
@@ -31,6 +31,7 @@
 __FBSDID("$FreeBSD$");
 
 #include "opt_isa.h"
+#include "opt_acpi.h"
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -53,9 +54,17 @@
 #include <machine/intr_machdep.h>
 #include "clock_if.h"
 
+#include <contrib/dev/acpica/include/acpi.h>
+#include <contrib/dev/acpica/include/accommon.h>
+#include <dev/acpica/acpivar.h>
+
 #define	RTC_LOCK	do { if (!kdb_active) mtx_lock_spin(&clock_lock); } while (0)
 #define	RTC_UNLOCK	do { if (!kdb_active) mtx_unlock_spin(&clock_lock); } while (0)
 
+#define IO_DELAY()	(void)inb(0x84)
+#define IO_RTC_ADDR	(IO_RTC + 0)
+#define IO_RTC_DATA	(IO_RTC + 1)
+
 int	atrtcclock_disable = 0;
 
 static	int	rtc_reg = -1;
@@ -62,6 +71,12 @@
 static	u_char	rtc_statusa = RTCSA_DIVIDER | RTCSA_NOPROF;
 static	u_char	rtc_statusb = RTCSB_24HR;
 
+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__)
+
 /*
  * RTC support routines
  */
@@ -73,10 +88,10 @@
 
 	RTC_LOCK;
 	if (rtc_reg != reg) {
-		inb(0x84);
+		IO_DELAY();
 		outb(IO_RTC, reg);
 		rtc_reg = reg;
-		inb(0x84);
+		IO_DELAY();
 	}
 	val = inb(IO_RTC + 1);
 	RTC_UNLOCK;
@@ -89,16 +104,36 @@
 
 	RTC_LOCK;
 	if (rtc_reg != reg) {
-		inb(0x84);
+		IO_DELAY();
 		outb(IO_RTC, reg);
 		rtc_reg = reg;
-		inb(0x84);
+		IO_DELAY();
 	}
 	outb(IO_RTC + 1, val);
-	inb(0x84);
+	IO_DELAY();
 	RTC_UNLOCK;
 }
 
+static void
+acpi_cmos_read(ACPI_PHYSICAL_ADDRESS address, UINT8 *buf, UINT32 buflen)
+{
+	UINT32 offset;
+
+	for (offset = 0; offset < buflen; ++offset) {
+		buf[offset] = rtcin(address + offset) & 0xff;
+	}
+}
+
+static void
+acpi_cmos_write(ACPI_PHYSICAL_ADDRESS address, const UINT8 *buf, UINT32 buflen)
+{
+	UINT32 offset;
+
+	for (offset = 0; offset < buflen; ++offset) {
+		writertc(address + offset, buf[offset]);
+	}
+}
+
 static __inline int
 readrtc(int port)
 {
@@ -161,9 +196,68 @@
 	struct resource *intr_res;
 	void *intr_handler;
 	struct eventtimer et;
+	ACPI_HANDLE acpi_handle;	/* Handle of the PNP0B00 node */
+	int acpi_handle_registered;	/* 0 = acpi_handle not registered */
 };
 
 static int
+acpi_check_rtc_byteaccess(int is_read, u_long addr)
+{
+	int retval = 1;	/* Success */
+
+	if (is_read) {
+		/* Reading 0x0C will muck with interrupts */
+		if (addr == 0x0C)
+			retval = 0;
+	} else {
+		if (!((addr <= 0x05 && (addr & 0x01)) ||
+		   (addr >= 0x30 && addr < 0x40)))
+			retval = 0;
+	}
+	return retval;
+}
+
+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) {
+		ATRTC_DBG_PRINTF(1, "%s: NULL parameter.\n", __FUNCTION__);
+		return AE_BAD_PARAMETER;
+	}
+	if (width > 32 || (width & 0x07) || address >= 64) {
+		ATRTC_DBG_PRINTF(1, "%s: Invalid width (%u) or address (0x%08lx).\n",
+			__FUNCTION__, width, address);
+		return AE_BAD_PARAMETER;
+	}
+	if (!acpi_check_rtc_byteaccess(function == ACPI_READ, address)) {
+		ATRTC_DBG_PRINTF(1, "%s: Bad CMOS %s access at address 0x%08lx.\n",
+			__FUNCTION__, function == ACPI_READ ? "read" : "write", address);
+		return AE_BAD_PARAMETER;
+	}
+
+	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:
+			ATRTC_DBG_PRINTF(1, "%s: Invalid function: %d.\n", __FUNCTION__, function);
+			return AE_BAD_PARAMETER;
+	}
+	ATRTC_DBG_PRINTF(1, "%s: %-5s%02u address=%04lx value=%08x\n",
+			__FUNCTION__, function == ACPI_READ ? "READ" : "WRITE",
+			width >> 3, address, *((UINT32 *)value));
+	return AE_OK;
+}
+
+static int
 rtc_start(struct eventtimer *et, sbintime_t first, sbintime_t period)
 {
 
@@ -245,10 +339,19 @@
 	int i;
 
 	sc = device_get_softc(dev);
+	sc->acpi_handle = acpi_get_handle(dev);
 	sc->port_res = bus_alloc_resource(dev, SYS_RES_IOPORT, &sc->port_rid,
 	    IO_RTC, IO_RTC + 1, 2, RF_ACTIVE);
 	if (sc->port_res == NULL)
 		device_printf(dev, "Warning: Couldn't map I/O.\n");
+	if (ACPI_FAILURE(AcpiInstallAddressSpaceHandler(sc->acpi_handle,
+					ACPI_ADR_SPACE_CMOS,
+					acpi_rtc_cmos_handler, NULL, sc)))
+	{
+		device_printf(dev, "Warning: Couldn't register ACPI CMOS address space handler.\n");
+		/* I assume the softc was memset() to 0? */
+	} else
+		sc->acpi_handle_registered = 1;
 	atrtc_start();
 	clock_register(dev, 1000000);
 	bzero(&sc->et, sizeof(struct eventtimer));
@@ -286,6 +389,17 @@
 	return(0);
 }
 
+static int atrtc_detach(device_t dev)
+{
+	struct atrtc_softc *sc;
+
+	sc = device_get_softc(dev);
+	if (sc->acpi_handle_registered)
+		AcpiRemoveAddressSpaceHandler(sc->acpi_handle,
+				ACPI_ADR_SPACE_CMOS, acpi_rtc_cmos_handler);
+	return bus_generic_detach(dev);
+}
+
 static int
 atrtc_resume(device_t dev)
 {
@@ -366,7 +480,7 @@
 	/* Device interface */
 	DEVMETHOD(device_probe,		atrtc_probe),
 	DEVMETHOD(device_attach,	atrtc_attach),
-	DEVMETHOD(device_detach,	bus_generic_detach),
+	DEVMETHOD(device_detach,	atrtc_detach),
 	DEVMETHOD(device_shutdown,	bus_generic_shutdown),
 	DEVMETHOD(device_suspend,	bus_generic_suspend),
 		/* XXX stop statclock? */
_______________________________________________
freebsd-acpi@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"

Reply via email to