[PARPORT] keeping up appearances


Philip Blundell (philb@gnu.org)
Fri, 19 Feb 1999 21:15:53 +0000


Since I'm still notionally one of the parport maintainers I guess I ought to
do something to earn my keep once in a while. Thus, here's a patch against
2.2.1+Tim's latest 1284 and 1284.3 stuff.

The functional change is to fix up the most obviously broken bits of IRQ
handling so that shared interrupts have a chance of working. As Joerg
pointed out it's unacceptable for the parport code to go calling
disable_irq() because it doesn't know what else might be using the same
irq number. With the advent of PCI based parallel port cards this is no
longer just an m68k problem, it will bite people on PCs as well.
Fortunately, the parport layer has always had the `enable_irq' and
`disable_irq' methods to deal with exactly this issue; it's just a case of
pressing them into service.

This means that if you have a driver that can generate interrupts (ie
port->irq != PARPORT_IRQ_NONE) you *must* provide valid functions for these
two methods. Leaving them at NULL will cause a crash. Can the m68k people
take a look and see whether this deals with your concerns?

I also went through parport.h and fixed most of the comments to have a
consistent style.

None of this is tested, but you guessed that already. Share and enjoy.

p.

--- clean/linux/include/linux/parport.h Fri Feb 19 21:04:57 1999
+++ linux/include/linux/parport.h Fri Feb 19 21:00:19 1999
@@ -50,11 +50,10 @@
         PARPORT_CLASS_SCSIADAPTER
 } parport_device_class;
 
-/* The "modes" entry in parport is a bit field representing the following
- * modes.
- * Note that PARPORT_MODE_PCECPEPP is for the SMC EPP+ECP mode which is NOT
- * 100% compatible with EPP.
- */
+/* The "modes" entry in parport is a bit field representing the
+ following modes. Note that PARPORT_MODE_PCECPEPP is for the SMC
+ EPP+ECP mode which is NOT 100% compatible with EPP. */
+/* @@@ PARPORT_MODE_ECR isn't a mode. Take it out? */
 #define PARPORT_MODE_PCSPP (1<<0)
 #define PARPORT_MODE_PCPS2 (1<<1)
 #define PARPORT_MODE_PCEPP (1<<2)
@@ -62,19 +61,17 @@
 #define PARPORT_MODE_PCECP (1<<4)
 #define PARPORT_MODE_PCECPEPP (1<<5)
 #define PARPORT_MODE_PCECPPS2 (1<<6)
+
 /* The following aren't explicitly represented in the modes entry
- * (although maybe PPF should be).
- */
+ (although maybe PPF should be). */
 #define PARPORT_MODE_PCECPPPF (1<<7) /* Parallel Port FIFO */
 #define PARPORT_MODE_PCECPTST (1<<8) /* Test */
 #define PARPORT_MODE_PCECPCFG (1<<9) /* Configuration */
 
-/* IEEE1284 modes.
- * Nibble mode, byte mode, ECP, ECPRLE and EPP are their
- * own 'extensibility request' values. Others are special.
- *
- * 'Real' ECP modes must have the IEEE1284_MODE_ECP bit set.
- */
+/* IEEE1284 modes:
+ Nibble mode, byte mode, ECP, ECPRLE and EPP are their own
+ 'extensibility request' values. Others are special.
+ 'Real' ECP modes must have the IEEE1284_MODE_ECP bit set. */
 #define IEEE1284_MODE_NIBBLE 0
 #define IEEE1284_MODE_BYTE (1<<0)
 #define IEEE1284_MODE_COMPAT (1<<8)
@@ -105,11 +102,21 @@
         unsigned int ecr;
 };
 
+/* used by both parport_amiga and parport_mfc3 */
+struct amiga_parport_state {
+ unsigned char data; /* ciaa.prb */
+ unsigned char datadir; /* ciaa.ddrb */
+ unsigned char status; /* ciab.pra & 7 */
+ unsigned char statusdir;/* ciab.ddrb & 7 */
+};
+
 struct parport_state {
         union {
                 struct pc_parport_state pc;
                 /* ARC has no state. */
                 /* AX uses same state information as PC */
+ struct amiga_parport_state amiga;
+ /* Atari has not state. */
                 void *misc;
         } u;
 };
@@ -288,36 +295,34 @@
         unsigned char *dma_buffer;
 };
 
