tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

advice on modifications to the pcf8583 driver



I have made some changes to the Philips pcf8583 (pcfrtc) driver to
support devices at address 0x51 as well as the existing 0x50 and also
permit the device to work in counter mode.

The existing driver only attaches devices at address 0x50, a trivial
change allows address 0x51 as well so that two devices can share the
same bus.

Currently the driver reads the status/config register and reports the
device operating mode, however there is no support for modifying the
mode and after a power on reset the chip always comes up in 32768hz
mode. 

The device will actually operate different modes:

mode 0: 32768hz using a Xtal (tod clock mode)
mode 1: external 50hz input  (tod clock mode)
mode 2: counter mode with an external pulse
mode 3: factory test mode.

The 50hz mode may be useful for some countries as the grid timebase is
reasonably accurate when viewed on a 24 hour basis, but the counter
mode allows arbirary counting of events which is useful for rain gauges
etc. 

In order to support mode selection, code has been added to support a
flag field on the device config statement. 

e.g:

pcfrtc*         at iic? addr 0x50 
pcfrtc*         at iic? addr 0x51 flags 2

if the flag field is missing or set to 0 then the code operates as
standard including support for the NVRAM used for bootstapping acorn26.

QUESTION 1:

I use: struct cfdata *cf = device_cfdata(self); and then test cf-
>cf_flags if the config does not have a flags field (as example 1) then
it seems that cf_flags is set to 0. Is this reliable?

----

The code takes the flag field and if non-zero writes a new value into
the config/status register. The code then reads and reports the status
register as before but then only sets up the tod callbacks in mode 0
and 1.

In mode 2 it creates an INT environment variable which maps to the
counter fields.
The counter is a 6 digit BCD value so can count to 1000000. Envstat
reports the current value. The counter will overflow back to zero,
however it is probably useful to have a mechanism to reset the counter
on demand and so a sysctl hw variable is created and zeroed. If this is
set to 1 by:

hw.pcfrtc1*.counter_reset=1

then the counters are zeroed after the next envstat call to read the
counter and the sysctl variable is automagically reset to 0.

QUESTION 2:

Does this seem a reasonable approach? The code is simple and
straightforward and avoids having to muck around with IOCTL's, but is
it acceptable :-)

----

There appears to be a lack of man pages for either the pcf8583 or
pcfrtc and it is not clear which what to document. My gut feeling is to
write a man page for the pcf8583 and document the chip with reference
to the config with pcfrtc. 

The code has been tested on RPI zero 2 W NetBSD arm64.anduin.org.uk
11.99.5 with suitable dtc. One pcf8583 (counter mode) and bme280 on
iic2 and the other pcf8583 (32768hz) on iic1. tod clock function works
fine.

[     1.000004] iic1 at bsciic1: I2C bus
[     1.000004] pcfrtc0 at iic1 addr 0x50: PCF8583 Real-time
Clock/Counter/NVRAM
[     1.000004] pcfrtc0:  32.768 kHz clock
[     1.000004] iic2 at bsciic2: I2C bus
[     1.000004] pcfrtc1 at iic2 addr 0x51: PCF8583 Real-time
Clock/Counter/NVRAM
[     1.000004] pcfrtc1:  event counter
[     1.000004] bmx280thp0 at iic2 addr 0x76
[     1.000004] bmx280thp0: Bosch Sensortec BME280, Chip ID: 0x60

                 Current  CritMax  WarnMax  WarnMin  CritMin  Unit
[bmx280thp0]
  temperature:    21.590                                      degC
     pressure:  1003.110                                       hPa
     humidity:    51.936                                       %rH
[pcfrtc1]
      counter:         3        0        0        0        0 none
[vcmbox0]
  temperature:    38.628   85.000                             degC

arm64root(arm64)root$ sysctl -a |grep hw.pcf
hw.pcfrtc1.counter_reset = 0

Unified diff attached against current.

Any comments and advice appreciated

Cheers,
Dave 
--- pcf8583.c.orig	2026-03-15 21:44:28.606118234 +0000
+++ pcf8583.c	2026-03-15 21:44:25.222579821 +0000
@@ -40,6 +40,15 @@
  *
  * This driver is partially derived from Ben Harris's PCF8583 driver
  * for NetBSD/acorn26.
+ *
+ * Additional code written by Dave Tyson to support dual addressing 0x50/0x51
+ * and to select chip operating mode using optional flag in kernel config.
+ * Default with no flag set or flag 0 is existing tod clock mode, flag 1 sets
+ * 50hz input (tod clock), flag 2 sets counter mode, flag 3 sets test mode.
+ *
+ * Setting counter mode provides an envsys variable mapped to the counter and
+ * exposes a hw syctl variable which can set to 1 to zero the counter registers
+ * after the next envstat call. 
  */
 
 #include <sys/cdefs.h>
@@ -49,6 +58,7 @@
 #include <sys/systm.h>
 #include <sys/device.h>
 #include <sys/kernel.h>
+#include <sys/sysctl.h>
 #include <sys/fcntl.h>
 #include <sys/uio.h>
 #include <sys/conf.h>
@@ -59,6 +69,7 @@
 #include <dev/i2c/i2cvar.h>
 #include <dev/i2c/pcf8583reg.h>
 #include <dev/i2c/pcf8583var.h>
+#include <dev/sysmon/sysmonvar.h>
 
 #include "ioconf.h"
 
@@ -68,10 +79,15 @@
 	int sc_address;
 	int sc_open;
 	struct todr_chip_handle sc_todr;
+	struct sysmon_envsys *sc_sme;
+	envsys_data_t sc_counter;
+	struct sysctllog *sc_pcfrtclog;
+	int reset_counter;
 };
 
 static int  pcfrtc_match(device_t, cfdata_t, void *);
 static void pcfrtc_attach(device_t, device_t, void *);
+static int pcfrtc_sysctl_setup(struct pcfrtc_softc *sc);
 
 CFATTACH_DECL_NEW(pcfrtc, sizeof(struct pcfrtc_softc),
 	pcfrtc_match, pcfrtc_attach, NULL, NULL);
@@ -102,13 +118,16 @@
 			      uint8_t);
 static int pcfrtc_gettime(struct todr_chip_handle *, struct timeval *);
 static int pcfrtc_settime(struct todr_chip_handle *, struct timeval *);
