Re: [PARPORT] Re: IMM driver weirdness.


Andrea Arcangeli (andrea@e-mind.com)
Fri, 25 Sep 1998 18:02:20 +0200 (CEST)


On Thu, 24 Sep 1998, David Campbell wrote:

>> The offending code is:
>>
>> --- imm_detect() ---
>> /* Claim the bus so it remembers what we do to the control
>> * registers. [ CTR and ECP ]
>> */
>> if (imm_pb_claim(i))
>> while (imm_hosts[i].p_busy)
>> schedule(); /* We are safe to schedule here */
>> --- imm_detect() ---

I am the author of such code. The code make perfectly sense and the
improvement suggested by Tim improve it (but _only_ improve it). The only
reason it' s written as now is that at the time I wrote the code
parport_claim_or_block didn' t exists.

>> while (port_busy) {
>> current->timeout = jiffies + timeout;
>> current->state = TASK_INTERRUPTIBLE;
>> schedule();
>> if (signals_pending(current)) {
>> /* someone hit ^C to interrupt us */
>> goto out;
>> }
>> }

Yes this make perfectly sense but note that if you need it you are using a
buggy pardevice at the same time or you have discovered a new bug in
parport. A pardevice _should_ release parport in mean after
PARPORT_DEFAULT_TIMESLICE sec if the parport wait queue is not empty. This
is the only reason it' s not implement as you suggested at first.

>Voting time, what is everyones (Tim, Phil, Grant, Andrea, anyone else?) ideas
>on this subject?

The change suggested make perfectly sense but it doesn' t fix bugs and
should be not needed in a bug free world.

Here a patch:

--- linux/drivers/scsi/ppa.c~ Thu Sep 17 18:43:10 1998
+++ linux/drivers/scsi/ppa.c Fri Sep 25 17:57:24 1998
@@ -139,8 +139,20 @@
          * registers. [ CTR and ECP ]
          */
         if (ppa_pb_claim(i))
+ {
+ unsigned long now = jiffies;
             while (ppa_hosts[i].p_busy)
+ {
                 schedule(); /* We are safe to schedule here */
+ if (jiffies > now + 3*HZ)
+ {
+ printk(KERN_ERR "ppa%d: failed to claim parport because a "
+ "pardevice is owning the port for too longtime!\n",
+ i);
+ return 0;
+ }
+ }
+ }
 
         ppb = PPA_BASE(i) = ppa_hosts[i].dev->port->base;
         w_ctr(ppb, 0x0c);

I have not tried it yet but compile ;-).

Now I need to know _why_ dg2fef need such change.

>(Don't you hate it when reality creeps in on your kernel hacking time?)

Yes ;-).

Andrea[s] 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:23 EST