mmc: Soft Command queue implementation for eMMC5.1 device

PATCH Superseded Chuanxiao Dong 2 participants 5 messages Mar 17, 2015 – Apr 7, 2015 MMC, SD and SDIO

lore

Share & tools

lore

Chuanxiao Dong

author

2015-03-17 11:11 UTC #0 lore raw

cover mmc: Soft Command queue implementation for eMMC5.1 device

Hello

This serial of patches can implement the eMMC5.1 device command queue feature even a host controller doesn't support CMDQ. Several weeks ago I submitted RFC patches for review and collected some review comments. Right now these comments are fixed thus I submit again as the offical patches. Please have a review.

Chuanxiao Dong (5):
mmc: replace sbc to precmd and add postcmd
mmc: host: add runtime PM for host class dev
mmc: queue: change mqrq_cur and mqrq_pre to mq qdepth
mmc: core: add support for CMDQ feature in MMC Core stack
mmc: sdhci: add SW CMDQ support for SDHCI host

 drivers/mmc/card/block.c      |  538 ++++++++++++++++++++++++++++++++++++++---
 drivers/mmc/card/queue.c      |  213 ++++++++--------
 drivers/mmc/card/queue.h      |   14 +-
 drivers/mmc/core/core.c       |   85 ++++++-
 drivers/mmc/core/host.c       |   13 +
 drivers/mmc/core/mmc.c        |   37 ++-
 drivers/mmc/host/dw_mmc.c     |    8 +-
 drivers/mmc/host/mmci.c       |   14 +-
 drivers/mmc/host/omap_hsmmc.c |   18 +-
 drivers/mmc/host/sdhci-acpi.c |    1 -
 drivers/mmc/host/sdhci-pci.c  |    1 -
 drivers/mmc/host/sdhci.c      |  137 +++++++++--
 drivers/mmc/host/sdhci.h      |    1 +
 include/linux/mmc/card.h      |    3 +
 include/linux/mmc/core.h      |    5 +-
 include/linux/mmc/host.h      |    5 +
 include/linux/mmc/mmc.h       |   21 ++
 include/linux/mmc/pm.h        |    1 +
 18 files changed, 911 insertions(+), 204 deletions(-)

Alex Lemberg

2015-04-02 08:57 UTC #1 lore raw

Hi Chuanxiao,

Quoted 8 lines

-----Original Message-----
From: [email protected] [mailto:linux-mmc- [email protected]] On Behalf Of Chuanxiao Dong
Sent: Tuesday, March 17, 2015 1:10 PM
To: [email protected]
Subject: [PATCH 0/5] mmc: Soft Command queue implementation for eMMC5.1 device

Quoted 7 lines

Hello
This serial of patches can implement the eMMC5.1 device command queue feature even a host controller doesn't support CMDQ. Several weeks ago I submitted RFC patches for review and collected some review comments. Right now these comments are fixed thus I submit again as the offical patches. Please have a review.

Thanks for sending the new patch.
We have reviewed and see issues still exists like in previous patch version. The driver flow where request with the same TID is executed right after completing its data transfer, is still possible.
We suspect that the new solution is not covering the case of driver thread context switch, Specifically the following condition in block.c:

+ if (host->areq == areq)
+ goto next;

Thanks,
Alex

Quoted 33 lines

Chuanxiao Dong (5):
mmc: replace sbc to precmd and add postcmd
mmc: host: add runtime PM for host class dev
mmc: queue: change mqrq_cur and mqrq_pre to mq qdepth
mmc: core: add support for CMDQ feature in MMC Core stack
mmc: sdhci: add SW CMDQ support for SDHCI host
 drivers/mmc/card/block.c      |  538
++++++++++++++++++++++++++++++++++++++---
 drivers/mmc/card/queue.c      |  213 ++++++++--------
 drivers/mmc/card/queue.h      |   14 +-
 drivers/mmc/core/core.c       |   85 ++++++-
 drivers/mmc/core/host.c       |   13 +
 drivers/mmc/core/mmc.c        |   37 ++-
 drivers/mmc/host/dw_mmc.c     |    8 +-
 drivers/mmc/host/mmci.c       |   14 +-
 drivers/mmc/host/omap_hsmmc.c |   18 +-
 drivers/mmc/host/sdhci-acpi.c |    1 -
 drivers/mmc/host/sdhci-pci.c  |    1 -
 drivers/mmc/host/sdhci.c      |  137 +++++++++--
 drivers/mmc/host/sdhci.h      |    1 +
 include/linux/mmc/card.h      |    3 +
 include/linux/mmc/core.h      |    5 +-
 include/linux/mmc/host.h      |    5 +
 include/linux/mmc/mmc.h       |   21 ++
 include/linux/mmc/pm.h        |    1 +
 18 files changed, 911 insertions(+), 204 deletions(-)
