> >>> + /* for SDUC */
> >>> + u8 has_ext_addr;
> >>> + u8 ext_addr;
> >>> + u16 reserved;
> >>
> >> Is there a reason for has_ext_addr being u8?
> > Theoretically a single bit suffices, and since ext_addr uses only 6
> > bits, I had that bit to spare in ext_addr, but I see no reason to be cheap here -
> see the reserved u16.
> >
> >> What's the reserved for?
> > Not to break the packed 4bytes alignment of mmc_command.
>
> FWIW, that's what it looks like so I was a bit surprised:
>
> pahole -C mmc_command vmlinux
> struct mmc_command {
> u32 opcode; /* 0 4 */
> u32 arg; /* 4 4 */
> u32 resp[4]; /* 8 16 */
> unsigned int flags; /* 24 4 */
> bool has_ext_addr; /* 28 1 */
> u8 ext_addr; /* 29 1 */
> u16 reserved; /* 30 2 */
> unsigned int retries; /* 32 4 */
> int error; /* 36 4 */
> unsigned int busy_timeout; /* 40 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> struct mmc_data * data; /* 48 8 */
> struct mmc_request * mrq; /* 56 8 */
>
> /* size: 64, cachelines: 1, members: 12 */
> /* sum members: 60, holes: 1, sum holes: 4 */ };
Moving the sduc members to the end minimizes the padding further:
$ pahole -C mmc_command vmlinux
struct mmc_command {
u32 opcode; /* 0 4 */
u32 arg; /* 4 4 */
u32 resp[4]; /* 8 16 */
unsigned int flags; /* 24 4 */
unsigned int retries; /* 28 4 */
int error; /* 32 4 */
unsigned int busy_timeout; /* 36 4 */
struct mmc_data * data; /* 40 8 */
struct mmc_request * mrq; /* 48 8 */
u8 has_ext_addr; /* 56 1 */
u8 ext_addr; /* 57 1 */
u16 reserved; /* 58 2 */
/* size: 64, cachelines: 1, members: 12 */
/* padding: 4 */
};
I guess I can do that.
Thanks,
Avri
>
> has_ext_addr also should be equivalent to:
> mmc_card_ult_capacity(card) && opcode in [17,18,24,25,32,33] but the flag is
> also fine.
>
> I'm a bit hesitant to put my R-b but it all looks plausible to me FWIW.
