public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-09  4:05 ` [PATCH 01/18] lirc core device driver infrastructure Jarod Wilson
@ 2008-09-09  4:05   ` Jarod Wilson
  2008-09-09 16:14     ` Jonathan Corbet
  0 siblings, 1 reply; 22+ messages in thread
From: Jarod Wilson @ 2008-09-09  4:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jarod Wilson, Jarod Wilson, Janne Grunau, Christoph Bartelmus

Signed-off-by: Jarod Wilson <jarod@redhat.com>
Signed-off-by: Janne Grunau <j@jannau.net>
CC: Christoph Bartelmus <lirc@bartelmus.de>
---
 drivers/input/lirc/Kconfig       |    7 +
 drivers/input/lirc/Makefile      |    1 +
 drivers/input/lirc/lirc_serial.c | 1312 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 1320 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/lirc/lirc_serial.c

diff --git a/drivers/input/lirc/Kconfig b/drivers/input/lirc/Kconfig
index 2a04d9b..73403b9 100644
--- a/drivers/input/lirc/Kconfig
+++ b/drivers/input/lirc/Kconfig
@@ -18,4 +18,11 @@ config LIRC_DEV
 	help
 	  LIRC device loadable module support, required for most LIRC drivers
 
+config LIRC_SERIAL
+	tristate "Homebrew Serial Port Receiver"
+	default n
+	depends on LIRC_DEV
+	help
+	  Driver for Homebrew Serial Port Receivers
+
 endif
diff --git a/drivers/input/lirc/Makefile b/drivers/input/lirc/Makefile
index cdb4c45..7d76128 100644
--- a/drivers/input/lirc/Makefile
+++ b/drivers/input/lirc/Makefile
@@ -6,3 +6,4 @@
 EXTRA_CFLAGS =-DIRCTL_DEV_MAJOR=61 -DLIRC_SERIAL_TRANSMITTER -I$(src)
 
 obj-$(CONFIG_LIRC_DEV)		+= lirc_dev.o
+obj-$(CONFIG_LIRC_SERIAL)	+= lirc_serial.o
diff --git a/drivers/input/lirc/lirc_serial.c b/drivers/input/lirc/lirc_serial.c
new file mode 100644
index 0000000..465edd9
--- /dev/null
+++ b/drivers/input/lirc/lirc_serial.c
@@ -0,0 +1,1312 @@
+/****************************************************************************
+ ** lirc_serial.c ***********************************************************
+ ****************************************************************************
+ *
+ * lirc_serial - Device driver that records pulse- and pause-lengths
+ *	       (space-lengths) between DDCD event on a serial port.
+ *
+ * Copyright (C) 1996,97 Ralph Metzler <rjkm@thp.uni-koeln.de>
+ * Copyright (C) 1998 Trent Piepho <xyzzy@u.washington.edu>
+ * Copyright (C) 1998 Ben Pfaff <blp@gnu.org>
+ * Copyright (C) 1999 Christoph Bartelmus <lirc@bartelmus.de>
+ * Copyright (C) 2007 Andrei Tanas <andrei@tanas.ca> (suspend/resume support)
+ *  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.  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., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+/* Steve's changes to improve transmission fidelity:
+     - for systems with the rdtsc instruction and the clock counter, a
+       send_pule that times the pulses directly using the counter.
+       This means that the LIRC_SERIAL_TRANSMITTER_LATENCY fudge is
+       not needed. Measurement shows very stable waveform, even where
+       PCI activity slows the access to the UART, which trips up other
+       versions.
+     - For other system, non-integer-microsecond pulse/space lengths,
+       done using fixed point binary. So, much more accurate carrier
+       frequency.
+     - fine tuned transmitter latency, taking advantage of fractional
+       microseconds in previous change
+     - Fixed bug in the way transmitter latency was accounted for by
+       tuning the pulse lengths down - the send_pulse routine ignored
+       this overhead as it timed the overall pulse length - so the
+       pulse frequency was right but overall pulse length was too
+       long. Fixed by accounting for latency on each pulse/space
+       iteration.
+
+   Steve Davies <steve@daviesfam.org>  July 2001
+*/
+
+#include <linux/version.h>
+
+#include <linux/autoconf.h>
+
+#include <linux/module.h>
+#include <linux/errno.h>
+#include <linux/signal.h>
+#include <linux/sched.h>
+#include <linux/fs.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/kernel.h>
+#include <linux/major.h>
+#include <linux/serial_reg.h>
+#include <linux/time.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/wait.h>
+#include <linux/mm.h>
+#include <linux/delay.h>
+#include <linux/poll.h>
+#include <linux/platform_device.h>
+
+#include <asm/system.h>
+#include <linux/uaccess.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/fcntl.h>
+
+#if defined(LIRC_SERIAL_NSLU2)
+#include <asm/hardware.h>
+/* From Intel IXP42X Developer's Manual (#252480-005): */
+/* ftp://download.intel.com/design/network/manuals/25248005.pdf */
+#define UART_IE_IXP42X_UUE   0x40 /* IXP42X UART Unit enable */
+#define UART_IE_IXP42X_RTOIE 0x10 /* IXP42X Receiver Data Timeout int.enable */
+#ifndef NSLU2_LED_GRN_GPIO
+/* added in 2.6.22 */
+#define NSLU2_LED_GRN_GPIO NSLU2_LED_GRN
+#endif
+#endif
+
+#include "lirc.h"
+#include "lirc_dev.h"
+
+#define LIRC_DRIVER_NAME "lirc_serial"
+
+struct lirc_serial {
+	int signal_pin;
+	int signal_pin_change;
+	int on;
+	int off;
+	long (*send_pulse)(unsigned long length);
+	void (*send_space)(long length);
+	int features;
+};
+
+#define LIRC_HOMEBREW	0
+#define LIRC_IRDEO	   1
+#define LIRC_IRDEO_REMOTE    2
+#define LIRC_ANIMAX	  3
+#define LIRC_IGOR	    4
+#define LIRC_NSLU2	   5
+
+#ifdef LIRC_SERIAL_IRDEO
+static int type = LIRC_IRDEO;
+#elif defined(LIRC_SERIAL_IRDEO_REMOTE)
+static int type = LIRC_IRDEO_REMOTE;
+#elif defined(LIRC_SERIAL_ANIMAX)
+static int type = LIRC_ANIMAX;
+#elif defined(LIRC_SERIAL_IGOR)
+static int type = LIRC_IGOR;
+#elif defined(LIRC_SERIAL_NSLU2)
+static int type = LIRC_NSLU2;
+#else
+static int type = LIRC_HOMEBREW;
+#endif
+
+/* Set defaults for NSLU2 */
+#if defined(LIRC_SERIAL_NSLU2)
+#ifndef LIRC_IRQ
+#define LIRC_IRQ IRQ_IXP4XX_UART2
+#endif
+#ifndef LIRC_PORT
+#define LIRC_PORT (IXP4XX_UART2_BASE_VIRT + REG_OFFSET)
+#endif
+#ifndef LIRC_IOMMAP
+#define LIRC_IOMMAP IXP4XX_UART2_BASE_PHYS
+#endif
+#ifndef LIRC_IOSHIFT
+#define LIRC_IOSHIFT 2
+#endif
+#ifndef LIRC_ALLOW_MMAPPED_IO
+#define LIRC_ALLOW_MMAPPED_IO
+#endif
+#endif
+
+#if defined(LIRC_ALLOW_MMAPPED_IO)
+#ifndef LIRC_IOMMAP
+#define LIRC_IOMMAP 0
+#endif
+#ifndef LIRC_IOSHIFT
+#define LIRC_IOSHIFT 0
+#endif
+static int iommap = LIRC_IOMMAP;
+static int ioshift = LIRC_IOSHIFT;
+#endif
+
+#ifdef LIRC_SERIAL_SOFTCARRIER
+static int softcarrier = 1;
+#else
+static int softcarrier;
+#endif
+
+static int share_irq;
+static int debug;
+
+#define dprintk(fmt, args...)					\
+	do {							\
+		if (debug)					\
+			printk(KERN_DEBUG LIRC_DRIVER_NAME ": "	\
+			       fmt, ## args);			\
+	} while (0)
+
+/* forward declarations */
+static long send_pulse_irdeo(unsigned long length);
+static long send_pulse_homebrew(unsigned long length);
+static void send_space_irdeo(long length);
+static void send_space_homebrew(long length);
+
+static struct lirc_serial hardware[] = {
+	/* home-brew receiver/transmitter */
+	{
+		UART_MSR_DCD,
+		UART_MSR_DDCD,
+		UART_MCR_RTS|UART_MCR_OUT2|UART_MCR_DTR,
+		UART_MCR_RTS|UART_MCR_OUT2,
+		send_pulse_homebrew,
+		send_space_homebrew,
+		(
+#ifdef LIRC_SERIAL_TRANSMITTER
+		 LIRC_CAN_SET_SEND_DUTY_CYCLE|
+		 LIRC_CAN_SET_SEND_CARRIER|
+		 LIRC_CAN_SEND_PULSE|
+#endif
+		 LIRC_CAN_REC_MODE2)
+	},
+
+	/* IRdeo classic */
+	{
+		UART_MSR_DSR,
+		UART_MSR_DDSR,
+		UART_MCR_OUT2,
+		UART_MCR_RTS|UART_MCR_DTR|UART_MCR_OUT2,
+		send_pulse_irdeo,
+		send_space_irdeo,
+		(LIRC_CAN_SET_SEND_DUTY_CYCLE|
+		 LIRC_CAN_SEND_PULSE|
+		 LIRC_CAN_REC_MODE2)
+	},
+
+	/* IRdeo remote */
+	{
+		UART_MSR_DSR,
+		UART_MSR_DDSR,
+		UART_MCR_RTS|UART_MCR_DTR|UART_MCR_OUT2,
+		UART_MCR_RTS|UART_MCR_DTR|UART_MCR_OUT2,
+		send_pulse_irdeo,
+		send_space_irdeo,
+		(LIRC_CAN_SET_SEND_DUTY_CYCLE|
+		 LIRC_CAN_SEND_PULSE|
+		 LIRC_CAN_REC_MODE2)
+	},
+
+	/* AnimaX */
+	{
+		UART_MSR_DCD,
+		UART_MSR_DDCD,
+		0,
+		UART_MCR_RTS|UART_MCR_DTR|UART_MCR_OUT2,
+		NULL,
+		NULL,
+		LIRC_CAN_REC_MODE2
+	},
+
+	/* home-brew receiver/transmitter (Igor Cesko's variation) */
+	{
+		UART_MSR_DSR,
+		UART_MSR_DDSR,
+		UART_MCR_RTS|UART_MCR_OUT2|UART_MCR_DTR,
+		UART_MCR_RTS|UART_MCR_OUT2,
+		send_pulse_homebrew,
+		send_space_homebrew,
+		(
+#ifdef LIRC_SERIAL_TRANSMITTER
+		 LIRC_CAN_SET_SEND_DUTY_CYCLE|
+		 LIRC_CAN_SET_SEND_CARRIER|
+		 LIRC_CAN_SEND_PULSE|
+#endif
+		 LIRC_CAN_REC_MODE2)
+	},
+
+#if defined(LIRC_SERIAL_NSLU2)
+	/* Modified Linksys Network Storage Link USB 2.0 (NSLU2):
+	   We receive on CTS of the 2nd serial port (R142,LHS), we
+	   transmit with a IR diode between GPIO[1] (green status LED),
+	   and ground (Matthias Goebl <matthias.goebl@goebl.net>).
+	   See also http://www.nslu2-linux.org for this device */
+	{
+		UART_MSR_CTS,
+		UART_MSR_DCTS,
+		UART_MCR_RTS|UART_MCR_OUT2|UART_MCR_DTR,
+		UART_MCR_RTS|UART_MCR_OUT2,
+		send_pulse_homebrew,
+		send_space_homebrew,
+		(
+#ifdef LIRC_SERIAL_TRANSMITTER
+		 LIRC_CAN_SET_SEND_DUTY_CYCLE|
+		 LIRC_CAN_SET_SEND_CARRIER|
+		 LIRC_CAN_SEND_PULSE|
+#endif
+		 LIRC_CAN_REC_MODE2)
+	},
+#endif
+
+};
+
+#define RS_ISR_PASS_LIMIT 256
+
+/* A long pulse code from a remote might take upto 300 bytes.  The
+   daemon should read the bytes as soon as they are generated, so take
+   the number of keys you think you can push before the daemon runs
+   and multiply by 300.  The driver will warn you if you overrun this
+   buffer.  If you have a slow computer or non-busmastering IDE disks,
+   maybe you will need to increase this.  */
+
+/* This MUST be a power of two!  It has to be larger than 1 as well. */
+
+#define RBUF_LEN 256
+#define WBUF_LEN 256
+
+static int sense = -1;	/* -1 = auto, 0 = active high, 1 = active low */
+static int txsense;     /* 0 = active high, 1 = active low */
+
+#ifndef LIRC_IRQ
+#define LIRC_IRQ 4
+#endif
+#ifndef LIRC_PORT
+#define LIRC_PORT 0x3f8
+#endif
+
+static int io = LIRC_PORT;
+static int irq = LIRC_IRQ;
+
+static struct timeval lasttv = {0, 0};
+
+static struct lirc_buffer rbuf;
+
+static int wbuf[WBUF_LEN];
+
+static unsigned int freq = 38000;
+static unsigned int duty_cycle = 50;
+
+/* Initialized in init_timing_params() */
+static unsigned long period;
+static unsigned long pulse_width;
+static unsigned long space_width;
+
+#if defined(__i386__)
+/*
+  From:
+  Linux I/O port programming mini-HOWTO
+  Author: Riku Saikkonen <Riku.Saikkonen@hut.fi>
+  v, 28 December 1997
+
+  [...]
+  Actually, a port I/O instruction on most ports in the 0-0x3ff range
+  takes almost exactly 1 microsecond, so if you're, for example, using
+  the parallel port directly, just do additional inb()s from that port
+  to delay.
+  [...]
+*/
+/* transmitter latency 1.5625us 0x1.90 - this figure arrived at from
+ * comment above plus trimming to match actual measured frequency.
+ * This will be sensitive to cpu speed, though hopefully most of the 1.5us
+ * is spent in the uart access.  Still - for reference test machine was a
+ * 1.13GHz Athlon system - Steve
+ */
+
+/* changed from 400 to 450 as this works better on slower machines;
+   faster machines will use the rdtsc code anyway */
+
+#define LIRC_SERIAL_TRANSMITTER_LATENCY 450
+
+#else
+
+/* does anybody have information on other platforms ? */
+/* 256 = 1<<8 */
+#define LIRC_SERIAL_TRANSMITTER_LATENCY 256
+
+#endif  /* __i386__ */
+
+static inline unsigned int sinp(int offset)
+{
+#if defined(LIRC_ALLOW_MMAPPED_IO)
+	if (iommap != 0) {
+		/* the register is memory-mapped */
+		offset <<= ioshift;
+		return readb(io + offset);
+	}
+#endif
+	return inb(io + offset);
+}
+
+static inline void soutp(int offset, int value)
+{
+#if defined(LIRC_ALLOW_MMAPPED_IO)
+	if (iommap != 0) {
+		/* the register is memory-mapped */
+		offset <<= ioshift;
+		writeb(value, io + offset);
+	}
+#endif
+	outb(value, io + offset);
+}
+
+static inline void on(void)
+{
+#if defined(LIRC_SERIAL_NSLU2)
+	/* On NSLU2, we put the transmit diode between the output of the green
+	   status LED and ground */
+	if (type == LIRC_NSLU2) {
+		gpio_line_set(NSLU2_LED_GRN_GPIO, IXP4XX_GPIO_LOW);
+		return;
+	}
+#endif
+	if (txsense)
+		soutp(UART_MCR, hardware[type].off);
+	else
+		soutp(UART_MCR, hardware[type].on);
+}
+
+static inline void off(void)
+{
+#if defined(LIRC_SERIAL_NSLU2)
+	if (type == LIRC_NSLU2) {
+		gpio_line_set(NSLU2_LED_GRN_GPIO, IXP4XX_GPIO_HIGH);
+		return;
+	}
+#endif
+	if (txsense)
+		soutp(UART_MCR, hardware[type].on);
+	else
+		soutp(UART_MCR, hardware[type].off);
+}
+
+#ifndef MAX_UDELAY_MS
+#define MAX_UDELAY_US 5000
+#else
+#define MAX_UDELAY_US (MAX_UDELAY_MS*1000)
+#endif
+
+static inline void safe_udelay(unsigned long usecs)
+{
+	while (usecs > MAX_UDELAY_US) {
+		udelay(MAX_UDELAY_US);
+		usecs -= MAX_UDELAY_US;
+	}
+	udelay(usecs);
+}
+
+#ifdef USE_RDTSC
+/* This is an overflow/precision juggle, complicated in that we can't
+   do long long divide in the kernel */
+
+/* When we use the rdtsc instruction to measure clocks, we keep the
+ * pulse and space widths as clock cycles.  As this is CPU speed
+ * dependent, the widths must be calculated in init_port and ioctl
+ * time
+ */
+
+/* So send_pulse can quickly convert microseconds to clocks */
+static unsigned long conv_us_to_clocks;
+
+static inline int init_timing_params(unsigned int new_duty_cycle,
+		unsigned int new_freq)
+{
+	unsigned long long loops_per_sec, work;
+
+	duty_cycle = new_duty_cycle;
+	freq = new_freq;
+
+	loops_per_sec = current_cpu_data.loops_per_jiffy;
+	loops_per_sec *= HZ;
+
+	/* How many clocks in a microsecond?, avoiding long long divide */
+	work = loops_per_sec;
+	work *= 4295;  /* 4295 = 2^32 / 1e6 */
+	conv_us_to_clocks = (work>>32);
+
+	/* Carrier period in clocks, approach good up to 32GHz clock,
+	   gets carrier frequency within 8Hz */
+	period = loops_per_sec>>3;
+	period /= (freq>>3);
+
+	/* Derive pulse and space from the period */
+
+	pulse_width = period*duty_cycle/100;
+	space_width = period - pulse_width;
+	dprintk("in init_timing_params, freq=%d, duty_cycle=%d, "
+		"clk/jiffy=%ld, pulse=%ld, space=%ld, "
+		"conv_us_to_clocks=%ld\n",
+		freq, duty_cycle, current_cpu_data.loops_per_jiffy,
+		pulse_width, space_width, conv_us_to_clocks);
+	return 0;
+}
+#else /* ! USE_RDTSC */
+static inline int init_timing_params(unsigned int new_duty_cycle,
+		unsigned int new_freq)
+{
+/* period, pulse/space width are kept with 8 binary places -
+ * IE multiplied by 256. */
+	if (256*1000000L/new_freq*new_duty_cycle/100 <=
+	    LIRC_SERIAL_TRANSMITTER_LATENCY)
+		return -EINVAL;
+	if (256*1000000L/new_freq*(100-new_duty_cycle)/100 <=
+	    LIRC_SERIAL_TRANSMITTER_LATENCY)
+		return -EINVAL;
+	duty_cycle = new_duty_cycle;
+	freq = new_freq;
+	period = 256*1000000L/freq;
+	pulse_width = period*duty_cycle/100;
+	space_width = period-pulse_width;
+	dprintk("in init_timing_params, freq=%d pulse=%ld, "
+		"space=%ld\n", freq, pulse_width, space_width);
+	return 0;
+}
+#endif /* USE_RDTSC */
+
+
+/* return value: space length delta */
+
+static long send_pulse_irdeo(unsigned long length)
+{
+	long rawbits;
+	int i;
+	unsigned char output;
+	unsigned char chunk, shifted;
+
+	/* how many bits have to be sent ? */
+	rawbits = length*1152/10000;
+	if (duty_cycle > 50)
+		chunk = 3;
+	else
+		chunk = 1;
+	for (i = 0, output = 0x7f; rawbits > 0; rawbits -= 3) {
+		shifted = chunk<<(i*3);
+		shifted >>= 1;
+		output &= (~shifted);
+		i++;
+		if (i == 3) {
+			soutp(UART_TX, output);
+			while (!(sinp(UART_LSR) & UART_LSR_THRE))
+				;
+			output = 0x7f;
+			i = 0;
+		}
+	}
+	if (i != 0) {
+		soutp(UART_TX, output);
+		while (!(sinp(UART_LSR) & UART_LSR_TEMT))
+			;
+	}
+
+	if (i == 0)
+		return (-rawbits)*10000/1152;
+	else
+		return (3-i)*3*10000/1152 + (-rawbits)*10000/1152;
+}
+
+#ifdef USE_RDTSC
+/* Version that uses Pentium rdtsc instruction to measure clocks */
+
+/* This version does sub-microsecond timing using rdtsc instruction,
+ * and does away with the fudged LIRC_SERIAL_TRANSMITTER_LATENCY
+ * Implicitly i586 architecture...  - Steve
+ */
+
+static inline long send_pulse_homebrew_softcarrier(unsigned long length)
+{
+	int flag;
+	unsigned long target, start, now;
+
+	/* Get going quick as we can */
+	rdtscl(start); on();
+	/* Convert length from microseconds to clocks */
+	length *= conv_us_to_clocks;
+	/* And loop till time is up - flipping at right intervals */
+	now = start;
+	target = pulse_width;
+	flag = 1;
+	while ((now-start) < length) {
+		/* Delay till flip time */
+		do {
+			rdtscl(now);
+		} while ((now-start) < target);
+
+		/* flip */
+		if (flag) {
+			rdtscl(now); off();
+			target += space_width;
+		} else {
+			rdtscl(now); on();
+			target += pulse_width;
+		}
+		flag = !flag;
+	}
+	rdtscl(now);
+	return ((now-start)-length) / conv_us_to_clocks;
+}
+#else /* ! USE_RDTSC */
+/* Version using udelay() */
+
+/* here we use fixed point arithmetic, with 8
+   fractional bits.  that gets us within 0.1% or so of the right average
+   frequency, albeit with some jitter in pulse length - Steve */
+
+/* To match 8 fractional bits used for pulse/space length */
+
+static inline long send_pulse_homebrew_softcarrier(unsigned long length)
+{
+	int flag;
+	unsigned long actual, target, d;
+	length <<= 8;
+
+	actual = 0; target = 0; flag = 0;
+	while (actual < length) {
+		if (flag) {
+			off();
+			target += space_width;
+		} else {
+			on();
+			target += pulse_width;
+		}
+		d = (target-actual-LIRC_SERIAL_TRANSMITTER_LATENCY+128)>>8;
+		/* Note - we've checked in ioctl that the pulse/space
+		   widths are big enough so that d is > 0 */
+		udelay(d);
+		actual += (d<<8)+LIRC_SERIAL_TRANSMITTER_LATENCY;
+		flag = !flag;
+	}
+	return (actual-length)>>8;
+}
+#endif /* USE_RDTSC */
+
+static long send_pulse_homebrew(unsigned long length)
+{
+	if (length <= 0)
+		return 0;
+
+	if (softcarrier)
+		return send_pulse_homebrew_softcarrier(length);
+	else {
+		on();
+		safe_udelay(length);
+		return 0;
+	}
+}
+
+static void send_space_irdeo(long length)
+{
+	if (length <= 0)
+		return;
+
+	safe_udelay(length);
+}
+
+static void send_space_homebrew(long length)
+{
+	off();
+	if (length <= 0)
+		return;
+	safe_udelay(length);
+}
+
+static inline void rbwrite(int l)
+{
+	if (lirc_buffer_full(&rbuf)) {
+		/* no new signals will be accepted */
+		dprintk("Buffer overrun\n");
+		return;
+	}
+	_lirc_buffer_write_1(&rbuf, (void *)&l);
+}
+
+static inline void frbwrite(int l)
+{
+	/* simple noise filter */
+	static int pulse, space;
+	static unsigned int ptr;
+
+	if (ptr > 0 && (l & PULSE_BIT)) {
+		pulse += l & PULSE_MASK;
+		if (pulse > 250) {
+			rbwrite(space);
+			rbwrite(pulse | PULSE_BIT);
+			ptr = 0;
+			pulse = 0;
+		}
+		return;
+	}
+	if (!(l & PULSE_BIT)) {
+		if (ptr == 0) {
+			if (l > 20000) {
+				space = l;
+				ptr++;
+				return;
+			}
+		} else {
+			if (l > 20000) {
+				space += pulse;
+				if (space > PULSE_MASK)
+					space = PULSE_MASK;
+				space += l;
+				if (space > PULSE_MASK)
+					space = PULSE_MASK;
+				pulse = 0;
+				return;
+			}
+			rbwrite(space);
+			rbwrite(pulse | PULSE_BIT);
+			ptr = 0;
+			pulse = 0;
+		}
+	}
+	rbwrite(l);
+}
+
+static irqreturn_t irq_handler(int i, void *blah)
+{
+	struct timeval tv;
+	int status, counter, dcd;
+	long deltv;
+	int data;
+	static int last_dcd = -1;
+
+	if ((sinp(UART_IIR) & UART_IIR_NO_INT)) {
+		/* not our interrupt */
+		return IRQ_RETVAL(IRQ_NONE);
+	}
+
+	counter = 0;
+	do {
+		counter++;
+		status = sinp(UART_MSR);
+		if (counter > RS_ISR_PASS_LIMIT) {
+			printk(KERN_WARNING LIRC_DRIVER_NAME ": AIEEEE: "
+			       "We're caught!\n");
+			break;
+		}
+		if ((status&hardware[type].signal_pin_change) && sense != -1) {
+			/* get current time */
+			do_gettimeofday(&tv);
+
+			/* New mode, written by Trent Piepho
+			   <xyzzy@u.washington.edu>. */
+
+			/* The old format was not very portable.
+			   We now use an int to pass pulses
+			   and spaces to user space.
+
+			   If PULSE_BIT is set a pulse has been
+			   received, otherwise a space has been
+			   received.  The driver needs to know if your
+			   receiver is active high or active low, or
+			   the space/pulse sense could be
+			   inverted. The bits denoted by PULSE_MASK are
+			   the length in microseconds. Lengths greater
+			   than or equal to 16 seconds are clamped to
+			   PULSE_MASK.  All other bits are unused.
+			   This is a much simpler interface for user
+			   programs, as well as eliminating "out of
+			   phase" errors with space/pulse
+			   autodetection. */
+
+			/* calculate time since last interrupt in
+			   microseconds */
+			dcd = (status & hardware[type].signal_pin) ? 1 : 0;
+
+			if (dcd == last_dcd) {
+				printk(KERN_WARNING LIRC_DRIVER_NAME
+				": ignoring spike: %d %d %lx %lx %lx %lx\n",
+				dcd, sense,
+				tv.tv_sec, lasttv.tv_sec,
+				tv.tv_usec, lasttv.tv_usec);
+				continue;
+			}
+
+			deltv = tv.tv_sec-lasttv.tv_sec;
+			if (tv.tv_sec < lasttv.tv_sec ||
+			    (tv.tv_sec == lasttv.tv_sec &&
+			     tv.tv_usec < lasttv.tv_usec)) {
+				printk(KERN_WARNING LIRC_DRIVER_NAME
+				       ": AIEEEE: your clock just jumped "
+				       "backwards\n");
+				printk(KERN_WARNING LIRC_DRIVER_NAME
+				       ": %d %d %lx %lx %lx %lx\n",
+				       dcd, sense,
+				       tv.tv_sec, lasttv.tv_sec,
+				       tv.tv_usec, lasttv.tv_usec);
+				data = PULSE_MASK;
+			} else if (deltv > 15) {
+				data = PULSE_MASK; /* really long time */
+				if (!(dcd^sense)) {
+					/* sanity check */
+					printk(KERN_WARNING LIRC_DRIVER_NAME
+					       ": AIEEEE: "
+					       "%d %d %lx %lx %lx %lx\n",
+					       dcd, sense,
+					       tv.tv_sec, lasttv.tv_sec,
+					       tv.tv_usec, lasttv.tv_usec);
+					/* detecting pulse while this
+					   MUST be a space! */
+					sense = sense ? 0 : 1;
+				}
+			} else
+				data = (int) (deltv*1000000 +
+					       tv.tv_usec -
+					       lasttv.tv_usec);
+			frbwrite(dcd^sense ? data : (data|PULSE_BIT));
+			lasttv = tv;
+			last_dcd = dcd;
+			wake_up_interruptible(&rbuf.wait_poll);
+		}
+	} while (!(sinp(UART_IIR) & UART_IIR_NO_INT)); /* still pending ? */
+	return IRQ_RETVAL(IRQ_HANDLED);
+}
+
+static void hardware_init_port(void)
+{
+	unsigned long flags;
+	local_irq_save(flags);
+
+	/* Set DLAB 0. */
+	soutp(UART_LCR, sinp(UART_LCR) & (~UART_LCR_DLAB));
+
+	/* First of all, disable all interrupts */
+	soutp(UART_IER, sinp(UART_IER) &
+	      (~(UART_IER_MSI|UART_IER_RLSI|UART_IER_THRI|UART_IER_RDI)));
+
+	/* Clear registers. */
+	sinp(UART_LSR);
+	sinp(UART_RX);
+	sinp(UART_IIR);
+	sinp(UART_MSR);
+
+#if defined(LIRC_SERIAL_NSLU2)
+	if (type == LIRC_NSLU2) {
+		/* Setup NSLU2 UART */
+
+		/* Enable UART */
+		soutp(UART_IER, sinp(UART_IER) | UART_IE_IXP42X_UUE);
+		/* Disable Receiver data Time out interrupt */
+		soutp(UART_IER, sinp(UART_IER) & ~UART_IE_IXP42X_RTOIE);
+		/* set out2 = interupt unmask; off() doesn't set MCR
+		   on NSLU2 */
+		soutp(UART_MCR, UART_MCR_RTS|UART_MCR_OUT2);
+	}
+#endif
+
+	/* Set line for power source */
+	off();
+
+	/* Clear registers again to be sure. */
+	sinp(UART_LSR);
+	sinp(UART_RX);
+	sinp(UART_IIR);
+	sinp(UART_MSR);
+
+	switch (type) {
+	case LIRC_IRDEO:
+	case LIRC_IRDEO_REMOTE:
+		/* setup port to 7N1 @ 115200 Baud */
+		/* 7N1+start = 9 bits at 115200 ~ 3 bits at 38kHz */
+
+		/* Set DLAB 1. */
+		soutp(UART_LCR, sinp(UART_LCR) | UART_LCR_DLAB);
+		/* Set divisor to 1 => 115200 Baud */
+		soutp(UART_DLM, 0);
+		soutp(UART_DLL, 1);
+		/* Set DLAB 0 +  7N1 */
+		soutp(UART_LCR, UART_LCR_WLEN7);
+		/* THR interrupt already disabled at this point */
+		break;
+	default:
+		break;
+	}
+
+	local_irq_restore(flags);
+}
+
+static int init_port(void)
+{
+	int i, nlow, nhigh;
+
+	/* Reserve io region. */
+#if defined(LIRC_ALLOW_MMAPPED_IO)
+	/* Future MMAP-Developers: Attention!
+	   For memory mapped I/O you *might* need to use ioremap() first,
+	   for the NSLU2 it's done in boot code. */
+	if (((iommap != 0)
+	     && (request_mem_region(iommap, 8<<ioshift,
+				    LIRC_DRIVER_NAME) == NULL))
+	   || ((iommap == 0)
+	       && (request_region(io, 8, LIRC_DRIVER_NAME) == NULL))) {
+#else
+	if (request_region(io, 8, LIRC_DRIVER_NAME) == NULL) {
+#endif
+		printk(KERN_ERR  LIRC_DRIVER_NAME
+		       ": port %04x already in use\n", io);
+		printk(KERN_WARNING LIRC_DRIVER_NAME
+		       ": use 'setserial /dev/ttySX uart none'\n");
+		printk(KERN_WARNING LIRC_DRIVER_NAME
+		       ": or compile the serial port driver as module and\n");
+		printk(KERN_WARNING LIRC_DRIVER_NAME
+		       ": make sure this module is loaded first\n");
+		return -EBUSY;
+	}
+
+	hardware_init_port();
+
+	/* Initialize pulse/space widths */
+	init_timing_params(duty_cycle, freq);
+
+	/* If pin is high, then this must be an active low receiver. */
+	if (sense == -1) {
+		/* wait 1/2 sec for the power supply */
+
+		set_current_state(TASK_INTERRUPTIBLE);
+		schedule_timeout(HZ/2);
+
+		/* probe 9 times every 0.04s, collect "votes" for
+		   active high/low */
+		nlow = 0;
+		nhigh = 0;
+		for (i = 0; i < 9; i++) {
+			if (sinp(UART_MSR) & hardware[type].signal_pin)
+				nlow++;
+			else
+				nhigh++;
+			schedule_timeout(HZ/25);
+		}
+		sense = (nlow >= nhigh ? 1 : 0);
+		printk(KERN_INFO  LIRC_DRIVER_NAME  ": auto-detected active "
+		       "%s receiver\n", sense ? "low" : "high");
+	} else
+		printk(KERN_INFO  LIRC_DRIVER_NAME  ": Manually using active "
+		       "%s receiver\n", sense ? "low" : "high");
+
+	return 0;
+}
+
+static int set_use_inc(void *data)
+{
+	int result;
+	unsigned long flags;
+
+	/* Init read buffer. */
+	if (lirc_buffer_init(&rbuf, sizeof(int), RBUF_LEN) < 0)
+		return -ENOMEM;
+
+	/* initialize timestamp */
+	do_gettimeofday(&lasttv);
+
+	result = request_irq(irq, irq_handler,
+			   IRQF_DISABLED | (share_irq ? IRQF_SHARED : 0),
+			   LIRC_DRIVER_NAME, (void *)&hardware);
+
+	switch (result) {
+	case -EBUSY:
+		printk(KERN_ERR LIRC_DRIVER_NAME ": IRQ %d busy\n", irq);
+		lirc_buffer_free(&rbuf);
+		return -EBUSY;
+	case -EINVAL:
+		printk(KERN_ERR LIRC_DRIVER_NAME
+		       ": Bad irq number or handler\n");
+		lirc_buffer_free(&rbuf);
+		return -EINVAL;
+	default:
+		dprintk("Interrupt %d, port %04x obtained\n", irq, io);
+		break;
+	};
+
+	local_irq_save(flags);
+
+	/* Set DLAB 0. */
+	soutp(UART_LCR, sinp(UART_LCR) & (~UART_LCR_DLAB));
+
+	soutp(UART_IER, sinp(UART_IER)|UART_IER_MSI);
+
+	local_irq_restore(flags);
+
+	return 0;
+}
+
+static void set_use_dec(void *data)
+{	unsigned long flags;
+
+	local_irq_save(flags);
+
+	/* Set DLAB 0. */
+	soutp(UART_LCR, sinp(UART_LCR) & (~UART_LCR_DLAB));
+
+	/* First of all, disable all interrupts */
+	soutp(UART_IER, sinp(UART_IER) &
+	      (~(UART_IER_MSI|UART_IER_RLSI|UART_IER_THRI|UART_IER_RDI)));
+	local_irq_restore(flags);
+
+	free_irq(irq, (void *)&hardware);
+
+	dprintk("freed IRQ %d\n", irq);
+	lirc_buffer_free(&rbuf);
+}
+
+static ssize_t lirc_write(struct file *file, const char *buf,
+			 size_t n, loff_t *ppos)
+{
+	int i, count;
+	unsigned long flags;
+	long delta = 0;
+
+	if (!(hardware[type].features&LIRC_CAN_SEND_PULSE))
+		return -EBADF;
+
+	if (n % sizeof(int))
+		return -EINVAL;
+	count = n / sizeof(int);
+	if (count > WBUF_LEN || count % 2 == 0)
+		return -EINVAL;
+	if (copy_from_user(wbuf, buf, n))
+		return -EFAULT;
+	local_irq_save(flags);
+	if (type == LIRC_IRDEO) {
+		/* DTR, RTS down */
+		on();
+	}
+	for (i = 0; i < count; i++) {
+		if (i%2)
+			hardware[type].send_space(wbuf[i]-delta);
+		else
+			delta = hardware[type].send_pulse(wbuf[i]);
+	}
+	off();
+	local_irq_restore(flags);
+	return n;
+}
+
+static int lirc_ioctl(struct inode *node, struct file *filep, unsigned int cmd,
+		      unsigned long arg)
+{
+	int result;
+	unsigned long value;
+	unsigned int ivalue;
+
+	switch (cmd) {
+	case LIRC_GET_SEND_MODE:
+		if (!(hardware[type].features&LIRC_CAN_SEND_MASK))
+			return -ENOIOCTLCMD;
+
+		result = put_user(LIRC_SEND2MODE
+				  (hardware[type].features&LIRC_CAN_SEND_MASK),
+				  (unsigned long *) arg);
+		if (result)
+			return result;
+		break;
+
+	case LIRC_SET_SEND_MODE:
+		if (!(hardware[type].features&LIRC_CAN_SEND_MASK))
+			return -ENOIOCTLCMD;
+
+		result = get_user(value, (unsigned long *) arg);
+		if (result)
+			return result;
+		/* only LIRC_MODE_PULSE supported */
+		if (value != LIRC_MODE_PULSE)
+			return -ENOSYS;
+		break;
+
+	case LIRC_GET_LENGTH:
+		return -ENOSYS;
+		break;
+
+	case LIRC_SET_SEND_DUTY_CYCLE:
+		dprintk("SET_SEND_DUTY_CYCLE\n");
+		if (!(hardware[type].features&LIRC_CAN_SET_SEND_DUTY_CYCLE))
+			return -ENOIOCTLCMD;
+
+		result = get_user(ivalue, (unsigned int *) arg);
+		if (result)
+			return result;
+		if (ivalue <= 0 || ivalue > 100)
+			return -EINVAL;
+		return init_timing_params(ivalue, freq);
+		break;
+
+	case LIRC_SET_SEND_CARRIER:
+		dprintk("SET_SEND_CARRIER\n");
+		if (!(hardware[type].features&LIRC_CAN_SET_SEND_CARRIER))
+			return -ENOIOCTLCMD;
+
+		result = get_user(ivalue, (unsigned int *) arg);
+		if (result)
+			return result;
+		if (ivalue > 500000 || ivalue < 20000)
+			return -EINVAL;
+		return init_timing_params(duty_cycle, ivalue);
+		break;
+
+	default:
+		return -ENOIOCTLCMD;
+	}
+	return 0;
+}
+
+static struct file_operations lirc_fops = {
+	.write	= lirc_write,
+};
+
+static struct lirc_plugin plugin = {
+	.name		= LIRC_DRIVER_NAME,
+	.minor		= -1,
+	.code_length	= 1,
+	.sample_rate	= 0,
+	.data		= NULL,
+	.add_to_buf	= NULL,
+	.get_queue	= NULL,
+	.rbuf		= &rbuf,
+	.set_use_inc	= set_use_inc,
+	.set_use_dec	= set_use_dec,
+	.ioctl		= lirc_ioctl,
+	.fops		= &lirc_fops,
+	.dev		= NULL,
+	.owner		= THIS_MODULE,
+};
+
+#ifdef MODULE
+
+static struct platform_device *lirc_serial_dev;
+
+static int __devinit lirc_serial_probe(struct platform_device *dev)
+{
+	return 0;
+}
+
+static int __devexit lirc_serial_remove(struct platform_device *dev)
+{
+	return 0;
+}
+
+static int lirc_serial_suspend(struct platform_device *dev,
+			       pm_message_t state)
+{
+	/* Set DLAB 0. */
+	soutp(UART_LCR, sinp(UART_LCR) & (~UART_LCR_DLAB));
+
+	/* Disable all interrupts */
+	soutp(UART_IER, sinp(UART_IER) &
+	      (~(UART_IER_MSI|UART_IER_RLSI|UART_IER_THRI|UART_IER_RDI)));
+
+	/* Clear registers. */
+	sinp(UART_LSR);
+	sinp(UART_RX);
+	sinp(UART_IIR);
+	sinp(UART_MSR);
+
+	return 0;
+}
+
+static int lirc_serial_resume(struct platform_device *dev)
+{
+	unsigned long flags;
+
+	hardware_init_port();
+
+	local_irq_save(flags);
+	/* Enable Interrupt */
+	do_gettimeofday(&lasttv);
+	soutp(UART_IER, sinp(UART_IER)|UART_IER_MSI);
+	off();
+
+	lirc_buffer_clear(&rbuf);
+
+	local_irq_restore(flags);
+
+	return 0;
+}
+
+static struct platform_driver lirc_serial_driver = {
+	.probe		= lirc_serial_probe,
+	.remove		= __devexit_p(lirc_serial_remove),
+	.suspend	= lirc_serial_suspend,
+	.resume		= lirc_serial_resume,
+	.driver		= {
+		.name	= "lirc_serial",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init lirc_serial_init(void)
+{
+	int result;
+
+	result = platform_driver_register(&lirc_serial_driver);
+	if (result) {
+		printk("lirc register returned %d\n", result);
+		return result;
+	}
+
+	lirc_serial_dev = platform_device_alloc("lirc_serial", 0);
+	if (!lirc_serial_dev) {
+		result = -ENOMEM;
+		goto exit_driver_unregister;
+	}
+
+	result = platform_device_add(lirc_serial_dev);
+	if (result)
+		goto exit_device_put;
+
+	return 0;
+
+exit_device_put:
+	platform_device_put(lirc_serial_dev);
+exit_driver_unregister:
+	platform_driver_unregister(&lirc_serial_driver);
+	return result;
+}
+
+static void __exit lirc_serial_exit(void)
+{
+	platform_device_unregister(lirc_serial_dev);
+	platform_driver_unregister(&lirc_serial_driver);
+}
+
+int __init init_module(void)
+{
+	int result;
+
+	result = lirc_serial_init();
+	if (result)
+		return result;
+	switch (type) {
+	case LIRC_HOMEBREW:
+	case LIRC_IRDEO:
+	case LIRC_IRDEO_REMOTE:
+	case LIRC_ANIMAX:
+	case LIRC_IGOR:
+#if defined(LIRC_SERIAL_NSLU2)
+	case LIRC_NSLU2:
+#endif
+		break;
+	default:
+		result = -EINVAL;
+		goto exit_serial_exit;
+	}
+	if (!softcarrier) {
+		switch (type) {
+		case LIRC_HOMEBREW:
+		case LIRC_IGOR:
+		case LIRC_NSLU2:
+			hardware[type].features &=
+				~(LIRC_CAN_SET_SEND_DUTY_CYCLE|
+				  LIRC_CAN_SET_SEND_CARRIER);
+			break;
+		}
+	}
+	result = init_port();
+	if (result < 0)
+		goto exit_serial_exit;
+	plugin.features = hardware[type].features;
+	plugin.minor = lirc_register_plugin(&plugin);
+	if (plugin.minor < 0) {
+		printk(KERN_ERR  LIRC_DRIVER_NAME
+		       ": register_chrdev failed!\n");
+		result = -EIO;
+		goto exit_release;
+	}
+	return 0;
+exit_release:
+	release_region(io, 8);
+exit_serial_exit:
+	lirc_serial_exit();
+	return result;
+}
+
+void __exit cleanup_module(void)
+{
+	lirc_serial_exit();
+#if defined(LIRC_ALLOW_MMAPPED_IO)
+	if (iommap != 0)
+		release_mem_region(iommap, 8<<ioshift);
+	else
+		release_region(io, 8);
+#else
+	release_region(io, 8);
+#endif
+	lirc_unregister_plugin(plugin.minor);
+	dprintk("cleaned up module\n");
+}
+
+MODULE_DESCRIPTION("Infra-red receiver driver for serial ports.");
+MODULE_AUTHOR("Ralph Metzler, Trent Piepho, Ben Pfaff, "
+	      "Christoph Bartelmus, Andrei Tanas");
+MODULE_LICENSE("GPL");
+
+module_param(type, int, 0444);
+#if defined(LIRC_SERIAL_NSLU2)
+MODULE_PARM_DESC(type, "Hardware type (0 = home-brew, 1 = IRdeo,"
+		 " 2 = IRdeo Remote, 3 = AnimaX, 4 = IgorPlug,"
+		 " 5 = NSLU2 RX:CTS2/TX:GreenLED)");
+#else
+MODULE_PARM_DESC(type, "Hardware type (0 = home-brew, 1 = IRdeo,"
+		 " 2 = IRdeo Remote, 3 = AnimaX, 4 = IgorPlug)");
+#endif
+
+module_param(io, int, 0444);
+MODULE_PARM_DESC(io, "I/O address base (0x3f8 or 0x2f8)");
+
+#if defined(LIRC_ALLOW_MMAPPED_IO)
+/* some architectures (e.g. intel xscale) have memory mapped registers */
+module_param(iommap, bool, 0444);
+MODULE_PARM_DESC(iommap, "physical base for memory mapped I/O"
+		" (0 = no memory mapped io)");
+
+/* some architectures (e.g. intel xscale) align the 8bit serial registers
+   on 32bit word boundaries.
+   See linux-kernel/serial/8250.c serial_in()/out() */
+module_param(ioshift, int, 0444);
+MODULE_PARM_DESC(ioshift, "shift I/O register offset (0 = no shift)");
+#endif
+
+module_param(irq, int, 0444);
+MODULE_PARM_DESC(irq, "Interrupt (4 or 3)");
+
+module_param(share_irq, bool, 0444);
+MODULE_PARM_DESC(share_irq, "Share interrupts (0 = off, 1 = on)");
+
+module_param(sense, bool, 0444);
+MODULE_PARM_DESC(sense, "Override autodetection of IR receiver circuit"
+		 " (0 = active high, 1 = active low )");
+
+#ifdef LIRC_SERIAL_TRANSMITTER
+module_param(txsense, bool, 0444);
+MODULE_PARM_DESC(txsense, "Sense of transmitter circuit"
+		 " (0 = active high, 1 = active low )");
+#endif
+
+module_param(softcarrier, bool, 0444);
+MODULE_PARM_DESC(softcarrier, "Software carrier (0 = off, 1 = on)");
+
+module_param(debug, bool, 0644);
+MODULE_PARM_DESC(debug, "Enable debugging messages");
+
+#endif /* MODULE */
-- 
1.6.0.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-09  4:05   ` [PATCH 02/18] lirc serial port receiver/transmitter device driver Jarod Wilson
@ 2008-09-09 16:14     ` Jonathan Corbet
  2008-09-09 19:51       ` Stefan Lippers-Hollmann
  2008-09-10 17:40       ` Jarod Wilson
  0 siblings, 2 replies; 22+ messages in thread
From: Jonathan Corbet @ 2008-09-09 16:14 UTC (permalink / raw)
  To: Jarod Wilson
  Cc: linux-kernel, Jarod Wilson, Jarod Wilson, Janne Grunau,
	Christoph Bartelmus

> +config LIRC_SERIAL
> +	tristate "Homebrew Serial Port Receiver"
> +	default n
> +	depends on LIRC_DEV
> +	help
> +	  Driver for Homebrew Serial Port Receivers

What receivers might these be?  Do any actually exist?

> +#ifdef LIRC_SERIAL_IRDEO
> +static int type = LIRC_IRDEO;
> +#elif defined(LIRC_SERIAL_IRDEO_REMOTE)
> +static int type = LIRC_IRDEO_REMOTE;
> +#elif defined(LIRC_SERIAL_ANIMAX)
> +static int type = LIRC_ANIMAX;
> +#elif defined(LIRC_SERIAL_IGOR)
> +static int type = LIRC_IGOR;
> +#elif defined(LIRC_SERIAL_NSLU2)
> +static int type = LIRC_NSLU2;
> +#else
> +static int type = LIRC_HOMEBREW;
> +#endif

So where do all these LIRC_SERIAL_* macros come from?  I can't really tell
what this bunch of ifdeffery is doing or how one might influence it.

> +
> +static struct lirc_serial hardware[] = {
> +	/* home-brew receiver/transmitter */
> +	{
> +		UART_MSR_DCD,
> +		UART_MSR_DDCD,
> +		UART_MCR_RTS|UART_MCR_OUT2|UART_MCR_DTR,
> +		UART_MCR_RTS|UART_MCR_OUT2,
> +		send_pulse_homebrew,
> +		send_space_homebrew,
> +		(
> +#ifdef LIRC_SERIAL_TRANSMITTER
> +		 LIRC_CAN_SET_SEND_DUTY_CYCLE|
> +		 LIRC_CAN_SET_SEND_CARRIER|
> +		 LIRC_CAN_SEND_PULSE|
> +#endif
> +		 LIRC_CAN_REC_MODE2)
> +	},

It would be really nice to use the .field=value structure initialization
conventions here.

> +#if defined(__i386__)
> +/*
> +  From:
> +  Linux I/O port programming mini-HOWTO
> +  Author: Riku Saikkonen <Riku.Saikkonen@hut.fi>
> +  v, 28 December 1997
> +
> +  [...]
> +  Actually, a port I/O instruction on most ports in the 0-0x3ff range
> +  takes almost exactly 1 microsecond, so if you're, for example,using
> +  the parallel port directly, just do additional inb()s from that port
> +  to delay.
> +  [...]
> +*/
> +/* transmitter latency 1.5625us 0x1.90 - this figure arrived at from
> + * comment above plus trimming to match actual measured frequency.
> + * This will be sensitive to cpu speed, though hopefully most of the 1.5us
> + * is spent in the uart access.  Still - for reference test machine was a
> + * 1.13GHz Athlon system - Steve
> + */
> +
> +/* changed from 400 to 450 as this works better on slower machines;
> +   faster machines will use the rdtsc code anyway */
> +
> +#define LIRC_SERIAL_TRANSMITTER_LATENCY 450
> +
> +#else
> +
> +/* does anybody have information on other platforms ? */
> +/* 256 = 1<<8 */
> +#define LIRC_SERIAL_TRANSMITTER_LATENCY 256
> +
> +#endif  /* __i386__ */

This is a little scary.  Maybe hrtimers would be a better way of handling
your timing issues?

> +static inline unsigned int sinp(int offset)
> +{
> +#if defined(LIRC_ALLOW_MMAPPED_IO)
> +	if (iommap != 0) {
> +		/* the register is memory-mapped */
> +		offset <<= ioshift;
> +		return readb(io + offset);
> +	}
> +#endif
> +	return inb(io + offset);
> +}

This all looks like a reimplementation of ioport_map() and the associated
ioread*() and iowrite*() functions...?

> +#ifdef USE_RDTSC
> +/* Version that uses Pentium rdtsc instruction to measure clocks */
> +
> +/* This version does sub-microsecond timing using rdtsc instruction,
> + * and does away with the fudged LIRC_SERIAL_TRANSMITTER_LATENCY
> + * Implicitly i586 architecture...  - Steve
> + */
> +
> +static inline long send_pulse_homebrew_softcarrier(unsigned long length) 
> +{
> +	int flag;
> +	unsigned long target, start, now;
> +
> +	/* Get going quick as we can */
> +	rdtscl(start); on();
> +	/* Convert length from microseconds to clocks */
> +	length *= conv_us_to_clocks;
> +	/* And loop till time is up - flipping at right intervals */
> +	now = start;
> +	target = pulse_width;
> +	flag = 1;
> +	while ((now-start) < length) {
> +		/* Delay till flip time */
> +		do {
> +			rdtscl(now);
> +		} while ((now-start) < target);

This looks like a hard busy wait, without even an occasional, polite,
cpu_relax() call.  There's got to be a better way?

The i2c code has the result of a lot of bit-banging work, I wonder if
there's something there which could be helpful here.

> +static irqreturn_t irq_handler(int i, void *blah)
> +{
> +	struct timeval tv;
> +	int status, counter, dcd;
> +	long deltv;
> +	int data;
> +	static int last_dcd = -1;
> +
> +	if ((sinp(UART_IIR) & UART_IIR_NO_INT)) {
> +		/* not our interrupt */
> +		return IRQ_RETVAL(IRQ_NONE);
> +	}

That should just be IRQ_NONE, no?

> +static void hardware_init_port(void)
> +{
> +	unsigned long flags;
> +	local_irq_save(flags);

That won't help you if an interrupt is handled by another processor.  This
needs proper locking, methinks.

Nothing in this function does anything to assure itself that the port
actually exists and is the right kind of hardware.  Maybe that can't really
be done with this kind of device?

> +static int init_port(void)
> +{
 ...
> +	if (sense == -1) {
> +		/* wait 1/2 sec for the power supply */
> +
> +		set_current_state(TASK_INTERRUPTIBLE);
> +		schedule_timeout(HZ/2);

msleep(), maybe?

> +static int set_use_inc(void *data)
> +{
> +	int result;
> +	unsigned long flags;
> +
> +	/* Init read buffer. */
> +	if (lirc_buffer_init(&rbuf, sizeof(int), RBUF_LEN) < 0)
> +		return -ENOMEM;
> +
> +	/* initialize timestamp */
> +	do_gettimeofday(&lasttv);
> +
> +	result = request_irq(irq, irq_handler,
> +			   IRQF_DISABLED | (share_irq ? IRQF_SHARED : 0),
> +			   LIRC_DRIVER_NAME, (void *)&hardware);
> +
> +	switch (result) {
> +	case -EBUSY:
> +		printk(KERN_ERR LIRC_DRIVER_NAME ": IRQ %d busy\n", irq);
> +		lirc_buffer_free(&rbuf);
> +		return -EBUSY;
> +	case -EINVAL:
> +		printk(KERN_ERR LIRC_DRIVER_NAME
> +		       ": Bad irq number or handler\n");
> +		lirc_buffer_free(&rbuf);
> +		return -EINVAL;
> +	default:
> +		dprintk("Interrupt %d, port %04x obtained\n", irq,
> io);
> +		break;
> +	};
> +
> +	local_irq_save(flags);
> +
> +	/* Set DLAB 0. */
> +	soutp(UART_LCR, sinp(UART_LCR) & (~UART_LCR_DLAB));
> +
> +	soutp(UART_IER, sinp(UART_IER)|UART_IER_MSI);
> +
> +	local_irq_restore(flags);
> +
> +	return 0;
> +}

OK, so set_use_inc() really is just an open() function.  It still seems
like a strange duplication.

Again, local_irq_save() seems insufficient here.  You need a lock to
serialize access to the hardware.

jon

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-09 16:14     ` Jonathan Corbet
@ 2008-09-09 19:51       ` Stefan Lippers-Hollmann
  2008-09-09 19:56         ` Jarod Wilson
  2008-09-10 17:40       ` Jarod Wilson
  1 sibling, 1 reply; 22+ messages in thread
From: Stefan Lippers-Hollmann @ 2008-09-09 19:51 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: Jarod Wilson, linux-kernel, Jarod Wilson, Janne Grunau,
	Christoph Bartelmus

Hi

On Dienstag, 9. September 2008, Jonathan Corbet wrote:
> > +config LIRC_SERIAL
> > +	tristate "Homebrew Serial Port Receiver"
> > +	default n
> > +	depends on LIRC_DEV
> > +	help
> > +	  Driver for Homebrew Serial Port Receivers
> 
> What receivers might these be?  Do any actually exist?
[...]

This might actually be the most common setup, as it is very easy to 
build[1] with minimal soldering skills and has also been used by various 
vendors (older models of the TechniSat SkyStar2 DVB-S budget card, to name 
just one example; it is only slowly being replaced by alternative means, 
as serial ports are becoming rare on newer systems.

Regards
	Stefan Lippers-Hollmann

[1]	http://www.lirc.org/receivers.html

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-09 19:51       ` Stefan Lippers-Hollmann
@ 2008-09-09 19:56         ` Jarod Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Jarod Wilson @ 2008-09-09 19:56 UTC (permalink / raw)
  To: Stefan Lippers-Hollmann, Janne Grunau
  Cc: Jonathan Corbet, linux-kernel, Christoph Bartelmus

On Tuesday 09 September 2008 15:51:12 Stefan Lippers-Hollmann wrote:
> Hi
>
> On Dienstag, 9. September 2008, Jonathan Corbet wrote:
> > > +config LIRC_SERIAL
> > > +	tristate "Homebrew Serial Port Receiver"
> > > +	default n
> > > +	depends on LIRC_DEV
> > > +	help
> > > +	  Driver for Homebrew Serial Port Receivers
> >
> > What receivers might these be?  Do any actually exist?
>
> [...]
>
> This might actually be the most common setup, as it is very easy to
> build[1] with minimal soldering skills and has also been used by various
> vendors (older models of the TechniSat SkyStar2 DVB-S budget card, to name
> just one example; it is only slowly being replaced by alternative means,
> as serial ports are becoming rare on newer systems.
>
> Regards
> 	Stefan Lippers-Hollmann
>
> [1]	http://www.lirc.org/receivers.html

One frequently-used source of pre-built serial receivers (and transmitters) 
here in the US is http://irblaster.info/.

Hey, that actually just reminded me... I think I *do* have a serial IR 
receiver laying about somewhere that came w/one of my TechniSat SkyStar 
HD-5000 cards... Thank you, Stefan! ;)

-- 
Jarod Wilson
jarod@redhat.com


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-09 16:14     ` Jonathan Corbet
  2008-09-09 19:51       ` Stefan Lippers-Hollmann
@ 2008-09-10 17:40       ` Jarod Wilson
  1 sibling, 0 replies; 22+ messages in thread
