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.
