[PATCH] qla2xxx: Fix the ARM build

[PATCH] qla2xxx: Fix the ARM build

[Bart Van Assche]

This patch fixes the build errors reported by the kernel test robot on June
7th. Does this perhaps mean that so far nobody has tried to use the qla2xxx
driver on an ARM system? For the kernel test robot output, see also
https://lore.kernel.org/lkml/202006070558.Cy93XggE%25lkp@intel.com/.

Cc: Nilesh Javali <njavali@marvell.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Daniel Wagner <dwagner@suse.de>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/qla2xxx/qla_def.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index 42dbf90d4651..edc9c082dc6e 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -46,7 +46,7 @@ typedef struct {
 	uint8_t al_pa;
 	uint8_t area;
 	uint8_t domain;
-} le_id_t;
+} __packed le_id_t;
 
 #include "qla_bsg.h"
 #include "qla_dsd.h"
@@ -1841,8 +1841,8 @@ typedef union {
 	struct {
 		uint8_t reserved;
 		uint8_t standard;
-	} id;
-} target_id_t;
+	} __packed id;
+} __packed target_id_t;
 
 #define SET_TARGET_ID(ha, to, from)			\
 do {							\

Re: [PATCH] qla2xxx: Fix the ARM build

[Daniel Wagner]

Hi Bart,

On Mon, Jun 08, 2020 at 09:14:03PM -0700, Bart Van Assche wrote:
> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> index 42dbf90d4651..edc9c082dc6e 100644
> --- a/drivers/scsi/qla2xxx/qla_def.h
> +++ b/drivers/scsi/qla2xxx/qla_def.h
> @@ -46,7 +46,7 @@ typedef struct {
>  	uint8_t al_pa;
>  	uint8_t area;
>  	uint8_t domain;
> -} le_id_t;
> +} __packed le_id_t;
>  
>  #include "qla_bsg.h"
>  #include "qla_dsd.h"
> @@ -1841,8 +1841,8 @@ typedef union {
>  	struct {
>  		uint8_t reserved;
>  		uint8_t standard;
> -	} id;
> -} target_id_t;
> +	} __packed id;
> +} __packed target_id_t;

This is a bit strange. Why is that only these two definitions need this
treatment? With gcc 6.3.0 on Debian stretch, the compiler did the
right thing for x86_64, ARMv7 and ARMv8. In all cases target_id_t is 2 bytes
long.

Or does this happen because target_id_t is embedded into cmd_entry_t?

Here is the test code:

#include <stdio.h>
#include <stdint.h>

typedef union {
	uint16_t	extended;
	struct {
		uint8_t reserved;
		uint8_t standard;
	} id;
} target_id_t;

int main(int argc, char *argv[])
{
	printf("sizeof(target_id_t) = %zu\n", sizeof(target_id_t));
	return 0;
}

The only thing which is different to the above code is the use uint16_t
instead of __le16. But I thought this should make a difference.

Thanks,
Daniel

Re: [PATCH] qla2xxx: Fix the ARM build

[Bart Van Assche]

On 2020-06-09 00:30, Daniel Wagner wrote:
> Hi Bart,
> 
> On Mon, Jun 08, 2020 at 09:14:03PM -0700, Bart Van Assche wrote:
>> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
>> index 42dbf90d4651..edc9c082dc6e 100644
>> --- a/drivers/scsi/qla2xxx/qla_def.h
>> +++ b/drivers/scsi/qla2xxx/qla_def.h
>> @@ -46,7 +46,7 @@ typedef struct {
>>  	uint8_t al_pa;
>>  	uint8_t area;
>>  	uint8_t domain;
>> -} le_id_t;
>> +} __packed le_id_t;
>>  
>>  #include "qla_bsg.h"
>>  #include "qla_dsd.h"
>> @@ -1841,8 +1841,8 @@ typedef union {
>>  	struct {
>>  		uint8_t reserved;
>>  		uint8_t standard;
>> -	} id;
>> -} target_id_t;
>> +	} __packed id;
>> +} __packed target_id_t;
> 
> This is a bit strange. Why is that only these two definitions need this
> treatment? With gcc 6.3.0 on Debian stretch, the compiler did the
> right thing for x86_64, ARMv7 and ARMv8. In all cases target_id_t is 2 bytes
> long.

Hi Daniel,

That's a good catch. I checked again and the __packed annotations for
target_id_t are not necessary. I will send a v2 of this patch.

Thanks,

Bart.