<p>Felix Held has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/27654">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">device/pnp_device: rewrite pnp_get_ioresource<br><br>The rewrite simplifies the code and fixes the limit on the case of IO<br>resources with the length of 1 byte.<br><br>This patch is inspired by the comment of Nico Huber on<br>Ia99b785dcd9cf56fb236ad7ade54656851f88a5e<br><br>If the ones in the mask aren't a continuous block, now a warning message gets<br>printed.<br><br>Change-Id: I0089278f5adf65e8f084580bc5bc1eb2f77b87f7<br>Signed-off-by: Felix Held <felix-coreboot@felixheld.de><br>---<br>M src/device/pnp_device.c<br>1 file changed, 24 insertions(+), 32 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/27654/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/device/pnp_device.c b/src/device/pnp_device.c</span><br><span>index 5600231..edf3343 100644</span><br><span>--- a/src/device/pnp_device.c</span><br><span>+++ b/src/device/pnp_device.c</span><br><span>@@ -7,6 +7,7 @@</span><br><span> * Copyright (C) 2005 Tyan</span><br><span> * (Written by Yinghai Lu <yhlu@tyan.com> for Tyan)</span><br><span> * Copyright (C) 2013 Nico Huber <nico.h@gmx.de></span><br><span style="color: hsl(120, 100%, 40%);">+ * Copyright (C) 2018 Felix Held <felix-coreboot@felixheld.de></span><br><span> *</span><br><span> * This program is free software; you can redistribute it and/or modify</span><br><span> * it under the terms of the GNU General Public License as published by</span><br><span>@@ -25,6 +26,7 @@</span><br><span> #include <arch/io.h></span><br><span> #include <device/device.h></span><br><span> #include <device/pnp.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <assert.h></span><br><span> </span><br><span> /* PNP config mode wrappers */</span><br><span> </span><br><span>@@ -194,8 +196,10 @@</span><br><span> static void pnp_get_ioresource(struct device *dev, u8 index, u16 mask)</span><br><span> {</span><br><span> struct resource *resource;</span><br><span style="color: hsl(0, 100%, 40%);">- unsigned moving, gran, step;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int bit;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* If none of the mask bits is set, the resource would occupy the whole</span><br><span style="color: hsl(120, 100%, 40%);">+ IO space leading to IO resource conflicts with the other devices */</span><br><span> if (!mask) {</span><br><span> printk(BIOS_ERR, "ERROR: device %s index %d has no mask.\n",</span><br><span> dev_path(dev), index);</span><br><span>@@ -203,43 +207,31 @@</span><br><span> }</span><br><span> </span><br><span> resource = new_resource(dev, index);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* Initilize the resource. */</span><br><span style="color: hsl(0, 100%, 40%);">- resource->limit = 0xffff;</span><br><span> resource->flags |= IORESOURCE_IO;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Get the resource size... */</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Calculate IO region size which is determined by the first one from</span><br><span style="color: hsl(120, 100%, 40%);">+ the LSB of the mask. */</span><br><span style="color: hsl(120, 100%, 40%);">+ for (bit = 0; bit <= 15 && (mask & (1 << bit)) == 0; ++bit)</span><br><span style="color: hsl(120, 100%, 40%);">+ ;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- moving = mask;</span><br><span style="color: hsl(0, 100%, 40%);">- gran = 15;</span><br><span style="color: hsl(0, 100%, 40%);">- step = 1 << gran;</span><br><span style="color: hsl(120, 100%, 40%);">+ resource->gran = bit;</span><br><span style="color: hsl(120, 100%, 40%);">+ resource->align = bit;</span><br><span style="color: hsl(120, 100%, 40%);">+ resource->size = 1 << bit;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Find the first bit that moves. */</span><br><span style="color: hsl(0, 100%, 40%);">- while ((moving & step) == 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- gran--;</span><br><span style="color: hsl(0, 100%, 40%);">- step >>= 1;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Calculate IO region address limit which is determined by the first</span><br><span style="color: hsl(120, 100%, 40%);">+ one from the MSB of the mask. */</span><br><span style="color: hsl(120, 100%, 40%);">+ for (bit = 15; bit != 0 && (mask & (1 << bit)) == 0; --bit)</span><br><span style="color: hsl(120, 100%, 40%);">+ ;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Now find the first bit that does not move. */</span><br><span style="color: hsl(0, 100%, 40%);">- while ((moving & step) != 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- gran--;</span><br><span style="color: hsl(0, 100%, 40%);">- step >>= 1;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ resource->limit = (1 << (bit + 1)) - 1;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /*</span><br><span style="color: hsl(0, 100%, 40%);">- * Of the moving bits the last bit in the first group,</span><br><span style="color: hsl(0, 100%, 40%);">- * tells us the size of this resource.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">- if ((moving & step) == 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- gran++;</span><br><span style="color: hsl(0, 100%, 40%);">- step <<= 1;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* Set the resource size and alignment. */</span><br><span style="color: hsl(0, 100%, 40%);">- resource->gran = gran;</span><br><span style="color: hsl(0, 100%, 40%);">- resource->align = gran;</span><br><span style="color: hsl(0, 100%, 40%);">- resource->limit = mask | (step - 1);</span><br><span style="color: hsl(0, 100%, 40%);">- resource->size = 1 << gran;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* The block of ones in the mask is expected to be continuous.</span><br><span style="color: hsl(120, 100%, 40%);">+ If there is any zero inbetween the block of ones, it is ignored</span><br><span style="color: hsl(120, 100%, 40%);">+ in the calculation of the resource size and limit. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (mask != (resource->limit ^ (resource->size - 1)))</span><br><span style="color: hsl(120, 100%, 40%);">+ printk(BIOS_WARNING,</span><br><span style="color: hsl(120, 100%, 40%);">+ "WARNING: mask of device %s index %d wrong.\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ dev_path(dev), index);</span><br><span> }</span><br><span> </span><br><span> static void get_resources(struct device *dev, struct pnp_info *info)</span><br><span></span><br></pre><p>To view, visit <a href="https://review.coreboot.org/27654">change 27654</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/27654"/><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-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I0089278f5adf65e8f084580bc5bc1eb2f77b87f7 </div>
<div style="display:none"> Gerrit-Change-Number: 27654 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Felix Held <felix-coreboot@felixheld.de> </div>