Re: [PATCH] mmc: align data size for host which only supportspower-of-2 block

[Pierre Ossman]

On Fri,  6 Mar 2009 11:15:21 +0800
Bryan Wu <cooloney@xxxxxxxxxx> wrote:

> From: Cliff Cai <cliff.cai@xxxxxxxxxx>
> 
> Signed-off-by: Cliff Cai <cliff.cai@xxxxxxxxxx>
> Signed-off-by: Bryan Wu <cooloney@xxxxxxxxxx>
> ---

This patch seems premature as there is no associated modification of
any of the host drivers.

>  drivers/mmc/core/core.c  |    8 +++++++-
>  include/linux/mmc/host.h |    1 +
>  2 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index df6ce4a..15119df 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -321,7 +321,13 @@ unsigned int mmc_align_data_size(struct mmc_card *card, unsigned int sz)
>  	 * the core about its problems yet, so for now we just 32-bit
>  	 * align the size.
>  	 */
> -	sz = ((sz + 3) / 4) * 4;
> +
> +	/* Align size for host which only supports power-of-2 block */
> +	if (card->host->powerof2_block) {
> +		if (sz & (sz - 1))
> +			sz = 1 << fls(sz);
> +	} else
> +		sz = ((sz + 3) / 4) * 4;
>  
>  	return sz;
>  }

At the very least, the comment at the top of this function must go. But
really, if we want to improve this we should probably do it properly
and have flags for the different limitations that are available.

Padding to a power of two size can also mean a rather large padding. We
might need to check the host data limits after doing the adjustment.

> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 4e45725..7416ed1 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -162,6 +162,7 @@ struct mmc_host {
>  	struct dentry		*debugfs_root;
>  
>  	unsigned long		private[0] ____cacheline_aligned;
> +	unsigned int		powerof2_block;	/* host only supports power-of-2 block */
>  };
>  
>  extern struct mmc_host *mmc_alloc_host(int extra, struct device *);

This is just broken. Putting it after private completely breaks
accesses to the host driver private data.

Also, there are a whole bunch of alignment issues that can occur. We
need some kind of flags field or a bitfield for this.

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/