patch-2.2.10.gz: review

Ulrich Windl (ulrich.windl@rz.uni-regensburg.de)
Tue, 15 Jun 1999 08:36:33 +0200


--Message-Boundary-22123
Content-type: text/plain; charset=US-ASCII
Content-transfer-encoding: 7BIT
Content-description: Mail message body

Hello,

browsing patch-2.2.10.gz some things fell in my eye that I have
described in the attached text. I'm not sure that all are bugs, but
maybe someone with better knowledge take a look (aic7xxx stuff).

I also could not resist making perfect HTML 4.0 from a document that
used illegal HTML comments. The document can be automatically
validated now (patch provided).

Regards,
Ulrich

--Message-Boundary-22123
Content-type: text/plain; charset=ISO-8859-1
Content-transfer-encoding: Quoted-printable
Content-description: Text from file '2210.txt'

Here are a few comments on patch-2.2.10:

The file Documentation/video4linux/API.html contains invalid HTML comments=
.
I took the chance to make a valid vanilla HTML 4.0 document from it:

--- linux/Documentation/video4linux/API.html Mon Jun 14 20:25:07 1999
+++ /tmp/API.html Mon Jun 14 22:42:17 1999
@@ -1,12 +1,18 @@
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN">
<HTML><HEAD>
+<STYLE type=3D"text/css">
+BODY {background: white; color: black}
+</STYLE>
<TITLE>Video4Linux Kernel API Reference v0.1:19990430</TITLE>
+<LINK REV=3D"MADE" HREF=3D"mailto:Fred Gleason <fredg@wava.com>">
</HEAD>
-<! Revision History: >
-<! 4/30/1999 - Fred Gleason (fredg@wava.com)>
-<! Documented extensions for the Radio Data System (RDS) extensions >
-<BODY bgcolor=3D"#ffffff">
+<!-- Revision History: -->
+<!-- 4/30/1999 - Fred Gleason (fredg@wava.com) -->
+<!-- Documented extensions for the Radio Data System (RDS) extensions -->
+<!-- Made "HTML 4.0 clean" by Ulrich Windl -->
+<BODY>
<H3>Devices</H3>
-Video4Linux provides the following sets of device files. These live on th=
e
+<P>Video4Linux provides the following sets of device files. These live on=
the
character device formerly known as "/dev/bttv". /dev/bttv should be a
symlink to /dev/video0 for most people.
<P>
@@ -24,7 +30,7 @@
applies to radio cards. Teletext interfaces talk the existing VTX API.
<P>
<H3>Capability Query Ioctl</H3>
-The <B>VIDIOCGCAP</B> ioctl call is used to obtain the capability
+<P>The <B>VIDIOCGCAP</B> ioctl call is used to obtain the capability
information for a video device. The <b>struct video_capability</b> object
passed to the ioctl is completed and returned. It contains the following
information
@@ -64,7 +70,7 @@
direction. For example the quickcam has 3 fixed settings.
<P>
<H3>Frame Buffer</H3>
-Capture cards that drop data directly onto the frame buffer must be told =
the
+<P>Capture cards that drop data directly onto the frame buffer must be to=
ld the
base address of the frame buffer, its size and organisation. This is a
privileged ioctl and one that eventually X itself should set.
<P>
@@ -89,7 +95,7 @@
access.
<P>
<H3>Capture Windows</H3>
-The capture area is described by a <b>struct video_window</b>. This defin=
es
+<P>The capture area is described by a <b>struct video_window</b>. This de=
fines
a capture area and the clipping information if relevant. The
<b>VIDIOCGWIN</b> ioctl recovers the current settings and the
<b>VIDIOCSWIN</b> sets new values. A successful call to <b>VIDIOCSWIN</b>=