+static void pcfrtc_readcount(struct sysmon_envsys *, envsys_data_t *) ;
+static int pcfrtc_sysctl_check(SYSCTLFN_ARGS);
 
 int
 pcfrtc_match(device_t parent, cfdata_t cf, void *aux)
 {
 	struct i2c_attach_args *ia = aux;
 
-	if ((ia->ia_addr & PCF8583_ADDRMASK) == PCF8583_ADDR)
+	if ((ia->ia_addr & PCF8583_ADDRMASK) == PCF8583_ADDR ||
+	    (ia->ia_addr & PCF8583_ADDRMASK) == PCF8583_ADDR2)
 		return (I2C_MATCH_ADDRESS_ONLY);
 
 	return (0);
@@ -118,16 +137,30 @@
 pcfrtc_attach(device_t parent, device_t self, void *aux)
 {
 	struct pcfrtc_softc *sc = device_private(self);
+	struct cfdata *cf = device_cfdata(self);
 	struct i2c_attach_args *ia = aux;
-	uint8_t cmdbuf[1], csr;
+	uint8_t cmdbuf[1], csr, mode;
+	int error;
 
 	sc->sc_tag = ia->ia_tag;
 	sc->sc_address = ia->ia_addr;
 	sc->sc_dev = self;
 
-	aprint_naive(": Real-time Clock/NVRAM\n");
-	aprint_normal(": PCF8583 Real-time Clock/NVRAM\n");
+	aprint_naive(": Real-time Clock/Counter/NVRAM\n");
+	aprint_normal(": PCF8583 Real-time Clock/Counter/NVRAM\n");
+
+	/* if flags set & !zero then need to change device mode */
 
+	if (cf->cf_flags != 0) {
+		mode = cf->cf_flags << 4 & PCF8583_CSR_FN_MASK;
+		cmdbuf[0] = PCF8583_REG_CSR;
+		if (iic_exec(sc->sc_tag, I2C_OP_WRITE_WITH_STOP, sc->sc_address,
+	    	 cmdbuf, 1, &mode, 1, 0) != 0) {
+			aprint_error_dev(self, "unable to write CSR\n");
+			return;
+		}
+	}
+ 
 	cmdbuf[0] = PCF8583_REG_CSR;
 	if (iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP, sc->sc_address,
 	    cmdbuf, 1, &csr, 1, 0) != 0) {
@@ -160,11 +193,51 @@
 
 	sc->sc_open = 0;
 
-	sc->sc_todr.todr_dev = self;
-	sc->sc_todr.todr_gettime = pcfrtc_gettime;
-	sc->sc_todr.todr_settime = pcfrtc_settime;
+	switch (csr & PCF8583_CSR_FN_MASK) {
+
+	case PCF8583_CSR_FN_32768HZ:
+	case PCF8583_CSR_FN_50HZ:
+		sc->sc_todr.todr_dev = self;
+		sc->sc_todr.todr_gettime = pcfrtc_gettime;
+		sc->sc_todr.todr_settime = pcfrtc_settime;
+
+		todr_attach(&sc->sc_todr);
+		break;
 
-	todr_attach(&sc->sc_todr);
+	case PCF8583_CSR_FN_EVENT:
+		if ((sc->sc_sme = sysmon_envsys_create()) == NULL) {
+			aprint_error_dev(sc->sc_dev,
+			  "unable to create sysmon structure");
+			sc->sc_sme = NULL;
+			return ;
+		}
+		sc->sc_counter.units=ENVSYS_INTEGER;
+		sc->sc_counter.state=ENVSYS_SINVALID;
+		strlcpy(sc->sc_counter.desc, "counter",
+		 sizeof(sc->sc_counter.desc));
+		if (sysmon_envsys_sensor_attach(sc->sc_sme, &sc->sc_counter)) {
+			sysmon_envsys_destroy(sc->sc_sme);
+			return;
+		}
+		
+		sc->sc_sme->sme_name = device_xname(self) ;
+		sc->sc_sme->sme_refresh = pcfrtc_readcount;
+		sc->sc_sme->sme_cookie = sc;
+
+		if (sysmon_envsys_register(sc->sc_sme)) {
+			sysmon_envsys_destroy(sc->sc_sme);
+			return;
+		}
+
+		/* setup sysctl for resetting counter */
+
+		if ((error = pcfrtc_sysctl_setup(sc)) !=0) {
+			aprint_error_dev(sc->sc_dev, "cannot setup sysctl tree (%d)\n", error);
+			sysmon_envsys_destroy(sc->sc_sme);
+		}
+		sc->reset_counter = 0;
+		break ;
+	}
 }
 
 /*ARGSUSED*/
@@ -499,3 +572,96 @@
 	iic_release_bus(tag, 0);
 	return (0);
 }
