From nobody Fri Dec 17 15:49:30 2021 X-Original-To: freebsd-arm@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 0500818F5B13 for ; Fri, 17 Dec 2021 15:49:45 +0000 (UTC) (envelope-from marklmi@yahoo.com) Received: from sonic301-21.consmr.mail.gq1.yahoo.com (sonic301-21.consmr.mail.gq1.yahoo.com [98.137.64.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4JFthm3LYpz3tnr for ; Fri, 17 Dec 2021 15:49:44 +0000 (UTC) (envelope-from marklmi@yahoo.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1639756175; bh=naWhg+xkEidC+x3oVCm4F9VwMwXJan3BlUfShKZg6II=; h=Subject:From:In-Reply-To:Date:Cc:References:To:From:Subject:Reply-To; b=eDw8MoxUeu+gkbX7LYBc+swHpheD0p57DcHZsDSt1ctWhxG0DmpvefnovNQRAyaW90RJYluigraUlEVIY2IFsmAVQ/qPccriv3sU1Y7UvpFe46v0Dua0KhCfYEGVuDCDG14f0vkdltXIKKN6OeYpCavWnO9MEoxh9qbz0J1lWtLNzbcvJYQ3PACwz+EHauC8CRNpF/jpWNRnP3XBescBVBwaIt2y8/11dEiDqBhPTBZLQYNKQlOS2nF+iWGkZ6fvhuloTnOleVeMZ2UqICnXdmOk0tbQitb8lQB3kPo/fF0sJ/IAFsiDKqkWAuXoShkWMRnIq9i3uC9j/qA2O1TDPA== X-SONIC-DKIM-SIGN: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1639756175; bh=mmq8HC0YcBVd5CtLU/VjAh7IGLn2cQD07cibiPQGxXr=; h=X-Sonic-MF:Subject:From:Date:To:From:Subject; b=e62PPnVMqfONzJAmpNIq3+aqTyzY7PtH0c3txnQNoimMkiJDfFSqscYSnsBEvuB/6aldSLtMw1siZPvc5OrK00G6x7V0KJ/1Ms8SruhuPSeKdnfl6grw43N97utoktZLWjMnqeuAHB7/3WbqgHwnCL378dWBexa7vXvNkJufiBExMtq4SPCIKVuHaBQt3SNdOb1hQBaH0S6LieF83pZryetYps2GKoN/pqM45RAvgJ9i92ZYWbpclNbbvKxeRtL1y3Y/2E3JewQtliI/rKH1N2joTzd7m77WKwITgT2XbiAymL+GpD5ZOTKLmf1QDTfTgy4wGiK1vtErX9TcPNr6AA== X-YMail-OSG: YcVK.vkVM1lqLqLgXQ7S1OUI6UOaML.R3qV5On7Drp40AfHhVI5Z4goz7uP3CTb jjdrbMKqhMlku_fD2PBkzEkgNMIKxhboxDpxQKUvbh7JICOilgSPPWV67xGWjbL_3UakPtJdYszz Yq96P02Ixlm6sifVM26MBhUrDtYQdOCy3TIWPVmLt8EK3UBqsxGoGoWgNxIPU6xamWdpX1fZ7_ft BznetgHa05la0kZFHFq2we4SJxXs4Vr2zMg7tvp5Y7_h65Z97QTdWZ3MwcW.c7TE_8fZ.gttmj_c VEHNgrUl3xAqCsceuAldRMqzx9NkJF2k8iF9UjiTwFbDgAKDa0R5lJVyRHF8UanL8gdJf0GzU0dp u0mCeCdjtPuqOCaBh_enaWKzrCUKodQiKsVSaW7YUWGSB5yMbkCREaiZ52jT1JDS5CDdSaqdx3dB InHlnVdKt.r.pyRMOKIF8XYGlVlmQLiVK532tONXgbPYjlQhjR6h2_j8tAtC6.JV9CNqQEZ1h1nG VLw1z0xNq6uuNk9YNUhPfrg_FRdctlXyQIz._avggWE3zGvZ_DBuMt_h_DYu916LqOw2Dc7rvNBy ggUjBP_m.fh2Bmhg1lDmKtogUB2GaiSxUvLHHVLBf8SI.2StRXde.94aRXTDOd1JzshpgD2kxo8H QxkUPPHkLVTNAHDV5BQ24A3uwL1__eXo71DbUGDjneeNJh7V4rkpXjxa.l5af8X28VXXRP25z_ra z9slotmJ9jef9GZ7_crsiF5fQGSmoayRu.I4.B22UQfvNJzBsgMMon_55UH4_hIb4l.2rHctsShz y9QCsUnDdB0g7tpupexKhjdu18ZRXWuMsEKmT9cUm0ZEJB6pn71l8DVLDue1oaWxHnRrdN2zsOlW RCu4dOuZL6sM.Cy4ppVofBdXWdl_dZfKbMXgiuX.31CZci8XH8I6.aLvEmvLjo1BVVIQdU4xqU7Z QFiyAYjGSak_lA4ld5zYMsRz0jNB.M7b9UHqhqQMr.pCuSd24b6D65fM7oW9PFKbLqs3mO5s2hFR XxdyQ15uZ9XGX2YeBEDJJ.qfIbEUNlkHvBu.c3N0vvpzzVUQbhwOjnQuTd.wjoj4N2BXLHpEAfwe iKRTnsdBz.hgNciGBlR5cqGwCRHKNekxFMACLBUKyB6HIQLDUj3ZOY0LyIW14Cf1K8YLwM7SeMfz 2LIwllre8Dji8h964Q6G.ctY1tYsW5unMkdWZysbzOw0ccV7nZAtHd7z4OJ.5cC6vA7Xk49Jz8Rt jSCy4Pgs13zbfh2RKa_EL2EPH6OcEovIGhI9jezeD_a_zZJwEfk_GbTQOnl8vPBIbOAwt_IZ.iHn kRGG.9s49ONherQ1NxLVUtArw5s2VkR90sWnmlNm8knYWuyHOpkIXa1E6XVfrkyf0dXKTruJEknL .G4Zpx26JLUS0YqH6jX70TK5U4CzCwWurFsHOc2VP8mAZdEdadx51PF9YHS1rrN0h54ZlxRObx2W YWIG6a_UpCIcEd2p_gwfM35fm3I25nudseztT5MEE0tTWCWiIoagrS8beWYikr52Obd0mIjshVq5 viTG41RA1eEn6RSnoBjL069C2Huc2T1waOUQPD9TE4T28zdD7bOXBftqwAYAIINldLcueSuIM.Mv cYLH9Wv2cF5k4tA7vXxTVXZ4iR4w8uefF_9qpSoPUAVKBJdjKSDD62BHGHbhb.2P11ightBQrY.W ysk51OwXkr1wsscwUU4FJL8NeSR2Nzc2JYMx2cbPJNPy_DakY82S8hdTSBp_T9GZRq8NrDYWL881 epL27zII1_NHR53yFI93zEneDQQ7oXJMIxoTvsbj14.iha7aHR39qOuHMx8U3zMrVIvzEBfduCu7 9Q5LuLwsyskIAhbiq0.Wzc5wb5CLGo5PaQ.4ACOUtq94GAtlZ_FLDxDgxNzOgsNJ6eU.2gvD1SeU p_TS42G7atizC_9VvAkIMbUHY_0xL.4A51fni.K9WQ7oJaqEHeXj3lcrgLrU20O6whoj4dVAzFwy LTtJYQe06W_GSx8rRRN8yyiyd_lTnm2yERCTfVKtgjkab4oi_4KRA.np7PhjI2xDEyWA6XYi4Qsj 8VDtAu8HoKelj1xWC71bn9zJBF_MNRH3SLECX19SoHZWkDm7zqAsDyVFPYI2yb5H4exVBec6kmLN dU9SEAdNydWh8XxxTb1HsEIULQZmyOdendsA6YL0.YWSWuHQlt9FJq9RrjpDFDWSgC1Ur6diP6IR 8RGmVpbK108abbbKXAjsg2hQglBLVh8nMjGxS.UWMZGVPh3u.oALDpZFHvdCfgOcWMWWL3cE13._ cxC_bizTReCnkzZ1FEkM- X-Sonic-MF: Received: from sonic.gate.mail.ne1.yahoo.com by sonic301.consmr.mail.gq1.yahoo.com with HTTP; Fri, 17 Dec 2021 15:49:35 +0000 Received: by kubenode512.mail-prod1.omega.bf1.yahoo.com (VZM Hermes SMTP Server) with ESMTPA ID f25d025290411deee803fdd8ab34476e; Fri, 17 Dec 2021 15:49:32 +0000 (UTC) Content-Type: text/plain; charset=utf-8 List-Id: Porting FreeBSD to ARM processors List-Archive: https://lists.freebsd.org/archives/freebsd-arm List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-arm@freebsd.org Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.120.0.1.13\)) Subject: Re: Rock64 configuration fails to boot for main 22c4ab6cb015 but worked for main 06bd74e1e39c (Nov 21): e.MMC mishandled? In-Reply-To: Date: Fri, 17 Dec 2021 07:49:30 -0800 Cc: Emmanuel Vadot , Free BSD Content-Transfer-Encoding: quoted-printable Message-Id: <27734923-C09E-429A-B54C-5123AEC37641@yahoo.com> References: <243CBFC7-DFB5-4F8B-A8A3-CFF78455148D.ref@yahoo.com> <243CBFC7-DFB5-4F8B-A8A3-CFF78455148D@yahoo.com> <20211209081930.7970b6995a8f7c5f7466227d@bidouilliste.com> <053617FD-AA34-4A3F-853A-4D2E44F8254B@yahoo.com> <43901D57-9C39-4FAC-A2BE-CCE642791705@yahoo.com> <8DAA50A1-3CF0-4AFA-9977-58FE15D4F171@yahoo.com> <21B0478B-340F-4BB2-9189-B5A6AE458134@yahoo.com> <7717F6CF-0239-4DC0-B23F-B9D5F75C0A8D@yahoo.com> <7EFA98DF-325F-4821-A040-FB4A9E66AB8F@yahoo.com> To: =?utf-8?Q?Kornel_Dul=C4=99ba?= X-Mailer: Apple Mail (2.3654.120.0.1.13) X-Rspamd-Queue-Id: 4JFthm3LYpz3tnr X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] Reply-To: marklmi@yahoo.com From: Mark Millard via freebsd-arm X-Original-From: Mark Millard X-ThisMailContainsUnwantedMimeParts: N On 2021-Dec-17, at 03:17, Kornel Dul=C4=99ba = wrote: >>>>> [I've cut out the history: just presenting some new evidence.] >>>>>=20 >>>>> First, a little context from getting to the db> prompt. >>>>>=20 >>>>> db> ps >>>>> pid ppid pgrp uid state wmesg wchan cmd >>>>> 18 0 0 0 DL syncer 0xffff000000eca5a8 [syncer] >>>>> 17 0 0 0 DL vlruwt 0xffffa00007d2ea60 [vnlru] >>>>> 16 0 0 0 DL (threaded) = [bufdaemon] >>>>> 100089 D qsleep 0xffff000000ec9478 = [bufdaemon] >>>>> 100092 D - 0xffff000000c11100 = [bufspacedaemon-0] >>>>> 100093 D - 0xffff000000c21680 = [bufspacedaemon-1] >>>>> 9 0 0 0 DL psleep 0xffff000000ef0650 = [vmdaemon] >>>>> 8 0 0 0 DL (threaded) = [pagedaemon] >>>>> 100087 D psleep 0xffff000000ee2b38 = [dom0] >>>>> 100094 D launds 0xffff000000ee2b44 = [laundry: dom0] >>>>> 100095 D umarcl 0xffff0000007b38d8 [uma] >>>>> 7 0 0 0 DL mmcsd d 0xffffa00007b72e00 = [mmcsd0boot1: mmc/sd] >>>>> 6 0 0 0 DL mmcsd d 0xffffa00007b71300 = [mmcsd0boot0: mmc/sd] >>>>> 5 0 0 0 DL mmcreq 0xffff00009b5d0710 [mmcsd0: = mmc/sd card] >>>>> 4 0 0 0 DL - 0xffff000000ccc020 = [rand_harvestq] >>>>> 15 0 0 0 DL (threaded) [usb] >>>>> . . . >>>>>=20 >>>>> and "mmcreq" is from the while loop in: >>>>>=20 >>>>> static int >>>>> mmc_wait_for_req(struct mmc_softc *sc, struct mmc_request *req) >>>>> { >>>>>=20 >>>>> req->done =3D mmc_wakeup; >>>>> req->done_data =3D sc; >>>>> if (__predict_false(mmc_debug > 1)) { >>>>> device_printf(sc->dev, "REQUEST: CMD%d arg %#x flags = %#x", >>>>> req->cmd->opcode, req->cmd->arg, req->cmd->flags); >>>>> if (req->cmd->data) { >>>>> printf(" data %d\n", = (int)req->cmd->data->len); >>>>> } else >>>>> printf("\n"); >>>>> } >>>>> MMCBR_REQUEST(device_get_parent(sc->dev), sc->dev, req); >>>>> MMC_LOCK(sc); >>>>> while ((req->flags & MMC_REQ_DONE) =3D=3D 0) >>>>> msleep(req, &sc->sc_mtx, 0, "mmcreq", 0); >>>>> MMC_UNLOCK(sc); >>>>> if (__predict_false(mmc_debug > 2 || (mmc_debug > 0 && >>>>> req->cmd->error !=3D MMC_ERR_NONE))) >>>>> device_printf(sc->dev, "CMD%d RESULT: %d\n", >>>>> req->cmd->opcode, req->cmd->error); >>>>> return (0); >>>>> } >>>>>=20 >>>>> So it appears that the error report: >>>>>=20 >>>>> mmcsd0: Error indicated: 4 Failed >>>>>=20 >>>>> ends up associated with (req->flags & MMC_REQ_DONE) =3D=3D 0 = staying >>>>> true in the above code: an unbounded loop with MMC_LOCK(sc) = active. >>>>> The "4" in the error report seems to be from: >>>>>=20 >>>>> #define MMC_ERR_FAILED 4 >>>>>=20 >>>>> It looks like there are some problems with handling errors, = problems >>>>> such that it gets stuck looping (no panic, no progress). >>>>>=20 >>>>> That seems to be separate from why the MMC_ERR_FAILED was = generated >>>>> in the first place. So: 2 problems, not just one. Thus it may be a >>>>> good context for tackling the looping problem with a known example >>>>> failure to look at. >>>>>=20 >>>>>=20 >>>>>=20 >>>>> Just for reference, I tried "boot -v" with debug.verbose_sysinit=3D1= in place, >>>>> just to capture and report the tail of the output for the boot = failure. >>>>>=20 >>>>> . . . >>>>> subsystem f000000 >>>>> release_aps(0)... Release APs...done >>>>> done. >>>>> intr_irq_shuffle(0)... Trying to mount root from = ufs:/dev/gpt/Rock64root []... >>>>> done. >>>>> netisr_start(0)... done. >>>>> taskqgroup_bind_softirq(0)... done. >>>>> GEOM: new disk mmcsd0 >>>>> GEOM: new disk mmcsd0boot0 >>>>> GEOM: new disk mmcsd0boot1 >>>>> smp_after_idle_runnable(0)... done. >>>>> taskqgroup_bind_if_config_tqg(0)... done. >>>>> taskqgroup_bind_if_io_tqg(0)... done. >>>>> tmr_setup_user_access(0)... done. >>>>> subsystem f000001 >>>>> mmcsd0: Error indicated: 4 Failed >>>>> epoch_init_smp(0)... done. >>>>> subsystem f100000 >>>>> racctd_init(0)... done. >>>>> subsystem fffffff >>>>> start_periodic_resettodr(0)... done. >>>>> oktousecallout(0)... done. >>>>> clknode_finish(0)... Unresolved linked clock found: hdmi_phy >>>>> Unresolved linked clock found: usb480m_phy >>>>> done. >>>>> regulator_constraint(0)... done. >>>>> regulator_shutdown(0)... regulator: shutting down unused = regulators >>>>> regulator: shutting down vcc_sd... busy >>>>> done. >>>>> uhub0: 1 port with 1 removable, self powered >>>>> uhub2: 2 ports with 2 removable, self powered >>>>> uhub3: 1 port with 1 removable, self powered >>>>> uhub1: 1 port with 1 removable, self powered >>>>> ugen4.2: at usbus4 >>>>> umass0 on uhub2 >>>>> umass0: = on usbus4 >>>>> umass0: SCSI over Bulk-Only; quirks =3D 0x0000 >>>>> umass0:0:0: Attached to scbus0 >>>>> pass0 at umass-sim0 bus 0 scbus0 target 0 lun 0 >>>>> pass0: Fixed Direct Access SPC-4 SCSI = device >>>>> pass0: Serial Number REPLACED >>>>> pass0: 400.000MB/s transfers >>>>> da0 at umass-sim0 bus 0 scbus0 target 0 lun 0 >>>>> da0: Fixed Direct Access SPC-4 SCSI = device >>>>> da0: Serial Number REPLACED >>>>> da0: 400.000MB/s transfers >>>>> da0: 953869MB (1953525168 512 byte sectors) >>>>> da0: quirks=3D0x2 >>>>> da0: Delete methods: >>>>> random: unblocking device. >>>>>=20 >>>>> No more output after that. >>>>=20 >>>> As for why MMC_ERR_FAILED results, the following code diff is >>>> intended to suggest what I think may be incomplete about sticking >>>> to what the device-specific code supports vs. does not support >>>> (not supporting HS200 here). The code does compile in my context >>>> but is untested. >>>=20 >>> It is now tested (at least to be a useful hack): no longer am I >>> running an older 1400042 kernel. For reference, >>>=20 >>> # uname -apKU >>> FreeBSD Rock64_RPi_4_3_2v1p2 14.0-CURRENT FreeBSD 14.0-CURRENT #18 = main-n251456-22c4ab6cb015-dirty: Sun Dec 12 00:34:53 PST 2021 = root@CA72_16Gp_ZFS:/usr/obj/BUILDs/main-CA53-nodbg-clang/usr/main-src/arm6= 4.aarch64/sys/GENERIC-NODBG-CA53 arm64 aarch64 1400043 1400043 >>>=20 >>> And it reports during the boot (other than the "REPLACED"): >>>=20 >>> mmcsd0: 125GB at mmc0 52.0MHz/8bit/1016-block >>>=20 >>> So it no longer sets up a mode that the rk3328-specific-code does = not >>> actually support. >>>=20 >>> (Nothing that I've done here deals with the looping issue when there >>> is a MMC_ERR_FAILED or the like.) >>>=20 >>>> The email handling may mess up some leading >>>> whitespace --but, again, I'm only trying to suggest a type of >>>> change. >>>>=20 >>>> # git -C /usr/main-src/ diff /usr/main-src/sys/dev/mmc >>>> diff --git a/sys/dev/mmc/mmc.c b/sys/dev/mmc/mmc.c >>>> index 9c73dfd57ce0..dffd1c382684 100644 >>>> --- a/sys/dev/mmc/mmc.c >>>> +++ b/sys/dev/mmc/mmc.c >>>> @@ -59,6 +59,7 @@ __FBSDID("$FreeBSD$"); >>>> #include >>>> #include >>>> #include >>>> +#include >>>> #include >>>> #include >>>> #include >>>> @@ -1512,6 +1513,8 @@ mmc_timing_to_string(enum mmc_bus_timing = timing) >>>> static bool >>>> mmc_host_timing(device_t dev, enum mmc_bus_timing timing) >>>> { >>>> + kobjop_desc_t kobj_desc; >>>> + kobj_method_t *kobj_method; >>>> int host_caps; >>>>=20 >>>> host_caps =3D mmcbr_get_caps(dev); >>>> @@ -1543,14 +1546,37 @@ mmc_host_timing(device_t dev, enum = mmc_bus_timing timing) >>>> case bus_timing_mmc_ddr52: >>>> return (HOST_TIMING_CAP(host_caps, = MMC_CAP_MMC_DDR52)); >>>> case bus_timing_mmc_hs200: >>>> - return (HOST_TIMING_CAP(host_caps, = MMC_CAP_MMC_HS200_120) || >>>> - HOST_TIMING_CAP(host_caps, = MMC_CAP_MMC_HS200_180)); >>>> case bus_timing_mmc_hs400: >>>> - return (HOST_TIMING_CAP(host_caps, = MMC_CAP_MMC_HS400_120) || >>>> - HOST_TIMING_CAP(host_caps, = MMC_CAP_MMC_HS400_180)); >>>> case bus_timing_mmc_hs400es: >>>> - return (HOST_TIMING_CAP(host_caps, = MMC_CAP_MMC_HS400 | >>>> - MMC_CAP_MMC_ENH_STROBE)); >>>> + /* >>>> + * Disable eMMC modes that require use of >>>> + * MMC_SEND_TUNING_BLOCK_HS200 to set things up if = either the >>>> + * tune or re-tune method is the default NULL = implementation. >>>> + */ >>>> + kobj_desc =3D &mmcbr_tune_desc; >>>> + kobj_method =3D = kobj_lookup_method(((kobj_t)dev)->ops->cls, NULL, >>>> + kobj_desc); >>>> + if (kobj_method =3D=3D &kobj_desc->deflt) >>>> + return (false); >>>> + kobj_desc =3D &mmcbr_retune_desc; >>>> + kobj_method =3D = kobj_lookup_method(((kobj_t)dev)->ops->cls, NULL, >>>> + kobj_desc); >>>> + if (kobj_method =3D=3D &kobj_desc->deflt) { >>>> + return (false); >>>> + } >>>> + >>>> + /* >>>> + * Otherwise track the host capabilities. >>>> + */ >>>> + if (timing =3D=3D bus_timing_mmc_hs200) >>>> + return (HOST_TIMING_CAP(host_caps, = MMC_CAP_MMC_HS200_120) || >>>> + HOST_TIMING_CAP(host_caps, = MMC_CAP_MMC_HS200_180)); >>>> + if (timing =3D=3D bus_timing_mmc_hs400) >>>> + return (HOST_TIMING_CAP(host_caps, = MMC_CAP_MMC_HS400_120) || >>>> + HOST_TIMING_CAP(host_caps, = MMC_CAP_MMC_HS400_180)); >>>> + if (timing =3D=3D bus_timing_mmc_hs400es) >>>> + return (HOST_TIMING_CAP(host_caps, = MMC_CAP_MMC_HS400 | >>>> + MMC_CAP_MMC_ENH_STROBE)); >>>> } >>>>=20 >>>> #undef HOST_TIMING_CAP >>>>=20 >>>>=20 >>>> In other words: have mmc_host_timing avoid returning true for some >>>> combinations that definitely do not have sufficient software = support >>>> present at the time. (So far as I can tell, the rk3328's get the >>>> NULL-implementations as things are.) >>>>=20 >>>> I expect that this sort of thing would go back to using >>>> MMC_CAP_MMC_DDR52 for the rk3328's, as an example. Working, but in = a >>>> slower mode, the same mode as FreeBSD was previously using. >>>>=20 >>>> A possible incompleteness in the suggestion is that there is also a >>>> drive-strength setting involved. If that also had "kobj" = interfacing >>>> and NULL-implementation possibilities, then in the future there = would >>>> be more to test for possibly forcing return-false than I did above. >>>>=20 >>>> Hopefully this sort of thing would help, possibly more than just = for >>>> rk3328's. >>>=20 >>>=20 >>=20 >> As for what was happening without my patch . . . >>=20 >> sys/dev/mmc/mmcbr_if.m defines: >>=20 >> static int >> null_retune(device_t brdev __unused, device_t reqdev __unused, >> bool reset __unused) >> { >>=20 >> return (0); >> } >>=20 >> static int >> null_tune(device_t brdev __unused, device_t reqdev __unused, >> bool hs400 __unused) >> { >>=20 >> return (0); >> } >> . . . >> # >> # Called by the mmcbus with the bridge claimed to execute initial = tuning. >> # >> METHOD int tune { >> device_t brdev; >> device_t reqdev; >> bool hs400; >> } DEFAULT null_tune; >>=20 >> # >> # Called by the mmcbus with the bridge claimed to execute re-tuning. >> # >> METHOD int retune { >> device_t brdev; >> device_t reqdev; >> bool reset; >> } DEFAULT null_retune; >> . . . >>=20 >> It is these success-reporting no-op routines that were being >> used to attempt the tuning: so there was no tuning done. >>=20 >> The code that I added detects that these routines would be >> used and avoids allowing contexts that would involve putting >> them to use with HS200 mode. >>=20 >> I'll note that there is another such null_* routine that the >> code (even with my patch) does not deal with avoiding the use >> of: >>=20 >> . . . >> static int >> null_switch_vccq(device_t brdev __unused, device_t reqdev = __unused) >> { >>=20 >> return (0); >> } >> . . . >> # >> # Called by the mmcbus to switch the signaling voltage (VCCQ). >> # >> METHOD int switch_vccq { >> device_t brdev; >> device_t reqdev; >> } DEFAULT null_switch_vccq; >> . . . >>=20 >> /usr/main-src/sys/dev/sdhci/sdhci.c has somewhat analogous code for >> somewhat analogous null_* routines. null_set_uhs_timing for that is >> from sys/dev/sdhci/sdhci_if.m (but the other two are again the above >> null_tune and null_retune routines, so not repeated here): >>=20 >> . . . >> static void >> null_set_uhs_timing(device_t brdev __unused, >> struct sdhci_slot *slot __unused) >> { >>=20 >> } >> . . . >> METHOD void set_uhs_timing { >> device_t brdev; >> struct sdhci_slot *slot; >> } DEFAULT null_set_uhs_timing; >> . . . >>=20 >> sdhci_init_slot(device_t dev, struct sdhci_slot *slot, int num) >> in sdhci.c looks like (in part): >>=20 >> . . . >> /* >> * Disable UHS-I and eMMC modes if the set_uhs_timing method = is the >> * default NULL implementation. >> */ >> kobj_desc =3D &sdhci_set_uhs_timing_desc; >> kobj_method =3D kobj_lookup_method(((kobj_t)dev)->ops->cls, = NULL, >> kobj_desc); >> if (kobj_method =3D=3D &kobj_desc->deflt) >> host_caps &=3D ~(MMC_CAP_UHS_SDR12 | MMC_CAP_UHS_SDR25 = | >> MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_DDR50 | = MMC_CAP_UHS_SDR104 | >> MMC_CAP_MMC_DDR52 | MMC_CAP_MMC_HS200 | = MMC_CAP_MMC_HS400); >>=20 >> #define SDHCI_CAP_MODES_TUNING(caps2) = \ >> (((caps2) & SDHCI_TUNE_SDR50 ? MMC_CAP_UHS_SDR50 : 0) | = \ >> MMC_CAP_UHS_DDR50 | MMC_CAP_UHS_SDR104 | MMC_CAP_MMC_HS200 | = \ >> MMC_CAP_MMC_HS400) >>=20 >> /* >> * Disable UHS-I and eMMC modes that require (re-)tuning if = either >> * the tune or re-tune method is the default NULL = implementation. >> */ >> kobj_desc =3D &mmcbr_tune_desc; >> kobj_method =3D kobj_lookup_method(((kobj_t)dev)->ops->cls, = NULL, >> kobj_desc); >> if (kobj_method =3D=3D &kobj_desc->deflt) >> goto no_tuning; >> kobj_desc =3D &mmcbr_retune_desc; >> kobj_method =3D kobj_lookup_method(((kobj_t)dev)->ops->cls, = NULL, >> kobj_desc); >> if (kobj_method =3D=3D &kobj_desc->deflt) { >> no_tuning: >> host_caps &=3D ~(SDHCI_CAP_MODES_TUNING(caps2)); >> } >> . . . >>=20 >> What I've done in my patch is analogous to what the the code shown >> after the #define SDHCI_CAP_MODES_TUNING above does, translated to >> fit the mmc's pre-existing code structure. >=20 > Good catch. For some reason mmcbr_tune/mmcbr_retune are not > implemented in sdhci_fdt.c. > This looks like a separate bug/issue. > Note that pretty much all other SDHCI drivers (sdhci_pci.c, > sdhci_xenon.c, sdhci_fsl_fdt.c, sdhci_acpi.c) provide some > implementation of mmcbr_tune/mmcbr_retune. > I agree that the logic in sdhci.c should disable HS200 if those > methods are implemented. > Could you try hooking mmcbr_tune and mmcbr_retune to their generic > sdhci implementations? What valid generic sdhci implementations? There is no such thing as far as I can tell. JESD84-B51 (e.MMC v5.1's standard) does nothing to standardize how adjustments are made during HS200 tuning, beyond how to get a known-pattern to test against (via: CMD21). That, of itself, does not make any adjustments. Also, as far as I can tell, the mmc that I changed and the sdhci code quoted above are independent at run-time. One or the other is used, not both. Otherwise the above quoted code would have prevented HS200 from being attempted. > If we're lucky this could be enough to make HS200 work on rock64, > though it's unlikely. I do not see that there is any generic sdhci tuning code to enable the use of: just avoiding use of code that is a no-op that falsely indicates success by avoiding needing HS200 tuning. =3D=3D=3D Mark Millard marklmi at yahoo.com