Patch of a new driver for kernel 2.4.x that need review

Patch of a new driver for kernel 2.4.x that need review

[Bouchard, Sebastien]

Hi,

Here is a driver (only for 2.4.x) I've done to provide support of a hardware
module (available on some ATCA board) used with telecom expension card on
the new ATCA platform. This module provide redundant reference clock for
telecom hardware with alarm handling when there is a new event (ex.: one of
the ref clock is gone).
This char driver provide IOCTL for configuration of this module and
interrupt handler for processing alarm events.

I send this driver so people in this mailing list can do a review of the
code.

Please reply me directly to my email, i'm not subscribed to the mailing
list.

Thanks
Sebastien Bouchard 
Software designer
Kontron Canada Inc. 
<mailto:sebastien.bouchard@ca.kontron.com> 
<http://www.kontron.com/> 


diff -ruN 2.4.31-ori/drivers/char/Config.in
2.4.31-mod/drivers/char/Config.in
--- 2.4.31-ori/drivers/char/Config.in	Sat Aug  7 19:26:04 2004
+++ 2.4.31-mod/drivers/char/Config.in	Wed Jun 22 09:34:57 2005
@@ -401,4 +401,6 @@
    dep_tristate 'HP OB600 C/CT Pop-up mouse support' CONFIG_OBMOUSE
$CONFIG_INPUT_MOUSEDEV
 fi
 
+tristate 'Telecom clock support' CONFIG_TELCLOCK
+
 endmenu
diff -ruN 2.4.31-ori/drivers/char/Makefile 2.4.31-mod/drivers/char/Makefile
--- 2.4.31-ori/drivers/char/Makefile	Sat Aug  7 19:26:04 2004
+++ 2.4.31-mod/drivers/char/Makefile	Wed Jun 22 09:36:14 2005
@@ -323,6 +323,7 @@
 obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
 obj-$(CONFIG_INDYDOG) += indydog.o
 obj-$(CONFIG_8xx_WDT) += mpc8xx_wdt.o
+obj-$(CONFIG_TELCLOCK) += tlclk.o
 
 subdir-$(CONFIG_MWAVE) += mwave
 ifeq ($(CONFIG_MWAVE),y)
diff -ruN 2.4.31-ori/drivers/char/tlclk.c 2.4.31-mod/drivers/char/tlclk.c
--- 2.4.31-ori/drivers/char/tlclk.c	Wed Dec 31 19:00:00 1969
+++ 2.4.31-mod/drivers/char/tlclk.c	Wed Jun 22 09:43:10 2005
@@ -0,0 +1,470 @@
+/*
+ * Telecom Clock driver for Wainwright board
+ *
+ * Copyright (C) 2005 Kontron Canada
+ *
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <sebastien.bouchard@ca.kontron.com>
+ *
+ * Description : This is the TELECOM CLOCK module driver for the ATCA
platform.
+ */
+#ifndef __KERNEL__
+#  define __KERNEL__
+#endif
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/sched.h>
+#include <linux/kernel.h>	/* printk() */
+#include <linux/fs.h>		/* everything... */
+#include <linux/errno.h>	/* error codes */
+#include <linux/delay.h>	/* udelay */
+#include <asm/uaccess.h>
+#include <linux/slab.h>
+#include <linux/ioport.h>
+#include <linux/devfs_fs_kernel.h>	/* Devfs support */
+#include <linux/interrupt.h>
+#include <linux/spinlock.h>
+#include <linux/timer.h>
+#include <linux/tqueue.h>	/* interrupt task */
+#include <asm/io.h>		/* inb/outb */
+
+#include "tlclk.h"	/* TELECOM IOCTL DEFINE */
+
+MODULE_AUTHOR("Sebastien Bouchard <sebastien.bouchard@ca.kontron.com>");
+MODULE_LICENSE("GPL");
+
+/* Telecom clock I/O register definition */
+#define TLCLK_BASE 0xa08            
+#define TLCLK_REG0 TLCLK_BASE
+#define TLCLK_REG1 TLCLK_BASE+1
+#define TLCLK_REG2 TLCLK_BASE+2
+#define TLCLK_REG3 TLCLK_BASE+3
+#define TLCLK_REG4 TLCLK_BASE+4
+#define TLCLK_REG5 TLCLK_BASE+5
+#define TLCLK_REG6 TLCLK_BASE+6
+#define TLCLK_REG7 TLCLK_BASE+7
+
+#define SET_PORT_BITS(port, mask, val) outb(((inb(port) & mask) | val),
port)
+
+/* 0 = Dynamic allocation of the major device number */
+#define TLCLK_MAJOR 252
+
+static int telclk_interrupt;	 /* Contain the interrupt used for telecom
clock */
+
+static int int_events;		/* Event that generate a interrupt */
+static int got_event;		/* if events processing have been done */
+
+static struct timer_list switchover_timer;
+
+struct tlclk_alarms *alarm_events;
+
+spinlock_t event_lock = SPIN_LOCK_UNLOCKED;
+
+/* DEVFS support or not */
+#ifdef CONFIG_DEVFS_FS
+devfs_handle_t devfs_handle;
+#else
+static int tlclk_major = TLCLK_MAJOR;
+#endif
+
+static void switchover_timeout(unsigned long data);
+void tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs);
+
+DECLARE_WAIT_QUEUE_HEAD(wq);
+/*
+*  Function : Module I/O functions
+*  Description : Almost all the control stuff is done here, check I/O dn
for help.
+*/
+static int
+tlclk_ioctl(struct inode *inode,
+	    struct file *filp, unsigned int cmd, unsigned long arg)
+{
+   unsigned long flags;
+	unsigned char val;
+	val = (char) arg;
+
+	if (_IOC_TYPE(cmd) != TLCLK_IOC_MAGIC)
+		return -ENOTTY;
+
+	if (_IOC_NR(cmd) > TLCLK_IOC_MAXNR)
+		return -ENOTTY;
+
+	switch (cmd) {
+	case IOCTL_RESET:
+		SET_PORT_BITS(TLCLK_REG4, 0xfd, val);
+		break;
+
+	case IOCTL_MODE_SELECT:
+		SET_PORT_BITS(TLCLK_REG0, 0xcf, val);
+		break;
+
+	case IOCTL_REFALIGN:
+		/* GENERATING 0 to 1 transistion */
+		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
+		udelay(2);
+		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0x08);
+		udelay(2);
+		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
+		break;
+
+	case IOCTL_HARDWARE_SWITCHING:
+		SET_PORT_BITS(TLCLK_REG0, 0x7f, val);
+		break;
+
+	case IOCTL_HARDWARE_SWITCHING_MODE:
+		SET_PORT_BITS(TLCLK_REG0, 0xbf, val);
+		break;
+
+	case IOCTL_FILTER_SELECT:
+		SET_PORT_BITS(TLCLK_REG0, 0xfb, val);
+		break;
+
+	case IOCTL_SELECT_REF_FREQUENCY:
+	   spin_lock_irqsave(&event_lock, flags);
+		SET_PORT_BITS(TLCLK_REG1, 0xfd, val);
+		spin_unlock_irqrestore(&event_lock, flags);
+		break;
+
+	case IOCTL_SELECT_REDUNDANT_CLOCK:
+	   spin_lock_irqsave(&event_lock, flags);
+		SET_PORT_BITS(TLCLK_REG1, 0xfe, val);
+		spin_unlock_irqrestore(&event_lock, flags);
+		
+		break;
+
+	case IOCTL_SELECT_AMCB1_TRANSMIT_CLOCK:
+		if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
+			SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x5);
+			SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
+		} else if (val >= CLK_8_592MHz) {
+			SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x7);
+			switch (val) {
+			case CLK_8_592MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+				break;
+			case CLK_11_184MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
+				break;
+			case CLK_34_368MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
+				break;
+			case CLK_44_736MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+				break;
+			}
+		} else
+			SET_PORT_BITS(TLCLK_REG3, 0xf8, val);
+		break;
+
+		break;
+	case IOCTL_SELECT_AMCB2_TRANSMIT_CLOCK:
+		if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
+			SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x28);
+			SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
+		} else if (val >= CLK_8_592MHz) {
+			SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x38);
+			switch (val) {
+			case CLK_8_592MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+				break;
+			case CLK_11_184MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
+				break;
+			case CLK_34_368MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
+				break;
+			case CLK_44_736MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+				break;
+			}
+		} else
+			SET_PORT_BITS(TLCLK_REG3, 0xc7, val << 3);
+		break;
+		break;
+	case IOCTL_TEST_MODE:
+		SET_PORT_BITS(TLCLK_REG4, 0xfd, 2);
+		break;
+
+	case IOCTL_ENABLE_CLKA0_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG2, 0xfe, val);
+		break;
+	case IOCTL_ENABLE_CLKB0_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG2, 0xfd, val << 1);
+		break;
+	case IOCTL_ENABLE_CLKA1_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG2, 0xfb, val << 2);
+		break;
+	case IOCTL_ENABLE_CLKB1_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG2, 0xf7, val << 3);
+		break;
+	case IOCTL_ENABLE_CLK3A_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG3, 0xbf, val << 6);
+		break;
+	case IOCTL_ENABLE_CLK3B_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG3, 0x7f, val << 7);
+		break;
+	case IOCTL_READ_ALARMS:
+		return (inb(TLCLK_REG2) & 0xf0);
+		break;
+	case IOCTL_READ_INTERRUPT_SWITCH:
+		return inb(TLCLK_REG6);
+		break;
+	case IOCTL_READ_CURRENT_REF:
+		return ((inb(TLCLK_REG1) & 0x08) >> 3);
+		break;
+	}
+
+	return 0;
+}
+
+/*
+*  Function : Module Opening
+*  Description : Called when a program open the 
+*  /dev/telclock file related to the module.
+*/
+static int
+tlclk_open(struct inode *inode, struct file *filp)
+{
+	int result;
+#ifdef MODULE
+	if (!MOD_IN_USE) {
+		MOD_INC_USE_COUNT;
+#endif
+		/* Make sure there is no interrupt pending will 
+		   *  initialising interrupt handler */
+		inb(TLCLK_REG6);
+
+		result = request_irq(telclk_interrupt, &tlclk_interrupt,
+				     SA_SHIRQ, "telclock", tlclk_interrupt);
+		printk("\ntelclock: Reserving IRQ%d...\n",
telclk_interrupt);
+		if (result == -EBUSY) {
+			printk(KERN_ERR
+			       "telclock: Interrupt can't be reserved!\n");
+			return -EBUSY;
+		}
+		inb(TLCLK_REG6);	/* Clear interrupt events */
+#ifdef MODULE
+	} else
+		return -EBUSY;
+#endif
+	return 0;
+}
+
+/*
+*  Function : Module Releasing
+*  Description : Called when a program stop using the module.
+*/
+static int
+tlclk_release(struct inode *inode, struct file *filp)
+{
+	MOD_DEC_USE_COUNT;
+	if (!MOD_IN_USE)
+		free_irq(telclk_interrupt, tlclk_interrupt);
+	return 0;
+}
+
+ssize_t
+tlclk_read(struct file * filp, char *buf, size_t count, loff_t * f_pos)
+{
+	int count0 = sizeof (struct tlclk_alarms);
+	wait_event_interruptible(wq, got_event);
+	if (copy_to_user
+	    (buf, (char *) alarm_events, sizeof (struct tlclk_alarms)))
+		return -EFAULT;
+   
+   memset(alarm_events, 0, sizeof (struct tlclk_alarms));
+	got_event = 0;
+	
+	return count0;
+}
+
+ssize_t
+tlclk_write(struct file * filp, const char *buf, size_t count, loff_t *
f_pos)
+{
+	return 0;
+}
+
+/*
+* This is where you set what function is called when an action is done to
your  * /dev file.
+*/
+static struct file_operations tlclk_fops = {
+	read:tlclk_read,
+	write:tlclk_write,
+	ioctl:tlclk_ioctl,
+	open:tlclk_open,
+	release:tlclk_release,
+
+};
+/*
+* Function : Module Initialisation                      
+* Description : Called at module loading, 
+* all the OS registering stuff is her
+*/
+static int __init
+tlclk_init(void)
+{
+	alarm_events = kmalloc(sizeof (struct tlclk_alarms), GFP_KERNEL);
+	
+	if(!alarm_events)
+		   return -EBUSY;
+   
+   memset(alarm_events, 0, sizeof (struct tlclk_alarms));
+
+/* Read telecom clock IRQ number (Set by BIOS) */
+
+	telclk_interrupt = (inb(TLCLK_REG7) & 0x0f);
+
+	printk(KERN_WARNING "telclock: Reserving I/O region...\n");
+
+	if (check_region(TLCLK_BASE, 8)) {
+		printk(KERN_ERR
+		       "telclock: I/O region already used by another
driver!\n");
+		return -EBUSY;
+	} else {
+		request_region(TLCLK_BASE, 8, "telclock");
+	}
+
+/* DEVFS or NOT? */
+
+#ifdef CONFIG_DEVFS_FS
+	devfs_handle = devfs_register(NULL, "telclock",
+				      DEVFS_FL_AUTO_DEVNUM, TLCLK_MAJOR,
+				      0,
+				      S_IFCHR | S_IRUGO | S_IWUGO,
+				      &tlclk_fops, NULL);
+	if (!devfs_handle)
+		return -EBUSY;
+#else
+	tlclk_major = register_chrdev(tlclk_major, "telclock", &tlclk_fops);
+
+	if (tlclk_major < 0) {
+		printk(KERN_ERR "telclock: can't get major! %d\n",
tlclk_major);
+		return tlclk_major;
+	}
+#endif
+
+	init_timer(&switchover_timer);
+	switchover_timer.function = switchover_timeout;
+	switchover_timer.data = 0;
+
+	return 0;
+}
+
+/*
+*  Function : Module Cleaning

+*  Description : Called when unloading the module
+*/
+static void __exit
+tlclk_cleanup(void)
+{
+#ifdef CONFIG_DEVFS_FS
+	devfs_unregister(devfs_handle);
+#else
+	unregister_chrdev(tlclk_major, "telclock");
+#endif
+	release_region(TLCLK_BASE, 8);
+	del_timer_sync(&switchover_timer);
+	kfree(alarm_events);
+	
+}
+static void
+switchover_timeout(unsigned long data)
+{
+	if ((data & 1)) {
+		if ((inb(TLCLK_REG1) & 0x08) != (data & 0x08))
+			alarm_events->switchover_primary++;
+	} else {
+		if ((inb(TLCLK_REG1) & 0x08) != (data & 0x08))
+			alarm_events->switchover_secondary++;
+	}
+
+	/* Alarm processing is done, wake up read task */
+	del_timer(&switchover_timer);
+	got_event = 1;
+	wake_up(&wq);
+}
+/*
+*  Function : Interrupt Handler

+*  Description :
+*  Handling of alarms interrupt.
+*  
+*/
+void
+tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs)
+{
+	unsigned long flags;
+	spin_lock_irqsave(&event_lock, flags);
+	int_events = inb(TLCLK_REG6);	            /* Read and clear
interrupt events */
+	spin_unlock_irqrestore(&event_lock, flags);
+	
+	/* Primary_Los changed from 0 to 1 ? */
+	if (int_events & PRI_LOS_01_MASK) {
+		if (inb(TLCLK_REG2) & SEC_LOST_MASK)
+			alarm_events->lost_clocks++;
+		else
+			alarm_events->lost_primary_clock++;
+	}
+
+	/* Primary_Los changed from 1 to 0 ? */
+	if (int_events & PRI_LOS_10_MASK) {
+		alarm_events->primary_clock_back++;
+		spin_lock_irqsave(&event_lock, flags);
+		SET_PORT_BITS(TLCLK_REG1, 0xFE, 1);
+		spin_unlock_irqrestore(&event_lock, flags);
+	}
+	/* Secondary_Los changed from 0 to 1 ? */
+	if (int_events & SEC_LOS_01_MASK) {
+		if (inb(TLCLK_REG2) & PRI_LOST_MASK)
+			alarm_events->lost_clocks++;
+		else
+			alarm_events->lost_secondary_clock++;
+	}
+	/* Secondary_Los changed from 1 to 0 ? */
+	if (int_events & SEC_LOS_10_MASK) {
+		alarm_events->secondary_clock_back++;
+		spin_lock_irqsave(&event_lock, flags);
+		SET_PORT_BITS(TLCLK_REG1, 0xFE, 0);
+		spin_unlock_irqrestore(&event_lock, flags);
+	}
+	if (int_events & HOLDOVER_10_MASK)
+		alarm_events->pll_end_holdover++;
+
+	if (int_events & UNLOCK_01_MASK)
+		alarm_events->pll_lost_sync++;
+
+	if (int_events & UNLOCK_10_MASK)
+		alarm_events->pll_sync++;
+
+	/* Holdover changed from 0 to 1 ? */
+	if (int_events & HOLDOVER_01_MASK) {
+		alarm_events->pll_holdover++;
+
+		switchover_timer.expires = jiffies + 1;	/* TIMEOUT in ~10ms
*/
+		switchover_timer.data = inb(TLCLK_REG1);
+		add_timer(&switchover_timer);
+	} else {                                     
+	   got_event = 1;
+		wake_up(&wq);
+	}
+}
+
+module_init(tlclk_init);
+module_exit(tlclk_cleanup);
diff -ruN 2.4.31-ori/drivers/char/tlclk.h 2.4.31-mod/drivers/char/tlclk.h
--- 2.4.31-ori/drivers/char/tlclk.h	Wed Dec 31 19:00:00 1969
+++ 2.4.31-mod/drivers/char/tlclk.h	Wed Jun 22 09:43:10 2005
@@ -0,0 +1,167 @@
+/*
+ * Telecom Clock driver for Wainwright board
+ *
+ * Copyright (C) 2005 Kontron Canada
+ *
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <sebastien.bouchard@ca.kontron.com>
+ *
+ */
+ 
+/* Ioctl definitions  */
+
+/* Use 0xA1 as magic number */
+#define TLCLK_IOC_MAGIC 0xA1
+
+/*Hardware Reset of the PLL */
+
+#define RESET_ON 0x00
+#define RESET_OFF 0x01
+#define IOCTL_RESET _IO(TLCLK_IOC_MAGIC,  1)
+
+#define IOCTL_REFALIGN _IO(TLCLK_IOC_MAGIC,  2)
+
+/* MODE SELECT */
+
+#define NORMAL_MODE 0x00
+#define HOLDOVER_MODE 0x10
+#define FREERUN_MODE 0x20
+
+#define IOCTL_MODE_SELECT _IOR(TLCLK_IOC_MAGIC,  3, char)
+
+/* FILTER SELECT */
+
+#define FILTER_6HZ 0x04
+#define FILTER_12HZ 0x00
+
+#define IOCTL_FILTER_SELECT _IOR(TLCLK_IOC_MAGIC,  4, char)
+
+/* SELECT REFERENCE FREQUENCY */
+
+#define REF_CLK1_8kHz 0x00
+#define REF_CLK2_19_44MHz 0x02
+
+#define IOCTL_SELECT_REF_FREQUENCY _IOR(TLCLK_IOC_MAGIC,  6, char)
+
+/* Select primary or secondary redundant clock */
+
+#define PRIMARY_CLOCK 0x00
+#define SECONDARY_CLOCK 0x01
+#define IOCTL_SELECT_REDUNDANT_CLOCK _IOR(TLCLK_IOC_MAGIC,  7, char)
+
+/* CLOCK TRANSMISSION DEFINE */
+
+#define CLK_8kHz 0xff
+#define CLK_16_384MHz 0xfb
+
+#define CLK_1_544MHz 0x00
+#define CLK_2_048MHz 0x01
+#define CLK_4_096MHz 0x02
+#define CLK_6_312MHz 0x03
+#define CLK_8_192MHz 0x04
+#define CLK_19_440MHz 0x06
+
+#define CLK_8_592MHz 0x08
+#define CLK_11_184MHz 0x09
+#define CLK_34_368MHz 0x0b
+#define CLK_44_736MHz 0x0a
+
+#define IOCTL_SELECT_AMCB1_TRANSMIT_CLOCK _IOR(TLCLK_IOC_MAGIC,  9, char)
+#define IOCTL_SELECT_AMCB2_TRANSMIT_CLOCK _IOR(TLCLK_IOC_MAGIC,  10, char)
+
+/* RECEIVED REFERENCE */
+
+#define AMC_B1 0
+#define AMC_B2 1
+
+#define IOCTL_SELECT_RECEIVED_REF_CLK3A _IOR(TLCLK_IOC_MAGIC,  11, char)
+#define IOCTL_SELECT_RECEIVED_REF_CLK3B _IOR(TLCLK_IOC_MAGIC,  12, char)
+
+/* OEM COMMAND - NOT IN FINAL VERSION */
+
+#define IOCTL_TEST_MODE _IO(TLCLK_IOC_MAGIC,  13)
+
+/* HARDWARE SWITCHING DEFINE */
+
+#define HW_ENABLE 0x80
+#define HW_DISABLE 0x00
+
+#define IOCTL_HARDWARE_SWITCHING _IOR(TLCLK_IOC_MAGIC,  14, char)
+
+/* HARDWARE SWITCHING MODE DEFINE */
+
+#define PLL_HOLDOVER 0x40
+#define LOST_CLOCK 0x00
+
+#define IOCTL_HARDWARE_SWITCHING_MODE _IOR(TLCLK_IOC_MAGIC,  15, char)
+
+/* CLOCK OUTPUT DEFINE */
+
+#define IOCTL_ENABLE_CLKA0_OUTPUT _IOR(TLCLK_IOC_MAGIC,  16, char)
+#define IOCTL_ENABLE_CLKB0_OUTPUT _IOR(TLCLK_IOC_MAGIC,  17, char)
+#define IOCTL_ENABLE_CLKA1_OUTPUT _IOR(TLCLK_IOC_MAGIC,  18, char)
+#define IOCTL_ENABLE_CLKB1_OUTPUT _IOR(TLCLK_IOC_MAGIC,  19, char)
+
+#define IOCTL_ENABLE_CLK3A_OUTPUT _IOR(TLCLK_IOC_MAGIC,  20, char)
+#define IOCTL_ENABLE_CLK3B_OUTPUT _IOR(TLCLK_IOC_MAGIC,  21, char)
+
+/* ALARMS DEFINE */
+
+#define UNLOCK_MASK 0x10
+#define HOLDOVER_MASK 0x20
+#define SEC_LOST_MASK 0x40
+#define PRI_LOST_MASK 0x80
+
+#define IOCTL_READ_ALARMS _IO(TLCLK_IOC_MAGIC,  22)
+
+/* INTERRUPT CAUSE DEFINE */
+
+#define PRI_LOS_01_MASK 0x01
+#define PRI_LOS_10_MASK 0x02
+
+#define SEC_LOS_01_MASK 0x04
+#define SEC_LOS_10_MASK 0x08
+
+#define HOLDOVER_01_MASK 0x10
+#define HOLDOVER_10_MASK 0x20
+
+#define UNLOCK_01_MASK 0x40
+#define UNLOCK_10_MASK 0x80
+
+#define IOCTL_READ_INTERRUPT_SWITCH _IO(TLCLK_IOC_MAGIC,  23)
+
+#define IOCTL_READ_CURRENT_REF _IO(TLCLK_IOC_MAGIC,  25)
+
+/* MAX NUMBER OF IOCTL */
+#define TLCLK_IOC_MAXNR 25
+
+struct tlclk_alarms {
+   unsigned int lost_clocks;
+   unsigned int lost_primary_clock;
+   unsigned int lost_secondary_clock;
+   unsigned int primary_clock_back;
+   unsigned int secondary_clock_back;
+   unsigned int switchover_primary;
+   unsigned int switchover_secondary;
+   unsigned int pll_holdover;
+   unsigned int pll_end_holdover;
+   unsigned int pll_lost_sync;
+   unsigned int pll_sync;
+};
+
Sebastien Bouchard 
Software designer
Kontron Canada Inc. 
<mailto:sebastien.bouchard@ca.kontron.com> 
<http://www.kontron.com/> 

Re: Patch of a new driver for kernel 2.4.x that need review

[Pekka Enberg]

Hi,

Some comments below. Please note that I am more familiar with 2.6 so
some of these might not apply.

                       Pekka

On 6/22/05, Bouchard, Sebastien <Sebastien.Bouchard@ca.kontron.com> wrote:
> --- 2.4.31-ori/drivers/char/tlclk.c     Wed Dec 31 19:00:00 1969
> +++ 2.4.31-mod/drivers/char/tlclk.c     Wed Jun 22 09:43:10 2005
> +/* Telecom clock I/O register definition */
> +#define TLCLK_BASE 0xa08
> +#define TLCLK_REG0 TLCLK_BASE
> +#define TLCLK_REG1 TLCLK_BASE+1
> +#define TLCLK_REG2 TLCLK_BASE+2
> +#define TLCLK_REG3 TLCLK_BASE+3
> +#define TLCLK_REG4 TLCLK_BASE+4
> +#define TLCLK_REG5 TLCLK_BASE+5
> +#define TLCLK_REG6 TLCLK_BASE+6
> +#define TLCLK_REG7 TLCLK_BASE+7

Please use enums instead.

> +/*
> +*  Function : Module I/O functions
> +*  Description : Almost all the control stuff is done here, check I/O dn
> for help.
> +*/

Please use kerneldoc format.

> +static struct file_operations tlclk_fops = {
> +       read:tlclk_read,
> +       write:tlclk_write,
> +       ioctl:tlclk_ioctl,
> +       open:tlclk_open,
> +       release:tlclk_release,

Please use C99 struct initializers.

> +
> +};
> +/*
> +* Function : Module Initialisation
> +* Description : Called at module loading,
> +* all the OS registering stuff is her
> +*/
> +static int __init
> +tlclk_init(void)
> +{
> +       alarm_events = kmalloc(sizeof (struct tlclk_alarms), GFP_KERNEL);
> +
> +       if(!alarm_events)
> +                  return -EBUSY;
> +
> +   memset(alarm_events, 0, sizeof (struct tlclk_alarms));
> +
> +/* Read telecom clock IRQ number (Set by BIOS) */
> +
> +       telclk_interrupt = (inb(TLCLK_REG7) & 0x0f);
> +
> +       printk(KERN_WARNING "telclock: Reserving I/O region...\n");
> +
> +       if (check_region(TLCLK_BASE, 8)) {
> +               printk(KERN_ERR
> +                      "telclock: I/O region already used by another
> driver!\n");
> +               return -EBUSY;

You're leaking alarm_events here.

> +       } else {
> +               request_region(TLCLK_BASE, 8, "telclock");

Please put nominal case first (i.e. make this the first block). 

> +       }
> +
> +/* DEVFS or NOT? */
> +
> +#ifdef CONFIG_DEVFS_FS
> +       devfs_handle = devfs_register(NULL, "telclock",
> +                                     DEVFS_FL_AUTO_DEVNUM, TLCLK_MAJOR,
> +                                     0,
> +                                     S_IFCHR | S_IRUGO | S_IWUGO,
> +                                     &tlclk_fops, NULL);
> +       if (!devfs_handle)
> +               return -EBUSY;

You're leaking alarm_events and reserved region here. (Use gotos for
error handling, btw.)

> +#else
> +       tlclk_major = register_chrdev(tlclk_major, "telclock", &tlclk_fops);
> +
> +       if (tlclk_major < 0) {
> +               printk(KERN_ERR "telclock: can't get major! %d\n",
> tlclk_major);
> +               return tlclk_major;

Same as before plus leaking devfs handle.

> +       }
> +#endif
> +
> +       init_timer(&switchover_timer);
> +       switchover_timer.function = switchover_timeout;
> +       switchover_timer.data = 0;
> +
> +       return 0;
> +}

> diff -ruN 2.4.31-ori/drivers/char/tlclk.h 2.4.31-mod/drivers/char/tlclk.h
> --- 2.4.31-ori/drivers/char/tlclk.h     Wed Dec 31 19:00:00 1969
> +++ 2.4.31-mod/drivers/char/tlclk.h     Wed Jun 22 09:43:10 2005
> +/* Ioctl definitions  */
> +
> +/* Use 0xA1 as magic number */
> +#define TLCLK_IOC_MAGIC 0xA1
> +
> +/*Hardware Reset of the PLL */
> +
> +#define RESET_ON 0x00
> +#define RESET_OFF 0x01

Please use enums instead (applies to other parts of this file too).

Re: Patch of a new driver for kernel 2.4.x that need review

[Willy Tarreau]

Hi,

On Wed, Jun 22, 2005 at 10:43:40PM +0300, Pekka Enberg wrote:
> On 6/22/05, Bouchard, Sebastien <Sebastien.Bouchard@ca.kontron.com> wrote:
> > --- 2.4.31-ori/drivers/char/tlclk.c     Wed Dec 31 19:00:00 1969
> > +++ 2.4.31-mod/drivers/char/tlclk.c     Wed Jun 22 09:43:10 2005
> > +/* Telecom clock I/O register definition */
> > +#define TLCLK_BASE 0xa08
> > +#define TLCLK_REG0 TLCLK_BASE
> > +#define TLCLK_REG1 TLCLK_BASE+1
> > +#define TLCLK_REG2 TLCLK_BASE+2
> > +#define TLCLK_REG3 TLCLK_BASE+3
> > +#define TLCLK_REG4 TLCLK_BASE+4
> > +#define TLCLK_REG5 TLCLK_BASE+5
> > +#define TLCLK_REG6 TLCLK_BASE+6
> > +#define TLCLK_REG7 TLCLK_BASE+7
> 
> Please use enums instead.

I dont agree with you here : enums are good to simply specify an ordering.
But they must not be used to specify static mapping. Eg: if REG4 *must* be
equal to BASE+4, you should not use enums, otherwise it will render the
code unreadable. I personnaly don't want to count the position of REG7 in
the enum to discover that it's at BASE+7.

> > +#define RESET_ON 0x00
> > +#define RESET_OFF 0x01
> 
> Please use enums instead (applies to other parts of this file too).

Same comment here : if writing the *bit* 0 asserts the reset line, and
writing the *bit* 1 deasserts it, enum is not adequate at all. Enums are
adequate when you don't care about the values themselves, eg the sysctl
entries, etc... (eventhough most of them are statically assigned). But
if you need something more verbose to say exactly "write 7 to port 123",
it's better to use defines (or even variables sometimes) than enums.

Regards,
Willy

Re: Patch of a new driver for kernel 2.4.x that need review

[Bill Gatliff]

Willy:

Willy Tarreau wrote:

>Hi,
>
>On Wed, Jun 22, 2005 at 10:43:40PM +0300, Pekka Enberg wrote:
>  
>
>>On 6/22/05, Bouchard, Sebastien <Sebastien.Bouchard@ca.kontron.com> wrote:
>>    
>>
>>>+#define TLCLK_REG7 TLCLK_BASE+7
>>>      
>>>
>>Please use enums instead.
>>    
>>
>
>I dont agree with you here : enums are good to simply specify an ordering.
>But they must not be used to specify static mapping. Eg: if REG4 *must* be
>equal to BASE+4, you should not use enums, otherwise it will render the
>code unreadable. I personnaly don't want to count the position of REG7 in
>the enum to discover that it's at BASE+7.
>  
>

What Sebastien is after is something like this:

	enum tclk_regid {TCLK_BASE=0xa80, TCLK_REG0=TCLK_BASE, TCLK_REG1=TCLK_BASE+1...};
	enum tclk_regid tclk;

And then later on, if you ask gdb with the value of tclk is, it can tell you "TCLK_REG1", instead of just 0xa801.  You can also assign values to tclk from within gdb using the enumerations, rather than magic numbers.

If you insist on using #defines, then you need to do them like this at the very least:

	#define TCLK_REG7 (TCLK_BASE+7)

... so as to prevent operator precedence problems later on.  I.e. what happens here:

	tclk = TCLK_REG7 / 2;

Not implying that the above is a realistic example, I'm just pointing out a potential gotcha that is easily avoided...



b.g.

-- 
Bill Gatliff
"A DTI spokesman said Wicks would use his debut announcement to
'reaffirm the government's commitment to developing wind' to tackle
the threat of climate change." -- The Observer, May 22, 2005
bgat@billgatliff.com

Re: Patch of a new driver for kernel 2.4.x that need review

[Willy Tarreau]

On Wed, Jun 22, 2005 at 03:59:12PM -0500, Bill Gatliff wrote:
 
> What Sebastien is after is something like this:
> 
> 	enum tclk_regid {TCLK_BASE=0xa80, TCLK_REG0=TCLK_BASE, 
> 	TCLK_REG1=TCLK_BASE+1...};
> 	enum tclk_regid tclk;
> 
> And then later on, if you ask gdb with the value of tclk is, it can tell 
> you "TCLK_REG1", instead of just 0xa801.  You can also assign values to 
> tclk from within gdb using the enumerations, rather than magic numbers.

Bill, this is a good reason, I agree with you. What I did not want to see
was something like :

  enum { TCLK_BASE=0xa80, TCLK_REG0, TCLK_REG1, ... }

> If you insist on using #defines, then you need to do them like this at 
> the very least:
> 
> 	#define TCLK_REG7 (TCLK_BASE+7)
> 
> ... so as to prevent operator precedence problems later on.  I.e. what 
> happens here:
> 
> 	tclk = TCLK_REG7 / 2;
> 
> Not implying that the above is a realistic example, I'm just pointing out 
> a potential gotcha that is easily avoided...

indeed, nearly all defines need to get lots of parenthesis for the exact
same reason.

Thanks for pointing out the gdb trick.
Willy

Re: Patch of a new driver for kernel 2.4.x that need review

[Pekka Enberg]

Bill,

On 6/22/05, Bill Gatliff <bgat@billgatliff.com> wrote:
> What Sebastien is after is something like this:

Please don't trim down cc's when replying. You just cost me five
minutes to write an obsolete reply :)

                          Pekka

Re: Patch of a new driver for kernel 2.4.x that need review

[Pekka J Enberg]

Hi Willy, 

Willy Tarreau writes:
> I dont agree with you here : enums are good to simply specify an ordering.
> But they must not be used to specify static mapping. Eg: if REG4 *must* be
> equal to BASE+4, you should not use enums, otherwise it will render the
> code unreadable. I personnaly don't want to count the position of REG7 in
> the enum to discover that it's at BASE+7.

Sorry, what do you have to count with the following? 

enum {
       TLCLK_REG0 = TLCLK_BASE,
       TLCLK_REG1 = TLCLK_BASE+1,
       TLCLK_REG2 = TLCLK_BASE+2,
}; 

Please note that enums are a general way of specifying _constants_ with the 
type int, not necessarily named enumerations. 

Willy Tarreau writes:
> if you need something more verbose to say exactly "write 7 to port 123",
> it's better to use defines (or even variables sometimes) than enums.

I would agree on variables (for some special cases) but not for defines. The 
problem with defines is that you can override a constant without even 
noticing it. Therefore, always use enums when you can, and defines only when 
you must. 

                  Pekka 

Re: Patch of a new driver for kernel 2.4.x that need review

[Willy Tarreau]

Hi Pekka,

On Thu, Jun 23, 2005 at 07:16:17AM +0300, Pekka J Enberg wrote:
> Hi Willy, 
> 
> Willy Tarreau writes:
> >I dont agree with you here : enums are good to simply specify an 
> >ordering.
> >But they must not be used to specify static mapping. Eg: if REG4 *must* 
> >be
> >equal to BASE+4, you should not use enums, otherwise it will render the
> >code unreadable. I personnaly don't want to count the position of REG7 in
> >the enum to discover that it's at BASE+7.
> 
> Sorry, what do you have to count with the following? 
> 
> enum {
>       TLCLK_REG0 = TLCLK_BASE,
>       TLCLK_REG1 = TLCLK_BASE+1,
>       TLCLK_REG2 = TLCLK_BASE+2,
> }; 

Sorry for the noise, I replied in a second mail that I was perfectly OK
with this usage. What I though you wanted to propose was the simplest for
of enum where only the first value is specified, and which is a nightmare
to debug afterwards. Bill Gatliff also suggested that gdb can display and
use the symbolic values while it's not the case on defines.
 
Regards,
Willy

Re: Patch of a new driver for kernel 2.4.x that need review

[Mark Gross]

On Wednesday 22 June 2005 21:49, Willy Tarreau wrote:
> Hi Pekka,
>
> On Thu, Jun 23, 2005 at 07:16:17AM +0300, Pekka J Enberg wrote:
> > Hi Willy,
> >
> > Willy Tarreau writes:
> > >I dont agree with you here : enums are good to simply specify an
> > >ordering.
> > >But they must not be used to specify static mapping. Eg: if REG4 *must*
> > >be
> > >equal to BASE+4, you should not use enums, otherwise it will render the
> > >code unreadable. I personnaly don't want to count the position of REG7
> > > in the enum to discover that it's at BASE+7.
> >
> > Sorry, what do you have to count with the following?
> >
> > enum {
> >       TLCLK_REG0 = TLCLK_BASE,
> >       TLCLK_REG1 = TLCLK_BASE+1,
> >       TLCLK_REG2 = TLCLK_BASE+2,
> > };
>
> Sorry for the noise, I replied in a second mail that I was perfectly OK
> with this usage. What I though you wanted to propose was the simplest for
> of enum where only the first value is specified, and which is a nightmare
> to debug afterwards. Bill Gatliff also suggested that gdb can display and
> use the symbolic values while it's not the case on defines.
>
> Regards,
> Willy
>

I'm helping out Sebastien a bit with maintaining this driver attached is an update that attempts to 
address the commens from this thread.  The 2.6 version of this driver will be posted separately.

--mgross

diff -urN -X dontdiff_osdl vanilla/linux-2.4.31/drivers/char/Config.in linux-2.4/drivers/char/Config.in
--- vanilla/linux-2.4.31/drivers/char/Config.in	2004-08-07 16:26:04.000000000 -0700
+++ linux-2.4/drivers/char/Config.in	2005-07-06 12:30:28.000000000 -0700
@@ -401,4 +401,6 @@
    dep_tristate 'HP OB600 C/CT Pop-up mouse support' CONFIG_OBMOUSE $CONFIG_INPUT_MOUSEDEV
 fi
 
+tristate 'Telecom clock support' CONFIG_TELCLOCK
+
 endmenu
diff -urN -X dontdiff_osdl vanilla/linux-2.4.31/drivers/char/Makefile linux-2.4/drivers/char/Makefile
--- vanilla/linux-2.4.31/drivers/char/Makefile	2004-08-07 16:26:04.000000000 -0700
+++ linux-2.4/drivers/char/Makefile	2005-07-06 12:30:28.000000000 -0700
@@ -323,6 +323,7 @@
 obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
 obj-$(CONFIG_INDYDOG) += indydog.o
 obj-$(CONFIG_8xx_WDT) += mpc8xx_wdt.o
+obj-$(CONFIG_TELCLOCK) += tlclk.o
 
 subdir-$(CONFIG_MWAVE) += mwave
 ifeq ($(CONFIG_MWAVE),y)
diff -urN -X dontdiff_osdl vanilla/linux-2.4.31/drivers/char/tlclk.c linux-2.4/drivers/char/tlclk.c
--- vanilla/linux-2.4.31/drivers/char/tlclk.c	1969-12-31 16:00:00.000000000 -0800
+++ linux-2.4/drivers/char/tlclk.c	2005-07-06 13:21:24.000000000 -0700
@@ -0,0 +1,459 @@
+/*
+ * Telecom Clock driver for Wainwright board
+ *
+ * Copyright (C) 2005 Kontron Canada
+ *
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <sebastien.bouchard@ca.kontron.com>
+ *
+ * Description : This is the TELECOM CLOCK module driver for the ATCA platform.
+ */
+#ifndef __KERNEL__
+#  define __KERNEL__
+#endif
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/sched.h>
+#include <linux/kernel.h>	/* printk() */
+#include <linux/fs.h>		/* everything... */
+#include <linux/errno.h>	/* error codes */
+#include <linux/delay.h>	/* udelay */
+#include <asm/uaccess.h>
+#include <linux/slab.h>
+#include <linux/ioport.h>
+#include <linux/devfs_fs_kernel.h>	/* Devfs support */
+#include <linux/interrupt.h>
+#include <linux/spinlock.h>
+#include <linux/timer.h>
+#include <linux/tqueue.h>	/* interrupt task */
+#include <asm/io.h>		/* inb/outb */
+
+#include "tlclk.h"	/* TELECOM IOCTL DEFINE */
+
+MODULE_AUTHOR("Sebastien Bouchard <sebastien.bouchard@ca.kontron.com>");
+MODULE_LICENSE("GPL");
+
+/* Telecom clock I/O register definition */
+#define TLCLK_BASE 0xa08            
+#define TLCLK_REG0 TLCLK_BASE
+#define TLCLK_REG1 (TLCLK_BASE+1)
+#define TLCLK_REG2 (TLCLK_BASE+2)
+#define TLCLK_REG3 (TLCLK_BASE+3)
+#define TLCLK_REG4 (TLCLK_BASE+4)
+#define TLCLK_REG5 (TLCLK_BASE+5)
+#define TLCLK_REG6 (TLCLK_BASE+6)
+#define TLCLK_REG7 (TLCLK_BASE+7)
+
+#define SET_PORT_BITS(port, mask, val) outb(((inb(port) & mask) | val), port)
+
+/* 0 = Dynamic allocation of the major device number */
+#define TLCLK_MAJOR 252
+
+/* Contain the interrupt used for telecom clock */
+static int telclk_interrupt; 
+
+static int int_events;		/* Event that generate a interrupt */
+static int got_event;		/* if events processing have been done */
+
+static struct timer_list switchover_timer;
+
+struct tlclk_alarms *alarm_events;
+
+spinlock_t event_lock = SPIN_LOCK_UNLOCKED;
+
+/* DEVFS support or not */
+#ifdef CONFIG_DEVFS_FS
+devfs_handle_t devfs_handle;
+#else
+static int tlclk_major = TLCLK_MAJOR;
+#endif
+
+static void switchover_timeout(unsigned long data);
+void tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs);
+
+DECLARE_WAIT_QUEUE_HEAD(wq);
+/*
+*  Function : Module I/O functions
+*  Description : Almost all the control stuff is done here, check I/O dn for
+*  help.
+*/
+static int
+tlclk_ioctl(struct inode *inode,
+	    struct file *filp, unsigned int cmd, unsigned long arg)
+{
+	unsigned long flags;
+	unsigned char val;
+	val = (unsigned char) arg;
+
+	if (_IOC_TYPE(cmd) != TLCLK_IOC_MAGIC)
+		return -ENOTTY;
+
+	if (_IOC_NR(cmd) > TLCLK_IOC_MAXNR)
+		return -ENOTTY;
+
+	switch (cmd) {
+	case IOCTL_RESET:
+		SET_PORT_BITS(TLCLK_REG4, 0xfd, val);
+		break;
+	case IOCTL_MODE_SELECT:
+		SET_PORT_BITS(TLCLK_REG0, 0xcf, val);
+		break;
+	case IOCTL_REFALIGN:
+		/* GENERATING 0 to 1 transistion */
+		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
+		udelay(2);
+		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0x08);
+		udelay(2);
+		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
+		break;
+	case IOCTL_HARDWARE_SWITCHING:
+		SET_PORT_BITS(TLCLK_REG0, 0x7f, val);
+		break;
+	case IOCTL_HARDWARE_SWITCHING_MODE:
+		SET_PORT_BITS(TLCLK_REG0, 0xbf, val);
+		break;
+	case IOCTL_FILTER_SELECT:
+		SET_PORT_BITS(TLCLK_REG0, 0xfb, val);
+		break;
+	case IOCTL_SELECT_REF_FREQUENCY:
+		spin_lock_irqsave(&event_lock, flags);
+		SET_PORT_BITS(TLCLK_REG1, 0xfd, val);
+		spin_unlock_irqrestore(&event_lock, flags);
+		break;
+	case IOCTL_SELECT_REDUNDANT_CLOCK:
+		spin_lock_irqsave(&event_lock, flags);
+		SET_PORT_BITS(TLCLK_REG1, 0xfe, val);
+		spin_unlock_irqrestore(&event_lock, flags);
+		break;
+	case IOCTL_SELECT_AMCB1_TRANSMIT_CLOCK:
+		if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
+			SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x5);
+			SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
+		} else if (val >= CLK_8_592MHz) {
+			SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x7);
+			switch (val) {
+			case CLK_8_592MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+				break;
+			case CLK_11_184MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
+				break;
+			case CLK_34_368MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
+				break;
+			case CLK_44_736MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+				break;
+			}
+		} else
+			SET_PORT_BITS(TLCLK_REG3, 0xf8, val);
+		break;
+	case IOCTL_SELECT_AMCB2_TRANSMIT_CLOCK:
+		if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
+			SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x28);
+			SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
+		} else if (val >= CLK_8_592MHz) {
+			SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x38);
+			switch (val) {
+			case CLK_8_592MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+				break;
+			case CLK_11_184MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
+				break;
+			case CLK_34_368MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
+				break;
+			case CLK_44_736MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+				break;
+			}
+		} else
+			SET_PORT_BITS(TLCLK_REG3, 0xc7, val << 3);
+		break;
+	case IOCTL_TEST_MODE:
+		SET_PORT_BITS(TLCLK_REG4, 0xfd, 2);
+		break;
+	case IOCTL_ENABLE_CLKA0_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG2, 0xfe, val);
+		break;
+	case IOCTL_ENABLE_CLKB0_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG2, 0xfd, val << 1);
+		break;
+	case IOCTL_ENABLE_CLKA1_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG2, 0xfb, val << 2);
+		break;
+	case IOCTL_ENABLE_CLKB1_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG2, 0xf7, val << 3);
+		break;
+	case IOCTL_ENABLE_CLK3A_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG3, 0xbf, val << 6);
+		break;
+	case IOCTL_ENABLE_CLK3B_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG3, 0x7f, val << 7);
+		break;
+	case IOCTL_READ_ALARMS:
+		return (inb(TLCLK_REG2) & 0xf0);
+		break;
+	case IOCTL_READ_INTERRUPT_SWITCH:
+		return inb(TLCLK_REG6);
+		break;
+	case IOCTL_READ_CURRENT_REF:
+		return ((inb(TLCLK_REG1) & 0x08) >> 3);
+		break;
+	}
+
+	return 0;
+}
+
+/*
+*  Function : Module Opening
+*  Description : Called when a program open the 
+*  /dev/telclock file related to the module.
+*/
+static int
+tlclk_open(struct inode *inode, struct file *filp)
+{
+	int result;
+#ifdef MODULE
+	if (!MOD_IN_USE) {
+		MOD_INC_USE_COUNT;
+#endif
+		/* Make sure there is no interrupt pending will 
+		   *  initialising interrupt handler */
+		inb(TLCLK_REG6);
+
+		result = request_irq(telclk_interrupt, &tlclk_interrupt,
+					SA_SHIRQ, "telclock", tlclk_interrupt);
+		printk("\ntelclock: Reserving IRQ%d...\n", telclk_interrupt);
+		if (result == -EBUSY) {
+			printk(KERN_ERR
+				"telclock: Interrupt can't be reserved!\n");
+			return -EBUSY;
+		}
+		inb(TLCLK_REG6);	/* Clear interrupt events */
+#ifdef MODULE
+	} else
+		return -EBUSY;
+#endif
+	return 0;
+}
+
+/*
+*  Function : Module Releasing
+*  Description : Called when a program stop using the module.
+*/
+static int
+tlclk_release(struct inode *inode, struct file *filp)
+{
+	MOD_DEC_USE_COUNT;
+	if (!MOD_IN_USE)
+		free_irq(telclk_interrupt, tlclk_interrupt);
+	return 0;
+}
+
+ssize_t
+tlclk_read(struct file * filp, char *buf, size_t count, loff_t * f_pos)
+{
+	int count0 = sizeof(struct tlclk_alarms);
+	wait_event_interruptible(wq, got_event);
+	if (copy_to_user(buf, alarm_events, sizeof(struct tlclk_alarms)))
+		return -EFAULT;
+
+	memset(alarm_events, 0, sizeof(struct tlclk_alarms));
+	got_event = 0;
+	
+	return count0;
+}
+
+ssize_t
+tlclk_write(struct file * filp, const char *buf, size_t count, loff_t * f_pos)
+{
+	return 0;
+}
+
+/*
+* This is where you set what function is called when an action is done to your
+*  /dev file.
+*/
+static struct file_operations tlclk_fops = {
+	.read = tlclk_read,
+	.write = tlclk_write,
+	.ioctl = tlclk_ioctl,
+	.open = tlclk_open,
+	.release = tlclk_release,
+
+};
+/*
+* Function : Module Initialisation                      
+* Description : Called at module loading, 
+* all the OS registering stuff is her
+*/
+static int __init
+tlclk_init(void)
+{
+/* DEVFS or NOT? */
+#ifdef CONFIG_DEVFS_FS
+	devfs_handle = devfs_register(NULL, "telclock",
+					DEVFS_FL_AUTO_DEVNUM, TLCLK_MAJOR,
+					0,
+					S_IFCHR | S_IRUGO | S_IWUGO,
+					&tlclk_fops, NULL);
+	if (!devfs_handle)
+		goto out1;
+#else
+	tlclk_major = register_chrdev(tlclk_major, "telclock", &tlclk_fops);
+
+	if (tlclk_major < 0) {
+		printk(KERN_ERR "telclock: can't get major! %d\n", tlclk_major);
+		return tlclk_major;
+	}
+#endif
+
+	alarm_events = kmalloc(sizeof(struct tlclk_alarms), GFP_KERNEL);
+	
+	if (!alarm_events)
+		goto out1;
+	
+	memset(alarm_events, 0, sizeof(struct tlclk_alarms));
+
+/* Read telecom clock IRQ number (Set by BIOS) */
+
+	printk(KERN_WARNING "telclock: Reserving I/O region...\n");
+
+	if ( !request_region(TLCLK_BASE, 8, "telclock") ) {
+		printk(KERN_ERR "telclock: request_region failed!\n");
+			goto out2;
+	}
+	telclk_interrupt = (inb(TLCLK_REG7) & 0x0f);
+
+	init_timer(&switchover_timer);
+	switchover_timer.function = switchover_timeout;
+	switchover_timer.data = 0;
+
+	return 0;
+out2:
+	kfree(alarm_events);
+out1:
+	return -EBUSY;
+}
+
+/*
+*  Function : Module Cleaning 
+*  Description : Called when unloading the module
+*/
+static void __exit
+tlclk_cleanup(void)
+{
+#ifdef CONFIG_DEVFS_FS
+	devfs_unregister(devfs_handle);
+#else
+	unregister_chrdev(tlclk_major, "telclock");
+#endif
+	release_region(TLCLK_BASE, 8);
+	del_timer_sync(&switchover_timer);
+	kfree(alarm_events);
+	
+}
+static void
+switchover_timeout(unsigned long data)
+{
+	if ((data & 1)) {
+		if ((inb(TLCLK_REG1) & 0x08) != (data & 0x08))
+			alarm_events->switchover_primary++;
+	} else {
+		if ((inb(TLCLK_REG1) & 0x08) != (data & 0x08))
+			alarm_events->switchover_secondary++;
+	}
+
+	/* Alarm processing is done, wake up read task */
+	del_timer(&switchover_timer);
+	got_event = 1;
+	wake_up(&wq);
+}
+/*
+*  Function : Interrupt Handler 
+*  Description :
+*  Handling of alarms interrupt.
+*  
+*/
+void
+tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs)
+{
+	unsigned long flags;
+	spin_lock_irqsave(&event_lock, flags);
+	/* Read and clear interrupt events */
+	int_events = inb(TLCLK_REG6);
+	spin_unlock_irqrestore(&event_lock, flags);
+	
+	/* Primary_Los changed from 0 to 1 ? */
+	if (int_events & PRI_LOS_01_MASK) {
+		if (inb(TLCLK_REG2) & SEC_LOST_MASK)
+			alarm_events->lost_clocks++;
+		else
+			alarm_events->lost_primary_clock++;
+	}
+
+	/* Primary_Los changed from 1 to 0 ? */
+	if (int_events & PRI_LOS_10_MASK) {
+		alarm_events->primary_clock_back++;
+		spin_lock_irqsave(&event_lock, flags);
+		SET_PORT_BITS(TLCLK_REG1, 0xFE, 1);
+		spin_unlock_irqrestore(&event_lock, flags);
+	}
+	/* Secondary_Los changed from 0 to 1 ? */
+	if (int_events & SEC_LOS_01_MASK) {
+		if (inb(TLCLK_REG2) & PRI_LOST_MASK)
+			alarm_events->lost_clocks++;
+		else
+			alarm_events->lost_secondary_clock++;
+	}
+	/* Secondary_Los changed from 1 to 0 ? */
+	if (int_events & SEC_LOS_10_MASK) {
+		alarm_events->secondary_clock_back++;
+		spin_lock_irqsave(&event_lock, flags);
+		SET_PORT_BITS(TLCLK_REG1, 0xFE, 0);
+		spin_unlock_irqrestore(&event_lock, flags);
+	}
+	if (int_events & HOLDOVER_10_MASK)
+		alarm_events->pll_end_holdover++;
+
+	if (int_events & UNLOCK_01_MASK)
+		alarm_events->pll_lost_sync++;
+
+	if (int_events & UNLOCK_10_MASK)
+		alarm_events->pll_sync++;
+
+	/* Holdover changed from 0 to 1 ? */
+	if (int_events & HOLDOVER_01_MASK) {
+		alarm_events->pll_holdover++;
+
+		switchover_timer.expires = jiffies + 1;	/* TIMEOUT in ~10ms */
+		switchover_timer.data = inb(TLCLK_REG1);
+		add_timer(&switchover_timer);
+	} else {
+		got_event = 1;
+		wake_up(&wq);
+	}
+}
+
+module_init(tlclk_init);
+module_exit(tlclk_cleanup);
diff -urN -X dontdiff_osdl vanilla/linux-2.4.31/drivers/char/tlclk.h linux-2.4/drivers/char/tlclk.h
--- vanilla/linux-2.4.31/drivers/char/tlclk.h	1969-12-31 16:00:00.000000000 -0800
+++ linux-2.4/drivers/char/tlclk.h	2005-07-06 12:30:28.000000000 -0700
@@ -0,0 +1,167 @@
+/*
+ * Telecom Clock driver for Wainwright board
+ *
+ * Copyright (C) 2005 Kontron Canada
+ *
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <sebastien.bouchard@ca.kontron.com>
+ *
+ */
+ 
+/* Ioctl definitions  */
+
+/* Use 0xA1 as magic number */
+#define TLCLK_IOC_MAGIC 0xA1
+
+/*Hardware Reset of the PLL */
+
+#define RESET_ON 0x00
+#define RESET_OFF 0x01
+#define IOCTL_RESET _IO(TLCLK_IOC_MAGIC,  1)
+
+#define IOCTL_REFALIGN _IO(TLCLK_IOC_MAGIC,  2)
+
+/* MODE SELECT */
+
+#define NORMAL_MODE 0x00
+#define HOLDOVER_MODE 0x10
+#define FREERUN_MODE 0x20
+
+#define IOCTL_MODE_SELECT _IOR(TLCLK_IOC_MAGIC,  3, char)
+
+/* FILTER SELECT */
+
+#define FILTER_6HZ 0x04
+#define FILTER_12HZ 0x00
+
+#define IOCTL_FILTER_SELECT _IOR(TLCLK_IOC_MAGIC,  4, char)
+
+/* SELECT REFERENCE FREQUENCY */
+
+#define REF_CLK1_8kHz 0x00
+#define REF_CLK2_19_44MHz 0x02
+
+#define IOCTL_SELECT_REF_FREQUENCY _IOR(TLCLK_IOC_MAGIC,  6, char)
+
+/* Select primary or secondary redundant clock */
+
+#define PRIMARY_CLOCK 0x00
+#define SECONDARY_CLOCK 0x01
+#define IOCTL_SELECT_REDUNDANT_CLOCK _IOR(TLCLK_IOC_MAGIC,  7, char)
+
+/* CLOCK TRANSMISSION DEFINE */
+
+#define CLK_8kHz 0xff
+#define CLK_16_384MHz 0xfb
+
+#define CLK_1_544MHz 0x00
+#define CLK_2_048MHz 0x01
+#define CLK_4_096MHz 0x02
+#define CLK_6_312MHz 0x03
+#define CLK_8_192MHz 0x04
+#define CLK_19_440MHz 0x06
+
+#define CLK_8_592MHz 0x08
+#define CLK_11_184MHz 0x09
+#define CLK_34_368MHz 0x0b
+#define CLK_44_736MHz 0x0a
+
+#define IOCTL_SELECT_AMCB1_TRANSMIT_CLOCK _IOR(TLCLK_IOC_MAGIC,  9, char)
+#define IOCTL_SELECT_AMCB2_TRANSMIT_CLOCK _IOR(TLCLK_IOC_MAGIC,  10, char)
+
+/* RECEIVED REFERENCE */
+
+#define AMC_B1 0
+#define AMC_B2 1
+
+#define IOCTL_SELECT_RECEIVED_REF_CLK3A _IOR(TLCLK_IOC_MAGIC,  11, char)
+#define IOCTL_SELECT_RECEIVED_REF_CLK3B _IOR(TLCLK_IOC_MAGIC,  12, char)
+
+/* OEM COMMAND - NOT IN FINAL VERSION */
+
+#define IOCTL_TEST_MODE _IO(TLCLK_IOC_MAGIC,  13)
+
+/* HARDWARE SWITCHING DEFINE */
+
+#define HW_ENABLE 0x80
+#define HW_DISABLE 0x00
+
+#define IOCTL_HARDWARE_SWITCHING _IOR(TLCLK_IOC_MAGIC,  14, char)
+
+/* HARDWARE SWITCHING MODE DEFINE */
+
+#define PLL_HOLDOVER 0x40
+#define LOST_CLOCK 0x00
+
+#define IOCTL_HARDWARE_SWITCHING_MODE _IOR(TLCLK_IOC_MAGIC,  15, char)
+
+/* CLOCK OUTPUT DEFINE */
+
+#define IOCTL_ENABLE_CLKA0_OUTPUT _IOR(TLCLK_IOC_MAGIC,  16, char)
+#define IOCTL_ENABLE_CLKB0_OUTPUT _IOR(TLCLK_IOC_MAGIC,  17, char)
+#define IOCTL_ENABLE_CLKA1_OUTPUT _IOR(TLCLK_IOC_MAGIC,  18, char)
+#define IOCTL_ENABLE_CLKB1_OUTPUT _IOR(TLCLK_IOC_MAGIC,  19, char)
+
+#define IOCTL_ENABLE_CLK3A_OUTPUT _IOR(TLCLK_IOC_MAGIC,  20, char)
+#define IOCTL_ENABLE_CLK3B_OUTPUT _IOR(TLCLK_IOC_MAGIC,  21, char)
+
+/* ALARMS DEFINE */
+
+#define UNLOCK_MASK 0x10
+#define HOLDOVER_MASK 0x20
+#define SEC_LOST_MASK 0x40
+#define PRI_LOST_MASK 0x80
+
+#define IOCTL_READ_ALARMS _IO(TLCLK_IOC_MAGIC,  22)
+
+/* INTERRUPT CAUSE DEFINE */
+
+#define PRI_LOS_01_MASK 0x01
+#define PRI_LOS_10_MASK 0x02
+
+#define SEC_LOS_01_MASK 0x04
+#define SEC_LOS_10_MASK 0x08
+
+#define HOLDOVER_01_MASK 0x10
+#define HOLDOVER_10_MASK 0x20
+
+#define UNLOCK_01_MASK 0x40
+#define UNLOCK_10_MASK 0x80
+
+#define IOCTL_READ_INTERRUPT_SWITCH _IO(TLCLK_IOC_MAGIC,  23)
+
+#define IOCTL_READ_CURRENT_REF _IO(TLCLK_IOC_MAGIC,  25)
+
+/* MAX NUMBER OF IOCTL */
+#define TLCLK_IOC_MAXNR 25
+
+struct tlclk_alarms {
+	unsigned int lost_clocks;
+	unsigned int lost_primary_clock;
+	unsigned int lost_secondary_clock;
+	unsigned int primary_clock_back;
+	unsigned int secondary_clock_back;
+	unsigned int switchover_primary;
+	unsigned int switchover_secondary;
+	unsigned int pll_holdover;
+	unsigned int pll_end_holdover;
+	unsigned int pll_lost_sync;
+	unsigned int pll_sync;
+};
+

