Aaron Durbin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36683 )
Change subject: region: add rdev_chain_full() ......................................................................
region: add rdev_chain_full()
Instead of open coding an offset of 0 and querying the size of a region device provide a rdev_chain_full() helper function that does that for the caller. For the existing users that match this pattern convert them to using rdev_chain_full().
Change-Id: Ie316790a8a5b16a7f7e22f86f58bd2e633c19450 Signed-off-by: Aaron Durbin adurbin@chromium.org --- M src/commonlib/include/commonlib/cbfs.h M src/commonlib/include/commonlib/region.h M src/lib/fmap.c M src/lib/region_file.c 4 files changed, 11 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/36683/1
diff --git a/src/commonlib/include/commonlib/cbfs.h b/src/commonlib/include/commonlib/cbfs.h index cadc8c9..b0aa9d3 100644 --- a/src/commonlib/include/commonlib/cbfs.h +++ b/src/commonlib/include/commonlib/cbfs.h @@ -32,14 +32,13 @@ static inline void cbfs_file_data(struct region_device *data, const struct cbfsf *file) { - rdev_chain(data, &file->data, 0, region_device_sz(&file->data)); + rdev_chain_full(data, &file->data); }
static inline void cbfs_file_metadata(struct region_device *metadata, const struct cbfsf *file) { - rdev_chain(metadata, &file->metadata, 0, - region_device_sz(&file->metadata)); + rdev_chain_full(metadata, &file->metadata); }
/* diff --git a/src/commonlib/include/commonlib/region.h b/src/commonlib/include/commonlib/region.h index dca12dc..bb2abaa 100644 --- a/src/commonlib/include/commonlib/region.h +++ b/src/commonlib/include/commonlib/region.h @@ -73,6 +73,13 @@ int rdev_chain(struct region_device *child, const struct region_device *parent, size_t offset, size_t size);
+static inline int rdev_chain_full(struct region_device *child, + const struct region_device *parent) +{ + /* Chain full size of parent. */ + return rdev_chain(child, parent, 0, region_device_sz(parent)); +} +
/* A region_device operations. */ struct region_device_ops { diff --git a/src/lib/fmap.c b/src/lib/fmap.c index a427102..f3ff071 100644 --- a/src/lib/fmap.c +++ b/src/lib/fmap.c @@ -49,8 +49,7 @@
cache = car_get_var_ptr(&fmap_cache); if (region_device_sz(&cache->rdev)) - return rdev_chain(fmrd, &cache->rdev, 0, - region_device_sz(&cache->rdev)); + return rdev_chain_full(fmrd, &cache->rdev); }
boot_device_init(); diff --git a/src/lib/region_file.c b/src/lib/region_file.c index e42c2af..05d619c 100644 --- a/src/lib/region_file.c +++ b/src/lib/region_file.c @@ -208,7 +208,7 @@ f->slot = RF_FATAL;
/* Keep parent around for accessing data later. */ - if (rdev_chain(&f->rdev, p, 0, region_device_sz(p))) + if (rdev_chain_full(&f->rdev, p)) return -1;
if (rdev_readat(p, &mb, 0, sizeof(mb)) < 0) {
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36683 )
Change subject: region: add rdev_chain_full() ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/36683/1/src/commonlib/include/commo... File src/commonlib/include/commonlib/region.h:
https://review.coreboot.org/c/coreboot/+/36683/1/src/commonlib/include/commo... PS1, Line 80: return rdev_chain(child, parent, 0, region_device_sz(parent)); nit: could also just be memcpy()?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36683 )
Change subject: region: add rdev_chain_full() ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36683/1/src/commonlib/include/commo... File src/commonlib/include/commonlib/region.h:
https://review.coreboot.org/c/coreboot/+/36683/1/src/commonlib/include/commo... PS1, Line 80: return rdev_chain(child, parent, 0, region_device_sz(parent));
nit: could also just be memcpy()?
Not if parent is a root. We don't want non-root rdevs that have a lifetime that disappears.
Hello Julius Werner, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36683
to look at the new patch set (#2).
Change subject: region: add rdev_chain_full() ......................................................................
region: add rdev_chain_full()
Instead of open coding an offset of 0 and querying the size of a region device provide a rdev_chain_full() helper function that does that for the caller. For the existing users that match this pattern convert them to using rdev_chain_full().
Change-Id: Ie316790a8a5b16a7f7e22f86f58bd2e633c19450 Signed-off-by: Aaron Durbin adurbin@chromium.org --- M src/commonlib/include/commonlib/cbfs.h M src/commonlib/include/commonlib/region.h M src/lib/fmap.c M src/lib/region_file.c 4 files changed, 11 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/36683/2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36683 )
Change subject: region: add rdev_chain_full() ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36683/1/src/commonlib/include/commo... File src/commonlib/include/commonlib/region.h:
https://review.coreboot.org/c/coreboot/+/36683/1/src/commonlib/include/commo... PS1, Line 80: return rdev_chain(child, parent, 0, region_device_sz(parent));
Not if parent is a root. We don't want non-root rdevs that have a lifetime that disappears.
Oh right, missed that part.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36683 )
Change subject: region: add rdev_chain_full() ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36683 )
Change subject: region: add rdev_chain_full() ......................................................................
region: add rdev_chain_full()
Instead of open coding an offset of 0 and querying the size of a region device provide a rdev_chain_full() helper function that does that for the caller. For the existing users that match this pattern convert them to using rdev_chain_full().
Change-Id: Ie316790a8a5b16a7f7e22f86f58bd2e633c19450 Signed-off-by: Aaron Durbin adurbin@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/36683 Reviewed-by: Julius Werner jwerner@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/commonlib/include/commonlib/cbfs.h M src/commonlib/include/commonlib/region.h M src/lib/fmap.c M src/lib/region_file.c 4 files changed, 11 insertions(+), 7 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/src/commonlib/include/commonlib/cbfs.h b/src/commonlib/include/commonlib/cbfs.h index cadc8c9..b0aa9d3 100644 --- a/src/commonlib/include/commonlib/cbfs.h +++ b/src/commonlib/include/commonlib/cbfs.h @@ -32,14 +32,13 @@ static inline void cbfs_file_data(struct region_device *data, const struct cbfsf *file) { - rdev_chain(data, &file->data, 0, region_device_sz(&file->data)); + rdev_chain_full(data, &file->data); }
static inline void cbfs_file_metadata(struct region_device *metadata, const struct cbfsf *file) { - rdev_chain(metadata, &file->metadata, 0, - region_device_sz(&file->metadata)); + rdev_chain_full(metadata, &file->metadata); }
/* diff --git a/src/commonlib/include/commonlib/region.h b/src/commonlib/include/commonlib/region.h index dca12dc..f27a494 100644 --- a/src/commonlib/include/commonlib/region.h +++ b/src/commonlib/include/commonlib/region.h @@ -73,7 +73,6 @@ int rdev_chain(struct region_device *child, const struct region_device *parent, size_t offset, size_t size);
- /* A region_device operations. */ struct region_device_ops { void *(*mmap)(const struct region_device *, size_t, size_t); @@ -145,6 +144,13 @@ return rdev_mmap(rd, 0, region_device_sz(rd)); }
+static inline int rdev_chain_full(struct region_device *child, + const struct region_device *parent) +{ + /* Chain full size of parent. */ + return rdev_chain(child, parent, 0, region_device_sz(parent)); +} + /* * Compute relative offset of the child (c) w.r.t. the parent (p). Returns < 0 * when child is not within the parent's region. diff --git a/src/lib/fmap.c b/src/lib/fmap.c index a427102..f3ff071 100644 --- a/src/lib/fmap.c +++ b/src/lib/fmap.c @@ -49,8 +49,7 @@
cache = car_get_var_ptr(&fmap_cache); if (region_device_sz(&cache->rdev)) - return rdev_chain(fmrd, &cache->rdev, 0, - region_device_sz(&cache->rdev)); + return rdev_chain_full(fmrd, &cache->rdev); }
boot_device_init(); diff --git a/src/lib/region_file.c b/src/lib/region_file.c index e42c2af..05d619c 100644 --- a/src/lib/region_file.c +++ b/src/lib/region_file.c @@ -208,7 +208,7 @@ f->slot = RF_FATAL;
/* Keep parent around for accessing data later. */ - if (rdev_chain(&f->rdev, p, 0, region_device_sz(p))) + if (rdev_chain_full(&f->rdev, p)) return -1;
if (rdev_readat(p, &mb, 0, sizeof(mb)) < 0) {
-
Aaron Durbin (Code Review) -
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)