Hannes,
Thanks for the review.
> + uint16_t secured_login:1;
> + uint16_t app_sess_online:1;
How very uncommon; defining a uint16_t bitfield.
Why not make it uint32_t?
But adding it to the start of the structure you also induced a possible padding here; the compiler will most likely pad it to 64 bit here.
QT: ack. Will make it uniform with v3.
> +#include "qla_def.h"
> +//#include "qla_edif.h"
> +
Add a commented out header in a new file?
Why?
QT: short indicision while splitting the patch. Will fix in next version.
> + if (appid.app_vid == 0x73730001) {
Huh? A hard-coded value?
Please don't. Use a #define here to make it clear what this value is supposed to mean.
QT: ack. Will fix in v3.
> +
> + return rval;
> +}
And you can kill 'rval' by just call 'return' in the if branches.
Plus this function should probably be a 'bool' function ...
QT: ack. Will fix in v3.
> + ql_dbg(ql_dbg_edif, fcport->vha, 0xf086,
> + "%s: waited ONLY for session - %8phC, loopid=%x portid=%06x fcport=%p state=%u, cnt=%u\n",
> + __func__, fcport->port_name, fcport->loop_id,
> + fcport->d_id.b24, fcport, fcport->disc_state, cnt);
Indentation.
QT: ack. will make msg shorter for various indentation.
----
>
Isn't this an optional feature?
Maybe make it a compile-time option to enable EDIF?
QT: It is. There is a driver knob that control the feature comes in patch 9 of this series.
Regards,
Quinn Tran
