2012-02-15 09:36:54

by Maya Erez

Subject: Re: [PATCH v4 2/2] mmc: core: Support packed command for eMMC4.5 device

> @@ -1065,12 +1075,60 @@ static int mmc_blk_err_check(struct mmc_card
> *card,
> if (!brq->data.bytes_xfered)
> return MMC_BLK_RETRY;
>
> + if (mq_mrq->packed_cmd != MMC_PACKED_NONE) {
> + if (unlikely(brq->data.blocks << 9 != brq->data.bytes_xfered))
> + return MMC_BLK_PARTIAL;
> + else
> + return MMC_BLK_SUCCESS;
> + }
> +
> if (blk_rq_bytes(req) != brq->data.bytes_xfered)
> return MMC_BLK_PARTIAL;
>
> return MMC_BLK_SUCCESS;
> }
I think it would be best to keep the request length (brq->data.blocks << 9
or blk_rq_bytes(req)) in a variable and use it in the original if above.
This way you can avoid doubling the exit points from the function.
Thanks,
Maya Erez
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2012-02-16 00:09:50

by Seungwon Jeon

Subject: RE: [PATCH v4 2/2] mmc: core: Support packed command for eMMC4.5 device

Maya Erez <[email protected]> wrote:
> > @@ -1065,12 +1075,60 @@ static int mmc_blk_err_check(struct mmc_card
> > *card,
> > if (!brq->data.bytes_xfered)
> > return MMC_BLK_RETRY;
> >
> > + if (mq_mrq->packed_cmd != MMC_PACKED_NONE) {
> > + if (unlikely(brq->data.blocks << 9 != brq->data.bytes_xfered))
> > + return MMC_BLK_PARTIAL;
> > + else
> > + return MMC_BLK_SUCCESS;
> > + }
> > +
> > if (blk_rq_bytes(req) != brq->data.bytes_xfered)
> > return MMC_BLK_PARTIAL;
> >
> > return MMC_BLK_SUCCESS;
> > }
> I think it would be best to keep the request length (brq->data.blocks << 9
> or blk_rq_bytes(req)) in a variable and use it in the original if above.
> This way you can avoid doubling the exit points from the function.
In case of packed command, brq->data.blocks contains the sum of blocks for individual request.
That means blk_rq_bytes(req) doesn't represent all packed length bytes.
And "brq->data.blocks << 9" is not idential to blk_rq_bytes(req) in non-packed command.
"brq->data.blocks" is overwritten during request preparation by the following reasons.
- brq->data.blocks > card->host->max_blk_count
- disable_multi
- do_rel_wr
So "brq->data.blocks << 9" is not good choice for request length in non-packed command.

Thanks,
Seungwon Jeon

> Thanks,
> Maya Erez
> Consultant for Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html