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

[merez@codeaurora.org]

Hi Jaehoon,

I don't have additional comments.

Thanks,
Maya
On Sun, July 22, 2012 6:28 pm, Jaehoon Chung wrote:
> 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
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Sent by consultant of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum