[coreboot-gerrit] Change in coreboot[master]: device/pnp_device: rewrite pnp_get_ioresource

[Felix Held (Code Review)]

Felix Held has uploaded this change for review. ( https://review.coreboot.org/27654


Change subject: device/pnp_device: rewrite pnp_get_ioresource
......................................................................

device/pnp_device: rewrite pnp_get_ioresource

The rewrite simplifies the code and fixes the limit on the case of IO
resources with the length of 1 byte.

This patch is inspired by the comment of Nico Huber on
Ia99b785dcd9cf56fb236ad7ade54656851f88a5e

If the ones in the mask aren't a continuous block, now a warning message gets
printed.

Change-Id: I0089278f5adf65e8f084580bc5bc1eb2f77b87f7
Signed-off-by: Felix Held <felix-coreboot at felixheld.de>
---
M src/device/pnp_device.c
1 file changed, 24 insertions(+), 32 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/27654/1

diff --git a/src/device/pnp_device.c b/src/device/pnp_device.c
index 5600231..edf3343 100644
--- a/src/device/pnp_device.c
+++ b/src/device/pnp_device.c
@@ -7,6 +7,7 @@
  * Copyright (C) 2005 Tyan
  * (Written by Yinghai Lu <yhlu at tyan.com> for Tyan)
  * Copyright (C) 2013 Nico Huber <nico.h at gmx.de>
+ * Copyright (C) 2018 Felix Held <felix-coreboot at felixheld.de>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -25,6 +26,7 @@
 #include <arch/io.h>
 #include <device/device.h>
 #include <device/pnp.h>
+#include <assert.h>
 
 /* PNP config mode wrappers */
 
@@ -194,8 +196,10 @@
 static void pnp_get_ioresource(struct device *dev, u8 index, u16 mask)
 {
 	struct resource *resource;
-	unsigned moving, gran, step;
+	unsigned int bit;
 
+	/* If none of the mask bits is set, the resource would occupy the whole
+	   IO space leading to IO resource conflicts with the other devices */
 	if (!mask) {
 		printk(BIOS_ERR, "ERROR: device %s index %d has no mask.\n",
 				dev_path(dev), index);
@@ -203,43 +207,31 @@
 	}
 
 	resource = new_resource(dev, index);
-
-	/* Initilize the resource. */
-	resource->limit = 0xffff;
 	resource->flags |= IORESOURCE_IO;
 
-	/* Get the resource size... */
+	/* Calculate IO region size which is determined by the first one from
+	   the LSB of the mask. */
+	for (bit = 0; bit <= 15 && (mask & (1 << bit)) == 0; ++bit)
+		;
 
-	moving = mask;
-	gran = 15;
-	step = 1 << gran;
+	resource->gran  = bit;
+	resource->align = bit;
+	resource->size  = 1 << bit;
 
-	/* Find the first bit that moves. */
-	while ((moving & step) == 0) {
-		gran--;
-		step >>= 1;
-	}
+	/* Calculate IO region address limit which is determined by the first
+	   one from the MSB of the mask. */
+	for (bit = 15; bit != 0 && (mask & (1 << bit)) == 0; --bit)
+		;
 
-	/* Now find the first bit that does not move. */
-	while ((moving & step) != 0) {
-		gran--;
-		step >>= 1;
-	}
+	resource->limit = (1 << (bit + 1)) - 1;
 
-	/*
-	 * Of the moving bits the last bit in the first group,
-	 * tells us the size of this resource.
-	 */
-	if ((moving & step) == 0) {
-		gran++;
-		step <<= 1;
-	}
-
-	/* Set the resource size and alignment. */
-	resource->gran  = gran;
-	resource->align = gran;
-	resource->limit = mask | (step - 1);
-	resource->size  = 1 << gran;
+	/* The block of ones in the mask is expected to be continuous.
+	   If there is any zero inbetween the block of ones, it is ignored
+	   in the calculation of the resource size and limit. */
+	if (mask != (resource->limit ^ (resource->size - 1)))
+		printk(BIOS_WARNING,
+			"WARNING: mask of device %s index %d wrong.\n",
+			dev_path(dev), index);
 }
 
 static void get_resources(struct device *dev, struct pnp_info *info)

-- 
To view, visit https://review.coreboot.org/27654
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0089278f5adf65e8f084580bc5bc1eb2f77b87f7
Gerrit-Change-Number: 27654
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot at felixheld.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180727/b5a23a63/attachment.html>