+
+int pcfrtc_sysctl_check(SYSCTLFN_ARGS)
+{
+	struct sysctlnode node;
+	int t, error;
+
+	t = *(int *)rnode->sysctl_data;
+
+	node = *rnode;
+	node.sysctl_data = &t;
+	error = sysctl_lookup(SYSCTLFN_CALL(&node));
+	if (error || newp == NULL)
+		return (error);
+
+	if (t < 0 || t > 1)
+		return (EINVAL);
+
+	*(int *)rnode->sysctl_data = t;
+	return 0;
+}
+
+static int pcfrtc_sysctl_setup(struct pcfrtc_softc *sc)
+{
+	int error;
+	const struct sysctlnode *cnode;
+	int sysctlroot_num;
+
+	if ((error = sysctl_createv(&sc->sc_pcfrtclog, 0, NULL, &cnode,
+		0, CTLTYPE_NODE, device_xname(sc->sc_dev),
+		SYSCTL_DESCR("pcfrtc counter reset"), NULL, 0, NULL, 0,
+		CTL_HW, CTL_CREATE, CTL_EOL)) != 0)
+		return error;
+	sysctlroot_num = cnode->sysctl_num;
+
+	if ((error = sysctl_createv(&sc->sc_pcfrtclog, 0, NULL, &cnode,
+		CTLFLAG_READWRITE, CTLTYPE_INT, "counter_reset",
+		SYSCTL_DESCR("counter reset"), pcfrtc_sysctl_check, 0,
+		&sc->reset_counter, 0, CTL_HW,
+		sysctlroot_num, CTL_CREATE, CTL_EOL)) != 0)
+		return error;
+	return 0;
+}
+
+static void pcfrtc_readcount(struct sysmon_envsys *sme, envsys_data_t *edata)
+{
+	struct pcfrtc_softc *sc = sme->sme_cookie;
+	int err, i;
+	uint8_t bcd[3],cmdbuf[1];
+	uint8_t zero=0;
+ 
+	if ((err = iic_acquire_bus(sc->sc_tag, 0))) {
+		aprint_error_dev(sc->sc_dev,
+		    "pcfrtc_readcount: failed to acquire I2C bus\n");
+		return;
+	}
+
+	for (i = PCF8583_COUNTER0; i <= PCF8583_COUNTER2; i++) {
+                cmdbuf[0] = i;
+                
+                if ((err = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP,
+                             sc->sc_address, cmdbuf, 1,
+                             &bcd[i-1], 1, 0))) {
+                        iic_release_bus(sc->sc_tag, 0);
+                        aprint_error_dev(sc->sc_dev,
+                            "pcfrtc_readcount: failed to read register at %x\n", i);
+                        return;
+                }
+        }
+ 
+
+	edata->value_cur = bcdtobin(bcd[0]) + bcdtobin(bcd[1])*100 + bcdtobin(bcd[2])*10000 ;
+	edata->state = ENVSYS_SVALID ;
+	
+	/* if reset_counter = 1 then clear totals */
+
+	if (sc->reset_counter != 0) {
+		for (i = PCF8583_COUNTER0; i <= PCF8583_COUNTER2; i++) {
+                	cmdbuf[0] = i;
+                
+                	if ((err = iic_exec(sc->sc_tag, I2C_OP_WRITE_WITH_STOP,
+					sc->sc_address, cmdbuf, 1, &zero, 1, 0))) {
+                        	iic_release_bus(sc->sc_tag, 0);
+                        	aprint_error_dev(sc->sc_dev,
+					"pcfrtc_readcount: failed to write register at %x\n", i);
+				return;
+                	}
+        	}
+ 
+		sc->reset_counter = 0 ;
+	}
+
+	iic_release_bus(sc->sc_tag, 0);
+}
--- pcf8583reg.h.orig	2026-03-15 21:44:52.282566044 +0000
+++ pcf8583reg.h	2026-03-15 21:44:41.071076581 +0000
@@ -22,6 +22,7 @@
  */
 #define PCF8583_ADDRMASK	0x3ff
 #define PCF8583_ADDR		0x50
+#define PCF8583_ADDR2		0x51
 
 #define PCF8583_REG_CSR		0x00
 #define PCF8583_REG_CENTI	0x01
@@ -86,4 +87,7 @@
 #define PCF8583_ALMCTL_TIMERALM		0x40
 #define PCF8583_ALMCTL_ALMINT		0x80
 
+#define PCF8583_COUNTER0		0x01
+#define PCF8583_COUNTER1		0x02
+#define PCF8583_COUNTER2		0x03
 #endif	/* _PCF8583REG_H */


Home | Main Index | Thread Index | Old Index