Change in coreboot[master]: nb/intel/x4x: Factor out hiding PCI devs in pure fn

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Change in em100[master]: Add...

Arthur Heymans (Code Review)

27 Nov 2019 27 Nov '19

10:04 p.m.

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37291 )

Change subject: nb/intel/x4x: Factor out hiding PCI devs in pure fn ......................................................................

nb/intel/x4x: Factor out hiding PCI devs in pure fn

This increases readability.

Also change the update expression. '--variable' does not make much sense there.

Change-Id: I64db2460115f5fb35ca197b83440f8ee47470761 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/northbridge/intel/x4x/northbridge.c 1 file changed, 21 insertions(+), 20 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37291/1

diff --git a/src/northbridge/intel/x4x/northbridge.c b/src/northbridge/intel/x4x/northbridge.c index 39f24d3..efdaeef 100644 --- a/src/northbridge/intel/x4x/northbridge.c +++ b/src/northbridge/intel/x4x/northbridge.c @@ -206,44 +206,45 @@ dev->ops = &cpu_bus_ops; }

+static void hide_pci_fn(const int dev_bit_base, const struct device *dev) +{ + if (!dev || dev->enabled) + return; + const unsigned int fn = PCI_FUNC(dev->path.pci.devfn); + const struct device *const d0f0 = pcidev_on_root(0, 0); + pci_update_config32(d0f0, D0F0_DEVEN, ~(1 << (dev_bit_base + fn)), 0); +} + +static void hide_pci_dev(const int dev, int functions, const int dev_bit_base) +{ + for (; functions >= 0; functions--) + hide_pci_fn(dev_bit_base, pcidev_on_root(dev, functions)); +} + static void x4x_init(void *const chip_info) { - int dev, fn, bit_base; - + int dev; struct device *const d0f0 = pcidev_on_root(0x0, 0);

/* Hide internal functions based on devicetree info. */ - for (dev = 6; dev > 0; --dev) { + for (dev = 6; dev > 0; dev--) { switch (dev) { case 6: /* PEG1: only on P45 */ - fn = 0; - bit_base = 13; + hide_pci_dev(dev, 0, 13); break; case 3: /* ME */ - fn = 3; - bit_base = 6; + hide_pci_dev(dev, 3, 6); break; case 2: /* IGD */ - fn = 1; - bit_base = 3; + hide_pci_dev(dev, 1, 3); break; case 1: /* PEG0 */ - fn = 0; - bit_base = 1; + hide_pci_dev(dev, 0, 1); break; case 4: /* Nothing to do */ case 5: continue; } - for (; fn >= 0; --fn) { - const struct device *const d = - pcidev_on_root(dev, fn); - if (!d || d->enabled) - continue; - const u32 deven = pci_read_config32(d0f0, D0F0_DEVEN); - pci_write_config32(d0f0, D0F0_DEVEN, - deven & ~(1 << (bit_base + fn))); - } }

const u32 deven = pci_read_config32(d0f0, D0F0_DEVEN);

Attachments:

attachment.htm (text/html — 6.1 KB)

Show replies by date

Angel Pons (Code Review)

27 Nov 27 Nov

10:45 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37291 )

Change subject: nb/intel/x4x: Factor out hiding PCI devs in pure fn ......................................................................

Patch Set 1: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/37291/1/src/northbridge/intel/x4x/n... File src/northbridge/intel/x4x/northbridge.c:

https://review.coreboot.org/c/coreboot/+/37291/1/src/northbridge/intel/x4x/n... PS1, Line 230: for (dev = 6; dev > 0; dev--) { I'm not even sure if a loop is even useful at this point. This is actually a sequence of four instructions:

hide_pci_dev(6, 0, 13); hide_pci_dev(3, 3, 6); hide_pci_dev(2, 1, 3); hide_pci_dev(1, 0, 1);

-- To view, visit https://review.coreboot.org/c/coreboot/+/37291 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64db2460115f5fb35ca197b83440f8ee47470761 Gerrit-Change-Number: 37291 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Edward O'Callaghan quasisec@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 27 Nov 2019 21:45:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Arthur Heymans (Code Review)

10:49 p.m.

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37291 )

Change subject: nb/intel/x4x: Factor out hiding PCI devs in pure fn ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37291/1/src/northbridge/intel/x4x/n... File src/northbridge/intel/x4x/northbridge.c:

https://review.coreboot.org/c/coreboot/+/37291/1/src/northbridge/intel/x4x/n... PS1, Line 230: for (dev = 6; dev > 0; dev--) {

...

I'm not even sure if a loop is even useful at this point. This is actually a sequence of four instructions:

hide_pci_dev(6, 0, 13); hide_pci_dev(3, 3, 6); hide_pci_dev(2, 1, 3); hide_pci_dev(1, 0, 1);

Thx.

-- To view, visit https://review.coreboot.org/c/coreboot/+/37291 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64db2460115f5fb35ca197b83440f8ee47470761 Gerrit-Change-Number: 37291 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Edward O'Callaghan quasisec@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 27 Nov 2019 21:49:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Arthur Heymans (Code Review)

28 Nov 28 Nov

12:29 a.m.

Hello Patrick Rudolph, Edward O'Callaghan, Angel Pons, build bot (Jenkins), Damien Zammit,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37291

to look at the new patch set (#2).

Change subject: nb/intel/x4x: Factor out hiding PCI devs in pure fn ......................................................................

nb/intel/x4x: Factor out hiding PCI devs in pure fn

This increases readability.

Also change the update expression. '--variable' does not make much sense there.

Change-Id: I64db2460115f5fb35ca197b83440f8ee47470761 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/northbridge/intel/x4x/northbridge.c 1 file changed, 21 insertions(+), 35 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37291/2

build bot (Jenkins) (Code Review)

12:30 a.m.

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37291 )

Change subject: nb/intel/x4x: Factor out hiding PCI devs in pure fn ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37291/2/src/northbridge/intel/x4x/n... File src/northbridge/intel/x4x/northbridge.c:

https://review.coreboot.org/c/coreboot/+/37291/2/src/northbridge/intel/x4x/n... PS2, Line 229: /* Hide internal functions based on devicetree info. */ trailing whitespace

-- To view, visit https://review.coreboot.org/c/coreboot/+/37291 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64db2460115f5fb35ca197b83440f8ee47470761 Gerrit-Change-Number: 37291 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Edward O'Callaghan quasisec@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 27 Nov 2019 23:30:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Arthur Heymans (Code Review)

12:30 a.m.

Hello Patrick Rudolph, Edward O'Callaghan, Angel Pons, build bot (Jenkins), Damien Zammit,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37291

to look at the new patch set (#3).

Change subject: nb/intel/x4x: Factor out hiding PCI devs in pure fn ......................................................................

nb/intel/x4x: Factor out hiding PCI devs in pure fn

This increases readability.

Also change the update expression. '--variable' does not make much sense there.

Change-Id: I64db2460115f5fb35ca197b83440f8ee47470761 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/northbridge/intel/x4x/northbridge.c 1 file changed, 20 insertions(+), 34 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37291/3

Edward O'Callaghan (Code Review)

2:18 a.m.

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37291 )

Change subject: nb/intel/x4x: Factor out hiding PCI devs in pure fn ......................................................................

Patch Set 3: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/37291 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64db2460115f5fb35ca197b83440f8ee47470761 Gerrit-Change-Number: 37291 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Edward O'Callaghan quasisec@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 28 Nov 2019 01:18:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Arthur Heymans (Code Review)

9:33 a.m.

Hello Patrick Rudolph, Edward O'Callaghan, Edward O'Callaghan, Angel Pons, build bot (Jenkins), Damien Zammit,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37291

to look at the new patch set (#4).

Change subject: nb/intel/x4x: Factor out hiding PCI devs in pure fn ......................................................................

nb/intel/x4x: Factor out hiding PCI devs in pure fn

This increases readability.

Also change the update expression. '--variable' does not make much sense there.

Change-Id: I64db2460115f5fb35ca197b83440f8ee47470761 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/northbridge/intel/x4x/northbridge.c 1 file changed, 19 insertions(+), 34 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37291/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/37291 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64db2460115f5fb35ca197b83440f8ee47470761 Gerrit-Change-Number: 37291 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Edward O'Callaghan quasisec@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Edward O'Callaghan (Code Review)

10:59 a.m.

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37291 )

Change subject: nb/intel/x4x: Factor out hiding PCI devs in pure fn ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/37291 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64db2460115f5fb35ca197b83440f8ee47470761 Gerrit-Change-Number: 37291 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Edward O'Callaghan quasisec@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Thu, 28 Nov 2019 09:59:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Angel Pons (Code Review)

1 Dec 1 Dec

