Name of user not set #1002476 has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36193 )
Change subject: arch/acpi.h: Use of typedef for acpi_vfct ......................................................................
arch/acpi.h: Use of typedef for acpi_vfct
Use of typedef and modify the usage accordingly.
Change-Id: I875ef2fa31e65750233fa8da2b76d8db5db44f2d Signed-off-by: Himanshu Sahdev himanshusah@hcl.com --- M src/arch/x86/acpi.c M src/arch/x86/include/arch/acpi.h M src/device/pci_rom.c 3 files changed, 11 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/36193/1
diff --git a/src/arch/x86/acpi.c b/src/arch/x86/acpi.c index 8e4de82..e28da53 100644 --- a/src/arch/x86/acpi.c +++ b/src/arch/x86/acpi.c @@ -737,14 +737,14 @@ }
void acpi_create_vfct(struct device *device, - struct acpi_vfct *vfct, + acpi_vfct_t *vfct, unsigned long (*acpi_fill_vfct)(struct device *device, - struct acpi_vfct *vfct_struct, unsigned long current)) + acpi_vfct_t *vfct_struct, unsigned long current)) { acpi_header_t *header = &(vfct->header); - unsigned long current = (unsigned long)vfct + sizeof(struct acpi_vfct); + unsigned long current = (unsigned long)vfct + sizeof(acpi_vfct_t);
- memset((void *)vfct, 0, sizeof(struct acpi_vfct)); + memset((void *)vfct, 0, sizeof(acpi_vfct_t));
if (!header) return; diff --git a/src/arch/x86/include/arch/acpi.h b/src/arch/x86/include/arch/acpi.h index c3420f8..fdffc30 100644 --- a/src/arch/x86/include/arch/acpi.h +++ b/src/arch/x86/include/arch/acpi.h @@ -265,14 +265,14 @@ } __packed acpi_vfct_image_hdr_t;
/* VFCT (VBIOS Fetch Table) */ -struct acpi_vfct { +typedef struct acpi_vfct { struct acpi_table_header header; u8 TableUUID[16]; u32 VBIOSImageOffset; u32 Lib1ImageOffset; u32 Reserved[4]; acpi_vfct_image_hdr_t image_hdr; -} __packed; +} __packed acpi_vfct_t;
typedef struct acpi_ivrs_info { } __packed acpi_ivrs_info_t; @@ -864,9 +864,9 @@ unsigned long (*acpi_fill_slit)(unsigned long current));
void acpi_create_vfct(struct device *device, - struct acpi_vfct *vfct, + acpi_vfct_t *vfct, unsigned long (*acpi_fill_vfct)(struct device *device, - struct acpi_vfct *vfct_struct, + acpi_vfct_t *vfct_struct, unsigned long current));
void acpi_create_ipmi(struct device *device, diff --git a/src/device/pci_rom.c b/src/device/pci_rom.c index b2b2266..3676f9c 100644 --- a/src/device/pci_rom.c +++ b/src/device/pci_rom.c @@ -197,7 +197,7 @@ }
static unsigned long -pci_rom_acpi_fill_vfct(struct device *device, struct acpi_vfct *vfct_struct, +pci_rom_acpi_fill_vfct(struct device *device, acpi_vfct_t *vfct_struct, unsigned long current) { acpi_vfct_image_hdr_t *header = &vfct_struct->image_hdr; @@ -245,10 +245,10 @@
/* AMD/ATI uses VFCT */ if (device->vendor == PCI_VENDOR_ID_ATI) { - struct acpi_vfct *vfct; + acpi_vfct_t *vfct;
current = ALIGN_UP(current, 8); - vfct = (struct acpi_vfct *)current; + vfct = (acpi_vfct_t *)current; acpi_create_vfct(device, vfct, pci_rom_acpi_fill_vfct); if (vfct->header.length) { printk(BIOS_DEBUG, "ACPI: * VFCT at %lx\n", current);
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36193 )
Change subject: arch/acpi.h: Use of typedef for acpi_vfct ......................................................................
Patch Set 1: Code-Review-1
Again, why?
Name of user not set #1002476 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36193 )
Change subject: arch/acpi.h: Use of typedef for acpi_vfct ......................................................................
Patch Set 1:
Patch Set 1: Code-Review-1
Again, why?
Thanks for your review and comments!!! I agree with you but after analyzing this file "/coreboot/src/arch/x86/include/arch/acpi.h", since almost all other struct defined in this file has used typedef that is the reason I have tried to provide the same kind of coding style as a base. Also, the typedef would be better for readability purposes.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36193 )
Change subject: arch/acpi.h: Use of typedef for acpi_vfct ......................................................................
Patch Set 1: Code-Review+2
Patch Set 1:
Patch Set 1: Code-Review-1
Again, why?
Thanks for your review and comments!!! I agree with you but after analyzing this file "/coreboot/src/arch/x86/include/arch/acpi.h", since almost all other struct defined in this file has used typedef that is the reason I have tried to provide the same kind of coding style as a base. Also, the typedef would be better for readability purposes.
After reading the whole file, I have to agree with you, though I don't like the different style from other files. It is however a more recent change, so whoever added it followed standard style instead of following file specific style, thus validating your analysis.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36193 )
Change subject: arch/acpi.h: Use of typedef for acpi_vfct ......................................................................
arch/acpi.h: Use of typedef for acpi_vfct
Use of typedef and modify the usage accordingly.
Change-Id: I875ef2fa31e65750233fa8da2b76d8db5db44f2d Signed-off-by: Himanshu Sahdev himanshusah@hcl.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/36193 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Richard Spiegel richard.spiegel@silverbackltd.com --- M src/arch/x86/acpi.c M src/arch/x86/include/arch/acpi.h M src/device/pci_rom.c 3 files changed, 11 insertions(+), 11 deletions(-)
Approvals: build bot (Jenkins): Verified Richard Spiegel: Looks good to me, approved
diff --git a/src/arch/x86/acpi.c b/src/arch/x86/acpi.c index 8e4de82..e28da53 100644 --- a/src/arch/x86/acpi.c +++ b/src/arch/x86/acpi.c @@ -737,14 +737,14 @@ }
void acpi_create_vfct(struct device *device, - struct acpi_vfct *vfct, + acpi_vfct_t *vfct, unsigned long (*acpi_fill_vfct)(struct device *device, - struct acpi_vfct *vfct_struct, unsigned long current)) + acpi_vfct_t *vfct_struct, unsigned long current)) { acpi_header_t *header = &(vfct->header); - unsigned long current = (unsigned long)vfct + sizeof(struct acpi_vfct); + unsigned long current = (unsigned long)vfct + sizeof(acpi_vfct_t);
- memset((void *)vfct, 0, sizeof(struct acpi_vfct)); + memset((void *)vfct, 0, sizeof(acpi_vfct_t));
if (!header) return; diff --git a/src/arch/x86/include/arch/acpi.h b/src/arch/x86/include/arch/acpi.h index c3420f8..fdffc30 100644 --- a/src/arch/x86/include/arch/acpi.h +++ b/src/arch/x86/include/arch/acpi.h @@ -265,14 +265,14 @@ } __packed acpi_vfct_image_hdr_t;
/* VFCT (VBIOS Fetch Table) */ -struct acpi_vfct { +typedef struct acpi_vfct { struct acpi_table_header header; u8 TableUUID[16]; u32 VBIOSImageOffset; u32 Lib1ImageOffset; u32 Reserved[4]; acpi_vfct_image_hdr_t image_hdr; -} __packed; +} __packed acpi_vfct_t;
typedef struct acpi_ivrs_info { } __packed acpi_ivrs_info_t; @@ -864,9 +864,9 @@ unsigned long (*acpi_fill_slit)(unsigned long current));
void acpi_create_vfct(struct device *device, - struct acpi_vfct *vfct, + acpi_vfct_t *vfct, unsigned long (*acpi_fill_vfct)(struct device *device, - struct acpi_vfct *vfct_struct, + acpi_vfct_t *vfct_struct, unsigned long current));
void acpi_create_ipmi(struct device *device, diff --git a/src/device/pci_rom.c b/src/device/pci_rom.c index b2b2266..3676f9c 100644 --- a/src/device/pci_rom.c +++ b/src/device/pci_rom.c @@ -197,7 +197,7 @@ }
static unsigned long -pci_rom_acpi_fill_vfct(struct device *device, struct acpi_vfct *vfct_struct, +pci_rom_acpi_fill_vfct(struct device *device, acpi_vfct_t *vfct_struct, unsigned long current) { acpi_vfct_image_hdr_t *header = &vfct_struct->image_hdr; @@ -245,10 +245,10 @@
/* AMD/ATI uses VFCT */ if (device->vendor == PCI_VENDOR_ID_ATI) { - struct acpi_vfct *vfct; + acpi_vfct_t *vfct;
current = ALIGN_UP(current, 8); - vfct = (struct acpi_vfct *)current; + vfct = (acpi_vfct_t *)current; acpi_create_vfct(device, vfct, pci_rom_acpi_fill_vfct); if (vfct->header.length) { printk(BIOS_DEBUG, "ACPI: * VFCT at %lx\n", current);
-
Name of user not set (Code Review) -
Patrick Georgi (Code Review)
-
Richard Spiegel (Code Review)