Re: Please change /proc/rtc (PATCH)

Ulrich Windl (Ulrich.Windl@rz.uni-regensburg.de)
Mon, 22 Apr 1996 16:19:21 +0200


On 22 Apr 96 at 1:44, Michael J. Micek wrote:

>
> > The current format of /proc/rtc is such that it would be very hard to
> > machine-parse, and even harder if the format ever changed. In
> > general, if a normal bash "read" command can't parse the format, it is
> > too complex.
>
> Makes sense.
>
> > * Essay format hard to parse; tend to create parsers that
> > break on any change
> > * Date format ambiguous internationally
> > (use YYYY-MM-DD format for dates, it is always unambiguous)
> > * Redundant header
> >
> > I would suggest formatting like this (based on /proc/cpuinfo format):
> >
> > date : 1996-04-21
> > time : 14:40:54
> > alarm : **:**:**

"--" instead of "**" wouldn't be that black, and this way look more
"off" tahn "on" ;-)

> > daylight : no
> > bcd : yes
> > 24hr : yes
> > sqwave : no
> > alarm_int : no
> > update_int : no
> > periodic_int : no
> > periodic_freq : 1024
> > battery_ok : yes
>
>
> Here's a patch (untested, but compiles okay) to do (almost)
> exactly this. I'd send it to Paul Gortmaker, but I don't
> know his address, either. I'm sending it to Alessandro
> Rubini, since he is listed as the maintainer of misc devices.
>
> I didn't really think about this; it's a no-brainer, so I
> just did it. Let me know if I'm off-base.
>
> (I padded with spaces instead of \t's. Oops.)
>
>
> ===BEGIN===
>
> --- rtc.c.old Fri Apr 19 17:37:52 1996
> +++ rtc.c Mon Apr 22 00:49:39 1996
> @@ -552,7 +552,6 @@
> restore_flags(flags);
>
> p = buf;
> - p += sprintf(p, "Real Time Clock Status:\n");
>
> get_rtc_time(&tm);
>
> @@ -560,9 +559,10 @@
> * There is no way to tell if the luser has the RTC set for local
> * time or for Universal Standard Time (GMT). Probably local though.
> */
> - p += sprintf(p, "\tRTC reports %02d:%02d:%02d of %d-%d-%d.\n",
> - tm.tm_hour, tm.tm_min, tm.tm_sec, tm.tm_mday,
> - tm.tm_mon + 1, tm.tm_year + 1900);
> + p += sprintf(p, "date : %04d-%02d-%02d\n",
> + tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday);
> + p += sprintf(p, "time : %02d-%02d-%02d\n",

Why not using ':' as separator for time? 17-33-59 looks strange as
time.

> + tm.tm_hour, tm.tm_min, tm.tm_sec);
>
> get_rtc_alm_time(&tm);
>
> @@ -571,36 +571,44 @@
> * match any value for that particular field. Values that are
> * greater than a valid time, but less than 0xc0 shouldn't appear.
> */
> - p += sprintf(p, "\tAlarm set to match: ");
> + p += sprintf(p, "alarm : ");

Isn't "alarm" a "periodic alarm"?

> if (tm.tm_hour <= 24)
> - p += sprintf(p, "hour=%d, ", tm.tm_hour);
> + p += sprintf(p, "%02d", tm.tm_hour);
> else
> - p += sprintf(p, "hour=any, ");
> + p += sprintf(p, "**");
> + p += sprintf(p, "-");
> if (tm.tm_min <= 59)
> - p += sprintf(p, "min=%d, ", tm.tm_min);
> + p += sprintf(p, "%02d", tm.tm_min);
> else
> - p += sprintf(p, "min=any, ");
> + p += sprintf(p, "**");
> + p += sprintf(p, "-");
> if (tm.tm_sec <= 59)
> - p += sprintf(p, "sec=%d.\n", tm.tm_sec);
> + p += sprintf(p, "%02d", tm.tm_sec);
> else
> - p += sprintf(p, "sec=any.\n");
> + p += sprintf(p, "**");
> + p += sprintf(p, "\n");

What about some mixed (and more compact) format like "alarm: **:17:33"
(with a personal preference for "--" instaed of "**")?

>
> - p += sprintf(p, "\tMisc. settings: daylight=%s; BCD=%s; 24hr=%s; Sq-Wave=%s.\n",
> - ((ctrl & RTC_DST_EN) ? "yes" : "no" ),
> - ((ctrl & RTC_DM_BINARY) ? "no" : "yes" ),
> - ((ctrl & RTC_24H) ? "yes" : "no" ),
> + p += sprintf(p, "daylight : %s\n",

We have daylight even during winter (even when it's shorter then).
What about a plain "DST" for daylight saving time?

> + ((ctrl & RTC_DST_EN) ? "yes" : "no" ));
> + p += sprintf(p, "bcd : %s\n",
> + ((ctrl & RTC_DM_BINARY) ? "no" : "yes" ));

"bcd" should be capitalized to "BCD".

> + p += sprintf(p, "24hr : %s\n",
> + ((ctrl & RTC_24H) ? "yes" : "no" ));
> + p += sprintf(p, "sqwave : %s\n",
> ((ctrl & RTC_SQWE) ? "yes" : "no" ));

Any problems with "square wave" instead?

>
> - p += sprintf(p, "\tInterrupt for: alarm=%s; update=%s; periodic=%s.\n",
> - ((ctrl & RTC_AIE) ? "yes" : "no" ),
> - ((ctrl & RTC_UIE) ? "yes" : "no" ),
> + p += sprintf(p, "alarm_int : %s\n",
> + ((ctrl & RTC_AIE) ? "yes" : "no" ));
> + p += sprintf(p, "update_int : %s\n",
> + ((ctrl & RTC_UIE) ? "yes" : "no" ));
> + p += sprintf(p, "periodic_int : %s\n",
> ((ctrl & RTC_PIE) ? "yes" : "no" ));
>
> - p += sprintf(p, "\tPeriodic interrupt rate set to %dHz.\n",
> + p += sprintf(p, "periodic_freq : %d\n",
> (freq ? (65536/(1<<freq)) : 0));

What about a plain "frequency: %d Hz\n" with "Hz" being a IS unit?

>
> - p += sprintf(p, "\tRTC reports that CMOS battery is %s.\n",
> - (batt ? "okay" : "dead"));
> + p += sprintf(p, "battery_ok : %s\n",
> + (batt ? "yes" : "no"));
>
> return p - buf;
> }
>
> ===END===
>
> --
> Michael J. Micek, peripatetic philosopher. (currently) mmicek@muddcs.cs.hmc.edu
> Am hirable (consulting, problem-solving, whatever). Finger for details.

Meant to be positive critics...