Add new fields to mmc_data in preparation to SDIO CMD53 block mode support

[kibab]

Summary

SDIO command CMD53 (IO_RW_EXTENDED) allows data transfers using blocks of 1-2048 bytes,
with a maximum of 511 blocks per request.
Extend mmc_data structure to properly describe such requests,
and initialize the new fields in kernel and userland consumers.

No actual driver changes happen yet, these will follow in the separate changes.
This change breaks kernel ABI for CAM calls, so camcontrol should be recompiled.

Test Plan

No functional changes.

[bz]

This is missing files I think. The mmcreg.h structure changes are missing from this one. This won't compile.

[kibab]

In D19779#424220, @bz wrote:

This is missing files I think. The mmcreg.h structure changes are missing from this one. This won't compile.

Sorry, something has gone wrong with my tree. I will reupload.

[imp]

In D19779#424228, @kibab wrote:

In D19779#424220, @bz wrote:

This is missing files I think. The mmcreg.h structure changes are missing from this one. This won't compile.

Sorry, something has gone wrong with my tree. I will reupload.

IF you jhust added the block count and block size to the end, there's no need to rebuild camcontrol...

[kibab]

Added (hopefully) missing files.

[imp]

sys/dev/mmc/mmcreg.h

193–194

It looks like if we add these to the end, we don't break binary compat...
But does camcontrol even know about mmc_data yet?

[kibab]

In D19779#424273, @imp wrote:

In D19779#424228, @kibab wrote:

In D19779#424220, @bz wrote:

This is missing files I think. The mmcreg.h structure changes are missing from this one. This won't compile.

Sorry, something has gone wrong with my tree. I will reupload.

IF you jhust added the block count and block size to the end, there's no need to rebuild camcontrol...

As you will see, these fields were added not to the end, but rather to the place where I felt most appropriate :-) Since we are at -CURRENT and don't plan to MFC, I thought this should be fine?

[kibab]

sys/dev/mmc/mmcreg.h

193–194

Well, if you don't zero those fields when passing structure to the kernel, these fields end up being filled with the random garbage, and this changes the behavior of the SDHC driver because now it thinks that the upper layer requested the custom block size.
And this happens regardless of the position of these fields in the struct.
camcontrol does know about mmc_data, see the code in this change:-)

[bz]

sbin/camcontrol/camcontrol.c

7796

Well, given there is a memset(,0,) above it you don't need the extra assignments, do you?

sys/cam/mmc/mmc_da.c

799

On a side-note; that malloc can fail...

1818

The softc mmcdata is allocated with M_ZERO; if it is not used anywhere else than here block_count and block_size are never touched so wouldn't need to be set to 0.
In general, if needed, I'd prefer an memset(,0,) on the structure as that way it'll not repeat the problem with other struct fields in the future?

sys/dev/mmc/mmcreg.h

579

These all are independent on the change kind-of and only needed for using the change later; really nothing to do with block_count and block_size. Can you commit them on their own please?

[imp]

sys/dev/mmc/mmcreg.h

193–194

But we bzero the CCB, which is larger than mmc_data so it won't be garbage...
And you could also add a flag MMC_DATA_BLOCK_SIZE that needs to be set for these values to be used.

[kibab]

Addressed comments

[kibab]

Killed CMD53-related changes, leaving only field changes.

[kibab]

Updated; please take a look.

sbin/camcontrol/camcontrol.c

7796

that memset() is for mmc_data variable, not for mmc_d. But I will better do another memset(,0,) for mmc_d as well.

sys/cam/mmc/mmc_da.c

799

Added a check for that.

1818

That mmcdata ends up being used in each sddastart(), so we _do_ need to initialize the fields. But you are right, I'd rather add memset(,0,).

sys/dev/mmc/mmcreg.h

193–194

CCB contains struct mmc_command and mmc_command contains a _pointer_ to mmc_data, not the actual mmc_data. So it will be garbaged unless initialized properly.

579

Done

[bz]

OK, this seems fine to me.

[kibab]

Reupload the change.

[kibab]

Try again

[imp]

sys/cam/mmc/mmc_da.c

794

In theory, we should retry at some other level... In practice, this will never happen and it's likely super hard to do that retry right anyway with any kind of meaningful testing... Other periph drivers do this so it's fine.

sys/dev/mmc/mmcreg.h

202

so you have this flag, but don't use it.
And you've not moved the new elements to the end.

[kibab]

sys/dev/mmc/mmcreg.h

202

Currently there is no driver that can use this flag. mmc_da is not one of them.

Moving elements -- ack.

[kibab]

• Move new fields to the end