Change in coreboot[master]: util/sconfig: Drop id from struct device maintained by sconfig
Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41005 )
Change subject: util/sconfig: Drop id from struct device maintained by sconfig ......................................................................
util/sconfig: Drop id from struct device maintained by sconfig
This change drops the id field from struct device as used by sconfig. It was primarily used for generating unique device names. This was maintained within device structure so that the order in which the device tree entries were parsed is clear. Since the ids are assigned in parsing order, it is problematic when a device is moved from base devicetree to override tree. The entire parsing order changes which makes it really difficult to compare what really changed in static.c file.
By moving the dev name assignment to happen later when doing pass0 of static.c generation, the difference in static.c file is minimized when adding support for override trees.
BUG=b:155549176
Change-Id: I31870ace5a2fd7d5f95ab5e30d794c3bc959ed46 Signed-off-by: Furquan Shaikh furquan@google.com --- M util/sconfig/main.c M util/sconfig/sconfig.h 2 files changed, 7 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/41005/1
diff --git a/util/sconfig/main.c b/util/sconfig/main.c index b48f992..27fca44 100644 --- a/util/sconfig/main.c +++ b/util/sconfig/main.c @@ -98,7 +98,6 @@
static struct device base_root_dev = { .name = "dev_root", - .id = 0, .chip_instance = &mainboard_instance, .path = " .type = DEVICE_PATH_ROOT ", .parent = &base_root_bus, @@ -113,7 +112,6 @@
static struct device override_root_dev = { .name = "override_root", - .id = 0, /* * Override tree root device points to the same mainboard chip instance * as the base tree root device. It should not cause any side-effects @@ -429,7 +427,6 @@ { struct device *dev = S_ALLOC(sizeof(*dev));
- dev->id = ++count; dev->parent = parent; dev->subsystem_vendor = -1; dev->subsystem_device = -1; @@ -510,10 +507,6 @@ new_d->path_a = path_a; new_d->path_b = path_b;
- char *name = S_ALLOC(10); - sprintf(name, "_dev%d", new_d->id); - new_d->name = name; - new_d->enabled = status & 0x01; new_d->hidden = (status >> 1) & 0x01; new_d->mandatory = (status >> 2) & 0x01; @@ -699,12 +692,18 @@
static void pass0(FILE *fil, FILE *head, struct device *ptr, struct device *next) { + static int dev_id; + if (ptr == &base_root_dev) { fprintf(fil, "STORAGE struct bus %s_links[];\n", ptr->name); return; }
+ char *name = S_ALLOC(10); + sprintf(name, "_dev%d", dev_id++); + ptr->name = name; + fprintf(fil, "STORAGE struct device %s;\n", ptr->name); if (ptr->res) fprintf(fil, "STORAGE struct resource %s_res[];\n", diff --git a/util/sconfig/sconfig.h b/util/sconfig/sconfig.h index 60842f1..a76506d 100644 --- a/util/sconfig/sconfig.h +++ b/util/sconfig/sconfig.h @@ -43,10 +43,7 @@
struct chip; struct chip_instance { - /* - * Monotonically increasing ID for each newly allocated - * node(chip/device). - */ + /* Monotonically increasing ID for each chip instance. */ int id;
/* Pointer to registers for this chip. */ @@ -98,9 +95,6 @@ };
struct device { - /* Monotonically increasing ID for the device. */ - int id; - /* Indicates device status (enabled / hidden or not). */ int enabled; int hidden;
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41005 )
Change subject: util/sconfig: Drop id from struct device maintained by sconfig ......................................................................
Patch Set 2: Code-Review+2
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41005 )
Change subject: util/sconfig: Drop id from struct device maintained by sconfig ......................................................................
util/sconfig: Drop id from struct device maintained by sconfig
This change drops the id field from struct device as used by sconfig. It was primarily used for generating unique device names. This was maintained within device structure so that the order in which the device tree entries were parsed is clear. Since the ids are assigned in parsing order, it is problematic when a device is moved from base devicetree to override tree. The entire parsing order changes which makes it really difficult to compare what really changed in static.c file.
By moving the dev name assignment to happen later when doing pass0 of static.c generation, the difference in static.c file is minimized when adding support for override trees.
BUG=b:155549176
Change-Id: I31870ace5a2fd7d5f95ab5e30d794c3bc959ed46 Signed-off-by: Furquan Shaikh furquan@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41005 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org --- M util/sconfig/main.c M util/sconfig/sconfig.h 2 files changed, 7 insertions(+), 14 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved
diff --git a/util/sconfig/main.c b/util/sconfig/main.c index b48f992..27fca44 100644 --- a/util/sconfig/main.c +++ b/util/sconfig/main.c @@ -98,7 +98,6 @@
static struct device base_root_dev = { .name = "dev_root", - .id = 0, .chip_instance = &mainboard_instance, .path = " .type = DEVICE_PATH_ROOT ", .parent = &base_root_bus, @@ -113,7 +112,6 @@
static struct device override_root_dev = { .name = "override_root", - .id = 0, /* * Override tree root device points to the same mainboard chip instance * as the base tree root device. It should not cause any side-effects @@ -429,7 +427,6 @@ { struct device *dev = S_ALLOC(sizeof(*dev));
- dev->id = ++count; dev->parent = parent; dev->subsystem_vendor = -1; dev->subsystem_device = -1; @@ -510,10 +507,6 @@ new_d->path_a = path_a; new_d->path_b = path_b;
- char *name = S_ALLOC(10); - sprintf(name, "_dev%d", new_d->id); - new_d->name = name; - new_d->enabled = status & 0x01; new_d->hidden = (status >> 1) & 0x01; new_d->mandatory = (status >> 2) & 0x01; @@ -699,12 +692,18 @@
static void pass0(FILE *fil, FILE *head, struct device *ptr, struct device *next) { + static int dev_id; + if (ptr == &base_root_dev) { fprintf(fil, "STORAGE struct bus %s_links[];\n", ptr->name); return; }
+ char *name = S_ALLOC(10); + sprintf(name, "_dev%d", dev_id++); + ptr->name = name; + fprintf(fil, "STORAGE struct device %s;\n", ptr->name); if (ptr->res) fprintf(fil, "STORAGE struct resource %s_res[];\n", diff --git a/util/sconfig/sconfig.h b/util/sconfig/sconfig.h index 60842f1..a76506d 100644 --- a/util/sconfig/sconfig.h +++ b/util/sconfig/sconfig.h @@ -43,10 +43,7 @@
struct chip; struct chip_instance { - /* - * Monotonically increasing ID for each newly allocated - * node(chip/device). - */ + /* Monotonically increasing ID for each chip instance. */ int id;
/* Pointer to registers for this chip. */ @@ -98,9 +95,6 @@ };
struct device { - /* Monotonically increasing ID for the device. */ - int id; - /* Indicates device status (enabled / hidden or not). */ int enabled; int hidden;
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)