Aamir Bohra has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34625 )
Change subject: src/arch/x86/acpi: Constify struct device instances ......................................................................
src/arch/x86/acpi: Constify struct device instances
Constify the struct device arguments in below APIs:
acpi_device_name acpi_device_path_fill acpi_device_path acpi_write_dbg2_pci_uart acpi_device_scope acpi_device_path_join
The APIs does not seem to modify the argument and is using device argument as reference to device only.
Change-Id: Ic2ce045f17efa288eb41503795723d0ad5ec78bd Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/arch/x86/acpi.c M src/arch/x86/acpi_device.c M src/arch/x86/include/arch/acpi.h M src/arch/x86/include/arch/acpi_device.h 4 files changed, 12 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/34625/1
diff --git a/src/arch/x86/acpi.c b/src/arch/x86/acpi.c index 8ab993e..e4ccd37 100644 --- a/src/arch/x86/acpi.c +++ b/src/arch/x86/acpi.c @@ -946,7 +946,7 @@ }
unsigned long acpi_write_dbg2_pci_uart(acpi_rsdp_t *rsdp, unsigned long current, - struct device *dev, uint8_t access_size) + const struct device *dev, uint8_t access_size) { acpi_dbg2_header_t *dbg2 = (acpi_dbg2_header_t *)current; struct resource *res; diff --git a/src/arch/x86/acpi_device.c b/src/arch/x86/acpi_device.c index 0fb8f3b..47bcc52 100644 --- a/src/arch/x86/acpi_device.c +++ b/src/arch/x86/acpi_device.c @@ -53,9 +53,9 @@ }
/* Locate and return the ACPI name for this device */ -const char *acpi_device_name(struct device *dev) +const char *acpi_device_name(const struct device *dev) { - struct device *pdev = dev; + const struct device *pdev = dev; const char *name = NULL;
if (!dev) @@ -82,7 +82,7 @@ }
/* Recursive function to find the root device and print a path from there */ -static ssize_t acpi_device_path_fill(struct device *dev, char *buf, +static ssize_t acpi_device_path_fill(const struct device *dev, char *buf, size_t buf_len, size_t cur) { const char *name = acpi_device_name(dev); @@ -117,7 +117,7 @@ * Warning: just as with dev_path() this uses a static buffer * so should not be called mulitple times in one statement */ -const char *acpi_device_path(struct device *dev) +const char *acpi_device_path(const struct device *dev) { static char buf[DEVICE_PATH_MAX] = {};
@@ -131,7 +131,7 @@ }
/* Return the path of the parent device as the ACPI Scope for this device */ -const char *acpi_device_scope(struct device *dev) +const char *acpi_device_scope(const struct device *dev) { static char buf[DEVICE_PATH_MAX] = {};
@@ -145,7 +145,7 @@ }
/* Concatentate the device path and provided name suffix */ -const char *acpi_device_path_join(struct device *dev, const char *name) +const char *acpi_device_path_join(const struct device *dev, const char *name) { static char buf[DEVICE_PATH_MAX] = {}; ssize_t len; diff --git a/src/arch/x86/include/arch/acpi.h b/src/arch/x86/include/arch/acpi.h index 6251b98..259efcd 100644 --- a/src/arch/x86/include/arch/acpi.h +++ b/src/arch/x86/include/arch/acpi.h @@ -901,7 +901,7 @@ const char *device_path);
unsigned long acpi_write_dbg2_pci_uart(acpi_rsdp_t *rsdp, unsigned long current, - struct device *dev, uint8_t access_size); + const struct device *dev, uint8_t access_size); void acpi_create_dmar(acpi_dmar_t *dmar, enum dmar_flags flags, unsigned long (*acpi_fill_dmar)(unsigned long)); unsigned long acpi_create_dmar_drhd(unsigned long current, u8 flags, diff --git a/src/arch/x86/include/arch/acpi_device.h b/src/arch/x86/include/arch/acpi_device.h index 3569546..d74af9d 100644 --- a/src/arch/x86/include/arch/acpi_device.h +++ b/src/arch/x86/include/arch/acpi_device.h @@ -62,10 +62,10 @@ #define ACPI_DT_NAMESPACE_HID "PRP0001"
struct device; -const char *acpi_device_name(struct device *dev); -const char *acpi_device_path(struct device *dev); -const char *acpi_device_scope(struct device *dev); -const char *acpi_device_path_join(struct device *dev, const char *name); +const char *acpi_device_name(const struct device *dev); +const char *acpi_device_path(const struct device *dev); +const char *acpi_device_scope(const struct device *dev); +const char *acpi_device_path_join(const struct device *dev, const char *name); int acpi_device_status(const struct device *dev);
/*
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34625 )
Change subject: src/arch/x86/acpi: Constify struct device instances ......................................................................
Patch Set 1: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/34625/1/src/arch/x86/acpi.c File src/arch/x86/acpi.c:
https://review.coreboot.org/c/coreboot/+/34625/1/src/arch/x86/acpi.c@948 PS1, Line 948: unsigned long acpi_write_dbg2_pci_uart(acpi_rsdp_t *rsdp, unsigned long current, align with (
https://review.coreboot.org/c/coreboot/+/34625/1/src/arch/x86/include/arch/a... File src/arch/x86/include/arch/acpi.h:
https://review.coreboot.org/c/coreboot/+/34625/1/src/arch/x86/include/arch/a... PS1, Line 904: const struct device *dev, uint8_t access_size); align with (
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34625 )
Change subject: src/arch/x86/acpi: Constify struct device instances ......................................................................
Patch Set 1: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/34625/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34625/1//COMMIT_MSG@17 PS1, Line 17: is are
https://review.coreboot.org/c/coreboot/+/34625/1//COMMIT_MSG@17 PS1, Line 17: does do
Hello Patrick Rudolph, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34625
to look at the new patch set (#2).
Change subject: src/arch/x86/acpi: Constify struct device instances ......................................................................
src/arch/x86/acpi: Constify struct device instances
Constify the struct device arguments in below APIs:
acpi_device_name acpi_device_path_fill acpi_device_path acpi_write_dbg2_pci_uart acpi_device_scope acpi_device_path_join
The APIs do not seem to modify the argument and are using device argument as reference to device only.
Change-Id: Ic2ce045f17efa288eb41503795723d0ad5ec78bd Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/arch/x86/acpi.c M src/arch/x86/acpi_device.c M src/arch/x86/include/arch/acpi.h M src/arch/x86/include/arch/acpi_device.h 4 files changed, 12 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/34625/2
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34625 )
Change subject: src/arch/x86/acpi: Constify struct device instances ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34625/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34625/1//COMMIT_MSG@17 PS1, Line 17: does
do
Done
https://review.coreboot.org/c/coreboot/+/34625/1//COMMIT_MSG@17 PS1, Line 17: is
are
Done
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34625 )
Change subject: src/arch/x86/acpi: Constify struct device instances ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34625/1/src/arch/x86/acpi.c File src/arch/x86/acpi.c:
https://review.coreboot.org/c/coreboot/+/34625/1/src/arch/x86/acpi.c@948 PS1, Line 948: unsigned long acpi_write_dbg2_pci_uart(acpi_rsdp_t *rsdp, unsigned long current,
align with (
That would push it over 80 chars, will have to then split in 3 lines, did not look good. Can we keep as is?
https://review.coreboot.org/c/coreboot/+/34625/1/src/arch/x86/include/arch/a... File src/arch/x86/include/arch/acpi.h:
https://review.coreboot.org/c/coreboot/+/34625/1/src/arch/x86/include/arch/a... PS1, Line 904: const struct device *dev, uint8_t access_size);
align with (
That would push it over 80 chars, will have to then split in 3 lines, did not look good. Can we keep as is?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34625 )
Change subject: src/arch/x86/acpi: Constify struct device instances ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34625 )
Change subject: src/arch/x86/acpi: Constify struct device instances ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34625/1/src/arch/x86/acpi.c File src/arch/x86/acpi.c:
https://review.coreboot.org/c/coreboot/+/34625/1/src/arch/x86/acpi.c@948 PS1, Line 948: unsigned long acpi_write_dbg2_pci_uart(acpi_rsdp_t *rsdp, unsigned long current,
That would push it over 80 chars, will have to then split in 3 lines, did not look good. […]
We're past the 4.10 release, so there will be a clang-format run soon. Should we get this in now and let clang-format resolve this?
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34625 )
Change subject: src/arch/x86/acpi: Constify struct device instances ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34625/1/src/arch/x86/acpi.c File src/arch/x86/acpi.c:
https://review.coreboot.org/c/coreboot/+/34625/1/src/arch/x86/acpi.c@948 PS1, Line 948: unsigned long acpi_write_dbg2_pci_uart(acpi_rsdp_t *rsdp, unsigned long current,
We're past the 4.10 release, so there will be a clang-format run soon. […]
I'm fine with that. I can't wait to not have to worry about spacing.
https://review.coreboot.org/c/coreboot/+/34625/1/src/arch/x86/include/arch/a... File src/arch/x86/include/arch/acpi.h:
https://review.coreboot.org/c/coreboot/+/34625/1/src/arch/x86/include/arch/a... PS1, Line 904: const struct device *dev, uint8_t access_size);
That would push it over 80 chars, will have to then split in 3 lines, did not look good. […]
Done
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34625 )
Change subject: src/arch/x86/acpi: Constify struct device instances ......................................................................
src/arch/x86/acpi: Constify struct device instances
Constify the struct device arguments in below APIs:
acpi_device_name acpi_device_path_fill acpi_device_path acpi_write_dbg2_pci_uart acpi_device_scope acpi_device_path_join
The APIs do not seem to modify the argument and are using device argument as reference to device only.
Change-Id: Ic2ce045f17efa288eb41503795723d0ad5ec78bd Signed-off-by: Aamir Bohra aamir.bohra@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34625 Reviewed-by: Subrata Banik subrata.banik@intel.com Reviewed-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/arch/x86/acpi.c M src/arch/x86/acpi_device.c M src/arch/x86/include/arch/acpi.h M src/arch/x86/include/arch/acpi_device.h 4 files changed, 12 insertions(+), 12 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, but someone else must approve Subrata Banik: Looks good to me, approved Patrick Rudolph: Looks good to me, but someone else must approve
diff --git a/src/arch/x86/acpi.c b/src/arch/x86/acpi.c index 8ab993e..e4ccd37 100644 --- a/src/arch/x86/acpi.c +++ b/src/arch/x86/acpi.c @@ -946,7 +946,7 @@ }
unsigned long acpi_write_dbg2_pci_uart(acpi_rsdp_t *rsdp, unsigned long current, - struct device *dev, uint8_t access_size) + const struct device *dev, uint8_t access_size) { acpi_dbg2_header_t *dbg2 = (acpi_dbg2_header_t *)current; struct resource *res; diff --git a/src/arch/x86/acpi_device.c b/src/arch/x86/acpi_device.c index 0fb8f3b..47bcc52 100644 --- a/src/arch/x86/acpi_device.c +++ b/src/arch/x86/acpi_device.c @@ -53,9 +53,9 @@ }
/* Locate and return the ACPI name for this device */ -const char *acpi_device_name(struct device *dev) +const char *acpi_device_name(const struct device *dev) { - struct device *pdev = dev; + const struct device *pdev = dev; const char *name = NULL;
if (!dev) @@ -82,7 +82,7 @@ }
/* Recursive function to find the root device and print a path from there */ -static ssize_t acpi_device_path_fill(struct device *dev, char *buf, +static ssize_t acpi_device_path_fill(const struct device *dev, char *buf, size_t buf_len, size_t cur) { const char *name = acpi_device_name(dev); @@ -117,7 +117,7 @@ * Warning: just as with dev_path() this uses a static buffer * so should not be called mulitple times in one statement */ -const char *acpi_device_path(struct device *dev) +const char *acpi_device_path(const struct device *dev) { static char buf[DEVICE_PATH_MAX] = {};
@@ -131,7 +131,7 @@ }
/* Return the path of the parent device as the ACPI Scope for this device */ -const char *acpi_device_scope(struct device *dev) +const char *acpi_device_scope(const struct device *dev) { static char buf[DEVICE_PATH_MAX] = {};
@@ -145,7 +145,7 @@ }
/* Concatentate the device path and provided name suffix */ -const char *acpi_device_path_join(struct device *dev, const char *name) +const char *acpi_device_path_join(const struct device *dev, const char *name) { static char buf[DEVICE_PATH_MAX] = {}; ssize_t len; diff --git a/src/arch/x86/include/arch/acpi.h b/src/arch/x86/include/arch/acpi.h index 6251b98..259efcd 100644 --- a/src/arch/x86/include/arch/acpi.h +++ b/src/arch/x86/include/arch/acpi.h @@ -901,7 +901,7 @@ const char *device_path);
unsigned long acpi_write_dbg2_pci_uart(acpi_rsdp_t *rsdp, unsigned long current, - struct device *dev, uint8_t access_size); + const struct device *dev, uint8_t access_size); void acpi_create_dmar(acpi_dmar_t *dmar, enum dmar_flags flags, unsigned long (*acpi_fill_dmar)(unsigned long)); unsigned long acpi_create_dmar_drhd(unsigned long current, u8 flags, diff --git a/src/arch/x86/include/arch/acpi_device.h b/src/arch/x86/include/arch/acpi_device.h index 3569546..d74af9d 100644 --- a/src/arch/x86/include/arch/acpi_device.h +++ b/src/arch/x86/include/arch/acpi_device.h @@ -62,10 +62,10 @@ #define ACPI_DT_NAMESPACE_HID "PRP0001"
struct device; -const char *acpi_device_name(struct device *dev); -const char *acpi_device_path(struct device *dev); -const char *acpi_device_scope(struct device *dev); -const char *acpi_device_path_join(struct device *dev, const char *name); +const char *acpi_device_name(const struct device *dev); +const char *acpi_device_path(const struct device *dev); +const char *acpi_device_scope(const struct device *dev); +const char *acpi_device_path_join(const struct device *dev, const char *name); int acpi_device_status(const struct device *dev);
/*
-
Aamir Bohra (Code Review) -
Martin Roth (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
Subrata Banik (Code Review)