| Message ID | cover.1584428736.git.baolin.wang7@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Introduce the request_atomic() for the host | expand |
On 17/03/20 12:14 pm, Baolin Wang wrote: > This patch set introduces a new request_atomic() interface for the > MMC host controller, which is used to submit a request to host in > the atomic context, such as in the irq hard handler, to reduce the > request latency. > > Any comments are welcome. Thanks. > > Note: Adrian pointed out that it is not good if moving the polling of > inhibit bits in sdhci_send_command() into the interrupt context, but > now I have not found a better way to address Adrian's concern. Moveover > this is an unusual abnormal case and the original code has the same > problem, so I plan to create another patch set to talk about and fix > this issue. I tend to think the API requires the possibility for host controllers to return "busy", so that should be sorted out first. > > Changes from v1: > - Re-split the changes to make them more clear suggested by Ulf. > - Factor out the auto CMD23 checking into a separate function. > > Baolin Wang (3): > mmc: host: Introduce the request_atomic() for the host > mmc: host: sdhci: Implement the request_atomic() API > mmc: host: sdhci-sprd: Implement the request_atomic() API > > drivers/mmc/host/mmc_hsq.c | 5 ++++- > drivers/mmc/host/sdhci-sprd.c | 23 ++++++++++++++++++++--- > drivers/mmc/host/sdhci.c | 27 +++++++++++++++++++-------- > drivers/mmc/host/sdhci.h | 1 + > include/linux/mmc/host.h | 3 +++ > 5 files changed, 47 insertions(+), 12 deletions(-) >
On Tue, Mar 17, 2020 at 9:25 PM Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 17/03/20 12:14 pm, Baolin Wang wrote: > > This patch set introduces a new request_atomic() interface for the > > MMC host controller, which is used to submit a request to host in > > the atomic context, such as in the irq hard handler, to reduce the > > request latency. > > > > Any comments are welcome. Thanks. > > > > Note: Adrian pointed out that it is not good if moving the polling of > > inhibit bits in sdhci_send_command() into the interrupt context, but > > now I have not found a better way to address Adrian's concern. Moveover > > this is an unusual abnormal case and the original code has the same > > problem, so I plan to create another patch set to talk about and fix > > this issue. > > I tend to think the API requires the possibility for host controllers to > return "busy", so that should be sorted out first. If request_atomic() can return 'busy', the HSQ need queue a work to dispatch this request to host again? I am thinking if I can introduce a new flag to avoid polling the status before sending commands, cause from the datasheet, I did not see we should need do this if the command complete and transfer complete interrupts are processed normally. At least on my platfrom, I did not see the inhibit bits are set. If we meet this issue, I think some abormal things are happened, we should give out errors. How do you think? > > > > Changes from v1: > > - Re-split the changes to make them more clear suggested by Ulf. > > - Factor out the auto CMD23 checking into a separate function. > > > > Baolin Wang (3): > > mmc: host: Introduce the request_atomic() for the host > > mmc: host: sdhci: Implement the request_atomic() API > > mmc: host: sdhci-sprd: Implement the request_atomic() API > > > > drivers/mmc/host/mmc_hsq.c | 5 ++++- > > drivers/mmc/host/sdhci-sprd.c | 23 ++++++++++++++++++++--- > > drivers/mmc/host/sdhci.c | 27 +++++++++++++++++++-------- > > drivers/mmc/host/sdhci.h | 1 + > > include/linux/mmc/host.h | 3 +++ > > 5 files changed, 47 insertions(+), 12 deletions(-) > > >
On 17/03/20 3:49 pm, Baolin Wang wrote: > On Tue, Mar 17, 2020 at 9:25 PM Adrian Hunter <adrian.hunter@intel.com> wrote: >> >> On 17/03/20 12:14 pm, Baolin Wang wrote: >>> This patch set introduces a new request_atomic() interface for the >>> MMC host controller, which is used to submit a request to host in >>> the atomic context, such as in the irq hard handler, to reduce the >>> request latency. >>> >>> Any comments are welcome. Thanks. >>> >>> Note: Adrian pointed out that it is not good if moving the polling of >>> inhibit bits in sdhci_send_command() into the interrupt context, but >>> now I have not found a better way to address Adrian's concern. Moveover >>> this is an unusual abnormal case and the original code has the same >>> problem, so I plan to create another patch set to talk about and fix >>> this issue. >> >> I tend to think the API requires the possibility for host controllers to >> return "busy", so that should be sorted out first. > > If request_atomic() can return 'busy', the HSQ need queue a work to > dispatch this request to host again? Sounds reasonable > > I am thinking if I can introduce a new flag to avoid polling the > status before sending commands, cause from the datasheet, I did not > see we should need do this if the command complete and transfer > complete interrupts are processed normally. At least on my platfrom, I > did not see the inhibit bits are set. If we meet this issue, I think > some abormal things are happened, we should give out errors. How do > you think? For the atomic path, some kind of warning would be ok. > >>> >>> Changes from v1: >>> - Re-split the changes to make them more clear suggested by Ulf. >>> - Factor out the auto CMD23 checking into a separate function. >>> >>> Baolin Wang (3): >>> mmc: host: Introduce the request_atomic() for the host >>> mmc: host: sdhci: Implement the request_atomic() API >>> mmc: host: sdhci-sprd: Implement the request_atomic() API >>> >>> drivers/mmc/host/mmc_hsq.c | 5 ++++- >>> drivers/mmc/host/sdhci-sprd.c | 23 ++++++++++++++++++++--- >>> drivers/mmc/host/sdhci.c | 27 +++++++++++++++++++-------- >>> drivers/mmc/host/sdhci.h | 1 + >>> include/linux/mmc/host.h | 3 +++ >>> 5 files changed, 47 insertions(+), 12 deletions(-) >>> >> > >
On Tue, Mar 17, 2020 at 11:07 PM Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 17/03/20 3:49 pm, Baolin Wang wrote: > > On Tue, Mar 17, 2020 at 9:25 PM Adrian Hunter <adrian.hunter@intel.com> wrote: > >> > >> On 17/03/20 12:14 pm, Baolin Wang wrote: > >>> This patch set introduces a new request_atomic() interface for the > >>> MMC host controller, which is used to submit a request to host in > >>> the atomic context, such as in the irq hard handler, to reduce the > >>> request latency. > >>> > >>> Any comments are welcome. Thanks. > >>> > >>> Note: Adrian pointed out that it is not good if moving the polling of > >>> inhibit bits in sdhci_send_command() into the interrupt context, but > >>> now I have not found a better way to address Adrian's concern. Moveover > >>> this is an unusual abnormal case and the original code has the same > >>> problem, so I plan to create another patch set to talk about and fix > >>> this issue. > >> > >> I tend to think the API requires the possibility for host controllers to > >> return "busy", so that should be sorted out first. > > > > If request_atomic() can return 'busy', the HSQ need queue a work to > > dispatch this request to host again? > > Sounds reasonable > > > > > I am thinking if I can introduce a new flag to avoid polling the > > status before sending commands, cause from the datasheet, I did not > > see we should need do this if the command complete and transfer > > complete interrupts are processed normally. At least on my platfrom, I > > did not see the inhibit bits are set. If we meet this issue, I think > > some abormal things are happened, we should give out errors. How do > > you think? > > For the atomic path, some kind of warning would be ok. OK. I will try in next version. Thanks. > > > >>> > >>> Changes from v1: > >>> - Re-split the changes to make them more clear suggested by Ulf. > >>> - Factor out the auto CMD23 checking into a separate function. > >>> > >>> Baolin Wang (3): > >>> mmc: host: Introduce the request_atomic() for the host > >>> mmc: host: sdhci: Implement the request_atomic() API > >>> mmc: host: sdhci-sprd: Implement the request_atomic() API > >>> > >>> drivers/mmc/host/mmc_hsq.c | 5 ++++- > >>> drivers/mmc/host/sdhci-sprd.c | 23 ++++++++++++++++++++--- > >>> drivers/mmc/host/sdhci.c | 27 +++++++++++++++++++-------- > >>> drivers/mmc/host/sdhci.h | 1 + > >>> include/linux/mmc/host.h | 3 +++ > >>> 5 files changed, 47 insertions(+), 12 deletions(-) > >>> > >> > > > > >