From nobody Thu Jul 31 17:39:02 2025 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4btGXl03Dhz62qdm; Thu, 31 Jul 2025 17:39:03 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4btGXk5xHtz3ZYh; Thu, 31 Jul 2025 17:39:02 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1753983542; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=ho7j54M9kGVdu3aQf+ttpXQpSRyMDoOQdsNHSQSw8e8=; b=moHUSjsc71/2+awn3iqppl+N26lIrK5Lx2oRN5KpXHBE8G3k7iiTmOFJnfD9Z7pGPESQEp h+4daA5UsO4ZZcCCv6d+3g2I1d8cL02ktHZbKvhIdxuYSbrh/gB4Ee1dIMPaZtVsrATKMp 03+xkmLq2SE73tsJsstT7EJeI0eEXrsj4cGxKidqUxfMki+Y/n+dRRNeXlAvO6yLrDhZL6 a8r5CGbXtXrg617msRaYFaUZqI/T6UUJZFOknjR24W0ezRj9iupCYHslnspdOBT/L8869g fmgmNzsJhZ1apTh54RHjM87rvIqtUgj9WuKgJkBdz9BALKMcxmFdBG1uIdi7gg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1753983542; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=ho7j54M9kGVdu3aQf+ttpXQpSRyMDoOQdsNHSQSw8e8=; b=h87XjLYZirg/KaTiyHhVz+FS/oWbl4nmx77qY/Wzax0mYD9uYf58ALyQh+jLeHF1Rd+evu i+fbv8L7ihh4+bQFurVkgJ6K8JtBkdcteX4l53i2ZuF4po2wnpkmEltWlr/zHHDqFoHRY3 /hPASFm6iF80tULrahUnTNjqvXaA+GOgoLa35HfpXUQRMnR32gQD4taQ0Fk6Oi06yxV01+ pVErSLtfM5GI0km8pxcz8PvJHZZV8v+slFOVWV+S+ZMWjSENyLaU3h1FXAQM3k04Nkou9I v0wGiB0cdDDI+KSSp5bBChe23AhQE8iRTl5l7uRDXnQFVUGXk6bZSh1gF6+9MA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1753983542; a=rsa-sha256; cv=none; b=gMz+0uyBdxYkMDVW1mqZm5Vps3KVpXgh0kX/y1Bmsbs2wZnjACU24E/3L34kwRI1Lo82B7 g5Hr4llX3zowJhT70A5T6FtOiP1hnOKC95dVZnquF9v1K3QXJX4KkR82EAQfA2qseljsGV +rVkq2kWnhxbam566dSnzloyav09fL6CrcCzxvcUzH0rG9S3zOa+lXolPZ1m2zVFtnqAGD WB3D6RHnO1VI+ecZTFeWiDubdbPrRBj55HN9yPo2XICx2RGfHw8Q1sI1ExlSlAfA6NRX2U idXs8duG9c0JZ0XtPNOndq/IEvPTlRSJtpYKBbDFWCWOdx0pGyUZT4ntRi4xjw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4btGXk5LnKz1B3S; Thu, 31 Jul 2025 17:39:02 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 56VHd2w4015297; Thu, 31 Jul 2025 17:39:02 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 56VHd2p8015294; Thu, 31 Jul 2025 17:39:02 GMT (envelope-from git) Date: Thu, 31 Jul 2025 17:39:02 GMT Message-Id: <202507311739.56VHd2p8015294@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: "Bjoern A. Zeeb" Subject: git: 0f8a8416a4e0 - main - dwmmc: cleanup cmd and locking, consistency between mmc and mmccam List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: bz X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 0f8a8416a4e07ddeaecc4eafd6f418da0f21efc7 Auto-Submitted: auto-generated The branch main has been updated by bz: URL: https://cgit.FreeBSD.org/src/commit/?id=0f8a8416a4e07ddeaecc4eafd6f418da0f21efc7 commit 0f8a8416a4e07ddeaecc4eafd6f418da0f21efc7 Author: Bjoern A. Zeeb AuthorDate: 2025-07-29 12:36:26 +0000 Commit: Bjoern A. Zeeb 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); }