Re: [PATCH 15/15] Add Documentation/DocBook/scsi_midlayer.tmpl and add to Makefile.
On Mon, 2007-10-29 at 05:12 -0500, Rob Landley wrote: Updated drivers/scsi/* patch attached. This looks good; could you redo it, adding the missing scsi docbook tmpl file and add a change log and signed off by in the manner listed in Documentation/SubmittingPatches and it can go in. Thanks, James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/15] Add Documentation/DocBook/scsi_midlayer.tmpl and add to Makefile.
On Fri, 26 Oct 2007 23:18:00 -0500 Rob Landley wrote: From: Rob Landley [EMAIL PROTECTED] Add Documentation/DocBook/scsi_midlayer.tmpl and add to Makefile. I have comments for all 15 patches here. a. You should cc: the maintainer who you want/expect to apply the patches. Always. Andrew Morton is the only person who trolls for patches on a mailing list. :) b. The function Description: section header is not strictly required by scripts/kernel-doc. It will assume that the first text section after parameters is the Description: section. FYI. c. Extraneous whitespace. Git or quilt check for this. I don't know about hg... patch 2: Warning: trailing whitespace in line 60 of drivers/scsi/scsicam.c patch 6: Warning: trailing whitespace in line 185 of drivers/scsi/scsi_ioctl.c patch 15: Warning: trailing whitespace in lines 8,43 of Documentation/DocBook/scsi_midlayer.tmpl The docbook builds cleanly and all of my following comments are just for cleanups/fixes. General: SCSI, not scsi (in text descriptions, sentences, etc.) General: IRQ, not irq (in text) General: LLD, not lld (in text) (or LLDD, not lldd) Chap. 1: Can't SCSI commands also be 32 bytes long? Chap. 3: Mid-layer: It would make more sense to me to put extra comments like Main file for the scsi midlayer. just after the source file name instead of after all of the documented interfaces in that file. scsi_finish_command - needs a short description on the first kernel-doc line. scsi_track_queue_full - ditto __scsi_device_lookup_by_target: fix text: The only reason why drivers would want to use this is because they're need to access the device list in irq context. s/would want to/should/ s/they're/they/ scsi_eh_finish_cmd: thus we really s/thus/Thus/ scsi_eh_get_sense: proccessed (sp) This has the unfortunate side effect that if a shost adapter does not automatically request sense information, that we end up shutting it down before we request it. Ugh. Fix. scsi_sense_desc_find: short function description needs to be all on one line and no blank line before parameters. scsi_get_sense_info_fld: same comments as above. scsi_init_devinfo: Don't end kernel-doc with **/ (this is just a convention, not a syntax rule). HTML output of the function description is there 2 times. I'll look into this problem. scsi_mode_sense: function short description must be on one line only (can be a long line if needed) Description text is repeated in html output; this is usually due to the function desc. being on multiple lines. scsi_device_set_state: short func desc on one line only. scsi_internal_device_block: short func desc on one line only. scsi_kunmap_atomic_sg: short func desc on one line only. scsi_target_reap: no blank line before parameter list scsi_inq_str: short func desc on one line only. scsi_host_set_state: short func desc on one line only. scsi_host_lookup: no blank line before parameter list fc_host_post_event: short func desc on one line only. fc_host_post_vendor_event: a fc_host s/a/an/ fc_remove_host: short func desc on one line only. fc_remote_port_add: short func desc on one line only. fc_remote_port_delete: short func desc on one line only. fc_remote_port_rolechg: short func desc on one line only. iscsi_destroy_conn: This can be called from a LLD or iscsi_transport. s/a/an/ sas_remove_children: sas_remove_host: sas_phy_alloc: sas_phy_add: sas_phy_free: sas_phy_delete: scsi_is_sas_phy: sas_port_delete: scsi_is_sas_port: sas_rphy_add: sas_rphy_free: sas_rphy_delete: sas_rphy_remove: scsi_is_sas_rphy: sas_attach_transport: sas_release_transport: s/--/-/ in first line of kernel-doc sas_port_add: no blank line before parameter list srp_rport_add: no blank line before parameter list srp_rport_del: srp_remove_host: srp_attach_transport: srp_release_transport: s/--/-/ in first line of kernel-doc ### Thanks, --- ~Randy - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/15] Add Documentation/DocBook/scsi_midlayer.tmpl and add to Makefile.
On Fri, 2007-10-26 at 23:05 -0700, Randy Dunlap wrote: On Fri, 26 Oct 2007 23:18:00 -0500 Rob Landley wrote: From: Rob Landley [EMAIL PROTECTED] Add Documentation/DocBook/scsi_midlayer.tmpl and add to Makefile. I have comments for all 15 patches here. a. You should cc: the maintainer who you want/expect to apply the patches. Always. Andrew Morton is the only person who trolls for patches on a mailing list. :) Realistically too, for this first batch that covers mostly the mid-layer and the transport classes, a single patch like you originally submitted was just fine. One patch per file isn't. Patches should be split across functional areas (in this case the function is simply adding the SCSI mid layer to the docbook build, so they can all go together). b. The function Description: section header is not strictly required by scripts/kernel-doc. It will assume that the first text section after parameters is the Description: section. FYI. c. Extraneous whitespace. Git or quilt check for this. I don't know about hg... Actually, the checkpatch.pl script in the scripts directory of the kernel is very good for this (I ran it on the original monolithic patch): [EMAIL PROTECTED] ./scripts/checkpatch.pl ~/tmp.mail ERROR: trailing whitespace #1065: FILE: drivers/scsi/scsi_ioctl.c:185: + * a pointer to a struct scsi_device. $ ERROR: trailing whitespace #1670: FILE: drivers/scsi/scsicam.c:60: + * Description : determine the BIOS mapping/geometry used for a drive in a $ total: 2 errors, 0 warnings, 1502 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. In addition, there seem to be a few functions that you added kerneldoc for which don't show up in the doc output. The one that caught my eye was scsi_cmd_get_serial(), but I think there are others. James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 15/15] Add Documentation/DocBook/scsi_midlayer.tmpl and add to Makefile.
From: Rob Landley [EMAIL PROTECTED] Add Documentation/DocBook/scsi_midlayer.tmpl and add to Makefile. Signed-off-by: Rob Landley [EMAIL PROTECTED] --- Documentation/DocBook/Makefile |2 Documentation/DocBook/scsi_midlayer.tmpl | 409 + 2 files changed, 410 insertions(+), 1 deletion(-) --- /dev/null 2007-04-23 10:59:00.0 -0500 +++ hg/Documentation/DocBook/scsi_midlayer.tmpl 2007-10-26 16:53:44.0 -0500 @@ -0,0 +1,409 @@ +?xml version=1.0 encoding=UTF-8? +!DOCTYPE book PUBLIC -//OASIS//DTD DocBook XML V4.1.2//EN + http://www.oasis-open.org/docbook/xml/4.1.2/docbookx.dtd; [] + +book id=scsimid + bookinfo +titleSCSI Mid Layer Guide/title + +authorgroup + author +firstnameJames/firstname +surnameBottomley/surname +affiliation + address +email[EMAIL PROTECTED]/email + /address +/affiliation + /author + + author +firstnameRob/firstname +surnameLandley/surname +affiliation + address +email[EMAIL PROTECTED]/email + /address +/affiliation + /author + +/authorgroup + +copyright + year2007/year + holderLinux Foundation/holder +/copyright + +legalnotice + para +This documentation is free software; you can redistribute +it and/or modify it under the terms of the GNU General Public +License version 2. + /para + + para +This program is distributed in the hope that it will be +useful, but WITHOUT ANY WARRANTY; without even the implied +warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. +For more details see the file COPYING in the source +distribution of Linux. + /para +/legalnotice + /bookinfo + + toc/toc + + chapter id=intro +titleIntroduction/title +sect1 id=protocol_vs_bus + titleProtocol vs bus/title + para +Once upon a time, the Small Computer Systems Interface defined both +a parallel I/O bus and a data protocol to connect a wide variety of +peripherals (disk drives, tape drives, modems, printers, scanners, +optical drives, test equipment, and medical devices) to a host +computer. + /para + para +Although the old parallel (fast/wide/ultra) SCSI bus has largely +fallen out of use, the SCSI command set is more widely used than ever +to communicate with devices over a number of different busses. + /para + para +The ulink url='http://www.t10.org/scsi-3.htm'SCSI protocol/ulink +is a big-endian peer-to-peer packet based protocol. SCSI commands +are 6, 10, 12, or 16 bytes long, often followed by an associated data +payload. + /para + para +SCSI commands can be transported over just about any kind of bus, and +are the default protocol for storage devices attached to USB, SATA, +SAS, Fibre Channel, FireWire, and ATAPI devices. SCSI packets are +also commonly exchanged over Infiniband, +ulink url='http://i2o.shadowconnect.com/faq.php'I20/ulink, TCP/IP +(ulink url='http://en.wikipedia.org/wiki/ISCSI'iSCSI/ulink), even +ulink url='http://cyberelk.net/tim/parport/parscsi.html'Parallel +ports/ulink. + /para +/sect1 +sect1 id=subsystem_design + titleDesign of the Linux SCSI subsystem/title + para +The SCSI subsystem uses a three layer design, with upper, mid, and low +layers. Every operation involving the SCSI subsystem (such as reading +a sector from a disk) uses one driver at each of the 3 levels: one +upper layer driver, one lower layer driver, and the scsi midlayer. + /para + para +The SCSI upper layer provides the interface between userspace and the +kernel, in the form of block and char device nodes for I/O and +ioctl(). The SCSI lower layer contains drivers for specific hardware +devices. + /para + para +In between is the SCSI mid-layer, analogous to a network routing +layer such as the IPv4 stack. The SCSI mid-layer routes a packet +based data protocol between the upper layer's /dev nodes and the +corresponding devices in the lower layer. It manages command queues, +provides error handling and power management functions, and responds +to ioctl() requests. + /para +/sect1 + /chapter + + chapter id=upper_layer +titleSCSI upper layer/title +para + The upper layer supports the user-kernel interface by providing + device nodes. +/para +sect1 id=sd + titlesd (SCSI Disk)/title + parasd (sd_mod.o)/para +!-- !Idrivers/scsi/sd.c -- +/sect1 +sect1 id=sr + titlesr (SCSI CD-ROM)/title + parasr (sr_mod.o)/para +/sect1 +sect1 id=st + titlest (SCSI