Re: Patch of a new driver for kernel 2.4.x that need review

[Pekka J Enberg]

Mark Gross writes:
> diff -urN -X dontdiff_osdl vanilla/linux-2.4.31/drivers/char/tlclk.c linux-2.4/drivers/char/tlclk.c
> --- vanilla/linux-2.4.31/drivers/char/tlclk.c	1969-12-31 16:00:00.000000000 -0800
> +++ linux-2.4/drivers/char/tlclk.c	2005-07-06 13:21:24.000000000 -0700
> @@ -0,0 +1,459 @@
> +
> +/* Telecom clock I/O register definition */
> +#define TLCLK_BASE 0xa08            
> +#define TLCLK_REG0 TLCLK_BASE
> +#define TLCLK_REG1 (TLCLK_BASE+1)
> +#define TLCLK_REG2 (TLCLK_BASE+2)
> +#define TLCLK_REG3 (TLCLK_BASE+3)
> +#define TLCLK_REG4 (TLCLK_BASE+4)
> +#define TLCLK_REG5 (TLCLK_BASE+5)
> +#define TLCLK_REG6 (TLCLK_BASE+6)
> +#define TLCLK_REG7 (TLCLK_BASE+7)

Enums are preferred. 

> +
> +#define SET_PORT_BITS(port, mask, val) outb(((inb(port) & mask) | val), port)

Static inline functions are preferred. 

> +
> +/* 0 = Dynamic allocation of the major device number */
> +#define TLCLK_MAJOR 252

Enums, please. 

> +spinlock_t event_lock = SPIN_LOCK_UNLOCKED;

For 2.6, DEFINE_SPINLOCK is preferred. 

> +
> +/* DEVFS support or not */
> +#ifdef CONFIG_DEVFS_FS
> +devfs_handle_t devfs_handle;
> +#else
> +static int tlclk_major = TLCLK_MAJOR;
> +#endif
> +
> +static void switchover_timeout(unsigned long data);
> +void tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs);

Please try to avoid forward declarations as much as possible. The function 
definitions can be moved further up. 

> +
> +DECLARE_WAIT_QUEUE_HEAD(wq);
> +/*
> +*  Function : Module I/O functions
> +*  Description : Almost all the control stuff is done here, check I/O dn for
> +*  help.
> +*/

Use kerneldoc format instead of inventing your own. (Appears in other places 
as well.) 

