git: 0f8a8416a4e0 - main - dwmmc: cleanup cmd and locking, consistency between mmc and mmccam
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 31 Jul 2025 17:39:02 UTC
The branch main has been updated by bz:
URL: https://cgit.FreeBSD.org/src/commit/?id=0f8a8416a4e07ddeaecc4eafd6f418da0f21efc7
commit 0f8a8416a4e07ddeaecc4eafd6f418da0f21efc7
Author: Bjoern A. Zeeb <bz@FreeBSD.org>
AuthorDate: 2025-07-29 12:36:26 +0000
Commit: Bjoern A. Zeeb <bz@FreeBSD.org>
CommitDate: 2025-07-31 17:37:54 +0000
dwmmc: cleanup cmd and locking, consistency between mmc and mmccam
In general sprinkle locking assertions and harmonized KASSERTs
throughout the upper part of the driver to document expectations.
In dwmmc_cmd_done() "cmd" should be set correctly and be used for
both MMCCAM and classic mmc rather than special-casing mmccam.
In dwmmc_next_operation() place variable declarations on the top
for both cases before the first debug and lock assertion calls;
then factor out common parts at the end and put both cases in the
same order.
By calling dwmmc_next_operation() directly from both dwmmc_request()
in the mmc case, and dwmmc_cam_request() in the mmccase (rather than)
chaining calls in the latter, we avoid unlocking the sc in the mmccam
case and have a consistent call path from both; also removing the
mmccam #ifdef from dwmmc_request() brings more clarity.
In dwmmc_next_operation() enhance the panic/error messages with
some extra information and assert that we come in with a cam pinfo
on CAM_ACTIVE_INDEX.
MFC after: 3 days
Reviewed by: manu
Differential Revision: https://reviews.freebsd.org/D51628
---
sys/dev/mmc/host/dwmmc.c | 83 +++++++++++++++++++++++++-----------------------
1 file changed, 44 insertions(+), 39 deletions(-)
diff --git a/sys/dev/mmc/host/dwmmc.c b/sys/dev/mmc/host/dwmmc.c
index 57992571982c..a422d86d6034 100644
--- a/sys/dev/mmc/host/dwmmc.c
+++ b/sys/dev/mmc/host/dwmmc.c
@@ -315,20 +315,11 @@ static void
dwmmc_cmd_done(struct dwmmc_softc *sc)
{
struct mmc_command *cmd;
-#ifdef MMCCAM
- union ccb *ccb;
-#endif
-#ifdef MMCCAM
- ccb = sc->ccb;
- if (ccb == NULL)
- return;
- cmd = &ccb->mmcio.cmd;
-#else
+ DWMMC_ASSERT_LOCKED(sc);
+
cmd = sc->curcmd;
-#endif
- if (cmd == NULL)
- return;
+ KASSERT(cmd != NULL, ("%s: sc %p curcmd %p == NULL", __func__, sc, cmd));
if (cmd->flags & MMC_RSP_PRESENT) {
if (cmd->flags & MMC_RSP_136) {
@@ -350,15 +341,17 @@ dwmmc_tasklet(struct dwmmc_softc *sc)
{
struct mmc_command *cmd;
+ DWMMC_ASSERT_LOCKED(sc);
+
cmd = sc->curcmd;
- if (cmd == NULL)
- return;
+ KASSERT(cmd != NULL, ("%s: sc %p curcmd %p == NULL", __func__, sc, cmd));
if (!sc->cmd_done)
return;
if (cmd->error != MMC_ERR_NONE || !cmd->data) {
dwmmc_next_operation(sc);
+
} else if (cmd->data && sc->dto_rcvd) {
if ((cmd->opcode == MMC_WRITE_MULTIPLE_BLOCK ||
cmd->opcode == MMC_READ_MULTIPLE_BLOCK) &&
@@ -383,6 +376,7 @@ dwmmc_intr(void *arg)
DWMMC_LOCK(sc);
cmd = sc->curcmd;
+ KASSERT(cmd != NULL, ("%s: sc %p curcmd %p == NULL", __func__, sc, cmd));
/* First handle SDMMC controller interrupts */
reg = READ4(sc, SDMMC_MINTSTS);
@@ -1093,6 +1087,9 @@ dwmmc_start_cmd(struct dwmmc_softc *sc, struct mmc_command *cmd)
uint32_t cmdr;
dprintf("%s\n", __func__);
+
+ DWMMC_ASSERT_LOCKED(sc);
+
sc->curcmd = cmd;
data = cmd->data;
@@ -1177,18 +1174,22 @@ dwmmc_start_cmd(struct dwmmc_softc *sc, struct mmc_command *cmd)
static void
dwmmc_next_operation(struct dwmmc_softc *sc)
{
- struct mmc_command *cmd;
- dprintf("%s\n", __func__);
#ifdef MMCCAM
union ccb *ccb;
+#else
+ struct mmc_request *req;
+#endif
+ struct mmc_command *cmd;
+ dprintf("%s\n", __func__);
+ DWMMC_ASSERT_LOCKED(sc);
+
+#ifdef MMCCAM
ccb = sc->ccb;
if (ccb == NULL)
return;
cmd = &ccb->mmcio.cmd;
#else
- struct mmc_request *req;
-
req = sc->req;
if (req == NULL)
return;
@@ -1205,7 +1206,7 @@ dwmmc_next_operation(struct dwmmc_softc *sc)
* mostly caused by multi-block write command
* followed by single-read.
*/
- while(READ4(sc, SDMMC_STATUS) & (SDMMC_STATUS_DATA_BUSY))
+ while (READ4(sc, SDMMC_STATUS) & (SDMMC_STATUS_DATA_BUSY))
continue;
if (sc->flags & PENDING_CMD) {
@@ -1219,50 +1220,44 @@ dwmmc_next_operation(struct dwmmc_softc *sc)
return;
}
-#ifdef MMCCAM
- sc->ccb = NULL;
sc->curcmd = NULL;
+#ifdef MMCCAM
ccb->ccb_h.status =
(ccb->mmcio.cmd.error == 0 ? CAM_REQ_CMP : CAM_REQ_CMP_ERR);
xpt_done(ccb);
+ sc->ccb = NULL;
#else
- sc->req = NULL;
- sc->curcmd = NULL;
req->done(req);
+ sc->req = NULL;
#endif
}
+#ifndef MMCCAM
static int
dwmmc_request(device_t brdev, device_t reqdev, struct mmc_request *req)
{
struct dwmmc_softc *sc;
- sc = device_get_softc(brdev);
-
dprintf("%s\n", __func__);
- DWMMC_LOCK(sc);
+ sc = device_get_softc(brdev);
-#ifdef MMCCAM
- sc->flags |= PENDING_CMD;
-#else
+ DWMMC_LOCK(sc);
if (sc->req != NULL) {
DWMMC_UNLOCK(sc);
return (EBUSY);
}
-
sc->req = req;
sc->flags |= PENDING_CMD;
if (sc->req->stop)
sc->flags |= PENDING_STOP;
-#endif
- dwmmc_next_operation(sc);
+ dwmmc_next_operation(sc);
DWMMC_UNLOCK(sc);
+
return (0);
}
-#ifndef MMCCAM
static int
dwmmc_get_ro(device_t brdev, device_t reqdev)
{
@@ -1505,10 +1500,15 @@ dwmmc_cam_request(device_t dev, union ccb *ccb)
struct ccb_mmcio *mmcio;
sc = device_get_softc(dev);
- mmcio = &ccb->mmcio;
-
DWMMC_LOCK(sc);
+ KASSERT(ccb->ccb_h.pinfo.index == CAM_ACTIVE_INDEX,
+ ("%s: ccb %p index %d != CAM_ACTIVE_INDEX: func=%#x %s status %#x\n",
+ __func__, ccb, ccb->ccb_h.pinfo.index, ccb->ccb_h.func_code,
+ xpt_action_name(ccb->ccb_h.func_code), ccb->ccb_h.status));
+
+ mmcio = &ccb->mmcio;
+
#ifdef DEBUG
if (__predict_false(bootverbose)) {
device_printf(sc->dev, "CMD%u arg %#x flags %#x dlen %u dflags %#x\n",
@@ -1519,16 +1519,21 @@ dwmmc_cam_request(device_t dev, union ccb *ccb)
#endif
if (mmcio->cmd.data != NULL) {
if (mmcio->cmd.data->len == 0 || mmcio->cmd.data->flags == 0)
- panic("data->len = %d, data->flags = %d -- something is b0rked",
- (int)mmcio->cmd.data->len, mmcio->cmd.data->flags);
+ panic("%s: data %p data->len = %d, data->flags = %d -- something is b0rked",
+ __func__, mmcio->cmd.data, (int)mmcio->cmd.data->len, mmcio->cmd.data->flags);
}
+
if (sc->ccb != NULL) {
- device_printf(sc->dev, "Controller still has an active command\n");
+ device_printf(sc->dev, "%s: Controller still has an active command: "
+ "sc->ccb %p new ccb %p\n", __func__, sc->ccb, ccb);
+ DWMMC_UNLOCK(sc);
return (EBUSY);
}
sc->ccb = ccb;
+ sc->flags |= PENDING_CMD;
+
+ dwmmc_next_operation(sc);
DWMMC_UNLOCK(sc);
- dwmmc_request(sc->dev, NULL, NULL);
return (0);
}