From: Jarod Wilson @ 2008-09-10 17:40 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: linux-kernel, Jarod Wilson, Janne Grunau, Christoph Bartelmus

On Tuesday 09 September 2008 12:14:22 Jonathan Corbet wrote:
> > +#ifdef LIRC_SERIAL_IRDEO
> > +static int type = LIRC_IRDEO;
> > +#elif defined(LIRC_SERIAL_IRDEO_REMOTE)
> > +static int type = LIRC_IRDEO_REMOTE;
> > +#elif defined(LIRC_SERIAL_ANIMAX)
> > +static int type = LIRC_ANIMAX;
> > +#elif defined(LIRC_SERIAL_IGOR)
> > +static int type = LIRC_IGOR;
> > +#elif defined(LIRC_SERIAL_NSLU2)
> > +static int type = LIRC_NSLU2;
> > +#else
> > +static int type = LIRC_HOMEBREW;
> > +#endif
>
> So where do all these LIRC_SERIAL_* macros come from?  I can't really tell
> what this bunch of ifdeffery is doing or how one might influence it.

Bleah. I believe these get passed in when building lirc userspace and drivers 
together, when manually selected the specific type of serial receiver you have 
in lirc's menu-driven configuration tool thingy...

In other words, they do us absolutely no good here. We just build for the 
default type (LIRC_HOMEBREW), and users can override the type as necessary via 
the 'type' module parameter. I'll nuke that chunk.

