RE: [PATCH] mmc: align data size for host which only supports power-of-2 block

[Cai, Cliff]

>
 

>-----Original Message-----
>From: Pierre Ossman [mailto:drzeus-mmc@xxxxxxxxx] 
>Sent: Sunday, March 15, 2009 4:31 AM
>To: Bryan Wu
>Cc: linux-kernel@xxxxxxxxxxxxxxx; Cai, Cliff; Bryan Wu
>Subject: Re: [PATCH] mmc: align data size for host which only 
>supports power-of-2 block
>
>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.

The SDH on Blackfin BF54x/BF51x only supports power-of-2 block
transfer,we will soon
Send the sdh driver to mainline as well.

>
>>  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.

Yes,but it's the higher level driver's job. 

>
>> 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.

Sure,it should be added before private.

Thanks

Cliff
--
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/