[Midnightbsd-cvs] src [12233] trunk/sys/dev/jedec_dimm/jedec_dimm.c: fix a bug where some devices lie about support

[laffer1 at midnightbsd.org]

Revision: 12233
          http://svnweb.midnightbsd.org/src/?rev=12233
Author:   laffer1
Date:     2019-08-08 20:04:09 -0400 (Thu, 08 Aug 2019)
Log Message:
-----------
fix a bug where some devices lie about support

Modified Paths:
--------------
    trunk/sys/dev/jedec_dimm/jedec_dimm.c

Modified: trunk/sys/dev/jedec_dimm/jedec_dimm.c
===================================================================
--- trunk/sys/dev/jedec_dimm/jedec_dimm.c	2019-08-08 23:06:02 UTC (rev 12232)
+++ trunk/sys/dev/jedec_dimm/jedec_dimm.c	2019-08-09 00:04:09 UTC (rev 12233)
@@ -1,4 +1,3 @@
-/* $MidnightBSD$ */
 /*-
  * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
  *
@@ -29,7 +28,8 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- * $FreeBSD: stable/10/sys/dev/jedec_dimm/jedec_dimm.c 331734 2018-03-29 15:01:01Z rpokala $
+ * $FreeBSD: stable/10/sys/dev/jedec_dimm/jedec_dimm.c 345838 2019-04-03 06:37:25Z rpokala $
+ * $MidnightBSD$
  */
 
 /* 
@@ -272,12 +272,16 @@
 	}
 
 	/* The MSBit of the TSOD-presence byte reports whether or not the TSOD
-	 * is in fact present. If it is, read manufacturer and device info from
-	 * it to confirm that it's a valid TSOD device. It's an error if any of
-	 * those bytes are unreadable; it's not an error if the device is simply
-	 * not known to us (tsod_match == NULL).
-	 * While DDR3 and DDR4 don't explicitly require a TSOD, essentially all
-	 * DDR3 and DDR4 DIMMs include one.
+	 * is in fact present. (While DDR3 and DDR4 don't explicitly require a
+	 * TSOD, essentially all DDR3 and DDR4 DIMMs include one.) But, as
+	 * discussed in [PR 235944], it turns out that some DIMMs claim to have
+	 * a TSOD when they actually don't. (Or maybe the firmware blocks it?)
+	 * <sigh>
+	 * If the SPD data says the TSOD is present, try to read manufacturer
+	 * and device info from it to confirm that it's a valid TSOD device.
+	 * If the data is unreadable, just continue as if the TSOD isn't there.
+	 * If the data was read successfully, see if it is a known TSOD device;
+	 * it's okay if it isn't (tsod_match == NULL).
 	 */
 	rc = smbus_readb(sc->smbus, sc->spd_addr, tsod_present_offset, &byte);
 	if (rc != 0) {
@@ -291,12 +295,14 @@
 		if (rc != 0) {
 			device_printf(dev,
 			    "failed to read TSOD Manufacturer ID\n");
-			goto out;
+			rc = 0;
+			goto no_tsod;
 		}
 		rc = jedec_dimm_readw_be(sc, TSOD_REG_DEV_REV, &devid);
 		if (rc != 0) {
 			device_printf(dev, "failed to read TSOD Device ID\n");
-			goto out;
+			rc = 0;
+			goto no_tsod;
 		}
 
 		tsod_match = jedec_dimm_tsod_match(vendorid, devid);
@@ -311,6 +317,7 @@
 			}
 		}
 	} else {
+no_tsod:
 		tsod_match = NULL;
 		tsod_present = false;
 	}
@@ -623,9 +630,12 @@
 		rc = smbus_writeb(sc->smbus,
 		    (JEDEC_DTI_PAGE | JEDEC_LSA_PAGE_SET1), 0, 0);
 		if (rc != 0) {
+			/* Some SPD devices (or SMBus controllers?) claim the
+			 * page-change command failed when it actually
+			 * succeeded. Log a message but soldier on.
+			 */
 			device_printf(sc->dev, "unable to change page: %d\n",
 			    rc);
-			goto out;
 		}
 		/* Add 256 to the store location, because we're in the second
 		 * page.