[coreboot-gerrit] Change in ...coreboot[master]: soc/intel/braswell/acpi/lpc.asl: Allocate used ROM size only

[Michał Żygowski (Code Review)]

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31822 )

Change subject: soc/intel/braswell/acpi/lpc.asl: Allocate used ROM size only
......................................................................

Patch Set 2: -Code-Review

(1 comment)

https://review.coreboot.org/#/c/31822/2/src/soc/intel/braswell/acpi/lpc.asl 
File src/soc/intel/braswell/acpi/lpc.asl:

https://review.coreboot.org/#/c/31822/2/src/soc/intel/braswell/acpi/lpc.asl@... 
PS2, Line 48: 				      ((CONFIG_COREBOOT_ROMSIZE_KB*1024) + 1),
> Yes, tested and working. […]

I guess this is not what Nico meant. Let's take a CONFIG_COREBOOT_ROMSIZE_KB as 8192, then:

Memory32Fixed(ReadOnly, 0xffffffff - ((CONFIG_COREBOOT_ROMSIZE_KB*1024) + 1), CONFIG_COREBOOT_ROMSIZE_KB*1024)

Will be

Memory32Fixed(ReadOnly, 0xffffffff - (8MiB + 1), 8MiB) which is not correct. Why?
because 0xffffffff - (8MiB +1) will be 0xff7fffff, but should be 0xff800000.

0xffffffff - (8MiB + 1) == 0xffffffff - 8MiB - 1 != 0xffffffff - 8MiB + 1

The '+1' must be  outside parentheses.