PATCH: Config option to pad SDIO sizes

[Pierre Ossman]

On Tue, 1 Jul 2008 11:39:00 +0200
"Johan Adolfsson" <Johan.Adolfsson at axis.com> wrote:

> 
> Havent tested, but I guess it makes sence to move it up from the driver,
> but here are some comments/thoughts:
> 

Please do some testing though, as I'd like to have this included in
2.6.27.

> +unsigned int mmc_align_data_size(struct mmc_card *card, unsigned int sz)
> ..
> +	 */
> +	if (sz & 3)
> +		sz = ((sz + 3) / 4) * 4;
> +
> +	return sz;
> 
> I suggest skip the if(), since it doesn't save any instructions
> compared to always doing the padding (not for x86, cris or arm at least).
> Isn't: sz = (sz + 3) &~3; clearer or can we trust all compilers to do 
> the right thing here?

No idea. I was more concerned about readability than anything else. I
doubt this code will be a performance problem.

> 
> Possibly make the function inline ?
> 

The ambition is to make it more complex to properly deal with more
nasty controllers, so it wouldn't stay inline for long anyway.

> 
> +	/*
> +	 * If we can still do this with just a byte transfer, then
> +	 * we're done.
> +	 */
> +	if ((sz < func->cur_blksize) && (sz < 512))
> +		return sz;
> vs
> 
> -	chunk = size;
> -	if ((chunk > card->func->cur_blksize) || (chunk > 512)) {
> -		chunk = (chunk + card->func->cur_blksize - 1) /
> -			card->func->cur_blksize * card->func->cur_blksize;
> -	}
> 
> indicates that it should really be: (note the added =)
> 
> +	if ((sz <= func->cur_blksize) && (sz <= 512))
> +		return sz;
> 
> Shouldn't it?
> 

Good catch. I've done a separate cleanup of the whole byte/block mode
mess, but we can leave that for a later discussion.

> 
> Is the added multi_block stuff really related to the pad size
> or just needed anyway ?

Just trying to solve a related problem at the same time (one that the
code ripped out of if_sdio sort of solves). Imagine if you have a
transfer of 1233 bytes. First you'd need to pad it to 1236 bytes to
handle 32-bit size alignment bugs. But transferring this chunk would
result in one request of 2 * 512 bytes, and one of 212 bytes. Padding
it to 1536 bytes allows you to a single request with 3 * 512 bytes.

> Isn't it a valid assumption that the cur_blksize is 32 bit aligned so that
> aligning size would not increase the size over the cur_blksize if 
> it was not already (and that is handled elsewhere)?

No, the block size might be anything. Most users will probably have
some power of two size, but it is not something we can rely on.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.