Re: [PATCH] bw-qcam: use data_reverse instead of manually poking the control register

On 9/26/07, Ray Lee <ray-lk@xxxxxxxxxxxxx> wrote:
On 9/26/07, Brett Warden <brett.warden@xxxxxxxxx> wrote:
On 9/26/07, Ray Lee <ray-lk@xxxxxxxxxxxxx> wrote:

Just as an aside, if you've tested this and it works, then there's no
point to keep the write_lpcontrol even as a comment. Kill those four
lines, and if someone's interested in what happened they'll just look
at the file history.

Point taken, thanks for the feedback.

---

diff --git a/drivers/media/video/bw-qcam.c b/drivers/media/video/bw-qcam.c
index 7d47cbe..0ba92e3 100644
--- a/drivers/media/video/bw-qcam.c
+++ b/drivers/media/video/bw-qcam.c
@@ -107,6 +107,11 @@ static inline void write_lpcontrol(struct
qcam_device *q, int d)
parport_write_control(q->pport, d);
}

+static inline void reverse_port(struct qcam_device *q)
+{
+ parport_data_reverse(q->pport);
+}
+
static int qc_waithand(struct qcam_device *q, int val);
static int qc_command(struct qcam_device *q, int command);
static int qc_readparam(struct qcam_device *q);
@@ -369,7 +374,7 @@ static void qc_reset(struct qcam_device *q)
break;

case QC_ANY:
- write_lpcontrol(q, 0x20);
+ reverse_port(q);
write_lpdata(q, 0x75);

if (read_lpdata(q) != 0x75) {
@@ -512,10 +517,12 @@ static inline int qc_readbytes(struct
qcam_device *q, char buffer[])
switch (q->port_mode & QC_MODE_MASK)
{
case QC_BIDIR: /* Bi-directional Port */
- write_lpcontrol(q, 0x26);
+ reverse_port(q);
+ write_lpcontrol(q, 0x6);
lo = (qc_waithand2(q, 1) >> 1);
hi = (read_lpstatus(q) >> 3) & 0x1f;
- write_lpcontrol(q, 0x2e);
+ reverse_port(q);
+ write_lpcontrol(q, 0xe);
lo2 = (qc_waithand2(q, 0) >> 1);
hi2 = (read_lpstatus(q) >> 3) & 0x1f;
switch (q->bpp)
@@ -613,10 +620,13 @@ static long qc_capture(struct qcam_device * q,
char __user *buf, unsigned long l

if ((q->port_mode & QC_MODE_MASK) == QC_BIDIR)
{
- write_lpcontrol(q, 0x2e); /* turn port around */
- write_lpcontrol(q, 0x26);
+ reverse_port(q); /* turn port around */
+ write_lpcontrol(q, 0xe);
+ reverse_port(q);
+ write_lpcontrol(q, 0x6);
(void) qc_waithand(q, 1);
- write_lpcontrol(q, 0x2e);
+ reverse_port(q);
+ write_lpcontrol(q, 0xe);
(void) qc_waithand(q, 0);
}

Better, and do you have time for two (possibly stupid) questions? In
each of the last cases it looks like the transformation is from a
write_lpcontrol -> reverse_port and a write_lpcontrol (old address -
0x20). Except the first one, which merely has the reverse_port. One
would think that there should be a write_lpcontrol(q, 0x0); after that
one.

Also, is the reverse port sticky, or does it only apply to the next
write? If it's only the next, then maybe a different name would be
better. If it's sticky, then I think the code is wrong...

In response to Randy's and your questions, here's what I understand:

The error message comes from parport_pc_write_control in
include/linux/parport_pc.h, which is called by bw-qcam's
write_lpcontrol():

static __inline__ void parport_pc_write_control (struct parport *p,
unsigned char d)
{
const unsigned char wm = (PARPORT_CONTROL_STROBE |
PARPORT_CONTROL_AUTOFD |
PARPORT_CONTROL_INIT |
PARPORT_CONTROL_SELECT);

/* Take this out when drivers have adapted to newer interface. */
if (d & 0x20) {
printk (KERN_DEBUG "%s (%s): use data_reverse for this!\n",
p->name, p->cad->name);
parport_pc_data_reverse (p);
}

__parport_pc_frob_control (p, wm, d & wm);
}

The mask wm works out to 0x0f, so first it calls
parport_pc_data_reverse(), then masks off the high nibble with the
reverse flag and makes the regular call.

Looking at that more carefully, I'm not sure whether I need to add the
write_lpcontrol(q, 0) after the first call. Other than that, I believe
I'm following the same procedure.

As for whether data_reverse is sticky, I don't know... I'll see what I
can find out.

--
Brett Warden
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages

  • Re: Obscure marshalling/pinvoke problem with 1, 2, 4, and 8 bytes structures
    ... Structure(unsigned char a0, unsigned char b0, unsigned char c0, ... DLLEXPORT Structure STDCALL GetStructure() ... This is likely because the managed PInvoke signature does ... problem, slightly different error message. ...
    (microsoft.public.dotnet.framework.interop)
  • Re: Obscure marshalling/pinvoke problem with 1, 2, 4, and 8 bytes
    ... Structure(unsigned char a0, unsigned char b0, unsigned char c0, ... DLLEXPORT Structure STDCALL GetStructure() ... This is likely because the managed PInvoke signature does ... problem, slightly different error message. ...
    (microsoft.public.dotnet.framework.interop)
  • Re: Byte-wise boolean optimisation
    ... Create a mask of the bits of interest say bits 0,1,3,7 ... You are potentially wasting time with the shift operators (depending ... on how smart your compiler is). ... unsigned char get_byte ...
    (comp.arch.embedded)
  • Re: Good brands for snorkel, fins, masks
    ... >>> A good mask is one that fits well, and does not interfere with your ... >>> peripheral vision. ... Also the material should be sticky. ...
    (rec.scuba)
  • Re: Good brands for snorkel, fins, masks
    ... >> A good mask is one that fits well, and does not interfere with your ... >> peripheral vision. ... Also the material should be sticky. ... A banana head is gonna complain about a sticky mask? ...
    (rec.scuba)