Re: [PATCH v7] mmc: support BKOPS feature for eMMC

[Jaehoon Chung <jh80.chung@samsung.com>]

On 03/12/2012 01:06 AM, Konstantin Dorfman wrote:

> Hello Jaehoon,
> 
> On Fri, January 20, 2012 8:48 am, Jaehoon Chung wrote:
> ...
>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>> index 4d41fa9..109d0f0 100644
>> --- a/drivers/mmc/core/mmc_ops.c
>> +++ b/drivers/mmc/core/mmc_ops.c
>> @@ -392,13 +392,22 @@ int mmc_switch(struct mmc_card *card, u8 set, u8
>> index, u8 value,
>>  		  (index << 16) |
>>  		  (value << 8) |
>>  		  set;
>> -	cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
>> +	cmd.flags = MMC_CMD_AC;
>> +	if (index == EXT_CSD_BKOPS_START &&
>> +	    card->ext_csd.raw_bkops_status < EXT_CSD_BKOPS_LEVEL_2)
>> +		cmd.flags |= MMC_RSP_SPI_R1 | MMC_RSP_R1;
>> +	else
>> +		cmd.flags |= MMC_RSP_SPI_R1B | MMC_RSP_R1B;
> 
> It is not good to use conditional on 'index', because this function
> (mmc_switch)
> is generic and in case you want functionality like here "in some cases cmd
>  should not wait for PROG_DONE" it is better to use different approach:
> 
> 1. Use internal function with additional parameter wait_for_prog_done:
> 
> int __mmc_switch(struct mmc_card *card, u8 set, u8 index,
>                  unsigned int timeout_ms, u8 wait_for_prog_done)
> {
> ...
>     if(wait_for_prog_done)
>       cmd.flags |= MMC_RSP_SPI_R1B | MMC_RSP_R1B;
>     else
>       cmd.flags |= MMC_RSP_SPI_R1 | MMC_RSP_R1;
> ...
> }
> 
> 2. implement mmc_switch through __mmc_switch()
>    int mmc_switch(struct mmc_card *card, u8 set, u8 index,
>                                    unsigned int timeout_ms)
>    {
>      return __mmc_switch(card, set, index, timeout_ms, 0);
>    }
> 3. when you need to start bkops, use: __mmc_switch(card, set, index,
> timeout_ms, 1);
> 
> Does it make sense?
> 

Hi Konstantin.

Your opinion is right. i will fix them.
Thanks for comments.

Best Regards,
Jaehoon Chung