* [PATCH v1 2/2] mmc: cavium: Remove redundant if-statement checkup [not found] <20210319121357.255176-1-huobean@gmail.com> @ 2021-03-19 12:13 ` Bean Huo 2021-03-19 14:09 ` Ulf Hansson 0 siblings, 1 reply; 5+ messages in thread From: Bean Huo @ 2021-03-19 12:13 UTC (permalink / raw) To: rric, ulf.hansson, linus.walleij, linux-mmc, linux-kernel; +Cc: beanhuo, stable From: Bean Huo <beanhuo@micron.com> Currently, we have two ways to issue multiple-block read/write the command to the eMMC. One is by normal IO request path fs->block->mmc. Another one is that we can issue multiple-block read/write through MMC ioctl interface. For the first path, mrq->stop, and mrq->stop->opcode will be initialized in mmc_blk_data_prep(). However, for the second IO path, mrq->stop is not initialized since it is a pre-defined multiple blocks read/write. Meanwhile, if it is open-ended multiple block read/write command, STOP_TRANSMISSION CMD12 will be issued later in mmc_blk_issue_drv_op(), since it is MMC_IOC_MULTI_CMD. So, delete these if-statement checkups, let these kinds of multiple-block read/write request go. Fixes 'ba3869ff32e4 ("mmc: cavium: Add core MMC driver for Cavium SOCs")' Cc: stable@vger.kernel.org Signed-off-by: Bean Huo <beanhuo@micron.com> --- drivers/mmc/host/cavium.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/mmc/host/cavium.c b/drivers/mmc/host/cavium.c index 95a41983c6c0..8fb7cbcf62ad 100644 --- a/drivers/mmc/host/cavium.c +++ b/drivers/mmc/host/cavium.c @@ -654,8 +654,7 @@ static void cvm_mmc_dma_request(struct mmc_host *mmc, struct mmc_data *data; u64 emm_dma, addr; - if (!mrq->data || !mrq->data->sg || !mrq->data->sg_len || - !mrq->stop || mrq->stop->opcode != MMC_STOP_TRANSMISSION) { + if (!mrq->data || !mrq->data->sg || !mrq->data->sg_len) { dev_err(&mmc->card->dev, "Error: %s no data\n", __func__); goto error; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1 2/2] mmc: cavium: Remove redundant if-statement checkup 2021-03-19 12:13 ` [PATCH v1 2/2] mmc: cavium: Remove redundant if-statement checkup Bean Huo @ 2021-03-19 14:09 ` Ulf Hansson 2021-03-19 15:42 ` Bean Huo 0 siblings, 1 reply; 5+ messages in thread From: Ulf Hansson @ 2021-03-19 14:09 UTC (permalink / raw) To: Bean Huo Cc: rric, Linus Walleij, linux-mmc, Linux Kernel Mailing List, Bean Huo (beanhuo), # 4.0+ On Fri, 19 Mar 2021 at 13:14, Bean Huo <huobean@gmail.com> wrote: > > From: Bean Huo <beanhuo@micron.com> > > Currently, we have two ways to issue multiple-block read/write the > command to the eMMC. One is by normal IO request path fs->block->mmc. > Another one is that we can issue multiple-block read/write through > MMC ioctl interface. For the first path, mrq->stop, and mrq->stop->opcode > will be initialized in mmc_blk_data_prep(). However, for the second IO > path, mrq->stop is not initialized since it is a pre-defined multiple > blocks read/write. As a matter of fact this way is also supported for the regular block I/O path. To make the mmc block driver to use it, mmc host drivers need to announce that it's supported by setting MMC_CAP_CMD23. It looks like that is what your patch should be targeted towards, can you have a look at this instead? Kind regards Uffe > > Meanwhile, if it is open-ended multiple block read/write command, > STOP_TRANSMISSION CMD12 will be issued later in mmc_blk_issue_drv_op(), > since it is MMC_IOC_MULTI_CMD. > > So, delete these if-statement checkups, let these kinds of multiple-block > read/write request go. > > Fixes 'ba3869ff32e4 ("mmc: cavium: Add core MMC driver for Cavium SOCs")' > Cc: stable@vger.kernel.org > Signed-off-by: Bean Huo <beanhuo@micron.com> > --- > drivers/mmc/host/cavium.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/cavium.c b/drivers/mmc/host/cavium.c > index 95a41983c6c0..8fb7cbcf62ad 100644 > --- a/drivers/mmc/host/cavium.c > +++ b/drivers/mmc/host/cavium.c > @@ -654,8 +654,7 @@ static void cvm_mmc_dma_request(struct mmc_host *mmc, > struct mmc_data *data; > u64 emm_dma, addr; > > - if (!mrq->data || !mrq->data->sg || !mrq->data->sg_len || > - !mrq->stop || mrq->stop->opcode != MMC_STOP_TRANSMISSION) { > + if (!mrq->data || !mrq->data->sg || !mrq->data->sg_len) { > dev_err(&mmc->card->dev, "Error: %s no data\n", __func__); > goto error; > } > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 2/2] mmc: cavium: Remove redundant if-statement checkup 2021-03-19 14:09 ` Ulf Hansson @ 2021-03-19 15:42 ` Bean Huo 2021-04-29 20:30 ` Bean Huo 0 siblings, 1 reply; 5+ messages in thread From: Bean Huo @ 2021-03-19 15:42 UTC (permalink / raw) To: Ulf Hansson Cc: rric, Linus Walleij, linux-mmc, Linux Kernel Mailing List, Bean Huo (beanhuo), # 4.0+ On Fri, 2021-03-19 at 15:09 +0100, Ulf Hansson wrote: > On Fri, 19 Mar 2021 at 13:14, Bean Huo <huobean@gmail.com> wrote: > > > From: Bean Huo <beanhuo@micron.com> > > Currently, we have two ways to issue multiple-block read/write the > > command to the eMMC. One is by normal IO request path fs->block- > > >mmc. > > Another one is that we can issue multiple-block read/write through > > MMC ioctl interface. For the first path, mrq->stop, and mrq->stop- > > >opcode > > will be initialized in mmc_blk_data_prep(). However, for the second > > IO > > path, mrq->stop is not initialized since it is a pre-defined > > multiple > > blocks read/write. > > > As a matter of fact this way is also supported for the regular block > > I/O path. To make the mmc block driver to use it, mmc host drivers > > need to announce that it's supported by setting MMC_CAP_CMD23. > > > > It looks like that is what your patch should be targeted towards, can > > you have a look at this instead? > > Hi Ulf, Thanks for your comments. I will look at that as your suggestion. The patch [1/2] is accepted, so I will just update this patch to the next version. Kind regards, Bean > > Kind regards > > Uffe ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 2/2] mmc: cavium: Remove redundant if-statement checkup 2021-03-19 15:42 ` Bean Huo @ 2021-04-29 20:30 ` Bean Huo 2021-05-10 14:01 ` Ulf Hansson 0 siblings, 1 reply; 5+ messages in thread From: Bean Huo @ 2021-04-29 20:30 UTC (permalink / raw) To: Ulf Hansson Cc: rric, Linus Walleij, linux-mmc, Linux Kernel Mailing List, Bean Huo (beanhuo), # 4.0+ On Fri, 2021-03-19 at 16:42 +0100, Bean Huo wrote: > On Fri, 2021-03-19 at 15:09 +0100, Ulf Hansson wrote: > > > On Fri, 19 Mar 2021 at 13:14, Bean Huo <huobean@gmail.com> wrote: > > > From: Bean Huo <beanhuo@micron.com> > > > Currently, we have two ways to issue multiple-block read/write > > > the > > > command to the eMMC. One is by normal IO request path fs->block- > > > > mmc. > > > Another one is that we can issue multiple-block read/write > > > through > > > MMC ioctl interface. For the first path, mrq->stop, and mrq- > > > >stop- > > > > opcode > > > will be initialized in mmc_blk_data_prep(). However, for the > > > second > > > IO > > > path, mrq->stop is not initialized since it is a pre-defined > > > multiple > > > blocks read/write. > > As a matter of fact this way is also supported for the regular > > block > > I/O path. To make the mmc block driver to use it, mmc host drivers > > need to announce that it's supported by setting MMC_CAP_CMD23. > > It looks like that is what your patch should be targeted towards, > > can > > you have a look at this instead? > > > Hi Ulf, > > Thanks for your comments. I will look at that as your suggestion. > > The patch [1/2] is accepted, so I will just update this patch to > > the next version. > > > > Kind regards, > > Bean Hi Uffe, Could you please firstly accept this patch? let the customer update their kernel. As I tried to develop the next version of the patch according to your suggestion, more changes will be involved. Also, no matter how to make the change general, below mrq->stop checkup should be deleted since it is obsolete. In the data transmission completion interrupt, mrq->stop will be checked again. - if (!mrq->data || !mrq->data->sg || !mrq->data->sg_len || - !mrq->stop || mrq->stop->opcode != MMC_STOP_TRANSMISSION) { + if (!mrq->data || !mrq->data->sg || !mrq->data->sg_len) { Kind regards Bean ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 2/2] mmc: cavium: Remove redundant if-statement checkup 2021-04-29 20:30 ` Bean Huo @ 2021-05-10 14:01 ` Ulf Hansson 0 siblings, 0 replies; 5+ messages in thread From: Ulf Hansson @ 2021-05-10 14:01 UTC (permalink / raw) To: Bean Huo Cc: rric, Linus Walleij, linux-mmc, Linux Kernel Mailing List, Bean Huo (beanhuo), # 4.0+ On Thu, 29 Apr 2021 at 22:30, Bean Huo <huobean@gmail.com> wrote: > > On Fri, 2021-03-19 at 16:42 +0100, Bean Huo wrote: > > On Fri, 2021-03-19 at 15:09 +0100, Ulf Hansson wrote: > > > > > On Fri, 19 Mar 2021 at 13:14, Bean Huo <huobean@gmail.com> wrote: > > > > From: Bean Huo <beanhuo@micron.com> > > > > Currently, we have two ways to issue multiple-block read/write > > > > the > > > > command to the eMMC. One is by normal IO request path fs->block- > > > > > mmc. > > > > Another one is that we can issue multiple-block read/write > > > > through > > > > MMC ioctl interface. For the first path, mrq->stop, and mrq- > > > > >stop- > > > > > opcode > > > > will be initialized in mmc_blk_data_prep(). However, for the > > > > second > > > > IO > > > > path, mrq->stop is not initialized since it is a pre-defined > > > > multiple > > > > blocks read/write. > > > As a matter of fact this way is also supported for the regular > > > block > > > I/O path. To make the mmc block driver to use it, mmc host drivers > > > need to announce that it's supported by setting MMC_CAP_CMD23. > > > It looks like that is what your patch should be targeted towards, > > > can > > > you have a look at this instead? > > > > > > Hi Ulf, > > > > Thanks for your comments. I will look at that as your suggestion. > > > > The patch [1/2] is accepted, so I will just update this patch to > > > > the next version. > > > > > > > > Kind regards, > > > > Bean > > > Hi Uffe, > Could you please firstly accept this patch? let the customer update > their kernel. As I tried to develop the next version of the patch > according to your suggestion, more changes will be involved. Also, no > matter how to make the change general, below mrq->stop checkup should > be deleted since it is obsolete. In the data transmission completion > interrupt, mrq->stop will be checked again. > > - if (!mrq->data || !mrq->data->sg || !mrq->data->sg_len || > - !mrq->stop || mrq->stop->opcode != MMC_STOP_TRANSMISSION) { > + if (!mrq->data || !mrq->data->sg || !mrq->data->sg_len) { > Well, I don't think the above checks are incorrect. Instead I think it points out that the cavium mmc driver lacks support for CMD23, while only open ended data transfers are supported. The proper way forward is instead to implement CMD23 support to the cavium mmc driver. In the same patch adding that support, you should be able to remove the above checks. Kind regards Uffe ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-10 14:44 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20210319121357.255176-1-huobean@gmail.com> 2021-03-19 12:13 ` [PATCH v1 2/2] mmc: cavium: Remove redundant if-statement checkup Bean Huo 2021-03-19 14:09 ` Ulf Hansson 2021-03-19 15:42 ` Bean Huo 2021-04-29 20:30 ` Bean Huo 2021-05-10 14:01 ` Ulf Hansson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).