> > +
> > +static struct lirc_serial hardware[] = {
> > +	/* home-brew receiver/transmitter */
> > +	{
> > +		UART_MSR_DCD,
> > +		UART_MSR_DDCD,
> > +		UART_MCR_RTS|UART_MCR_OUT2|UART_MCR_DTR,
> > +		UART_MCR_RTS|UART_MCR_OUT2,
> > +		send_pulse_homebrew,
> > +		send_space_homebrew,
> > +		(
> > +#ifdef LIRC_SERIAL_TRANSMITTER
> > +		 LIRC_CAN_SET_SEND_DUTY_CYCLE|
> > +		 LIRC_CAN_SET_SEND_CARRIER|
> > +		 LIRC_CAN_SEND_PULSE|
> > +#endif
> > +		 LIRC_CAN_REC_MODE2)
> > +	},
>
> It would be really nice to use the .field=value structure initialization
> conventions here.

Indeed. Done.

> > +#if defined(__i386__)
> > +/*
> > +  From:
> > +  Linux I/O port programming mini-HOWTO
> > +  Author: Riku Saikkonen <Riku.Saikkonen@hut.fi>
> > +  v, 28 December 1997
> > +
> > +  [...]
> > +  Actually, a port I/O instruction on most ports in the 0-0x3ff range
> > +  takes almost exactly 1 microsecond, so if you're, for example,using
> > +  the parallel port directly, just do additional inb()s from that port
> > +  to delay.
> > +  [...]
> > +*/
> > +/* transmitter latency 1.5625us 0x1.90 - this figure arrived at from
> > + * comment above plus trimming to match actual measured frequency.
> > + * This will be sensitive to cpu speed, though hopefully most of the
> > 1.5us + * is spent in the uart access.  Still - for reference test
> > machine was a + * 1.13GHz Athlon system - Steve
> > + */
> > +
> > +/* changed from 400 to 450 as this works better on slower machines;
> > +   faster machines will use the rdtsc code anyway */
> > +
> > +#define LIRC_SERIAL_TRANSMITTER_LATENCY 450
> > +
> > +#else
> > +
> > +/* does anybody have information on other platforms ? */
> > +/* 256 = 1<<8 */
> > +#define LIRC_SERIAL_TRANSMITTER_LATENCY 256
> > +
> > +#endif  /* __i386__ */
>
> This is a little scary.  Maybe hrtimers would be a better way of handling
> your timing issues?

Sounds plausible, will look into it. Of course, this partially hinges on the 
USE_RDTSC bits, more comments just a little ways down...

> > +static inline unsigned int sinp(int offset)
> > +{
> > +#if defined(LIRC_ALLOW_MMAPPED_IO)
> > +	if (iommap != 0) {
> > +		/* the register is memory-mapped */
> > +		offset <<= ioshift;
> > +		return readb(io + offset);
> > +	}
> > +#endif
> > +	return inb(io + offset);
> > +}
>
> This all looks like a reimplementation of ioport_map() and the associated
> ioread*() and iowrite*() functions...?

Probably. Will see about using those instead.

> > +#ifdef USE_RDTSC
> > +/* Version that uses Pentium rdtsc instruction to measure clocks */
> > +
> > +/* This version does sub-microsecond timing using rdtsc instruction,
> > + * and does away with the fudged LIRC_SERIAL_TRANSMITTER_LATENCY
> > + * Implicitly i586 architecture...  - Steve
> > + */
> > +
> > +static inline long send_pulse_homebrew_softcarrier(unsigned long length)
> > +{
> > +	int flag;
> > +	unsigned long target, start, now;
> > +
> > +	/* Get going quick as we can */
> > +	rdtscl(start); on();
> > +	/* Convert length from microseconds to clocks */
> > +	length *= conv_us_to_clocks;
> > +	/* And loop till time is up - flipping at right intervals */
> > +	now = start;
> > +	target = pulse_width;
> > +	flag = 1;
> > +	while ((now-start) < length) {
> > +		/* Delay till flip time */
> > +		do {
> > +			rdtscl(now);
> > +		} while ((now-start) < target);
>
> This looks like a hard busy wait, without even an occasional, polite,
> cpu_relax() call.  There's got to be a better way?
>
> The i2c code has the result of a lot of bit-banging work, I wonder if
> there's something there which could be helpful here.

Hrm... So at some point in the past, there was an "#if defined(rdtscl)" in the 
lirc_serial.c code that triggered USE_RDTSC being defined. At the moment, 
there's nothing defining it (I probably overzealously nuked it during clean-
up), so we're not even touching the above code. However, this is supposed to 
be the "better" code path wrt producing a reliable waveform, at least on 
platforms with rdtscl... Will have to do some investigating... This actually 
affects whether or not we bother with hrtimers as suggested above too, as 
LIRC_SERIAL_TRANSMITTER_LATENCY is not used at all in the USE_RDTSC case.

> > +static irqreturn_t irq_handler(int i, void *blah)
> > +{
> > +	struct timeval tv;
> > +	int status, counter, dcd;
> > +	long deltv;
> > +	int data;
> > +	static int last_dcd = -1;
> > +
> > +	if ((sinp(UART_IIR) & UART_IIR_NO_INT)) {
> > +		/* not our interrupt */
> > +		return IRQ_RETVAL(IRQ_NONE);
> > +	}
>
> That should just be IRQ_NONE, no?

Yeah, looks like it. Done.

> > +static void hardware_init_port(void)
> > +{
> > +	unsigned long flags;
> > +	local_irq_save(flags);
>
> That won't help you if an interrupt is handled by another processor.  This
> needs proper locking, methinks.

Yeah, working on implementing locking right now.

> Nothing in this function does anything to assure itself that the port
> actually exists and is the right kind of hardware.  Maybe that can't really
> be done with this kind of device?

We should probably try to make sure the port actually exists, but I don't 
think there's a whole lot (if anything) we can do as far as verifying the 
device itself.

> > +static int init_port(void)
> > +{
>
>  ...
>
> > +	if (sense == -1) {
> > +		/* wait 1/2 sec for the power supply */
> > +
> > +		set_current_state(TASK_INTERRUPTIBLE);
> > +		schedule_timeout(HZ/2);
>
> msleep(), maybe?

Yeah, looks like it.

> > +static int set_use_inc(void *data)
> > +{
> > +	int result;
> > +	unsigned long flags;
> > +
> > +	/* Init read buffer. */
> > +	if (lirc_buffer_init(&rbuf, sizeof(int), RBUF_LEN) < 0)
> > +		return -ENOMEM;
> > +
> > +	/* initialize timestamp */
> > +	do_gettimeofday(&lasttv);
> > +
> > +	result = request_irq(irq, irq_handler,
> > +			   IRQF_DISABLED | (share_irq ? IRQF_SHARED : 0),
> > +			   LIRC_DRIVER_NAME, (void *)&hardware);
> > +
> > +	switch (result) {
> > +	case -EBUSY:
> > +		printk(KERN_ERR LIRC_DRIVER_NAME ": IRQ %d busy\n", irq);
> > +		lirc_buffer_free(&rbuf);
> > +		return -EBUSY;
> > +	case -EINVAL:
> > +		printk(KERN_ERR LIRC_DRIVER_NAME
> > +		       ": Bad irq number or handler\n");
> > +		lirc_buffer_free(&rbuf);
> > +		return -EINVAL;
> > +	default:
> > +		dprintk("Interrupt %d, port %04x obtained\n", irq,
> > io);
> > +		break;
> > +	};
> > +
> > +	local_irq_save(flags);
> > +
> > +	/* Set DLAB 0. */
> > +	soutp(UART_LCR, sinp(UART_LCR) & (~UART_LCR_DLAB));
> > +
> > +	soutp(UART_IER, sinp(UART_IER)|UART_IER_MSI);
> > +
> > +	local_irq_restore(flags);
> > +
> > +	return 0;
> > +}
>
> OK, so set_use_inc() really is just an open() function.  It still seems
> like a strange duplication.

Going to let the duplication be for the moment, since I don't know the history 
behind why there's duplication, and there are enough other mountains to climb 
first... :)

> Again, local_irq_save() seems insufficient here.  You need a lock to
> serialize access to the hardware.

Will do.

-- 
Jarod Wilson
jarod@redhat.com


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
@ 2008-09-11 19:49 Stefan Bauer
  2008-09-12 16:24 ` Jarod Wilson
  0 siblings, 1 reply; 22+ messages in thread
From: Stefan Bauer @ 2008-09-11 19:49 UTC (permalink / raw)
  To: Jarod Wilson
  Cc: linux-kernel, Jarod Wilson, Janne Grunau, Christoph Bartelmus

Jarod Wilson wrote:

> On Tuesday 09 September 2008 12:14:22 Jonathan Corbet wrote:
>> > +#ifdef LIRC_SERIAL_IRDEO
>> > +static int type = LIRC_IRDEO;
>> > +#elif defined(LIRC_SERIAL_IRDEO_REMOTE)
>> > +static int type = LIRC_IRDEO_REMOTE;
>> > +#elif defined(LIRC_SERIAL_ANIMAX)
>> > +static int type = LIRC_ANIMAX;
>> > +#elif defined(LIRC_SERIAL_IGOR)
>> > +static int type = LIRC_IGOR;
>> > +#elif defined(LIRC_SERIAL_NSLU2)
>> > +static int type = LIRC_NSLU2;
>> > +#else
>> > +static int type = LIRC_HOMEBREW;
>> > +#endif
>>
>> So where do all these LIRC_SERIAL_* macros come from?  I can't really
>> tell what this bunch of ifdeffery is doing or how one might influence it.
> 
> Bleah. I believe these get passed in when building lirc userspace and
> drivers together, when manually selected the specific type of serial
> receiver you have in lirc's menu-driven configuration tool thingy...
> 
> In other words, they do us absolutely no good here. We just build for the
> default type (LIRC_HOMEBREW), and users can override the type as necessary
> via the 'type' module parameter. I'll nuke that chunk.
> 
>> > +
>> > +static struct lirc_serial hardware[] = {
>> > +	/* home-brew receiver/transmitter */
>> > +	{
>> > +		UART_MSR_DCD,
>> > +		UART_MSR_DDCD,
>> > +		UART_MCR_RTS|UART_MCR_OUT2|UART_MCR_DTR,
>> > +		UART_MCR_RTS|UART_MCR_OUT2,
>> > +		send_pulse_homebrew,
>> > +		send_space_homebrew,
>> > +		(
>> > +#ifdef LIRC_SERIAL_TRANSMITTER
>> > +		 LIRC_CAN_SET_SEND_DUTY_CYCLE|
>> > +		 LIRC_CAN_SET_SEND_CARRIER|
>> > +		 LIRC_CAN_SEND_PULSE|
>> > +#endif
>> > +		 LIRC_CAN_REC_MODE2)
>> > +	},
>>
>> It would be really nice to use the .field=value structure initialization
>> conventions here.
> 
> Indeed. Done.
> 
>> > +#if defined(__i386__)
>> > +/*
>> > +  From:
>> > +  Linux I/O port programming mini-HOWTO
>> > +  Author: Riku Saikkonen <Riku.Saikkonen@hut.fi>
>> > +  v, 28 December 1997
>> > +
>> > +  [...]
>> > +  Actually, a port I/O instruction on most ports in the 0-0x3ff range
>> > +  takes almost exactly 1 microsecond, so if you're, for example,using
>> > +  the parallel port directly, just do additional inb()s from that port
>> > +  to delay.
>> > +  [...]
>> > +*/
>> > +/* transmitter latency 1.5625us 0x1.90 - this figure arrived at from
>> > + * comment above plus trimming to match actual measured frequency.
>> > + * This will be sensitive to cpu speed, though hopefully most of the
>> > 1.5us + * is spent in the uart access.  Still - for reference test
>> > machine was a + * 1.13GHz Athlon system - Steve
>> > + */
>> > +
>> > +/* changed from 400 to 450 as this works better on slower machines;
>> > +   faster machines will use the rdtsc code anyway */
>> > +
>> > +#define LIRC_SERIAL_TRANSMITTER_LATENCY 450
>> > +
>> > +#else
>> > +
>> > +/* does anybody have information on other platforms ? */
>> > +/* 256 = 1<<8 */
>> > +#define LIRC_SERIAL_TRANSMITTER_LATENCY 256
>> > +
>> > +#endif  /* __i386__ */
>>
>> This is a little scary.  Maybe hrtimers would be a better way of handling
>> your timing issues?
> 
> Sounds plausible, will look into it. Of course, this partially hinges on
> the USE_RDTSC bits, more comments just a little ways down...
> 
>> > +static inline unsigned int sinp(int offset)
>> > +{
>> > +#if defined(LIRC_ALLOW_MMAPPED_IO)
>> > +	if (iommap != 0) {
>> > +		/* the register is memory-mapped */
>> > +		offset <<= ioshift;
>> > +		return readb(io + offset);
>> > +	}
>> > +#endif
>> > +	return inb(io + offset);
>> > +}
>>
>> This all looks like a reimplementation of ioport_map() and the associated
>> ioread*() and iowrite*() functions...?
> 
> Probably. Will see about using those instead.
> 
>> > +#ifdef USE_RDTSC
>> > +/* Version that uses Pentium rdtsc instruction to measure clocks */
>> > +
>> > +/* This version does sub-microsecond timing using rdtsc instruction,
>> > + * and does away with the fudged LIRC_SERIAL_TRANSMITTER_LATENCY
>> > + * Implicitly i586 architecture...  - Steve
>> > + */
>> > +
>> > +static inline long send_pulse_homebrew_softcarrier(unsigned long
>> > length) +{
>> > +	int flag;
>> > +	unsigned long target, start, now;
>> > +
>> > +	/* Get going quick as we can */
>> > +	rdtscl(start); on();
>> > +	/* Convert length from microseconds to clocks */
>> > +	length *= conv_us_to_clocks;
>> > +	/* And loop till time is up - flipping at right intervals */
>> > +	now = start;
>> > +	target = pulse_width;
>> > +	flag = 1;
>> > +	while ((now-start) < length) {
>> > +		/* Delay till flip time */
>> > +		do {
>> > +			rdtscl(now);
>> > +		} while ((now-start) < target);
>>
>> This looks like a hard busy wait, without even an occasional, polite,
>> cpu_relax() call.  There's got to be a better way?
>>
>> The i2c code has the result of a lot of bit-banging work, I wonder if
>> there's something there which could be helpful here.
> 
> Hrm... So at some point in the past, there was an "#if defined(rdtscl)" in
> the lirc_serial.c code that triggered USE_RDTSC being defined. At the
> moment, there's nothing defining it (I probably overzealously nuked it
> during clean- up), so we're not even touching the above code. However,
> this is supposed to be the "better" code path wrt producing a reliable
> waveform, at least on platforms with rdtscl... Will have to do some
> investigating... This actually affects whether or not we bother with
> hrtimers as suggested above too, as LIRC_SERIAL_TRANSMITTER_LATENCY is not
> used at all in the USE_RDTSC case.
> 
>> > +static irqreturn_t irq_handler(int i, void *blah)
>> > +{
>> > +	struct timeval tv;
>> > +	int status, counter, dcd;
>> > +	long deltv;
>> > +	int data;
>> > +	static int last_dcd = -1;
>> > +
>> > +	if ((sinp(UART_IIR) & UART_IIR_NO_INT)) {
>> > +		/* not our interrupt */
>> > +		return IRQ_RETVAL(IRQ_NONE);
>> > +	}
>>
>> That should just be IRQ_NONE, no?
> 
> Yeah, looks like it. Done.
> 
>> > +static void hardware_init_port(void)
>> > +{
>> > +	unsigned long flags;
>> > +	local_irq_save(flags);
>>
>> That won't help you if an interrupt is handled by another processor. 
>> This needs proper locking, methinks.
> 
> Yeah, working on implementing locking right now.
> 
>> Nothing in this function does anything to assure itself that the port
>> actually exists and is the right kind of hardware.  Maybe that can't
>> really be done with this kind of device?
> 
> We should probably try to make sure the port actually exists, but I don't
> think there's a whole lot (if anything) we can do as far as verifying the
> device itself.
> 
>> > +static int init_port(void)
>> > +{
>>
>>  ...
>>
>> > +	if (sense == -1) {
>> > +		/* wait 1/2 sec for the power supply */
>> > +
>> > +		set_current_state(TASK_INTERRUPTIBLE);
>> > +		schedule_timeout(HZ/2);
>>
>> msleep(), maybe?
> 
> Yeah, looks like it.
> 
>> > +static int set_use_inc(void *data)
>> > +{
>> > +	int result;
>> > +	unsigned long flags;
>> > +
>> > +	/* Init read buffer. */
>> > +	if (lirc_buffer_init(&rbuf, sizeof(int), RBUF_LEN) < 0)
>> > +		return -ENOMEM;
>> > +
>> > +	/* initialize timestamp */
>> > +	do_gettimeofday(&lasttv);
>> > +
>> > +	result = request_irq(irq, irq_handler,
>> > +			   IRQF_DISABLED | (share_irq ? IRQF_SHARED : 0),
>> > +			   LIRC_DRIVER_NAME, (void *)&hardware);
>> > +
>> > +	switch (result) {
>> > +	case -EBUSY:
>> > +		printk(KERN_ERR LIRC_DRIVER_NAME ": IRQ %d busy\n", irq);
>> > +		lirc_buffer_free(&rbuf);
>> > +		return -EBUSY;
>> > +	case -EINVAL:
>> > +		printk(KERN_ERR LIRC_DRIVER_NAME
>> > +		       ": Bad irq number or handler\n");
>> > +		lirc_buffer_free(&rbuf);
>> > +		return -EINVAL;
>> > +	default:
>> > +		dprintk("Interrupt %d, port %04x obtained\n", irq,
>> > io);
>> > +		break;
>> > +	};
>> > +
>> > +	local_irq_save(flags);
>> > +
>> > +	/* Set DLAB 0. */
>> > +	soutp(UART_LCR, sinp(UART_LCR) & (~UART_LCR_DLAB));
>> > +
>> > +	soutp(UART_IER, sinp(UART_IER)|UART_IER_MSI);
>> > +
>> > +	local_irq_restore(flags);
>> > +
>> > +	return 0;
>> > +}
>>
>> OK, so set_use_inc() really is just an open() function.  It still seems
>> like a strange duplication.
> 
> Going to let the duplication be for the moment, since I don't know the
> history behind why there's duplication, and there are enough other
> mountains to climb first... :)
> 
>> Again, local_irq_save() seems insufficient here.  You need a lock to
>> serialize access to the hardware.
> 
> Will do.

I just want to thank you very much for your work and give you my Tested-By. 
Todays git (b2e9c18a32423310a309f94ea5a659c4bb264378) works well here with 
lirc-0.8.3 userspace on a Pentium 3/i815-system.

But I've had a section mismatch in the lirc code, don't know if this is 
serious.

WARNING: drivers/input/lirc/lirc_serial.o(.init.text+0x11e): Section mismatch 
in reference from the function init_module() to the 
function .exit.text:lirc_serial_exit()
The function __init init_module() references
a function __exit lirc_serial_exit().
This is often seen when error handling in the init function
uses functionality in the exit path.
The fix is often to remove the __exit annotation of
lirc_serial_exit() so it may be used outside an exit section.


Regards,
Stefan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-11 19:49 [PATCH 02/18] lirc serial port receiver/transmitter device driver Stefan Bauer
@ 2008-09-12 16:24 ` Jarod Wilson
  2008-09-13  0:26   ` Janne Grunau
  2008-09-13  7:09   ` Christoph Bartelmus
  0 siblings, 2 replies; 22+ messages in thread
From: Jarod Wilson @ 2008-09-12 16:24 UTC (permalink / raw)
  To: Stefan Bauer, Janne Grunau; +Cc: linux-kernel, Christoph Bartelmus

On Thursday 11 September 2008 15:49:25 Stefan Bauer wrote:
> Jarod Wilson wrote:
[...]
> >> > +static inline unsigned int sinp(int offset)
> >> > +{
> >> > +#if defined(LIRC_ALLOW_MMAPPED_IO)
> >> > +	if (iommap != 0) {
> >> > +		/* the register is memory-mapped */
> >> > +		offset <<= ioshift;
> >> > +		return readb(io + offset);
> >> > +	}
> >> > +#endif
> >> > +	return inb(io + offset);
> >> > +}
> >>
> >> This all looks like a reimplementation of ioport_map() and the
> >> associated ioread*() and iowrite*() functions...?
> >
> > Probably. Will see about using those instead.

>From what I've been able to ascertain, reducing the above to...

static u8 sinp(int offset)
{
        if (iommap != 0)
                /* the register is memory-mapped */
                offset <<= ioshift;

        return inb(io + offset);
}

...should be sane. inb() either calls ioport_map() and ioread8() as needed, or 
defines inb() as readb() (or __raw_readb()) on platforms where its 
appropriate. (and similar for lirc_serial.c::soutp()).

> >> Nothing in this function does anything to assure itself that the port
> >> actually exists and is the right kind of hardware.  Maybe that can't
> >> really be done with this kind of device?
> >
> > We should probably try to make sure the port actually exists, but I don't
> > think there's a whole lot (if anything) we can do as far as verifying the
> > device itself.

I've borrowed the simple existence test from 
drivers/serial/8250.c::autoconfig(), which tries a few reads and writes from 
UART_IER. I think this is probably sufficient for verifying the port is legit.

Christoph B., you're definitely much more familiar with serial IR devices than 
I am... Is there any sort of test you can think of that we could use to try to 
verify the existence of an IR device hooked to the port? Or should we be happy 
we at least know there's a port and just assume the IR device is there?


> I just want to thank you very much for your work and give you my Tested-By.
> Todays git (b2e9c18a32423310a309f94ea5a659c4bb264378) works well here with
> lirc-0.8.3 userspace on a Pentium 3/i815-system.

Oh good! I haven't broken anything further w/my changes up to b2e9c18... ;) 
I've got another slew of updates to lirc_serial still pending though -- a few 
that I'm about to push, and at least one more chunk I still need to figure out 
(the hrtimers/rdtscl bits).

Continued testing would be much appreciated, since I still have no serial IR 
hardware myself (couldn't find the thingy that shipped with my Technisat card, 
but I do now have hardware on the way).

> But I've had a section mismatch in the lirc code, don't know if this is
> serious.
>
> WARNING: drivers/input/lirc/lirc_serial.o(.init.text+0x11e): Section
> mismatch in reference from the function init_module() to the
> function .exit.text:lirc_serial_exit()
> The function __init init_module() references
> a function __exit lirc_serial_exit().
> This is often seen when error handling in the init function
> uses functionality in the exit path.
> The fix is often to remove the __exit annotation of
> lirc_serial_exit() so it may be used outside an exit section.

Ah, I believe you're absolutely correct. Done.

-- 
Jarod Wilson
jarod@redhat.com


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-12 16:24 ` Jarod Wilson
@ 2008-09-13  0:26   ` Janne Grunau
  2008-09-13  8:41     ` Stefan Bauer
  2008-09-13  7:09   ` Christoph Bartelmus
  1 sibling, 1 reply; 22+ messages in thread
From: Janne Grunau @ 2008-09-13  0:26 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: Stefan Bauer, linux-kernel, Christoph Bartelmus

On Friday 12 September 2008 18:24:33 Jarod Wilson wrote:
> > I just want to thank you very much for your work and give you my
> > Tested-By. Todays git (b2e9c18a32423310a309f94ea5a659c4bb264378)
> > works well here with lirc-0.8.3 userspace on a Pentium
> > 3/i815-system.
>
> Oh good! I haven't broken anything further w/my changes up to
> b2e9c18... ;) I've got another slew of updates to lirc_serial still
> pending though

I hope I haven't broken anything with my lirc_dev changes. I doubt I'll 
have a change to test it before monday.

Janne

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-12 16:24 ` Jarod Wilson
  2008-09-13  0:26   ` Janne Grunau
@ 2008-09-13  7:09   ` Christoph Bartelmus
  1 sibling, 0 replies; 22+ messages in thread
From: Christoph Bartelmus @ 2008-09-13  7:09 UTC (permalink / raw)
  To: jwilson; +Cc: j, linux-kernel, stefan.bauer

Hi Jarod,

on 12 Sep 08 at 12:24, Jarod Wilson wrote:
> On Thursday 11 September 2008 15:49:25 Stefan Bauer wrote:
>> Jarod Wilson wrote:
>>>> Nothing in this function does anything to assure itself that the port
>>>> actually exists and is the right kind of hardware.  Maybe that can't
>>>> really be done with this kind of device?
>>>
>>> We should probably try to make sure the port actually exists, but I don't
>>> think there's a whole lot (if anything) we can do as far as verifying the
>>> device itself.

> I've borrowed the simple existence test from
> drivers/serial/8250.c::autoconfig(), which tries a few reads and writes from
> UART_IER. I think this is probably sufficient for verifying the port is
> legit.
>
> Christoph B., you're definitely much more familiar with serial IR devices
> than I am... Is there any sort of test you can think of that we could use to
> try to verify the existence of an IR device hooked to the port? Or should we
> be happy we at least know there's a port and just assume the IR device is
> there?

No, I can't think of anything else. OTOH there have never been real  
problems with this for the past 10 years...

Christoph

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-13  0:26   ` Janne Grunau
@ 2008-09-13  8:41     ` Stefan Bauer
  2008-09-15  3:55       ` Jarod Wilson
  0 siblings, 1 reply; 22+ messages in thread
From: Stefan Bauer @ 2008-09-13  8:41 UTC (permalink / raw)
  To: Janne Grunau; +Cc: Jarod Wilson, linux-kernel, Christoph Bartelmus

On Saturday 13 September 2008 02:26, Janne Grunau wrote:
> On Friday 12 September 2008 18:24:33 Jarod Wilson wrote:
> > > I just want to thank you very much for your work and give you my
> > > Tested-By. Todays git (b2e9c18a32423310a309f94ea5a659c4bb264378)
> > > works well here with lirc-0.8.3 userspace on a Pentium
> > > 3/i815-system.
> >
> > Oh good! I haven't broken anything further w/my changes up to
> > b2e9c18... ;) I've got another slew of updates to lirc_serial still
> > pending though
>
> I hope I haven't broken anything with my lirc_dev changes. I doubt I'll
> have a change to test it before monday.

Unfortunately, you did. Commit ea74897 (port lirc to dynamic device numbers) 
broke things.
This is what ea74897 and further (latest tested was dd13cc7) are telling me:

$ insmod drivers/input/lirc/lirc_dev.ko
$ insmod drivers/input/lirc/lirc_serial.ko
insmod: error inserting 'drivers/input/lirc/lirc_serial.ko': -1 Input/output 
error
$ dmesg | tail
lirc_dev: IR Remote Control driver registered, major 253 
lirc_serial: auto-detected active low receiver
lirc_dev: lirc_register_driver: sample_rate: 0
lirc_serial: register_chrdev failed!


There has also been a compile issue in the meanwhile, introduced with 95efa30 
(inb/outb and readb/writeb deal in u8 types), but this is gone with todays 
git pull :)


I'll keep on testing (unfortunately only at weekends),
regards, Stefan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-13  8:41     ` Stefan Bauer
@ 2008-09-15  3:55       ` Jarod Wilson
  2008-09-15 18:20         ` Jarod Wilson
  0 siblings, 1 reply; 22+ messages in thread
From: Jarod Wilson @ 2008-09-15  3:55 UTC (permalink / raw)
  To: Stefan Bauer; +Cc: Janne Grunau, linux-kernel, Christoph Bartelmus

On Saturday 13 September 2008 04:41:33 Stefan Bauer wrote:
> On Saturday 13 September 2008 02:26, Janne Grunau wrote:
> > On Friday 12 September 2008 18:24:33 Jarod Wilson wrote:
> > > > I just want to thank you very much for your work and give you my
> > > > Tested-By. Todays git (b2e9c18a32423310a309f94ea5a659c4bb264378)
> > > > works well here with lirc-0.8.3 userspace on a Pentium
> > > > 3/i815-system.
> > >
> > > Oh good! I haven't broken anything further w/my changes up to
> > > b2e9c18... ;) I've got another slew of updates to lirc_serial still
> > > pending though
> >
> > I hope I haven't broken anything with my lirc_dev changes. I doubt I'll
> > have a change to test it before monday.
>
> Unfortunately, you did. Commit ea74897 (port lirc to dynamic device
> numbers) broke things.
> This is what ea74897 and further (latest tested was dd13cc7) are telling
> me:
>
> $ insmod drivers/input/lirc/lirc_dev.ko
> $ insmod drivers/input/lirc/lirc_serial.ko
> insmod: error inserting 'drivers/input/lirc/lirc_serial.ko': -1
> Input/output error
> $ dmesg | tail
> lirc_dev: IR Remote Control driver registered, major 253
> lirc_serial: auto-detected active low receiver
> lirc_dev: lirc_register_driver: sample_rate: 0
> lirc_serial: register_chrdev failed!
>
>
> There has also been a compile issue in the meanwhile, introduced with
> 95efa30 (inb/outb and readb/writeb deal in u8 types), but this is gone with
> todays git pull :)
>
>
> I'll keep on testing (unfortunately only at weekends),
> regards, Stefan

I'd hope to get around to some testing myself much earlier in the weekend, but 
alas... Did just mix in a quick peed at lirc_i2c though:

...
lirc_dev: IR Remote Control driver registered, major 247 
bttv: driver version 0.9.17 loaded
bttv: using 8 buffers with 2080k (520 pages) each for capture
cx88/0: cx2388x v4l2 driver version 0.0.6 loaded
lirc_i2c: chip 0x10020 found @ 0x18 (Hauppauge IR)
lirc_dev: lirc_register_driver: sample_rate: 10

No register_chrdev failure reported, everything looks the same as prior to the 
dynamic dev num change (save the dev num, of course), and I've got a 
/dev/lirc0, but I'm unable to see any IR signals (start up lircd, run irw, 
press buttons on remote).

...and I just took a quick look at lirc_i2c... The result from 
lirc_register_driver() is never checked, whereas it is in the lirc_serial case 
(which is where the register_chrdev error msg came from). Narf. So its likely 
the same failure, just not noticed (will fix lirc_i2c in a sec).

-- 
Jarod Wilson
jarod@redhat.com


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-15  3:55       ` Jarod Wilson
@ 2008-09-15 18:20         ` Jarod Wilson
  2008-09-16  4:08           ` Jarod Wilson
  0 siblings, 1 reply; 22+ messages in thread
From: Jarod Wilson @ 2008-09-15 18:20 UTC (permalink / raw)
  To: Stefan Bauer; +Cc: Janne Grunau, linux-kernel, Christoph Bartelmus

On Sunday 14 September 2008 23:55:38 Jarod Wilson wrote:
> On Saturday 13 September 2008 04:41:33 Stefan Bauer wrote:
> > On Saturday 13 September 2008 02:26, Janne Grunau wrote:
> > > On Friday 12 September 2008 18:24:33 Jarod Wilson wrote:
> > > > > I just want to thank you very much for your work and give you my
> > > > > Tested-By. Todays git (b2e9c18a32423310a309f94ea5a659c4bb264378)
> > > > > works well here with lirc-0.8.3 userspace on a Pentium
> > > > > 3/i815-system.
> > > >
> > > > Oh good! I haven't broken anything further w/my changes up to
> > > > b2e9c18... ;) I've got another slew of updates to lirc_serial still
> > > > pending though
> > >
> > > I hope I haven't broken anything with my lirc_dev changes. I doubt I'll
> > > have a change to test it before monday.
> >
> > Unfortunately, you did. Commit ea74897 (port lirc to dynamic device
> > numbers) broke things.
> > This is what ea74897 and further (latest tested was dd13cc7) are telling
> > me:
> >
> > $ insmod drivers/input/lirc/lirc_dev.ko
> > $ insmod drivers/input/lirc/lirc_serial.ko
> > insmod: error inserting 'drivers/input/lirc/lirc_serial.ko': -1
> > Input/output error
> > $ dmesg | tail
> > lirc_dev: IR Remote Control driver registered, major 253
> > lirc_serial: auto-detected active low receiver
> > lirc_dev: lirc_register_driver: sample_rate: 0
> > lirc_serial: register_chrdev failed!
> >
> >
> > There has also been a compile issue in the meanwhile, introduced with
> > 95efa30 (inb/outb and readb/writeb deal in u8 types), but this is gone
> > with todays git pull :)
> >
> >
> > I'll keep on testing (unfortunately only at weekends),
> > regards, Stefan
>
> I'd hope to get around to some testing myself much earlier in the weekend,
> but alas... Did just mix in a quick peed at lirc_i2c though:
>
> ...
> lirc_dev: IR Remote Control driver registered, major 247
> bttv: driver version 0.9.17 loaded
> bttv: using 8 buffers with 2080k (520 pages) each for capture
> cx88/0: cx2388x v4l2 driver version 0.0.6 loaded
> lirc_i2c: chip 0x10020 found @ 0x18 (Hauppauge IR)
> lirc_dev: lirc_register_driver: sample_rate: 10
>
> No register_chrdev failure reported, everything looks the same as prior to
> the dynamic dev num change (save the dev num, of course), and I've got a
> /dev/lirc0, but I'm unable to see any IR signals (start up lircd, run irw,
> press buttons on remote).
>
> ...and I just took a quick look at lirc_i2c... The result from
> lirc_register_driver() is never checked, whereas it is in the lirc_serial
> case (which is where the register_chrdev error msg came from). Narf. So its
> likely the same failure, just not noticed (will fix lirc_i2c in a sec).

