RE: [PATCH v2 01/10] qla2xxx: Add start + stop bsg's

[Quinn Tran <qutran@marvell.com>]

Himanshu,

Thanks for reviewing.

> +	} edif;
>   } fc_port_t;
>   

Same nit as Hannes about using uint16_t, while correcting that please use Linux styles for comment throuout this patch. I would suggest scan through all patches and fix it in v2.

QT: ack. Will do for next re-submission.


> +#include "qla_def.h"
> +//#include "qla_edif.h"
> +

why comment out?  if not needed remove rather than comment it out.

QT:  short indicision in the patch splitting. Will fix in v3

> +static int
> +qla_edif_app_check(scsi_qla_host_t *vha, struct app_id appid) {
> +	int rval = 0;	/* assume failure */

Comment above does not make sense if you are assiging rval = 0.

QT: will revise in v3.

----
please use kernel-doc format for the funtion description

> +/*
> + * reset the session to auth wait.
> + */

QT: will have to circle back to this in the future phase as part of overall prettiness.

> +			__func__);

fix indentation for the print statement and no need for multiple lines for the parameters.

QT: ack. On all indentation comment.  I was following checkpatch recommendation.

----

> +	/* if we leave this running short waits are operational < 16 secs */
> +	qla_enode_stop(vha);        /* stop enode */


I don't really understand useage of the above stop fucntion, it prints message and returns back after checking vha->pur_cinfo.enode_flags, but does not take any action *if* the enode *is* active?

QT: will remove nondescript comment.   Those were past comment left behind, while the code intention has changed over time.  The intent here is to do cleanup if user shutdown (ex: ipsec stop).

-----
> +	qla_edb_stop(vha);          /* stop db */
> +

Same here for this function, it just prints message that doorbell is not enabled, but does not take any action if it *is* enabled.

Am I missing something?

QT:  Due to the patch splitting, this call is more of a skeleton here.   From the sum of all the patches, it perform various cleanup as part of user shutdown.

------
> +#define RX_DELAY_DELETE_TIMEOUT 20			// 30 second timeout

fix comment

QT:  Ack.  Will fix in v3


Regards,
Quinn Tran