Re: kern/60700: Enhancement and man page for PCF8583

["Julian Coleman via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR kern/60700; it has been noted by GNATS.

From: Julian Coleman <jdc%coris.org.uk@localhost>
To: dtyson%anduin.org.uk@localhost, gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/60700: Enhancement and man page for PCF8583
Date: Tue, 22 Sep 2026 10:22:27 +0200

 Hi,
 
 Just a couple of small points.
 
 > +	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;
 > +		}
 > +	}
 > + 
 
 Do we always need to set the mode here, or does the chip retain the setting
 across powerdown?  I can't see it from the data sheet but I wonder about
 writing on each boot, in case the chip has a limited number of writes.
 Would it be better to read the current value and only write if we want to
 change it?
 
 For the aprint lines, I think that we could display the mode as well.  It
 could be useful to see from the dmesg which mode has been selected, in case
 we are looking for some problem (e.g. accidently running the wrong kernel)
 and it would be simple to add.
 
 Regards,
 
 Julian
 
 --