Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35096 )
Change subject: devicetree: Fix improper use of chip_operations ......................................................................
devicetree: Fix improper use of chip_operations
Auto-discoverable PCI devices do not require field .enable_dev of chip_operations to be set. They are matched with PCI drivers by the use of PCI vendor and device ID fields.
The name given for the chip_operations struct must match the pathname the way it is present in the devicetree.cb files. If there was no match, util/sconfig would currently choose to use the empty weak declaration it creates in static.c file.
Change-Id: I684a087a1f8ee4e1a5fd83450cd371fcfdbb6847 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/generic/bayhub/bh720.c M src/drivers/ricoh/rce822/rce822.c M src/soc/cavium/common/pci/uart.c M src/southbridge/intel/i82870/pcibridge.c M src/southbridge/ti/pci7420/cardbus.c M src/southbridge/ti/pci7420/firewire.c 6 files changed, 13 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/35096/1
diff --git a/src/drivers/generic/bayhub/bh720.c b/src/drivers/generic/bayhub/bh720.c index 09fa578..b365132 100644 --- a/src/drivers/generic/bayhub/bh720.c +++ b/src/drivers/generic/bayhub/bh720.c @@ -82,12 +82,6 @@ .devices = pci_device_ids, };
-static void bh720_enable(struct device *dev) -{ - dev->ops = &bh720_ops; -} - -struct chip_operations bayhub_bh720_ops = { +struct chip_operations drivers_generic_bayhub_ops = { CHIP_NAME("BayHub Technology BH720 PCI to eMMC 5.0 HS200 bridge") - .enable_dev = bh720_enable, }; diff --git a/src/drivers/ricoh/rce822/rce822.c b/src/drivers/ricoh/rce822/rce822.c index 72862f5..485bdd0 100644 --- a/src/drivers/ricoh/rce822/rce822.c +++ b/src/drivers/ricoh/rce822/rce822.c @@ -71,3 +71,7 @@ .vendor = PCI_VENDOR_ID_RICOH, .devices = pci_device_ids, }; + +struct chip_operations drivers_ricoh_rce822_ops = { + CHIP_NAME("RICOH RCE822") +}; diff --git a/src/soc/cavium/common/pci/uart.c b/src/soc/cavium/common/pci/uart.c index 6e41e1d..efa1b37 100644 --- a/src/soc/cavium/common/pci/uart.c +++ b/src/soc/cavium/common/pci/uart.c @@ -38,3 +38,7 @@ .vendor = PCI_VENDOR_CAVIUM, .device = PCI_DEVICE_ID_CAVIUM_THUNDERX_UART, }; + +struct chip_operations soc_cavium_common_pci_ops = { + CHIP_NAME("Cavium ThunderX") +}; diff --git a/src/southbridge/intel/i82870/pcibridge.c b/src/southbridge/intel/i82870/pcibridge.c index a375568..cb6ace8 100644 --- a/src/southbridge/intel/i82870/pcibridge.c +++ b/src/southbridge/intel/i82870/pcibridge.c @@ -47,3 +47,7 @@ .vendor = PCI_VENDOR_ID_INTEL, .device = PCI_DEVICE_ID_INTEL_82870_1F0, }; + +struct chip_operations southbridge_intel_i82870_ops = { + CHIP_NAME("Intel 82870") +}; diff --git a/src/southbridge/ti/pci7420/cardbus.c b/src/southbridge/ti/pci7420/cardbus.c index 2c5679b..44a38ea 100644 --- a/src/southbridge/ti/pci7420/cardbus.c +++ b/src/southbridge/ti/pci7420/cardbus.c @@ -111,12 +111,6 @@ .device = 0xac8d, };
-static void ti_pci7420_enable_dev(struct device *dev) -{ - /* Nothing here yet */ -} - struct chip_operations southbridge_ti_pci7420_ops = { CHIP_NAME("Texas Instruments PCI7420/7620 Cardbus Controller") - .enable_dev = ti_pci7420_enable_dev, }; diff --git a/src/southbridge/ti/pci7420/firewire.c b/src/southbridge/ti/pci7420/firewire.c index 4058540..dac273a 100644 --- a/src/southbridge/ti/pci7420/firewire.c +++ b/src/southbridge/ti/pci7420/firewire.c @@ -48,13 +48,3 @@ .vendor = 0x104c, .device = 0x802e, }; - -static void ti_pci7420_firewire_enable_dev(struct device *dev) -{ - /* Nothing here yet */ -} - -struct chip_operations southbridge_ti_pci7420_firewire_ops = { - CHIP_NAME("Texas Instruments PCI7420/7620 FireWire (IEEE 1394)") - .enable_dev = ti_pci7420_firewire_enable_dev, -};
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35096 )
Change subject: devicetree: Fix improper use of chip_operations ......................................................................
Patch Set 1: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/35096/1/src/soc/cavium/common/pci/u... File src/soc/cavium/common/pci/uart.c:
https://review.coreboot.org/c/coreboot/+/35096/1/src/soc/cavium/common/pci/u... PS1, Line 43: Cavium ThunderX Minor: maybe use "Cavium ThunderX UART" ?
https://review.coreboot.org/c/coreboot/+/35096/1/src/southbridge/ti/pci7420/... File src/southbridge/ti/pci7420/firewire.c:
https://review.coreboot.org/c/coreboot/+/35096/1/src/southbridge/ti/pci7420/... PS1, Line 58: Shouldn't this be kept, like the one on cardbus?
Hello Patrick Rudolph, Angel Pons, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35096
to look at the new patch set (#3).
Change subject: devicetree: Fix improper use of chip_operations ......................................................................
devicetree: Fix improper use of chip_operations
Auto-discoverable PCI devices do not require field .enable_dev of chip_operations to be set. They are matched with PCI drivers by the use of PCI vendor and device ID fields.
The name given for the chip_operations struct must match the pathname the way it is present in the devicetree.cb files. If there was no match, util/sconfig would currently choose to use the empty weak declaration it creates in static.c file.
Change-Id: I684a087a1f8ee4e1a5fd83450cd371fcfdbb6847 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/generic/bayhub/bh720.c M src/drivers/ricoh/rce822/rce822.c M src/soc/cavium/common/pci/uart.c M src/southbridge/intel/i82870/pcibridge.c M src/southbridge/ti/pci7420/cardbus.c M src/southbridge/ti/pci7420/firewire.c 6 files changed, 13 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/35096/3
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35096 )
Change subject: devicetree: Fix improper use of chip_operations ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35096/1/src/soc/cavium/common/pci/u... File src/soc/cavium/common/pci/uart.c:
https://review.coreboot.org/c/coreboot/+/35096/1/src/soc/cavium/common/pci/u... PS1, Line 43: Cavium ThunderX
Minor: maybe use "Cavium ThunderX UART" ?
Done
https://review.coreboot.org/c/coreboot/+/35096/1/src/southbridge/ti/pci7420/... File src/southbridge/ti/pci7420/firewire.c:
https://review.coreboot.org/c/coreboot/+/35096/1/src/southbridge/ti/pci7420/... PS1, Line 58: CHIP_NAME("Texas Instruments PCI7420/7620 FireWire (IEEE 1394)")
Shouldn't this be kept, like the one on cardbus?
There can be only one chip_operations defined per directory. This one is not linked in, see naming requirements.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35096 )
Change subject: devicetree: Fix improper use of chip_operations ......................................................................
Patch Set 3:
Marhshall, Richard; bayhub change touches google/kahlee.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35096 )
Change subject: devicetree: Fix improper use of chip_operations ......................................................................
Patch Set 3: Code-Review+2
Thanks for the heads-up.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35096 )
Change subject: devicetree: Fix improper use of chip_operations ......................................................................
devicetree: Fix improper use of chip_operations
Auto-discoverable PCI devices do not require field .enable_dev of chip_operations to be set. They are matched with PCI drivers by the use of PCI vendor and device ID fields.
The name given for the chip_operations struct must match the pathname the way it is present in the devicetree.cb files. If there was no match, util/sconfig would currently choose to use the empty weak declaration it creates in static.c file.
Change-Id: I684a087a1f8ee4e1a5fd83450cd371fcfdbb6847 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35096 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/drivers/generic/bayhub/bh720.c M src/drivers/ricoh/rce822/rce822.c M src/soc/cavium/common/pci/uart.c M src/southbridge/intel/i82870/pcibridge.c M src/southbridge/ti/pci7420/cardbus.c M src/southbridge/ti/pci7420/firewire.c 6 files changed, 13 insertions(+), 23 deletions(-)
Approvals: build bot (Jenkins): Verified Marshall Dawson: Looks good to me, approved
diff --git a/src/drivers/generic/bayhub/bh720.c b/src/drivers/generic/bayhub/bh720.c index 09fa578..b365132 100644 --- a/src/drivers/generic/bayhub/bh720.c +++ b/src/drivers/generic/bayhub/bh720.c @@ -82,12 +82,6 @@ .devices = pci_device_ids, };
-static void bh720_enable(struct device *dev) -{ - dev->ops = &bh720_ops; -} - -struct chip_operations bayhub_bh720_ops = { +struct chip_operations drivers_generic_bayhub_ops = { CHIP_NAME("BayHub Technology BH720 PCI to eMMC 5.0 HS200 bridge") - .enable_dev = bh720_enable, }; diff --git a/src/drivers/ricoh/rce822/rce822.c b/src/drivers/ricoh/rce822/rce822.c index 72862f5..485bdd0 100644 --- a/src/drivers/ricoh/rce822/rce822.c +++ b/src/drivers/ricoh/rce822/rce822.c @@ -71,3 +71,7 @@ .vendor = PCI_VENDOR_ID_RICOH, .devices = pci_device_ids, }; + +struct chip_operations drivers_ricoh_rce822_ops = { + CHIP_NAME("RICOH RCE822") +}; diff --git a/src/soc/cavium/common/pci/uart.c b/src/soc/cavium/common/pci/uart.c index 6e41e1d..ff002ea 100644 --- a/src/soc/cavium/common/pci/uart.c +++ b/src/soc/cavium/common/pci/uart.c @@ -38,3 +38,7 @@ .vendor = PCI_VENDOR_CAVIUM, .device = PCI_DEVICE_ID_CAVIUM_THUNDERX_UART, }; + +struct chip_operations soc_cavium_common_pci_ops = { + CHIP_NAME("Cavium ThunderX UART") +}; diff --git a/src/southbridge/intel/i82870/pcibridge.c b/src/southbridge/intel/i82870/pcibridge.c index a375568..cb6ace8 100644 --- a/src/southbridge/intel/i82870/pcibridge.c +++ b/src/southbridge/intel/i82870/pcibridge.c @@ -47,3 +47,7 @@ .vendor = PCI_VENDOR_ID_INTEL, .device = PCI_DEVICE_ID_INTEL_82870_1F0, }; + +struct chip_operations southbridge_intel_i82870_ops = { + CHIP_NAME("Intel 82870") +}; diff --git a/src/southbridge/ti/pci7420/cardbus.c b/src/southbridge/ti/pci7420/cardbus.c index 2c5679b..44a38ea 100644 --- a/src/southbridge/ti/pci7420/cardbus.c +++ b/src/southbridge/ti/pci7420/cardbus.c @@ -111,12 +111,6 @@ .device = 0xac8d, };
-static void ti_pci7420_enable_dev(struct device *dev) -{ - /* Nothing here yet */ -} - struct chip_operations southbridge_ti_pci7420_ops = { CHIP_NAME("Texas Instruments PCI7420/7620 Cardbus Controller") - .enable_dev = ti_pci7420_enable_dev, }; diff --git a/src/southbridge/ti/pci7420/firewire.c b/src/southbridge/ti/pci7420/firewire.c index 4058540..dac273a 100644 --- a/src/southbridge/ti/pci7420/firewire.c +++ b/src/southbridge/ti/pci7420/firewire.c @@ -48,13 +48,3 @@ .vendor = 0x104c, .device = 0x802e, }; - -static void ti_pci7420_firewire_enable_dev(struct device *dev) -{ - /* Nothing here yet */ -} - -struct chip_operations southbridge_ti_pci7420_firewire_ops = { - CHIP_NAME("Texas Instruments PCI7420/7620 FireWire (IEEE 1394)") - .enable_dev = ti_pci7420_firewire_enable_dev, -};
-
Angel Pons (Code Review) -
Kyösti Mälkki (Code Review)
-
Marshall Dawson (Code Review)
-
Patrick Georgi (Code Review)