Hi Bin,
On 29 June 2015 at 03:21, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, Jun 26, 2015 at 1:55 AM, Simon Glass sjg@chromium.org wrote:
This driver should use the x86 PCI configuration functions. Also adjust its compatible string to something generic (i.e. without a vendor name).
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Rename the ops and ids arrays for consistency
- Drop the coreboot PCI driver which is no-longer needed
arch/x86/cpu/coreboot/pci.c | 21 --------------------- drivers/pci/pci_x86.c | 13 ++++++++----- 2 files changed, 8 insertions(+), 26 deletions(-)
diff --git a/arch/x86/cpu/coreboot/pci.c b/arch/x86/cpu/coreboot/pci.c index 67eb14c..af9f391 100644 --- a/arch/x86/cpu/coreboot/pci.c +++ b/arch/x86/cpu/coreboot/pci.c @@ -11,30 +11,9 @@
#include <common.h> #include <dm.h> -#include <errno.h> #include <pci.h> -#include <asm/io.h> #include <asm/pci.h>
Should <pci.h> and <asm/pci.h> be removed too?
OK.
-DECLARE_GLOBAL_DATA_PTR;
-static const struct dm_pci_ops pci_x86_ops = {
.read_config = pci_x86_read_config,.write_config = pci_x86_write_config,-};
-static const struct udevice_id pci_x86_ids[] = {
{ .compatible = "pci-x86" },{ }-};
-U_BOOT_DRIVER(pci_x86_drv) = {
.name = "pci_x86",.id = UCLASS_PCI,.of_match = pci_x86_ids,.ops = &pci_x86_ops,-};
static const struct udevice_id generic_pch_ids[] = { { .compatible = "intel,pch" }, { }
What's this? Is this required by some board?
Yes, it is needed for ivybridge (when booted from coreboot). The PCH contains the SPI and LPC peripherals so we need a driver to make those visible.
diff --git a/drivers/pci/pci_x86.c b/drivers/pci/pci_x86.c index 901bdca..89e8c11 100644 --- a/drivers/pci/pci_x86.c +++ b/drivers/pci/pci_x86.c @@ -7,18 +7,21 @@ #include <common.h> #include <dm.h> #include <pci.h> +#include <asm/pci.h>
-static const struct dm_pci_ops x86_pci_ops = { +static const struct dm_pci_ops pci_x86_ops = {
.read_config = pci_x86_read_config,.write_config = pci_x86_write_config,};
As I mentioned in previous discussion, I still think it's better to move the implementation of pci_x86_read_config() and pci_x86_write_config() from arch/x86/cpu/pci.c to this file (drivers/pci/pci_x86.c). I don't think calling arch/x86/cpu/ivybridge/pci.c would be a problem. The arch/x86/cpu/pci.c provides generic x86 pci config access methods which can be utilized by some variants of x86 pci controller (like ivybridge). And I also looked the arch/x86/cpu/ivybridge/pci.c driver and just wonder why should we create that ivybridge-specific driver? Can we move pci_ivybridge_probe() from that file to arch/x86/cpu/ivybridge/bd82x6x.c? This way, we no longer need the ivybridge-specific pci driver as x86 pci controller should be generic enough.
But pci_ivybridge_probe() calls the code in bd82x6x.c.
With ivybridge we attach all the device init to the PCI probe function. It should be possible to move more of this to driver model, but not in this patch.
-static const struct udevice_id x86_pci_ids[] = {
{ .compatible = "x86,pci" },+static const struct udevice_id pci_x86_ids[] = {
{ .compatible = "pci-x86" }, { }};
U_BOOT_DRIVER(pci_x86) = { .name = "pci_x86", .id = UCLASS_PCI,
.of_match = x86_pci_ids,.ops = &x86_pci_ops,
.of_match = pci_x86_ids,.ops = &pci_x86_ops,};
Regards, Simon