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

2015-09-29 Thread Matthew R. Ochs 
> On Sep 28, 2015, at 11:36 PM, Andrew Donnellan  
> wrote:
> On 26/09/15 09:18, Matthew R. Ochs wrote:
>> 
>>   */
>>  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.

I'll make a note of this.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2015-09-28 Thread Andrew Donnellan 

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 
Signed-off-by: Manoj N. Kumar 
Reviewed-by: Brian King 


Reviewed-by: Andrew Donnellan 

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
andrew.donnel...@au1.ibm.com  Australia Development Lab, Canberra
+61 2 6201 8874 (work)IBM Australia Limited

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2015-09-25 Thread Matthew R. Ochs 
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 
Signed-off-by: Manoj N. Kumar 
Reviewed-by: Brian King 
---
 drivers/scsi/cxlflash/main.c | 68 
 1 file changed, 25 insertions(+), 43 deletions(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 729f742..7c76227 100644
--- 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
  */
 static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp)
 {
@@ -597,7 +594,7 @@ out:
 
 /**
  * cxlflash_wait_for_pci_err_recovery() - wait for error recovery during probe
- * @cxlflash:  Internal structure associated with the host.
+ * @cfg:   Internal structure associated with the host.
  */
 static void cxlflash_wait_for_pci_err_recovery(struct cxlflash_cfg *cfg)
 {
@@ -611,7 +608,7 @@ static void cxlflash_wait_for_pci_err_recovery(struct 
cxlflash_cfg *cfg)
 
 /**
  * free_mem() - free memory associated with the AFU
- * @cxlflash:  Internal structure associated with the host.
+ * @cfg:   Internal structure associated with the host.
  */
 static void free_mem(struct cxlflash_cfg *cfg)
 {
@@ -633,7 +630,7 @@ static void free_mem(struct cxlflash_cfg *cfg)
 
 /**
  * stop_afu() - stops the AFU command timers and unmaps the MMIO space
- * @cxlflash:  Internal structure associated with the host.
+ * @cfg:   Internal structure associated with the host.
  *
  * Safe to call with AFU in a partially allocated/initialized state.
  */
@@ -655,7 +652,7 @@ static void stop_afu(struct cxlflash_cfg *cfg)
 
 /**
  * term_mc() - terminates the master context
- * @cxlflash:  Internal structure associated with the host.
+ * @cfg:   Internal structure associated with the host.
  * @level: Depth of allocation, where to begin waterfall tear down.
  *
  * Safe to call with AFU/MC in partially allocated/initialized state.
@@ -691,7 +688,7 @@ static void term_mc(struct cxlflash_cfg *cfg, enum 
undo_level level)
 
 /**
  * term_afu() - terminates the AFU
- * @cxlflash:  Internal structure associated with the host.
+ * @cfg:   Internal structure associated with the host.
  *
  * Safe to call with AFU/MC in partially allocated/initialized state.
  */
@@ -751,7 +748,7 @@ static void cxlflash_remove(struct pci_dev *pdev)
 
 /**
  * alloc_mem() - allocates the AFU and its command pool
- * @cxlflash:  Internal structure associated with the host.
+ * @cfg:   Internal structure associated with the host.
  *
  * A partially allocated state remains on failure.
  *
@@ -804,12 +801,9 @@ out:
 
 /**
  * init_pci() - initializes the host as a PCI device
- * @cxlflash:  Internal structure associated with the host.
+ * @cfg:   Internal structure associated with the host.
  *
- * Return:
- * 0 on success
- * -EIO on unable to communicate with device
- * A return code from the PCI sub-routines
+ * Return: 0 on success, -errno on failure
  */
 static int init_pci(struct cxlflash_cfg *cfg)
 {
@@ -889,11 +883,9 @@ out_release_regions:
 
 /**
  * init_scsi() - adds the host to the SCSI stack and kicks off host scan
- * @cxlflash:  Internal structure associated with the host.
+ * @cfg:   Internal structure associated with the host.
  *
- * Return:
- * 0 on success
- * A return code from adding the host
+ * Return: 0 on success, -errno on failure
  */
 static int init_scsi(struct cxlflash_cfg *cfg)
 {
@@ -1357,7 +1349,7 @@ out:
 
 /**
  * start_context() - starts the master context
- * @cxlflash:  Internal structure associated with the host.
+ * @cfg:   Internal structure associated with the host.
  *
  * Return: A success or failure value from CXL services.
  */
@@ -1375,12 +1367,10 @@ static int start_context(struct cxlflash_cfg *cfg)
 
 /**
  * read_vpd() - obtains the WWPNs from VPD
- * @cxlflash:  Internal structure associated with the host.
+ * @cfg:   Internal structure associated with the host.
  * @wwpn:  Array of size NUM_FC_PORTS to pass back WWPNs
  *
- * Return:
- * 0 on