> +
> +/*
> +*  Function : Module Opening
> +*  Description : Called when a program open the 
> +*  /dev/telclock file related to the module.
> +*/
> +static int
> +tlclk_open(struct inode *inode, struct file *filp)
> +{
> +	int result;
> +#ifdef MODULE
> +	if (!MOD_IN_USE) {
> +		MOD_INC_USE_COUNT;
> +#endif
> +		/* Make sure there is no interrupt pending will 
> +		   *  initialising interrupt handler */
> +		inb(TLCLK_REG6);
> +
> +		result = request_irq(telclk_interrupt, &tlclk_interrupt,
> +					SA_SHIRQ, "telclock", tlclk_interrupt);
> +		printk("\ntelclock: Reserving IRQ%d...\n", telclk_interrupt);
> +		if (result == -EBUSY) {

request_irq() can return other error codes too. It returns zero when it 
succeeds so check only for that. 

> +			printk(KERN_ERR
> +				"telclock: Interrupt can't be reserved!\n");
> +			return -EBUSY;

It's better to propagate the error code here. 

> +/*
> +*  Function : Interrupt Handler 
> +*  Description :
> +*  Handling of alarms interrupt.
> +*  
> +*/
> +void
> +tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs)
> +{

Why isn't this static? 

> diff -urN -X dontdiff_osdl vanilla/linux-2.4.31/drivers/char/tlclk.h linux-2.4/drivers/char/tlclk.h
> --- vanilla/linux-2.4.31/drivers/char/tlclk.h	1969-12-31 16:00:00.000000000 -0800
> +++ linux-2.4/drivers/char/tlclk.h	2005-07-06 12:30:28.000000000 -0700
> @@ -0,0 +1,167 @@
> +/* Ioctl definitions  */
> +
> +/* Use 0xA1 as magic number */
> +#define TLCLK_IOC_MAGIC 0xA1
> +
> +/*Hardware Reset of the PLL */
> +
> +#define RESET_ON 0x00
> +#define RESET_OFF 0x01
> +#define IOCTL_RESET _IO(TLCLK_IOC_MAGIC,  1)
> +
> +#define IOCTL_REFALIGN _IO(TLCLK_IOC_MAGIC,  2)
> +
> +/* MODE SELECT */
> +
> +#define NORMAL_MODE 0x00
> +#define HOLDOVER_MODE 0x10
> +#define FREERUN_MODE 0x20
> +

[snip, snip] 

Enums, please. 

Re: Patch of a new driver for kernel 2.4.x that need review

[Dmitry Torokhov]

Pekka J Enberg <penberg@cs.helsinki.fi> wrote:
> Mark Gross writes:
> 
> > +
> > +/* 0 = Dynamic allocation of the major device number */
> > +#define TLCLK_MAJOR 252
> 
> Enums, please. 
> 

But not here - it is a single constant, not a value of a distinct type.

-- 
Dmitry

Re: Patch of a new driver for kernel 2.4.x that need review

[Pekka J Enberg]

Hi Dmitry, 

Mark Gross writes:
> > > +
> > > +/* 0 = Dynamic allocation of the major device number */
> > > +#define TLCLK_MAJOR 252

Pekka J Enberg <penberg@cs.helsinki.fi> wrote:
> > Enums, please. 
> > 

Dmitry Torokhov writes:
> But not here - it is a single constant, not a value of a distinct type.

Fair enough, "static const int" would work here too. Defines should be 
avoided because they allow you to override a value without ever noticing it. 

            Pekka 

Re: Patch of a new driver for kernel 2.4.x that need review

[Dmitry Torokhov]

Hi Pekka,

Pekka J Enberg <penberg@cs.helsinki.fi> wrote:
> Hi Dmitry, 
> 
> Mark Gross writes:
> > > > +
> > > > +/* 0 = Dynamic allocation of the major device number */
> > > > +#define TLCLK_MAJOR 252
> 
> Pekka J Enberg <penberg@cs.helsinki.fi> wrote:
> > > Enums, please. 
> > > 
> 
> Dmitry Torokhov writes:
> > But not here - it is a single constant, not a value of a distinct type.
> 
> Fair enough, "static const int" would work here too. Defines should be 
> avoided because they allow you to override a value without ever noticing it.
> 

Would not this cause compiler to allocate memory for the constant?
I suppose if GCC is really good it could eliminate allocation since
nothing takes its address, but I am not sure.

With #define you can be sure that it is just a constant expression.

-- 
Dmitry

Re: Patch of a new driver for kernel 2.4.x that need review

[Pekka J Enberg]

Hi Dmitry, 

Pekka J Enberg <penberg@cs.helsinki.fi> wrote:
> > Fair enough, "static const int" would work here too. Defines should be 
> > avoided because they allow you to override a value without ever noticing it.

Dmitry Torokhov writes:
> Would not this cause compiler to allocate memory for the constant?
> I suppose if GCC is really good it could eliminate allocation since
> nothing takes its address, but I am not sure. 
> 
> With #define you can be sure that it is just a constant expression.

Constant propagation pass will eliminate the allocation. I checked GCC 3.3.5 
and does this with -O1. 

                Pekka 

Re: Patch of a new driver for kernel 2.4.x that need review

[Pekka J Enberg]

Mark Gross writes:
> +static int
> +tlclk_open(struct inode *inode, struct file *filp)
> +{
> +	int result;
> +#ifdef MODULE
> +	if (!MOD_IN_USE) {
> +		MOD_INC_USE_COUNT;
> +#endif
> +		/* Make sure there is no interrupt pending will 
> +		   *  initialising interrupt handler */
> +		inb(TLCLK_REG6);
> +
> +		result = request_irq(telclk_interrupt, &tlclk_interrupt,
> +					SA_SHIRQ, "telclock", tlclk_interrupt);

Instead of playing the MOD_IN_USE games, please either (1) grab the irq in 
module init (it's a shared IRQ after all) or (2) do the following: 

static int tlclk_used; 

static int tlclk_open(struct inode *inode, struct file *filp) {
       if (tlclk_used++)
               return 0; 

       // request_irq goes here.
} 

For an example, see the file drivers/input/mouse/amimouse.c (appears in 
2.6.12 at least). 

                        Pekka 

Re: Patch of a new driver for kernel 2.4.x that need review

[Pekka Enberg]

On 6/22/05, Bouchard, Sebastien <Sebastien.Bouchard@ca.kontron.com> wrote:
> +static int __init
> +tlclk_init(void)
> +{

[snip]

> +       if (check_region(TLCLK_BASE, 8)) {
> +               printk(KERN_ERR
> +                      "telclock: I/O region already used by another
> driver!\n");
> +               return -EBUSY;
> +       } else {
> +               request_region(TLCLK_BASE, 8, "telclock");

request_region can fail too so you'd better handle that.

                               Pekka

Re: Patch of a new driver for kernel 2.4.x that need review

[Alan Cox]

> > +       if (check_region(TLCLK_BASE, 8)) {
> > +               printk(KERN_ERR
> > +                      "telclock: I/O region already used by another
> > driver!\n");
> > +               return -EBUSY;
> > +       } else {
> > +               request_region(TLCLK_BASE, 8, "telclock");
> 
> request_region can fail too so you'd better handle that.

check_region() is essentially historical. It used to be that
check_region could fail and request_region did not. Once modules arrived
it becomes possible for drivers to race each other for resources so
request_region now atomically (with respect to itself) checks and if
available allocates a region.

check_region() shouldn't be used except for special cases where you
really want to see a space is free without requesting it. Even then if
you need to do any operation on the space while it is free you need to
request/act/release the region nto check_region for it.

Alan

Re: Patch of a new driver for kernel 2.4.x that need review

[Jesper Juhl]

On Wed, 22 Jun 2005, Bouchard, Sebastien wrote:

> Hi,
> 
> Here is a driver (only for 2.4.x) I've done to provide support of a hardware
> module (available on some ATCA board) used with telecom expension card on
> the new ATCA platform. This module provide redundant reference clock for
> telecom hardware with alarm handling when there is a new event (ex.: one of
> the ref clock is gone).
> This char driver provide IOCTL for configuration of this module and
> interrupt handler for processing alarm events.
> 
> I send this driver so people in this mailing list can do a review of the
> code.
> 

A few small comments below.


> Please reply me directly to my email, i'm not subscribed to the mailing
> list.
> 
> Thanks
> Sebastien Bouchard 
> Software designer
> Kontron Canada Inc. 
> <mailto:sebastien.bouchard@ca.kontron.com> 
> <http://www.kontron.com/> 
> 
> 
> diff -ruN 2.4.31-ori/drivers/char/Config.in
> 2.4.31-mod/drivers/char/Config.in
> --- 2.4.31-ori/drivers/char/Config.in	Sat Aug  7 19:26:04 2004
> +++ 2.4.31-mod/drivers/char/Config.in	Wed Jun 22 09:34:57 2005
> @@ -401,4 +401,6 @@
>     dep_tristate 'HP OB600 C/CT Pop-up mouse support' CONFIG_OBMOUSE
> $CONFIG_INPUT_MOUSEDEV
>  fi
>  
> +tristate 'Telecom clock support' CONFIG_TELCLOCK
> +
>  endmenu
> diff -ruN 2.4.31-ori/drivers/char/Makefile 2.4.31-mod/drivers/char/Makefile
> --- 2.4.31-ori/drivers/char/Makefile	Sat Aug  7 19:26:04 2004
> +++ 2.4.31-mod/drivers/char/Makefile	Wed Jun 22 09:36:14 2005
> @@ -323,6 +323,7 @@
>  obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
>  obj-$(CONFIG_INDYDOG) += indydog.o
>  obj-$(CONFIG_8xx_WDT) += mpc8xx_wdt.o
> +obj-$(CONFIG_TELCLOCK) += tlclk.o
>  
>  subdir-$(CONFIG_MWAVE) += mwave
>  ifeq ($(CONFIG_MWAVE),y)
> diff -ruN 2.4.31-ori/drivers/char/tlclk.c 2.4.31-mod/drivers/char/tlclk.c
> --- 2.4.31-ori/drivers/char/tlclk.c	Wed Dec 31 19:00:00 1969
> +++ 2.4.31-mod/drivers/char/tlclk.c	Wed Jun 22 09:43:10 2005
> @@ -0,0 +1,470 @@
> +/*
> + * Telecom Clock driver for Wainwright board
> + *
> + * Copyright (C) 2005 Kontron Canada
> + *
> + * All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or (at
> + * your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT.  See the GNU General Public License for more
> + * details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + *
> + * Send feedback to <sebastien.bouchard@ca.kontron.com>
> + *
> + * Description : This is the TELECOM CLOCK module driver for the ATCA
> platform.
> + */
> +#ifndef __KERNEL__
> +#  define __KERNEL__
> +#endif
> +
> +#include <linux/config.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/sched.h>
> +#include <linux/kernel.h>	/* printk() */
> +#include <linux/fs.h>		/* everything... */
> +#include <linux/errno.h>	/* error codes */
> +#include <linux/delay.h>	/* udelay */
> +#include <asm/uaccess.h>
> +#include <linux/slab.h>
> +#include <linux/ioport.h>
> +#include <linux/devfs_fs_kernel.h>	/* Devfs support */

In 2.6 devfs is in the process of being removed.. It'll probably stay in 
2.4, but long term (if/when you move the driver to 2.6) it's probably not 
worth bothering with.


> +#include <linux/interrupt.h>
> +#include <linux/spinlock.h>
> +#include <linux/timer.h>
> +#include <linux/tqueue.h>	/* interrupt task */
> +#include <asm/io.h>		/* inb/outb */
> +
> +#include "tlclk.h"	/* TELECOM IOCTL DEFINE */
> +
> +MODULE_AUTHOR("Sebastien Bouchard <sebastien.bouchard@ca.kontron.com>");
> +MODULE_LICENSE("GPL");
> +
> +/* Telecom clock I/O register definition */
> +#define TLCLK_BASE 0xa08            
> +#define TLCLK_REG0 TLCLK_BASE
> +#define TLCLK_REG1 TLCLK_BASE+1
> +#define TLCLK_REG2 TLCLK_BASE+2
> +#define TLCLK_REG3 TLCLK_BASE+3
> +#define TLCLK_REG4 TLCLK_BASE+4
> +#define TLCLK_REG5 TLCLK_BASE+5
> +#define TLCLK_REG6 TLCLK_BASE+6
> +#define TLCLK_REG7 TLCLK_BASE+7
> +
> +#define SET_PORT_BITS(port, mask, val) outb(((inb(port) & mask) | val),
> port)
> +
> +/* 0 = Dynamic allocation of the major device number */
> +#define TLCLK_MAJOR 252
> +
> +static int telclk_interrupt;	 /* Contain the interrupt used for telecom
> clock */
> +
> +static int int_events;		/* Event that generate a interrupt */
> +static int got_event;		/* if events processing have been done */
> +
> +static struct timer_list switchover_timer;
> +
> +struct tlclk_alarms *alarm_events;
> +
> +spinlock_t event_lock = SPIN_LOCK_UNLOCKED;
> +
> +/* DEVFS support or not */
> +#ifdef CONFIG_DEVFS_FS
> +devfs_handle_t devfs_handle;
> +#else
> +static int tlclk_major = TLCLK_MAJOR;
> +#endif
> +
> +static void switchover_timeout(unsigned long data);
> +void tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs);
> +
> +DECLARE_WAIT_QUEUE_HEAD(wq);
> +/*
> +*  Function : Module I/O functions
> +*  Description : Almost all the control stuff is done here, check I/O dn
> for help.
> +*/
> +static int
> +tlclk_ioctl(struct inode *inode,
> +	    struct file *filp, unsigned int cmd, unsigned long arg)
> +{
> +   unsigned long flags;

Indent this variable properly with a tab.

> +	unsigned char val;
> +	val = (char) arg;

"val" is explicitly "unsigned char", yet your cast only casts to plain 
"char" which may or may not be signed depending on platform. Cast it to 
unsigned char if that's what it needs to be, or declare a plain char 
variable and avoid the cast if both signed and unsigned char will do 
equally well.


> +
> +	if (_IOC_TYPE(cmd) != TLCLK_IOC_MAGIC)
> +		return -ENOTTY;
> +
> +	if (_IOC_NR(cmd) > TLCLK_IOC_MAXNR)
> +		return -ENOTTY;
> +

I haven't checked the complexity of _IOC_TYPE(), but if it's more than 
really trivial you might bennefit from assigning the result to a variable 
just once and then test that variable twice, on the other hand, if it's 
simple the above is just fine.


> +	switch (cmd) {
> +	case IOCTL_RESET:
> +		SET_PORT_BITS(TLCLK_REG4, 0xfd, val);
> +		break;
> +
> +	case IOCTL_MODE_SELECT:
> +		SET_PORT_BITS(TLCLK_REG0, 0xcf, val);
> +		break;
> +
> +	case IOCTL_REFALIGN:
> +		/* GENERATING 0 to 1 transistion */
> +		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
> +		udelay(2);
> +		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0x08);
> +		udelay(2);
> +		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
> +		break;
> +
> +	case IOCTL_HARDWARE_SWITCHING:
> +		SET_PORT_BITS(TLCLK_REG0, 0x7f, val);
> +		break;
> +
> +	case IOCTL_HARDWARE_SWITCHING_MODE:
> +		SET_PORT_BITS(TLCLK_REG0, 0xbf, val);
> +		break;
> +
> +	case IOCTL_FILTER_SELECT:
> +		SET_PORT_BITS(TLCLK_REG0, 0xfb, val);
> +		break;
> +
> +	case IOCTL_SELECT_REF_FREQUENCY:
> +	   spin_lock_irqsave(&event_lock, flags);

Why is this line not indented as far as the others? Don't mix tabs and few 
spaces, just indent by two tabs here.

> +		SET_PORT_BITS(TLCLK_REG1, 0xfd, val);
> +		spin_unlock_irqrestore(&event_lock, flags);
> +		break;
> +
> +	case IOCTL_SELECT_REDUNDANT_CLOCK:
> +	   spin_lock_irqsave(&event_lock, flags);

Indentation...

> +		SET_PORT_BITS(TLCLK_REG1, 0xfe, val);
> +		spin_unlock_irqrestore(&event_lock, flags);
> +		
> +		break;

Why the extra blank line before  break;  ???

> +
> +	case IOCTL_SELECT_AMCB1_TRANSMIT_CLOCK:
> +		if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
> +			SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x5);
> +			SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
> +		} else if (val >= CLK_8_592MHz) {
> +			SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x7);
> +			switch (val) {
> +			case CLK_8_592MHz:
> +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
> +				break;
> +			case CLK_11_184MHz:
> +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
> +				break;
> +			case CLK_34_368MHz:
> +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
> +				break;
> +			case CLK_44_736MHz:
> +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
> +				break;
> +			}
> +		} else
> +			SET_PORT_BITS(TLCLK_REG3, 0xf8, val);
> +		break;
> +
> +		break;

One break will do.

> +	case IOCTL_SELECT_AMCB2_TRANSMIT_CLOCK:
> +		if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
> +			SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x28);
> +			SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
> +		} else if (val >= CLK_8_592MHz) {
> +			SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x38);
> +			switch (val) {
> +			case CLK_8_592MHz:
> +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
> +				break;
> +			case CLK_11_184MHz:
> +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
> +				break;
> +			case CLK_34_368MHz:
> +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
> +				break;
> +			case CLK_44_736MHz:
> +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
> +				break;
> +			}
> +		} else
> +			SET_PORT_BITS(TLCLK_REG3, 0xc7, val << 3);
> +		break;
> +		break;

You really do like break it seems. A bit too much ;-) 

> +	case IOCTL_TEST_MODE:
> +		SET_PORT_BITS(TLCLK_REG4, 0xfd, 2);
> +		break;
> +

Why a blank line here before the next case when you have no breaks between 
case's below?  Be consistent - makes code easier on the eye to read when 
it's consistent.

> +	case IOCTL_ENABLE_CLKA0_OUTPUT:
> +		SET_PORT_BITS(TLCLK_REG2, 0xfe, val);
> +		break;
> +	case IOCTL_ENABLE_CLKB0_OUTPUT:
> +		SET_PORT_BITS(TLCLK_REG2, 0xfd, val << 1);
> +		break;
> +	case IOCTL_ENABLE_CLKA1_OUTPUT:
> +		SET_PORT_BITS(TLCLK_REG2, 0xfb, val << 2);
> +		break;
> +	case IOCTL_ENABLE_CLKB1_OUTPUT:
> +		SET_PORT_BITS(TLCLK_REG2, 0xf7, val << 3);
> +		break;
> +	case IOCTL_ENABLE_CLK3A_OUTPUT:
> +		SET_PORT_BITS(TLCLK_REG3, 0xbf, val << 6);
> +		break;
> +	case IOCTL_ENABLE_CLK3B_OUTPUT:
> +		SET_PORT_BITS(TLCLK_REG3, 0x7f, val << 7);
> +		break;
> +	case IOCTL_READ_ALARMS:
> +		return (inb(TLCLK_REG2) & 0xf0);
> +		break;
> +	case IOCTL_READ_INTERRUPT_SWITCH:
> +		return inb(TLCLK_REG6);
> +		break;
> +	case IOCTL_READ_CURRENT_REF:
> +		return ((inb(TLCLK_REG1) & 0x08) >> 3);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> +*  Function : Module Opening
> +*  Description : Called when a program open the 
> +*  /dev/telclock file related to the module.
> +*/
> +static int
> +tlclk_open(struct inode *inode, struct file *filp)
> +{
> +	int result;
> +#ifdef MODULE
> +	if (!MOD_IN_USE) {
> +		MOD_INC_USE_COUNT;
> +#endif
> +		/* Make sure there is no interrupt pending will 
> +		   *  initialising interrupt handler */
> +		inb(TLCLK_REG6);
> +
> +		result = request_irq(telclk_interrupt, &tlclk_interrupt,
> +				     SA_SHIRQ, "telclock", tlclk_interrupt);
> +		printk("\ntelclock: Reserving IRQ%d...\n",
> telclk_interrupt);
> +		if (result == -EBUSY) {
> +			printk(KERN_ERR
> +			       "telclock: Interrupt can't be reserved!\n");
> +			return -EBUSY;
> +		}
> +		inb(TLCLK_REG6);	/* Clear interrupt events */
> +#ifdef MODULE
> +	} else
> +		return -EBUSY;
> +#endif
> +	return 0;
> +}
> +
> +/*
> +*  Function : Module Releasing
> +*  Description : Called when a program stop using the module.
> +*/
> +static int
> +tlclk_release(struct inode *inode, struct file *filp)
> +{
> +	MOD_DEC_USE_COUNT;
> +	if (!MOD_IN_USE)
> +		free_irq(telclk_interrupt, tlclk_interrupt);
> +	return 0;
> +}
> +
> +ssize_t
> +tlclk_read(struct file * filp, char *buf, size_t count, loff_t * f_pos)
> +{
> +	int count0 = sizeof (struct tlclk_alarms);
                          ^^^
                           no space here...

> +	wait_event_interruptible(wq, got_event);
> +	if (copy_to_user
> +	    (buf, (char *) alarm_events, sizeof (struct tlclk_alarms)))
                         ^^^                  ^^^
                          no space here.       nor here..

Besides, the cast is superfluous, copy_to_user takes a void* as its second 
argument, so just get rid of the cast.


> +		return -EFAULT;
> +   
> +   memset(alarm_events, 0, sizeof (struct tlclk_alarms));

The space after sizeof should probably die... more elsewhere in the code 
that I'm not pointing out explicitly.


> +	got_event = 0;
> +	
> +	return count0;
> +}
> +
> +ssize_t
> +tlclk_write(struct file * filp, const char *buf, size_t count, loff_t *
> f_pos)
> +{
> +	return 0;
> +}
> +
> +/*
> +* This is where you set what function is called when an action is done to
> your  * /dev file.
> +*/
> +static struct file_operations tlclk_fops = {
> +	read:tlclk_read,
> +	write:tlclk_write,
> +	ioctl:tlclk_ioctl,
> +	open:tlclk_open,
> +	release:tlclk_release,
> +
> +};

C99 initializers please.


> +/*
> +* Function : Module Initialisation                      
> +* Description : Called at module loading, 
> +* all the OS registering stuff is her
> +*/
> +static int __init
> +tlclk_init(void)
> +{
> +	alarm_events = kmalloc(sizeof (struct tlclk_alarms), GFP_KERNEL);
> +	
> +	if(!alarm_events)

Space after "if" like so:   if (!alarm_events) 


> +		   return -EBUSY;
> +   
> +   memset(alarm_events, 0, sizeof (struct tlclk_alarms));
> +
> +/* Read telecom clock IRQ number (Set by BIOS) */
> +
> +	telclk_interrupt = (inb(TLCLK_REG7) & 0x0f);
> +
> +	printk(KERN_WARNING "telclock: Reserving I/O region...\n");
> +
> +	if (check_region(TLCLK_BASE, 8)) {

I don't recall how check_region/request_region worked in 2.4, but in 2.6 
you just need to call request_region directly, check_region is no longer 
used. That may not be true for 2.4, so I may be completely off the mark 
here...

> +		printk(KERN_ERR
> +		       "telclock: I/O region already used by another
> driver!\n");
> +		return -EBUSY;
> +	} else {
> +		request_region(TLCLK_BASE, 8, "telclock");
> +	}
> +
> +/* DEVFS or NOT? */
> +
> +#ifdef CONFIG_DEVFS_FS
> +	devfs_handle = devfs_register(NULL, "telclock",
> +				      DEVFS_FL_AUTO_DEVNUM, TLCLK_MAJOR,
> +				      0,
> +				      S_IFCHR | S_IRUGO | S_IWUGO,
> +				      &tlclk_fops, NULL);
> +	if (!devfs_handle)
> +		return -EBUSY;
> +#else
> +	tlclk_major = register_chrdev(tlclk_major, "telclock", &tlclk_fops);
> +
> +	if (tlclk_major < 0) {
> +		printk(KERN_ERR "telclock: can't get major! %d\n",
> tlclk_major);
> +		return tlclk_major;
> +	}
> +#endif
> +
> +	init_timer(&switchover_timer);
> +	switchover_timer.function = switchover_timeout;
> +	switchover_timer.data = 0;
> +
> +	return 0;
> +}
> +
> +/*
> +*  Function : Module Cleaning
> 
> +*  Description : Called when unloading the module
> +*/
> +static void __exit
> +tlclk_cleanup(void)
> +{
> +#ifdef CONFIG_DEVFS_FS
> +	devfs_unregister(devfs_handle);
> +#else
> +	unregister_chrdev(tlclk_major, "telclock");
> +#endif
> +	release_region(TLCLK_BASE, 8);
> +	del_timer_sync(&switchover_timer);
> +	kfree(alarm_events);
> +	
> +}
> +static void
> +switchover_timeout(unsigned long data)
> +{
> +	if ((data & 1)) {
> +		if ((inb(TLCLK_REG1) & 0x08) != (data & 0x08))
> +			alarm_events->switchover_primary++;
> +	} else {
> +		if ((inb(TLCLK_REG1) & 0x08) != (data & 0x08))
> +			alarm_events->switchover_secondary++;
> +	}
> +
> +	/* Alarm processing is done, wake up read task */
> +	del_timer(&switchover_timer);
> +	got_event = 1;
> +	wake_up(&wq);
> +}
> +/*
> +*  Function : Interrupt Handler
> 
> +*  Description :
> +*  Handling of alarms interrupt.
> +*  
> +*/
> +void
> +tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs)
> +{
> +	unsigned long flags;
> +	spin_lock_irqsave(&event_lock, flags);
> +	int_events = inb(TLCLK_REG6);	            /* Read and clear interrupt events */

Please make lines stay within 80 columns... There are other long lines, 
I'm just pointing out this one and trust you can locate the rest yourself.


> +	spin_unlock_irqrestore(&event_lock, flags);
> +	
> +	/* Primary_Los changed from 0 to 1 ? */
> +	if (int_events & PRI_LOS_01_MASK) {
> +		if (inb(TLCLK_REG2) & SEC_LOST_MASK)
> +			alarm_events->lost_clocks++;
> +		else
> +			alarm_events->lost_primary_clock++;
> +	}
> +
> +	/* Primary_Los changed from 1 to 0 ? */
> +	if (int_events & PRI_LOS_10_MASK) {
> +		alarm_events->primary_clock_back++;
> +		spin_lock_irqsave(&event_lock, flags);
> +		SET_PORT_BITS(TLCLK_REG1, 0xFE, 1);
> +		spin_unlock_irqrestore(&event_lock, flags);
> +	}
> +	/* Secondary_Los changed from 0 to 1 ? */
> +	if (int_events & SEC_LOS_01_MASK) {
> +		if (inb(TLCLK_REG2) & PRI_LOST_MASK)
> +			alarm_events->lost_clocks++;
> +		else
> +			alarm_events->lost_secondary_clock++;
> +	}
> +	/* Secondary_Los changed from 1 to 0 ? */
> +	if (int_events & SEC_LOS_10_MASK) {
> +		alarm_events->secondary_clock_back++;
> +		spin_lock_irqsave(&event_lock, flags);
> +		SET_PORT_BITS(TLCLK_REG1, 0xFE, 0);
> +		spin_unlock_irqrestore(&event_lock, flags);
> +	}
> +	if (int_events & HOLDOVER_10_MASK)
> +		alarm_events->pll_end_holdover++;
> +
> +	if (int_events & UNLOCK_01_MASK)
> +		alarm_events->pll_lost_sync++;
> +
> +	if (int_events & UNLOCK_10_MASK)
> +		alarm_events->pll_sync++;
> +
> +	/* Holdover changed from 0 to 1 ? */
> +	if (int_events & HOLDOVER_01_MASK) {
> +		alarm_events->pll_holdover++;
> +
> +		switchover_timer.expires = jiffies + 1;	/* TIMEOUT in ~10ms
> */
> +		switchover_timer.data = inb(TLCLK_REG1);
> +		add_timer(&switchover_timer);
> +	} else {                                     
> +	   got_event = 1;
> +		wake_up(&wq);
> +	}
> +}
> +
> +module_init(tlclk_init);
> +module_exit(tlclk_cleanup);
> diff -ruN 2.4.31-ori/drivers/char/tlclk.h 2.4.31-mod/drivers/char/tlclk.h
> --- 2.4.31-ori/drivers/char/tlclk.h	Wed Dec 31 19:00:00 1969
> +++ 2.4.31-mod/drivers/char/tlclk.h	Wed Jun 22 09:43:10 2005
> @@ -0,0 +1,167 @@
> +/*
> + * Telecom Clock driver for Wainwright board
> + *
> + * Copyright (C) 2005 Kontron Canada
> + *
> + * All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or (at
> + * your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT.  See the GNU General Public License for more
> + * details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + *
> + * Send feedback to <sebastien.bouchard@ca.kontron.com>
> + *
> + */
> + 
> +/* Ioctl definitions  */
> +
> +/* Use 0xA1 as magic number */
> +#define TLCLK_IOC_MAGIC 0xA1
> +
> +/*Hardware Reset of the PLL */
> +
> +#define RESET_ON 0x00
> +#define RESET_OFF 0x01
> +#define IOCTL_RESET _IO(TLCLK_IOC_MAGIC,  1)
> +
> +#define IOCTL_REFALIGN _IO(TLCLK_IOC_MAGIC,  2)
> +
> +/* MODE SELECT */
> +
> +#define NORMAL_MODE 0x00
> +#define HOLDOVER_MODE 0x10
> +#define FREERUN_MODE 0x20
> +
> +#define IOCTL_MODE_SELECT _IOR(TLCLK_IOC_MAGIC,  3, char)
> +
> +/* FILTER SELECT */
> +
> +#define FILTER_6HZ 0x04
> +#define FILTER_12HZ 0x00
> +
> +#define IOCTL_FILTER_SELECT _IOR(TLCLK_IOC_MAGIC,  4, char)
> +
> +/* SELECT REFERENCE FREQUENCY */
> +
> +#define REF_CLK1_8kHz 0x00
> +#define REF_CLK2_19_44MHz 0x02
> +
> +#define IOCTL_SELECT_REF_FREQUENCY _IOR(TLCLK_IOC_MAGIC,  6, char)
> +
> +/* Select primary or secondary redundant clock */
> +
> +#define PRIMARY_CLOCK 0x00
> +#define SECONDARY_CLOCK 0x01
> +#define IOCTL_SELECT_REDUNDANT_CLOCK _IOR(TLCLK_IOC_MAGIC,  7, char)
> +
> +/* CLOCK TRANSMISSION DEFINE */
> +
> +#define CLK_8kHz 0xff
> +#define CLK_16_384MHz 0xfb
> +
> +#define CLK_1_544MHz 0x00
> +#define CLK_2_048MHz 0x01
> +#define CLK_4_096MHz 0x02
> +#define CLK_6_312MHz 0x03
> +#define CLK_8_192MHz 0x04
> +#define CLK_19_440MHz 0x06
> +
> +#define CLK_8_592MHz 0x08
> +#define CLK_11_184MHz 0x09
> +#define CLK_34_368MHz 0x0b
> +#define CLK_44_736MHz 0x0a
> +
> +#define IOCTL_SELECT_AMCB1_TRANSMIT_CLOCK _IOR(TLCLK_IOC_MAGIC,  9, char)
> +#define IOCTL_SELECT_AMCB2_TRANSMIT_CLOCK _IOR(TLCLK_IOC_MAGIC,  10, char)
> +
> +/* RECEIVED REFERENCE */
> +
> +#define AMC_B1 0
> +#define AMC_B2 1
> +
> +#define IOCTL_SELECT_RECEIVED_REF_CLK3A _IOR(TLCLK_IOC_MAGIC,  11, char)
> +#define IOCTL_SELECT_RECEIVED_REF_CLK3B _IOR(TLCLK_IOC_MAGIC,  12, char)
> +
> +/* OEM COMMAND - NOT IN FINAL VERSION */
> +
> +#define IOCTL_TEST_MODE _IO(TLCLK_IOC_MAGIC,  13)
> +
> +/* HARDWARE SWITCHING DEFINE */
> +
> +#define HW_ENABLE 0x80
> +#define HW_DISABLE 0x00
> +
> +#define IOCTL_HARDWARE_SWITCHING _IOR(TLCLK_IOC_MAGIC,  14, char)
> +
> +/* HARDWARE SWITCHING MODE DEFINE */
> +
> +#define PLL_HOLDOVER 0x40
> +#define LOST_CLOCK 0x00
> +
> +#define IOCTL_HARDWARE_SWITCHING_MODE _IOR(TLCLK_IOC_MAGIC,  15, char)
> +
> +/* CLOCK OUTPUT DEFINE */
> +
> +#define IOCTL_ENABLE_CLKA0_OUTPUT _IOR(TLCLK_IOC_MAGIC,  16, char)
> +#define IOCTL_ENABLE_CLKB0_OUTPUT _IOR(TLCLK_IOC_MAGIC,  17, char)
> +#define IOCTL_ENABLE_CLKA1_OUTPUT _IOR(TLCLK_IOC_MAGIC,  18, char)
> +#define IOCTL_ENABLE_CLKB1_OUTPUT _IOR(TLCLK_IOC_MAGIC,  19, char)
> +
> +#define IOCTL_ENABLE_CLK3A_OUTPUT _IOR(TLCLK_IOC_MAGIC,  20, char)
> +#define IOCTL_ENABLE_CLK3B_OUTPUT _IOR(TLCLK_IOC_MAGIC,  21, char)
> +
> +/* ALARMS DEFINE */
> +
> +#define UNLOCK_MASK 0x10
> +#define HOLDOVER_MASK 0x20
> +#define SEC_LOST_MASK 0x40
> +#define PRI_LOST_MASK 0x80
> +
> +#define IOCTL_READ_ALARMS _IO(TLCLK_IOC_MAGIC,  22)
> +
> +/* INTERRUPT CAUSE DEFINE */
> +
> +#define PRI_LOS_01_MASK 0x01
> +#define PRI_LOS_10_MASK 0x02
> +
> +#define SEC_LOS_01_MASK 0x04
> +#define SEC_LOS_10_MASK 0x08
> +
> +#define HOLDOVER_01_MASK 0x10
> +#define HOLDOVER_10_MASK 0x20
> +
> +#define UNLOCK_01_MASK 0x40
> +#define UNLOCK_10_MASK 0x80
> +
> +#define IOCTL_READ_INTERRUPT_SWITCH _IO(TLCLK_IOC_MAGIC,  23)
> +
> +#define IOCTL_READ_CURRENT_REF _IO(TLCLK_IOC_MAGIC,  25)
> +
> +/* MAX NUMBER OF IOCTL */
> +#define TLCLK_IOC_MAXNR 25
> +
> +struct tlclk_alarms {
> +   unsigned int lost_clocks;
> +   unsigned int lost_primary_clock;
> +   unsigned int lost_secondary_clock;
> +   unsigned int primary_clock_back;
> +   unsigned int secondary_clock_back;
> +   unsigned int switchover_primary;
> +   unsigned int switchover_secondary;
> +   unsigned int pll_holdover;
> +   unsigned int pll_end_holdover;
> +   unsigned int pll_lost_sync;
> +   unsigned int pll_sync;

Please use tabs for indentation.


> +};
> +
> Sebastien Bouchard 
> Software designer
> Kontron Canada Inc. 
> <mailto:sebastien.bouchard@ca.kontron.com> 
> <http://www.kontron.com/> 



Kind regards,

Jesper Juhl

Re: Patch of a new driver for kernel 2.4.x that need review

[Mark Gross]

On Wednesday 22 June 2005 08:12, Bouchard, Sebastien wrote:
> Hi,
>
> Here is a driver (only for 2.4.x) I've done to provide support of a
> hardware module (available on some ATCA board) used with telecom expension
> card on the new ATCA platform. This module provide redundant reference
> clock for telecom hardware with alarm handling when there is a new event
> (ex.: one of the ref clock is gone).
> This char driver provide IOCTL for configuration of this module and
> interrupt handler for processing alarm events.
>
> I send this driver so people in this mailing list can do a review of the
> code.
>
> Please reply me directly to my email, i'm not subscribed to the mailing
> list.
>
> Thanks
> Sebastien Bouchard
> Software designer
> Kontron Canada Inc.
> <mailto:sebastien.bouchard@ca.kontron.com>
> <http://www.kontron.com/>

I'm helping out a bit with the maintaining of this driver for Sebastien.  
The following is a 2.6.12 port of Sebastien's 2.4 driver.  

--mgross

diff -urN -X dontdiff_osdl vanilla/linux-2.6.12/drivers/char/Kconfig linux-2.6.12/drivers/char/Kconfig
--- vanilla/linux-2.6.12/drivers/char/Kconfig	2005-06-17 12:48:29.000000000 -0700
+++ linux-2.6.12/drivers/char/Kconfig	2005-07-06 13:25:18.585951744 -0700
@@ -998,5 +998,16 @@
 
 source "drivers/char/tpm/Kconfig"
 
+config TELCLOCK
+	tristate "Telecom clock driver for ATCA"
+	depends on EXPERIMENTAL
+	default n
+	help
+	  The telecom clock device allows direct userspace access to the
+	  configuration of the telecom clock configuration settings.
+	  This device is used for hardware synchronization across the ATCA
+	  back plane fabric.
+
+
 endmenu
 
diff -urN -X dontdiff_osdl vanilla/linux-2.6.12/drivers/char/Makefile linux-2.6.12/drivers/char/Makefile
--- vanilla/linux-2.6.12/drivers/char/Makefile	2005-06-17 12:48:29.000000000 -0700
+++ linux-2.6.12/drivers/char/Makefile	2005-07-06 13:26:15.139354320 -0700
@@ -81,6 +81,7 @@
 obj-$(CONFIG_NWFLASH) += nwflash.o
 obj-$(CONFIG_SCx200_GPIO) += scx200_gpio.o
 obj-$(CONFIG_TANBAC_TB0219) += tb0219.o
+obj-$(CONFIG_TELCLOCK) += tlclk.o
 
 obj-$(CONFIG_WATCHDOG)	+= watchdog/
 obj-$(CONFIG_MWAVE) += mwave/
diff -urN -X dontdiff_osdl vanilla/linux-2.6.12/drivers/char/tlclk.c linux-2.6.12/drivers/char/tlclk.c
--- vanilla/linux-2.6.12/drivers/char/tlclk.c	1969-12-31 16:00:00.000000000 -0800
+++ linux-2.6.12/drivers/char/tlclk.c	2005-07-06 13:25:18.600949464 -0700
@@ -0,0 +1,447 @@
+/*
+ * Telecom Clock driver for Wainwright board
+ *
+ * Copyright (C) 2005 Kontron Canada
+ *
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <sebastien.bouchard@ca.kontron.com>
+ *
+ * Description : This is the TELECOM CLOCK module driver for the ATCA platform.
+ */
+#ifndef __KERNEL__
+#  define __KERNEL__
+#endif
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/sched.h>
+#include <linux/kernel.h>	/* printk() */
+#include <linux/fs.h>		/* everything... */
+#include <linux/errno.h>	/* error codes */
+#include <linux/delay.h>	/* udelay */
+#include <asm/uaccess.h>
+#include <linux/slab.h>
+#include <linux/ioport.h>
+#include <linux/devfs_fs_kernel.h>	/* Devfs support */
+#include <linux/interrupt.h>
+#include <linux/spinlock.h>
+#include <linux/timer.h>
+#include <asm/io.h>		/* inb/outb */
+
+#include "tlclk.h"	/* TELECOM IOCTL DEFINE */
+
+MODULE_AUTHOR("Sebastien Bouchard <sebastien.bouchard@ca.kontron.com>");
+MODULE_LICENSE("GPL");
+
+/* Telecom clock I/O register definition */
+#define TLCLK_BASE 0xa08            
+#define TLCLK_REG0 TLCLK_BASE
+#define TLCLK_REG1 (TLCLK_BASE+1)
+#define TLCLK_REG2 (TLCLK_BASE+2)
+#define TLCLK_REG3 (TLCLK_BASE+3)
+#define TLCLK_REG4 (TLCLK_BASE+4)
+#define TLCLK_REG5 (TLCLK_BASE+5)
+#define TLCLK_REG6 (TLCLK_BASE+6)
+#define TLCLK_REG7 (TLCLK_BASE+7)
+
+#define SET_PORT_BITS(port, mask, val) outb(((inb(port) & mask) | val), port)
+
+/* 0 = Dynamic allocation of the major device number */
+#define TLCLK_MAJOR 252
+
+/* Contain the interrupt used for telecom clock */
+static unsigned int telclk_interrupt;
+
+static int int_events;		/* Event that generate a interrupt */
+static int got_event;		/* if events processing have been done */
+
+static struct timer_list switchover_timer;
+
+struct tlclk_alarms *alarm_events;
+
+spinlock_t event_lock = SPIN_LOCK_UNLOCKED;
+
+/* DEVFS support or not */
+#ifdef CONFIG_DEVFS_FS
+devfs_handle_t devfs_handle;
+#else
+static int tlclk_major = TLCLK_MAJOR;
+#endif
+
+static void switchover_timeout(unsigned long data);
+irqreturn_t tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs);
+
+DECLARE_WAIT_QUEUE_HEAD(wq);
+/*
+*  Function : Module I/O functions
+*  Description : Almost all the control stuff is done here, check I/O dn for
+*  help.
+*/
+static int
+tlclk_ioctl(struct inode *inode,
+	    struct file *filp, unsigned int cmd, unsigned long arg)
+{
+	unsigned long flags;
+	unsigned char val;
+	val = (unsigned char) arg;
+
+	if (_IOC_TYPE(cmd) != TLCLK_IOC_MAGIC)
+		return -ENOTTY;
+
+	if (_IOC_NR(cmd) > TLCLK_IOC_MAXNR)
+		return -ENOTTY;
+
+	switch (cmd) {
+	case IOCTL_RESET:
+		SET_PORT_BITS(TLCLK_REG4, 0xfd, val);
+		break;
+	case IOCTL_MODE_SELECT:
+		SET_PORT_BITS(TLCLK_REG0, 0xcf, val);
+		break;
+	case IOCTL_REFALIGN:
+		/* GENERATING 0 to 1 transistion */
+		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
+		udelay(2);
+		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0x08);
+		udelay(2);
+		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
+		break;
+	case IOCTL_HARDWARE_SWITCHING:
+		SET_PORT_BITS(TLCLK_REG0, 0x7f, val);
+		break;
+	case IOCTL_HARDWARE_SWITCHING_MODE:
+		SET_PORT_BITS(TLCLK_REG0, 0xbf, val);
+		break;
+	case IOCTL_FILTER_SELECT:
+		SET_PORT_BITS(TLCLK_REG0, 0xfb, val);
+		break;
+	case IOCTL_SELECT_REF_FREQUENCY:
+		spin_lock_irqsave(&event_lock, flags);
+		SET_PORT_BITS(TLCLK_REG1, 0xfd, val);
+		spin_unlock_irqrestore(&event_lock, flags);
+		break;
+	case IOCTL_SELECT_REDUNDANT_CLOCK:
+		spin_lock_irqsave(&event_lock, flags);
+		SET_PORT_BITS(TLCLK_REG1, 0xfe, val);
+		spin_unlock_irqrestore(&event_lock, flags);
+		break;
+	case IOCTL_SELECT_AMCB1_TRANSMIT_CLOCK:
+		if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
+			SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x5);
+			SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
+		} else if (val >= CLK_8_592MHz) {
+			SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x7);
+			switch (val) {
+			case CLK_8_592MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+				break;
+			case CLK_11_184MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
+				break;
+			case CLK_34_368MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
+				break;
+			case CLK_44_736MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+				break;
+			}
+		} else
+			SET_PORT_BITS(TLCLK_REG3, 0xf8, val);
+		break;
+	case IOCTL_SELECT_AMCB2_TRANSMIT_CLOCK:
+		if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
+			SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x28);
+			SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
+		} else if (val >= CLK_8_592MHz) {
+			SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x38);
+			switch (val) {
+			case CLK_8_592MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+				break;
+			case CLK_11_184MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
+				break;
+			case CLK_34_368MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
+				break;
+			case CLK_44_736MHz:
+				SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+				break;
+			}
+		} else
+			SET_PORT_BITS(TLCLK_REG3, 0xc7, val << 3);
+		break;
+	case IOCTL_TEST_MODE:
+		SET_PORT_BITS(TLCLK_REG4, 0xfd, 2);
+		break;
+	case IOCTL_ENABLE_CLKA0_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG2, 0xfe, val);
+		break;
+	case IOCTL_ENABLE_CLKB0_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG2, 0xfd, val << 1);
+		break;
+	case IOCTL_ENABLE_CLKA1_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG2, 0xfb, val << 2);
+		break;
+	case IOCTL_ENABLE_CLKB1_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG2, 0xf7, val << 3);
+		break;
+	case IOCTL_ENABLE_CLK3A_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG3, 0xbf, val << 6);
+		break;
+	case IOCTL_ENABLE_CLK3B_OUTPUT:
+		SET_PORT_BITS(TLCLK_REG3, 0x7f, val << 7);
+		break;
+	case IOCTL_READ_ALARMS:
+		return (inb(TLCLK_REG2) & 0xf0);
+		break;
+	case IOCTL_READ_INTERRUPT_SWITCH:
+		return inb(TLCLK_REG6);
+		break;
+	case IOCTL_READ_CURRENT_REF:
+		return ((inb(TLCLK_REG1) & 0x08) >> 3);
+		break;
+	}
+
+	return 0;
+}
+
+/*
+*  Function : Module Opening
+*  Description : Called when a program open the 
+*  /dev/telclock file related to the module.
+*/
+static int
+tlclk_open(struct inode *inode, struct file *filp)
+{
+	int result;
+	/* Make sure there is no interrupt pending will 
+	   *  initialising interrupt handler */
+	inb(TLCLK_REG6);
+
+	result = request_irq(telclk_interrupt, &tlclk_interrupt,
+			     SA_SHIRQ, "telclock", tlclk_interrupt);
+	printk("\ntelclock: Reserving IRQ%d...\n", telclk_interrupt);
+	if (result == -EBUSY) {
+		printk(KERN_ERR
+		       "telclock: Interrupt can't be reserved!\n");
+		return -EBUSY;
+	}
+	inb(TLCLK_REG6);	/* Clear interrupt events */
+	return 0;
+}
+
+/*
+*  Function : Module Releasing
+*  Description : Called when a program stop using the module.
+*/
+static int
+tlclk_release(struct inode *inode, struct file *filp)
+{
+		free_irq(telclk_interrupt, tlclk_interrupt);
+	return 0;
+}
+
+ssize_t
+tlclk_read(struct file * filp, char *buf, size_t count, loff_t * f_pos)
+{
+	int count0 = sizeof(struct tlclk_alarms);
+	wait_event_interruptible(wq, got_event);
+	if (copy_to_user(buf, alarm_events, sizeof(struct tlclk_alarms)))
+		return -EFAULT;
+
+	memset(alarm_events, 0, sizeof(struct tlclk_alarms));
+	got_event = 0;
+	
+	return count0;
+}
+
+ssize_t
+tlclk_write(struct file * filp, const char *buf, size_t count, loff_t * f_pos)
+{
+	return 0;
+}
+
+/*
+* This is where you set what function is called when an action is done to your
+*  /dev file.
+*/
+static struct file_operations tlclk_fops = {
+	.read = tlclk_read,
+	.write = tlclk_write,
+	.ioctl = tlclk_ioctl,
+	.open = tlclk_open,
+	.release = tlclk_release,
+
+};
+/*
+* Function : Module Initialisation                      
+* Description : Called at module loading, 
+* all the OS registering stuff is her
+*/
+static int __init
+tlclk_init(void)
+{
+/* DEVFS or NOT? */
+#ifdef CONFIG_DEVFS_FS
+	devfs_handle = devfs_register(NULL, "telclock",
+					DEVFS_FL_AUTO_DEVNUM, TLCLK_MAJOR,
+					0,
+					S_IFCHR | S_IRUGO | S_IWUGO,
+					&tlclk_fops, NULL);
+	if (!devfs_handle)
+		goto out1;
+#else
+	tlclk_major = register_chrdev(tlclk_major, "telclock", &tlclk_fops);
+
+	if (tlclk_major < 0) {
+		printk(KERN_ERR "telclock: can't get major! %d\n", tlclk_major);
+		return tlclk_major;
+	}
+#endif
+
+	alarm_events = kmalloc(sizeof(struct tlclk_alarms), GFP_KERNEL);
+	
+	if (!alarm_events)
+		goto out1;
+	
+	memset(alarm_events, 0, sizeof(struct tlclk_alarms));
+
+/* Read telecom clock IRQ number (Set by BIOS) */
+
+	printk(KERN_WARNING "telclock: Reserving I/O region...\n");
+
+	if ( !request_region(TLCLK_BASE, 8, "telclock") ) {
+		printk(KERN_ERR "telclock: request_region failed!\n");
+			goto out2;
+	}
+	telclk_interrupt = (inb(TLCLK_REG7) & 0x0f);
+
+	init_timer(&switchover_timer);
+	switchover_timer.function = switchover_timeout;
+	switchover_timer.data = 0;
+
+	return 0;
+out2:
+	kfree(alarm_events);
+out1:
+	return -EBUSY;
+}
+
+/*
+*  Function : Module Cleaning
+*  Description : Called when unloading the module
+*/
+static void __exit
+tlclk_cleanup(void)
+{
+#ifdef CONFIG_DEVFS_FS
+	devfs_unregister(devfs_handle);
+#else
+	unregister_chrdev(tlclk_major, "telclock");
+#endif
+	release_region(TLCLK_BASE, 8);
+	del_timer_sync(&switchover_timer);
+	kfree(alarm_events);
+	
+}
+static void
+switchover_timeout(unsigned long data)
+{
+	if ((data & 1)) {
+		if ((inb(TLCLK_REG1) & 0x08) != (data & 0x08))
+			alarm_events->switchover_primary++;
+	} else {
+		if ((inb(TLCLK_REG1) & 0x08) != (data & 0x08))
+			alarm_events->switchover_secondary++;
+	}
+
+	/* Alarm processing is done, wake up read task */
+	del_timer(&switchover_timer);
+	got_event = 1;
+	wake_up(&wq);
+}
+/*
+*  Function : Interrupt Handler
+*  Description :
+*/
+irqreturn_t
+tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs)
+{
+	unsigned long flags;
+	spin_lock_irqsave(&event_lock, flags);
+	/* Read and clear interrupt events */
+	int_events = inb(TLCLK_REG6);
+	spin_unlock_irqrestore(&event_lock, flags);
+	
+	/* Primary_Los changed from 0 to 1 ? */
+	if (int_events & PRI_LOS_01_MASK) {
+		if (inb(TLCLK_REG2) & SEC_LOST_MASK)
+			alarm_events->lost_clocks++;
+		else
+			alarm_events->lost_primary_clock++;
+	}
+
+	/* Primary_Los changed from 1 to 0 ? */
+	if (int_events & PRI_LOS_10_MASK) {
+		alarm_events->primary_clock_back++;
+		spin_lock_irqsave(&event_lock, flags);
+		SET_PORT_BITS(TLCLK_REG1, 0xFE, 1);
+		spin_unlock_irqrestore(&event_lock, flags);
+	}
+	/* Secondary_Los changed from 0 to 1 ? */
+	if (int_events & SEC_LOS_01_MASK) {
+		if (inb(TLCLK_REG2) & PRI_LOST_MASK)
+			alarm_events->lost_clocks++;
+		else
+			alarm_events->lost_secondary_clock++;
+	}
+	/* Secondary_Los changed from 1 to 0 ? */
+	if (int_events & SEC_LOS_10_MASK) {
+		alarm_events->secondary_clock_back++;
+		spin_lock_irqsave(&event_lock, flags);
+		SET_PORT_BITS(TLCLK_REG1, 0xFE, 0);
+		spin_unlock_irqrestore(&event_lock, flags);
+	}
+	if (int_events & HOLDOVER_10_MASK)
+		alarm_events->pll_end_holdover++;
+
+	if (int_events & UNLOCK_01_MASK)
+		alarm_events->pll_lost_sync++;
+
+	if (int_events & UNLOCK_10_MASK)
+		alarm_events->pll_sync++;
+
+	/* Holdover changed from 0 to 1 ? */
+	if (int_events & HOLDOVER_01_MASK) {
+		alarm_events->pll_holdover++;
+
+		switchover_timer.expires = jiffies + 1;	/* TIMEOUT in ~10ms */
+		switchover_timer.data = inb(TLCLK_REG1);
+		add_timer(&switchover_timer);
+	} else {
+		got_event = 1;
+		wake_up(&wq);
+	}
+	return IRQ_HANDLED;
+}
+
+module_init(tlclk_init);
+module_exit(tlclk_cleanup);
diff -urN -X dontdiff_osdl vanilla/linux-2.6.12/drivers/char/tlclk.h linux-2.6.12/drivers/char/tlclk.h
--- vanilla/linux-2.6.12/drivers/char/tlclk.h	1969-12-31 16:00:00.000000000 -0800
+++ linux-2.6.12/drivers/char/tlclk.h	2005-07-06 13:25:18.607948400 -0700
@@ -0,0 +1,167 @@
+/*
+ * Telecom Clock driver for Wainwright board
+ *
+ * Copyright (C) 2005 Kontron Canada
+ *
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <sebastien.bouchard@ca.kontron.com>
+ *
+ */
+ 
+/* Ioctl definitions  */
+
+/* Use 0xA1 as magic number */
+#define TLCLK_IOC_MAGIC 0xA1
+
+/*Hardware Reset of the PLL */
+
+#define RESET_ON 0x00
+#define RESET_OFF 0x01
+#define IOCTL_RESET _IO(TLCLK_IOC_MAGIC,  1)
+
+#define IOCTL_REFALIGN _IO(TLCLK_IOC_MAGIC,  2)
+
+/* MODE SELECT */
+
+#define NORMAL_MODE 0x00
+#define HOLDOVER_MODE 0x10
+#define FREERUN_MODE 0x20
+
+#define IOCTL_MODE_SELECT _IOR(TLCLK_IOC_MAGIC,  3, char)
+
+/* FILTER SELECT */
+
+#define FILTER_6HZ 0x04
+#define FILTER_12HZ 0x00
+
+#define IOCTL_FILTER_SELECT _IOR(TLCLK_IOC_MAGIC,  4, char)
+
+/* SELECT REFERENCE FREQUENCY */
+
+#define REF_CLK1_8kHz 0x00
+#define REF_CLK2_19_44MHz 0x02
+
+#define IOCTL_SELECT_REF_FREQUENCY _IOR(TLCLK_IOC_MAGIC,  6, char)
+
+/* Select primary or secondary redundant clock */
+
+#define PRIMARY_CLOCK 0x00
+#define SECONDARY_CLOCK 0x01
+#define IOCTL_SELECT_REDUNDANT_CLOCK _IOR(TLCLK_IOC_MAGIC,  7, char)
+
+/* CLOCK TRANSMISSION DEFINE */
+
+#define CLK_8kHz 0xff
+#define CLK_16_384MHz 0xfb
+
+#define CLK_1_544MHz 0x00
+#define CLK_2_048MHz 0x01
+#define CLK_4_096MHz 0x02
+#define CLK_6_312MHz 0x03
+#define CLK_8_192MHz 0x04
+#define CLK_19_440MHz 0x06
+
+#define CLK_8_592MHz 0x08
+#define CLK_11_184MHz 0x09
+#define CLK_34_368MHz 0x0b
+#define CLK_44_736MHz 0x0a
+
+#define IOCTL_SELECT_AMCB1_TRANSMIT_CLOCK _IOR(TLCLK_IOC_MAGIC,  9, char)
+#define IOCTL_SELECT_AMCB2_TRANSMIT_CLOCK _IOR(TLCLK_IOC_MAGIC,  10, char)
+
+/* RECEIVED REFERENCE */
+
+#define AMC_B1 0
+#define AMC_B2 1
+
+#define IOCTL_SELECT_RECEIVED_REF_CLK3A _IOR(TLCLK_IOC_MAGIC,  11, char)
+#define IOCTL_SELECT_RECEIVED_REF_CLK3B _IOR(TLCLK_IOC_MAGIC,  12, char)
+
+/* OEM COMMAND - NOT IN FINAL VERSION */
+
+#define IOCTL_TEST_MODE _IO(TLCLK_IOC_MAGIC,  13)
+
+/* HARDWARE SWITCHING DEFINE */
+
+#define HW_ENABLE 0x80
+#define HW_DISABLE 0x00
+
+#define IOCTL_HARDWARE_SWITCHING _IOR(TLCLK_IOC_MAGIC,  14, char)
+
+/* HARDWARE SWITCHING MODE DEFINE */
+
+#define PLL_HOLDOVER 0x40
+#define LOST_CLOCK 0x00
+
+#define IOCTL_HARDWARE_SWITCHING_MODE _IOR(TLCLK_IOC_MAGIC,  15, char)
+
+/* CLOCK OUTPUT DEFINE */
+
+#define IOCTL_ENABLE_CLKA0_OUTPUT _IOR(TLCLK_IOC_MAGIC,  16, char)
+#define IOCTL_ENABLE_CLKB0_OUTPUT _IOR(TLCLK_IOC_MAGIC,  17, char)
+#define IOCTL_ENABLE_CLKA1_OUTPUT _IOR(TLCLK_IOC_MAGIC,  18, char)
+#define IOCTL_ENABLE_CLKB1_OUTPUT _IOR(TLCLK_IOC_MAGIC,  19, char)
+
+#define IOCTL_ENABLE_CLK3A_OUTPUT _IOR(TLCLK_IOC_MAGIC,  20, char)
+#define IOCTL_ENABLE_CLK3B_OUTPUT _IOR(TLCLK_IOC_MAGIC,  21, char)
+
+/* ALARMS DEFINE */
+
+#define UNLOCK_MASK 0x10
+#define HOLDOVER_MASK 0x20
+#define SEC_LOST_MASK 0x40
+#define PRI_LOST_MASK 0x80
+
+#define IOCTL_READ_ALARMS _IO(TLCLK_IOC_MAGIC,  22)
+
+/* INTERRUPT CAUSE DEFINE */
+
+#define PRI_LOS_01_MASK 0x01
+#define PRI_LOS_10_MASK 0x02
+
+#define SEC_LOS_01_MASK 0x04
+#define SEC_LOS_10_MASK 0x08
+
+#define HOLDOVER_01_MASK 0x10
+#define HOLDOVER_10_MASK 0x20
+
+#define UNLOCK_01_MASK 0x40
+#define UNLOCK_10_MASK 0x80
+
+#define IOCTL_READ_INTERRUPT_SWITCH _IO(TLCLK_IOC_MAGIC,  23)
+
+#define IOCTL_READ_CURRENT_REF _IO(TLCLK_IOC_MAGIC,  25)
+
+/* MAX NUMBER OF IOCTL */
+#define TLCLK_IOC_MAXNR 25
+
+struct tlclk_alarms {
+	unsigned int lost_clocks;
+	unsigned int lost_primary_clock;
+	unsigned int lost_secondary_clock;
+	unsigned int primary_clock_back;
+	unsigned int secondary_clock_back;
+	unsigned int switchover_primary;
+	unsigned int switchover_secondary;
+	unsigned int pll_holdover;
+	unsigned int pll_end_holdover;
+	unsigned int pll_lost_sync;
+	unsigned int pll_sync;
+};
+