@@ -105,7 +111,7 @@
<TR><TD><b>height</b><TD>The height of the image capture.</TD>
<TR><TD><b>chromakey</b><TD>A host order RGB32 value for the chroma key.<=
/TD>
<TR><TD><b>flags</b><TD>Additional capture flags.</TD>
-<TR><TD><b>clips</b><TD>A list of clipping rectangles. <em>(Set only)</em=
)</TD>
+<TR><TD><b>clips</b><TD>A list of clipping rectangles. <em>(Set only)</em=
></TD>
<TR><TD><b>clipcount</b><TD>The number of clipping rectangles. <em>(Set o=
nly)</em></TD>
</TABLE>
<P>
@@ -136,7 +142,7 @@
<TR><TD><b>decimation</b></TD><TD>Decimation to apply</TD>
<TR><TD><b>flags</b></TD><TD>Flag settings for grabbing</TD>
</TABLE>
-The available flags are
+<P>The available flags are
<P>
<TABLE>
<TR><TH>Name</TH><TH>Description</TH>
@@ -145,7 +151,7 @@
</TABLE>
<P>
<H3>Video Sources</H3>
-Each video4linux video or audio device captures from one or more
+<P>Each video4linux video or audio device captures from one or more
source <b>channels</b>. Each channel can be queries with the
<b>VDIOCGCHAN</b> ioctl call. Before invoking this function the caller
must set the channel field to the channel that is being queried. On retur=
n
@@ -186,6 +192,7 @@
</TABLE>
<P>
<H3>Image Properties</H3>
+<P>
The image properties of the picture can be queried with the <b>VIDIOCGPIC=
T</b>
ioctl which fills in a <b>struct video_picture</b>. The <b>VIDIOCSPICT</b=
>
ioctl allows values to be changed. All values except for the palette type
@@ -223,6 +230,7 @@
</TABLE>
<P>
<H3>Tuning</H3>
+<P>
Each video input channel can have one or more tuners associated with it. =
Many
devices will not have tuners. TV cards and radio cards will have one or m=
ore
tuners attached.
@@ -271,7 +279,7 @@
set by the <b>VIDIOCSFREQ</b> ioctl.
<P>
<H3>Audio</H3>
-TV and Radio devices have one or more audio inputs that may be selected.
+<P>TV and Radio devices have one or more audio inputs that may be selecte=
d.
The audio properties are queried by passing a <b>struct video_audio</b> t=
o <b>VIDIOCGAUDIO</b> ioctl. The
<b>VIDIOCSAUDIO</b> ioctl sets audio properties.
<P>
@@ -310,7 +318,7 @@
</TABLE>
<P>
<H3>Reading Images</H3>
-Each call to the <b>read</b> syscall returns the next available image fro=
m
+<P>Each call to the <b>read</b> syscall returns the next available image =
from
the device. It is up to the caller to set the format and then to pass a
suitable size buffer and length to the function. Not all devices will sup=
port
read operations.
@@ -355,7 +363,7 @@
</TABLE>
<P>
<H3>RDS Datastreams</H3>
-For radio devices that support it, it is possible to receive Radio Data
+<P>For radio devices that support it, it is possible to receive Radio Dat=
a
System (RDS) data by means of a read() on the device. The data is packed=
in
groups of three, as follows:
<TABLE>

In linux/drivers/pci/oldproc.c the part of the patch seems strange:

DEVICE( ADAPTEC, ADAPTEC_5800, "AIC-5800"),
+ DEVICE( ADAPTEC, ADAPTEC_3860, "AIC-7860"),
DEVICE( ADAPTEC, ADAPTEC_7860, "AIC-7860"),

Shouldn't it be something like

+ DEVICE( ADAPTEC, ADAPTEC_3860, "AIC-3860"),

?

Another bit definition that looks strange is in
linux/drivers/scsi/aic7xxx/aic7xxx.reg:

+register CRCCONTROL1 {
+ address 0x09d
+ access_mode RW
+ bit CRCONSEEN 0x80 /* CRC ON Single Edge ENable */
+ bit CRCVALCHKEN 0x40 /* CRC Value Check Enable */
+ bit CRCENDCHKEN 0x20 /* CRC End Check Enable */
+ bit CRCREQCHKEN 0x10
+ bit TARGCRCENDEN 0x08 /* Enable End CRC transfer when targe=
t */
+ bit TARGCRCCNTEN 0x40 /* Enable CRC transfer when target */
+}

Shouldn=B4t it possibly be

+ bit TARGCRCCNTEN 0x04 /* Enable CRC transfer when target */

Finally some cosmetics: In linux/drivers/scsi/aic7xxx_proc.c some lines re=
ad:

+#ifdef CONFIG_AIC7XXX_TCQ_ON_BY_DEFAULT
+ size +=3D sprintf(BLS, " TCQ Enabled By Default : Enabled\n");
+#else
+ size +=3D sprintf(BLS, " TCQ Enabled By Default : Disabled\n");

Shouldn't these be simplified to

+#ifdef CONFIG_AIC7XXX_TCQ_ON_BY_DEFAULT
+ size +=3D sprintf(BLS, " TCQ Enabled By Default : Yes\n");
+#else
+ size +=3D sprintf(BLS, " TCQ Enabled By Default : No\n");

?

Later the line

+ size +=3D sprintf(BLS, " AIC7XXX_RESET_DELAY : %d\n", AIC7XXX_RESET=
_DELAY);

could read

+ size +=3D sprintf(BLS, " Delay after bus reset : %d\n", AIC7XXX_RESET=
_DELAY);

as well. I mean: Either use the symbols from the source or use words
that mean something even for dummies. "Tag Queue Depth" could also be
"TCQ Depth" with the latest wording. The "Transinfo settings:" are
really hard for dummies.

Regards,
Ulrich Windl

--Message-Boundary-22123--

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