[U-Boot] [PATCH v2 5/7] x86: pci: Tidy up the generic x86 PCI driver

[Bin Meng]

Hi Simon,

On Fri, Jun 26, 2015 at 1:55 AM, Simon Glass <sjg at 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 at 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?

>
> -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?

> 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.

>
> -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,
Bin