Very Computer

Right now there's a bit of a mess with all of the BIN_TO_BCD/BCD_TO_BIN
macros in the kernel.  It's defined in a half dozen places, and worse
yet, not all places use them the same way.  Most users do something
like:
if ( ... )
   BIN_TO_BCD(x);

But in a few places, it's used as:
if ( ... )
   y = BIN_TO_BCD(x);

The following creates include/linux/bcd.h which has the 'normal'
BIN_TO_BCD macros, as well as CONVERT_{BIN,BCD}_TO_{BCD,BIN},
which are for the second case.  This patch removes all other defines,
adds <linux/bcd.h> to <linux/mc146818rtc.h> for compatibility, and adds
<linux/bcd.h> directly to anything which was implicitly including
<linux/mc146818rtc.h> before.

This was originally sent back on August 12th, without any comment.

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

===== arch/arm/kernel/time.c 1.8 vs edited =====
--- 1.8/arch/arm/kernel/time.c  Sat Aug  3 06:39:48 2002
+++ edited/arch/arm/kernel/time.c       Tue Sep 17 11:01:55 2002
@@ -47,14 +47,6 @@
 /* change this if you have some constant time drift */
 #define USECS_PER_JIFFY        (1000000/HZ)

-#ifndef BCD_TO_BIN
-#define BCD_TO_BIN(val) ((val)=((val)&15) + ((val)>>4)*10)
-#endif
-
-#ifndef BIN_TO_BCD
-#define BIN_TO_BCD(val) ((val)=(((val)/10)<<4) + (val)%10)
-#endif
-
 static int dummy_set_rtc(void)
 {
        return 0;
===== arch/cris/drivers/ds1302.c 1.3 vs edited =====
--- 1.3/arch/cris/drivers/ds1302.c      Tue Feb  5 08:24:37 2002
+++ edited/arch/cris/drivers/ds1302.c   Tue Sep 17 11:01:55 2002
@@ -97,6 +97,7 @@
 #include <linux/module.h>
 #include <linux/miscdevice.h>
 #include <linux/delay.h>
+#include <linux/bcd.h>

 #include <asm/uaccess.h>
 #include <asm/system.h>
===== arch/cris/kernel/time.c 1.6 vs edited =====
--- 1.6/arch/cris/kernel/time.c Sun Jun 16 17:39:47 2002
+++ edited/arch/cris/kernel/time.c      Tue Sep 17 11:01:55 2002
@@ -32,6 +32,7 @@
 #include <linux/interrupt.h>
 #include <linux/time.h>
 #include <linux/delay.h>
+#include <linux/bcd.h>

 #include <asm/segment.h>
 #include <asm/io.h>
===== arch/m68k/sun3x/time.c 1.3 vs edited =====
--- 1.3/arch/m68k/sun3x/time.c  Sun Feb 24 05:50:59 2002
+++ edited/arch/m68k/sun3x/time.c       Tue Sep 17 11:01:55 2002
@@ -11,6 +11,7 @@
 #include <linux/kernel_stat.h>
 #include <linux/interrupt.h>
 #include <linux/rtc.h>
+#include <linux/bcd.h>

 #include <asm/irq.h>
 #include <asm/io.h>
@@ -36,9 +37,6 @@
 #define C_SIGN    0x20
 #define C_CALIB   0x1f

-#define BCD_TO_BIN(val) (((val)&15) + ((val)>>4)*10)
-#define BIN_TO_BCD(val) (((val/10) << 4) | (val % 10))
-
 int sun3x_hwclk(int set, struct rtc_time *t)
 {
        volatile struct mostek_dt *h =
@@ -49,23 +47,23 @@

        if(set) {
                h->csr |= C_WRITE;
-               h->sec = BIN_TO_BCD(t->tm_sec);
-               h->min = BIN_TO_BCD(t->tm_min);
-               h->hour = BIN_TO_BCD(t->tm_hour);
-               h->wday = BIN_TO_BCD(t->tm_wday);
-               h->mday = BIN_TO_BCD(t->tm_mday);
-               h->month = BIN_TO_BCD(t->tm_mon);
-               h->year = BIN_TO_BCD(t->tm_year);
+               h->sec = CONVERT_BIN_TO_BCD(t->tm_sec);
+               h->min = CONVERT_BIN_TO_BCD(t->tm_min);
+               h->hour = CONVERT_BIN_TO_BCD(t->tm_hour);
+               h->wday = CONVERT_BIN_TO_BCD(t->tm_wday);
+               h->mday = CONVERT_BIN_TO_BCD(t->tm_mday);
+               h->month = CONVERT_BIN_TO_BCD(t->tm_mon);
+               h->year = CONVERT_BIN_TO_BCD(t->tm_year);
                h->csr &= ~C_WRITE;
        } else {
                h->csr |= C_READ;
-               t->tm_sec = BCD_TO_BIN(h->sec);
-               t->tm_min = BCD_TO_BIN(h->min);
-               t->tm_hour = BCD_TO_BIN(h->hour);
-               t->tm_wday = BCD_TO_BIN(h->wday);
-               t->tm_mday = BCD_TO_BIN(h->mday);
-               t->tm_mon = BCD_TO_BIN(h->month);
-               t->tm_year = BCD_TO_BIN(h->year);
+               t->tm_sec = CONVERT_BCD_TO_BIN(h->sec);
+               t->tm_min = CONVERT_BCD_TO_BIN(h->min);
+               t->tm_hour = CONVERT_BCD_TO_BIN(h->hour);
+               t->tm_wday = CONVERT_BCD_TO_BIN(h->wday);
+               t->tm_mday = CONVERT_BCD_TO_BIN(h->mday);
+               t->tm_mon = CONVERT_BCD_TO_BIN(h->month);
+               t->tm_year = CONVERT_BCD_TO_BIN(h->year);
                h->csr &= ~C_READ;
        }

===== arch/mips/ddb5xxx/common/rtc_ds1386.c 1.1 vs edited =====
--- 1.1/arch/mips/ddb5xxx/common/rtc_ds1386.c   Tue Feb  5 13:17:17 2002
+++ edited/arch/mips/ddb5xxx/common/rtc_ds1386.c        Tue Sep 17 11:01:55 2002
@@ -20,6 +20,7 @@

 #include <linux/types.h>
 #include <linux/time.h>
+#include <linux/bcd.h>

 #include <asm/time.h>
 #include <asm/addrspace.h>
@@ -28,12 +29,6 @@

 #define        EPOCH           2000

-#undef BCD_TO_BIN
-#define BCD_TO_BIN(val) (((val)&15) + ((val)>>4)*10)
-
-#undef BIN_TO_BCD
-#define BIN_TO_BCD(val) ((((val)/10)<<4) + (val)%10)
-
 #define        READ_RTC(x)     *(volatile unsigned char*)(rtc_base+x)
 #define        WRITE_RTC(x, y) *(volatile unsigned char*)(rtc_base+x) = y

@@ -52,11 +47,11 @@
        WRITE_RTC(0xB, byte);

        /* read time data */
-       year = BCD_TO_BIN(READ_RTC(0xA)) + EPOCH;
-       month = BCD_TO_BIN(READ_RTC(0x9) & 0x1f);
-       day = BCD_TO_BIN(READ_RTC(0x8));
-       minute = BCD_TO_BIN(READ_RTC(0x2));
-       second = BCD_TO_BIN(READ_RTC(0x1));
+       year = CONVERT_BCD_TO_BIN(READ_RTC(0xA)) + EPOCH;
+       month = CONVERT_BCD_TO_BIN(READ_RTC(0x9) & 0x1f);
+       day = CONVERT_BCD_TO_BIN(READ_RTC(0x8));
+       minute = CONVERT_BCD_TO_BIN(READ_RTC(0x2));
+       second = CONVERT_BCD_TO_BIN(READ_RTC(0x1));

        /* hour is special - deal with it later */
        temp = READ_RTC(0x4);
@@ -68,11 +63,11 @@
        /* calc hour */
        if (temp & 0x40) {
                /* 12 hour format */
-               hour = BCD_TO_BIN(temp & 0x1f);
+               hour = CONVERT_BCD_TO_BIN(temp & 0x1f);
                if (temp & 0x20) hour += 12;                /* PM */
        } else {
                /* 24 hour format */
-               hour = BCD_TO_BIN(temp & 0x3f);
+               hour = CONVERT_BCD_TO_BIN(temp & 0x3f);
        }

        return mktime(year, month, day, hour, minute, second);
@@ -95,19 +90,19 @@
        to_tm(t, &tm);

        /* check each field one by one */
-       year = BIN_TO_BCD(tm.tm_year - EPOCH);
+       year = CONVERT_BIN_TO_BCD(tm.tm_year - EPOCH);
        if (year != READ_RTC(0xA)) {
                WRITE_RTC(0xA, year);
        }

        temp = READ_RTC(0x9);
-       month = BIN_TO_BCD(tm.tm_mon);
+       month = CONVERT_BIN_TO_BCD(tm.tm_mon);
        if (month != (temp & 0x1f)) {
                WRITE_RTC( 0x9,
                           (month & 0x1f) | (temp & ~0x1f) );
        }

-       day = BIN_TO_BCD(tm.tm_mday);
+       day = CONVERT_BIN_TO_BCD(tm.tm_mday);
        if (day != READ_RTC(0x8)) {
                WRITE_RTC(0x8, day);
        }
@@ -117,22 +112,22 @@
                /* 12 hour format */
                hour = 0x40;
                if (tm.tm_hour > 12) {
-                       hour |= 0x20 | (BIN_TO_BCD(hour-12) & 0x1f);
+                       hour |= 0x20 | (CONVERT_BIN_TO_BCD(hour-12) & 0x1f);
                } else {
-                       hour |= BIN_TO_BCD(tm.tm_hour);
+                       hour |= CONVERT_BIN_TO_BCD(tm.tm_hour);
                }
        } else {
                /* 24 hour format */
-               hour = BIN_TO_BCD(tm.tm_hour) & 0x3f;
+               hour = CONVERT_BIN_TO_BCD(tm.tm_hour) & 0x3f;
        }
        if (hour != temp) WRITE_RTC(0x4, hour);

-       minute = BIN_TO_BCD(tm.tm_min);
+       minute = CONVERT_BIN_TO_BCD(tm.tm_min);
        if (minute != READ_RTC(0x2)) {
                WRITE_RTC(0x2, minute);
        }

-       second = BIN_TO_BCD(tm.tm_sec);
+       second = CONVERT_BIN_TO_BCD(tm.tm_sec);
        if (second != READ_RTC(0x1)) {
                WRITE_RTC(0x1, second);
        }
===== arch/mips64/sgi-ip27/ip27-rtc.c 1.4 vs edited =====
--- 1.4/arch/mips64/sgi-ip27/ip27-rtc.c Thu May 23 09:06:16 2002
+++ edited/arch/mips64/sgi-ip27/ip27-rtc.c      Tue Sep 17 11:01:55 2002
@@ -35,6 +35,7 @@
 #include <linux/poll.h>
 #include <linux/proc_fs.h>
 #include <linux/smp_lock.h>
+#include <linux/bcd.h>

 #include <asm/m48t35.h>
 #include <asm/sn/ioc3.h>
===== arch/mips64/sgi-ip27/ip27-timer.c 1.2 vs edited =====
--- 1.2/arch/mips64/sgi-ip27/ip27-timer.c       Tue Feb  5 00:45:04 2002
+++ edited/arch/mips64/sgi-ip27/ip27-timer.c    Tue Sep 17 11:01:55 2002
@@ -11,6 +11,7 @@
 #include <linux/param.h>
 #include <linux/timex.h>
 #include <linux/mm.h>            
+#include <linux/bcd.h>

 #include <asm/pgtable.h>
 #include <asm/sgialib.h>
===== arch/ppc/iSeries/mf.c 1.2 vs edited =====
--- 1.2/arch/ppc/iSeries/mf.c   Sun Jun  2 23:49:59 2002
+++ edited/arch/ppc/iSeries/mf.c        Tue Sep 17 11:01:55 2002
@@ -40,6 +40,7 @@
 #include <asm/iSeries/iSeries_proc.h>
 #include <asm/uaccess.h>
 #include <linux/pci.h>
+#include <linux/bcd.h>

 /*
===== arch/ppc/kernel/todc_time.c 1.1 vs edited =====
--- 1.1/arch/ppc/kernel/todc_time.c     Sun Feb 10 04:20:03 2002
+++ edited/arch/ppc/kernel/todc_time.c  Tue Sep 17 11:01:55 2002
@@ -19,6 +19,7 @@
 #include <linux/kernel.h>
 #include <linux/time.h>
 #include <linux/timex.h>
+#include <linux/bcd.h>

 #include <asm/machdep.h>
 #include <asm/io.h>
===== arch/ppc/platforms/gemini_setup.c 1.11 vs edited =====
--- 1.11/arch/ppc/platforms/gemini_setup.c      Mon May 27 08:05:50 2002
+++ edited/arch/ppc/platforms/gemini_setup.c    Tue Sep 17 11:01:55 2002
@@ -27,6 +27,7 @@
 #include <linux/irq.h>
 #include <linux/seq_file.h>
 #include <linux/root_dev.h>
+#include <linux/bcd.h>

 #include <asm/system.h>
 #include <asm/pgtable.h>
===== arch/ppc/platforms/prep_time.c 1.6 vs edited =====
--- 1.6/arch/ppc/platforms/prep_time.c  Sun Feb 10 04:41:25 2002
+++ edited/arch/ppc/platforms/prep_time.c       Tue Sep 17 11:01:55 2002
@@ -22,6 +22,7 @@
 #include <linux/timex.h>
 #include <linux/kernel_stat.h>
 #include <linux/init.h>
+#include <linux/bcd.h>

 #include <asm/sections.h>
 #include <asm/segment.h>
===== arch/ppc64/kernel/mf.c 1.2 vs edited =====
--- 1.2/arch/ppc64/kernel/mf.c  Wed Apr 24 21:46:36 2002
+++ edited/arch/ppc64/kernel/mf.c       Tue Sep 17 11:01:55 2002
@@ -40,6 +40,7 @@
 #include <asm/iSeries/iSeries_proc.h>
 #include <asm/uaccess.h>
 #include <linux/pci.h>
+#include <linux/bcd.h>

 extern struct pci_dev * iSeries_vio_dev;

===== arch/sh/kernel/rtc.c 1.5 vs edited =====
--- 1.5/arch/sh/kernel/rtc.c    Tue Feb  5 08:24:41 2002
+++ edited/arch/sh/kernel/rtc.c Tue Sep 17 11:01:55 2002
@@ -9,17 +9,10 @@
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/time.h>
+#include <linux/bcd.h>

 #include <asm/io.h>
 #include <asm/rtc.h>
-
-#ifndef BCD_TO_BIN
-#define BCD_TO_BIN(val) ((val)=((val)&15) + ((val)>>4)*10)
-#endif
-
-#ifndef BIN_TO_BCD
-#define BIN_TO_BCD(val) ((val)=(((val)/10)<<4) + (val)%10)
-#endif

 void sh_rtc_gettimeofday(struct timeval *tv)
 {
===== arch/sparc64/kernel/time.c 1.14 vs edited =====
--- 1.14/arch/sparc64/kernel/time.c     Fri Sep 13 14:41:56 2002
+++ edited/arch/sparc64/kernel/time.c   Tue
...

read more »

hmmm... removing all the private definitions certainly makes good sense,
but having both CONVERT_foo and foo seems a bit wonky...

IMO it would be better to have BIN_TO_BCD which returns a value, and
__BIN_TO_BCD which has side effects but returns no value...

        Jeff

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Well, this was done in part to minimize change.  The version which
returns no value is far more common than the one which does, and would
require changing a lot more files (and would also make getting this into
2.4 harder too, which I would like to do someday if this gets into 2.5).
The other reason is that CONVERT_foo makes it quite obvious what is being
done, where as __xxx at least in my mind has namespace imlpications
(like how it's used in libc, etc.  But kernel namespace isn't like the
rest I know..).

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

The other thing, is that in general people seem to expect BIN_TO_BCD(X) to
not return a value, and just convert X.  Would it be better to replace
CONVERT_x to __x then ?

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

My gut feeling is that the users in the majority -- the ones that don't
return a value -- are still abnormal.  Side effects on arguments are the
rare case in C, even if it is the common case here.

But to answer your question, I think s/CONVERT_x/__x/ is better than
nothing...

        Jeff

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/