Re: [PATCH 2/9] sd: Fix functions description

2017-04-23 Thread Damien Le Moal
Bart,

On 4/22/17 02:28, Bart Van Assche wrote:
> On Fri, 2017-04-21 at 18:16 +0900, damien.lem...@wdc.com wrote:
>> - *   Returns 0 if successful. Returns a negated errno value in case
>> - *   of error.
>> + * Returns 0 if successful. Returns a negated errno value in case
>> + * of error.
> 
> Hello Damien,
> 
> The argument name fixes are definitely welcome in my opinion. But I see
> that this patch not only fixes argument names and function descriptions
> but also includes whitespace-only changes like the above. Sorry but I
> don't like whitespace-only changes ...

Yes, I understand, you were clear about that.
The "white space" changes intent are to match the preferred format for
functions documentation (From Documentation/doc-guide/kerne-doc.rst)

  /**
   * foobar() - Brief description of foobar.
   * @arg: Description of argument of foobar.
   *
   * Longer description of foobar.
   *
   * Return: Description of return value of foobar.
   */

If that is wrong/unnecessary, then I will rewrite that patch to only
limit its scope to the argument name fixes.

Best regards.

-- 
Damien Le Moal,
Western Digital


Re: [PATCH 2/9] sd: Fix functions description

2017-04-21 Thread Bart Van Assche
On Fri, 2017-04-21 at 18:16 +0900, damien.lem...@wdc.com wrote:
> - *   Returns 0 if successful. Returns a negated errno value in case
> - *   of error.
> + * Returns 0 if successful. Returns a negated errno value in case
> + * of error.

Hello Damien,

The argument name fixes are definitely welcome in my opinion. But I see
that this patch not only fixes argument names and function descriptions
but also includes whitespace-only changes like the above. Sorry but I
don't like whitespace-only changes ...

Bart.

[PATCH 2/9] sd: Fix functions description

2017-04-21 Thread damien . lemoal
From: Damien Le Moal 

Use regular format comments documenting functions, fix argument names,
etc

No functional change is introduced by this patch.

Signed-off-by: Damien Le Moal 
---
 drivers/scsi/sd.c | 272 +++---
 1 file changed, 136 insertions(+), 136 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index db362da..935ce34 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -705,11 +705,10 @@ static void sd_config_discard(struct scsi_disk *sdkp, 
unsigned int mode)
 
 /**
  * sd_setup_discard_cmnd - unmap blocks on thinly provisioned device
- * @sdp: scsi device to operate on
- * @rq: Request to prepare
+ * @cmd: command to prepare
  *
- * Will issue either UNMAP or WRITE SAME(16) depending on preference
- * indicated by target device.
+ * Will set either UNMAP, WRITE SAME(10) or WRITE SAME(16) depending
+ * on preference indicated by target device.
  **/
 static int sd_setup_discard_cmnd(struct scsi_cmnd *cmd)
 {
@@ -827,7 +826,7 @@ static void sd_config_write_same(struct scsi_disk *sdkp)
  * sd_setup_write_same_cmnd - write the same data to multiple blocks
  * @cmd: command to prepare
  *
- * Will issue either WRITE SAME(10) or WRITE SAME(16) depending on
+ * Will setup either WRITE SAME(10) or WRITE SAME(16) depending on
  * preference indicated by target device.
  **/
 static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
@@ -1189,20 +1188,18 @@ static void sd_uninit_command(struct scsi_cmnd *SCpnt)
 }
 
 /**
- * sd_open - open a scsi disk device
- * @inode: only i_rdev member may be used
- * @filp: only f_mode and f_flags may be used
+ * sd_open - open a scsi disk device
+ * @bdev: Block device of the scsi disk to open
+ * @mode: FMODE_* mask
  *
- * Returns 0 if successful. Returns a negated errno value in case
- * of error.
+ * Returns 0 if successful. Returns a negated errno value in case
+ * of error.
  *
- * Note: This can be called from a user context (e.g. fsck(1) )
- * or from within the kernel (e.g. as a result of a mount(1) ).
- * In the latter case @inode and @filp carry an abridged amount
- * of information as noted above.
+ * Note: This can be called from a user context (e.g. fsck(1) )
+ * or from within the kernel (e.g. as a result of a mount(1) ).
  *
- * Locking: called with bdev->bd_mutex held.
- **/
+ * Locking: called with bdev->bd_mutex held.
+ */
 static int sd_open(struct block_device *bdev, fmode_t mode)
 {
struct scsi_disk *sdkp = scsi_disk_get(bdev->bd_disk);
@@ -1265,18 +1262,16 @@ static int sd_open(struct block_device *bdev, fmode_t 
mode)
 }
 
 /**
- * sd_release - invoked when the (last) close(2) is called on this
- * scsi disk.
- * @inode: only i_rdev member may be used
- * @filp: only f_mode and f_flags may be used
- *
- * Returns 0.
+ * sd_release - invoked when the (last) close(2) is called on this
+ * scsi disk.
+ * @disk: disk to release
+ * @mode: FMODE_* mask
  *
- * Note: may block (uninterruptible) if error recovery is underway
- * on this disk.
+ * Note: may block (uninterruptible) if error recovery is underway
+ * on this disk.
  *
- * Locking: called with bdev->bd_mutex held.
- **/
+ * Locking: called with bdev->bd_mutex held.
+ */
 static void sd_release(struct gendisk *disk, fmode_t mode)
 {
struct scsi_disk *sdkp = scsi_disk(disk);
@@ -1323,19 +1318,19 @@ static int sd_getgeo(struct block_device *bdev, struct 
hd_geometry *geo)
 }
 
 /**
- * sd_ioctl - process an ioctl
- * @inode: only i_rdev/i_bdev members may be used
- * @filp: only f_mode and f_flags may be used
- * @cmd: ioctl command number
- * @arg: this is third argument given to ioctl(2) system call.
- * Often contains a pointer.
+ * sd_ioctl - process an ioctl
+ * @bdev: target block device
+ * @mode: FMODE_* mask
+ * @cmd: ioctl command number
+ * @arg: this is third argument given to ioctl(2) system call.
+ *   Often contains a pointer.
  *
- * Returns 0 if successful (some ioctls return positive numbers on
- * success as well). Returns a negated errno value in case of error.
+ * Returns 0 if successful (some ioctls return positive numbers on
+ * success as well). Returns a negated errno value in case of error.
  *
- * Note: most ioctls are forward onto the block subsystem or further
- * down in the scsi subsystem.
- **/
+ * Note: most ioctls are forward onto the block subsystem or further
+ * down in the scsi subsystem.
+ */
 static int sd_ioctl(struct block_device *bdev, fmode_t mode,
unsigned int cmd, unsigned long arg)
 {
@@ -1415,14 +1410,14 @@ static int media_not_present(struct scsi_disk *sdkp,
 }
 
 /**
- * sd_check_events - check media events
- * @disk: kernel device descriptor
- * @clearing: disk events currently being cleared
+ * sd_check_events - check media events
+ * @disk: kernel device