[6678] in linux-scsi channel archive

home help back first fref pref prev next nref lref last post

patch-2.2.10.gz: review

daemon@ATHENA.MIT.EDU (Ulrich Windl)
Tue Jun 15 02:38:55 1999

From: "Ulrich Windl" <ulrich.windl@rz.uni-regensburg.de>
To: linux-kernel@vger.rutgers.edu
Date: 	Tue, 15 Jun 1999 08:36:33 +0200
Cc: linux-scsi@vger.rutgers.edu


--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-scsi" in
the body of a message to majordomo@vger.rutgers.edu

home help back first fref pref prev next nref lref last post