Messages in this thread |  | | Subject | Re: [PATCH 1/6] GenWQE PCI support, health monitoring and recovery | From | Frank Haverkamp <> | Date | Wed, 04 Dec 2013 10:59:33 +0100 |
| |
Hi Arnd,
thanks for helping to review the code.
Am Dienstag, den 03.12.2013, 16:05 +0100 schrieb Arnd Bergmann: > On Tuesday 03 December 2013, Frank Haverkamp wrote: > > Ohh, sorry __u64 of course: > > > > /* common struct for chip image exchange */ > > struct genwqe_bitstream { > > __u64 data_addr; /* pointer to image data */ > > __u32 size; /* size of image file */ > > __u32 crc; /* crc of this image */ > > __u8 partition; /* '0', '1', or 'v' */ > > __u64 target_addr; /* starting address in Flash */ > > __u8 uid; /* 1=host/x=dram */ > > > > __u64 slu_id; /* informational/sim: SluID */ > > __u64 app_id; /* informational/sim: AppID */ > > > > __u16 retc; /* returned from processing */ > > __u16 attn; /* attention code from > > processing */ > > __u32 progress; /* progress code from processing > > */ > > }; > > > > and than I do in my userspace application: > > > > load.data_addr = (unsigned long)buf; > > > > Is that ok, or must I consider more? > > > > I haven't followed the recent discussions, jumping into the middle here: > The structure above is not safe for a generic ioctl interface because it > has different padding on x86-32 and x86-64, where __u64 has different > alignment. > > You can try to avoid the implicit padding by sorting the members by size, > by making some lignments for 32-bit and 64-bit. I avoid umembers larger or by adding explicit padding. > > Arnd >
Ok, let me try to sort my entries a little differently and modify some sizes, to avoid different alignments for 32-bit and 64-bit. I avoid using __u8 now such that I always have nice 64-bit blocks. Would the following version work?
struct genwqe_bitstream { __u64 data_addr; /* pointer to image data */ __u32 size; /* size of image file */ __u32 crc; /* crc of this image */ __u64 target_addr; /* starting address in Flash */ __u32 partition; /* '0', '1', or 'v' */ __u32 uid; /* 1=host/x=dram */
__u64 slu_id; /* informational/sim: SluID */ __u64 app_id; /* informational/sim: AppID */
__u16 retc; /* returned from processing */ __u16 attn; /* attention code from processing */ __u32 progress; /* progress code from processing */ };
If not, I might have missed something, and I would appreciate if you could make up an example how a good version should look like.
Thanks
Frank
|  |