Re: Patch of a new driver for kernel 2.4.x that need review

[randy_dunlap]

On Wed, 6 Jul 2005 14:14:44 -0700 Mark Gross wrote:

| On Wednesday 22 June 2005 08:12, Bouchard, Sebastien wrote:
| > Hi,
| >
| > Here is a driver (only for 2.4.x) I've done to provide support of a
| > hardware module (available on some ATCA board) used with telecom expension
| > card on the new ATCA platform. This module provide redundant reference
| > clock for telecom hardware with alarm handling when there is a new event
| > (ex.: one of the ref clock is gone).
| > This char driver provide IOCTL for configuration of this module and
| > interrupt handler for processing alarm events.
| >
| > I send this driver so people in this mailing list can do a review of the
| > code.
| >
| > Please reply me directly to my email, i'm not subscribed to the mailing
| > list.
| >
| > Thanks
| > Sebastien Bouchard
| > Software designer
| > Kontron Canada Inc.
| > <mailto:sebastien.bouchard@ca.kontron.com>
| > <http://www.kontron.com/>
| 
| I'm helping out a bit with the maintaining of this driver for Sebastien.  
| The following is a 2.6.12 port of Sebastien's 2.4 driver.  

I'd be surprised if Marcelo was taking new drivers for 2.4.x.

2.6.x driver comments below.

| diff -urN -X dontdiff_osdl vanilla/linux-2.6.12/drivers/char/Kconfig linux-2.6.12/drivers/char/Kconfig
| --- vanilla/linux-2.6.12/drivers/char/Kconfig	2005-06-17 12:48:29.000000000 -0700
| +++ linux-2.6.12/drivers/char/Kconfig	2005-07-06 13:25:18.585951744 -0700
| @@ -998,5 +998,16 @@
|  
|  source "drivers/char/tpm/Kconfig"
|  
| +config TELCLOCK
| +	tristate "Telecom clock driver for ATCA"
| +	depends on EXPERIMENTAL
| +	default n
| +	help
| +	  The telecom clock device allows direct userspace access to the
| +	  configuration of the telecom clock configuration settings.
| +	  This device is used for hardware synchronization across the ATCA
| +	  back plane fabric.

usually:  backplane

| +
| +

Just 1 blank line, please.

|  endmenu
|  

| diff -urN -X dontdiff_osdl vanilla/linux-2.6.12/drivers/char/tlclk.c linux-2.6.12/drivers/char/tlclk.c
| --- vanilla/linux-2.6.12/drivers/char/tlclk.c	1969-12-31 16:00:00.000000000 -0800
| +++ linux-2.6.12/drivers/char/tlclk.c	2005-07-06 13:25:18.600949464 -0700
| @@ -0,0 +1,447 @@
| +/*
| + * Telecom Clock driver for Wainwright board
| + *
| + * Copyright (C) 2005 Kontron Canada
| + *
| + * All rights reserved.
| + *
| + * This program is free software; you can redistribute it and/or modify
| + * it under the terms of the GNU General Public License as published by
| + * the Free Software Foundation; either version 2 of the License, or (at
| + * your option) any later version.
| + *
| + * This program is distributed in the hope that it will be useful, but
| + * WITHOUT ANY WARRANTY; without even the implied warranty of
| + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
| + * NON INFRINGEMENT.  See the GNU General Public License for more
| + * details.
| + *
| + * You should have received a copy of the GNU General Public License
| + * along with this program; if not, write to the Free Software
| + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
| + *
| + * Send feedback to <sebastien.bouchard@ca.kontron.com>
| + *
| + * Description : This is the TELECOM CLOCK module driver for the ATCA platform.
| + */
| +#ifndef __KERNEL__
| +#  define __KERNEL__
| +#endif

Not needed.

| +
| +#include <linux/config.h>
| +#include <linux/module.h>
| +#include <linux/init.h>
| +#include <linux/sched.h>
| +#include <linux/kernel.h>	/* printk() */
| +#include <linux/fs.h>		/* everything... */
| +#include <linux/errno.h>	/* error codes */
| +#include <linux/delay.h>	/* udelay */
| +#include <asm/uaccess.h>

Try to put all <asm> includes after the <linux> includes
unless it's not possible for some reason.

| +#include <linux/slab.h>
| +#include <linux/ioport.h>
| +#include <linux/devfs_fs_kernel.h>	/* Devfs support */

Devfs is being removed this month...

| +#include <linux/interrupt.h>
| +#include <linux/spinlock.h>
| +#include <linux/timer.h>
| +#include <asm/io.h>		/* inb/outb */
| +
| +#include "tlclk.h"	/* TELECOM IOCTL DEFINE */
| +
| +MODULE_AUTHOR("Sebastien Bouchard <sebastien.bouchard@ca.kontron.com>");
| +MODULE_LICENSE("GPL");
| +
| +/* Telecom clock I/O register definition */
| +#define TLCLK_BASE 0xa08            
| +#define TLCLK_REG0 TLCLK_BASE
| +#define TLCLK_REG1 (TLCLK_BASE+1)
| +#define TLCLK_REG2 (TLCLK_BASE+2)
| +#define TLCLK_REG3 (TLCLK_BASE+3)
| +#define TLCLK_REG4 (TLCLK_BASE+4)
| +#define TLCLK_REG5 (TLCLK_BASE+5)
| +#define TLCLK_REG6 (TLCLK_BASE+6)
| +#define TLCLK_REG7 (TLCLK_BASE+7)
| +
| +#define SET_PORT_BITS(port, mask, val) outb(((inb(port) & mask) | val), port)
| +
| +/* 0 = Dynamic allocation of the major device number */
| +#define TLCLK_MAJOR 252
| +
| +/* Contain the interrupt used for telecom clock */

eh?  contains?  this is self-evident, drop it.

| +static unsigned int telclk_interrupt;
| +
| +static int int_events;		/* Event that generate a interrupt */
| +static int got_event;		/* if events processing have been done */
| +
| +static struct timer_list switchover_timer;
| +
| +struct tlclk_alarms *alarm_events;
| +
| +spinlock_t event_lock = SPIN_LOCK_UNLOCKED;

Use
DEFINE_SPINLOCK(event_lock);

| +
| +/* DEVFS support or not */

Drop it, going away.

| +#ifdef CONFIG_DEVFS_FS
| +devfs_handle_t devfs_handle;
| +#else
| +static int tlclk_major = TLCLK_MAJOR;
| +#endif
| +
| +static void switchover_timeout(unsigned long data);
| +irqreturn_t tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs);
| +
| +DECLARE_WAIT_QUEUE_HEAD(wq);
| +/*
| +*  Function : Module I/O functions
| +*  Description : Almost all the control stuff is done here, check I/O dn for
| +*  help.

Please just use kerneldoc comment format.

| +*/
| +static int
| +tlclk_ioctl(struct inode *inode,
| +	    struct file *filp, unsigned int cmd, unsigned long arg)
| +{
| +	unsigned long flags;
| +	unsigned char val;

Blank line between data and code, please.

Of course, as has been mentioned many times on lkml, we are trying
not to add new ioctls to Linux.  sysfs is preferred (or some other
fs, but not procfs).  If ioctls are the only possibility here,
it needs some justification.

| +	val = (unsigned char) arg;
| +
| +	if (_IOC_TYPE(cmd) != TLCLK_IOC_MAGIC)
| +		return -ENOTTY;
| +
| +	if (_IOC_NR(cmd) > TLCLK_IOC_MAXNR)
| +		return -ENOTTY;
| +
| +	switch (cmd) {
| +	case IOCTL_RESET:
| +		SET_PORT_BITS(TLCLK_REG4, 0xfd, val);

Where do all of these magic values like 0xfd come from
and what do they mean?

| +		break;
| +	case IOCTL_MODE_SELECT:
| +		SET_PORT_BITS(TLCLK_REG0, 0xcf, val);
| +		break;
| +	case IOCTL_REFALIGN:
| +		/* GENERATING 0 to 1 transistion */
| +		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
| +		udelay(2);
| +		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0x08);
| +		udelay(2);
| +		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
| +		break;
| +	case IOCTL_HARDWARE_SWITCHING:
| +		SET_PORT_BITS(TLCLK_REG0, 0x7f, val);
| +		break;
| +	case IOCTL_HARDWARE_SWITCHING_MODE:
| +		SET_PORT_BITS(TLCLK_REG0, 0xbf, val);
| +		break;
| +	case IOCTL_FILTER_SELECT:
| +		SET_PORT_BITS(TLCLK_REG0, 0xfb, val);
| +		break;
| +	case IOCTL_SELECT_REF_FREQUENCY:
| +		spin_lock_irqsave(&event_lock, flags);
| +		SET_PORT_BITS(TLCLK_REG1, 0xfd, val);
| +		spin_unlock_irqrestore(&event_lock, flags);
| +		break;

Why do some of these need spinlocks (above and below) while
others don't?
Aha, the interrupt handler uses REG2, REG6, and REG1.
So do the other users of REG2 and REG1 below also need
spinlocks?

| +	case IOCTL_SELECT_REDUNDANT_CLOCK:
| +		spin_lock_irqsave(&event_lock, flags);
| +		SET_PORT_BITS(TLCLK_REG1, 0xfe, val);
| +		spin_unlock_irqrestore(&event_lock, flags);
| +		break;
| +	case IOCTL_SELECT_AMCB1_TRANSMIT_CLOCK:
| +		if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
| +			SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x5);
| +			SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
| +		} else if (val >= CLK_8_592MHz) {
| +			SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x7);
| +			switch (val) {
| +			case CLK_8_592MHz:
| +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
| +				break;
| +			case CLK_11_184MHz:
| +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
| +				break;
| +			case CLK_34_368MHz:
| +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
| +				break;
| +			case CLK_44_736MHz:
| +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
| +				break;
| +			}
| +		} else
| +			SET_PORT_BITS(TLCLK_REG3, 0xf8, val);
| +		break;
| +	case IOCTL_SELECT_AMCB2_TRANSMIT_CLOCK:
| +		if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
| +			SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x28);
| +			SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
| +		} else if (val >= CLK_8_592MHz) {
| +			SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x38);
| +			switch (val) {
| +			case CLK_8_592MHz:
| +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
| +				break;
| +			case CLK_11_184MHz:
| +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
| +				break;
| +			case CLK_34_368MHz:
| +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
| +				break;
| +			case CLK_44_736MHz:
| +				SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
| +				break;
| +			}
| +		} else
| +			SET_PORT_BITS(TLCLK_REG3, 0xc7, val << 3);
| +		break;
| +	case IOCTL_TEST_MODE:
| +		SET_PORT_BITS(TLCLK_REG4, 0xfd, 2);
| +		break;
| +	case IOCTL_ENABLE_CLKA0_OUTPUT:
| +		SET_PORT_BITS(TLCLK_REG2, 0xfe, val);
| +		break;
| +	case IOCTL_ENABLE_CLKB0_OUTPUT:
| +		SET_PORT_BITS(TLCLK_REG2, 0xfd, val << 1);
| +		break;
| +	case IOCTL_ENABLE_CLKA1_OUTPUT:
| +		SET_PORT_BITS(TLCLK_REG2, 0xfb, val << 2);
| +		break;
| +	case IOCTL_ENABLE_CLKB1_OUTPUT:
| +		SET_PORT_BITS(TLCLK_REG2, 0xf7, val << 3);
| +		break;
| +	case IOCTL_ENABLE_CLK3A_OUTPUT:
| +		SET_PORT_BITS(TLCLK_REG3, 0xbf, val << 6);
| +		break;
| +	case IOCTL_ENABLE_CLK3B_OUTPUT:
| +		SET_PORT_BITS(TLCLK_REG3, 0x7f, val << 7);
| +		break;
| +	case IOCTL_READ_ALARMS:
| +		return (inb(TLCLK_REG2) & 0xf0);
| +		break;
| +	case IOCTL_READ_INTERRUPT_SWITCH:
| +		return inb(TLCLK_REG6);
| +		break;
| +	case IOCTL_READ_CURRENT_REF:
| +		return ((inb(TLCLK_REG1) & 0x08) >> 3);
| +		break;
| +	}
| +
| +	return 0;
| +}
| +
| +/*
| +*  Function : Module Opening
| +*  Description : Called when a program open the 
| +*  /dev/telclock file related to the module.

Use kerneldoc format.

| +*/
| +static int
| +tlclk_open(struct inode *inode, struct file *filp)
| +{
| +	int result;

Blank line here.

| +	/* Make sure there is no interrupt pending will 
        .......................................... while ??

| +	   *  initialising interrupt handler */
| +	inb(TLCLK_REG6);
| +
| +	result = request_irq(telclk_interrupt, &tlclk_interrupt,
| +			     SA_SHIRQ, "telclock", tlclk_interrupt);
| +	printk("\ntelclock: Reserving IRQ%d...\n", telclk_interrupt);

KERN_DEBUG ?? or some KERN_ level.

| +	if (result == -EBUSY) {
| +		printk(KERN_ERR
| +		       "telclock: Interrupt can't be reserved!\n");

This printk is where the interrupt number is needed.
The other one (above) is just debug info and not really needed.

| +		return -EBUSY;
| +	}
| +	inb(TLCLK_REG6);	/* Clear interrupt events */
| +	return 0;
| +}
| +
| +/*
| +*  Function : Module Releasing
| +*  Description : Called when a program stop using the module.

Use kerneldoc format, please.

| +*/
| +static int
| +tlclk_release(struct inode *inode, struct file *filp)
| +{
| +		free_irq(telclk_interrupt, tlclk_interrupt);

Extra tab ^ here.

| +	return 0;
| +}
| +
| +ssize_t
| +tlclk_read(struct file * filp, char *buf, size_t count, loff_t * f_pos)

buf is __user, right?
Oh, use '*' consistently, like *filp & *f_pos.
Please run thru sparse.

| +{
| +	int count0 = sizeof(struct tlclk_alarms);

Blank line here.

| +	wait_event_interruptible(wq, got_event);
| +	if (copy_to_user(buf, alarm_events, sizeof(struct tlclk_alarms)))
| +		return -EFAULT;
| +
| +	memset(alarm_events, 0, sizeof(struct tlclk_alarms));
| +	got_event = 0;
| +	
| +	return count0;
| +}
| +
| +ssize_t
| +tlclk_write(struct file * filp, const char *buf, size_t count, loff_t * f_pos)

Make buf __user.

| +{
| +	return 0;
| +}
| +
| +/*
| +* This is where you set what function is called when an action is done to your
| +*  /dev file.
| +*/
| +static struct file_operations tlclk_fops = {
| +	.read = tlclk_read,
| +	.write = tlclk_write,
| +	.ioctl = tlclk_ioctl,
| +	.open = tlclk_open,
| +	.release = tlclk_release,
| +
| +};
| +/*
| +* Function : Module Initialisation                      
| +* Description : Called at module loading, 
| +* all the OS registering stuff is her

Please use kerneldoc format instead.

| +*/
| +static int __init
| +tlclk_init(void)
| +{
| +/* DEVFS or NOT? */

Drop devfs code.

| +#ifdef CONFIG_DEVFS_FS
| +	devfs_handle = devfs_register(NULL, "telclock",
| +					DEVFS_FL_AUTO_DEVNUM, TLCLK_MAJOR,
| +					0,
| +					S_IFCHR | S_IRUGO | S_IWUGO,
| +					&tlclk_fops, NULL);
| +	if (!devfs_handle)
| +		goto out1;
| +#else
| +	tlclk_major = register_chrdev(tlclk_major, "telclock", &tlclk_fops);
| +
| +	if (tlclk_major < 0) {
| +		printk(KERN_ERR "telclock: can't get major! %d\n", tlclk_major);
| +		return tlclk_major;
| +	}
| +#endif
| +
| +	alarm_events = kmalloc(sizeof(struct tlclk_alarms), GFP_KERNEL);

Use kcalloc() to alloc and clear (drop the memset below).

| +	
| +	if (!alarm_events)
| +		goto out1;
| +	
| +	memset(alarm_events, 0, sizeof(struct tlclk_alarms));
| +
| +/* Read telecom clock IRQ number (Set by BIOS) */
| +
| +	printk(KERN_WARNING "telclock: Reserving I/O region...\n");

That shouldn't be a WARNING.  It's debug, but could be dropped completely.

| +
| +	if ( !request_region(TLCLK_BASE, 8, "telclock") ) {
| +		printk(KERN_ERR "telclock: request_region failed!\n");

Ah, but add the failing I/O base address to this printk.

| +			goto out2;
| +	}
| +	telclk_interrupt = (inb(TLCLK_REG7) & 0x0f);
| +
| +	init_timer(&switchover_timer);
| +	switchover_timer.function = switchover_timeout;
| +	switchover_timer.data = 0;
| +
| +	return 0;
| +out2:
| +	kfree(alarm_events);
| +out1:
| +	return -EBUSY;
| +}
| +
| +/*
| +*  Function : Module Cleaning
| +*  Description : Called when unloading the module

Use kerneldoc format.

| +*/
| +static void __exit
| +tlclk_cleanup(void)
| +{
| +#ifdef CONFIG_DEVFS_FS
| +	devfs_unregister(devfs_handle);
| +#else
| +	unregister_chrdev(tlclk_major, "telclock");
| +#endif

Drop devfs support.

| +	release_region(TLCLK_BASE, 8);
| +	del_timer_sync(&switchover_timer);
| +	kfree(alarm_events);
| +	
| +}
| +static void
| +switchover_timeout(unsigned long data)
| +{
| +	if ((data & 1)) {
| +		if ((inb(TLCLK_REG1) & 0x08) != (data & 0x08))
| +			alarm_events->switchover_primary++;
| +	} else {
| +		if ((inb(TLCLK_REG1) & 0x08) != (data & 0x08))
| +			alarm_events->switchover_secondary++;
| +	}
| +
| +	/* Alarm processing is done, wake up read task */
| +	del_timer(&switchover_timer);
| +	got_event = 1;
| +	wake_up(&wq);
| +}
| +/*
| +*  Function : Interrupt Handler
| +*  Description :

Use kerneldoc format.

| +*/
| +irqreturn_t
| +tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs)
| +{
| +	unsigned long flags;

Blank line here.

| +	spin_lock_irqsave(&event_lock, flags);
| +	/* Read and clear interrupt events */
| +	int_events = inb(TLCLK_REG6);
| +	spin_unlock_irqrestore(&event_lock, flags);
| +	
| +	/* Primary_Los changed from 0 to 1 ? */
| +	if (int_events & PRI_LOS_01_MASK) {
| +		if (inb(TLCLK_REG2) & SEC_LOST_MASK)
| +			alarm_events->lost_clocks++;
| +		else
| +			alarm_events->lost_primary_clock++;
| +	}
| +
| +	/* Primary_Los changed from 1 to 0 ? */
| +	if (int_events & PRI_LOS_10_MASK) {
| +		alarm_events->primary_clock_back++;
| +		spin_lock_irqsave(&event_lock, flags);
| +		SET_PORT_BITS(TLCLK_REG1, 0xFE, 1);
| +		spin_unlock_irqrestore(&event_lock, flags);
| +	}
| +	/* Secondary_Los changed from 0 to 1 ? */
| +	if (int_events & SEC_LOS_01_MASK) {
| +		if (inb(TLCLK_REG2) & PRI_LOST_MASK)
| +			alarm_events->lost_clocks++;
| +		else
| +			alarm_events->lost_secondary_clock++;
| +	}
| +	/* Secondary_Los changed from 1 to 0 ? */
| +	if (int_events & SEC_LOS_10_MASK) {
| +		alarm_events->secondary_clock_back++;
| +		spin_lock_irqsave(&event_lock, flags);
| +		SET_PORT_BITS(TLCLK_REG1, 0xFE, 0);
| +		spin_unlock_irqrestore(&event_lock, flags);
| +	}
| +	if (int_events & HOLDOVER_10_MASK)
| +		alarm_events->pll_end_holdover++;
| +
| +	if (int_events & UNLOCK_01_MASK)
| +		alarm_events->pll_lost_sync++;
| +
| +	if (int_events & UNLOCK_10_MASK)
| +		alarm_events->pll_sync++;
| +
| +	/* Holdover changed from 0 to 1 ? */
| +	if (int_events & HOLDOVER_01_MASK) {
| +		alarm_events->pll_holdover++;
| +
| +		switchover_timer.expires = jiffies + 1;	/* TIMEOUT in ~10ms */
| +		switchover_timer.data = inb(TLCLK_REG1);
| +		add_timer(&switchover_timer);
| +	} else {
| +		got_event = 1;
| +		wake_up(&wq);
| +	}
| +	return IRQ_HANDLED;

Since the interrupt is allocated as shared, is there any checking
for whether the interrupt actually belonged to this hardware?
and return IRQ_NONE if not for this interrupt handler...

| +}
| +
| +module_init(tlclk_init);
| +module_exit(tlclk_cleanup);
| diff -urN -X dontdiff_osdl vanilla/linux-2.6.12/drivers/char/tlclk.h linux-2.6.12/drivers/char/tlclk.h
| --- vanilla/linux-2.6.12/drivers/char/tlclk.h	1969-12-31 16:00:00.000000000 -0800
| +++ linux-2.6.12/drivers/char/tlclk.h	2005-07-06 13:25:18.607948400 -0700
| @@ -0,0 +1,167 @@
| +/*
| + * Telecom Clock driver for Wainwright board
| + *
| + * Copyright (C) 2005 Kontron Canada
| + *
| + * All rights reserved.
| + */
| + 
| +/* Ioctl definitions  */
| +
| +/* Use 0xA1 as magic number */
| +#define TLCLK_IOC_MAGIC 0xA1

Add this to Documentation/magic-number.txt .

| +
| +/*Hardware Reset of the PLL */
| +
| +#define RESET_ON 0x00
| +#define RESET_OFF 0x01

Please use TAB to align the numeric values.

| +#define IOCTL_RESET _IO(TLCLK_IOC_MAGIC,  1)
| +
| +#define IOCTL_REFALIGN _IO(TLCLK_IOC_MAGIC,  2)
| +
| +/* MODE SELECT */

Lots of these ALL CAPS headings are too much like shouting.

| +
| +#define NORMAL_MODE 0x00
| +#define HOLDOVER_MODE 0x10
| +#define FREERUN_MODE 0x20

Align numeric values... (more below [not marked]).

| +
| +#define IOCTL_MODE_SELECT _IOR(TLCLK_IOC_MAGIC,  3, char)
| +
| +/* FILTER SELECT */
| +
| +#define FILTER_6HZ 0x04
| +#define FILTER_12HZ 0x00
| +
| +#define IOCTL_FILTER_SELECT _IOR(TLCLK_IOC_MAGIC,  4, char)
| +
| +/* SELECT REFERENCE FREQUENCY */
| +
| +#define REF_CLK1_8kHz 0x00
| +#define REF_CLK2_19_44MHz 0x02
| +
| +#define IOCTL_SELECT_REF_FREQUENCY _IOR(TLCLK_IOC_MAGIC,  6, char)
| +
| +/* Select primary or secondary redundant clock */
| +
| +#define PRIMARY_CLOCK 0x00
| +#define SECONDARY_CLOCK 0x01
| +#define IOCTL_SELECT_REDUNDANT_CLOCK _IOR(TLCLK_IOC_MAGIC,  7, char)
| +
| +/* CLOCK TRANSMISSION DEFINE */
| +
| +#define CLK_8kHz 0xff
| +#define CLK_16_384MHz 0xfb
| +
| +#define CLK_1_544MHz 0x00
| +#define CLK_2_048MHz 0x01
| +#define CLK_4_096MHz 0x02
| +#define CLK_6_312MHz 0x03
| +#define CLK_8_192MHz 0x04
| +#define CLK_19_440MHz 0x06
| +
| +#define CLK_8_592MHz 0x08
| +#define CLK_11_184MHz 0x09
| +#define CLK_34_368MHz 0x0b
| +#define CLK_44_736MHz 0x0a
| +
| +#define IOCTL_SELECT_AMCB1_TRANSMIT_CLOCK _IOR(TLCLK_IOC_MAGIC,  9, char)
| +#define IOCTL_SELECT_AMCB2_TRANSMIT_CLOCK _IOR(TLCLK_IOC_MAGIC,  10, char)
| +
| +/* RECEIVED REFERENCE */
| +
| +#define AMC_B1 0
| +#define AMC_B2 1
| +
| +#define IOCTL_SELECT_RECEIVED_REF_CLK3A _IOR(TLCLK_IOC_MAGIC,  11, char)
| +#define IOCTL_SELECT_RECEIVED_REF_CLK3B _IOR(TLCLK_IOC_MAGIC,  12, char)
| +
| +/* OEM COMMAND - NOT IN FINAL VERSION */
| +
| +#define IOCTL_TEST_MODE _IO(TLCLK_IOC_MAGIC,  13)
| +
| +/* HARDWARE SWITCHING DEFINE */
| +
| +#define HW_ENABLE 0x80
| +#define HW_DISABLE 0x00
| +
| +#define IOCTL_HARDWARE_SWITCHING _IOR(TLCLK_IOC_MAGIC,  14, char)
| +
| +/* HARDWARE SWITCHING MODE DEFINE */
| +
| +#define PLL_HOLDOVER 0x40
| +#define LOST_CLOCK 0x00
| +
| +#define IOCTL_HARDWARE_SWITCHING_MODE _IOR(TLCLK_IOC_MAGIC,  15, char)
| +
| +/* CLOCK OUTPUT DEFINE */
| +
| +#define IOCTL_ENABLE_CLKA0_OUTPUT _IOR(TLCLK_IOC_MAGIC,  16, char)
| +#define IOCTL_ENABLE_CLKB0_OUTPUT _IOR(TLCLK_IOC_MAGIC,  17, char)
| +#define IOCTL_ENABLE_CLKA1_OUTPUT _IOR(TLCLK_IOC_MAGIC,  18, char)
| +#define IOCTL_ENABLE_CLKB1_OUTPUT _IOR(TLCLK_IOC_MAGIC,  19, char)
| +
| +#define IOCTL_ENABLE_CLK3A_OUTPUT _IOR(TLCLK_IOC_MAGIC,  20, char)
| +#define IOCTL_ENABLE_CLK3B_OUTPUT _IOR(TLCLK_IOC_MAGIC,  21, char)
| +
| +/* ALARMS DEFINE */
| +
| +#define UNLOCK_MASK 0x10
| +#define HOLDOVER_MASK 0x20
| +#define SEC_LOST_MASK 0x40
| +#define PRI_LOST_MASK 0x80
| +
| +#define IOCTL_READ_ALARMS _IO(TLCLK_IOC_MAGIC,  22)

Why is READ_ALARMS not an _IOR() ?

| +
| +/* INTERRUPT CAUSE DEFINE */
| +
| +#define PRI_LOS_01_MASK 0x01
| +#define PRI_LOS_10_MASK 0x02
| +
| +#define SEC_LOS_01_MASK 0x04
| +#define SEC_LOS_10_MASK 0x08
| +
| +#define HOLDOVER_01_MASK 0x10
| +#define HOLDOVER_10_MASK 0x20
| +
| +#define UNLOCK_01_MASK 0x40
| +#define UNLOCK_10_MASK 0x80
| +
| +#define IOCTL_READ_INTERRUPT_SWITCH _IO(TLCLK_IOC_MAGIC,  23)
| +
| +#define IOCTL_READ_CURRENT_REF _IO(TLCLK_IOC_MAGIC,  25)

Why are these 2 not _IOR() ?

| +
| +/* MAX NUMBER OF IOCTL */
| +#define TLCLK_IOC_MAXNR 25
| +
| +struct tlclk_alarms {
| +	unsigned int lost_clocks;
| +	unsigned int lost_primary_clock;
| +	unsigned int lost_secondary_clock;
| +	unsigned int primary_clock_back;
| +	unsigned int secondary_clock_back;
| +	unsigned int switchover_primary;
| +	unsigned int switchover_secondary;
| +	unsigned int pll_holdover;
| +	unsigned int pll_end_holdover;
| +	unsigned int pll_lost_sync;
| +	unsigned int pll_sync;
| +};
| +
| -

---
~Randy

Re: Patch of a new driver for kernel 2.4.x that need review

[Greg KH]

On Wed, Jul 06, 2005 at 02:14:44PM -0700, Mark Gross wrote:
> +#ifndef __KERNEL__
> +#  define __KERNEL__
> +#endif

Not needed.

> +
> +#include <linux/config.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/sched.h>
> +#include <linux/kernel.h>	/* printk() */
> +#include <linux/fs.h>		/* everything... */
> +#include <linux/errno.h>	/* error codes */
> +#include <linux/delay.h>	/* udelay */
> +#include <asm/uaccess.h>
> +#include <linux/slab.h>
> +#include <linux/ioport.h>
> +#include <linux/devfs_fs_kernel.h>	/* Devfs support */
> +#include <linux/interrupt.h>
> +#include <linux/spinlock.h>
> +#include <linux/timer.h>
> +#include <asm/io.h>		/* inb/outb */

Put asm/ at the end.

> +
> +#include "tlclk.h"	/* TELECOM IOCTL DEFINE */

No new ioctls please.  Use sysfs or something else.

> +
> +MODULE_AUTHOR("Sebastien Bouchard <sebastien.bouchard@ca.kontron.com>");
> +MODULE_LICENSE("GPL");
> +
> +/* Telecom clock I/O register definition */
> +#define TLCLK_BASE 0xa08            
> +#define TLCLK_REG0 TLCLK_BASE
> +#define TLCLK_REG1 (TLCLK_BASE+1)
> +#define TLCLK_REG2 (TLCLK_BASE+2)
> +#define TLCLK_REG3 (TLCLK_BASE+3)
> +#define TLCLK_REG4 (TLCLK_BASE+4)
> +#define TLCLK_REG5 (TLCLK_BASE+5)
> +#define TLCLK_REG6 (TLCLK_BASE+6)
> +#define TLCLK_REG7 (TLCLK_BASE+7)
> +
> +#define SET_PORT_BITS(port, mask, val) outb(((inb(port) & mask) | val), port)
> +
> +/* 0 = Dynamic allocation of the major device number */
> +#define TLCLK_MAJOR 252

Is this a reserved major?

> +/* DEVFS support or not */
> +#ifdef CONFIG_DEVFS_FS

DEVFS is obsolete and will be removed very soon now.  You don't need to
add support for it in your driver.

> +/*
> +*  Function : Module Opening
> +*  Description : Called when a program open the 
> +*  /dev/telclock file related to the module.
> +*/

Function comments like this are not needed at all.

> +struct tlclk_alarms {
> +	unsigned int lost_clocks;
> +	unsigned int lost_primary_clock;
> +	unsigned int lost_secondary_clock;
> +	unsigned int primary_clock_back;
> +	unsigned int secondary_clock_back;
> +	unsigned int switchover_primary;
> +	unsigned int switchover_secondary;
> +	unsigned int pll_holdover;
> +	unsigned int pll_end_holdover;
> +	unsigned int pll_lost_sync;
> +	unsigned int pll_sync;
> +};

If you are going to pass a structure accross the kernel/userspace
boundry, you need to explicitly mark the variable sizes.  Use __u8,
__u16, or __u32 here (unless they are signed, then use __s8, etc.)

All of those values look like they should just be exported as individual
sysfs files, right?

thanks,

greg k-h

Re: Patch of a new driver for kernel 2.4.x that need review

[Mark Gross]

On Wednesday 06 July 2005 14:14, Mark Gross wrote:
> On Wednesday 22 June 2005 08:12, Bouchard, Sebastien wrote:
> > Hi,
> >
> > Here is a driver (only for 2.4.x) I've done to provide support of a
> > hardware module (available on some ATCA board) used with telecom expension
> > card on the new ATCA platform. This module provide redundant reference
> > clock for telecom hardware with alarm handling when there is a new event
> > (ex.: one of the ref clock is gone).
> > This char driver provide IOCTL for configuration of this module and
> > interrupt handler for processing alarm events.
> >
> > I send this driver so people in this mailing list can do a review of the
> > code.
> >
> > Please reply me directly to my email, i'm not subscribed to the mailing
> > list.
> >
> > Thanks
> > Sebastien Bouchard
> > Software designer
> > Kontron Canada Inc.
> > <mailto:sebastien.bouchard@ca.kontron.com>
> > <http://www.kontron.com/>
> 
> I'm helping out a bit with the maintaining of this driver for Sebastien.  
> The following is a 2.6.12 port of Sebastien's 2.4 driver.  
> 
> --mgross

The following is an update to the earlier 2.6 driver that uses a sysfs interface 
to implement the IOCTL functions.  I put the attributes under /sys/class/misc/tlclk.

I can't say I'm a believer in the "goodness" of using sysfs over the IOCTL's, as it 
added quite a bit of code bloat to this driver, but hay it should work well enough anyway.

Also, note this device, is accessed / controlled via the FPGA on the ATCA, its function 
is to synchronize signaling hardware across blades in an ATCA Chassis.  It tends 
to not talk to the OS. 

Please tell me what you think :)

diff -urN -X dontdiff linux-2.6.12.2/drivers/char/Kconfig linux-2.6.12.2-tlclk/drivers/char/Kconfig
--- linux-2.6.12.2/drivers/char/Kconfig 2005-06-29 16:00:53.000000000 -0700
+++ linux-2.6.12.2-tlclk/drivers/char/Kconfig	2005-08-07 10:39:55.000000000 -0700
@@ -998,5 +998,15 @@
 
 source "drivers/char/tpm/Kconfig"
 
+config TELCLOCK
+	tristate "Telecom clock driver for ATCA"
+	depends on EXPERIMENTAL
+	default n
+	help
+	  The telecom clock device allows direct userspace access to the
+	  configuration of the telecom clock configuration settings.
+	  This device is used for hardware synchronization across the ATCA
+	  backplane fabric.
+
 endmenu
 
diff -urN -X dontdiff linux-2.6.12.2/drivers/char/Makefile linux-2.6.12.2-tlclk/drivers/char/Makefile
--- linux-2.6.12.2/drivers/char/Makefile	2005-06-29 16:00:53.000000000 -0700
+++ linux-2.6.12.2-tlclk/drivers/char/Makefile	2005-08-07 10:39:55.000000000 -0700
@@ -81,6 +81,7 @@
 obj-$(CONFIG_NWFLASH) += nwflash.o
 obj-$(CONFIG_SCx200_GPIO) += scx200_gpio.o
 obj-$(CONFIG_TANBAC_TB0219) += tb0219.o
+obj-$(CONFIG_TELCLOCK) += tlclk.o
 
 obj-$(CONFIG_WATCHDOG)	+= watchdog/
 obj-$(CONFIG_MWAVE) += mwave/
