Subject: Re: [PATCH v4 23/32] cxlflash: Fix function prolog parameters and return codes

From: Andrew Donnellan <[email-masked]>
Date: Tue, 29 Sep 2015 14:36:11 +1000

On 26/09/15 09:18, Matthew R. Ochs wrote:
> Several function prologs have incorrect parameter names and return
> code descriptions. This can lead to confusion when reviewing the
> source and creates inaccurate documentation.
>
> To remedy, update the function prologs to properly reflect parameter
> names and return codes.
>
> Signed-off-by: Matthew R. Ochs <[email-masked]>
> Signed-off-by: Manoj N. Kumar <[email-masked]>
> Reviewed-by: Brian King <[email-masked]>

Reviewed-by: Andrew Donnellan <[email-masked]>

See further comments below.

> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -401,8 +401,7 @@ static void wait_resp(struct afu *afu, struct afu_cmd *cmd)
>    * @tmfcmd:	TMF command to send.
>    *
>    * Return:
> - *	0 on success
> - *	SCSI_MLQUEUE_HOST_BUSY when host is busy
> + *	0 on success or SCSI_MLQUEUE_HOST_BUSY
>    */
>   static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd)
>   {
> @@ -491,9 +490,7 @@ static const char *cxlflash_driver_info(struct Scsi_Host *host)
>    * @host:	SCSI host associated with device.
>    * @scp:	SCSI command to send.
>    *
> - * Return:
> - *	0 on success
> - *	SCSI_MLQUEUE_HOST_BUSY when host is busy
> + * Return: 0 on success or SCSI_MLQUEUE_HOST_BUSY
>    */

I'd prefer it to say "SCSI_MLQUEUE_HOST_BUSY on failure". (Aesthetically 
I prefer having it on a separate line, but that's just personal preference.)

As an aside, while checking the correctness of this, I found that the 
comment for cxlflash_send_cmd() states that it returns -1 on failure, 
when the only error value it actually returns is SCSI_MLQUEUE_HOST_BUSY. 
If you send a v5 you might want to fix this.

Andrew

-- 
Andrew Donnellan              Software Engineer, OzLabs
[email-masked]  Australia Development Lab, Canberra
+61 2 6201 8874 (work)        IBM Australia Limited