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

From: Seungwon Jeon <tgih.jun@samsung.com>
To: "'Namjae Jeon'" <linkinjeon@gmail.com>
Cc: "'Saugata Das'" <saugata.das@linaro.org>,
	linux-mmc@vger.kernel.org, "'Chris Ball'" <cjb@laptop.org>,
	linux-kernel@vger.kernel.org
Subject: RE: [PATCH v3 2/2] mmc: core: Support packed command for eMMC4.5 device
Date: Thu, 26 Jan 2012 15:05:03 +0900	[thread overview]
Message-ID: <000c01ccdbf0$71fe58b0$55fb0a10$%jun@samsung.com> (raw)
In-Reply-To: <CAKYAXd-pCS4kRsg0PJp98YRqz_P0CtTKWEWiQJarE_iHqiesYQ@mail.gmail.com>

Namjae Jeon <linkinjeon@gmail.com>:
> 2012/1/25 Namjae Jeon <linkinjeon@gmail.com>:
> >>> >> +
> >>> >> +static int mmc_blk_issue_packed_rd(struct mmc_queue *mq,
> >>> >> +               struct mmc_queue_req *mq_rq)
> >>> >> +{
> >>> >> +       struct mmc_blk_data *md = mq->data;
> >>> >> +       struct mmc_card *card = md->queue.card;
> >>> >> +       int status, ret = -EIO, retry = 2;
> > Hi. Seungwon.
> > First Thansk for your reply.
> > There is one more my review comment.
> > I think that EIO of ret is not needed. there is no case to be skipped
> > if/ese condition in do while loop.
Oh, no need practically.
Thank you for your inspection.

> 
> Hi. Seungwon.
> 
>   if ((!mmc_host_is_spi(card->host) && rq_data_dir(req) != READ) ||
> +                       (mq_mrq->packed_cmd == MMC_PACKED_WR_HDR)) {
> 
> it seems more better that upper if condition is changed like the below.
Do you have any reason for your comment?
Packed command(4.5 feature) is regardless of SPI.
SPI mode has been removed since version 4.3.

> 
> if (!mmc_host_is_spi(card->host) && ((rq_data_dir(req) != READ) ||
> +                       (mq_mrq->packed_cmd == MMC_PACKED_WR_HDR))) {
> 
> 
> +++ b/drivers/mmc/core/host.c
> @@ -346,6 +346,8 @@ struct mmc_host *mmc_alloc_host(int extra, struct
> device *dev)
>        host->max_blk_size = 512;
>        host->max_blk_count = PAGE_CACHE_SIZE / 512;
> 
> +       host->packed_min = 2;
> +
>        return host;
> 
> if packed_min is fixed value without increasing/decreasing,  it seems
> more better to use macro.
> Thanks.
packed_min is not fixed value.
Could you refer the following is changes log about v3?

Changes in v3:
	- Add a variable member in mmc_host for minimum number of packed entries.
	  This value can be overridden by host.

Best regards,
Seungwon Jeon.

> --
> 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