On Fri, 26 Oct 2007 23:18:00 -0500 Rob Landley wrote: > From: Rob Landley <rob@xxxxxxxxxxx> > > 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
