On Fri, Nov 11, 2011 at 7:24 AM, Jae hoon Chung <jh80.chung@xxxxxxxxx> wrote:
> Hi per.
>
> 2011/11/8 Per Forlin <per.lkml@xxxxxxxxx>:
>> Hi Jaehoon,
>>
>> On Tue, Nov 8, 2011 at 1:43 PM, Jae hoon Chung <jh80.chung@xxxxxxxxx> wrote:
>>> Hi Per.
>>>
>>> 2011/11/5 Per Forlin <per.lkml@xxxxxxxxx>:
>>>> Hi Jaehoon,
>>>>
>>>> On Wed, Nov 2, 2011 at 11:28 AM, Jaehoon Chung <jh80.chung@xxxxxxxxxxx> wrote:
>>>>> Enable eMMC background operations (BKOPS) feature.
>>>>>
>>>>> If URGENT_BKOPS is set after a response, note that BKOPS
>>>>> are required. After all I/O requests are finished, run
>>>>> BKOPS if required. Should read/write operations be requested
>>>>> during BKOPS, first issue HPI to interrupt the ongoing BKOPS
>>>>> and then service the request.
>>>>>
>>>>> If you want to enable this feature, set MMC_CAP2_BKOPS.
>>>>>
>>>>> Signed-off-by: Jaehoon Chung <jh80.chung@xxxxxxxxxxx>
>>>>> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
>>>>> CC: Hanumath Prasad <hanumath.prasad@xxxxxxxxxxxxxx>
>>>>>
>>>>> ---
>>>>> Changelog V2:
>>>>> - Use EXCEPTION_STATUS instead of URGENT_BKOPS
>>>>> - Add function to check Exception_status(for eMMC4.5)
>>>>> - remove unnecessary code.
>>>>>
>>>>> drivers/mmc/card/block.c | 9 +++++
>>>>> drivers/mmc/card/queue.c | 4 ++
>>>>> drivers/mmc/core/core.c | 87 ++++++++++++++++++++++++++++++++++++++++++++
>>>>> drivers/mmc/core/mmc.c | 9 ++++-
>>>>> drivers/mmc/core/mmc_ops.c | 4 ++
>>>>> include/linux/mmc/card.h | 12 ++++++
>>>>> include/linux/mmc/core.h | 3 ++
>>>>> include/linux/mmc/host.h | 1 +
>>>>> include/linux/mmc/mmc.h | 14 +++++++
>>>>> 9 files changed, 142 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>>> index a1cb21f..fbfb405 100644
>>>>> --- a/drivers/mmc/card/block.c
>>>>> +++ b/drivers/mmc/card/block.c
>>>>> @@ -1192,6 +1192,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
>>>>> case MMC_BLK_SUCCESS:
>>>>> case MMC_BLK_PARTIAL:
>>>>> /*
>>>>> + * Check BKOPS urgency from each R1 response
>>>>> + */
>>>>> + if (mmc_card_mmc(card) &&
>>>>> + (brq->cmd.resp[0] & R1_EXCEPTION_EVENT)) {
>>>>> + if (mmc_is_exception_event(card,
>>>>> + EXT_CSD_URGENT_BKOPS))
>>>>> + mmc_card_set_need_bkops(card);
>>>>> + }
>>>>> + /*
>>>>> * A block was successfully transferred.
>>>>> */
>>>>> mmc_blk_reset_success(md, type);
>>>>> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
>>>>> index dcad59c..20bb4a5 100644
>>>>> --- a/drivers/mmc/card/queue.c
>>>>> +++ b/drivers/mmc/card/queue.c
>>>>> @@ -61,6 +61,9 @@ static int mmc_queue_thread(void *d)
>>>>> spin_unlock_irq(q->queue_lock);
>>>>>
>>>>> if (req || mq->mqrq_prev->req) {
>>>>> + if (mmc_card_doing_bkops(mq->card))
>>>>> + mmc_interrupt_bkops(mq->card);
>>>>> +
>>>>> set_current_state(TASK_RUNNING);
>>>>> mq->issue_fn(mq, req);
>>>>> } else {
>>>>> @@ -68,6 +71,7 @@ static int mmc_queue_thread(void *d)
>>>>> set_current_state(TASK_RUNNING);
>>>>> break;
>>>>> }
>>>>> + mmc_start_bkops(mq->card);
>>>>> up(&mq->thread_sem);
>>>>> schedule();
>>>>> down(&mq->thread_sem);
>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>> index 5278ffb..41ea923 100644
>>>>> --- a/drivers/mmc/core/core.c
>>>>> +++ b/drivers/mmc/core/core.c
>>>>> @@ -238,6 +238,50 @@ mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
>>>>> host->ops->request(host, mrq);
>>>>> }
>>>>>
>>>>> +/**
>>>>> + * mmc_start_bkops - start BKOPS for supported cards
>>>>> + * @card: MMC card to start BKOPS
>>>>> + *
>>>>> + * Start background operations whenever requested.
>>>>> + * when the urgent BKOPS bit is set in a R1 command response
>>>>> + * then background operations should be started immediately.
>>>>> +*/
>>>>> +void mmc_start_bkops(struct mmc_card *card)
>>>>> +{
>>>>> + int err;
>>>>> + unsigned long flags;
>>>>> +
>>>>
>>>>> + card->host->caps2 |= MMC_CAP2_BKOPS;
>>>> I guess this should be set by the host driver?
>>> You're right...it's my mistake...i will remove this..
>>>>
>>>>> + if ((!card || !card->ext_csd.bkops_en) &&
>>>>> + !(card->host->caps2 & MMC_CAP2_BKOPS))
>>>> I don't really understand this. If card is NULL it will seg-fault.
>>>>
>>>> Do you mean:
>>>> if (!card)
>>>> return
>>>>
>>>> if (!card->ext_csd.bkops_en ||
>>>> !(card->host->caps2 & MMC_CAP2_BKOPS))
>>>> return
>>>>
>>>> Do you really need to check card != NULL? Consider BUG_ON
>>> Maybe if (!card && !card->ext_csd.bkops_en) is correctly...i think...
>> If card is NULL it will seg-fault.
>> Did you mean: if (card && !card->ext_csd.bkops_en)
>>
>> I would suggest.
>> # return if card is NULL
>> if (!card)
>> return
>>
>> # if card is set, check the values.
>> if (!card->ext_csd.bkops_en ||
>> !(card->host->caps2 & MMC_CAP2_BKOPS))
>> return
>>
> You're right...i will use BUGON()...and modify for ext_bkops_en checking
>>
>>>>
>>>>> + return;
>>>>> +
>>>>> + /*
>>>>> + * If card is already doing bkops or need for
>>>>> + * bkops flag is not set, then do nothing just
>>>>> + * return
>>>>> + */
>>>>> + if (mmc_card_doing_bkops(card)
>>>>> + || !mmc_card_need_bkops(card))
>>>>> + return;
>>>>> +
>>>>> + mmc_claim_host(card->host);
>>>>> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>>>> + EXT_CSD_BKOPS_START, 1, 0);
>>>>> + if (err) {
>>>>> + pr_warning("error %d starting bkops\n", err);
>>>>> + mmc_card_clr_need_bkops(card);
>>>>> + goto out;
>>>>> + }
>>>>> + spin_lock_irqsave(&card->host->lock, flags);
>>>>> + mmc_card_clr_need_bkops(card);
>>>>> + mmc_card_set_doing_bkops(card);
>>>>> + spin_unlock_irqrestore(&card->host->lock, flags);
>>>>> +out:
>>>>> + mmc_release_host(card->host);
>>>>> +}
>>>>> +EXPORT_SYMBOL(mmc_start_bkops);
>>>>> +
>>>>> static void mmc_wait_done(struct mmc_request *mrq)
>>>>> {
>>>>> complete(&mrq->completion);
>>>>> @@ -466,6 +510,49 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries
>>>>> EXPORT_SYMBOL(mmc_wait_for_cmd);
>>>>>
>>>>> /**
>>>>> + * mmc_interrupt_bkops - interrupt ongoing BKOPS
>>>>> + * @card: MMC card to check BKOPS
>>>>> + *
>>>>> + * Send HPI command to interrupt 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_interrupt_bkops(struct mmc_card *card)
>>>>> +{
>>>>> + int err = 0;
>>>>> + unsigned long flags;
>>>>> +
>>>>> + BUG_ON(!card);
>>>>> +
>>>>> + err = mmc_interrupt_hpi(card);
>>>>> +
>>>>> + spin_lock_irqsave(&card->host->lock, flags);
>>>>> + mmc_card_clr_doing_bkops(card);
>>>>> + spin_unlock_irqrestore(&card->host->lock, flags);
>>>>> +
>>>>> + return err;
>>>>> +}
>>>>> +EXPORT_SYMBOL(mmc_interrupt_bkops);
>>>>> +
>>>>> +int mmc_is_exception_event(struct mmc_card *card, unsigned int value)
>>>>> +{
>>>>> + int err;
>>>>> + u8 ext_csd[512];
>>>>> +
>>>>> + /* In eMMC 4.41, R1_EXCEPTION_EVENT is URGENT_BKOPS */
>>>>> + if (card->ext_csd.rev == 5)
>>>>> + return 1;
>>>>> +
>>>>> + err = mmc_send_ext_csd(card, ext_csd);
>>>>> + if (err)
>>>>> + return err;
>>>>> +
>>>>> + return (ext_csd[EXT_CSD_EXCEPTION_STATUS] & value) ? 1 : 0;
>>>>> +}
>>>>> +EXPORT_SYMBOL(mmc_is_exception_event);
>>>>> +
>>>>> +/**
>>>>> * mmc_set_data_timeout - set the timeout for a data command
>>>>> * @data: data phase for command
>>>>> * @card: the MMC card associated with the data transfer
>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>> index 3627044..af40fbb 100644
>>>>> --- a/drivers/mmc/core/mmc.c
>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>> @@ -448,6 +448,14 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd)
>>>>> }
>>>>>
>>>>> if (card->ext_csd.rev >= 5) {
>>>>> + /* check whether the eMMC card support BKOPS */
>>>>> + if (ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1) {
>>>>> + card->ext_csd.bkops = 1;
>>>>> + card->ext_csd.bkops_en = 1;
>>>> What if the card has not enabled bkops? This bit could be unset.
>>>> card->ext_csd.bkops_en = ext_csd[EXT_CSD_BKOPS_EN];
>>> When card is supported the bkops, bkops_en bit is set by default...
>>>
>> How do you know if the bkops_en is really enabled in th ext_csd?
>> It could be unset in the ext_csd, right?
>
> In emmc4.5 spec, if card is supported the bkops, bkops_en is set by default.
If bkops_en is set by default it should be in the the ext_csd too, right?
It should be safer to trust the value of ext_csd[EXT_CSD_BKOPS_EN] in
this case I think.
Regards,
Per
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