11:15 a.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37291 )

Change subject: nb/intel/x4x: Factor out hiding PCI devs in pure fn ......................................................................

Patch Set 4: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/37291/1/src/northbridge/intel/x4x/n... File src/northbridge/intel/x4x/northbridge.c:

https://review.coreboot.org/c/coreboot/+/37291/1/src/northbridge/intel/x4x/n... PS1, Line 230: for (dev = 6; dev > 0; dev--) {

...

...
I'm not even sure if a loop is even useful at this point. […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/37291 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64db2460115f5fb35ca197b83440f8ee47470761 Gerrit-Change-Number: 37291 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Edward O'Callaghan quasisec@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 01 Dec 2019 10:15:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Comment-In-Reply-To: Arthur Heymans arthur@aheymans.xyz Gerrit-MessageType: comment

Arthur Heymans (Code Review)

1:39 p.m.

Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37291 )

Change subject: nb/intel/x4x: Factor out hiding PCI devs in pure fn ......................................................................

nb/intel/x4x: Factor out hiding PCI devs in pure fn

This increases readability.

Also change the update expression. '--variable' does not make much sense there.

Change-Id: I64db2460115f5fb35ca197b83440f8ee47470761 Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/37291 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Edward O'Callaghan quasisec@chromium.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/x4x/northbridge.c 1 file changed, 19 insertions(+), 34 deletions(-)

Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved

diff --git a/src/northbridge/intel/x4x/northbridge.c b/src/northbridge/intel/x4x/northbridge.c index 39f24d3..252e14f 100644 --- a/src/northbridge/intel/x4x/northbridge.c +++ b/src/northbridge/intel/x4x/northbridge.c @@ -206,45 +206,30 @@ dev->ops = &cpu_bus_ops; }

+static void hide_pci_fn(const int dev_bit_base, const struct device *dev) +{ + if (!dev || dev->enabled) + return; + const unsigned int fn = PCI_FUNC(dev->path.pci.devfn); + const struct device *const d0f0 = pcidev_on_root(0, 0); + pci_update_config32(d0f0, D0F0_DEVEN, ~(1 << (dev_bit_base + fn)), 0); +} + +static void hide_pci_dev(const int dev, int functions, const int dev_bit_base) +{ + for (; functions >= 0; functions--) + hide_pci_fn(dev_bit_base, pcidev_on_root(dev, functions)); +} + static void x4x_init(void *const chip_info) { - int dev, fn, bit_base; - struct device *const d0f0 = pcidev_on_root(0x0, 0);

/* Hide internal functions based on devicetree info. */ - for (dev = 6; dev > 0; --dev) { - switch (dev) { - case 6: /* PEG1: only on P45 */ - fn = 0; - bit_base = 13; - break; - case 3: /* ME */ - fn = 3; - bit_base = 6; - break; - case 2: /* IGD */ - fn = 1; - bit_base = 3; - break; - case 1: /* PEG0 */ - fn = 0; - bit_base = 1; - break; - case 4: /* Nothing to do */ - case 5: - continue; - } - for (; fn >= 0; --fn) { - const struct device *const d = - pcidev_on_root(dev, fn); - if (!d || d->enabled) - continue; - const u32 deven = pci_read_config32(d0f0, D0F0_DEVEN); - pci_write_config32(d0f0, D0F0_DEVEN, - deven & ~(1 << (bit_base + fn))); - } - } + hide_pci_dev(6, 0, 13); /* PEG1: only on P45 */ + hide_pci_dev(3, 3, 6); /* ME */ + hide_pci_dev(2, 1, 3); /* IGD */ + hide_pci_dev(1, 0, 1); /* PEG0 */

const u32 deven = pci_read_config32(d0f0, D0F0_DEVEN); if (!(deven & (0xf << 6)))

-- To view, visit https://review.coreboot.org/c/coreboot/+/37291 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64db2460115f5fb35ca197b83440f8ee47470761 Gerrit-Change-Number: 37291 Gerrit-PatchSet: 5 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Edward O'Callaghan quasisec@chromium.org Gerrit-Reviewer: Edward O'Callaghan quasisec@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1811

days inactive

1815

days old

coreboot-gerrit@coreboot.org

10 comments

4 participants

tags (0)

participants (4)

Angel Pons (Code Review)
Arthur Heymans (Code Review)
build bot (Jenkins) (Code Review)
Edward O'Callaghan (Code Review)