Andrea Arcangeli (andrea@e-mind.com)
Mon, 14 Dec 1998 18:29:03 +0100 (CET)
Today I finally had 2/3 hour to look into your new 1284 code. The 1284
code is sure a good thing since it allow us to avoids code duplication and
simplify the code. I have some bugfix or improvement against Tim' s latest
patch.
Without this patch I dubit that the new lp (that uses 1284 functions)
would not cause an Oops using parport_ax or _arc. An alternative patch
would be to add a check for null in read_block() and write_block() in
parport_ieee1284.c but I think it would only decrease performance because
such functions are likely to exists...
--- /tmp/linux-ac9+timieee/drivers/misc/parport_arc.c Sat Dec 12 13:16:55 1998
+++ linux/drivers/misc/parport_arc.c Mon Dec 14 18:18:01 1998
@@ -74,6 +74,8 @@
#endif
}
+static void parport_arc_null_func(void) { }
+
static struct parport_operations parport_arc_ops =
{
arc_write_data,
@@ -116,15 +118,15 @@
arc_dec_use_count,
arc_fill_inode,
- NULL, /* epp_write_block */
- NULL, /* epp_read_block */
+ (parport_write_block_fn) parport_arc_null_func, /* epp_write_block */
+ (parport_read_block_fn) parport_arc_null_func, /* epp_read_block */
- NULL, /* ecp_write_block */
- NULL, /* epp_write_block */
-
- NULL, /* compat_write_block */
- NULL, /* nibble_read_block */
- NULL /* byte_read_block */
+ (parport_write_block_fn) parport_arc_null_func, /* ecp_write_block */
+ (parport_read_block_fn) parport_arc_null_func, /* ecp_read_block */
+
+ (parport_write_block_fn) parport_arc_null_func, /* compat_write_block */
+ (parport_read_block_fn) parport_arc_null_func, /* nibble_read_block */
+ (parport_read_block_fn) parport_arc_null_func, /* byte_read_block */
};
/* --- Initialisation code -------------------------------- */
diff -urN --exclude *.o --exclude *.a --exclude CVS --exclude .* /tmp/linux-ac9+timieee/drivers/misc/parport_ax.c linux/drivers/misc/parport_ax.c
--- /tmp/linux-ac9+timieee/drivers/misc/parport_ax.c Sat Dec 12 13:16:55 1998
+++ linux/drivers/misc/parport_ax.c Mon Dec 14 17:47:44 1998
@@ -283,6 +283,8 @@
#endif
}
+static void parport_ax_null_func(void) { }
+
static struct parport_operations parport_ax_ops =
{
parport_ax_write_data,
@@ -323,17 +325,17 @@
parport_ax_inc_use_count,
parport_ax_dec_use_count,
- parport_ax_fill_inode
+ parport_ax_fill_inode,
- NULL, /* epp_write_block */
- NULL, /* epp_read_block */
+ (parport_write_block_fn) parport_ax_null_func, /* epp_write_block */
+ (parport_read_block_fn) parport_ax_null_func, /* epp_read_block */
- NULL, /* ecp_write_block */
- NULL, /* ecp_read_block */
+ (parport_write_block_fn) parport_ax_null_func, /* ecp_write_block */
+ (parport_read_block_fn) parport_ax_null_func, /* ecp_read_block */
- NULL, /* compat_write_block */
- NULL, /* nibble_read_block */
- NULL /* byte_read_block */
+ (parport_write_block_fn) parport_ax_null_func, /* compat_write_block */
+ (parport_read_block_fn) parport_ax_null_func, /* nibble_read_block */
+ (parport_read_block_fn) parport_ax_null_func, /* byte_read_block */
};
Little cleanup/improvement:
diff -urN --exclude *.o --exclude *.a --exclude CVS --exclude .* /tmp/linux-ac9+timieee/drivers/misc/parport_ieee1284.c linux/drivers/misc/parport_ieee1284.c
--- /tmp/linux-ac9+timieee/drivers/misc/parport_ieee1284.c Mon Dec 14 17:52:11 1998
+++ linux/drivers/misc/parport_ieee1284.c Mon Dec 14 17:41:21 1998
@@ -282,11 +282,12 @@
sema_init (&sem, 0);
error = fn (port, buffer, len, callback, &info);
- if (!error)
- /* Wait for completion. */
- down_interruptible (&sem);
+ if (error)
+ return error;
- return error ? error : info.count;
+ /* Wait for completion. */
+ down_interruptible (&sem);
+ return info.count;
}
static inline
@@ -301,11 +302,12 @@
sema_init (&sem, 0);
error = fn (port, buffer, len, callback, &info);
- if (!error)
- /* Wait for completion. */
- down_interruptible (&sem);
+ if (error)
+ return error;
- return error ? error : info.count;
+ /* Wait for completion. */
+ down_interruptible (&sem);
+ return info.count;
}
/* Handle an interrupt. */
This patch will implement a policy where the ieee_ops lowlevel function
has to be called with a callback not null. This will improve performance
because ~always there' s a valid callback set.
--- /tmp/linux-ac9+timieee/drivers/misc/parport_ieee1284_ops.c Mon Dec 14 17:52:11 1998
+++ linux/drivers/misc/parport_ieee1284_ops.c Mon Dec 14 18:09:04 1998
@@ -6,7 +6,7 @@
#include <linux/parport.h>
#include <linux/delay.h>
-#define DEBUG /* undef me for production */
+#undef DEBUG /* undef me for production */
#ifdef DEBUG
#define DPRINTK(stuff...) printk (stuff)
@@ -180,7 +180,7 @@
port->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
/* Call the callback function. */
- if (fn) fn (port, handle, count);
+ fn (port, handle, count);
return signal_pending (current) ? -EINTR : 0;
}
@@ -246,7 +246,7 @@
port->ieee1284.phase = IEEE1284_PH_FWD_IDLE;
- if (fn) fn (port, handle, written);
+ fn (port, handle, written);
return 0;
}
@@ -257,7 +257,7 @@
void *, size_t),
void *handle)
{
- if (fn) fn (port, handle, 0);
+ fn (port, handle, 0);
return -ENOSYS; /* FIXME */
}
@@ -348,7 +348,7 @@
}
/* Call the callback. */
- if (fn) fn (port, handle, count);
+ fn (port, handle, count);
return 0;
}
@@ -426,7 +426,7 @@
}
/* Call the callback. */
- if (fn) fn (port, handle, count);
+ fn (port, handle, count);
return 0;
}
@@ -442,7 +442,7 @@
if (ecp_forward_to_reverse (port))
return -EIO;
- if (fn) fn (port, handle, 0);
+ fn (port, handle, 0);
return -ENOSYS; /* FIXME */
}
@@ -453,6 +453,6 @@
void *, size_t),
void *handle)
{
- if (fn) fn (port, handle, 0);
+ fn (port, handle, 0);
return -ENOSYS; /* FIXME */
}
This will allow other pardevice to run in the meantime. Tim told me also a
better fix but also only this will work rasonable in the meantime (the
important thing is not to stall other pardevice).
--- /tmp/linux-ac9+timieee/drivers/char/lp.c Mon Dec 14 17:52:11 1998
+++ linux/drivers/char/lp.c Mon Dec 14 17:07:35 1998
@@ -277,11 +277,16 @@
static void lp_error (int minor)
{
+ int polling;
+
if (LP_F(minor) & LP_ABORT)
return;
+ polling = !(lp_table[minor].dev->flags & PARPORT_DEV_IEEE1284IRQ);
+ if (polling) lp_parport_release(minor);
current->state = TASK_INTERRUPTIBLE;
schedule_timeout (LP_TIMEOUT_POLLED);
+ if (polling) lp_parport_claim(minor);
}
static int lp_check_status(int minor)
I just included patch-2.1.131-ac8-1284 + the patches above in my arca-tree
to handle more testing from people on more hardware...
Tim and what about things like tunelp -w now?
Andrea Arcangeli
-- To unsubscribe, send mail to: linux-parport-request@torque.net --
-- with the single word "unsubscribe" in the body of the message. --
This archive was generated by hypermail 2.0b3 on Wed 30 Dec 1998 - 10:18:54 EST