On Tue, 2017-12-19 at 22:56 -0800, Himanshu Madhani wrote:
> +#define QLA_ENA_FW_RES_TRACKING(_ha) { \
> + int i; \
> + _ha->base_qpair->fw_res_tracking = 1; \
> + for (i = 0; i < _ha->max_qpairs; i++) { \
> + if (_ha->queue_pair_map[i]) \
> + _ha->queue_pair_map[i]->fw_res_tracking = 1; \
> + } \
> +}
> +
> +#define QLA_DIS_FW_RES_TRACKING(_ha) { \
> + int i; \
> + _ha->base_qpair->fw_res_tracking = 0; \
> + for (i = 0; i < _ha->max_qpairs; i++) { \
> + if (_ha->queue_pair_map[i]) \
> + _ha->queue_pair_map[i]->fw_res_tracking = 0; \
> + } \
> +}
Has this patch been verified with checkpatch? Did checkpatch suggest to surround
these macros with do / while (0)?
> int
> qla2x00_dfs_setup(scsi_qla_host_t *vha)
> @@ -504,6 +776,33 @@ qla2x00_dfs_setup(scsi_qla_host_t *vha)
> "Unable to create debugFS naqp node.\n");
> goto out;
> }
> +
> + if (ql2xtrackfwres) {
> + ha->tgt.dfs_ini_iocbs =
> + debugfs_create_file("reserve_ini_iocbs",
> + 0400, ha->dfs_dir, vha, &dfs_ini_iocbs_ops);
> + if (!ha->tgt.dfs_ini_iocbs) {
> + ql_log(ql_log_warn, vha, 0xd011,
> + "Unable to create debugFS reserve_ini_iocbs node.\n");
> + goto out;
> + }
The patch description shows how to control the new attributes through sysfs
but this code adds new debugfs attributes. Sorry but that really confuses me.
> +int ql2xtrackfwres;
> +module_param(ql2xtrackfwres, int, 0444);
> +MODULE_PARM_DESC(ql2xtrackfwres,
> + "Track FW resource. 0(default): disabled");
Why is this a module parameter instead of e.g. a sysfs attribute? Module
parameters are annoying if the qla2xxx driver is not built as a module.
> @@ -6412,6 +6433,9 @@ qlt_enable_vha(struct scsi_qla_host *vha)
> qla24xx_disable_vp(vha);
> qla24xx_enable_vp(vha);
> } else {
> + if (ql2xtrackfwres && (IS_QLA83XX(ha) || IS_QLA27XX(ha)))
> + QLA_ENA_FW_RES_TRACKING(ha);
Why is there a dependency on the adapter type in the above code?
Thanks,
Bart.