So I've done a bit of work to fix up a few drivers so they properly check to 
see that lirc_register_driver() actually succeeded, and then accidentally 
stumbled onto the fix for the failure when merging some coverity-inspired 
patches from Erik Hovland. lirc_dev.c's lirc_cdev_add() function had an 
inverted kmalloc failure check. With that fixed, lirc_i2c appears to be 
behaving for me now, although I can't actually check to be 100% certain until 
I get home tonight.

...
lirc_dev: IR Remote Control driver registered, major 240 
lirc_i2c: probe 0x1a @ ivtv i2c driver #0: no
lirc_i2c: probe 0x18 @ ivtv i2c driver #0: yes
lirc_i2c: chip 0x10020 found @ 0x18 (Hauppauge IR)
lirc_dev: lirc_register_driver: sample_rate: 10
lirc_dev: driver lirc_i2c registered at minor number = 0
lirc_dev (lirc_i2c[0]): poll thread started
lirc_i2c: ir_attach: success (minor 0)
...

Fixes are in the git tree now for anyone else who wants to or is willing to 
test. Will verify myself tonight w/lirc_i2c and lirc_mceusb2 at a minimum.

-- 
Jarod Wilson
jarod@redhat.com


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-15 18:20         ` Jarod Wilson
@ 2008-09-16  4:08           ` Jarod Wilson
  2008-09-18 14:00             ` Jarod Wilson
  2008-09-19 18:05             ` Stefan Bauer
  0 siblings, 2 replies; 22+ messages in thread
