<p style="white-space: pre-wrap; word-wrap: break-word;">>> Do you really need this in all its complexity? Many things seem overkill and the ramstage part doesn't fit into our current driver model.<br>> <br>> Yes, I need. I need to access devices behind a bridge, before and after PCI enumeration. The only complexity is the possibility to access/program all the BARs. Currently, I only need BAR0.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't see how your eMMC-reset ramstage code does anything useful. You just check a register value without any immediate action. So you can just let the payload read the register.</p><p style="white-space: pre-wrap; word-wrap: break-word;">That aside, you can just query the resource after allocation (e.g. with pci_get_resource()). Manual reading of the register would only be necessary before resource allocation. But that would have to be handled much more precisely (maybe I missed it, but I don't see anything reserving the temporary BAR, so having that enabled during allocation is dangerous).</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">I could just force a particular range of addresses, but it does not add much complexity to allow user define a preferred range.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">I would handle this very differently. For instance a function that takes a pci path, what resource to enable and a callback pointer. It enables the resource, calls back, takes it down again. Just do the whole procedure every time you want to talk to the device. That would keep coreboot's code flow free of any state to track.</p><p><a href="https://review.coreboot.org/c/coreboot/+/30215">View Change</a></p><p>4 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/30215/4/src/drivers/generic/temp_pcie_bridge/bridge_rom.c">File src/drivers/generic/temp_pcie_bridge/bridge_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/30215/4/src/drivers/generic/temp_pcie_bridge/bridge_rom.c@28">Patch Set #4, Line 28:</a> <code style="font-family:monospace,monospace"> outl(reg_2_cf8(dev, reg), 0xCF8);</code></p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Existing access functions assume bus 0.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">no (cf. PCI_DEV())</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/30215/4/src/drivers/generic/temp_pcie_bridge/bridge_rom.c@66">Patch Set #4, Line 66:</a> <code style="font-family:monospace,monospace">                value &= start;</code></p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">It does not matter.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">it does</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">I just need to assign a base address to one or more BAR registers. I don't care about alignment or holes...</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">You should. If it's aligning down, you might end up with an address below `next_mem`.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/30215/4/src/drivers/generic/temp_pcie_bridge/bridge_rom.c@79">Patch Set #4, Line 79:</a> <code style="font-family:monospace,monospace">  pci_write32(dev, TEMP_BASE(number), value);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">If you see the code, I'm forcing 32-bit resource, as it's temporary anyway.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Which code? I haven't seen it.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/30215/4/src/drivers/generic/temp_pcie_bridge/bridge_rom.c@80">Patch Set #4, Line 80:</a> <code style="font-family:monospace,monospace">  command |= PCI_COMMAND_MASTER;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I'm programming a device behind a bridge, before PCI initialization. So yes, it's needed.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Please elaborate, I'm not used to it. Does it have to be master to be able to send an asynchronous reply?</p></li></ul></li></ul><p>To view, visit <a href="https://review.coreboot.org/c/coreboot/+/30215">change 30215</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/+/30215"/><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: Iaec1fcc4f8db8c3b4cfd3786d3ff589dc9cb22f5 </div>
<div style="display:none"> Gerrit-Change-Number: 30215 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </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: Sat, 15 Dec 2018 11:27:18 +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>