Marc Jones has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45270 )
Change subject: xeon_sp: Update skx pci_irq.asl for ACPI2.0 notation ......................................................................
xeon_sp: Update skx pci_irq.asl for ACPI2.0 notation
Use the C style operators instead of the ACPI1.x polish notation. This is much easier to read. It also matches the cpx code for easier merging later.
This generates the same ASL code. Checked with BUILD_TIMLESS.
Change-Id: Id44138894d2ffed4c93afe5d4bbb4d59b538b577 Signed-off-by: Marc Jones marcjones@sysproconsulting.com --- M src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl 1 file changed, 2 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/45270/1
diff --git a/src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl b/src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl index dcb6fe2..b2a2ebf 100644 --- a/src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl +++ b/src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl @@ -8,10 +8,8 @@ * PIRQ routing control is in PCR ITSS region. */
-OperationRegion (ITSS, SystemMemory, - Add (PCR_ITSS_PIRQA_ROUT, - Add (CONFIG_PCR_BASE_ADDRESS, - ShiftLeft (PID_ITSS, PCR_PORTID_SHIFT))), 8) +OperationRegion (ITSS, SystemMemory, PCR_ITSS_PIRQA_ROUT + + CONFIG_PCR_BASE_ADDRESS + (PID_ITSS << PCR_PORTID_SHIFT), 8) Field (ITSS, ByteAcc, NoLock, Preserve) { PIRA, 8, /* PIRQA Routing Control */
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45270 )
Change subject: xeon_sp: Update skx pci_irq.asl for ACPI2.0 notation ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... File src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl:
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... PS1, Line 84: Or can this be converted to acpi2 notation ?
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45270 )
Change subject: xeon_sp: Update skx pci_irq.asl for ACPI2.0 notation ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... File src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl:
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... PS1, Line 61: Store oops, this one also :)
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... PS1, Line 64: ShiftLeft same
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... PS1, Line 71: Decrement (Local0) \ : Store also this
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... PS1, Line 76: And old acpi
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45270 )
Change subject: xeon_sp: Update skx pci_irq.asl for ACPI2.0 notation ......................................................................
Patch Set 1: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/45270/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45270/1//COMMIT_MSG@13 PS1, Line 13: BUILD_TIMLESS nit: tim*e*less
I would also mention which board you used to test this. Of course, I wouldn't expect this commit to change the old i440bx boards 😄
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... File src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl:
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... PS1, Line 61: Store
oops, this one also :)
We can change these after the files are merged. Is that OK for you?
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45270 )
Change subject: xeon_sp: Update skx pci_irq.asl for ACPI2.0 notation ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... File src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl:
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... PS1, Line 61: Store
We can change these after the files are merged. […]
sure
Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Jonathan Zhang, Johnny Lin, Jay Talbott, Angel Pons, Morgan Jang, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45270
to look at the new patch set (#2).
Change subject: xeon_sp: Update skx ITSS OperationRegion to ACPI2.0 notation ......................................................................
xeon_sp: Update skx ITSS OperationRegion to ACPI2.0 notation
Use the C style operators instead of the ACPI1.x polish notation. This is much easier to read. It also matches the cpx code for easier merging later.
This generates the same ASL code. Checked with BUILD_TIMLESS on TiogaPass..
Change-Id: Id44138894d2ffed4c93afe5d4bbb4d59b538b577 Signed-off-by: Marc Jones marcjones@sysproconsulting.com --- M src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl 1 file changed, 2 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/45270/2
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45270 )
Change subject: xeon_sp: Update skx ITSS OperationRegion to ACPI2.0 notation ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... File src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl:
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... PS1, Line 61: Store
sure
I've updated the summary to correctly describe the change. I didn't update the entire file, just the most complicated OperationRegion statement. This will make a merge the cpx ASL easier.
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... PS1, Line 64: ShiftLeft
same
Ack
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... PS1, Line 71: Decrement (Local0) \ : Store
also this
Ack
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... PS1, Line 76: And
old acpi
Ack
https://review.coreboot.org/c/coreboot/+/45270/1/src/soc/intel/xeon_sp/skx/a... PS1, Line 84: Or
can this be converted to acpi2 notation ?
Ack
Jay Talbott has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45270 )
Change subject: xeon_sp: Update skx ITSS OperationRegion to ACPI2.0 notation ......................................................................
Patch Set 2: Code-Review+1
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45270 )
Change subject: xeon_sp: Update skx ITSS OperationRegion to ACPI2.0 notation ......................................................................
Patch Set 2: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45270 )
Change subject: xeon_sp: Update skx ITSS OperationRegion to ACPI2.0 notation ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45270/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45270/1//COMMIT_MSG@13 PS1, Line 13: BUILD_TIMLESS
nit: tim*e*less […]
Ack
https://review.coreboot.org/c/coreboot/+/45270/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45270/2//COMMIT_MSG@14 PS2, Line 14: .. One trailing period is enough
https://review.coreboot.org/c/coreboot/+/45270/2//COMMIT_MSG@14 PS2, Line 14: BUILD_TIMLESS BUILD_TIM*E*LESS
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45270 )
Change subject: xeon_sp: Update skx ITSS OperationRegion to ACPI2.0 notation ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45270/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45270/2//COMMIT_MSG@14 PS2, Line 14: BUILD_TIMLESS
BUILD_TIM*E*LESS
Ack
https://review.coreboot.org/c/coreboot/+/45270/2//COMMIT_MSG@14 PS2, Line 14: ..
One trailing period is enough
Ack
Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Jonathan Zhang, Jay Talbott, Johnny Lin, Angel Pons, Morgan Jang, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45270
to look at the new patch set (#3).
Change subject: soc/intel/xeon_sp/skx: Update ITSS OperationRegion to ACPI2.0 notation ......................................................................
soc/intel/xeon_sp/skx: Update ITSS OperationRegion to ACPI2.0 notation
Prepare for merge with cpx. Use the C style operators instead of the ACPI1.x polish notation. This is much easier to read and matches the cpx code.
This generates the same ASL code. Checked with BUILD_TIMELESS on TiogaPass.
Change-Id: Id44138894d2ffed4c93afe5d4bbb4d59b538b577 Signed-off-by: Marc Jones marcjones@sysproconsulting.com --- M src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl 1 file changed, 2 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/45270/3
Jay Talbott has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45270 )
Change subject: soc/intel/xeon_sp/skx: Update ITSS OperationRegion to ACPI2.0 notation ......................................................................
Patch Set 3: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45270 )
Change subject: soc/intel/xeon_sp/skx: Update ITSS OperationRegion to ACPI2.0 notation ......................................................................
Patch Set 4: Code-Review+2
Marc Jones has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45270 )
Change subject: soc/intel/xeon_sp/skx: Update ITSS OperationRegion to ACPI2.0 notation ......................................................................
soc/intel/xeon_sp/skx: Update ITSS OperationRegion to ACPI2.0 notation
Prepare for merge with cpx. Use the C style operators instead of the ACPI1.x polish notation. This is much easier to read and matches the cpx code.
This generates the same ASL code. Checked with BUILD_TIMELESS on TiogaPass.
Change-Id: Id44138894d2ffed4c93afe5d4bbb4d59b538b577 Signed-off-by: Marc Jones marcjones@sysproconsulting.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/45270 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Jay Talbott JayTalbott@sysproconsulting.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl 1 file changed, 2 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Jay Talbott: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl b/src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl index dcb6fe2..b2a2ebf 100644 --- a/src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl +++ b/src/soc/intel/xeon_sp/skx/acpi/pci_irq.asl @@ -8,10 +8,8 @@ * PIRQ routing control is in PCR ITSS region. */
-OperationRegion (ITSS, SystemMemory, - Add (PCR_ITSS_PIRQA_ROUT, - Add (CONFIG_PCR_BASE_ADDRESS, - ShiftLeft (PID_ITSS, PCR_PORTID_SHIFT))), 8) +OperationRegion (ITSS, SystemMemory, PCR_ITSS_PIRQA_ROUT + + CONFIG_PCR_BASE_ADDRESS + (PID_ITSS << PCR_PORTID_SHIFT), 8) Field (ITSS, ByteAcc, NoLock, Preserve) { PIRA, 8, /* PIRQA Routing Control */
-
Angel Pons (Code Review) -
HAOUAS Elyes (Code Review)
-
Jay Talbott (Code Review)
-
Jonathan Zhang (Code Review)
-
Marc Jones (Code Review)