diff -urN -X dontdiff linux-2.6.12.2/drivers/char/tlclk.c linux-2.6.12.2-tlclk/drivers/char/tlclk.c
--- linux-2.6.12.2/drivers/char/tlclk.c	1969-12-31 16:00:00.000000000 -0800
+++ linux-2.6.12.2-tlclk/drivers/char/tlclk.c	2005-08-07 12:40:58.000000000 -0700
@@ -0,0 +1,933 @@
+/*
+ * Telecom Clock driver for Wainwright board
+ *
+ * Copyright (C) 2005 Kontron Canada
+ *
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <sebastien.bouchard@ca.kontron.com>
+ * 
+ * 2.6 driver version being maintained by <mark.gross@intel.com>
+ *
+ * Description : This is the TELECOM CLOCK module driver for the ATCA platform.
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/sched.h>
+#include <linux/kernel.h> /* printk() */
+#include <linux/fs.h>  /* everything... */
+#include <linux/errno.h> /* error codes */
+#include <linux/delay.h> /* udelay */
+#include <linux/slab.h>
+#include <linux/ioport.h>
+#include <linux/interrupt.h>
+#include <linux/spinlock.h>
+#include <linux/timer.h>
+#include <linux/sysfs.h>
+#include <linux/device.h>
+#include <linux/miscdevice.h>
+#include <asm/io.h>  /* inb/outb */
+#include <asm/uaccess.h>
+
+#include "tlclk.h"  /* TELECOM IOCTL DEFINE */
+
+MODULE_AUTHOR("Sebastien Bouchard <sebastien.bouchard@ca.kontron.com>");
+MODULE_LICENSE("GPL");
+
+/* Telecom clock I/O register definition */
+#define TLCLK_BASE 0xa08
+#define TLCLK_REG0 TLCLK_BASE
+#define TLCLK_REG1 (TLCLK_BASE+1)
+#define TLCLK_REG2 (TLCLK_BASE+2)
+#define TLCLK_REG3 (TLCLK_BASE+3)
+#define TLCLK_REG4 (TLCLK_BASE+4)
+#define TLCLK_REG5 (TLCLK_BASE+5)
+#define TLCLK_REG6 (TLCLK_BASE+6)
+#define TLCLK_REG7 (TLCLK_BASE+7)
+
+#define SET_PORT_BITS(port, mask, val) outb(((inb(port) & mask) | val), port)
+
+/* 0 = Dynamic allocation of the major device number */
+#define TLCLK_MAJOR 252
+
+/*
+uncomment to include classic IOCTL's 
+#define TLCLK_IOCTL
+*/
+
+/* sysFS interface definition:
+Uppon loading the driver will create a sysfs directory under class/misc/tlclk.
+
+This directory exports the following interfaces.
+alarms    :
+current_ref   :
+enable_clk3a_output  :
+enable_clk3b_output  :
+enable_clka0_output  :
+enable_clka1_output  :
+enable_clkb0_output  :
+enable_clkb1_output  :
+filter_select   :
+hardware_switching  :
+hardware_switching_mode  :
+interrupt_switch  :
+mode_select   :
+refalign   :
+reset    :
+select_amcb1_transmit_clock :
+select_amcb2_transmit_clock :
+select_redundant_clock  :
+select_ref_frequency  :
+test_mode   :
+
+All sysfs interaces are integers in hex format, i.e echo 99 > refalign
+has the same effect as echo 0x99 > refalign.
+
+*/
+
+#if CONFIG_DEBUG_KERNEL 
+#define debug_printk( args... ) printk( args)
+#else
+#define debug_printk( args... )
+#endif
+
+
+static unsigned int telclk_interrupt;
+
+static int int_events;  /* Event that generate a interrupt */
+static int got_event;  /* if events processing have been done */
+
+static struct timer_list switchover_timer;
+
+static struct tlclk_alarms *alarm_events;
+
+DEFINE_SPINLOCK(event_lock);
+
+static int tlclk_major = TLCLK_MAJOR;
+
+static void switchover_timeout(unsigned long data);
+irqreturn_t tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs);
+
+DECLARE_WAIT_QUEUE_HEAD(wq);
+
+#ifdef TLCLK_IOCTL
+static int
+tlclk_ioctl(struct inode *inode,
+     struct file *filp, unsigned int cmd, unsigned long arg)
+{
+	unsigned long flags;
+	unsigned char val;
+	int ret_val = 0;
+
+	val = (unsigned char)arg;
+	if (_IOC_TYPE(cmd) != TLCLK_IOC_MAGIC)
+		return -ENOTTY;
+
+	if (_IOC_NR(cmd) > TLCLK_IOC_MAXNR)
+		return -ENOTTY;
+
+	spin_lock_irqsave(&event_lock, flags);
+	switch (cmd) {
+	case IOCTL_RESET:
+		SET_PORT_BITS(TLCLK_REG4, 0xfd, val);
+		break;
+	case IOCTL_MODE_SELECT:
+		SET_PORT_BITS(TLCLK_REG0, 0xcf, val);
+		break;
+	case IOCTL_REFALIGN:
+		/* GENERATING 0 to 1 transistion */
+		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
+		udelay(2);
+		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0x08);
+		udelay(2);
+		SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
+  break;
+ case IOCTL_HARDWARE_SWITCHING:
+  SET_PORT_BITS(TLCLK_REG0, 0x7f, val);
+  break;
+ case IOCTL_HARDWARE_SWITCHING_MODE:
+  SET_PORT_BITS(TLCLK_REG0, 0xbf, val);
+  break;
+ case IOCTL_FILTER_SELECT:
+  SET_PORT_BITS(TLCLK_REG0, 0xfb, val);
+  break;
+ case IOCTL_SELECT_REF_FREQUENCY:
+  SET_PORT_BITS(TLCLK_REG1, 0xfd, val);
+  break;
+ case IOCTL_SELECT_REDUNDANT_CLOCK:
+  SET_PORT_BITS(TLCLK_REG1, 0xfe, val);
+  break;
+ case IOCTL_SELECT_AMCB1_TRANSMIT_CLOCK:
+  if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
+   SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x5);
+   SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
+  } else if (val >= CLK_8_592MHz) {
+   SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x7);
+   switch (val) {
+   case CLK_8_592MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+    break;
+   case CLK_11_184MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
+    break;
+   case CLK_34_368MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
+    break;
+   case CLK_44_736MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+    break;
+   }
+  } else
+   SET_PORT_BITS(TLCLK_REG3, 0xf8, val);
+  break;
+ case IOCTL_SELECT_AMCB2_TRANSMIT_CLOCK:
+  if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
+   SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x28);
+   SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
+  } else if (val >= CLK_8_592MHz) {
+   SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x38);
+   switch (val) {
+   case CLK_8_592MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+    break;
+   case CLK_11_184MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
+    break;
+   case CLK_34_368MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
+    break;
+   case CLK_44_736MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+    break;
+   }
+  } else
+   SET_PORT_BITS(TLCLK_REG3, 0xc7, val << 3);
+  break;
+ case IOCTL_TEST_MODE:
+  SET_PORT_BITS(TLCLK_REG4, 0xfd, 2);
+  break;
+ case IOCTL_ENABLE_CLKA0_OUTPUT:
+  SET_PORT_BITS(TLCLK_REG2, 0xfe, val);
+  break;
+ case IOCTL_ENABLE_CLKB0_OUTPUT:
+  SET_PORT_BITS(TLCLK_REG2, 0xfd, val << 1);
+  break;
+ case IOCTL_ENABLE_CLKA1_OUTPUT:
+  SET_PORT_BITS(TLCLK_REG2, 0xfb, val << 2);
+  break;
+ case IOCTL_ENABLE_CLKB1_OUTPUT:
+  SET_PORT_BITS(TLCLK_REG2, 0xf7, val << 3);
+  break;
+ case IOCTL_ENABLE_CLK3A_OUTPUT:
+  SET_PORT_BITS(TLCLK_REG3, 0xbf, val << 6);
+  break;
+ case IOCTL_ENABLE_CLK3B_OUTPUT:
+  SET_PORT_BITS(TLCLK_REG3, 0x7f, val << 7);
+  break;
+ case IOCTL_READ_ALARMS:
+  ret_val = (inb(TLCLK_REG2) & 0xf0);
+  break;
+ case IOCTL_READ_INTERRUPT_SWITCH:
+  ret_val = inb(TLCLK_REG6);
+  break;
+ case IOCTL_READ_CURRENT_REF:
+  ret_val = ((inb(TLCLK_REG1) & 0x08) >> 3);
+  break;
+ }
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return ret_val;
+}
+#endif
+
+
+static int tlclk_open(struct inode *inode, struct file *filp)
+{
+ int result;
+
+ /* Make sure there is no interrupt pending while 
+  * initialising interrupt handler */
+ inb(TLCLK_REG6);
+
+ /* This device is wired through the FPGA IO space of the ATCA blade 
+  * we can't share this IRQ */
+ result = request_irq(telclk_interrupt, &tlclk_interrupt,
+        SA_INTERRUPT, "telclock", tlclk_interrupt);
+ if (result == -EBUSY) {
+  printk(KERN_ERR "telclock: Interrupt can't be reserved!\n");
+  return -EBUSY;
+ }
+ inb(TLCLK_REG6); /* Clear interrupt events */
+
+ return 0;
+}
+
+static int tlclk_release(struct inode *inode, struct file *filp)
+{
+ free_irq(telclk_interrupt, tlclk_interrupt);
+
+ return 0;
+}
+
+ssize_t
+tlclk_read(struct file * filp, char __user * buf, size_t count, loff_t * f_pos)
+{
+ int count0 = sizeof(struct tlclk_alarms);
+
+ wait_event_interruptible(wq, got_event);
+ if (copy_to_user(buf, alarm_events, sizeof(struct tlclk_alarms)))
+  return -EFAULT;
+
+ memset(alarm_events, 0, sizeof(struct tlclk_alarms));
+ got_event = 0;
+
+ return count0;
+}
+
+ssize_t
+tlclk_write(struct file * filp, const char __user * buf, size_t count,
+     loff_t * f_pos)
+{
+ return 0;
+}
+
+static struct file_operations tlclk_fops = {
+ .read = tlclk_read,
+ .write = tlclk_write,
+#ifdef TLCLK_IOCTL
+ .ioctl = tlclk_ioctl,
+#endif
+ .open = tlclk_open,
+ .release = tlclk_release,
+
+};
+
+#ifdef CONFIG_SYSFS
+static struct miscdevice tlclk_miscdev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "tlclk",
+ .fops = &tlclk_fops,
+};
+
+static ssize_t show_current_ref(struct class_device *d, char * buf)
+{
+ unsigned long ret_val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&event_lock, flags);
+  ret_val = ((inb(TLCLK_REG1) & 0x08) >> 3);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return sprintf(buf, "0x%lX\n", ret_val);
+}
+
+static CLASS_DEVICE_ATTR(current_ref, S_IRUGO, show_current_ref, NULL);
+
+
+static ssize_t show_interrupt_switch(struct class_device *d, char * buf)
+{
+ unsigned long ret_val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&event_lock, flags);
+  ret_val = inb(TLCLK_REG6);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return sprintf(buf, "0x%lX\n", ret_val);
+}
+
+static CLASS_DEVICE_ATTR(interrupt_switch, S_IRUGO, 
+  show_interrupt_switch, NULL);
+
+
+static ssize_t show_alarms(struct class_device *d, char * buf)
+{
+ unsigned long ret_val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&event_lock, flags);
+  ret_val = (inb(TLCLK_REG2) & 0xf0);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return sprintf(buf, "0x%lX\n", ret_val);
+}
+
+static CLASS_DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
+
+
+static ssize_t store_enable_clk3b_output(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+  SET_PORT_BITS(TLCLK_REG3, 0x7f, val << 7);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(enable_clk3b_output, S_IWUGO, NULL, 
+  store_enable_clk3b_output);
+
+
+
+static ssize_t store_enable_clk3a_output(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long flags;
+ unsigned long tmp;
+ unsigned char val;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+  SET_PORT_BITS(TLCLK_REG3, 0xbf, val << 6);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(enable_clk3a_output, S_IWUGO, NULL, 
+  store_enable_clk3a_output);
+
+
+
+static ssize_t store_enable_clkb1_output(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long flags;
+ unsigned long tmp;
+ unsigned char val;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+  SET_PORT_BITS(TLCLK_REG2, 0xf7, val << 3);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(enable_clkb1_output, S_IWUGO, NULL, 
+  store_enable_clkb1_output);
+
+
+static ssize_t store_enable_clka1_output(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long flags;
+ unsigned long tmp;
+ unsigned char val;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+  SET_PORT_BITS(TLCLK_REG2, 0xfb, val << 2);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(enable_clka1_output, S_IWUGO, NULL, 
+  store_enable_clka1_output);
+
+
+static ssize_t store_enable_clkb0_output(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long flags;
+ unsigned long tmp;
+ unsigned char val;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+  SET_PORT_BITS(TLCLK_REG2, 0xfd, val << 1);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(enable_clkb0_output, S_IWUGO, NULL, 
+  store_enable_clkb0_output);
+
+
+static ssize_t store_enable_clka0_output(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long flags;
+ unsigned long tmp;
+ unsigned char val;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG2, 0xfe, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(enable_clka0_output, S_IWUGO, NULL, 
+  store_enable_clka0_output);
+
+
+static ssize_t store_test_mode(struct class_device *d, const char * buf, 
+ size_t count)
+{
+ unsigned long flags;
+ unsigned long tmp;
+ unsigned char val;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG4, 0xfd, 2);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(test_mode, S_IWUGO, NULL, store_test_mode);
+
+static ssize_t store_select_amcb2_transmit_clock(struct class_device *d, 
+ const char * buf, size_t count)
+{
+ unsigned long flags;
+ unsigned long tmp;
+ unsigned char val;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+  if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
+   SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x28);
+   SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
+  } else if (val >= CLK_8_592MHz) {
+   SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x38);
+   switch (val) {
+   case CLK_8_592MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+    break;
+   case CLK_11_184MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
+    break;
+   case CLK_34_368MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
+    break;
+   case CLK_44_736MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+    break;
+   }
+  } else
+   SET_PORT_BITS(TLCLK_REG3, 0xc7, val << 3);
+  
+ 
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(select_amcb2_transmit_clock, S_IWUGO, NULL, 
+ store_select_amcb2_transmit_clock);
+
+static ssize_t store_select_amcb1_transmit_clock(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+  if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
+   SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x5);
+   SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
+  } else if (val >= CLK_8_592MHz) {
+   SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x7);
+   switch (val) {
+   case CLK_8_592MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+    break;
+   case CLK_11_184MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
+    break;
+   case CLK_34_368MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
+    break;
+   case CLK_44_736MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+    break;
+   }
+  } else
+   SET_PORT_BITS(TLCLK_REG3, 0xf8, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(select_amcb1_transmit_clock, S_IWUGO, NULL, 
+  store_select_amcb1_transmit_clock);
+
+static ssize_t store_select_redundant_clock(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG1, 0xfe, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(select_redundant_clock, S_IWUGO, NULL, 
+  store_select_redundant_clock);
+
+static ssize_t store_select_ref_frequency(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG1, 0xfd, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(select_ref_frequency, S_IWUGO, NULL, 
+  store_select_ref_frequency);
+
+static ssize_t store_filter_select(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG0, 0xfb, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(filter_select, S_IWUGO, NULL, store_filter_select);
+
+static ssize_t store_hardware_switching_mode(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG0, 0xbf, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(hardware_switching_mode, S_IWUGO, NULL, 
+  store_hardware_switching_mode);
+
+
+static ssize_t store_hardware_switching(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG0, 0x7f, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(hardware_switching, S_IWUGO, NULL, 
+  store_hardware_switching);
+
+static ssize_t store_refalign (struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
+ udelay(2);
+ SET_PORT_BITS(TLCLK_REG0, 0xf7, 0x08);
+ udelay(2);
+ SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(refalign, S_IWUGO, NULL, store_refalign);
+
+static ssize_t store_mode_select (struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG0, 0xcf, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(mode_select, S_IWUGO, NULL, store_mode_select);
+
+static ssize_t store_reset (struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG4, 0xfd, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(reset, S_IWUGO, NULL, store_reset);
+#endif /*  CONFIG_SYSFS */
+
+static int __init tlclk_init(void)
+{
+ int ret;
+#ifdef  CONFIG_SYSFS
+ struct class_device *class;
+#endif
+ ret = register_chrdev(tlclk_major, "telclock", &tlclk_fops);
+
+ if (ret < 0) {
+  printk(KERN_ERR "telclock: can't get major! %d\n", tlclk_major);
+  return ret;
+ }
+
+ alarm_events = kcalloc(sizeof(struct tlclk_alarms), 1, GFP_KERNEL);
+
+ if (!alarm_events)
+  goto out1;
+
+/* Read telecom clock IRQ number (Set by BIOS) */
+
+ if (!request_region(TLCLK_BASE, 8, "telclock")) {
+  printk(KERN_ERR "tlclk: request_region failed! 0x%X\n",
+   TLCLK_BASE);
+  goto out2;
+ }
+ telclk_interrupt = (inb(TLCLK_REG7) & 0x0f);
+
+ init_timer(&switchover_timer);
+ switchover_timer.function = switchover_timeout;
+ switchover_timer.data = 0;
+ 
+#ifdef  CONFIG_SYSFS
+ if( 0 > (ret = misc_register(&tlclk_miscdev )) ) {
+  printk(KERN_ERR" misc_register retruns %d \n", ret);
+  goto out2;
+ }
+ class = tlclk_miscdev.class;
+ class_device_create_file(class, &class_device_attr_current_ref);
+ class_device_create_file(class, &class_device_attr_interrupt_switch);
+ class_device_create_file(class, &class_device_attr_alarms);
+ class_device_create_file(class, &class_device_attr_enable_clk3b_output);
+ class_device_create_file(class, &class_device_attr_enable_clk3a_output);
+ class_device_create_file(class, &class_device_attr_enable_clkb1_output);
+ class_device_create_file(class, &class_device_attr_enable_clka1_output);
+ class_device_create_file(class, &class_device_attr_enable_clkb0_output);
+ class_device_create_file(class, &class_device_attr_enable_clka0_output);
+ class_device_create_file(class, &class_device_attr_test_mode);
+ class_device_create_file(class, &class_device_attr_select_amcb2_transmit_clock);
+ class_device_create_file(class, &class_device_attr_select_amcb1_transmit_clock);
+ class_device_create_file(class, &class_device_attr_select_redundant_clock);
+ class_device_create_file(class, &class_device_attr_select_ref_frequency);
+ class_device_create_file(class, &class_device_attr_filter_select);
+ class_device_create_file(class, &class_device_attr_hardware_switching_mode);
+ class_device_create_file(class, &class_device_attr_hardware_switching);
+ class_device_create_file(class, &class_device_attr_refalign);
+ class_device_create_file(class, &class_device_attr_mode_select);
+ class_device_create_file(class, &class_device_attr_reset);
+
+#endif /*  CONFIG_SYSFS */
+ return 0;
+out2:
+ kfree(alarm_events);
+out1:
+ return -EBUSY;
+}
+
+static void __exit tlclk_cleanup(void)
+{
+#ifdef  CONFIG_SYSFS
+ misc_deregister(&tlclk_miscdev);
+#endif
+ unregister_chrdev(tlclk_major, "telclock");
+
+ release_region(TLCLK_BASE, 8);
+ del_timer_sync(&switchover_timer);
+ kfree(alarm_events);
+
+}
+static void switchover_timeout(unsigned long data)
+{
+ if ((data & 1)) {
+  if ((inb(TLCLK_REG1) & 0x08) != (data & 0x08))
+   alarm_events->switchover_primary++;
+ } else {
+  if ((inb(TLCLK_REG1) & 0x08) != (data & 0x08))
+   alarm_events->switchover_secondary++;
+ }
+
+ /* Alarm processing is done, wake up read task */
+ del_timer(&switchover_timer);
+ got_event = 1;
+ wake_up(&wq);
+}
+
+irqreturn_t tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&event_lock, flags);
+ /* Read and clear interrupt events */
+ int_events = inb(TLCLK_REG6);
+
+ /* Primary_Los changed from 0 to 1 ? */
+ if (int_events & PRI_LOS_01_MASK) {
+  if (inb(TLCLK_REG2) & SEC_LOST_MASK)
+   alarm_events->lost_clocks++;
+  else
+   alarm_events->lost_primary_clock++;
+ }
+
+ /* Primary_Los changed from 1 to 0 ? */
+ if (int_events & PRI_LOS_10_MASK) {
+  alarm_events->primary_clock_back++;
+  SET_PORT_BITS(TLCLK_REG1, 0xFE, 1);
+ }
+ /* Secondary_Los changed from 0 to 1 ? */
+ if (int_events & SEC_LOS_01_MASK) {
+  if (inb(TLCLK_REG2) & PRI_LOST_MASK)
+   alarm_events->lost_clocks++;
+  else
+   alarm_events->lost_secondary_clock++;
+ }
+ /* Secondary_Los changed from 1 to 0 ? */
+ if (int_events & SEC_LOS_10_MASK) {
+  alarm_events->secondary_clock_back++;
+  SET_PORT_BITS(TLCLK_REG1, 0xFE, 0);
+ }
+ if (int_events & HOLDOVER_10_MASK)
+  alarm_events->pll_end_holdover++;
+
+ if (int_events & UNLOCK_01_MASK)
+  alarm_events->pll_lost_sync++;
+
+ if (int_events & UNLOCK_10_MASK)
+  alarm_events->pll_sync++;
+
+ /* Holdover changed from 0 to 1 ? */
+ if (int_events & HOLDOVER_01_MASK) {
+  alarm_events->pll_holdover++;
+
+  switchover_timer.expires = jiffies + 1; /* TIMEOUT in ~10ms */
+  switchover_timer.data = inb(TLCLK_REG1);
+  add_timer(&switchover_timer);
+ } else {
+  got_event = 1;
+  wake_up(&wq);
+ }
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return IRQ_HANDLED;
+}
+
+module_init(tlclk_init);
+module_exit(tlclk_cleanup);
diff -urN -X dontdiff linux-2.6.12.2/drivers/char/tlclk.h linux-2.6.12.2-tlclk/drivers/char/tlclk.h
--- linux-2.6.12.2/drivers/char/tlclk.h 1969-12-31 16:00:00.000000000 -0800
+++ linux-2.6.12.2-tlclk/drivers/char/tlclk.h 2005-08-07 10:39:55.000000000 -0700
@@ -0,0 +1,166 @@
+/*
+ * Telecom Clock driver for Wainwright board
+ *
+ * Copyright (C) 2005 Kontron Canada
+ *
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <sebastien.bouchard@ca.kontron.com>
+ *
+ */
+
+/* Ioctl definitions  */
+
+/* Use 0xA1 as magic number */
+#define TLCLK_IOC_MAGIC 0xA1
+
+/*Hardware Reset of the PLL */
+
+#define RESET_ON 0x00
+#define RESET_OFF 0x01
+#define IOCTL_RESET   _IO(TLCLK_IOC_MAGIC,  1)
+
+#define IOCTL_REFALIGN   _IO(TLCLK_IOC_MAGIC,  2)
+
+/* MODE SELECT */
+
+#define NORMAL_MODE  0x00
+#define HOLDOVER_MODE 0x10
+#define FREERUN_MODE 0x20
+
+#define IOCTL_MODE_SELECT  _IOR(TLCLK_IOC_MAGIC,  3, char)
+
+/* FILTER SELECT */
+
+#define FILTER_6HZ 0x04
+#define FILTER_12HZ 0x00
+
+#define IOCTL_FILTER_SELECT  _IOR(TLCLK_IOC_MAGIC,  4, char)
+
+/* SELECT REFERENCE FREQUENCY */
+
+#define REF_CLK1_8kHz  0x00
+#define REF_CLK2_19_44MHz 0x02
+
+#define IOCTL_SELECT_REF_FREQUENCY _IOR(TLCLK_IOC_MAGIC,  6, char)
+
+/* Select primary or secondary redundant clock */
+
+#define PRIMARY_CLOCK 0x00
+#define SECONDARY_CLOCK 0x01
+#define IOCTL_SELECT_REDUNDANT_CLOCK _IOR(TLCLK_IOC_MAGIC,  7, char)
+
+/* CLOCK TRANSMISSION DEFINE */
+
+#define CLK_8kHz 0xff
+#define CLK_16_384MHz 0xfb
+
+#define CLK_1_544MHz 0x00
+#define CLK_2_048MHz 0x01
+#define CLK_4_096MHz 0x02
+#define CLK_6_312MHz 0x03
+#define CLK_8_192MHz 0x04
+#define CLK_19_440MHz 0x06
+
+#define CLK_8_592MHz 0x08
+#define CLK_11_184MHz 0x09
+#define CLK_34_368MHz 0x0b
+#define CLK_44_736MHz 0x0a
+
+#define IOCTL_SELECT_AMCB1_TRANSMIT_CLOCK _IOR(TLCLK_IOC_MAGIC,  9, char)
+#define IOCTL_SELECT_AMCB2_TRANSMIT_CLOCK _IOR(TLCLK_IOC_MAGIC,  10, char)
+
+/* RECEIVED REFERENCE */
+
+#define AMC_B1 0
+#define AMC_B2 1
+
+#define IOCTL_SELECT_RECEIVED_REF_CLK3A _IOR(TLCLK_IOC_MAGIC,  11, char)
+#define IOCTL_SELECT_RECEIVED_REF_CLK3B _IOR(TLCLK_IOC_MAGIC,  12, char)
+
+/* OEM COMMAND - NOT IN FINAL VERSION */
+
+#define IOCTL_TEST_MODE _IO(TLCLK_IOC_MAGIC,  13)
+
+/* HARDWARE SWITCHING DEFINE */
+
+#define HW_ENABLE 0x80
+#define HW_DISABLE 0x00
+
+#define IOCTL_HARDWARE_SWITCHING _IOR(TLCLK_IOC_MAGIC,  14, char)
+
+/* HARDWARE SWITCHING MODE DEFINE */
+
+#define PLL_HOLDOVER 0x40
+#define LOST_CLOCK 0x00
+
+#define IOCTL_HARDWARE_SWITCHING_MODE _IOR(TLCLK_IOC_MAGIC,  15, char)
+
+/* CLOCK OUTPUT DEFINE */
+
+#define IOCTL_ENABLE_CLKA0_OUTPUT _IOR(TLCLK_IOC_MAGIC,  16, char)
+#define IOCTL_ENABLE_CLKB0_OUTPUT _IOR(TLCLK_IOC_MAGIC,  17, char)
+#define IOCTL_ENABLE_CLKA1_OUTPUT _IOR(TLCLK_IOC_MAGIC,  18, char)
+#define IOCTL_ENABLE_CLKB1_OUTPUT _IOR(TLCLK_IOC_MAGIC,  19, char)
+
+#define IOCTL_ENABLE_CLK3A_OUTPUT _IOR(TLCLK_IOC_MAGIC,  20, char)
+#define IOCTL_ENABLE_CLK3B_OUTPUT _IOR(TLCLK_IOC_MAGIC,  21, char)
+
+/* ALARMS DEFINE */
+
+#define UNLOCK_MASK 0x10
+#define HOLDOVER_MASK 0x20
+#define SEC_LOST_MASK 0x40
+#define PRI_LOST_MASK 0x80
+
+#define IOCTL_READ_ALARMS  _IO(TLCLK_IOC_MAGIC,  22)
+
+/* INTERRUPT CAUSE DEFINE */
+
+#define PRI_LOS_01_MASK  0x01
+#define PRI_LOS_10_MASK  0x02
+
+#define SEC_LOS_01_MASK  0x04
+#define SEC_LOS_10_MASK  0x08
+
+#define HOLDOVER_01_MASK 0x10
+#define HOLDOVER_10_MASK 0x20
+
+#define UNLOCK_01_MASK  0x40
+#define UNLOCK_10_MASK  0x80
+
+#define IOCTL_READ_INTERRUPT_SWITCH _IO(TLCLK_IOC_MAGIC,  23)
+
+#define IOCTL_READ_CURRENT_REF  _IO(TLCLK_IOC_MAGIC,  25)
+
+/* MAX NUMBER OF IOCTL */
+#define TLCLK_IOC_MAXNR 25
+
+struct tlclk_alarms {
+ __u32 lost_clocks;
+ __u32 lost_primary_clock;
+ __u32 lost_secondary_clock;
+ __u32 primary_clock_back;
+ __u32 secondary_clock_back;
+ __u32 switchover_primary;
+ __u32 switchover_secondary;
+ __u32 pll_holdover;
+ __u32 pll_end_holdover;
+ __u32 pll_lost_sync;
+ __u32 pll_sync;
+};
 

-- 
--mgross
BTW: This may or may not be the opinion of my employer, more likely not.  

Re: Patch of a new driver for kernel 2.4.x that need review

[Pekka Enberg]

On 8/8/05, Mark Gross <mgross@linux.intel.com> wrote:
> Please tell me what you think :)

The formatting seems completely messed up presumably because of your
email client.

On 8/8/05, Mark Gross <mgross@linux.intel.com> wrote:
> + alarm_events = kcalloc(sizeof(struct tlclk_alarms), 1, GFP_KERNEL);

The first argument to kcalloc() is number of elements and the second
one is size of one element. In this case, however, use the new
kzalloc() which can be found in 2.6.13-rc5-mm1.

                                                   Pekka

Re: Patch of a new driver for kernel 2.4.x that need review

[Mark Gross]

On Monday 08 August 2005 08:35, Mark Gross wrote:
> On Wednesday 06 July 2005 14:14, Mark Gross wrote:
> > On Wednesday 22 June 2005 08:12, Bouchard, Sebastien wrote:
> > > Hi,
> > >
> > > Here is a driver (only for 2.4.x) I've done to provide support of a
> > > hardware module (available on some ATCA board) used with telecom expension
> > > card on the new ATCA platform. This module provide redundant reference
> > > clock for telecom hardware with alarm handling when there is a new event
> > > (ex.: one of the ref clock is gone).
> > > This char driver provide IOCTL for configuration of this module and
> > > interrupt handler for processing alarm events.
> > >
> > > I send this driver so people in this mailing list can do a review of the
> > > code.
> > >
> > > Please reply me directly to my email, i'm not subscribed to the mailing
> > > list.
> > >
> > > Thanks
> > > Sebastien Bouchard
> > > Software designer
> > > Kontron Canada Inc.
> > > <mailto:sebastien.bouchard@ca.kontron.com>
> > > <http://www.kontron.com/>
> > 
> > I'm helping out a bit with the maintaining of this driver for Sebastien.  
> > The following is a 2.6.12 port of Sebastien's 2.4 driver.  
> > 
> > --mgross
> 
> The following is an update to the earlier 2.6 driver that uses a sysfs interface 
> to implement the IOCTL functions.  I put the attributes under /sys/class/misc/tlclk.
> 
> I can't say I'm a believer in the "goodness" of using sysfs over the IOCTL's, as it 
> added quite a bit of code bloat to this driver, but hay it should work well enough anyway.
> 
> Also, note this device, is accessed / controlled via the FPGA on the ATCA, its function 
> is to synchronize signaling hardware across blades in an ATCA Chassis.  It tends 
> to not talk to the OS. 
> 
> Please tell me what you think :)
Again but with email word wrap turned off :(
Also fixes my miss use of kcalloc parameter list.

diff -urN -X dontdiff linux-2.6.12.3/drivers/char/Kconfig linux-2.6.12.3-tlclk/drivers/char/Kconfig
--- linux-2.6.12.3/drivers/char/Kconfig 2005-07-15 14:18:57.000000000 -0700
+++ linux-2.6.12.3-tlclk/drivers/char/Kconfig 2005-08-08 08:22:45.000000000 -0700
@@ -998,5 +998,15 @@
 
 source "drivers/char/tpm/Kconfig"
 
+config TELCLOCK
+ tristate "Telecom clock driver for ATCA"
+ depends on EXPERIMENTAL
+ default n
+ help
+   The telecom clock device allows direct userspace access to the
+   configuration of the telecom clock configuration settings.
+   This device is used for hardware synchronization across the ATCA
+   backplane fabric.
+
 endmenu
 
diff -urN -X dontdiff linux-2.6.12.3/drivers/char/Makefile linux-2.6.12.3-tlclk/drivers/char/Makefile
--- linux-2.6.12.3/drivers/char/Makefile 2005-07-15 14:18:57.000000000 -0700
+++ linux-2.6.12.3-tlclk/drivers/char/Makefile 2005-08-08 08:22:45.000000000 -0700
@@ -81,6 +81,7 @@
 obj-$(CONFIG_NWFLASH) += nwflash.o
 obj-$(CONFIG_SCx200_GPIO) += scx200_gpio.o
 obj-$(CONFIG_TANBAC_TB0219) += tb0219.o
+obj-$(CONFIG_TELCLOCK) += tlclk.o
 
 obj-$(CONFIG_WATCHDOG) += watchdog/
 obj-$(CONFIG_MWAVE) += mwave/
diff -urN -X dontdiff linux-2.6.12.3/drivers/char/tlclk.c linux-2.6.12.3-tlclk/drivers/char/tlclk.c
--- linux-2.6.12.3/drivers/char/tlclk.c 1969-12-31 16:00:00.000000000 -0800
+++ linux-2.6.12.3-tlclk/drivers/char/tlclk.c 2005-08-09 09:37:58.000000000 -0700
@@ -0,0 +1,933 @@
+/*
+ * Telecom Clock driver for Wainwright board
+ *
+ * Copyright (C) 2005 Kontron Canada
+ *
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <sebastien.bouchard@ca.kontron.com>
+ * 
+ * 2.6 driver version being maintained by <mark.gross@intel.com>
+ *
+ * Description : This is the TELECOM CLOCK module driver for the ATCA platform.
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/sched.h>
+#include <linux/kernel.h> /* printk() */
+#include <linux/fs.h>  /* everything... */
+#include <linux/errno.h> /* error codes */
+#include <linux/delay.h> /* udelay */
+#include <linux/slab.h>
+#include <linux/ioport.h>
+#include <linux/interrupt.h>
+#include <linux/spinlock.h>
+#include <linux/timer.h>
+#include <linux/sysfs.h>
+#include <linux/device.h>
+#include <linux/miscdevice.h>
+#include <asm/io.h>  /* inb/outb */
+#include <asm/uaccess.h>
+
+#include "tlclk.h"  /* TELECOM IOCTL DEFINE */
+
+MODULE_AUTHOR("Sebastien Bouchard <sebastien.bouchard@ca.kontron.com>");
+MODULE_LICENSE("GPL");
+
+/* Telecom clock I/O register definition */
+#define TLCLK_BASE 0xa08
+#define TLCLK_REG0 TLCLK_BASE
+#define TLCLK_REG1 (TLCLK_BASE+1)
+#define TLCLK_REG2 (TLCLK_BASE+2)
+#define TLCLK_REG3 (TLCLK_BASE+3)
+#define TLCLK_REG4 (TLCLK_BASE+4)
+#define TLCLK_REG5 (TLCLK_BASE+5)
+#define TLCLK_REG6 (TLCLK_BASE+6)
+#define TLCLK_REG7 (TLCLK_BASE+7)
+
+#define SET_PORT_BITS(port, mask, val) outb(((inb(port) & mask) | val), port)
+
+/* 0 = Dynamic allocation of the major device number */
+#define TLCLK_MAJOR 252
+
+/*
+uncomment to include classic IOCTL's 
+#define TLCLK_IOCTL
+*/
+
+/* sysFS interface definition:
+Uppon loading the driver will create a sysfs directory under class/misc/tlclk.
+
+This directory exports the following interfaces.
+alarms    :
+current_ref   :
+enable_clk3a_output  :
+enable_clk3b_output  :
+enable_clka0_output  :
+enable_clka1_output  :
+enable_clkb0_output  :
+enable_clkb1_output  :
+filter_select   :
+hardware_switching  :
+hardware_switching_mode  :
+interrupt_switch  :
+mode_select   :
+refalign   :
+reset    :
+select_amcb1_transmit_clock :
+select_amcb2_transmit_clock :
+select_redundant_clock  :
+select_ref_frequency  :
+test_mode   :
+
+All sysfs interaces are integers in hex format, i.e echo 99 > refalign
+has the same effect as echo 0x99 > refalign.
+
+*/
+
+#if CONFIG_DEBUG_KERNEL 
+#define debug_printk( args... ) printk( args)
+#else
+#define debug_printk( args... )
+#endif
+
+
+static unsigned int telclk_interrupt;
+
+static int int_events;  /* Event that generate a interrupt */
+static int got_event;  /* if events processing have been done */
+
+static struct timer_list switchover_timer;
+
+static struct tlclk_alarms *alarm_events;
+
+DEFINE_SPINLOCK(event_lock);
+
+static int tlclk_major = TLCLK_MAJOR;
+
+static void switchover_timeout(unsigned long data);
+irqreturn_t tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs);
+
+DECLARE_WAIT_QUEUE_HEAD(wq);
+
+#ifdef TLCLK_IOCTL
+static int
+tlclk_ioctl(struct inode *inode,
+     struct file *filp, unsigned int cmd, unsigned long arg)
+{
+ unsigned long flags;
+ unsigned char val;
+ int ret_val = 0;
+
+ val = (unsigned char)arg;
+ if (_IOC_TYPE(cmd) != TLCLK_IOC_MAGIC)
+  return -ENOTTY;
+
+ if (_IOC_NR(cmd) > TLCLK_IOC_MAXNR)
+  return -ENOTTY;
+
+ spin_lock_irqsave(&event_lock, flags);
+ switch (cmd) {
+ case IOCTL_RESET:
+  SET_PORT_BITS(TLCLK_REG4, 0xfd, val);
+  break;
+ case IOCTL_MODE_SELECT:
+  SET_PORT_BITS(TLCLK_REG0, 0xcf, val);
+  break;
+ case IOCTL_REFALIGN:
+  /* GENERATING 0 to 1 transistion */
+  SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
+  udelay(2);
+  SET_PORT_BITS(TLCLK_REG0, 0xf7, 0x08);
+  udelay(2);
+  SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
+  break;
+ case IOCTL_HARDWARE_SWITCHING:
+  SET_PORT_BITS(TLCLK_REG0, 0x7f, val);
+  break;
+ case IOCTL_HARDWARE_SWITCHING_MODE:
+  SET_PORT_BITS(TLCLK_REG0, 0xbf, val);
+  break;
+ case IOCTL_FILTER_SELECT:
+  SET_PORT_BITS(TLCLK_REG0, 0xfb, val);
+  break;
+ case IOCTL_SELECT_REF_FREQUENCY:
+  SET_PORT_BITS(TLCLK_REG1, 0xfd, val);
+  break;
+ case IOCTL_SELECT_REDUNDANT_CLOCK:
+  SET_PORT_BITS(TLCLK_REG1, 0xfe, val);
+  break;
+ case IOCTL_SELECT_AMCB1_TRANSMIT_CLOCK:
+  if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
+   SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x5);
+   SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
+  } else if (val >= CLK_8_592MHz) {
+   SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x7);
+   switch (val) {
+   case CLK_8_592MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+    break;
+   case CLK_11_184MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
+    break;
+   case CLK_34_368MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
+    break;
+   case CLK_44_736MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+    break;
+   }
+  } else
+   SET_PORT_BITS(TLCLK_REG3, 0xf8, val);
+  break;
+ case IOCTL_SELECT_AMCB2_TRANSMIT_CLOCK:
+  if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
+   SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x28);
+   SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
+  } else if (val >= CLK_8_592MHz) {
+   SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x38);
+   switch (val) {
+   case CLK_8_592MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+    break;
+   case CLK_11_184MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
+    break;
+   case CLK_34_368MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
+    break;
+   case CLK_44_736MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+    break;
+   }
+  } else
+   SET_PORT_BITS(TLCLK_REG3, 0xc7, val << 3);
+  break;
+ case IOCTL_TEST_MODE:
+  SET_PORT_BITS(TLCLK_REG4, 0xfd, 2);
+  break;
+ case IOCTL_ENABLE_CLKA0_OUTPUT:
+  SET_PORT_BITS(TLCLK_REG2, 0xfe, val);
+  break;
+ case IOCTL_ENABLE_CLKB0_OUTPUT:
+  SET_PORT_BITS(TLCLK_REG2, 0xfd, val << 1);
+  break;
+ case IOCTL_ENABLE_CLKA1_OUTPUT:
+  SET_PORT_BITS(TLCLK_REG2, 0xfb, val << 2);
+  break;
+ case IOCTL_ENABLE_CLKB1_OUTPUT:
+  SET_PORT_BITS(TLCLK_REG2, 0xf7, val << 3);
+  break;
+ case IOCTL_ENABLE_CLK3A_OUTPUT:
+  SET_PORT_BITS(TLCLK_REG3, 0xbf, val << 6);
+  break;
+ case IOCTL_ENABLE_CLK3B_OUTPUT:
+  SET_PORT_BITS(TLCLK_REG3, 0x7f, val << 7);
+  break;
+ case IOCTL_READ_ALARMS:
+  ret_val = (inb(TLCLK_REG2) & 0xf0);
+  break;
+ case IOCTL_READ_INTERRUPT_SWITCH:
+  ret_val = inb(TLCLK_REG6);
+  break;
+ case IOCTL_READ_CURRENT_REF:
+  ret_val = ((inb(TLCLK_REG1) & 0x08) >> 3);
+  break;
+ }
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return ret_val;
+}
+#endif
+
+
+static int tlclk_open(struct inode *inode, struct file *filp)
+{
+ int result;
+
+ /* Make sure there is no interrupt pending while 
+  * initialising interrupt handler */
+ inb(TLCLK_REG6);
+
+ /* This device is wired through the FPGA IO space of the ATCA blade 
+  * we can't share this IRQ */
+ result = request_irq(telclk_interrupt, &tlclk_interrupt,
+        SA_INTERRUPT, "telclock", tlclk_interrupt);
+ if (result == -EBUSY) {
+  printk(KERN_ERR "telclock: Interrupt can't be reserved!\n");
+  return -EBUSY;
+ }
+ inb(TLCLK_REG6); /* Clear interrupt events */
+
+ return 0;
+}
+
+static int tlclk_release(struct inode *inode, struct file *filp)
+{
+ free_irq(telclk_interrupt, tlclk_interrupt);
+
+ return 0;
+}
+
+ssize_t
+tlclk_read(struct file * filp, char __user * buf, size_t count, loff_t * f_pos)
+{
+ int count0 = sizeof(struct tlclk_alarms);
+
+ wait_event_interruptible(wq, got_event);
+ if (copy_to_user(buf, alarm_events, sizeof(struct tlclk_alarms)))
+  return -EFAULT;
+
+ memset(alarm_events, 0, sizeof(struct tlclk_alarms));
+ got_event = 0;
+
+ return count0;
+}
+
+ssize_t
+tlclk_write(struct file * filp, const char __user * buf, size_t count,
+     loff_t * f_pos)
+{
+ return 0;
+}
+
+static struct file_operations tlclk_fops = {
+ .read = tlclk_read,
+ .write = tlclk_write,
+#ifdef TLCLK_IOCTL
+ .ioctl = tlclk_ioctl,
+#endif
+ .open = tlclk_open,
+ .release = tlclk_release,
+
+};
+
+#ifdef CONFIG_SYSFS
+static struct miscdevice tlclk_miscdev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "tlclk",
+ .fops = &tlclk_fops,
+};
+
+static ssize_t show_current_ref(struct class_device *d, char * buf)
+{
+ unsigned long ret_val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&event_lock, flags);
+  ret_val = ((inb(TLCLK_REG1) & 0x08) >> 3);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return sprintf(buf, "0x%lX\n", ret_val);
+}
+
+static CLASS_DEVICE_ATTR(current_ref, S_IRUGO, show_current_ref, NULL);
+
+
+static ssize_t show_interrupt_switch(struct class_device *d, char * buf)
+{
+ unsigned long ret_val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&event_lock, flags);
+  ret_val = inb(TLCLK_REG6);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return sprintf(buf, "0x%lX\n", ret_val);
+}
+
+static CLASS_DEVICE_ATTR(interrupt_switch, S_IRUGO, 
+  show_interrupt_switch, NULL);
+
+
+static ssize_t show_alarms(struct class_device *d, char * buf)
+{
+ unsigned long ret_val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&event_lock, flags);
+  ret_val = (inb(TLCLK_REG2) & 0xf0);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return sprintf(buf, "0x%lX\n", ret_val);
+}
+
+static CLASS_DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
+
+
+static ssize_t store_enable_clk3b_output(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+  SET_PORT_BITS(TLCLK_REG3, 0x7f, val << 7);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(enable_clk3b_output, S_IWUGO, NULL, 
+  store_enable_clk3b_output);
+
+
+
+static ssize_t store_enable_clk3a_output(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long flags;
+ unsigned long tmp;
+ unsigned char val;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+  SET_PORT_BITS(TLCLK_REG3, 0xbf, val << 6);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(enable_clk3a_output, S_IWUGO, NULL, 
+  store_enable_clk3a_output);
+
+
+
+static ssize_t store_enable_clkb1_output(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long flags;
+ unsigned long tmp;
+ unsigned char val;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+  SET_PORT_BITS(TLCLK_REG2, 0xf7, val << 3);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(enable_clkb1_output, S_IWUGO, NULL, 
+  store_enable_clkb1_output);
+
+
+static ssize_t store_enable_clka1_output(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long flags;
+ unsigned long tmp;
+ unsigned char val;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+  SET_PORT_BITS(TLCLK_REG2, 0xfb, val << 2);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(enable_clka1_output, S_IWUGO, NULL, 
+  store_enable_clka1_output);
+
+
+static ssize_t store_enable_clkb0_output(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long flags;
+ unsigned long tmp;
+ unsigned char val;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+  SET_PORT_BITS(TLCLK_REG2, 0xfd, val << 1);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(enable_clkb0_output, S_IWUGO, NULL, 
+  store_enable_clkb0_output);
+
+
+static ssize_t store_enable_clka0_output(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long flags;
+ unsigned long tmp;
+ unsigned char val;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG2, 0xfe, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(enable_clka0_output, S_IWUGO, NULL, 
+  store_enable_clka0_output);
+
+
+static ssize_t store_test_mode(struct class_device *d, const char * buf, 
+ size_t count)
+{
+ unsigned long flags;
+ unsigned long tmp;
+ unsigned char val;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG4, 0xfd, 2);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(test_mode, S_IWUGO, NULL, store_test_mode);
+
+static ssize_t store_select_amcb2_transmit_clock(struct class_device *d, 
+ const char * buf, size_t count)
+{
+ unsigned long flags;
+ unsigned long tmp;
+ unsigned char val;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+  if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
+   SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x28);
+   SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
+  } else if (val >= CLK_8_592MHz) {
+   SET_PORT_BITS(TLCLK_REG3, 0xc7, 0x38);
+   switch (val) {
+   case CLK_8_592MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+    break;
+   case CLK_11_184MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
+    break;
+   case CLK_34_368MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
+    break;
+   case CLK_44_736MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+    break;
+   }
+  } else
+   SET_PORT_BITS(TLCLK_REG3, 0xc7, val << 3);
+  
+ 
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(select_amcb2_transmit_clock, S_IWUGO, NULL, 
+ store_select_amcb2_transmit_clock);
+
+static ssize_t store_select_amcb1_transmit_clock(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+  if ((val == CLK_8kHz) || (val == CLK_16_384MHz)) {
+   SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x5);
+   SET_PORT_BITS(TLCLK_REG1, 0xfb, ~val);
+  } else if (val >= CLK_8_592MHz) {
+   SET_PORT_BITS(TLCLK_REG3, 0xf8, 0x7);
+   switch (val) {
+   case CLK_8_592MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 1);
+    break;
+   case CLK_11_184MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 0);
+    break;
+   case CLK_34_368MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 3);
+    break;
+   case CLK_44_736MHz:
+    SET_PORT_BITS(TLCLK_REG0, 0xfc, 2);
+    break;
+   }
+  } else
+   SET_PORT_BITS(TLCLK_REG3, 0xf8, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(select_amcb1_transmit_clock, S_IWUGO, NULL, 
+  store_select_amcb1_transmit_clock);
+
+static ssize_t store_select_redundant_clock(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG1, 0xfe, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(select_redundant_clock, S_IWUGO, NULL, 
+  store_select_redundant_clock);
+
+static ssize_t store_select_ref_frequency(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG1, 0xfd, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(select_ref_frequency, S_IWUGO, NULL, 
+  store_select_ref_frequency);
+
+static ssize_t store_filter_select(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG0, 0xfb, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(filter_select, S_IWUGO, NULL, store_filter_select);
+
+static ssize_t store_hardware_switching_mode(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG0, 0xbf, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(hardware_switching_mode, S_IWUGO, NULL, 
+  store_hardware_switching_mode);
+
+
+static ssize_t store_hardware_switching(struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG0, 0x7f, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(hardware_switching, S_IWUGO, NULL, 
+  store_hardware_switching);
+
+static ssize_t store_refalign (struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
+ udelay(2);
+ SET_PORT_BITS(TLCLK_REG0, 0xf7, 0x08);
+ udelay(2);
+ SET_PORT_BITS(TLCLK_REG0, 0xf7, 0);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(refalign, S_IWUGO, NULL, store_refalign);
+
+static ssize_t store_mode_select (struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG0, 0xcf, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(mode_select, S_IWUGO, NULL, store_mode_select);
+
+static ssize_t store_reset (struct class_device *d, 
+  const char * buf, size_t count)
+{
+ unsigned long tmp;
+ unsigned char val;
+ unsigned long flags;
+
+ sscanf(buf, "%lX", &tmp);
+ debug_printk(KERN_ERR "tmp = 0x%lX \n", tmp);
+
+ val = (unsigned char)tmp;
+ spin_lock_irqsave(&event_lock, flags);
+ SET_PORT_BITS(TLCLK_REG4, 0xfd, val);
+ spin_unlock_irqrestore(&event_lock, flags);
+
+ return strnlen(buf, count);
+}
+
+static CLASS_DEVICE_ATTR(reset, S_IWUGO, NULL, store_reset);
+#endif /*  CONFIG_SYSFS */
+
+static int __init tlclk_init(void)
+{
+ int ret;
+#ifdef  CONFIG_SYSFS
+ struct class_device *class;
+#endif
+ ret = register_chrdev(tlclk_major, "telclock", &tlclk_fops);
+
+ if (ret < 0) {
+  printk(KERN_ERR "telclock: can't get major! %d\n", tlclk_major);
+  return ret;
+ }
+
+ alarm_events = kcalloc(1, sizeof(struct tlclk_alarms), GFP_KERNEL);
+
+ if (!alarm_events)
+  goto out1;
+
+/* Read telecom clock IRQ number (Set by BIOS) */
+
+ if (!request_region(TLCLK_BASE, 8, "telclock")) {
+  printk(KERN_ERR "tlclk: request_region failed! 0x%X\n",
+   TLCLK_BASE);
+  goto out2;
+ }
+ telclk_interrupt = (inb(TLCLK_REG7) & 0x0f);
+
+ init_timer(&switchover_timer);
+ switchover_timer.function = switchover_timeout;
+ switchover_timer.data = 0;
+ 
+#ifdef  CONFIG_SYSFS
+ if( 0 > (ret = misc_register(&tlclk_miscdev )) ) {
+  printk(KERN_ERR" misc_register retruns %d \n", ret);
+  goto out2;
+ }
+ class = tlclk_miscdev.class;
+ class_device_create_file(class, &class_device_attr_current_ref);
+ class_device_create_file(class, &class_device_attr_interrupt_switch);
+ class_device_create_file(class, &class_device_attr_alarms);
+ class_device_create_file(class, &class_device_attr_enable_clk3b_output);
+ class_device_create_file(class, &class_device_attr_enable_clk3a_output);
+ class_device_create_file(class, &class_device_attr_enable_clkb1_output);
+ class_device_create_file(class, &class_device_attr_enable_clka1_output);
+ class_device_create_file(class, &class_device_attr_enable_clkb0_output);
+ class_device_create_file(class, &class_device_attr_enable_clka0_output);
+ class_device_create_file(class, &class_device_attr_test_mode);
+ class_device_create_file(class, &class_device_attr_select_amcb2_transmit_clock);
+ class_device_create_file(class, &class_device_attr_select_amcb1_transmit_clock);
+ class_device_create_file(class, &class_device_attr_select_redundant_clock);
+ class_device_create_file(class, &class_device_attr_select_ref_frequency);
+ class_device_create_file(class, &class_device_attr_filter_select);
+ class_device_create_file(class, &class_device_attr_hardware_switching_mode);
+ class_device_create_file(class, &class_device_attr_hardware_switching);
+ class_device_create_file(class, &class_device_attr_refalign);
+ class_device_create_file(class, &class_device_attr_mode_select);
+ class_device_create_file(class, &class_device_attr_reset);
+
+#endif /*  CONFIG_SYSFS */
+ return 0;
+out2:
+ kfree(alarm_events);
+out1:
+ return -EBUSY;
+}
+
+static void __exit tlclk_cleanup(void)
+{
+#ifdef  CONFIG_SYSFS
+ misc_deregister(&tlclk_miscdev);
+#endif
+ unregister_chrdev(tlclk_major, "telclock");
+
+ release_region(TLCLK_BASE, 8);
+ del_timer_sync(&switchover_timer);
+ kfree(alarm_events);
+
+}
+static void switchover_timeout(unsigned long data)
+{
+ if ((data & 1)) {
+  if ((inb(TLCLK_REG1) & 0x08) != (data & 0x08))
+   alarm_events->switchover_primary++;
+ } else {
+  if ((inb(TLCLK_REG1) & 0x08) != (data & 0x08))
+   alarm_events->switchover_secondary++;
+ }
+
+ /* Alarm processing is done, wake up read task */
+ del_timer(&switchover_timer);
+ got_event = 1;
+ wake_up(&wq);
+}
+
+irqreturn_t tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&event_lock, flags);
+ /* Read and clear interrupt events */
+ int_events = inb(TLCLK_REG6);
+
+ /* Primary_Los changed from 0 to 1 ? */
+ if (int_events & PRI_LOS_01_MASK) {
+  if (inb(TLCLK_REG2) & SEC_LOST_MASK)
+   alarm_events->lost_clocks++;
+  else
+   alarm_events->lost_primary_clock++;
+ }
+
+ /* Primary_Los changed from 1 to 0 ? */
+ if (int_events & PRI_LOS_10_MASK) {
+  alarm_events->primary_clock_back++;
+  SET_PORT_BITS(TLCLK_REG1, 0xFE, 1);
+ }
+ /* Secondary_Los changed from 0 to 1 ? */
+ if (int_events & SEC_LOS_01_MASK) {
+  if (inb(TLCLK_REG2) & PRI_LOST_MASK)
+   alarm_events->lost_clocks++;
+  else
+   alarm_events->lost_secondary_clock++;
+ }
+ /* Secondary_Los changed from 1 to 0 ? */
+ if (int_events & SEC_LOS_10_MASK) {
+  alarm_events->secondary_clock_back++;
+  SET_PORT_BITS(TLCLK_REG1, 0xFE, 0);
+ }
+ if (int_events & HOLDOVER_10_MASK)
+  alarm_events->pll_end_holdover++;
+
+ if (int_events & UNLOCK_01_MASK)
+  alarm_events->pll_lost_sync++;
+
+ if (int_events & UNLOCK_10_MASK)
+  alarm_events->pll_sync++;
+
+ /* Holdover changed from 0 to 1 ? */
+ if (int_events & HOLDOVER_01_MASK) {
+  alarm_events->pll_holdover++;
+
+  switchover_timer.expires = jiffies + 1; /* TIMEOUT in ~10ms */
+  switchover_timer.data = inb(TLCLK_REG1);
+  add_timer(&switchover_timer);
+ } else {
+		got_event = 1;
+		wake_up(&wq);
+	}
+	spin_unlock_irqrestore(&event_lock, flags);
+
+	return IRQ_HANDLED;
+}
+
+module_init(tlclk_init);
+module_exit(tlclk_cleanup);
diff -urN -X dontdiff linux-2.6.12.3/drivers/char/tlclk.h linux-2.6.12.3-tlclk/drivers/char/tlclk.h
--- linux-2.6.12.3/drivers/char/tlclk.h 1969-12-31 16:00:00.000000000 -0800
+++ linux-2.6.12.3-tlclk/drivers/char/tlclk.h 2005-08-08 08:22:45.000000000 -0700
@@ -0,0 +1,166 @@
+/*
+ * Telecom Clock driver for Wainwright board
+ *
+ * Copyright (C) 2005 Kontron Canada
+ *
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Send feedback to <sebastien.bouchard@ca.kontron.com>
+ *
+ */
+
+/* Ioctl definitions  */
+
+/* Use 0xA1 as magic number */
+#define TLCLK_IOC_MAGIC 0xA1
+
+/*Hardware Reset of the PLL */
+
+#define RESET_ON 0x00
+#define RESET_OFF 0x01
+#define IOCTL_RESET   _IO(TLCLK_IOC_MAGIC,  1)
+
+#define IOCTL_REFALIGN   _IO(TLCLK_IOC_MAGIC,  2)
+
+/* MODE SELECT */
+
+#define NORMAL_MODE  0x00
+#define HOLDOVER_MODE 0x10
+#define FREERUN_MODE 0x20
+
+#define IOCTL_MODE_SELECT  _IOR(TLCLK_IOC_MAGIC,  3, char)
+
+/* FILTER SELECT */
+
+#define FILTER_6HZ 0x04
+#define FILTER_12HZ 0x00
+
+#define IOCTL_FILTER_SELECT  _IOR(TLCLK_IOC_MAGIC,  4, char)
+
+/* SELECT REFERENCE FREQUENCY */
+
+#define REF_CLK1_8kHz  0x00
+#define REF_CLK2_19_44MHz 0x02
+
+#define IOCTL_SELECT_REF_FREQUENCY _IOR(TLCLK_IOC_MAGIC,  6, char)
+
+/* Select primary or secondary redundant clock */
+
+#define PRIMARY_CLOCK 0x00
+#define SECONDARY_CLOCK 0x01
+#define IOCTL_SELECT_REDUNDANT_CLOCK _IOR(TLCLK_IOC_MAGIC,  7, char)
+
+/* CLOCK TRANSMISSION DEFINE */
+
+#define CLK_8kHz 0xff
+#define CLK_16_384MHz 0xfb
+
+#define CLK_1_544MHz 0x00
+#define CLK_2_048MHz 0x01
+#define CLK_4_096MHz 0x02
+#define CLK_6_312MHz 0x03
+#define CLK_8_192MHz 0x04
+#define CLK_19_440MHz 0x06
+
+#define CLK_8_592MHz 0x08
+#define CLK_11_184MHz 0x09
+#define CLK_34_368MHz 0x0b
+#define CLK_44_736MHz 0x0a
+
+#define IOCTL_SELECT_AMCB1_TRANSMIT_CLOCK _IOR(TLCLK_IOC_MAGIC,  9, char)
+#define IOCTL_SELECT_AMCB2_TRANSMIT_CLOCK _IOR(TLCLK_IOC_MAGIC,  10, char)
+
+/* RECEIVED REFERENCE */
+
+#define AMC_B1 0
+#define AMC_B2 1
+
+#define IOCTL_SELECT_RECEIVED_REF_CLK3A _IOR(TLCLK_IOC_MAGIC,  11, char)
+#define IOCTL_SELECT_RECEIVED_REF_CLK3B _IOR(TLCLK_IOC_MAGIC,  12, char)
+
+/* OEM COMMAND - NOT IN FINAL VERSION */
+
+#define IOCTL_TEST_MODE _IO(TLCLK_IOC_MAGIC,  13)
+
+/* HARDWARE SWITCHING DEFINE */
+
+#define HW_ENABLE 0x80
+#define HW_DISABLE 0x00
+
+#define IOCTL_HARDWARE_SWITCHING _IOR(TLCLK_IOC_MAGIC,  14, char)
+
+/* HARDWARE SWITCHING MODE DEFINE */
+
+#define PLL_HOLDOVER 0x40
+#define LOST_CLOCK 0x00
+
+#define IOCTL_HARDWARE_SWITCHING_MODE _IOR(TLCLK_IOC_MAGIC,  15, char)
+
+/* CLOCK OUTPUT DEFINE */
+
+#define IOCTL_ENABLE_CLKA0_OUTPUT _IOR(TLCLK_IOC_MAGIC,  16, char)
+#define IOCTL_ENABLE_CLKB0_OUTPUT _IOR(TLCLK_IOC_MAGIC,  17, char)
+#define IOCTL_ENABLE_CLKA1_OUTPUT _IOR(TLCLK_IOC_MAGIC,  18, char)
+#define IOCTL_ENABLE_CLKB1_OUTPUT _IOR(TLCLK_IOC_MAGIC,  19, char)
+
+#define IOCTL_ENABLE_CLK3A_OUTPUT _IOR(TLCLK_IOC_MAGIC,  20, char)
+#define IOCTL_ENABLE_CLK3B_OUTPUT _IOR(TLCLK_IOC_MAGIC,  21, char)
+
+/* ALARMS DEFINE */
+
+#define UNLOCK_MASK 0x10
+#define HOLDOVER_MASK 0x20
+#define SEC_LOST_MASK 0x40
+#define PRI_LOST_MASK 0x80
+
+#define IOCTL_READ_ALARMS  _IO(TLCLK_IOC_MAGIC,  22)
+
+/* INTERRUPT CAUSE DEFINE */
+
+#define PRI_LOS_01_MASK  0x01
+#define PRI_LOS_10_MASK  0x02
+
+#define SEC_LOS_01_MASK  0x04
+#define SEC_LOS_10_MASK  0x08
+
+#define HOLDOVER_01_MASK 0x10
+#define HOLDOVER_10_MASK 0x20
+
+#define UNLOCK_01_MASK  0x40
+#define UNLOCK_10_MASK  0x80
+
+#define IOCTL_READ_INTERRUPT_SWITCH _IO(TLCLK_IOC_MAGIC,  23)
+
+#define IOCTL_READ_CURRENT_REF  _IO(TLCLK_IOC_MAGIC,  25)
+
+/* MAX NUMBER OF IOCTL */
+#define TLCLK_IOC_MAXNR 25
+
+struct tlclk_alarms {
+ __u32 lost_clocks;
+ __u32 lost_primary_clock;
+ __u32 lost_secondary_clock;
+ __u32 primary_clock_back;
+ __u32 secondary_clock_back;
+ __u32 switchover_primary;
+ __u32 switchover_secondary;
+ __u32 pll_holdover;
+ __u32 pll_end_holdover;
+ __u32 pll_lost_sync;
+ __u32 pll_sync;
+};


-- 
--mgross
BTW: This may or may not be the opinion of my employer, more likely not.  

Re: Patch of a new driver for kernel 2.4.x that need review

[Nishanth Aravamudan]

On 09.08.2005 [09:56:27 -0700], Mark Gross wrote:
> On Monday 08 August 2005 08:35, Mark Gross wrote:
> > On Wednesday 06 July 2005 14:14, Mark Gross wrote:
> > > On Wednesday 22 June 2005 08:12, Bouchard, Sebastien wrote:
> > > > Hi,
> > > >
> > > > Here is a driver (only for 2.4.x) I've done to provide support
> > > > of a hardware module (available on some ATCA board) used with
> > > > telecom expension card on the new ATCA platform. This module
> > > > provide redundant reference clock for telecom hardware with
> > > > alarm handling when there is a new event (ex.: one of the ref
> > > > clock is gone).  This char driver provide IOCTL for
> > > > configuration of this module and interrupt handler for
> > > > processing alarm events.
> > > >
> > > > I send this driver so people in this mailing list can do a
> > > > review of the code.
> > > >
> > > > Please reply me directly to my email, i'm not subscribed to the
> > > > mailing list.
> > > >
> > > > Thanks
> > > > Sebastien Bouchard
> > > > Software designer
> > > > Kontron Canada Inc.
> > > > <mailto:sebastien.bouchard@ca.kontron.com>
> > > > <http://www.kontron.com/>
> > > 
> > > I'm helping out a bit with the maintaining of this driver for
> > > Sebastien.   The following is a 2.6.12 port of Sebastien's 2.4
> > > driver.  
> > > 
> > > --mgross
> > 
> > The following is an update to the earlier 2.6 driver that uses a
> > sysfs interface to implement the IOCTL functions.  I put the
> > attributes under /sys/class/misc/tlclk.
> > 
> > I can't say I'm a believer in the "goodness" of using sysfs over the
> > IOCTL's, as it added quite a bit of code bloat to this driver, but
> > hay it should work well enough anyway.
> > 
> > Also, note this device, is accessed / controlled via the FPGA on the
> > ATCA, its function is to synchronize signaling hardware across
> > blades in an ATCA Chassis.  It tends to not talk to the OS. 
> > 
> > Please tell me what you think :)
> Again but with email word wrap turned off :(
> Also fixes my miss use of kcalloc parameter list.

<snip>

> diff -urN -X dontdiff linux-2.6.12.3/drivers/char/tlclk.c linux-2.6.12.3-tlclk/drivers/char/tlclk.c
> --- linux-2.6.12.3/drivers/char/tlclk.c 1969-12-31 16:00:00.000000000 -0800
> +++ linux-2.6.12.3-tlclk/drivers/char/tlclk.c 2005-08-09 09:37:58.000000000 -0700

<snip>

> +irqreturn_t tlclk_interrupt(int irq, void *dev_id, struct pt_regs *regs)

<snip>

> +  switchover_timer.expires = jiffies + 1; /* TIMEOUT in ~10ms */

This is not a 10 millisecond timeout, it is a 1 jiffy timeout. Yes, in
2.4, where HZ=100 by default, or in 2.6 with CONFIG_HZ set to 100, that
corresponds to 10 millseconds (or so ;), but not with HZ=250 or 1000. I
think you want:

switchover_timer.expires = jiffies + msecs_to_jiffies(10);

which will handle rounding correctly.

Also, consider using TIMER_INITIALIZER() for setting these timer fields.

<snip>

Thanks,
Nish

Re: Patch of a new driver for kernel 2.4.x that need review

[moreau francis]

Pekka J Enberg wrote:

> Hi Willy,
> Willy Tarreau writes:
>
>> I dont agree with you here : enums are good to simply specify an ordering.
>> But they must not be used to specify static mapping. Eg: if REG4 *must* be
>> equal to BASE+4, you should not use enums, otherwise it will render the
>> code unreadable. I personnaly don't want to count the position of REG7 in
>> the enum to discover that it's at BASE+7.
>
>
> Sorry, what do you have to count with the following?
> enum {
>       TLCLK_REG0 = TLCLK_BASE,
>       TLCLK_REG1 = TLCLK_BASE+1,
>       TLCLK_REG2 = TLCLK_BASE+2,
> };
> Please note that enums are a general way of specifying _constants_ with the
type int, not necessarily named enumerations. 

BTW, a lot of drivers use define for register mapping in kernel. Since enum
is not a new C feature, I'm wondering why kernel have prefered using define
in the past...

thanks,

      Francis





	

	
		
___________________________________________________________________________ 
Appel audio GRATUIT partout dans le monde avec le nouveau Yahoo! Messenger 
Téléchargez cette version sur http://fr.messenger.yahoo.com