RE: [PATCH v6 2/3] mmc: core: Support packed write command for eMMC4.5device

[Seungwon Jeon]

Maya Erez <merez@xxxxxxxxxxxxxx> wrote:
> > Maya Erez <merez@xxxxxxxxxxxxxx> wrote:
> >> > @@ -1313,10 +1609,17 @@ static int mmc_blk_issue_rw_rq(struct
> >> mmc_queue
> >> *mq, struct request *rqc)
> >> >  			 * A block was successfully transferred.
> >> >  			 */
> >> >  			mmc_blk_reset_success(md, type);
> >> > -			spin_lock_irq(&md->lock);
> >> > -			ret = __blk_end_request(req, 0,
> >> > +
> >> > +			if (mq_rq->packed_cmd != MMC_PACKED_NONE) {
> >> > +				ret = mmc_blk_end_packed_req(mq, mq_rq);
> >> If a specific request in the packed request consistantly fails, there is
> >> nothing to stop us from sending the same packed request in an endless
> >> loop.
> > There is various error case. This patch reused the existing error
> > handling.
> > What is that case we need to consider?
> >
> > Best regards,
> > Seungwon Jeon
> 
> This is different from unpacked requests handling since in the packed err
> check function you don't always return the error returned from
> mmc_blk_err_check. In case the EXT_CSD_PACKED_INDEXED_ERROR is set you
> return MMC_BLK_PARTIAL which is handled differently in the
> mmc_blk_issue_rw_rd.
> In our tests we set to 1 the packed bit in CMD23 arg of the first req (in
> the header). As a result, mmc_blk_err_check returned MMC_BLK_CMD_ERR.
> However, mmc_blk_packed_err_check returned MMC_BLK_PARTIAL (since the card
> indicated the index of the first request as the failed request).
> mmc_blk_issue_rw_rd handles MMC_BLK_PARTIAL by sending the packed command
> from the failed index and on, but since the packed bit was still set, the
> same error occurred and was handled the same way and we ended up with an
> endless loop.
> I hope my description is clear, let me know if you have further questions.
I tested your test case equally.
Even though your test makes the header parameter incorrect artificially
and keeps going with wrong setting repeatedly, we need to assure that
the similar result can be found practically with normal running.
I'll test it heavily and check more.
And if you have more review about this version, please let me know.

Thanks for your review.
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 majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/