From: Jarod Wilson @ 2008-09-16  4:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: Janne Grunau, Stefan Bauer, Christoph Bartelmus

On Monday 15 September 2008 14:20:31 Jarod Wilson wrote:
> On Sunday 14 September 2008 23:55:38 Jarod Wilson wrote:
> > On Saturday 13 September 2008 04:41:33 Stefan Bauer wrote:
> > > On Saturday 13 September 2008 02:26, Janne Grunau wrote:
> > > > I hope I haven't broken anything with my lirc_dev changes. I doubt
> > > > I'll have a change to test it before monday.
> > >
> > > Unfortunately, you did. Commit ea74897 (port lirc to dynamic device
> > > numbers) broke things.
[...]
> So I've done a bit of work to fix up a few drivers so they properly check
> to see that lirc_register_driver() actually succeeded, and then
> accidentally stumbled onto the fix for the failure when merging some
> coverity-inspired patches from Erik Hovland. lirc_dev.c's lirc_cdev_add()
> function had an inverted kmalloc failure check. With that fixed, lirc_i2c
> appears to be behaving for me now, although I can't actually check to be
> 100% certain until I get home tonight.
[...]
> Fixes are in the git tree now for anyone else who wants to or is willing to
> test. Will verify myself tonight w/lirc_i2c and lirc_mceusb2 at a minimum.

