<p><a href="https://review.coreboot.org/c/coreboot/+/30225">View Change</a></p><p>5 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/30225/1//COMMIT_MSG">Commit Message:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/30225/1//COMMIT_MSG@14">Patch Set #1, Line 14:</a> <code style="font-family:monospace,monospace">second time). This might save up to 100 milliseconds (exact value TBD).</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">We are using a bayhub eMMC which will not accept any command until a software reset (bit 0 of regist […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">You can't pass the exact situation through cbmem unless the reset has actually finished. If it hasn't, it might still finish in the meantime until the payload driver takes over. That's why I think that checking it at the end of ramstage is futile and unnecessary. You need the code in the payload (to wait for a reset to finish) anyway.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/30225/1/src/drivers/emmc/emmc_rom.c">File src/drivers/emmc/emmc_rom.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/30225/1/src/drivers/emmc/emmc_rom.c@40">Patch Set #1, Line 40:</a> <code style="font-family:monospace,monospace"> byte = 0; /* preclear */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I just used byte above with bit 0 set. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">You mean the assignment in the next line might fail? wouldn't that violate C?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/30225/1/src/drivers/emmc/emmc_rom.c@43">Patch Set #1, Line 43:</a> <code style="font-family:monospace,monospace"> pinfo->status = EMMC_CONFIRMED_RESET_ACCEPTED;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Can you point me to documentation / a datasheet where this behaviour<br>is specified? What particularly concerns me is for how long the<br>EMMC_RESET_ALL bit is considered to be set at minimum? Or will it<br>always be set on the first read after setting it (no matter how much<br>time passed until the read)?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/30225/1/src/include/device/emmc.h">File src/include/device/emmc.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/30225/1/src/include/device/emmc.h@23">Patch Set #1, Line 23:</a> <code style="font-family:monospace,monospace">#define EMMC_RESET_COMPLETED 4</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">If cbmem for reset status is not found or is status is EMMC_SEND_RESET, something went wrong and pay […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">AFAICS, all you need to do in coreboot is to set EMMC_RESET_ALL and notify<br>the payload that you did. Everything else can be handled in the payload:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> if (!coreboot_send_reset)<br> set EMMC_RESET_ALL;<br> while (still EMMC_RESET_ALL)<br> wait;</pre><p style="white-space: pre-wrap; word-wrap: break-word;">All your other states seem to be artificial software additions without any<br>meaning for the hardware.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/30225/1/src/include/device/emmc.h@51">Patch Set #1, Line 51:</a> <code style="font-family:monospace,monospace"> */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">What I'm saying is that other code can use the same cbmem id. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Why should other code use the same cbmem id? It seems to me you are extending<br>the concept of cbmem id without a reason.</p></li></ul></li></ul><p>To view, visit <a href="https://review.coreboot.org/c/coreboot/+/30225">change 30225</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/c/coreboot/+/30225"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I4bb55fde23e3b70da1cca76e5672e880317b8bd2 </div>
<div style="display:none"> Gerrit-Change-Number: 30225 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Richard Spiegel <richard.spiegel@silverbackltd.com> </div>
<div style="display:none"> Gerrit-Reviewer: Daniel Kurtz <djkurtz@google.com> </div>
<div style="display:none"> Gerrit-Reviewer: Martin Roth <martinroth@google.com> </div>
<div style="display:none"> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> </div>
<div style="display:none"> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> </div>
<div style="display:none"> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Spiegel <richard.spiegel@silverbackltd.com> </div>
<div style="display:none"> Gerrit-Reviewer: Simon Glass <sjg@chromium.org> </div>
<div style="display:none"> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> </div>
<div style="display:none"> Gerrit-CC: Nico Huber <nico.h@gmx.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 18 Dec 2018 16:22:22 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: Richard Spiegel <richard.spiegel@silverbackltd.com> </div>
<div style="display:none"> Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>