Re: [PARPORT] ECP + other bug fixed to parport driver


Frederick Barnes (frmb@mail.cern.ch)
Sun, 26 Sep 1999 12:50:16 +0200 (CEST)


On Sun, 26 Sep 1999, Philip Blundell wrote:

> >-unsigned char __frob_control (struct parport *p, unsigned char mask,
> >- unsigned char val)
> >-{
>
> Does this leave the only version of __frob_control being the inlined one in
> parport_pc.h? If so I suspect that's bad. On the other hand I think the
> old code was probably wrong too, it should have either been static or had a
> `parport_pc' name prefix.

I wasn't too sure about the removal of that either (Jamie did that one).
I'll leave it up to you to decide whether you really want to remove it.
Lemme know your thoughts on that.

> > unsigned char parport_pc_read_control(struct parport *p)
> > {
> >+ const unsigned char wm = (PARPORT_CONTROL_STROBE |
> >+ PARPORT_CONTROL_AUTOFD |
> >+ PARPORT_CONTROL_INIT |
> >+ PARPORT_CONTROL_SELECT);
> > const struct parport_pc_private *priv = p->physport->private_data;
> >- return priv->ctr; /* Use soft copy */
> >+ return priv->ctr & wm; /* Use soft copy */
>
> I'm not totally convinced about this change. Can you explain why it's
> required?

In the existing __frob_control, the following existed:
    return priv->ctr = ctr & wm; /* update soft copy */

with wm being the 4 control bits. If the direction, or interrupt, bits
were set, they would be cleared as it was written back to priv->ctr. Any
subsequent call to __frob_control() would then clear the
direction/interrupt bit if it had been set previously. For this reason we
thought it better (and correct) to keep direction/interrupt bits in
priv->ctr and mask the return value from parport_pc_read_control().

> As an aside, your patch contained some debugging and formatting changes that
> made it a bit harder to see what was really being changed. In future, could
> you send those changes separately? It would also be a help if you could use
> the `-p' option to diff.

Sure, no probs.

Fred.

-- 
+--------------------------------------------------------------------------+
| Fred Barnes                                                              |
| Frederick.Barnes@cern.ch                        http://teddy.xylene.com/ |
+--------------------------------------------------------------------------+

-- 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 Sun 26 Sep 1999 - 06:52:43 EDT