Current git tree works for receiving IR signals via lirc_serial (tested by 
Janne) and via lirc_mceusb2 (tested by me). Something is still slightly 
haywire with lirc_i2c though. It registers fine, but IR signals simply aren't 
making it to lircd. Probably unrelated to the dynamic device num changes, 
looking into it.

-- 
Jarod Wilson
jarod@redhat.com


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-16  4:08           ` Jarod Wilson
@ 2008-09-18 14:00             ` Jarod Wilson
  2008-09-19 18:05             ` Stefan Bauer
  1 sibling, 0 replies; 22+ messages in thread
From: Jarod Wilson @ 2008-09-18 14:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: Janne Grunau, Stefan Bauer, Christoph Bartelmus, Erik Hovland

On Tuesday 16 September 2008 00:08:43 Jarod Wilson wrote:
> Current git tree works for receiving IR signals via lirc_serial (tested by
> Janne) and via lirc_mceusb2 (tested by me). Something is still slightly
> haywire with lirc_i2c though. It registers fine, but IR signals simply
> aren't making it to lircd. Probably unrelated to the dynamic device num
> changes, looking into it.

Despite compiling fine w/o #include'ing linux/i2c-algo-bit.h and coverity 
apparently thinking its not needed, its really quite necessary for things to 
function. Restored it, and lirc_i2c now works again.

-- 
Jarod Wilson
jarod@redhat.com


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-16  4:08           ` Jarod Wilson
  2008-09-18 14:00             ` Jarod Wilson
@ 2008-09-19 18:05             ` Stefan Bauer
  2008-09-19 18:26               ` Janne Grunau
  1 sibling, 1 reply; 22+ messages in thread
From: Stefan Bauer @ 2008-09-19 18:05 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: linux-kernel, Janne Grunau, Christoph Bartelmus

On Tuesday 16 September 2008 06:08, Jarod Wilson wrote:
> Current git tree works for receiving IR signals via lirc_serial (tested by
> Janne) and via lirc_mceusb2 (tested by me). Something is still slightly
> haywire with lirc_i2c though. It registers fine, but IR signals simply
> aren't making it to lircd. Probably unrelated to the dynamic device num
> changes, looking into it.

Thank you for keeping me up to date, here are my test results with todays 
(36e60bd457588b2a2fd37e0c1d0652a9fe214d85) git:

- Compiles fine, except some warnings:

drivers/input/lirc/lirc_dev.h:60:
  warning: 'lirc_buffer_available' defined but not used
drivers/input/lirc/lirc_dev.h:72:
  warning: 'lirc_buffer_write' defined but not used
drivers/input/lirc/lirc_dev.h:56:
  warning: 'lirc_buffer_empty' defined but not used
drivers/input/lirc/lirc_dev.h:60:
  warning: 'lirc_buffer_available' defined but not used
drivers/input/lirc/lirc_dev.h:66:
  warning: 'lirc_buffer_read' defined but not used

- Module loads fine, with this messages:

lirc_dev: IR Remote Control driver registered, major 252 
lirc_serial: auto-detected active low receiver
lirc_dev: lirc_register_driver: sample_rate: 0

- But lircd crashes immediately every time irexec/irw tries to connect:

BUG: unable to handle kernel NULL pointer dereference at 0000000c
IP: [<cabc9006>] :lirc_dev:lirc_buffer_clear+0x6/0x17
*pde = 00000000 
Oops: 0002 [#1] 
Modules linked in: lirc_serial lirc_dev ves1820 arc4 ecb crypto_blkcipher 
cryptomgr crypto_algapi rt61pci rt2x00pci rt2x00lib led_class dvb_ttpci 
saa7146_vv saa7146 videobuf_dma_sg videobuf_core mac80211 videodev 
v4l1_compat ttpci_eeprom ehci_hcd uhci_hcd cfg80211 eeprom_93cx6

Pid: 2501, comm: lircd Not tainted (2.6.27-rc5 #7)
EIP: 0060:[<cabc9006>] EFLAGS: 00010046 CPU: 0
EIP is at lirc_buffer_clear+0x6/0x17 [lirc_dev]
EAX: 00000000 EBX: c7825cc0 ECX: cabc94f1 EDX: 00000246
ESI: fffffff0 EDI: c726537c EBP: c6bdf000 ESP: c6b73eac
 DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
Process lircd (pid: 2501, ti=c6b72000 task=c72ef7a0 task.ti=c6b72000)
Stack: cabc958f 00000000 c7825d3c c014e15c 00000000 c6bdf000 c726537c 00000000 
       c014e09b c014b052 c78c2900 c759fd80 c6b73f14 c6bdf000 c6b73f14 00000003 
       c014b156 c6bdf000 00000000 00000000 00000000 c01544ae 00000002 00000026 
Call Trace:
 [<cabc958f>] irctl_open+0x9e/0x131 [lirc_dev]
 [<c014e15c>] chrdev_open+0xc1/0xf6
 [<c014e09b>] chrdev_open+0x0/0xf6
 [<c014b052>] __dentry_open+0xf2/0x1da
 [<c014b156>] nameidata_to_filp+0x1c/0x2c
 [<c01544ae>] do_filp_open+0x28b/0x545
 [<c015bc37>] alloc_fd+0x46/0xad
 [<c014ae8d>] do_sys_open+0x42/0xb6
 [<c014af45>] sys_open+0x1e/0x23
 [<c010297d>] sysenter_do_call+0x12/0x25
 [<c02f0000>] ioctl_standard_iw_point+0x12d/0x200
 =======================
Code: <c7> 40 0c 00 00 00 00 c7 40 08 00 00 00 00 52 9d c3 53 89 c3 8b 40 
EIP: [<cabc9006>] lirc_buffer_clear+0x6/0x17 [lirc_dev] SS:ESP 0068:c6b73eac
---[ end trace 782e5196b9791a10 ]---


If wanted I could start a bisect on this.


Regards,
Stefan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-19 18:05             ` Stefan Bauer
@ 2008-09-19 18:26               ` Janne Grunau
  2008-09-19 18:53                 ` Stefan Bauer
  2008-09-19 18:54                 ` Jarod Wilson
  0 siblings, 2 replies; 22+ messages in thread
From: Janne Grunau @ 2008-09-19 18:26 UTC (permalink / raw)
  To: Stefan Bauer; +Cc: Jarod Wilson, linux-kernel, Christoph Bartelmus

On Friday 19 September 2008 20:05:11 Stefan Bauer wrote:
>
> Thank you for keeping me up to date, here are my test results with
> todays (36e60bd457588b2a2fd37e0c1d0652a9fe214d85) git:

> - But lircd crashes immediately every time irexec/irw tries to
> connect:
>
> BUG: unable to handle kernel NULL pointer dereference at 0000000c

not necessary, it is
commit bc2039245c6390bd414a61c9395604c4155a58c6
Author: Janne Grunau <j@jannau.net>
Date:   Thu Sep 18 20:49:19 2008 +0200

    reimplement lirc_buffer as kfifo wrapper

Janne

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-19 18:26               ` Janne Grunau
@ 2008-09-19 18:53                 ` Stefan Bauer
  2008-09-19 19:24                   ` Janne Grunau
  2008-09-20  0:10                   ` Janne Grunau
  2008-09-19 18:54                 ` Jarod Wilson
  1 sibling, 2 replies; 22+ messages in thread
