2002-09-17 18:25:17

by Tom Rini

[permalink] [raw]
Subject: [PATCH][RESEND] Cleanup (BIN|BCD)_TO_(BCD|BIN) usage/macros

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 Sep 17 11:01:56 2002
@@ -316,14 +316,6 @@
return (data1 == data2); /* Was the write blocked? */
}

-#ifndef BCD_TO_BIN
-#define BCD_TO_BIN(val) (((val)&15) + ((val)>>4)*10)
-#endif
-
-#ifndef BIN_TO_BCD
-#define BIN_TO_BCD(val) ((((val)/10)<<4) + (val)%10)
-#endif
-
/* Probe for the real time clock chip. */
static void __init set_system_time(void)
{
===== drivers/scsi/sr_vendor.c 1.8 vs edited =====
--- 1.8/drivers/scsi/sr_vendor.c Wed Sep 4 14:24:17 2002
+++ edited/drivers/scsi/sr_vendor.c Tue Sep 17 11:01:57 2002
@@ -37,6 +37,7 @@
#include <linux/config.h>
#include <linux/errno.h>
#include <linux/string.h>
+#include <linux/bcd.h>

#include <linux/blk.h>
#include "scsi.h"
@@ -147,8 +148,6 @@
/* This function gets called after a media change. Checks if the CD is
multisession, asks for offset etc. */

-#define BCD_TO_BIN(x) ((((int)x & 0xf0) >> 4)*10 + ((int)x & 0x0f))
-
int sr_cd_check(struct cdrom_device_info *cdi)
{
Scsi_CD *cd = cdi->handle;
@@ -213,9 +212,9 @@
no_multi = 1;
break;
}
- min = BCD_TO_BIN(buffer[15]);
- sec = BCD_TO_BIN(buffer[16]);
- frame = BCD_TO_BIN(buffer[17]);
+ min = CONVERT_BCD_TO_BIN(buffer[15]);
+ sec = CONVERT_BCD_TO_BIN(buffer[16]);
+ frame = CONVERT_BCD_TO_BIN(buffer[17]);
sector = min * CD_SECS * CD_FRAMES + sec * CD_FRAMES + frame;
break;
}
@@ -240,9 +239,9 @@
}
if (rc != 0)
break;
- min = BCD_TO_BIN(buffer[1]);
- sec = BCD_TO_BIN(buffer[2]);
- frame = BCD_TO_BIN(buffer[3]);
+ min = CONVERT_BCD_TO_BIN(buffer[1]);
+ sec = CONVERT_BCD_TO_BIN(buffer[2]);
+ frame = CONVERT_BCD_TO_BIN(buffer[3]);
sector = min * CD_SECS * CD_FRAMES + sec * CD_FRAMES + frame;
if (sector)
sector -= CD_MSF_OFFSET;
===== drivers/sgi/char/ds1286.c 1.4 vs edited =====
--- 1.4/drivers/sgi/char/ds1286.c Tue Feb 5 00:44:52 2002
+++ edited/drivers/sgi/char/ds1286.c Tue Sep 17 11:01:57 2002
@@ -36,6 +36,7 @@
#include <linux/poll.h>
#include <linux/rtc.h>
#include <linux/spinlock.h>
+#include <linux/bcd.h>

#include <asm/ds1286.h>
#include <asm/io.h>
===== include/asm-cris/rtc.h 1.2 vs edited =====
--- 1.2/include/asm-cris/rtc.h Tue Feb 5 00:40:14 2002
+++ edited/include/asm-cris/rtc.h Tue Sep 17 11:01:57 2002
@@ -39,11 +39,6 @@
#define RTC_INIT() (-1)
#endif

-/* conversions to and from the stupid RTC internal format */
-
-#define BCD_TO_BIN(x) x = (((x & 0xf0) >> 3) * 5 + (x & 0xf))
-#define BIN_TO_BCD(x) x = (x % 10) | ((x / 10) << 4)
-
/*
* The struct used to pass data via the following ioctl. Similar to the
* struct tm in <time.h>, but it needs to be here so that the kernel
===== include/asm-mips/ds1286.h 1.1 vs edited =====
--- 1.1/include/asm-mips/ds1286.h Tue Feb 5 10:39:45 2002
+++ edited/include/asm-mips/ds1286.h Tue Sep 17 11:01:57 2002
@@ -57,15 +57,4 @@
#define RTC_IPSW 0x40
#define RTC_TE 0x80

-/*
- * Conversion between binary and BCD.
- */
-#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
-
#endif /* _ASM_DS1286_h */
===== include/asm-mips64/ds1286.h 1.2 vs edited =====
--- 1.2/include/asm-mips64/ds1286.h Tue Feb 5 00:45:05 2002
+++ edited/include/asm-mips64/ds1286.h Tue Sep 17 11:01:57 2002
@@ -56,15 +56,4 @@
#define RTC_IPSW 0x40
#define RTC_TE 0x80

-/*
- * Conversion between binary and BCD.
- */
-#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
-
#endif /* _ASM_DS1286_h */
===== include/asm-mips64/m48t35.h 1.1 vs edited =====
--- 1.1/include/asm-mips64/m48t35.h Tue Feb 5 10:39:55 2002
+++ edited/include/asm-mips64/m48t35.h Tue Sep 17 11:01:57 2002
@@ -21,12 +21,4 @@
#define M48T35_RTC_STOPPED 0x80
#define M48T35_RTC_READ 0x40

-#ifndef BCD_TO_BIN
-#define BCD_TO_BIN(x) ((x)=((x)&15) + ((x)>>4)*10)
-#endif
-
-#ifndef BIN_TO_BCD
-#define BIN_TO_BCD(x) ((x)=(((x)/10)<<4) + (x)%10)
-#endif
-
#endif
===== include/asm-ppc/m48t35.h 1.3 vs edited =====
--- 1.3/include/asm-ppc/m48t35.h Tue Feb 5 00:54:05 2002
+++ edited/include/asm-ppc/m48t35.h Tue Sep 17 11:01:57 2002
@@ -76,14 +76,4 @@
#define M48T35_RTC_STOPPED 0x80
#define M48T35_RTC_READ 0x40

-
-/* read/write conversions */
-#ifndef BCD_TO_BIN
-#define BCD_TO_BIN(x) ((x)=((x)&15) + ((x)>>4)*10)
-#endif
-
-#ifndef BIN_TO_BCD
-#define BIN_TO_BCD(x) ((x)=(((x)/10)<<4) + (x)%10)
-#endif
-
#endif
===== include/asm-ppc/mk48t59.h 1.2 vs edited =====
--- 1.2/include/asm-ppc/mk48t59.h Tue Feb 5 00:40:23 2002
+++ edited/include/asm-ppc/mk48t59.h Tue Sep 17 11:01:57 2002
@@ -27,12 +27,4 @@
#define MK48T59_RTC_CONTROLB 0x1FF9
#define MK48T59_RTC_CB_STOP 0x80

-#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
-
#endif /* _PPC_MK48T59_H */
===== include/asm-ppc/nvram.h 1.2 vs edited =====
--- 1.2/include/asm-ppc/nvram.h Tue Feb 5 00:40:23 2002
+++ edited/include/asm-ppc/nvram.h Tue Sep 17 11:01:57 2002
@@ -26,14 +26,6 @@
#define MOTO_RTC_CONTROLA 0x1FF8
#define MOTO_RTC_CONTROLB 0x1FF9

-#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
-
/* PowerMac specific nvram stuffs */

enum {
===== include/asm-ppc/todc.h 1.1 vs edited =====
--- 1.1/include/asm-ppc/todc.h Sun Feb 10 03:45:43 2002
+++ edited/include/asm-ppc/todc.h Tue Sep 17 11:01:57 2002
@@ -355,14 +355,6 @@
todc_info->flags = clock_type ##_FLAGS; \
}

-#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
-
extern todc_info_t *todc_info;

unsigned char todc_direct_read_val(int addr);
===== include/asm-ppc64/nvram.h 1.2 vs edited =====
--- 1.2/include/asm-ppc64/nvram.h Fri Mar 15 19:14:30 2002
+++ edited/include/asm-ppc64/nvram.h Tue Sep 17 11:01:57 2002
@@ -28,12 +28,4 @@
#define MOTO_RTC_CONTROLA 0x1FF8
#define MOTO_RTC_CONTROLB 0x1FF9

-#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
-
#endif /* _PPC64_NVRAM_H */
===== include/linux/mc146818rtc.h 1.2 vs edited =====
--- 1.2/include/linux/mc146818rtc.h Tue Feb 5 00:45:03 2002
+++ edited/include/linux/mc146818rtc.h Tue Sep 17 11:01:57 2002
@@ -14,6 +14,7 @@
#include <asm/io.h>
#include <linux/rtc.h> /* get the user-level API */
#include <linux/spinlock.h> /* spinlock_t */
+#include <linux/bcd.h> /* (BIN|BCD)_TO_(BCD|BIN) */
#include <asm/mc146818rtc.h> /* register access macros */

extern spinlock_t rtc_lock; /* serialize CMOS RAM access */
@@ -86,16 +87,5 @@
#define RTC_VALID RTC_REG_D
# define RTC_VRT 0x80 /* valid RAM and time */
/**********************************************************************/
-
-/* example: !(CMOS_READ(RTC_CONTROL) & RTC_DM_BINARY)
- * determines if the following two #defines are needed
- */
-#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

#endif /* _MC146818RTC_H */
--- /dev/null 1969-12-31 17:00:00.000000000 -0700
+++ include/linux/bcd.h 2002-09-17 11:01:57.000000000 -0700
@@ -0,0 +1,28 @@
+/*
+ * include/linux/bcd.h: Macros to convert to/from Binary Coded Decimals.
+ *
+ * Originally in numerous files throughout the kernel.
+ *
+ * Author: Tom Rini
+ * [email protected]
+ * Copyright 2002 MontaVista Software Inc.
+ *
+ * Please read the COPYING file for all license details.
+ */
+
+#ifndef __LINUX_BCD_H__
+#define __LINUX_BCD_H__
+
+/*
+ * Just convert the value, and don't change the original.
+ */
+#define CONVERT_BIN_TO_BCD(val) ((((val) / 10) << 4) + (val) % 10)
+#define CONVERT_BCD_TO_BIN(val) (((val) & 15) + ((val) >> 4) * 10)
+
+/*
+ * Change the variable 'val' from one to the other.
+ */
+#define BIN_TO_BCD(val) ((val) = CONVERT_BIN_TO_BCD((val)))
+#define BCD_TO_BIN(val) ((val) = CONVERT_BCD_TO_BIN((val)))
+
+#endif /* __LINUX_BCD_H__ */


2002-09-17 18:35:32

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH][RESEND] Cleanup (BIN|BCD)_TO_(BCD|BIN) usage/macros

Tom Rini wrote:
> 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.


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



2002-09-17 18:45:02

by Tom Rini

[permalink] [raw]
Subject: Re: [PATCH][RESEND] Cleanup (BIN|BCD)_TO_(BCD|BIN) usage/macros

On Tue, Sep 17, 2002 at 02:39:59PM -0400, Jeff Garzik wrote:
> Tom Rini wrote:
> >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.
>
> 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...

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/

2002-09-20 14:34:56

by Tom Rini

[permalink] [raw]
Subject: Re: [PATCH][RESEND] Cleanup (BIN|BCD)_TO_(BCD|BIN) usage/macros

On Tue, Sep 17, 2002 at 02:39:59PM -0400, Jeff Garzik wrote:

> Tom Rini wrote:
> >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.
>
> 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...

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/

2002-09-20 14:51:34

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH][RESEND] Cleanup (BIN|BCD)_TO_(BCD|BIN) usage/macros

Tom Rini wrote:
> 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 ?


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