Tim Waugh (tim@cyberelk.demon.co.uk)
Sun, 4 Oct 1998 21:23:27 +0100 (BST)
Hi there,
Here are a few (hopefully constructive!) comments.
On Thu, 1 Oct 1998 schreite@helena.physik.uni-stuttgart.de wrote:
> #include <linux/tasks.h>
> #include <linux/parport.h>
> #include <linux/delay.h>
> #include <linux/kernel.h>
> +#include <linux/lp.h>
It shouldn't be necessary to include lp.h in code that is this generic.
> +#define DPORT data->port
> +#define DDATA data->data
> +#define DSTATUS data->status
> +#define DRESULT data->result
> +#define DSAVE_RESULT data->save_result
> +#define TIMEOUT -EIO
> +#define OK 0
> +#define TRYING_AGAIN PARPORT_TRYING_AGAIN
> +#define UDELAY(usec) { data->sum_delay+=(usec); udelay(usec); }
> +#define MAX_WAIT_SHORT 20
> +#define MAX_SUM_DELAY 10000
If it's possible to get away with fewer #define's, that would be good. I
find code confusing if there are too many macros. For instance:
> +#define DO_NEGO(IMODE, NAME, FORCE, NEXTSTATE) \
> + SET_STATE(NEXTSTATE); \
> + DRESULT=2; \
> + must_nego=( ((FORCE)) || \
> + ((IMODE)!=DPORT->cad->ieee1284.current_mode) || \
> + (DPORT->cad->ieee1284.flags&PARPORT_IEEE1284_ERROR) ); \
> + if (must_nego) { \
> + if ((IMODE)==IEEE1284_NONE) { \
> + CALL(parport_quit_ieee1284_modes_p); \
> + DRESULT=2; \
> + } else { \
> + DDATA=NEGO_MODE(IMODE); \
> + CALL(parport_ieee1284_negotiate_p); \
> + } \
> + } \
> + case NEXTSTATE: \
> + if (DRESULT!=2) { \
> + printk(NAME \
> + ": error while negotiating IEEE1284 mode\n"); \
> + RETURN_ERROR; \
> + }
I can see what this does now, but it took me quite some time to wade
through.
> + case IEEE1284_NONE:
> + printk("parport_ieee1284_write_block: compatibility mode"
> + "not yet available - wait for later
> Linux kernels!\n");
> + RETURN(-EIO);
> + /* FIXME: call the appropriate function in lp.c! */
Once this is in the kernel, there should be no dependencies on lp. The
code to do the work should be here, and lp will simple call the relevant
parport_ieee1284 functions.
Tim.
*/
-- 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:30 EST