Re: [RESEND PATCH v10] mmc: support BKOPS feature for eMMC

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

Hi Maya,

On 07/20/2012 08:58 PM, merez@codeaurora.org wrote:
> 
> On Thu, July 19, 2012 8:12 am, Jaehoon Chung wrote:
>> +void mmc_start_bkops(struct mmc_card *card, bool from_exception)
>> +{
>> +        int err;
>> +        int timeout;
>> +        bool use_busy_signal;
>> +
>> +        BUG_ON(!card);
>> +
>> +        if (!card->ext_csd.bkops_en || mmc_card_doing_bkops(card) ||
>> +                !(card->host->caps2 & MMC_CAP2_BKOPS))
>> +                return;
> To my opinion, the host cannot decide not to support BKOPs if it is
> enabled by the card. In such a case, the card will expect the host to
> start the BKOPs and may get into performance degradation.
> I think the MMC_CAP2_BKOPS should be removed.
> Does everyone agree or do I miss something?
Ok..i will remove capability. I saw that other people agreed your opinion.
> 
>> @@ -489,6 +568,53 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct
>> mmc_command *cmd, int retries
>>  EXPORT_SYMBOL(mmc_wait_for_cmd);
>>
>>  /**
>> + *	mmc_stop_bkops - stop ongoing BKOPS
>> + *	@card: MMC card to check BKOPS
>> + *
>> + *	Send HPI command to stop ongoing background operations,
>> + *	to allow rapid servicing of foreground operations,e.g. read/
>> + *	writes. Wait until the card comes out of the programming state
>> + *	to avoid errors in servicing read/write requests.
>> + */
>> +int mmc_stop_bkops(struct mmc_card *card)
>> +{
>> +	int err = 0;
>> +
>> +	BUG_ON(!card);
>> +	err = mmc_interrupt_hpi(card);
>> +
>> +	/*
>> +	 * if err is EINVAL, it's status that can't issue HPI.
>> +	 * it should complete the BKOPS.
>> +	 */
>> +	if (!err || (err == -EINVAL)) {
>> +		mmc_card_clr_doing_bkops(card);
>> +		err = 0;
>> +	}
> I don't understand the comment regarding the case where err equals
> -EINVAL. Can you please explain it?
When err is -EINVAL, already completed bkops.
After completed bkops, should be the trans status.
If my understanding is wrong, let me know.
> 
>> @@ -392,13 +393,25 @@ 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 (use_busy_signal)
>> +                cmd.flags |= MMC_RSP_SPI_R1B | MMC_RSP_R1B;
>> +        else
>> +                cmd.flags |= MMC_RSP_SPI_R1 | MMC_RSP_R1;
>> +
>> +
>>          cmd.cmd_timeout_ms = timeout_ms;
>>
>>          err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
>>          if (err)
>>                  return err;
>>
>> +        /*
>> +         * No need to check card status in case of BKOPS LEVEL1 switch
>> +         */
> 
> Change the comment to be general (don't mention BKOPs level 1), something
> like:
> No need to check card status in case of unblocking command.
> Also, 1 line comment should be /* comment */
Will fix.

Any other comment? if you have other comment, let me know.
I will send the patch after fixing on today.

Best Regards,
Jaehoon Chung
> 
> Thanks,
> Maya
>