-/* parport_register_port registers a new parallel port at the given address (if
- * one does not already exist) and returns a pointer to it. This entails
- * claiming the I/O region, IRQ and DMA.
- * NULL is returned if initialisation fails.
- */
+/* parport_register_port registers a new parallel port at the given
+ address (if one does not already exist) and returns a pointer to it.
+ This entails claiming the I/O region, IRQ and DMA. NULL is returned
+ if initialisation fails. */
 struct parport *parport_register_port(unsigned long base, int irq, int dma,
                                       struct parport_operations *ops);
 
 /* Unregister a port. */
 extern void parport_unregister_port(struct parport *port);
 
-/* parport_in_use returns nonzero if there are devices attached to a port. */
+/* parport_in_use returns nonzero if there are devices attached to a
+ port. */
 #define parport_in_use(x) ((x)->devices != NULL)
 
-/* Put a parallel port to sleep; release its hardware resources. Only possible
- * if no devices are registered. */
+/* Put a parallel port to sleep; release its hardware resources. Only
+ possible if no devices are registered. */
 extern void parport_quiesce(struct parport *);
 
-/* parport_enumerate returns a pointer to the linked list of all the ports
- * in this machine.
- */
+/* parport_enumerate returns a pointer to the linked list of all the
+ ports in this machine. */
 struct parport *parport_enumerate(void);
 