--
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

Dong, Chuanxiao

author

2015-04-02 10:52 UTC #2 lore raw

Thanks Alex for review.

Quoted 1 line

From my understanding, if host->areq == areq, then means the areq is the current running request. By this way, driver will clear the executable ready bit in slots and continue to find the next one. So the task with the same ID won't be executed at the same time.

+ do {
+ id = find_first_bit(&slots, mq->qdepth);
+ if (id < mq->qdepth) {
+ areq = &mq->mqrq[id].mmc_active;
+ if (host->areq == areq)
+ goto next;
.....
......
+next:
+ __clear_bit(id, &slots);
+ } while (status);

I also prepared patch set v1, which won't send CMD13 to poll cmdq ready during a cmdq request is executing by CMD46/47. Will send them out later.

Thanks
Chuanxiao

Quoted 74 lines

-----Original Message-----
From: Alex Lemberg [mailto:[email protected]]
Sent: Thursday, April 02, 2015 4:57 PM
To: Dong, Chuanxiao; [email protected]
Cc: Yuliy Izrailov
Subject: RE: [PATCH 0/5] mmc: Soft Command queue implementation for eMMC5.1 device
Hi Chuanxiao,
Quoted 8 lines
-----Original Message-----
From: [email protected] [mailto:linux-mmc- [email protected]] On Behalf Of Chuanxiao Dong
Sent: Tuesday, March 17, 2015 1:10 PM
To: [email protected]
Subject: [PATCH 0/5] mmc: Soft Command queue implementation for eMMC5.1 device
Quoted 7 lines
Hello
This serial of patches can implement the eMMC5.1 device command queue feature even a host controller doesn't support CMDQ. Several weeks ago I submitted RFC patches for review and collected some review comments. Right now these comments are fixed thus I submit again as the offical patches. Please have a review.
Thanks for sending the new patch.
We have reviewed and see issues still exists like in previous patch version. The driver flow where request with the same TID is executed right after completing its data transfer, is still possible.
We suspect that the new solution is not covering the case of driver thread context switch, Specifically the following condition in block.c:
(quoted diff) +2 -0
+			if (host->areq == areq)
+				goto next;
Thanks,
Alex
Quoted 33 lines
Chuanxiao Dong (5):
mmc: replace sbc to precmd and add postcmd
mmc: host: add runtime PM for host class dev
mmc: queue: change mqrq_cur and mqrq_pre to mq qdepth
mmc: core: add support for CMDQ feature in MMC Core stack
mmc: sdhci: add SW CMDQ support for SDHCI host
 drivers/mmc/card/block.c      |  538
++++++++++++++++++++++++++++++++++++++---
 drivers/mmc/card/queue.c      |  213 ++++++++--------
 drivers/mmc/card/queue.h      |   14 +-
 drivers/mmc/core/core.c       |   85 ++++++-
 drivers/mmc/core/host.c       |   13 +
 drivers/mmc/core/mmc.c        |   37 ++-
 drivers/mmc/host/dw_mmc.c     |    8 +-
 drivers/mmc/host/mmci.c       |   14 +-
 drivers/mmc/host/omap_hsmmc.c |   18 +-
 drivers/mmc/host/sdhci-acpi.c |    1 -
 drivers/mmc/host/sdhci-pci.c  |    1 -
 drivers/mmc/host/sdhci.c      |  137 +++++++++--
 drivers/mmc/host/sdhci.h      |    1 +
 include/linux/mmc/card.h      |    3 +
 include/linux/mmc/core.h      |    5 +-
 include/linux/mmc/host.h      |    5 +
 include/linux/mmc/mmc.h       |   21 ++
 include/linux/mmc/pm.h        |    1 +
 18 files changed, 911 insertions(+), 204 deletions(-)
--
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

Alex Lemberg

2015-04-06 13:48 UTC #3 lore raw

Hi Chuanxiao,

Quoted 8 lines

-----Original Message-----
From: Dong, Chuanxiao [mailto:[email protected]]
Sent: Thursday, April 02, 2015 1:52 PM
To: Alex Lemberg; [email protected]
Cc: Yuliy Izrailov
Subject: RE: [PATCH 0/5] mmc: Soft Command queue implementation for eMMC5.1 device

Quoted 6 lines

Thanks Alex for review.
From my understanding, if host->areq == areq, then means the areq is the current running request. By this way, driver will clear the executable ready bit in slots and continue to find the next one. So the task with the same ID won't be executed at the same time.

Right, but there is a case for context switch:

Review

Quoted 5 lines

(quoted diff) +4 -0

+	do {
+		id = find_first_bit(&slots, mq->qdepth);
+		if (id < mq->qdepth) {
+			areq = &mq->mqrq[id].mmc_active;

What if host->areq will be changed as the result of completion interrupt, in parallel or just before this if condition?

Quoted 89 lines

(quoted diff) +5 -0
+			if (host->areq == areq)
+				goto next;
.....
......
+next:
+		__clear_bit(id, &slots);
+	} while (status);
I also prepared patch set v1, which won't send CMD13 to poll cmdq ready during a cmdq request is executing by CMD46/47. Will send them out later.
Thanks
Chuanxiao
Quoted 8 lines
-----Original Message-----
From: Alex Lemberg [mailto:[email protected]]
Sent: Thursday, April 02, 2015 4:57 PM
To: Dong, Chuanxiao; [email protected]
Cc: Yuliy Izrailov
Subject: RE: [PATCH 0/5] mmc: Soft Command queue implementation for eMMC5.1 device
Quoted 16 lines
Hi Chuanxiao,
Quoted 13 lines
-----Original Message-----
From: [email protected] [mailto:linux-mmc- [email protected]] On Behalf Of Chuanxiao Dong
Sent: Tuesday, March 17, 2015 1:10 PM
To: [email protected]
Subject: [PATCH 0/5] mmc: Soft Command queue implementation for eMMC5.1 device
Hello
This serial of patches can implement the eMMC5.1 device command queue feature even a host controller doesn't support CMDQ. Several weeks ago I submitted RFC patches for review and collected some review
comments.
Quoted 50 lines
Quoted 2 lines
Right now these comments are fixed thus I submit again as the offical patches. Please have a review.
Thanks for sending the new patch.
We have reviewed and see issues still exists like in previous patch version. The driver flow where request with the same TID is executed right after completing its data transfer, is still possible.
We suspect that the new solution is not covering the case of driver thread context switch, Specifically the following condition in block.c:
(quoted diff) +2 -0
+			if (host->areq == areq)
+				goto next;
Thanks,
Alex
Quoted 33 lines
Chuanxiao Dong (5):
mmc: replace sbc to precmd and add postcmd
mmc: host: add runtime PM for host class dev
mmc: queue: change mqrq_cur and mqrq_pre to mq qdepth
mmc: core: add support for CMDQ feature in MMC Core stack
mmc: sdhci: add SW CMDQ support for SDHCI host
 drivers/mmc/card/block.c      |  538
++++++++++++++++++++++++++++++++++++++---
 drivers/mmc/card/queue.c      |  213 ++++++++--------
 drivers/mmc/card/queue.h      |   14 +-
 drivers/mmc/core/core.c       |   85 ++++++-
 drivers/mmc/core/host.c       |   13 +
 drivers/mmc/core/mmc.c        |   37 ++-
 drivers/mmc/host/dw_mmc.c     |    8 +-
 drivers/mmc/host/mmci.c       |   14 +-
 drivers/mmc/host/omap_hsmmc.c |   18 +-
 drivers/mmc/host/sdhci-acpi.c |    1 -
 drivers/mmc/host/sdhci-pci.c  |    1 -
 drivers/mmc/host/sdhci.c      |  137 +++++++++--
 drivers/mmc/host/sdhci.h      |    1 +
 include/linux/mmc/card.h      |    3 +
 include/linux/mmc/core.h      |    5 +-
 include/linux/mmc/host.h      |    5 +
 include/linux/mmc/mmc.h       |   21 ++
 include/linux/mmc/pm.h        |    1 +
 18 files changed, 911 insertions(+), 204 deletions(-)
--
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

Dong, Chuanxiao

author

2015-04-07 02:48 UTC #4 lore raw

Hi Alex,

Quoted 30 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Alex Lemberg
Sent: Monday, April 06, 2015 9:48 PM
To: Dong, Chuanxiao; [email protected]
Cc: Yuliy Izrailov
Subject: RE: [PATCH 0/5] mmc: Soft Command queue implementation for eMMC5.1 device
Hi Chuanxiao,
Quoted 8 lines
-----Original Message-----
From: Dong, Chuanxiao [mailto:[email protected]]
Sent: Thursday, April 02, 2015 1:52 PM
To: Alex Lemberg; [email protected]
Cc: Yuliy Izrailov
Subject: RE: [PATCH 0/5] mmc: Soft Command queue implementation for eMMC5.1 device
Quoted 6 lines
Thanks Alex for review.
From my understanding, if host->areq == areq, then means the areq is the current running request. By this way, driver will clear the executable ready bit in slots and continue to find the next one. So the task with the same ID won't be executed at the same time.
Right, but there is a case for context switch:
Quoted 1 line

Quoted 7 lines

Review
Quoted 4 lines
(quoted diff) +4 -0
+	do {
+		id = find_first_bit(&slots, mq->qdepth);
+		if (id < mq->qdepth) {
+			areq = &mq->mqrq[id].mmc_active;
What if host->areq will be changed as the result of completion interrupt, in parallel or just before this if condition?

Host->areq will be changed after this "if", in function "mmc_execute_cmdq".They are synchronize. Except "mmc_execute_cmdq", host->areq is changed nowhere.

Thanks
Chuanxiao

Quoted 96 lines

Quoted 11 lines
(quoted diff) +5 -0
+			if (host->areq == areq)
+				goto next;
.....
......
+next:
+		__clear_bit(id, &slots);
+	} while (status);
I also prepared patch set v1, which won't send CMD13 to poll cmdq ready during a cmdq request is executing by CMD46/47. Will send them out
later.
Quoted 23 lines
Thanks
Chuanxiao
Quoted 19 lines
-----Original Message-----
From: Alex Lemberg [mailto:[email protected]]
Sent: Thursday, April 02, 2015 4:57 PM
To: Dong, Chuanxiao; [email protected]
Cc: Yuliy Izrailov
Subject: RE: [PATCH 0/5] mmc: Soft Command queue implementation for eMMC5.1 device
Hi Chuanxiao,
Quoted 8 lines
-----Original Message-----
From: [email protected] [mailto:linux-mmc- [email protected]] On Behalf Of Chuanxiao Dong
Sent: Tuesday, March 17, 2015 1:10 PM
To: [email protected]
Subject: [PATCH 0/5] mmc: Soft Command queue implementation for eMMC5.1 device
Quoted 57 lines
Quoted 6 lines
Quoted 6 lines
Hello
This serial of patches can implement the eMMC5.1 device command queue feature even a host controller doesn't support CMDQ. Several weeks ago I submitted RFC patches for review and collected some review
comments.
Quoted 50 lines
Quoted 2 lines
Right now these comments are fixed thus I submit again as the offical patches. Please have a review.
Thanks for sending the new patch.
We have reviewed and see issues still exists like in previous patch version. The driver flow where request with the same TID is executed right after completing its data transfer, is still possible.
We suspect that the new solution is not covering the case of driver thread context switch, Specifically the following condition in block.c:
(quoted diff) +2 -0
+			if (host->areq == areq)
+				goto next;
Thanks,
Alex
Quoted 33 lines
Chuanxiao Dong (5):
mmc: replace sbc to precmd and add postcmd
mmc: host: add runtime PM for host class dev
mmc: queue: change mqrq_cur and mqrq_pre to mq qdepth
mmc: core: add support for CMDQ feature in MMC Core stack
mmc: sdhci: add SW CMDQ support for SDHCI host
 drivers/mmc/card/block.c      |  538
++++++++++++++++++++++++++++++++++++++---
 drivers/mmc/card/queue.c      |  213 ++++++++--------
 drivers/mmc/card/queue.h      |   14 +-
 drivers/mmc/core/core.c       |   85 ++++++-
 drivers/mmc/core/host.c       |   13 +
 drivers/mmc/core/mmc.c        |   37 ++-
 drivers/mmc/host/dw_mmc.c     |    8 +-
 drivers/mmc/host/mmci.c       |   14 +-
 drivers/mmc/host/omap_hsmmc.c |   18 +-
 drivers/mmc/host/sdhci-acpi.c |    1 -
 drivers/mmc/host/sdhci-pci.c  |    1 -
 drivers/mmc/host/sdhci.c      |  137 +++++++++--
 drivers/mmc/host/sdhci.h      |    1 +
 include/linux/mmc/card.h      |    3 +
 include/linux/mmc/core.h      |    5 +-
 include/linux/mmc/host.h      |    5 +
 include/linux/mmc/mmc.h       |   21 ++
 include/linux/mmc/pm.h        |    1 +
 18 files changed, 911 insertions(+), 204 deletions(-)
--
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
--
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