Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35673 )
Change subject: sb/intel/bd8x62x,i82801gx: Fix PCI bridge subsystem IDs ......................................................................
sb/intel/bd8x62x,i82801gx: Fix PCI bridge subsystem IDs
Implementation of ich_pci_dev_enable_resources() used to have a custom implementation to program PCI subsystem IDs for the (legacy) PCI bus bridge.
With the local implementation removed, we no longer need the custom .enable_resources callback.
Change-Id: I6f73fd0e4d5a1829d1555455c9a143f1d18a6116 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/southbridge/intel/bd82x6x/pci.c M src/southbridge/intel/i82801gx/pci.c 2 files changed, 2 insertions(+), 58 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/35673/1
diff --git a/src/southbridge/intel/bd82x6x/pci.c b/src/southbridge/intel/bd82x6x/pci.c index 6391de4..a222893 100644 --- a/src/southbridge/intel/bd82x6x/pci.c +++ b/src/southbridge/intel/bd82x6x/pci.c @@ -57,34 +57,6 @@ pci_write_config16(dev, SECSTS, reg16); }
-static void ich_pci_dev_enable_resources(struct device *dev) -{ - uint16_t command; - - command = pci_read_config16(dev, PCI_COMMAND); - command |= dev->command; - printk(BIOS_DEBUG, "%s cmd <- %02x\n", dev_path(dev), command); - pci_write_config16(dev, PCI_COMMAND, command); -} - -static void ich_pci_bus_enable_resources(struct device *dev) -{ - uint16_t ctrl; - /* enable IO in command register if there is VGA card - * connected with (even it does not claim IO resource) - */ - if (dev->link_list->bridge_ctrl & PCI_BRIDGE_CTL_VGA) - dev->command |= PCI_COMMAND_IO; - ctrl = pci_read_config16(dev, PCI_BRIDGE_CONTROL); - ctrl |= dev->link_list->bridge_ctrl; - ctrl |= (PCI_BRIDGE_CTL_PARITY | PCI_BRIDGE_CTL_SERR); /* error check */ - printk(BIOS_DEBUG, "%s bridge ctrl <- %04x\n", dev_path(dev), ctrl); - pci_write_config16(dev, PCI_BRIDGE_CONTROL, ctrl); - - /* This is the reason we need our own pci_bus_enable_resources */ - ich_pci_dev_enable_resources(dev); -} - static struct pci_operations pci_ops = { .set_subsystem = pci_dev_set_subsystem, }; @@ -92,7 +64,7 @@ static struct device_operations device_ops = { .read_resources = pci_bus_read_resources, .set_resources = pci_dev_set_resources, - .enable_resources = ich_pci_bus_enable_resources, + .enable_resources = pci_bus_enable_resources, .init = pci_init, .scan_bus = pci_scan_bridge, .ops_pci = &pci_ops, diff --git a/src/southbridge/intel/i82801gx/pci.c b/src/southbridge/intel/i82801gx/pci.c index 1d1f902..4d98e89 100644 --- a/src/southbridge/intel/i82801gx/pci.c +++ b/src/southbridge/intel/i82801gx/pci.c @@ -56,34 +56,6 @@ pci_write_config16(dev, SECSTS, reg16); }
-static void ich_pci_dev_enable_resources(struct device *dev) -{ - uint16_t command; - - command = pci_read_config16(dev, PCI_COMMAND); - command |= dev->command; - printk(BIOS_DEBUG, "%s cmd <- %02x\n", dev_path(dev), command); - pci_write_config16(dev, PCI_COMMAND, command); -} - -static void ich_pci_bus_enable_resources(struct device *dev) -{ - uint16_t ctrl; - /* enable IO in command register if there is VGA card - * connected with (even it does not claim IO resource) - */ - if (dev->link_list->bridge_ctrl & PCI_BRIDGE_CTL_VGA) - dev->command |= PCI_COMMAND_IO; - ctrl = pci_read_config16(dev, PCI_BRIDGE_CONTROL); - ctrl |= dev->link_list->bridge_ctrl; - ctrl |= (PCI_BRIDGE_CTL_PARITY | PCI_BRIDGE_CTL_SERR); /* error check */ - printk(BIOS_DEBUG, "%s bridge ctrl <- %04x\n", dev_path(dev), ctrl); - pci_write_config16(dev, PCI_BRIDGE_CONTROL, ctrl); - - /* This is the reason we need our own pci_bus_enable_resources */ - ich_pci_dev_enable_resources(dev); -} - static struct pci_operations pci_ops = { .set_subsystem = pci_dev_set_subsystem, }; @@ -91,7 +63,7 @@ static struct device_operations device_ops = { .read_resources = pci_bus_read_resources, .set_resources = pci_dev_set_resources, - .enable_resources = ich_pci_bus_enable_resources, + .enable_resources = pci_bus_enable_resources, .init = pci_init, .scan_bus = pci_scan_bridge, .ops_pci = &pci_ops,
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35673 )
Change subject: sb/intel/bd8x62x,i82801gx: Fix PCI bridge subsystem IDs ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/35673/1/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/pci.c:
https://review.coreboot.org/c/coreboot/+/35673/1/src/southbridge/intel/bd82x... PS1, Line 76: 0x2448 offtopic. This can be placed in a common place?
Petr Cvek has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35673 )
Change subject: sb/intel/bd8x62x,i82801gx: Fix PCI bridge subsystem IDs ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
I've tested the change to the usage of the standard pci_bus_enable_resources(), works on kontron 986lcd-m.
https://review.coreboot.org/c/coreboot/+/35673/1/src/southbridge/intel/i8280... File src/southbridge/intel/i82801gx/pci.c:
https://review.coreboot.org/c/coreboot/+/35673/1/src/southbridge/intel/i8280... PS1, Line 66: pci_bus_enable_resources This part of the patch works fine, tested on kontron 986lcd-m
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35673 )
Change subject: sb/intel/bd8x62x,i82801gx: Fix PCI bridge subsystem IDs ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35673/1/src/southbridge/intel/i8280... File src/southbridge/intel/i82801gx/pci.c:
https://review.coreboot.org/c/coreboot/+/35673/1/src/southbridge/intel/i8280... PS1, Line 66: pci_bus_enable_resources
This part of the patch works fine, tested on kontron 986lcd-m
Ack
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35673 )
Change subject: sb/intel/bd8x62x,i82801gx: Fix PCI bridge subsystem IDs ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35673/1/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/pci.c:
https://review.coreboot.org/c/coreboot/+/35673/1/src/southbridge/intel/bd82x... PS1, Line 76: 0x2448
offtopic. […]
Probably yes.
Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35673 )
Change subject: sb/intel/bd8x62x,i82801gx: Fix PCI bridge subsystem IDs ......................................................................
sb/intel/bd8x62x,i82801gx: Fix PCI bridge subsystem IDs
Implementation of ich_pci_dev_enable_resources() used to have a custom implementation to program PCI subsystem IDs for the (legacy) PCI bus bridge.
With the local implementation removed, we no longer need the custom .enable_resources callback.
Change-Id: I6f73fd0e4d5a1829d1555455c9a143f1d18a6116 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35673 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Arthur Heymans arthur@aheymans.xyz Reviewed-by: Petr Cvek petrcvekcz@gmail.com --- M src/southbridge/intel/bd82x6x/pci.c M src/southbridge/intel/i82801gx/pci.c 2 files changed, 2 insertions(+), 58 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Petr Cvek: Looks good to me, but someone else must approve
diff --git a/src/southbridge/intel/bd82x6x/pci.c b/src/southbridge/intel/bd82x6x/pci.c index 6391de4..a222893 100644 --- a/src/southbridge/intel/bd82x6x/pci.c +++ b/src/southbridge/intel/bd82x6x/pci.c @@ -57,34 +57,6 @@ pci_write_config16(dev, SECSTS, reg16); }
-static void ich_pci_dev_enable_resources(struct device *dev) -{ - uint16_t command; - - command = pci_read_config16(dev, PCI_COMMAND); - command |= dev->command; - printk(BIOS_DEBUG, "%s cmd <- %02x\n", dev_path(dev), command); - pci_write_config16(dev, PCI_COMMAND, command); -} - -static void ich_pci_bus_enable_resources(struct device *dev) -{ - uint16_t ctrl; - /* enable IO in command register if there is VGA card - * connected with (even it does not claim IO resource) - */ - if (dev->link_list->bridge_ctrl & PCI_BRIDGE_CTL_VGA) - dev->command |= PCI_COMMAND_IO; - ctrl = pci_read_config16(dev, PCI_BRIDGE_CONTROL); - ctrl |= dev->link_list->bridge_ctrl; - ctrl |= (PCI_BRIDGE_CTL_PARITY | PCI_BRIDGE_CTL_SERR); /* error check */ - printk(BIOS_DEBUG, "%s bridge ctrl <- %04x\n", dev_path(dev), ctrl); - pci_write_config16(dev, PCI_BRIDGE_CONTROL, ctrl); - - /* This is the reason we need our own pci_bus_enable_resources */ - ich_pci_dev_enable_resources(dev); -} - static struct pci_operations pci_ops = { .set_subsystem = pci_dev_set_subsystem, }; @@ -92,7 +64,7 @@ static struct device_operations device_ops = { .read_resources = pci_bus_read_resources, .set_resources = pci_dev_set_resources, - .enable_resources = ich_pci_bus_enable_resources, + .enable_resources = pci_bus_enable_resources, .init = pci_init, .scan_bus = pci_scan_bridge, .ops_pci = &pci_ops, diff --git a/src/southbridge/intel/i82801gx/pci.c b/src/southbridge/intel/i82801gx/pci.c index 1d1f902..4d98e89 100644 --- a/src/southbridge/intel/i82801gx/pci.c +++ b/src/southbridge/intel/i82801gx/pci.c @@ -56,34 +56,6 @@ pci_write_config16(dev, SECSTS, reg16); }
-static void ich_pci_dev_enable_resources(struct device *dev) -{ - uint16_t command; - - command = pci_read_config16(dev, PCI_COMMAND); - command |= dev->command; - printk(BIOS_DEBUG, "%s cmd <- %02x\n", dev_path(dev), command); - pci_write_config16(dev, PCI_COMMAND, command); -} - -static void ich_pci_bus_enable_resources(struct device *dev) -{ - uint16_t ctrl; - /* enable IO in command register if there is VGA card - * connected with (even it does not claim IO resource) - */ - if (dev->link_list->bridge_ctrl & PCI_BRIDGE_CTL_VGA) - dev->command |= PCI_COMMAND_IO; - ctrl = pci_read_config16(dev, PCI_BRIDGE_CONTROL); - ctrl |= dev->link_list->bridge_ctrl; - ctrl |= (PCI_BRIDGE_CTL_PARITY | PCI_BRIDGE_CTL_SERR); /* error check */ - printk(BIOS_DEBUG, "%s bridge ctrl <- %04x\n", dev_path(dev), ctrl); - pci_write_config16(dev, PCI_BRIDGE_CONTROL, ctrl); - - /* This is the reason we need our own pci_bus_enable_resources */ - ich_pci_dev_enable_resources(dev); -} - static struct pci_operations pci_ops = { .set_subsystem = pci_dev_set_subsystem, }; @@ -91,7 +63,7 @@ static struct device_operations device_ops = { .read_resources = pci_bus_read_resources, .set_resources = pci_dev_set_resources, - .enable_resources = ich_pci_bus_enable_resources, + .enable_resources = pci_bus_enable_resources, .init = pci_init, .scan_bus = pci_scan_bridge, .ops_pci = &pci_ops,
-
Arthur Heymans (Code Review) -
Kyösti Mälkki (Code Review)
-
Petr Cvek (Code Review)