-/* parport_register_device declares that a device is connected to a port, and
- * tells the kernel all it needs to know.
- * pf is the preemption function (may be NULL for no callback)
- * kf is the wake-up function (may be NULL for no callback)
- * irq_func is the interrupt handler (may be NULL for no interrupts)
- * handle is a user pointer that gets handed to callback functions.
- */
+/* parport_register_device declares that a device is connected to a
+ port, and tells the kernel all it needs to know. pf is the
+ preemption function (may be NULL for no callback) kf is the wake-up
+ function (may be NULL for no callback) irq_func is the interrupt
+ handler (may be NULL for no interrupts) handle is a user pointer
+ that gets handed to callback functions. */
 struct pardevice *parport_register_device(struct parport *port,
                           const char *name,
                           int (*pf)(void *), void (*kf)(void *),
@@ -327,30 +332,29 @@
 /* parport_unregister unlinks a device from the chain. */
 extern void parport_unregister_device(struct pardevice *dev);
 
-/* parport_claim tries to gain ownership of the port for a particular driver.
- * This may fail (return non-zero) if another driver is busy. If this
- * driver has registered an interrupt handler, it will be enabled.
- */
+/* parport_claim tries to gain ownership of the port for a particular
+ driver. This may fail (return non-zero) if another driver is busy.
+ If this driver has registered an interrupt handler, it will be
+ enabled. */
 extern int parport_claim(struct pardevice *dev);
 
-/* parport_claim_or_block is the same, but sleeps if the port cannot be
- claimed. Return value is 1 if it slept, 0 normally and -errno on error. */
+/* parport_claim_or_block is the same, but sleeps if the port cannot
+ be claimed. Return value is 1 if it slept, 0 normally and -errno
+ on error. */
 extern int parport_claim_or_block(struct pardevice *dev);
 
-/* parport_release reverses a previous parport_claim. This can never fail,
- * though the effects are undefined (except that they are bad) if you didn't
- * previously own the port. Once you have released the port you should make
- * sure that neither your code nor the hardware on the port tries to initiate
- * any communication without first re-claiming the port.
- * If you mess with the port state (enabling ECP for example) you should
- * clean up before releasing the port.
- */
+/* parport_release reverses a previous parport_claim. This can never
+ fail, though the effects are undefined (except that they are bad)
+ if you didn't previously own the port. Once you have released the
+ port you should make sure that neither your code nor the hardware
+ on the port tries to initiate any communication without first
+ re-claiming the port. If you mess with the port state (enabling
+ ECP for example) you should clean up before releasing the port. */
 
 extern void parport_release(struct pardevice *dev);
 
 /* parport_yield relinquishes the port if it would be helpful to other
- * drivers. The return value is the same as for parport_claim.
- */
+ drivers. The return value is the same as for parport_claim. */
 extern __inline__ int parport_yield(struct pardevice *dev)
 {
         unsigned long int timeslip = (jiffies - dev->time);
@@ -361,8 +365,7 @@
 }
 
 /* parport_yield_blocking is the same but uses parport_claim_or_block
- * instead of parport_claim.
- */
+ instead of parport_claim. */
 extern __inline__ int parport_yield_blocking(struct pardevice *dev)
 {
         unsigned long int timeslip = (jiffies - dev->time);
@@ -372,21 +375,13 @@
         return parport_claim_or_block(dev);
 }
 
-/*
- * Lowlevel drivers _can_ call this support function to handle irqs.
- */
+/* Lowlevel drivers _can_ call this support function to handle irqs. */
 extern __inline__ void parport_generic_irq(int irq, struct parport *port,
                                            struct pt_regs *regs)
 {
         read_lock(&port->cad_lock);
- if (!port->cad)
- goto out_unlock;
- if (port->cad->irq_func)
+ if (port->cad && port->cad->irq_func)
                 port->cad->irq_func(irq, port->cad->private, regs);
- else
- printk(KERN_ERR "%s: irq%d happened with irq_func NULL "
- "with %s as cad!\n", port->name, irq, port->cad->name);
- out_unlock:
         read_unlock(&port->cad_lock);
 }
 
@@ -479,7 +474,7 @@
 extern void (*parport_probe_hook)(struct parport *port);
 
 /* If PC hardware is the only type supported, we can optimise a bit. */
-#if (defined(CONFIG_PARPORT_PC) || defined(CONFIG_PARPORT_PC_MODULE)) && !(defined(CONFIG_PARPORT_AX) || defined(CONFIG_PARPORT_AX_MODULE)) && !(defined(CONFIG_PARPORT_ARC) || defined(CONFIG_PARPORT_ARC_MODULE)) && !defined(CONFIG_PARPORT_OTHER)
+#if (defined(CONFIG_PARPORT_PC) || defined(CONFIG_PARPORT_PC_MODULE)) && !(defined(CONFIG_PARPORT_AX) || defined(CONFIG_PARPORT_AX_MODULE)) && !(defined(CONFIG_PARPORT_ARC) || defined(CONFIG_PARPORT_ARC_MODULE)) && !(defined(CONFIG_PARPORT_AMIGA) || defined(CONFIG_PARPORT_AMIGA_MODULE)) && !(defined(CONFIG_PARPORT_MFC3) || defined(CONFIG_PARPORT_MFC3_MODULE)) && !(defined(CONFIG_PARPORT_ATARI) || defined(CONFIG_PARPORT_ATARI_MODULE)) && !defined(CONFIG_PARPORT_OTHER)
 #undef PARPORT_NEED_GENERIC_OPS
 #include <linux/parport_pc.h>
 #define parport_write_data(p,x) parport_pc_write_data(p,x)
@@ -502,6 +497,8 @@
 #define parport_epp_write_addr(p,x) parport_pc_write_epp_addr(p,x)
 #define parport_epp_read_addr(p) parport_pc_read_epp_addr(p)
 #define parport_epp_check_timeout(p) parport_pc_check_epp_timeout(p)
+#define parport_enable_irq(p) parport_pc_enable_irq(p)
+#define parport_disable_irq(p) parport_pc_disable_irq(p)
 #endif
 
 #ifdef PARPORT_NEED_GENERIC_OPS
@@ -526,6 +523,8 @@
 #define parport_epp_write_addr(p,x) (p)->ops->epp_write_addr(p,x)
 #define parport_epp_read_addr(p) (p)->ops->epp_read_addr(p)
 #define parport_epp_check_timeout(p) (p)->ops->epp_check_timeout(p)
+#define parport_enable_irq(p) (p)->ops->enable_irq(p)
+#define parport_disable_irq(p) (p)->ops->disable_irq(p)
 #endif
 
 #endif /* __KERNEL__ */
--- clean/linux/drivers/misc/parport_share.c Fri Feb 19 21:04:57 1999
+++ linux/drivers/misc/parport_share.c Fri Feb 19 21:03:59 1999
@@ -401,18 +401,12 @@
                 dev->waitprev = dev->waitnext = NULL;
         }
 
- if (oldcad && port->irq != PARPORT_IRQ_NONE && !oldcad->irq_func)
- /*
- * If there was an irq pending it should hopefully happen
- * before return from enable_irq(). -arca
- */
- enable_irq(port->irq);
-
- /*
- * Avoid running irq handlers if the pardevice doesn' t use it. -arca
- */
- if (port->irq != PARPORT_IRQ_NONE && !dev->irq_func)
- disable_irq(port->irq);
+ if (port->irq != PARPORT_IRQ_NONE) {
+ if (dev->irq_func)
+ parport_enable_irq(port);
+ else
+ parport_disable_irq(port);
+ }
 
         /* Now we do the change of devices */
         write_lock_irqsave(&port->cad_lock, flags);
@@ -523,8 +517,9 @@
          * Reenable irq and so discard the eventually pending irq while
          * cad is NULL. -arca
          */
+ /* @@@ Think this is bogus. Just lose it? --philb */
         if (port->irq != PARPORT_IRQ_NONE && !dev->irq_func)
- enable_irq(port->irq);
+ parport_enable_irq(port);
 
         /* Save control registers */
         port->ops->save_state(port, dev->state);

-- 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 Fri 19 Feb 1999 - 16:19:07 EST