From nobody Fri May 5 00:30:02 2023 X-Original-To: dev-commits-src-all@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 4QCBQy3XjYz4B1Mv; Fri, 5 May 2023 00:30:02 +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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4QCBQy2xS3z4BsT; Fri, 5 May 2023 00:30:02 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1683246602; 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=LUWeaDKLoYewqPMhuI4IWCuraONZtaY8nzSu6V9/57k=; b=do7ZpI/TGU2g7cSp3ipUSeMSTyqVFKNEwYbPgVlvq1y7leVwuxm5bQ6mRATXFr2MfJiM7F V1VPwmU9nfX1Fvdu70HSvVlLT72z5XuLgekJQUCd1SYxgHGCGnvBiRDEpyHY03PdP3Y79B /ycsAGGaXUE+FqxfQKjz6Jj5QnvWOAVourj0/fa/PS4J13g2Dp+YX0CMfp+Pr96jT893L4 jF73NrmHhFum3wd1yBu6Y3Y4+Oo+/vlG1e19IWQkQsgGFtDWbUMZ75fG0tjYsClOukwy5B ciozloimTXl5L/KJ+ono/eduEIRjgdhQ7QK1hTrSlsDAaU/rT1m83txqdGH7Gg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1683246602; 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=LUWeaDKLoYewqPMhuI4IWCuraONZtaY8nzSu6V9/57k=; b=hVFQZtlw4W+OUGYW0++NxxyJZwdDTjuHCcMw/Oz+mZ3Np6JEl1N2YB3B9HdDb/sUfTV50I Eo1g7x82xdG25NRtVnWi7yyASQB2PCwHk3HrMrnazPHBtF9ep4tw90liP1TG5ujhPNqlE/ qI3A4qrXbQN+Hne0niPPzYF1ptk6/lAQzNCvshVF5knQdwbpCx6xbZs/x+ikQgpG5eLFwD smUrl2Lhv5IUHY0f0o4fKYTpJI/zfXucTumW6JFvMfCluRQ5gxGhe53QL+plJYbmr3MTUA IreqyQdaMxE7gFyHG5481qr/MInpJ0XzRwwXI/754jaAx2bNoj1X0wwpb/5kqw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1683246602; a=rsa-sha256; cv=none; b=Ek1gOmhh8bWjJg2yopaMutEu7VDQHg/yZamYdDeehlqlZbfbhq0Bq0TtYpfLPH9fvTxjSY Dk54Y+zxEdJGF9TL/Fv2UJ/SPCxsTtoOQDvIX7+MCGmFhWfWLKVRm2pyHdzDi1CPPBN5cx GJLaZ2W+Km4nCBFOtj4Ychk/qXB4RnjsHWMovCdO29sFnCACQsNvEtqxggEaunDpBs3sxU doskkHMJryUhyf3/EkpBM0VTtXnOhf55UB3U3qyZLmIetgate1ieBHYgRYOByltvqrYlRS NpxiyTU7f3T4yCyZ7M9BasZLDZoVAu1hpLAhtdZbmCRoL7b6QGEPgU0GUujlmg== 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 4QCBQy21KgzYTB; Fri, 5 May 2023 00:30:02 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 3450U2lk030501; Fri, 5 May 2023 00:30:02 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 3450U2XO030498; Fri, 5 May 2023 00:30:02 GMT (envelope-from git) Date: Fri, 5 May 2023 00:30:02 GMT Message-Id: <202305050030.3450U2XO030498@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Ravi Pokala Subject: git: fa63175685bf - stable/13 - jedec_dimm(4): Refactor offset adjustment and page0 reset List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rpokala X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: fa63175685bf81e6b6ec2b1a6db95f8c57449939 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by rpokala: URL: https://cgit.FreeBSD.org/src/commit/?id=fa63175685bf81e6b6ec2b1a6db95f8c57449939 commit fa63175685bf81e6b6ec2b1a6db95f8c57449939 Author: Ravi Pokala AuthorDate: 2023-04-27 05:03:48 +0000 Commit: Ravi Pokala CommitDate: 2023-05-05 00:28:18 +0000 jedec_dimm(4): Refactor offset adjustment and page0 reset Offsets greater than 255 bytes reside on page1 of the SPD device. Accessing them requires switching to page1, and adjusting the absolute offset to be relative to the start of page1. After the access, the page must be set back to page0. These operations are performed in several places, so break them out into their own functions. Also, replace a pair of default cases, which should be impossible due to earlier checks, with __assert_unreachable(). Reviewed by: imp MFC after: 1 week Sponsored by: Panasas Differential Revision: https://reviews.freebsd.org/D39842 (cherry picked from commit 15d69c840d860a8c0decb063f768ad695427a0d4) --- sys/dev/jedec_dimm/jedec_dimm.c | 176 +++++++++++++++++++++++++++------------- 1 file changed, 121 insertions(+), 55 deletions(-) diff --git a/sys/dev/jedec_dimm/jedec_dimm.c b/sys/dev/jedec_dimm/jedec_dimm.c index 73ea10c5c5e1..240fbe230d9f 100644 --- a/sys/dev/jedec_dimm/jedec_dimm.c +++ b/sys/dev/jedec_dimm/jedec_dimm.c @@ -144,6 +144,9 @@ const struct jedec_dimm_tsod_dev { { 0x104a, 0x03, "ST Microelectronics TSOD" }, }; +static int jedec_dimm_adjust_offset(struct jedec_dimm_softc *sc, + uint16_t orig_offset, uint16_t *new_offset, bool *page_changed); + static int jedec_dimm_attach(device_t dev); static int jedec_dimm_capacity(struct jedec_dimm_softc *sc, enum dram_type type, @@ -164,11 +167,81 @@ static int jedec_dimm_probe(device_t dev); static int jedec_dimm_readw_be(struct jedec_dimm_softc *sc, uint8_t reg, uint16_t *val); +static int jedec_dimm_reset_page0(struct jedec_dimm_softc *sc); + static int jedec_dimm_temp_sysctl(SYSCTL_HANDLER_ARGS); static const char *jedec_dimm_tsod_match(uint16_t vid, uint16_t did); +/** + * The DDR4 SPD information is spread across two 256-byte pages, but the + * offsets in the spec are absolute, not per-page. If a given offset is on the + * second page, we need to change to that page and adjust the offset to be + * relative to that page. + * + * @author rpokala + * + * @param[in] sc + * Instance-specific context data + * + * @param[in] orig_offset + * The original value of the offset, before any adjustment is made. + * + * @param[out] new_offset + * The offset to actually read from, adjusted if needed. + * + * @param[out] page_changed + * Whether or not the page was actually changed, and the offset adjusted. + * *If 'true', caller must call reset_page0() before itself returning.* + */ +static int +jedec_dimm_adjust_offset(struct jedec_dimm_softc *sc, uint16_t orig_offset, + uint16_t *new_offset, bool *page_changed) +{ + int rc; + + /* Don't change the offset, or indicate that the page has been changed, + * until the page has actually been changed. + */ + *new_offset = orig_offset; + *page_changed = false; + + /* Change to the proper page. Offsets [0, 255] are in page0; offsets + * [256, 511] are in page1. + */ + if (orig_offset < JEDEC_SPD_PAGE_SIZE) { + /* Within page0; no adjustment or page-change required. */ + rc = 0; + goto out; + } else if (orig_offset >= (2 * JEDEC_SPD_PAGE_SIZE)) { + /* Outside of expected range. */ + rc = EINVAL; + device_printf(sc->dev, "invalid offset 0x%04x\n", orig_offset); + goto out; + } + + /* 'orig_offset' is in page1. Switch to that page, and adjust + * 'new_offset' accordingly if successful. + * + * For the page-change operation, only the DTI and LSA matter; the + * offset and write-value are ignored, so just use 0. + */ + rc = smbus_writeb(sc->smbus, (JEDEC_DTI_PAGE | JEDEC_LSA_PAGE_SET1), + 0, 0); + if (rc != 0) { + device_printf(sc->dev, + "unable to change page for offset 0x%04x: %d\n", + orig_offset, rc); + goto out; + } + *page_changed = true; + *new_offset = orig_offset - JEDEC_SPD_PAGE_SIZE; + +out: + return (rc); +} + /** * device_attach() method. Read the DRAM type, use that to determine the offsets * and lengths of the asset string fields. Calculate the capacity. If a TSOD is @@ -458,9 +531,7 @@ jedec_dimm_capacity(struct jedec_dimm_softc *sc, enum dram_type type, sdram_width_offset = SPD_OFFSET_DDR4_SDRAM_WIDTH; break; default: - device_printf(sc->dev, "unsupported dram_type 0x%02x\n", type); - rc = EINVAL; - goto out; + __assert_unreachable(); } rc = smbus_readb(sc->smbus, sc->spd_addr, bus_width_offset, @@ -673,14 +744,13 @@ jedec_dimm_dump(struct jedec_dimm_softc *sc, enum dram_type type) out: if (page_changed) { int rc2; - /* Switch back to page0 before returning. */ - rc2 = smbus_writeb(sc->smbus, - (JEDEC_DTI_PAGE | JEDEC_LSA_PAGE_SET0), 0, 0); - if (rc2 != 0) { - device_printf(sc->dev, "unable to restore page: %d\n", - rc2); + + rc2 = jedec_dimm_reset_page0(sc); + if ((rc2 != 0) && (rc == 0)) { + rc = rc2; } } + return (rc); } @@ -715,54 +785,29 @@ jedec_dimm_field_to_str(struct jedec_dimm_softc *sc, char *dst, size_t dstsz, uint16_t offset, uint16_t len, bool ascii) { uint8_t byte; + uint16_t new_offset; int i; int rc; bool page_changed; - /* Change to the proper page. Offsets [0, 255] are in page0; offsets - * [256, 512] are in page1. - * - * *The page must be reset to page0 before returning.* - * - * For the page-change operation, only the DTI and LSA matter; the - * offset and write-value are ignored, so use just 0. - * - * Mercifully, JEDEC defined the fields such that none of them cross - * pages, so we don't need to worry about that complication. - */ - if (offset < JEDEC_SPD_PAGE_SIZE) { - page_changed = false; - } else if (offset < (2 * JEDEC_SPD_PAGE_SIZE)) { - page_changed = true; - rc = smbus_writeb(sc->smbus, - (JEDEC_DTI_PAGE | JEDEC_LSA_PAGE_SET1), 0, 0); - if (rc != 0) { - device_printf(sc->dev, - "unable to change page for offset 0x%04x: %d\n", - offset, rc); - } - /* Adjust the offset to account for the page change. */ - offset -= JEDEC_SPD_PAGE_SIZE; - } else { - page_changed = false; - rc = EINVAL; - device_printf(sc->dev, "invalid offset 0x%04x\n", offset); + rc = jedec_dimm_adjust_offset(sc, offset, &new_offset, &page_changed); + if (rc != 0) { goto out; } /* Sanity-check (adjusted) offset and length; everything must be within * the same page. */ - if (offset >= JEDEC_SPD_PAGE_SIZE) { + if (new_offset >= JEDEC_SPD_PAGE_SIZE) { rc = EINVAL; - device_printf(sc->dev, "invalid offset 0x%04x\n", offset); + device_printf(sc->dev, "invalid offset 0x%04x\n", new_offset); goto out; } - if ((offset + len) >= JEDEC_SPD_PAGE_SIZE) { + if ((new_offset + len) >= JEDEC_SPD_PAGE_SIZE) { rc = EINVAL; device_printf(sc->dev, - "(offset + len) would cross page (0x%04x + 0x%04x)\n", - offset, len); + "(new_offset + len) would cross page (0x%04x + 0x%04x)\n", + new_offset, len); goto out; } @@ -793,11 +838,12 @@ jedec_dimm_field_to_str(struct jedec_dimm_softc *sc, char *dst, size_t dstsz, /* Read a byte at a time. */ for (i = 0; i < len; i++) { - rc = smbus_readb(sc->smbus, sc->spd_addr, (offset + i), &byte); + rc = smbus_readb(sc->smbus, sc->spd_addr, (new_offset + i), + &byte); if (rc != 0) { device_printf(sc->dev, "failed to read byte at 0x%02x: %d\n", - (offset + i), rc); + (new_offset + i), rc); goto out; } if (ascii) { @@ -827,13 +873,10 @@ jedec_dimm_field_to_str(struct jedec_dimm_softc *sc, char *dst, size_t dstsz, out: if (page_changed) { int rc2; - /* Switch back to page0 before returning. */ - rc2 = smbus_writeb(sc->smbus, - (JEDEC_DTI_PAGE | JEDEC_LSA_PAGE_SET0), 0, 0); - if (rc2 != 0) { - device_printf(sc->dev, - "unable to restore page for offset 0x%04x: %d\n", - offset, rc2); + + rc2 = jedec_dimm_reset_page0(sc); + if ((rc2 != 0) && (rc == 0)) { + rc = rc2; } } @@ -880,10 +923,7 @@ jedec_dimm_mfg_date(struct jedec_dimm_softc *sc, enum dram_type type, week_offset = SPD_OFFSET_DDR4_MOD_MFG_WEEK; break; default: - device_printf(sc->dev, "unsupported dram_type 0x%02x\n", type); - rc = EINVAL; - page_changed = false; - goto out; + __assert_unreachable(); } /* Change to the proper page. Offsets [0, 255] are in page0; offsets @@ -1041,6 +1081,32 @@ out: return (rc); } +/** + * Reset to the default condition of operating on page0. This must be called + * after a previous operation changed to page1. + * + * @author rpokala + * + * @param[in] sc + * Instance-specific context data + */ +static int +jedec_dimm_reset_page0(struct jedec_dimm_softc *sc) +{ + int rc; + + /* For the page-change operation, only the DTI and LSA matter; the + * offset and write-value are ignored, so just use 0. + */ + rc = smbus_writeb(sc->smbus, (JEDEC_DTI_PAGE | JEDEC_LSA_PAGE_SET0), + 0, 0); + if (rc != 0) { + device_printf(sc->dev, "unable to restore page: %d\n", rc); + } + + return (rc); +} + /** * Read the temperature data from the TSOD and convert it to the deciKelvin * value that the sysctl expects.