Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35172 )
Change subject: soc/intel/skylake: Clean up ......................................................................
Patch Set 7:
(10 comments)
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/acpi.... File src/soc/intel/skylake/acpi.c:
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/acpi.... PS7, Line 265: fadt->flags = ACPI_FADT_WBINVD | ACPI_FADT_C1_SUPPORTED | ACPI_FADT_C2_MP_SUPPORTED |
How exactly will this break? Just checked `git blame`, this statement has not been touched at all si […]
Ack
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/acpi.... PS7, Line 523: }
The line with the brace already provides enough clearance
a colon does, too. so why not drop all newlines^^ ;)
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/acpi.... PS7, Line 683: pm1_en |= PWRBTN_STS;
Where? I don't think it's necessary
tbh it's personal preference, but with that argument you could also drop 686 and 697.. *shrugs*
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/nhlt/... File src/soc/intel/skylake/nhlt/max98927.c:
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/nhlt/... PS7, Line 35:
This is to align the central `|` character.
Ack
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/nhlt/... PS7, Line 64: ARR
Yes. This is to align the array name on both lines, as it is the same.
Ack
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/nhlt/... File src/soc/intel/skylake/nhlt/nau88l25.c:
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/nhlt/... PS7, Line 59: ARRA
This is to align the array name on both lines, as it is the same.
Ack
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/nhlt/... File src/soc/intel/skylake/nhlt/rt5514.c:
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/nhlt/... PS7, Line 14:
This is to align the central `|` character.
Ack
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/nhlt/... PS7, Line 45: AR
This is to align the array name on both lines, as it is the same.
Ack
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/nhlt/... File src/soc/intel/skylake/nhlt/rt5663.c:
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/nhlt/... PS7, Line 59: A
This is to align the array name on both lines, as it is the same.
Ack ... and ack to all the others. I was simply confused :)
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/nhlt/... File src/soc/intel/skylake/nhlt/ssm4567.c:
https://review.coreboot.org/c/coreboot/+/35172/7/src/soc/intel/skylake/nhlt/... PS7, Line 57: A
This is to align the array name on both lines, as it is the same.
Ack