From: Stefan Bauer @ 2008-09-19 18:53 UTC (permalink / raw)
  To: Janne Grunau; +Cc: Jarod Wilson, linux-kernel, Christoph Bartelmus

On Friday 19 September 2008 20:26, Janne Grunau wrote:
> On Friday 19 September 2008 20:05:11 Stefan Bauer wrote:
> > Thank you for keeping me up to date, here are my test results with
> > todays (36e60bd457588b2a2fd37e0c1d0652a9fe214d85) git:
> >
> > - But lircd crashes immediately every time irexec/irw tries to
> > connect:
> >
> > BUG: unable to handle kernel NULL pointer dereference at 0000000c
>
> not necessary, it is
> commit bc2039245c6390bd414a61c9395604c4155a58c6
> Author: Janne Grunau <j@jannau.net>
> Date:   Thu Sep 18 20:49:19 2008 +0200
>
>     reimplement lirc_buffer as kfifo wrapper

Thank you for pointing me, but bc2039245c6390bd414a61c9395604c4155a58c6~1 
(lirc_streamzap can use lirc_buffer_clear) also crashes lircd when irexec is 
started.

BUG: unable to handle kernel NULL pointer dereference at 00000000
IP: [<c01118ce>] try_to_wake_up+0x16/0x7c
*pde = 00000000 
Oops: 0000 [#1] 
Modules linked in: lirc_serial lirc_dev ves1820 arc4 ecb crypto_blkcipher 
cryptomgr crypto_algapi rt61pci rt2x00pci rt2x00lib led_class dvb_ttpci 
saa7146_vv saa7146 mac80211 videobuf_dma_sg ehci_hcd uhci_hcd videobuf_core 
videodev v4l1_compat ttpci_eeprom cfg80211 eeprom_93cx6

Pid: 2939, comm: lircd Not tainted (2.6.27-rc5 #7)
EIP: 0060:[<c01118ce>] EFLAGS: 00010046 CPU: 0
EIP is at try_to_wake_up+0x16/0x7c
EAX: c04091c0 EBX: 0000000f ECX: 00000000 EDX: c3e85e98
ESI: c04091c0 EDI: 00000000 EBP: 00000000 ESP: c3e85e98
 DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
Process lircd (pid: 2939, ti=c3e84000 task=c3e866c0 task.ti=c3e84000)
Stack: 00000246 c6a16000 00000000 c691905c c6bed400 cadc05b3 00000000 c6a1607c 
       c014e15c 00000000 c6bed400 c691905c 00000000 c014e09b c014b052 c78c2a00 
       c774c180 c3e85f14 c6bed400 c3e85f14 00000003 c014b156 c6bed400 00000000 
Call Trace:
 [<cadc05b3>] irctl_open+0xdd/0x13c [lirc_dev]
 [<c014e15c>] chrdev_open+0xc1/0xf6
 [<c014e09b>] chrdev_open+0x0/0xf6
 [<c014b052>] __dentry_open+0xf2/0x1da
 [<c014b156>] nameidata_to_filp+0x1c/0x2c
 [<c01544ae>] do_filp_open+0x28b/0x545
 [<c015bc37>] alloc_fd+0x46/0xad
 [<c014ae8d>] do_sys_open+0x42/0xb6
 [<c014af45>] sys_open+0x1e/0x23
 [<c010297d>] sysenter_do_call+0x12/0x25
 [<c02f0000>] ioctl_standard_iw_point+0x12d/0x200
 =======================
Code: 00 00 00 00 5b 5e 5f c3 53 9c 5b fa e8 bb ff ff ff 53 9d 5b c3 55 57 89 
c7 56 53 89 d3 83 ec 04 89 e2 31 ed e8 3a fd ff ff 89 c6 <8b> 07 85 d8 74 41 
83 7f 48 00 75 25 31 c0 66 bd 01 00 e8 ba 40 
EIP: [<c01118ce>] try_to_wake_up+0x16/0x7c SS:ESP 0068:c3e85e98
---[ end trace 344ec27be5d90ccf ]---


Regards,
Stefan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-19 18:26               ` Janne Grunau
  2008-09-19 18:53                 ` Stefan Bauer
@ 2008-09-19 18:54                 ` Jarod Wilson
  2008-09-19 19:12                   ` Stefan Bauer
  1 sibling, 1 reply; 22+ messages in thread
From: Jarod Wilson @ 2008-09-19 18:54 UTC (permalink / raw)
  To: Janne Grunau; +Cc: Stefan Bauer, linux-kernel, Christoph Bartelmus

On Friday 19 September 2008 14:26:20 Janne Grunau wrote:
> On Friday 19 September 2008 20:05:11 Stefan Bauer wrote:
> > Thank you for keeping me up to date, here are my test results with
> > todays (36e60bd457588b2a2fd37e0c1d0652a9fe214d85) git:
> >
> > - But lircd crashes immediately every time irexec/irw tries to
> > connect:
> >
> > BUG: unable to handle kernel NULL pointer dereference at 0000000c
>
> not necessary, it is
> commit bc2039245c6390bd414a61c9395604c4155a58c6
> Author: Janne Grunau <j@jannau.net>
> Date:   Thu Sep 18 20:49:19 2008 +0200
>
>     reimplement lirc_buffer as kfifo wrapper

Hm... Two things... First, I neglected to pull some of Janne's later changes 
in the same area until just a bit ago, so perhaps this has already been 
remedied, because second, lirc_i2c is working as expected w/current head 
(98242cbdca...)


-- 
Jarod Wilson
jarod@redhat.com


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-19 18:54                 ` Jarod Wilson
@ 2008-09-19 19:12                   ` Stefan Bauer
  0 siblings, 0 replies; 22+ messages in thread
From: Stefan Bauer @ 2008-09-19 19:12 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: Janne Grunau, linux-kernel, Christoph Bartelmus

On Friday 19 September 2008 20:54, Jarod Wilson wrote:
> On Friday 19 September 2008 14:26:20 Janne Grunau wrote:
> > On Friday 19 September 2008 20:05:11 Stefan Bauer wrote:
> > > Thank you for keeping me up to date, here are my test results with
> > > todays (36e60bd457588b2a2fd37e0c1d0652a9fe214d85) git:
> > >
> > > - But lircd crashes immediately every time irexec/irw tries to
> > > connect:
> > >
> > > BUG: unable to handle kernel NULL pointer dereference at 0000000c
> >
> > not necessary, it is
> > commit bc2039245c6390bd414a61c9395604c4155a58c6
> > Author: Janne Grunau <j@jannau.net>
> > Date:   Thu Sep 18 20:49:19 2008 +0200
> >
> >     reimplement lirc_buffer as kfifo wrapper
>
> Hm... Two things... First, I neglected to pull some of Janne's later
> changes in the same area until just a bit ago, so perhaps this has already
> been remedied, because second, lirc_i2c is working as expected w/current
> head (98242cbdca...)

98242cbdca behaves exactly as 36e60bd4575.

-Stefan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-19 18:53                 ` Stefan Bauer
@ 2008-09-19 19:24                   ` Janne Grunau
  2008-09-20  0:10                   ` Janne Grunau
  1 sibling, 0 replies; 22+ messages in thread
From: Janne Grunau @ 2008-09-19 19:24 UTC (permalink / raw)
  To: Stefan Bauer; +Cc: Jarod Wilson, linux-kernel, Christoph Bartelmus

On Friday 19 September 2008 20:53:36 Stefan Bauer wrote:
> On Friday 19 September 2008 20:26, Janne Grunau wrote:
> > On Friday 19 September 2008 20:05:11 Stefan Bauer wrote:
> > > Thank you for keeping me up to date, here are my test results
> > > with todays (36e60bd457588b2a2fd37e0c1d0652a9fe214d85) git:
> > >
> > > - But lircd crashes immediately every time irexec/irw tries to
> > > connect:
> > >
> > > BUG: unable to handle kernel NULL pointer dereference at 0000000c
> >
> > not necessary, it is
> > commit bc2039245c6390bd414a61c9395604c4155a58c6
> > Author: Janne Grunau <j@jannau.net>
> > Date:   Thu Sep 18 20:49:19 2008 +0200
> >
> >     reimplement lirc_buffer as kfifo wrapper
>
> Thank you for pointing me, but
> bc2039245c6390bd414a61c9395604c4155a58c6~1 (lirc_streamzap can use
> lirc_buffer_clear) also crashes lircd when irexec is started.

ok, then it is not the obvious thing, I'll test in a couple of minutes

Janne

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-19 18:53                 ` Stefan Bauer
  2008-09-19 19:24                   ` Janne Grunau
@ 2008-09-20  0:10                   ` Janne Grunau
  2008-09-26 19:42                     ` Stefan Bauer
  1 sibling, 1 reply; 22+ messages in thread
From: Janne Grunau @ 2008-09-20  0:10 UTC (permalink / raw)
  To: Stefan Bauer; +Cc: Jarod Wilson, linux-kernel, Christoph Bartelmus

On Friday 19 September 2008 20:53:36 Stefan Bauer wrote:
> On Friday 19 September 2008 20:26, Janne Grunau wrote:
> >
> > not necessary, it is
> > commit bc2039245c6390bd414a61c9395604c4155a58c6
> > Author: Janne Grunau <j@jannau.net>
> > Date:   Thu Sep 18 20:49:19 2008 +0200
> >
> >     reimplement lirc_buffer as kfifo wrapper
>
> Thank you for pointing me, but
> bc2039245c6390bd414a61c9395604c4155a58c6~1 (lirc_streamzap can use
> lirc_buffer_clear) also crashes lircd when irexec is started.

yes, a different bug. both fixed in 
git://git.jannau.net/linux-2.6-lirc.git

but lirc serial still doesn't work after 
bc2039245c6390bd414a61c9395604c4155a58c6. I'll look at it tomorrow.

I have also good news: irrecord + lirc_serial is working with bc203924^. 
Not sure if something changed or if it was PEBCAK.

Janne

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/18] lirc serial port receiver/transmitter device driver
  2008-09-20  0:10                   ` Janne Grunau
@ 2008-09-26 19:42                     ` Stefan Bauer
  0 siblings, 0 replies; 22+ messages in thread
From: Stefan Bauer @ 2008-09-26 19:42 UTC (permalink / raw)
  To: Janne Grunau; +Cc: Jarod Wilson, linux-kernel, Christoph Bartelmus

Hi,

On Saturday 20 September 2008 02:10, Janne Grunau wrote:
> but lirc serial still doesn't work after
> bc2039245c6390bd414a61c9395604c4155a58c6. I'll look at it tomorrow.

just to tell you that I'm still there... :) Todays git 
(a842c8095aad7b71ffcd1e426185e53661d758c7) is also giving me this at first 
access to the device by lircd:

------------[ cut here ]------------
WARNING: at drivers/input/lirc/lirc_dev.h:37
lirc_buffer_clear+0x2c/0x30 [lirc_dev]()
calling lirc_buffer_clear on an uninitialized lirc_buffer
Modules linked in: lirc_serial lirc_dev ves1820 arc4 ecb crypto_blkcipher 
cryptomgr crypto_algapi rt61pci rt2x00pci rt2x00lib led_class dvb_ttpci 
saa7146_vv saa7146 videobuf_dma_sg mac80211 videobuf_core videodev ehci_hcd 
uhci_hcd ttpci_eeprom cfg80211 eeprom_93cx6 [last unloaded: lirc_dev]
Pid: 5144, comm: lircd Not tainted 2.6.27-rc5 #7
 [<c01151ce>] warn_slowpath+0x61/0x84
 [<c015971f>] __d_lookup+0x8b/0xbf
 [<c0151f26>] do_lookup+0x40/0x92
 [<c0158f6d>] dput+0x15/0x8d
 [<c0153533>] __link_path_walk+0x7ad/0x8b4
 [<c0153533>] __link_path_walk+0x7ad/0x8b4
 [<c01de0c1>] kobject_get+0xf/0x13
 [<c014dfbb>] cdev_get+0x1b/0x2d
 [<c014dfd4>] exact_lock+0x7/0xd
 [<c0242154>] kobj_lookup+0xda/0x104
 [<ca9fa15a>] lirc_buffer_clear+0x2c/0x30 [lirc_dev]
 [<ca9fa4f4>] lirc_dev_fop_open+0x9e/0x135 [lirc_dev]
 [<c014e15c>] chrdev_open+0xc1/0xf6
 [<c014e09b>] chrdev_open+0x0/0xf6
 [<c014b052>] __dentry_open+0xf2/0x1da
 [<c014b156>] nameidata_to_filp+0x1c/0x2c
 [<c01544ae>] do_filp_open+0x28b/0x545
 [<c015bc37>] alloc_fd+0x46/0xad
 [<c014ae8d>] do_sys_open+0x42/0xb6
 [<c014af45>] sys_open+0x1e/0x23
 [<c010297d>] sysenter_do_call+0x12/0x25
 [<c02f0000>] ioctl_standard_iw_point+0x12d/0x200
 =======================
---[ end trace 6c8eb5cbb580deaf ]---

Regards,
Stefan

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2008-09-26 19:42 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-11 19:49 [PATCH 02/18] lirc serial port receiver/transmitter device driver Stefan Bauer
2008-09-12 16:24 ` Jarod Wilson
2008-09-13  0:26   ` Janne Grunau
2008-09-13  8:41     ` Stefan Bauer
2008-09-15  3:55       ` Jarod Wilson
2008-09-15 18:20         ` Jarod Wilson
2008-09-16  4:08           ` Jarod Wilson
2008-09-18 14:00             ` Jarod Wilson
2008-09-19 18:05             ` Stefan Bauer
2008-09-19 18:26               ` Janne Grunau
2008-09-19 18:53                 ` Stefan Bauer
2008-09-19 19:24                   ` Janne Grunau
2008-09-20  0:10                   ` Janne Grunau
2008-09-26 19:42                     ` Stefan Bauer
2008-09-19 18:54                 ` Jarod Wilson
2008-09-19 19:12                   ` Stefan Bauer
2008-09-13  7:09   ` Christoph Bartelmus
  -- strict thread matches above, loose matches on Subject: below --
2008-09-09  4:05 [PATCH 0/18] linux infrared remote control drivers Jarod Wilson
2008-09-09  4:05 ` [PATCH 01/18] lirc core device driver infrastructure Jarod Wilson
2008-09-09  4:05   ` [PATCH 02/18] lirc serial port receiver/transmitter device driver Jarod Wilson
2008-09-09 16:14     ` Jonathan Corbet
2008-09-09 19:51       ` Stefan Lippers-Hollmann
2008-09-09 19:56         ` Jarod Wilson
2008-09-10 17:40       ` Jarod Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox