Hello Bart,
On Mon, 2019-12-09 at 10:02 -0800, Bart Van Assche wrote:
> Instead of changing endianness in-place and copying the data in two
> steps,
> do this in one step. This patch makes is a preparation step for
> fixing the
> endianness warnings reported by 'sparse' for the qla2xxx driver.
>
> Cc: Quinn Tran <qutran@xxxxxxxxxxx>
> Cc: Martin Wilck <mwilck@xxxxxxxx>
> Cc: Daniel Wagner <dwagner@xxxxxxx>
> Cc: Roman Bolshakov <r.bolshakov@xxxxxxxxx>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
> drivers/scsi/qla2xxx/qla_def.h | 2 +-
> drivers/scsi/qla2xxx/qla_init.c | 9 ++++-----
> 2 files changed, 5 insertions(+), 6 deletions(-)
I suppose you're aware that this patch conflicts with Roman's pending
patch "scsi: qla2xxx: Don't defer relogin unconditonally".
> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
> index 6c28f38f8021..ddd8bf7997a8 100644
> --- a/drivers/scsi/qla2xxx/qla_init.c
> +++ b/drivers/scsi/qla2xxx/qla_init.c
> @@ -5047,13 +5047,12 @@ qla2x00_configure_local_loop(scsi_qla_host_t *vha)
> rval = qla24xx_get_port_login_templ(vha,
> ha->init_cb_dma, (void *)ha->init_cb, sz);
> if (rval == QLA_SUCCESS) {
> + __be32 *q = &ha->plogi_els_payld.data[0];
> +
> bp = (uint32_t *)ha->init_cb;
> - for (i = 0; i < sz/4 ; i++, bp++)
> - *bp = cpu_to_be32(*bp);
> + for (i = 0; i < sz/4 ; i++, bp++, q++)
> + *q = cpu_to_be32(*bp);
>
> - memcpy(&ha->plogi_els_payld.data,
> - (void *)ha->init_cb,
> - sizeof(ha->plogi_els_payld.data));
> set_bit(RELOGIN_NEEDED, &vha->dpc_flags);
> } else {
> ql_dbg(ql_dbg_init, vha, 0x00d1,
A side effect of this patch would be that after return from
qla2x00_configure_local_loop(), the byte ordering in ha->init_cb
remains in CPU byte ordering, whereas before your patch, it would have
been converted to be32. I'm uncertain if that would matter later on.
The following is not a problems with your patch, but what's really
weird is that in qla24xx_get_port_login_templ() (which is only called
from here), the buffer is converted from le32 to CPU endianness, and
then here, in a second step, from CPU to be32. I'm wondering which byte
order this buffer is supposed to have, and whether that's different
depending on which mode the controller is operating in (the be32
conversion seems to be applied in N2N mode only). Moreover, looking at
the definition of init_cb_t in qla_def.h, this data structure actually
has mixed endianness, making me doubt that changing the endianness of
the whole buffer makes sense at all. Or is ha->init_cb simply being
abused in this part of the code?
I guess only Himanshu or Quinn can tell.
Martin
