On 5/15/20 9:22 PM, Bart Van Assche wrote:
On 2020-05-14 23:50, Hannes Reinecke wrote:
On 5/14/20 11:35 PM, Bart Van Assche wrote:
diff --git a/drivers/scsi/qla2xxx/qla_tmpl.c
b/drivers/scsi/qla2xxx/qla_tmpl.c
index f05a4fa2b9d7..b91ec1c3a3ae 100644
--- a/drivers/scsi/qla2xxx/qla_tmpl.c
+++ b/drivers/scsi/qla2xxx/qla_tmpl.c
@@ -922,9 +922,9 @@ qla27xx_firmware_info(struct scsi_qla_host *vha,
tmp->firmware_version[0] = vha->hw->fw_major_version;
tmp->firmware_version[1] = vha->hw->fw_minor_version;
tmp->firmware_version[2] = vha->hw->fw_subminor_version;
- tmp->firmware_version[3] = cpu_to_le32(
+ tmp->firmware_version[3] = (__force u32)cpu_to_le32(
vha->hw->fw_attributes_h << 16 | vha->hw->fw_attributes);
- tmp->firmware_version[4] = cpu_to_le32(
+ tmp->firmware_version[4] = (__force u32)cpu_to_le32(
vha->hw->fw_attributes_ext[1] << 16 |
vha->hw->fw_attributes_ext[0]);
}
Why do you need (__force u32) here?
It should be a 32bit array, and cpu_to_le32() trivially returns 32bits.
What's there to force?
The hw->fw_{major,minor,subminor}_version and also the
hw->fw_attributes_ext variables have been annotated as CPU endian. I
inserted the (__force u32) casts because that suppresses the endianness
warnings without affecting the generated code on little endian or big
endian systems. Thinking further about this, storing CPU endian values
in a firmware data structure is most likely wrong. How about modifying
patch 15/15 as follows?
drivers/scsi/qla2xxx/qla_tmpl.c | 10 +++++-----
drivers/scsi/qla2xxx/qla_tmpl.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_tmpl.c b/drivers/scsi/qla2xxx/qla_tmpl.c
index b91ec1c3a3ae..8dc82cfd38b2 100644
--- a/drivers/scsi/qla2xxx/qla_tmpl.c
+++ b/drivers/scsi/qla2xxx/qla_tmpl.c
@@ -919,12 +919,12 @@ static void
qla27xx_firmware_info(struct scsi_qla_host *vha,
struct qla27xx_fwdt_template *tmp)
{
- tmp->firmware_version[0] = vha->hw->fw_major_version;
- tmp->firmware_version[1] = vha->hw->fw_minor_version;
- tmp->firmware_version[2] = vha->hw->fw_subminor_version;
- tmp->firmware_version[3] = (__force u32)cpu_to_le32(
+ tmp->firmware_version[0] = cpu_to_le32(vha->hw->fw_major_version);
+ tmp->firmware_version[1] = cpu_to_le32(vha->hw->fw_minor_version);
+ tmp->firmware_version[2] = cpu_to_le32(vha->hw->fw_subminor_version);
+ tmp->firmware_version[3] = cpu_to_le32(
vha->hw->fw_attributes_h << 16 | vha->hw->fw_attributes);
- tmp->firmware_version[4] = (__force u32)cpu_to_le32(
+ tmp->firmware_version[4] = cpu_to_le32(
vha->hw->fw_attributes_ext[1] << 16 | vha->hw->fw_attributes_ext[0]);
}
diff --git a/drivers/scsi/qla2xxx/qla_tmpl.h b/drivers/scsi/qla2xxx/qla_tmpl.h
index bba8dc90acfb..89280b3477aa 100644
--- a/drivers/scsi/qla2xxx/qla_tmpl.h
+++ b/drivers/scsi/qla2xxx/qla_tmpl.h
@@ -27,7 +27,7 @@ struct __packed qla27xx_fwdt_template {
uint32_t saved_state[16];
uint32_t reserved_3[8];
- uint32_t firmware_version[5];
+ __le32 firmware_version[5];
};
#define TEMPLATE_TYPE_FWDUMP 99
Thanks,
Bart.
Yes, that's better.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer