On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

We don't want people changing the legacy PCI files while migration is in progress. Update the file headers to indicate that.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/pci/pci.c | 5 ++++- drivers/pci/pci_auto_old.c | 9 ++++++--- 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 645ecd4..4619089 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -9,7 +9,10 @@ */

/*

• • PCI routines

• • Old PCI routines
• • Do not change this file. Instead, convert your board to use CONFIG_DM_PCI
• • and change pci-uclass.c.
  */

#include <common.h> diff --git a/drivers/pci/pci_auto_old.c b/drivers/pci/pci_auto_old.c index 932eab8..1ff88b1 100644 --- a/drivers/pci/pci_auto_old.c +++ b/drivers/pci/pci_auto_old.c @@ -1,7 +1,5 @@ /*

• • arch/powerpc/kernel/pci_auto.c
• • PCI autoconfiguration library

• • PCI autoconfiguration library (legacy version, do not change)
  • Author: Matt Porter mporter@mvista.com

@@ -14,6 +12,11 @@ #include <errno.h> #include <pci.h>

+/*

• • Do not change this file. Instead, convert your board to use CONFIG_DM_PCI
• • and change pci_auto.c.
• */

/* the user can define CONFIG_SYS_PCI_CACHE_LINE_SIZE to avoid problems */ #ifndef CONFIG_SYS_PCI_CACHE_LINE_SIZE

#define CONFIG_SYS_PCI_CACHE_LINE_SIZE 8

Reviewed-by: Bin Meng bmeng.cn@gmail.com

Hi Simon,

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

At present we are using legacy functions even in the auto-configuration code used by driver model. Add a new pci_auto.c version which uses the correct API.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/pci/Makefile | 6 +- drivers/pci/pci-uclass.c | 18 +-- drivers/pci/pci_auto.c | 387 +++++++++++++++++++++++++++++++++++++++++++++ drivers/pci/pci_auto_old.c | 45 ------ include/pci.h | 9 +- 5 files changed, 402 insertions(+), 63 deletions(-) create mode 100644 drivers/pci/pci_auto.c

diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 6b761b4..62232fc 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -6,15 +6,15 @@ #

ifneq ($(CONFIG_DM_PCI),) -obj-$(CONFIG_PCI) += pci-uclass.o +obj-$(CONFIG_PCI) += pci-uclass.o pci_auto.o obj-$(CONFIG_DM_PCI_COMPAT) += pci_compat.o obj-$(CONFIG_PCI_SANDBOX) += pci_sandbox.o obj-$(CONFIG_SANDBOX) += pci-emul-uclass.o obj-$(CONFIG_X86) += pci_x86.o else -obj-$(CONFIG_PCI) += pci.o +obj-$(CONFIG_PCI) += pci.o pci_auto_old.o endif -obj-$(CONFIG_PCI) += pci_auto_common.o pci_auto_old.o pci_common.o pci_rom.o +obj-$(CONFIG_PCI) += pci_auto_common.o pci_common.o pci_rom.o

obj-$(CONFIG_FSL_PCI_INIT) += fsl_pci_init.o obj-$(CONFIG_PCI_INDIRECT_BRIDGE) += pci_indirect.o diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 5fe3072..8087e2d 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -403,7 +403,7 @@ int pci_auto_config_devices(struct udevice *bus) int ret;

            debug("%s: device %s\n", __func__, dev->name);

•           ret = pciauto_config_device(hose, pci_get_bdf(dev));
  

•           ret = dm_pciauto_config_device(dev);
            if (ret < 0)
                    return ret;
            max_bus = ret;
  

@@ -418,26 +418,16 @@ int pci_auto_config_devices(struct udevice *bus) return sub_bus; }

-int dm_pci_hose_probe_bus(struct pci_controller *hose, pci_dev_t bdf) +int dm_pci_hose_probe_bus(struct udevice *bus) {

•   struct udevice *parent, *bus;
    int sub_bus;
    int ret;
  
    debug("%s\n", __func__);
  

•   parent = hose->bus;
  

•   /* Find the bus within the parent */
  

•   ret = pci_bus_find_devfn(parent, PCI_MASK_BUS(bdf), &bus);
  

•   if (ret) {
  

•           debug("%s: Cannot find device %x on bus %s: %d\n", __func__,
  

•                 bdf, parent->name, ret);
  

•           return ret;
  

•   }
  
    sub_bus = pci_get_bus_max() + 1;
    debug("%s: bus = %d/%s\n", __func__, sub_bus, bus->name);
  

•   pciauto_prescan_setup_bridge(hose, bdf, sub_bus);
  

•   dm_pciauto_prescan_setup_bridge(bus, sub_bus);
  
    ret = device_probe(bus);
    if (ret) {
  

@@ -451,7 +441,7 @@ int dm_pci_hose_probe_bus(struct pci_controller *hose, pci_dev_t bdf) return -EPIPE; } sub_bus = pci_get_bus_max();

•   pciauto_postscan_setup_bridge(hose, bdf, sub_bus);
  

•   dm_pciauto_postscan_setup_bridge(bus, sub_bus);
  
    return sub_bus;
  

} diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c new file mode 100644 index 0000000..3dcbb4a --- /dev/null +++ b/drivers/pci/pci_auto.c @@ -0,0 +1,387 @@ +/*

• • arch/powerpc/kernel/pci_auto.c

This is wrong, and we can just remove this line.

• • PCI autoconfiguration library
• • Author: Matt Porter mporter@mvista.com
• • Copyright 2000 MontaVista Software Inc.
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <errno.h> +#include <pci.h>

+/* the user can define CONFIG_SYS_PCI_CACHE_LINE_SIZE to avoid problems */ +#ifndef CONFIG_SYS_PCI_CACHE_LINE_SIZE +#define CONFIG_SYS_PCI_CACHE_LINE_SIZE 8 +#endif

+void dm_pciauto_setup_device(struct udevice *dev, int bars_num,

•                        struct pci_region *mem,
  

•                        struct pci_region *prefetch, struct pci_region *io,
  

•                        bool enum_only)
  

+{

•   u32 bar_response;
  

•   pci_size_t bar_size;
  

•   u16 cmdstat = 0;
  

•   int bar, bar_nr = 0;
  

•   u8 header_type;
  

•   int rom_addr;
  

•   pci_addr_t bar_value;
  

•   struct pci_region *bar_res;
  

•   int found_mem64 = 0;
  

•   u16 class;
  

•   dm_pci_read_config16(dev, PCI_COMMAND, &cmdstat);
  

•   cmdstat = (cmdstat & ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) |
  

•                   PCI_COMMAND_MASTER;
  

•   for (bar = PCI_BASE_ADDRESS_0;
  

•           bar < PCI_BASE_ADDRESS_0 + (bars_num * 4); bar += 4) {
  

nits: indention of bar is wrong.

•           /* Tickle the BAR and get the response */
  

•           if (!enum_only)
  

•                   dm_pci_write_config32(dev, bar, 0xffffffff);
  

•           dm_pci_read_config32(dev, bar, &bar_response);
  

•           /* If BAR is not implemented go to the next BAR */
  

•           if (!bar_response)
  

•                   continue;
  

•           found_mem64 = 0;
  

•           /* Check the BAR type and set our address mask */
  

•           if (bar_response & PCI_BASE_ADDRESS_SPACE) {
  

•                   bar_size = ((~(bar_response & PCI_BASE_ADDRESS_IO_MASK))
  

•                              & 0xffff) + 1;
  

•                   if (!enum_only)
  

•                           bar_res = io;
  

•                   debug("PCI Autoconfig: BAR %d, I/O, size=0x%llx, ",
  

•                         bar_nr, (unsigned long long)bar_size);
  

•           } else {
  

•                   if ((bar_response & PCI_BASE_ADDRESS_MEM_TYPE_MASK) ==
  

•                        PCI_BASE_ADDRESS_MEM_TYPE_64) {
  

•                           u32 bar_response_upper;
  

•                           u64 bar64;
  

•                           if (!enum_only) {
  

•                                   dm_pci_write_config32(dev, bar + 4,
  

•                                                         0xffffffff);
  

•                           }
  

•                           dm_pci_read_config32(dev, bar + 4,
  

•                                                &bar_response_upper);
  

•                           bar64 = ((u64)bar_response_upper << 32) |
  

•                                           bar_response;
  

•                           bar_size = ~(bar64 & PCI_BASE_ADDRESS_MEM_MASK)
  

•                                           + 1;
  

•                           if (!enum_only)
  

•                                   found_mem64 = 1;
  

•                   } else {
  

•                           bar_size = (u32)(~(bar_response &
  

•                                           PCI_BASE_ADDRESS_MEM_MASK) + 1);
  

•                   }
  

•                   if (!enum_only) {
  

•                           if (prefetch && (bar_response &
  

•                                       PCI_BASE_ADDRESS_MEM_PREFETCH)) {
  

•                                   bar_res = prefetch;
  

•                           } else {
  

•                                   bar_res = mem;
  

•                           }
  

•                   }
  

•                   debug("PCI Autoconfig: BAR %d, %s, size=0x%llx, ",
  

•                         bar_nr, bar_res == prefetch ? "Prf" : "Mem",
  

•                         (unsigned long long)bar_size);
  

•           }
  

•           if (!enum_only && pciauto_region_allocate(bar_res, bar_size,
  

•                                                     &bar_value) == 0) {
  

•                   /* Write it out and update our limit */
  

•                   dm_pci_write_config32(dev, bar, (u32)bar_value);
  

•                   if (found_mem64) {
  

•                           bar += 4;
  

+#ifdef CONFIG_SYS_PCI_64BIT

•                           dm_pci_write_config32(dev, bar,
  

•                                                 (u32)(bar_value >> 32));
  

+#else

•                           /*
  

•                            * If we are a 64-bit decoder then increment to
  

•                            * the upper 32 bits of the bar and force it to
  

•                            * locate in the lower 4GB of memory.
  

•                            */
  

•                           dm_pci_write_config32(dev, bar, 0x00000000);
  

+#endif

•                   }
  

•           }
  

•           cmdstat |= (bar_response & PCI_BASE_ADDRESS_SPACE) ?
  

•                   PCI_COMMAND_IO : PCI_COMMAND_MEMORY;
  

•           debug("\n");
  

•           bar_nr++;
  

•   }
  

•   if (!enum_only) {
  

•           /* Configure the expansion ROM address */
  

•           dm_pci_read_config8(dev, PCI_HEADER_TYPE, &header_type);
  

•           header_type &= 0x7f;
  

•           if (header_type != PCI_HEADER_TYPE_CARDBUS) {
  

•                   rom_addr = (header_type == PCI_HEADER_TYPE_NORMAL) ?
  

•                           PCI_ROM_ADDRESS : PCI_ROM_ADDRESS1;
  

•                   dm_pci_write_config32(dev, rom_addr, 0xfffffffe);
  

•                   dm_pci_read_config32(dev, rom_addr, &bar_response);
  

•                   if (bar_response) {
  

•                           bar_size = -(bar_response & ~1);
  

•                           debug("PCI Autoconfig: ROM, size=%#x, ",
  

•                                 (unsigned int)bar_size);
  

•                           if (pciauto_region_allocate(mem, bar_size,
  

•                                                       &bar_value) == 0) {
  

•                                   dm_pci_write_config32(dev, rom_addr,
  

•                                                         bar_value);
  

•                           }
  

•                           cmdstat |= PCI_COMMAND_MEMORY;
  

•                           debug("\n");
  

•                   }
  

•           }
  

•   }
  

•   /* PCI_COMMAND_IO must be set for VGA device */
  

•   dm_pci_read_config16(dev, PCI_CLASS_DEVICE, &class);
  

•   if (class == PCI_CLASS_DISPLAY_VGA)
  

•           cmdstat |= PCI_COMMAND_IO;
  

•   dm_pci_write_config16(dev, PCI_COMMAND, cmdstat);
  

•   dm_pci_write_config8(dev, PCI_CACHE_LINE_SIZE,
  

•                        CONFIG_SYS_PCI_CACHE_LINE_SIZE);
  

•   dm_pci_write_config8(dev, PCI_LATENCY_TIMER, 0x80);
  

+}

+void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus) +{

•   struct pci_region *pci_mem;
  

•   struct pci_region *pci_prefetch;
  

•   struct pci_region *pci_io;
  

•   u16 cmdstat, prefechable_64;
  

•   /* The root controller has the region information */
  

•   struct pci_controller *ctlr_hose = pci_bus_to_hose(0);
  

•   pci_mem = ctlr_hose->pci_mem;
  

•   pci_prefetch = ctlr_hose->pci_prefetch;
  

•   pci_io = ctlr_hose->pci_io;
  

•   dm_pci_read_config16(dev, PCI_COMMAND, &cmdstat);
  

•   dm_pci_read_config16(dev, PCI_PREF_MEMORY_BASE, &prefechable_64);
  

•   prefechable_64 &= PCI_PREF_RANGE_TYPE_MASK;
  

•   /* Configure bus number registers */
  

•   dm_pci_write_config8(dev, PCI_PRIMARY_BUS, PCI_BUS(pci_get_bdf(dev)));
  

•   dm_pci_write_config8(dev, PCI_SECONDARY_BUS, sub_bus);
  

•   dm_pci_write_config8(dev, PCI_SUBORDINATE_BUS, 0xff);
  

•   if (pci_mem) {
  

•           /* Round memory allocator to 1MB boundary */
  

•           pciauto_region_align(pci_mem, 0x100000);
  

•           /*
  

•            * Set up memory and I/O filter limits, assume 32-bit
  

•            * I/O space
  

•            */
  

•           dm_pci_write_config16(dev, PCI_MEMORY_BASE,
  

•                                 (pci_mem->bus_lower & 0xfff00000) >> 16);
  

•           cmdstat |= PCI_COMMAND_MEMORY;
  

•   }
  

•   if (pci_prefetch) {
  

•           /* Round memory allocator to 1MB boundary */
  

•           pciauto_region_align(pci_prefetch, 0x100000);
  

•           /*
  

•            * Set up memory and I/O filter limits, assume 32-bit
  

•            * I/O space
  

•            */
  

•           dm_pci_write_config16(dev, PCI_PREF_MEMORY_BASE,
  

•                           (pci_prefetch->bus_lower & 0xfff00000) >> 16);
  

•           if (prefechable_64 == PCI_PREF_RANGE_TYPE_64)
  

+#ifdef CONFIG_SYS_PCI_64BIT

•                   dm_pci_write_config32(dev, PCI_PREF_BASE_UPPER32,
  

•                                         pci_prefetch->bus_lower >> 32);
  

+#else

•                   dm_pci_write_config32(dev, PCI_PREF_BASE_UPPER32, 0x0);
  

+#endif

•           cmdstat |= PCI_COMMAND_MEMORY;
  

•   } else {
  

•           /* We don't support prefetchable memory for now, so disable */
  

•           dm_pci_write_config16(dev, PCI_PREF_MEMORY_BASE, 0x1000);
  

•           dm_pci_write_config16(dev, PCI_PREF_MEMORY_LIMIT, 0x0);
  

•           if (prefechable_64 == PCI_PREF_RANGE_TYPE_64) {
  

•                   dm_pci_write_config16(dev, PCI_PREF_BASE_UPPER32, 0x0);
  

•                   dm_pci_write_config16(dev, PCI_PREF_LIMIT_UPPER32, 0x0);
  

•           }
  

•   }
  

•   if (pci_io) {
  

•           /* Round I/O allocator to 4KB boundary */
  

•           pciauto_region_align(pci_io, 0x1000);
  

•           dm_pci_write_config8(dev, PCI_IO_BASE,
  

•                                (pci_io->bus_lower & 0x0000f000) >> 8);
  

•           dm_pci_write_config16(dev, PCI_IO_BASE_UPPER16,
  

•                                 (pci_io->bus_lower & 0xffff0000) >> 16);
  

•           cmdstat |= PCI_COMMAND_IO;
  

•   }
  

•   /* Enable memory and I/O accesses, enable bus master */
  

•   dm_pci_write_config16(dev, PCI_COMMAND, cmdstat | PCI_COMMAND_MASTER);
  

+}

+void dm_pciauto_postscan_setup_bridge(struct udevice *dev, int sub_bus) +{

•   struct pci_region *pci_mem;
  

•   struct pci_region *pci_prefetch;
  

•   struct pci_region *pci_io;
  

•   /* The root controller has the region information */
  

•   struct pci_controller *ctlr_hose = pci_bus_to_hose(0);
  

•   pci_mem = ctlr_hose->pci_mem;
  

•   pci_prefetch = ctlr_hose->pci_prefetch;
  

•   pci_io = ctlr_hose->pci_io;
  

•   /* Configure bus number registers */
  

•   dm_pci_write_config8(dev, PCI_SUBORDINATE_BUS, sub_bus);
  

•   if (pci_mem) {
  

•           /* Round memory allocator to 1MB boundary */
  

•           pciauto_region_align(pci_mem, 0x100000);
  

•           dm_pci_write_config16(dev, PCI_MEMORY_LIMIT,
  

•                                 (pci_mem->bus_lower - 1) >> 16);
  

•   }
  

•   if (pci_prefetch) {
  

•           u16 prefechable_64;
  

•           dm_pci_read_config16(dev, PCI_PREF_MEMORY_LIMIT,
  

•                                &prefechable_64);
  

•           prefechable_64 &= PCI_PREF_RANGE_TYPE_MASK;
  

•           /* Round memory allocator to 1MB boundary */
  

•           pciauto_region_align(pci_prefetch, 0x100000);
  

•           dm_pci_write_config16(dev, PCI_PREF_MEMORY_LIMIT,
  

•                                 (pci_prefetch->bus_lower - 1) >> 16);
  

•           if (prefechable_64 == PCI_PREF_RANGE_TYPE_64)
  

+#ifdef CONFIG_SYS_PCI_64BIT

•                   dm_pci_write_config32(dev, PCI_PREF_LIMIT_UPPER32,
  

•                                   (pci_prefetch->bus_lower - 1) >> 32);
  

+#else

•                   dm_pci_write_config32(dev, PCI_PREF_LIMIT_UPPER32, 0x0);
  

+#endif

•   }
  

•   if (pci_io) {
  

•           /* Round I/O allocator to 4KB boundary */
  

•           pciauto_region_align(pci_io, 0x1000);
  

•           dm_pci_write_config8(dev, PCI_IO_LIMIT,
  

•                           ((pci_io->bus_lower - 1) & 0x0000f000) >> 8);
  

•           dm_pci_write_config16(dev, PCI_IO_LIMIT_UPPER16,
  

•                           ((pci_io->bus_lower - 1) & 0xffff0000) >> 16);
  

•   }
  

+}

+/*

• • HJF: Changed this to return int. I think this is required
• • to get the correct result when scanning bridges
• */

+int dm_pciauto_config_device(struct udevice *dev) +{

•   struct pci_region *pci_mem;
  

•   struct pci_region *pci_prefetch;
  

•   struct pci_region *pci_io;
  

•   unsigned int sub_bus = PCI_BUS(pci_get_bdf(dev));
  

•   unsigned short class;
  

•   bool enum_only = false;
  

•   int n;
  

+#ifdef CONFIG_PCI_ENUM_ONLY

•   enum_only = true;
  

+#endif

•   /* The root controller has the region information */
  

•   struct pci_controller *ctlr_hose = pci_bus_to_hose(0);
  

•   pci_mem = ctlr_hose->pci_mem;
  

•   pci_prefetch = ctlr_hose->pci_prefetch;
  

•   pci_io = ctlr_hose->pci_io;
  

•   dm_pci_read_config16(dev, PCI_CLASS_DEVICE, &class);
  

•   switch (class) {
  

•   case PCI_CLASS_BRIDGE_PCI:
  

•           debug("PCI Autoconfig: Found P2P bridge, device %d\n",
  

•                 PCI_DEV(pci_get_bdf(dev)));
  

•           dm_pciauto_setup_device(dev, 2, pci_mem, pci_prefetch, pci_io,
  

•                                   enum_only);
  

•           n = dm_pci_hose_probe_bus(dev);
  

•           if (n < 0)
  

•                   return n;
  

•           sub_bus = (unsigned int)n;
  

•           break;
  

•   case PCI_CLASS_BRIDGE_CARDBUS:
  

•           /*
  

•            * just do a minimal setup of the bridge,
  

•            * let the OS take care of the rest
  

•            */
  

•           dm_pciauto_setup_device(dev, 0, pci_mem, pci_prefetch, pci_io,
  

•                                   enum_only);
  

•           debug("PCI Autoconfig: Found P2CardBus bridge, device %d\n",
  

•                 PCI_DEV(pci_get_bdf(dev)));
  

•           break;
  

+#if defined(CONFIG_PCIAUTO_SKIP_HOST_BRIDGE)

•   case PCI_CLASS_BRIDGE_OTHER:
  

•           debug("PCI Autoconfig: Skipping bridge device %d\n",
  

•                 PCI_DEV(pci_get_bdf(dev)));
  

•           break;
  

+#endif +#if defined(CONFIG_MPC834x) && !defined(CONFIG_VME8349)

•   case PCI_CLASS_BRIDGE_OTHER:
  

•           /*
  

•            * The host/PCI bridge 1 seems broken in 8349 - it presents
  

•            * itself as 'PCI_CLASS_BRIDGE_OTHER' and appears as an _agent_
  

•            * device claiming resources io/mem/irq.. we only allow for
  

•            * the PIMMR window to be allocated (BAR0 - 1MB size)
  

•            */
  

•           debug("PCI Autoconfig: Broken bridge found, only minimal config\n");
  

•           dm_pciauto_setup_device(dev, 0, hose->pci_mem,
  

•                                   hose->pci_prefetch, hose->pci_io,
  

•                                   enum_only);
  

•           break;
  

+#endif

•   case PCI_CLASS_PROCESSOR_POWERPC: /* an agent or end-point */
  

•           debug("PCI AutoConfig: Found PowerPC device\n");
  

•   default:
  

•           dm_pciauto_setup_device(dev, 6, pci_mem, pci_prefetch, pci_io,
  

•                                   enum_only);
  

•           break;
  

•   }
  

•   return sub_bus;
  

+} diff --git a/drivers/pci/pci_auto_old.c b/drivers/pci/pci_auto_old.c index 1ff88b1..9126f78 100644 --- a/drivers/pci/pci_auto_old.c +++ b/drivers/pci/pci_auto_old.c @@ -180,18 +180,9 @@ void pciauto_prescan_setup_bridge(struct pci_controller *hose, struct pci_region *pci_io; u16 cmdstat, prefechable_64;

-#ifdef CONFIG_DM_PCI

•   /* The root controller has the region information */
  

•   struct pci_controller *ctlr_hose = pci_bus_to_hose(0);
  

•   pci_mem = ctlr_hose->pci_mem;
  

•   pci_prefetch = ctlr_hose->pci_prefetch;
  

•   pci_io = ctlr_hose->pci_io;
  

-#else pci_mem = hose->pci_mem; pci_prefetch = hose->pci_prefetch; pci_io = hose->pci_io; -#endif

    pci_hose_read_config_word(hose, dev, PCI_COMMAND, &cmdstat);
    pci_hose_read_config_word(hose, dev, PCI_PREF_MEMORY_BASE,

@@ -199,15 +190,10 @@ void pciauto_prescan_setup_bridge(struct pci_controller *hose, prefechable_64 &= PCI_PREF_RANGE_TYPE_MASK;

    /* Configure bus number registers */

-#ifdef CONFIG_DM_PCI

•   pci_hose_write_config_byte(hose, dev, PCI_PRIMARY_BUS, PCI_BUS(dev));
  

•   pci_hose_write_config_byte(hose, dev, PCI_SECONDARY_BUS, sub_bus);
  

-#else pci_hose_write_config_byte(hose, dev, PCI_PRIMARY_BUS, PCI_BUS(dev) - hose->first_busno); pci_hose_write_config_byte(hose, dev, PCI_SECONDARY_BUS, sub_bus - hose->first_busno); -#endif pci_hose_write_config_byte(hose, dev, PCI_SUBORDINATE_BUS, 0xff);

    if (pci_mem) {

@@ -274,26 +260,13 @@ void pciauto_postscan_setup_bridge(struct pci_controller *hose, struct pci_region *pci_prefetch; struct pci_region *pci_io;

-#ifdef CONFIG_DM_PCI

•   /* The root controller has the region information */
  

•   struct pci_controller *ctlr_hose = pci_bus_to_hose(0);
  

•   pci_mem = ctlr_hose->pci_mem;
  

•   pci_prefetch = ctlr_hose->pci_prefetch;
  

•   pci_io = ctlr_hose->pci_io;
  

-#else pci_mem = hose->pci_mem; pci_prefetch = hose->pci_prefetch; pci_io = hose->pci_io; -#endif

    /* Configure bus number registers */

-#ifdef CONFIG_DM_PCI

•   pci_hose_write_config_byte(hose, dev, PCI_SUBORDINATE_BUS, sub_bus);
  

-#else pci_hose_write_config_byte(hose, dev, PCI_SUBORDINATE_BUS, sub_bus - hose->first_busno); -#endif

    if (pci_mem) {
            /* Round memory allocator to 1MB boundary */

@@ -353,18 +326,9 @@ int pciauto_config_device(struct pci_controller *hose, pci_dev_t dev) unsigned short class; int n;

-#ifdef CONFIG_DM_PCI

•   /* The root controller has the region information */
  

•   struct pci_controller *ctlr_hose = pci_bus_to_hose(0);
  

•   pci_mem = ctlr_hose->pci_mem;
  

•   pci_prefetch = ctlr_hose->pci_prefetch;
  

•   pci_io = ctlr_hose->pci_io;
  

-#else pci_mem = hose->pci_mem; pci_prefetch = hose->pci_prefetch; pci_io = hose->pci_io; -#endif

    pci_hose_read_config_word(hose, dev, PCI_CLASS_DEVICE, &class);

@@ -376,12 +340,6 @@ int pciauto_config_device(struct pci_controller *hose, pci_dev_t dev) pciauto_setup_device(hose, dev, 2, pci_mem, pci_prefetch, pci_io);

-#ifdef CONFIG_DM_PCI

•           n = dm_pci_hose_probe_bus(hose, dev);
  

•           if (n < 0)
  

•                   return n;
  

•           sub_bus = (unsigned int)n;
  

-#else /* Passing in current_busno allows for sibling P2P bridges */ hose->current_busno++; pciauto_prescan_setup_bridge(hose, dev, hose->current_busno); @@ -396,7 +354,6 @@ int pciauto_config_device(struct pci_controller *hose, pci_dev_t dev) pciauto_postscan_setup_bridge(hose, dev, sub_bus);

            sub_bus = hose->current_busno;

-#endif break;

    case PCI_CLASS_BRIDGE_CARDBUS:

@@ -410,9 +367,7 @@ int pciauto_config_device(struct pci_controller *hose, pci_dev_t dev) debug("PCI Autoconfig: Found P2CardBus bridge, device %d\n", PCI_DEV(dev));

-#ifndef CONFIG_DM_PCI hose->current_busno++; -#endif break;

#if defined(CONFIG_PCIAUTO_SKIP_HOST_BRIDGE) diff --git a/include/pci.h b/include/pci.h index 2adca85..8d56fdd 100644 --- a/include/pci.h +++ b/include/pci.h @@ -995,7 +995,10 @@ int pci_find_device_id(struct pci_device_id *ids, int index,

• @bdf: PCI bus address to scan (PCI_BUS(bdf) is the bus number)
• @return 0 if OK, -ve on error

*/ -int dm_pci_hose_probe_bus(struct pci_controller *hose, pci_dev_t bdf); +int dm_pci_hose_probe_bus(struct udevice *bus);

+void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus); +void dm_pciauto_postscan_setup_bridge(struct udevice *dev, int sub_bus);

/**

• pci_bus_read_config() - Read a configuration value from a device

@@ -1166,6 +1169,10 @@ struct udevice *pci_get_controller(struct udevice *dev); int pci_get_regions(struct udevice *dev, struct pci_region **iop, struct pci_region **memp, struct pci_region **prefp);

+int dm_pciauto_config_device(struct udevice *dev);

+void dm_pciauto_region_init(struct pci_region *res);

I think these two APIs are internal to the pci-uclass driver itself, so there is no need to publish them?

/**

• struct dm_pci_emul_ops - PCI device emulator operations

*/

Regards, Bin

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

Add a function which scans the driver model device information rather than scanning the PCI bus again.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/pci/pci-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++ include/pci.h | 12 ++++++++++++ 2 files changed, 51 insertions(+)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 8087e2d..6376e43 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -194,6 +194,45 @@ int pci_find_device_id(struct pci_device_id *ids, int index, return -ENODEV; }

+static int dm_pci_bus_find_device(struct udevice *bus, unsigned int vendor,

•                             unsigned int device, int *indexp,
  

•                             struct udevice **devp)
  

+{

•   struct pci_child_platdata *pplat;
  

•   struct udevice *dev;
  

•   for (device_find_first_child(bus, &dev);
  

•        dev;
  

•        device_find_next_child(&dev)) {
  

•           pplat = dev_get_parent_platdata(dev);
  

•           if (pplat->vendor == vendor && pplat->device == device) {
  

•                   if (!(*indexp)--) {
  

•                           *devp = dev;
  

•                           return 0;
  

•                   }
  

•           }
  

•   }
  

•   return -ENODEV;
  

+}

+int dm_pci_find_device(unsigned int vendor, unsigned int device, int index,

•                  struct udevice **devp)
  

+{

•   struct udevice *bus;
  

•   /* Scan all known buses */
  

•   for (uclass_first_device(UCLASS_PCI, &bus);
  

•        bus;
  

•        uclass_next_device(&bus)) {
  

•           if (!dm_pci_bus_find_device(bus, vendor, device, &index, devp))
  

•                   return device_probe(*devp);
  

•   }
  

•   *devp = NULL;
  

•   return -ENODEV;
  

+}

int pci_bus_write_config(struct udevice *bus, pci_dev_t bdf, int offset, unsigned long value, enum pci_size_t size) { diff --git a/include/pci.h b/include/pci.h index 8d56fdd..f29804f 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1174,6 +1174,18 @@ int dm_pciauto_config_device(struct udevice *dev); void dm_pciauto_region_init(struct pci_region *res);

/**

• • dm_pci_find_device() - find a device by vendor/device ID
• • @vendor: Vendor ID
• • @device: Device ID
• • @index: 0 to find the first match, 1 for second, etc.
• • @devp: Returns pointer to the device, if found
• • @return 0 if found, -ve on error
• */

+int dm_pci_find_device(unsigned int vendor, unsigned int device, int index,

•                  struct udevice **devp);
  

+/**

• struct dm_pci_emul_ops - PCI device emulator operations

*/ struct dm_pci_emul_ops { --

Reviewed-by: Bin Meng bmeng.cn@gmail.com

Hi Simon,

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

Adjust the SCSI command to use driver model for its PCI interface.

Signed-off-by: Simon Glass sjg@chromium.org

common/cmd_scsi.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/common/cmd_scsi.c b/common/cmd_scsi.c index 31c4319..855016d 100644 --- a/common/cmd_scsi.c +++ b/common/cmd_scsi.c @@ -184,7 +184,7 @@ int scsi_get_disk_count(void) #if defined(CONFIG_PCI) && !defined(CONFIG_SCSI_AHCI_PLAT) void scsi_init(void) {

•   int busdevfunc;
  

•   int busdevfunc = -1;
    int i;
    /*
     * Find a device from the list, this driver will support a single
  

@@ -192,9 +192,21 @@ void scsi_init(void) */ for (i = 0; i < ARRAY_SIZE(scsi_device_list); i++) { /* get PCI Device ID */ +#ifdef CONFIG_DM_PCI

•           struct udevice *dev;
  

•           int ret;
  

•           ret = dm_pci_find_device(scsi_device_list[i].vendor,
  

•                                    scsi_device_list[i].device, 0, &dev);
  

•           if (!ret) {
  

•                   busdevfunc = pci_get_bdf(dev);
  

To keep naming consistency, can we rename pci_get_bdf() to dm_pci_get_bdf()?

•                   break;
  

•           }
  

+#else busdevfunc = pci_find_device(scsi_device_list[i].vendor, scsi_device_list[i].device, 0); +#endif if (busdevfunc != -1) break; } --

Regards, Bin

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

Add a function which scans the driver model device information rather than scanning the PCI bus again.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/pci/pci-uclass.c | 20 ++++++++++++++++++++ include/pci.h | 10 ++++++++++ 2 files changed, 30 insertions(+)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 6376e43..929d772 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -233,6 +233,26 @@ int dm_pci_find_device(unsigned int vendor, unsigned int device, int index, return -ENODEV; }

+int dm_pci_find_class(uint find_class, int index, struct udevice **devp) +{

•   struct udevice *dev;
  

•   /* Scan all known buses */
  

•   for (pci_find_first_device(&dev);
  

•        dev;
  

•        pci_find_next_device(&dev)) {
  

•           struct pci_child_platdata *pplat = dev_get_parent_platdata(dev);
  

•           if (pplat->class == find_class && !index--) {
  

•                   *devp = dev;
  

•                   return device_probe(*devp);
  

•           }
  

•   }
  

•   *devp = NULL;
  

•   return -ENODEV;
  

+}

int pci_bus_write_config(struct udevice *bus, pci_dev_t bdf, int offset, unsigned long value, enum pci_size_t size) { diff --git a/include/pci.h b/include/pci.h index f29804f..d4f6e02 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1186,6 +1186,16 @@ int dm_pci_find_device(unsigned int vendor, unsigned int device, int index, struct udevice **devp);

/**

• • dm_pci_find_class() - find a device by class
• • @find_class: 3-byte (24-bit) class value to find
• • @index: 0 to find the first match, 1 for second, etc.
• • @devp: Returns pointer to the device, if found
• • @return 0 if found, -ve on error
• */

+int dm_pci_find_class(uint find_class, int index, struct udevice **devp);

+/**

• struct dm_pci_emul_ops - PCI device emulator operations

*/ struct dm_pci_emul_ops { --

Reviewed-by: Bin Meng bmeng.cn@gmail.com

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

Add a driver-model function for reading the PCI BAR from a device.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/pci/pci-uclass.c | 13 +++++++++++++ include/pci.h | 9 +++++++++ 2 files changed, 22 insertions(+)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 929d772..4e37972 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1053,6 +1053,19 @@ int pci_get_regions(struct udevice *dev, struct pci_region **iop, return (*iop != NULL) + (*memp != NULL) + (*prefp != NULL); }

+u32 dm_pci_read_bar32(struct udevice *dev, int barnum) +{

•   u32 addr;
  

•   int bar;
  

•   bar = PCI_BASE_ADDRESS_0 + barnum * 4;
  

•   dm_pci_read_config32(dev, bar, &addr);
  

•   if (addr & PCI_BASE_ADDRESS_SPACE_IO)
  

•           return addr & PCI_BASE_ADDRESS_IO_MASK;
  

•   else
  

•           return addr & PCI_BASE_ADDRESS_MEM_MASK;
  

+}

UCLASS_DRIVER(pci) = { .id = UCLASS_PCI, .name = "pci", diff --git a/include/pci.h b/include/pci.h index d4f6e02..f1c0d71 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1174,6 +1174,15 @@ int dm_pciauto_config_device(struct udevice *dev); void dm_pciauto_region_init(struct pci_region *res);

/**

• • dm_pci_read_bar32() - read a base address register from a device
• • @dev: Device to check
• • @barnum: Bar number to read (numbered from 0)
• • @return: value of BAR
• */

+u32 dm_pci_read_bar32(struct udevice *dev, int barnum);

+/**

• dm_pci_find_device() - find a device by vendor/device ID
• @vendor: Vendor ID

--

Reviewed-by: Bin Meng bmeng.cn@gmail.com

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

Use the driver model version of the function to find the BAR. This updates the fdtdec function, of which ns16550 is the only user.

The fdtdec_get_pci_bdf() function is dropped for several reasons:

• with driver model we should use 'struct udevice *' rather than passing the device tree offset explicitly
• there are no other users in the tree
• the function parses for information which is already available in the PCI

device structure (specifically struct pci_child_platdata which is available at dev_get_parent_platdata(dev)

Signed-off-by: Simon Glass sjg@chromium.org

drivers/serial/ns16550.c | 3 +-- include/fdtdec.h | 23 +++----------------- lib/fdtdec.c | 55 ++++-------------------------------------------- 3 files changed, 8 insertions(+), 73 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 6433844..364531e 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -386,8 +386,7 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev) return ret; }

•           ret = fdtdec_get_pci_bar32(gd->fdt_blob, dev->of_offset,
  

•                                      &pci_addr, &bar);
  

•           ret = fdtdec_get_pci_bar32(dev, &pci_addr, &bar);
            if (ret)
                    return ret;
  
  

diff --git a/include/fdtdec.h b/include/fdtdec.h index 8e0f592..6afd53d 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -445,32 +445,15 @@ int fdtdec_get_pci_vendev(const void *blob, int node,

/**

• Look at the pci address of a device node that represents a PCI device

• • and parse the bus, device and function number from it. For some cases
• • like the bus number encoded in reg property is not correct after pci
• • enumeration, this function looks through the node's compatible strings
• • to get these numbers extracted instead.
• • @param blob FDT blob
• • @param node node to examine
• • @param addr pci address in the form of fdt_pci_addr
• • @param bdf returns bus, device, function triplet
• • @return 0 if ok, negative on error
• */

-int fdtdec_get_pci_bdf(const void *blob, int node,

•           struct fdt_pci_addr *addr, pci_dev_t *bdf);
  

-/**

• • Look at the pci address of a device node that represents a PCI device
  • and return base address of the pci device's registers.
• • @param blob FDT blob
• • @param node node to examine

• • @param dev device to examine
  • @param addr pci address in the form of fdt_pci_addr
  • @param bar returns base address of the pci device's registers
  • @return 0 if ok, negative on error
  */

-int fdtdec_get_pci_bar32(const void *blob, int node,

•           struct fdt_pci_addr *addr, u32 *bar);
  

+int fdtdec_get_pci_bar32(struct udevice *dev, struct fdt_pci_addr *addr,

•                    u32 *bar);
  
  

/**

• Look up a 32-bit integer property in a node and return it. The property

diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 4534177..4b9f196 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -5,6 +5,7 @@

#ifndef USE_HOSTCC #include <common.h> +#include <dm.h> #include <errno.h> #include <serial.h> #include <libfdt.h> @@ -283,58 +284,10 @@ int fdtdec_get_pci_vendev(const void *blob, int node, u16 *vendor, u16 *device) return -ENOENT; }

-int fdtdec_get_pci_bdf(const void *blob, int node,

•           struct fdt_pci_addr *addr, pci_dev_t *bdf)
  

+int fdtdec_get_pci_bar32(struct udevice *dev, struct fdt_pci_addr *addr,

•                    u32 *bar)
  

{

•   u16 dt_vendor, dt_device, vendor, device;
  

•   int ret;
  

•   /* get vendor id & device id from the compatible string */
  

•   ret = fdtdec_get_pci_vendev(blob, node, &dt_vendor, &dt_device);
  

•   if (ret)
  

•           return ret;
  

•   /* extract the bdf from fdt_pci_addr */
  

•   *bdf = addr->phys_hi & 0xffff00;
  

•   /* read vendor id & device id based on bdf */
  

•   pci_read_config_word(*bdf, PCI_VENDOR_ID, &vendor);
  

•   pci_read_config_word(*bdf, PCI_DEVICE_ID, &device);
  

•   /*
  

•    * Note there are two places in the device tree to fully describe
  

•    * a pci device: one is via compatible string with a format of
  

•    * "pciVVVV,DDDD" and the other one is the bdf numbers encoded in
  

•    * the device node's reg address property. We read the vendor id
  

•    * and device id based on bdf and compare the values with the
  

•    * "VVVV,DDDD". If they are the same, then we are good to use bdf
  

•    * to read device's bar. But if they are different, we have to rely
  

•    * on the vendor id and device id extracted from the compatible
  

•    * string and locate the real bdf by pci_find_device(). This is
  

•    * because normally we may only know device's device number and
  

•    * function number when writing device tree. The bus number is
  

•    * dynamically assigned during the pci enumeration process.
  

•    */
  

•   if ((dt_vendor != vendor) || (dt_device != device)) {
  

•           *bdf = pci_find_device(dt_vendor, dt_device, 0);
  

•           if (*bdf == -1)
  

•                   return -ENODEV;
  

•   }
  

•   return 0;
  

-}

-int fdtdec_get_pci_bar32(const void *blob, int node,

•           struct fdt_pci_addr *addr, u32 *bar)
  

-{

•   pci_dev_t bdf;
    int barnum;
  

•   int ret;
  

•   /* get pci devices's bdf */
  

•   ret = fdtdec_get_pci_bdf(blob, node, addr, &bdf);
  

•   if (ret)
  

•           return ret;
  
    /* extract the bar number from fdt_pci_addr */
    barnum = addr->phys_hi & 0xff;
  

@@ -342,7 +295,7 @@ int fdtdec_get_pci_bar32(const void *blob, int node, return -EINVAL;

    barnum = (barnum - PCI_BASE_ADDRESS_0) / 4;

•   *bar = pci_read_bar32(pci_bus_to_hose(PCI_BUS(bdf)), bdf, barnum);
  

•   *bar = dm_pci_read_bar32(dev, barnum);
  
    return 0;
  

}

Reviewed-by: Bin Meng bmeng.cn@gmail.com

Hi Simon,

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

Use the driver-model PCI functions here where possible. For now we have to search for the device with pci_bus_find_bdf() but at some point we can put this in a proper driver and avoid this.

Signed-off-by: Simon Glass sjg@chromium.org

arch/x86/cpu/ivybridge/bd82x6x.c | 6 ++++-- arch/x86/cpu/ivybridge/gma.c | 15 +++++++-------- arch/x86/include/asm/arch-ivybridge/bd82x6x.h | 3 +-- 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/x86/cpu/ivybridge/bd82x6x.c b/arch/x86/cpu/ivybridge/bd82x6x.c index 3e7a907..bf222fa 100644 --- a/arch/x86/cpu/ivybridge/bd82x6x.c +++ b/arch/x86/cpu/ivybridge/bd82x6x.c @@ -86,8 +86,10 @@ static int bd82x6x_probe(struct udevice *dev) debug("%s: Cannot find GMA node\n", __func__); return -EINVAL; }

•   ret = gma_func0_init(PCH_VIDEO_DEV, pci_bus_to_hose(0), blob,
  

•                        gma_node);
  

•   ret = pci_bus_find_bdf(PCH_VIDEO_DEV, &dev);
  

Again, can we rename this to dm_pci_bus_find_bdf() to keep naming consistency?

•   if (ret)
  

•           return ret;
  

•   ret = gma_func0_init(dev, blob, gma_node);
    if (ret)
            return ret;
  
  

diff --git a/arch/x86/cpu/ivybridge/gma.c b/arch/x86/cpu/ivybridge/gma.c index 89d4a5e..b76c81c 100644 --- a/arch/x86/cpu/ivybridge/gma.c +++ b/arch/x86/cpu/ivybridge/gma.c @@ -728,8 +728,7 @@ static int int15_handler(void) return res; }

-int gma_func0_init(pci_dev_t dev, struct pci_controller *hose,

•              const void *blob, int node)
  

+int gma_func0_init(struct udevice *dev, const void *blob, int node) { #ifdef CONFIG_VIDEO ulong start; @@ -740,16 +739,16 @@ int gma_func0_init(pci_dev_t dev, struct pci_controller *hose, int ret;

    /* IGD needs to be Bus Master */

•   reg32 = x86_pci_read_config32(dev, PCI_COMMAND);
  

•   dm_pci_read_config32(dev, PCI_COMMAND, &reg32);
    reg32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO;
  

•   x86_pci_write_config32(dev, PCI_COMMAND, reg32);
  

•   dm_pci_write_config32(dev, PCI_COMMAND, reg32);
  
    /* Use write-combining for the graphics memory, 256MB */
  

•   base = pci_read_bar32(hose, dev, 2);
  

•   base = dm_pci_read_bar32(dev, 2);
    mtrr_add_request(MTRR_TYPE_WRCOMB, base, 256 << 20);
    mtrr_commit(true);
  
  

•   gtt_bar = (void *)pci_read_bar32(pci_bus_to_hose(0), dev, 0);
  

•   gtt_bar = (void *)dm_pci_read_bar32(dev, 0);
    debug("GT bar %p\n", gtt_bar);
    ret = gma_pm_init_pre_vbios(gtt_bar);
    if (ret)
  

@@ -757,8 +756,8 @@ int gma_func0_init(pci_dev_t dev, struct pci_controller *hose,

#ifdef CONFIG_VIDEO start = get_timer(0);

•   ret = pci_run_vga_bios(dev, int15_handler, PCI_ROM_USE_NATIVE |
  

•                          PCI_ROM_ALLOW_FALLBACK);
  

•   ret = pci_run_vga_bios(pci_get_bdf(dev), int15_handler,
  

dm_pci_run_vga_bios() for naming consistency?

•                          PCI_ROM_USE_NATIVE | PCI_ROM_ALLOW_FALLBACK);
    debug("BIOS ran in %lums\n", get_timer(start));
  

#endif /* Post VBIOS init */ diff --git a/arch/x86/include/asm/arch-ivybridge/bd82x6x.h b/arch/x86/include/asm/arch-ivybridge/bd82x6x.h index 7786493..fcdf6e2 100644 --- a/arch/x86/include/asm/arch-ivybridge/bd82x6x.h +++ b/arch/x86/include/asm/arch-ivybridge/bd82x6x.h @@ -12,8 +12,7 @@ void bd82x6x_sata_enable(pci_dev_t dev, const void *blob, int node); void bd82x6x_pci_init(pci_dev_t dev); void bd82x6x_usb_ehci_init(pci_dev_t dev); void bd82x6x_usb_xhci_init(pci_dev_t dev); -int gma_func0_init(pci_dev_t dev, struct pci_controller *hose,

•              const void *blob, int node);
  

+int gma_func0_init(struct udevice *dev, const void *blob, int node); int bd82x6x_init(void);

/**

Regards, Bin

Hi Simon,

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

Adjust this code to use driver model for devices where possible. Since existing users have not been converted the old code must remain.

Signed-off-by: Simon Glass sjg@chromium.org

arch/x86/lib/bios_interrupts.c | 78 +++++++++++++++++++++++++++++++++++------- 1 file changed, 65 insertions(+), 13 deletions(-)

diff --git a/arch/x86/lib/bios_interrupts.c b/arch/x86/lib/bios_interrupts.c index 47d9f59..ef9f766 100644 --- a/arch/x86/lib/bios_interrupts.c +++ b/arch/x86/lib/bios_interrupts.c @@ -105,13 +105,15 @@ int int1a_handler(void) unsigned short func = (unsigned short)M.x86.R_EAX; int retval = 1; unsigned short devid, vendorid, devfn;

•   struct udevice *dev;
    /* Use short to get rid of gabage in upper half of 32-bit register */
    short devindex;
    unsigned char bus;
  

•   pci_dev_t dev;
  

•   pci_dev_t bdf;
    u32 dword;
    u16 word;
    u8 byte, reg;
  

•   int ret;
  
    switch (func) {
    case 0xb101: /* PCIBIOS Check */
  

@@ -131,17 +133,29 @@ int int1a_handler(void) devid = M.x86.R_ECX; vendorid = M.x86.R_EDX; devindex = M.x86.R_ESI;

•           dev = pci_find_device(vendorid, devid, devindex);
  

•           if (dev != -1) {
  

+#ifdef CONFIG_DM_PCI

•           /*
  

•            * TODO(sjg@chromium.org): Tidy up this code when all boards
  

•            * use DM
  

•            */
  

All x86 boards have been converted to DM PCI in v2016.01.rc1.

•           bdf = -1;
  

•           ret = dm_pci_find_device(vendorid, devid, devindex, &dev);
  

•           if (!ret)
  

•                   bdf = pci_get_bdf(dev);
  

+#else

•           bdf = pci_find_device(vendorid, devid, devindex);
  

+#endif

•           if (bdf != -1) {
                    unsigned short busdevfn;
  

•                   M.x86.R_EAX &= 0xffff00ff; /* Clear AH */
                    M.x86.R_EAX |= PCIBIOS_SUCCESSFUL;
                    /*
                     * busnum is an unsigned char;
                     * devfn is an int, so we mask it off.
                     */
  

•                   busdevfn = (PCI_BUS(dev) << 8) | PCI_DEV(dev) << 3 |
  

•                           PCI_FUNC(dev);
  

•                   busdevfn = (PCI_BUS(bdf) << 8) | PCI_DEV(bdf) << 3 |
  

•                           PCI_FUNC(bdf);
                    debug("0x%x: return 0x%x\n", func, busdevfn);
                    M.x86.R_EBX = busdevfn;
                    retval = 1;
  

@@ -160,35 +174,73 @@ int int1a_handler(void) devfn = M.x86.R_EBX & 0xff; bus = M.x86.R_EBX >> 8; reg = M.x86.R_EDI;

•           dev = PCI_BDF(bus, devfn >> 3, devfn & 7);
  

•           bdf = PCI_BDF(bus, devfn >> 3, devfn & 7);
  

•           /*
  

•            * TODO(sjg@chromium.org): Tidy up this code when all boards
  

•            * use DM
  

•            */
  

Ditto.

+#ifdef CONFIG_DM_PCI

•           ret = pci_bus_find_bdf(bdf, &dev);
  

•           if (ret) {
  

•                   debug("%s: Device %x not found\n", __func__, bdf);
  

•                   break;
  

•           }
  
            switch (func) {
            case 0xb108: /* Read Config Byte */
  

•                   byte = x86_pci_read_config8(dev, reg);
  

•                   dm_pci_read_config8(dev, reg, &byte);
                    M.x86.R_ECX = byte;
                    break;
            case 0xb109: /* Read Config Word */
  

•                   word = x86_pci_read_config16(dev, reg);
  

•                   dm_pci_read_config16(dev, reg, &word);
                    M.x86.R_ECX = word;
                    break;
            case 0xb10a: /* Read Config Dword */
  

•                   dword = x86_pci_read_config32(dev, reg);
  

•                   dm_pci_read_config32(dev, reg, &dword);
                    M.x86.R_ECX = dword;
                    break;
            case 0xb10b: /* Write Config Byte */
                    byte = M.x86.R_ECX;
  

•                   x86_pci_write_config8(dev, reg, byte);
  

•                   dm_pci_write_config8(dev, reg, byte);
                    break;
            case 0xb10c: /* Write Config Word */
                    word = M.x86.R_ECX;
  

•                   x86_pci_write_config16(dev, reg, word);
  

•                   dm_pci_write_config16(dev, reg, word);
                    break;
            case 0xb10d: /* Write Config Dword */
                    dword = M.x86.R_ECX;
  

•                   x86_pci_write_config32(dev, reg, dword);
  

•                   dm_pci_write_config32(dev, reg, dword);
                    break;
            }
  

+#else

•           switch (func) {
  

•           case 0xb108: /* Read Config Byte */
  

•                   byte = x86_pci_read_config8(bdf, reg);
  

•                   M.x86.R_ECX = byte;
  

•                   break;
  

•           case 0xb109: /* Read Config Word */
  

•                   word = x86_pci_read_config16(bdf, reg);
  

•                   M.x86.R_ECX = word;
  

•                   break;
  

•           case 0xb10a: /* Read Config Dword */
  

•                   dword = x86_pci_read_config32(bdf, reg);
  

•                   M.x86.R_ECX = dword;
  

•                   break;
  

•           case 0xb10b: /* Write Config Byte */
  

•                   byte = M.x86.R_ECX;
  

•                   x86_pci_write_config8(bdf, reg, byte);
  

•                   break;
  

•           case 0xb10c: /* Write Config Word */
  

•                   word = M.x86.R_ECX;
  

•                   x86_pci_write_config16(bdf, reg, word);
  

•                   break;
  

•           case 0xb10d: /* Write Config Dword */
  

•                   dword = M.x86.R_ECX;
  

•                   x86_pci_write_config32(bdf, reg, dword);
  

•                   break;
  

•           }
  

+#endif #ifdef CONFIG_REALMODE_DEBUG debug("0x%x: bus %d devfn 0x%x reg 0x%x val 0x%x\n", func, bus, devfn, reg, M.x86.R_ECX); --

Regards, Bin

Hi Simon,

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

Adjust these files to use the driver-model PCI API instead of the legacy functions.

Signed-off-by: Simon Glass sjg@chromium.org

arch/x86/cpu/ivybridge/gma.c | 2 +- drivers/pci/pci_rom.c | 31 ++++++++++++++++--------------- drivers/video/vesa_fb.c | 6 +++--- include/pci_rom.h | 2 +- 4 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/arch/x86/cpu/ivybridge/gma.c b/arch/x86/cpu/ivybridge/gma.c index b76c81c..122ff68 100644 --- a/arch/x86/cpu/ivybridge/gma.c +++ b/arch/x86/cpu/ivybridge/gma.c @@ -756,7 +756,7 @@ int gma_func0_init(struct udevice *dev, const void *blob, int node)

#ifdef CONFIG_VIDEO start = get_timer(0);

•   ret = pci_run_vga_bios(pci_get_bdf(dev), int15_handler,
  

•   ret = pci_run_vga_bios(dev, int15_handler,
                           PCI_ROM_USE_NATIVE | PCI_ROM_ALLOW_FALLBACK);
    debug("BIOS ran in %lums\n", get_timer(start));
  

#endif diff --git a/drivers/pci/pci_rom.c b/drivers/pci/pci_rom.c index d244543..6e3adf6 100644 --- a/drivers/pci/pci_rom.c +++ b/drivers/pci/pci_rom.c @@ -37,12 +37,12 @@ #include <asm/acpi.h> #endif

-__weak bool board_should_run_oprom(pci_dev_t dev) +__weak bool board_should_run_oprom(struct udevice *dev) { return true; }

-static bool should_load_oprom(pci_dev_t dev) +static bool should_load_oprom(struct udevice *dev) { #ifdef CONFIG_HAVE_ACPI_RESUME if (acpi_get_slp_type() == 3) @@ -61,7 +61,7 @@ __weak uint32_t board_map_oprom_vendev(uint32_t vendev) return vendev; }

-static int pci_rom_probe(pci_dev_t dev, uint class, +static int pci_rom_probe(struct udevice *dev, uint class,

class is not necessary as it can be got from dev_get_parent_platdata(dev).

                     struct pci_rom_header **hdrp)

{ struct pci_rom_header *rom_header; @@ -73,8 +73,8 @@ static int pci_rom_probe(pci_dev_t dev, uint class, u32 mapped_vendev; u32 rom_address;

•   pci_read_config_word(dev, PCI_VENDOR_ID, &vendor);
  

•   pci_read_config_word(dev, PCI_DEVICE_ID, &device);
  

•   dm_pci_read_config16(dev, PCI_VENDOR_ID, &vendor);
  

•   dm_pci_read_config16(dev, PCI_DEVICE_ID, &device);
  

The vendor and device are already in platdata, so we can get it via dev_get_parent_platdata(dev).

    vendev = vendor << 16 | device;
    mapped_vendev = board_map_oprom_vendev(vendev);
    if (vendev != mapped_vendev)

@@ -84,15 +84,15 @@ static int pci_rom_probe(pci_dev_t dev, uint class, rom_address = CONFIG_VGA_BIOS_ADDR; #else

•   pci_read_config_dword(dev, PCI_ROM_ADDRESS, &rom_address);
  

•   dm_pci_read_config32(dev, PCI_ROM_ADDRESS, &rom_address);
    if (rom_address == 0x00000000 || rom_address == 0xffffffff) {
            debug("%s: rom_address=%x\n", __func__, rom_address);
            return -ENOENT;
    }
  
    /* Enable expansion ROM address decoding. */
  

•   pci_write_config_dword(dev, PCI_ROM_ADDRESS,
  

•                          rom_address | PCI_ROM_ADDRESS_ENABLE);
  

•   dm_pci_write_config32(dev, PCI_ROM_ADDRESS,
  

•                         rom_address | PCI_ROM_ADDRESS_ENABLE);
  

#endif debug("Option ROM address %x\n", rom_address); rom_header = (struct pci_rom_header *)(unsigned long)rom_address; @@ -106,7 +106,7 @@ static int pci_rom_probe(pci_dev_t dev, uint class, le16_to_cpu(rom_header->signature)); #ifndef CONFIG_VGA_BIOS_ADDR /* Disable expansion ROM address decoding */

•           pci_write_config_dword(dev, PCI_ROM_ADDRESS, rom_address);
  

•           dm_pci_write_config32(dev, PCI_ROM_ADDRESS, rom_address);
  

#endif return -EINVAL; } @@ -259,7 +259,8 @@ void setup_video(struct screen_info *screen_info) screen_info->rsvd_pos = vesa->reserved_mask_pos; }

-int pci_run_vga_bios(pci_dev_t dev, int (*int15_handler)(void), int exec_method) +int pci_run_vga_bios(struct udevice *dev, int (*int15_handler)(void),

•                int exec_method)
  

{ struct pci_rom_header *rom, *ram; int vesa_mode = -1; @@ -268,7 +269,7 @@ int pci_run_vga_bios(pci_dev_t dev, int (*int15_handler)(void), int exec_method) int ret;

    /* Only execute VGA ROMs */

•   pci_read_config_dword(dev, PCI_REVISION_ID, &class);
  

•   dm_pci_read_config32(dev, PCI_REVISION_ID, &class);
  

class can be got from dev_get_parent_platdata(dev).

    if (((class >> 16) ^ PCI_CLASS_DISPLAY_VGA) & 0xff00) {
            debug("%s: Class %#x, should be %#x\n", __func__, class,
                  PCI_CLASS_DISPLAY_VGA);

@@ -322,12 +323,12 @@ int pci_run_vga_bios(pci_dev_t dev, int (*int15_handler)(void), int exec_method) #ifdef CONFIG_BIOSEMU BE_VGAInfo *info;

•           ret = biosemu_setup(dev, &info);
  

•           ret = biosemu_setup(pci_get_bdf(dev), &info);
            if (ret)
                    return ret;
            biosemu_set_interrupt_handler(0x15, int15_handler);
  

•           ret = biosemu_run(dev, (uchar *)ram, 1 << 16, info, true,
  

•                             vesa_mode, &mode_info);
  

•           ret = biosemu_run(pci_get_bdf(dev), (uchar *)ram, 1 << 16, info,
  

•                             true, vesa_mode, &mode_info);
            if (ret)
                    return ret;
  

#endif @@ -335,7 +336,7 @@ int pci_run_vga_bios(pci_dev_t dev, int (*int15_handler)(void), int exec_method) #ifdef CONFIG_X86 bios_set_interrupt_handler(0x15, int15_handler);

•           bios_run_on_x86(dev, (unsigned long)ram, vesa_mode,
  

•           bios_run_on_x86(pci_get_bdf(dev), (unsigned long)ram, vesa_mode,
                            &mode_info);
  

#endif } diff --git a/drivers/video/vesa_fb.c b/drivers/video/vesa_fb.c index a19651f..e1ee0a4 100644 --- a/drivers/video/vesa_fb.c +++ b/drivers/video/vesa_fb.c @@ -19,8 +19,8 @@ GraphicDevice ctfb; void *video_hw_init(void) { GraphicDevice *gdev = &ctfb;

•   struct udevice *dev;
    int bits_per_pixel;
  

•   pci_dev_t dev;
    int ret;
  
    printf("Video: ");
  

@@ -33,8 +33,8 @@ void *video_hw_init(void) return NULL; } if (vbe_get_video_info(gdev)) {

•           dev = pci_find_class(PCI_CLASS_DISPLAY_VGA << 8, 0);
  

•           if (dev < 0) {
  

•           ret = dm_pci_find_class(PCI_CLASS_DISPLAY_VGA << 8, 0, &dev);
  

•           if (ret) {
                    printf("no card detected\n");
                    return NULL;
            }
  

diff --git a/include/pci_rom.h b/include/pci_rom.h index 2f1665d..b4898e2 100644 --- a/include/pci_rom.h +++ b/include/pci_rom.h @@ -50,7 +50,7 @@ enum pci_rom_emul {

• @int15_handler: Function to call to handle int 0x15
• @exec_method: flags from enum pci_rom_emul

*/ -int pci_run_vga_bios(pci_dev_t dev, int (*int15_handler)(void), +int pci_run_vga_bios(struct udevice *dev, int (*int15_handler)(void), int exec_method);

/**

Regards, Bin

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

This function should take a struct udevice rather than pci_dev_t. Update it.

Signed-off-by: Simon Glass sjg@chromium.org

arch/x86/lib/bios.c | 3 ++- drivers/pci/pci_rom.c | 2 +- include/bios_emul.h | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/lib/bios.c b/arch/x86/lib/bios.c index 1d75cfc..2ad7219 100644 --- a/arch/x86/lib/bios.c +++ b/arch/x86/lib/bios.c @@ -242,9 +242,10 @@ static void vbe_set_graphics(int vesa_mode, struct vbe_mode_info *mode_info) vbe_set_mode(mode_info); }

-void bios_run_on_x86(pci_dev_t pcidev, unsigned long addr, int vesa_mode, +void bios_run_on_x86(struct udevice *dev, unsigned long addr, int vesa_mode, struct vbe_mode_info *mode_info) {

•   pci_dev_t pcidev = pci_get_bdf(dev);
    u32 num_dev;
  
    num_dev = PCI_BUS(pcidev) << 8 | PCI_DEV(pcidev) << 3 |
  

diff --git a/drivers/pci/pci_rom.c b/drivers/pci/pci_rom.c index 6e3adf6..1a857c5 100644 --- a/drivers/pci/pci_rom.c +++ b/drivers/pci/pci_rom.c @@ -336,7 +336,7 @@ int pci_run_vga_bios(struct udevice *dev, int (*int15_handler)(void), #ifdef CONFIG_X86 bios_set_interrupt_handler(0x15, int15_handler);

•           bios_run_on_x86(pci_get_bdf(dev), (unsigned long)ram, vesa_mode,
  

•           bios_run_on_x86(dev, (unsigned long)ram, vesa_mode,
                            &mode_info);
  

#endif } diff --git a/include/bios_emul.h b/include/bios_emul.h index 3643b82..80979ed 100644 --- a/include/bios_emul.h +++ b/include/bios_emul.h @@ -42,7 +42,7 @@ struct vbe_mode_info; int BootVideoCardBIOS(pci_dev_t pcidev, BE_VGAInfo **pVGAInfo, int cleanUp);

/* Run a BIOS ROM natively (only supported on x86 machines) */ -void bios_run_on_x86(pci_dev_t pcidev, unsigned long addr, int vesa_mode, +void bios_run_on_x86(struct udevice *dev, unsigned long addr, int vesa_mode, struct vbe_mode_info *mode_info);

/**

Reviewed-by: Bin Meng bmeng.cn@gmail.com

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

When driver model is being used, drop this old function.

Nits: can we reword this to something like the commit message in patch#13, like below?

Move these functions into the compatibility file so that they are not available by default.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/pci/pci_common.c | 51 ------------------------------------------------ drivers/pci/pci_compat.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/drivers/pci/pci_common.c b/drivers/pci/pci_common.c index a64792f..a78a812 100644 --- a/drivers/pci/pci_common.c +++ b/drivers/pci/pci_common.c @@ -312,54 +312,3 @@ pci_addr_t pci_hose_phys_to_bus(struct pci_controller *hose,

    return bus_addr;

}

-pci_dev_t pci_find_device(unsigned int vendor, unsigned int device, int index) -{

•   struct pci_device_id ids[2] = { {}, {0, 0} };
  

•   ids[0].vendor = vendor;
  

•   ids[0].device = device;
  

•   return pci_find_devices(ids, index);
  

-}

-pci_dev_t pci_hose_find_devices(struct pci_controller *hose, int busnum,

•                           struct pci_device_id *ids, int *indexp)
  

-{

•   int found_multi = 0;
  

•   u16 vendor, device;
  

•   u8 header_type;
  

•   pci_dev_t bdf;
  

•   int i;
  

•   for (bdf = PCI_BDF(busnum, 0, 0);
  

•        bdf < PCI_BDF(busnum + 1, 0, 0);
  

•        bdf += PCI_BDF(0, 0, 1)) {
  

•           if (pci_skip_dev(hose, bdf))
  

•                   continue;
  

•           if (!PCI_FUNC(bdf)) {
  

•                   pci_read_config_byte(bdf, PCI_HEADER_TYPE,
  

•                                        &header_type);
  

•                   found_multi = header_type & 0x80;
  

•           } else {
  

•                   if (!found_multi)
  

•                           continue;
  

•           }
  

•           pci_read_config_word(bdf, PCI_VENDOR_ID, &vendor);
  

•           pci_read_config_word(bdf, PCI_DEVICE_ID, &device);
  

•           for (i = 0; ids[i].vendor != 0; i++) {
  

•                   if (vendor == ids[i].vendor &&
  

•                       device == ids[i].device) {
  

•                           if ((*indexp) <= 0)
  

•                                   return bdf;
  

•                           (*indexp)--;
  

•                   }
  

•           }
  

•   }
  

•   return -1;
  

-} diff --git a/drivers/pci/pci_compat.c b/drivers/pci/pci_compat.c index 712c48f..688c98a 100644 --- a/drivers/pci/pci_compat.c +++ b/drivers/pci/pci_compat.c @@ -36,3 +36,54 @@ pci_dev_t pci_find_devices(struct pci_device_id *ids, int index) return -1; return pci_get_bdf(dev); }

+pci_dev_t pci_find_device(unsigned int vendor, unsigned int device, int index) +{

•   struct pci_device_id ids[2] = { {}, {0, 0} };
  

•   ids[0].vendor = vendor;
  

•   ids[0].device = device;
  

•   return pci_find_devices(ids, index);
  

+}

+pci_dev_t pci_hose_find_devices(struct pci_controller *hose, int busnum,

•                           struct pci_device_id *ids, int *indexp)
  

+{

•   int found_multi = 0;
  

•   u16 vendor, device;
  

•   u8 header_type;
  

•   pci_dev_t bdf;
  

•   int i;
  

•   for (bdf = PCI_BDF(busnum, 0, 0);
  

•        bdf < PCI_BDF(busnum + 1, 0, 0);
  

•        bdf += PCI_BDF(0, 0, 1)) {
  

•           if (pci_skip_dev(hose, bdf))
  

•                   continue;
  

•           if (!PCI_FUNC(bdf)) {
  

•                   pci_read_config_byte(bdf, PCI_HEADER_TYPE,
  

•                                        &header_type);
  

•                   found_multi = header_type & 0x80;
  

•           } else {
  

•                   if (!found_multi)
  

•                           continue;
  

•           }
  

•           pci_read_config_word(bdf, PCI_VENDOR_ID, &vendor);
  

•           pci_read_config_word(bdf, PCI_DEVICE_ID, &device);
  

•           for (i = 0; ids[i].vendor != 0; i++) {
  

•                   if (vendor == ids[i].vendor &&
  

•                       device == ids[i].device) {
  

•                           if ((*indexp) <= 0)
  

•                                   return bdf;
  

•                           (*indexp)--;
  

•                   }
  

•           }
  

•   }
  

•   return -1;
  

+}

Reviewed-by: Bin Meng bmeng.cn@gmail.com

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

Move this function into the compatibility file so that it is not available by default.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/pci/pci_common.c | 42 ------------------------------------------ drivers/pci/pci_compat.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/drivers/pci/pci_common.c b/drivers/pci/pci_common.c index a78a812..b46be1b 100644 --- a/drivers/pci/pci_common.c +++ b/drivers/pci/pci_common.c @@ -79,48 +79,6 @@ const char *pci_class_str(u8 class) }; }

-pci_dev_t pci_find_class(uint find_class, int index) -{

•   int bus;
  

•   int devnum;
  

•   pci_dev_t bdf;
  

•   uint32_t class;
  

•   for (bus = 0; bus <= pci_last_busno(); bus++) {
  

•           for (devnum = 0; devnum < PCI_MAX_PCI_DEVICES - 1; devnum++) {
  

•                   pci_read_config_dword(PCI_BDF(bus, devnum, 0),
  

•                                         PCI_CLASS_REVISION, &class);
  

•                   if (class >> 16 == 0xffff)
  

•                           continue;
  

•                   for (bdf = PCI_BDF(bus, devnum, 0);
  

•                                   bdf <= PCI_BDF(bus, devnum,
  

•                                           PCI_MAX_PCI_FUNCTIONS - 1);
  

•                                   bdf += PCI_BDF(0, 0, 1)) {
  

•                           pci_read_config_dword(bdf, PCI_CLASS_REVISION,
  

•                                                 &class);
  

•                           class >>= 8;
  

•                           if (class != find_class)
  

•                                   continue;
  

•                           /*
  

•                            * Decrement the index. We want to return the
  

•                            * correct device, so index is 0 for the first
  

•                            * matching device, 1 for the second, etc.
  

•                            */
  

•                           if (index) {
  

•                                   index--;
  

•                                   continue;
  

•                           }
  

•                           /* Return index'th controller. */
  

•                           return bdf;
  

•                   }
  

•           }
  

•   }
  

•   return -ENODEV;
  

-}

__weak int pci_skip_dev(struct pci_controller *hose, pci_dev_t dev) { /* diff --git a/drivers/pci/pci_compat.c b/drivers/pci/pci_compat.c index 688c98a..5f238ca 100644 --- a/drivers/pci/pci_compat.c +++ b/drivers/pci/pci_compat.c @@ -87,3 +87,45 @@ pci_dev_t pci_hose_find_devices(struct pci_controller *hose, int busnum,

    return -1;

}

+pci_dev_t pci_find_class(uint find_class, int index) +{

•   int bus;
  

•   int devnum;
  

•   pci_dev_t bdf;
  

•   uint32_t class;
  

•   for (bus = 0; bus <= pci_last_busno(); bus++) {
  

•           for (devnum = 0; devnum < PCI_MAX_PCI_DEVICES - 1; devnum++) {
  

•                   pci_read_config_dword(PCI_BDF(bus, devnum, 0),
  

•                                         PCI_CLASS_REVISION, &class);
  

•                   if (class >> 16 == 0xffff)
  

•                           continue;
  

•                   for (bdf = PCI_BDF(bus, devnum, 0);
  

•                                   bdf <= PCI_BDF(bus, devnum,
  

•                                           PCI_MAX_PCI_FUNCTIONS - 1);
  

•                                   bdf += PCI_BDF(0, 0, 1)) {
  

•                           pci_read_config_dword(bdf, PCI_CLASS_REVISION,
  

•                                                 &class);
  

•                           class >>= 8;
  

•                           if (class != find_class)
  

•                                   continue;
  

•                           /*
  

•                            * Decrement the index. We want to return the
  

•                            * correct device, so index is 0 for the first
  

•                            * matching device, 1 for the second, etc.
  

•                            */
  

•                           if (index) {
  

•                                   index--;
  

•                                   continue;
  

•                           }
  

•                           /* Return index'th controller. */
  

•                           return bdf;
  

•                   }
  

•           }
  

•   }
  

•   return -ENODEV;
  

+}

Reviewed-by: Bin Meng bmeng.cn@gmail.com

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

Adjust all Tegra boards to use driver model for Ethernet, now that the required drivers are converted.

Signed-off-by: Simon Glass sjg@chromium.org

arch/arm/mach-tegra/Kconfig | 1 + board/compulab/trimslice/trimslice.c | 8 -------- board/nvidia/cardhu/cardhu.c | 6 ------ board/nvidia/jetson-tk1/jetson-tk1.c | 6 ------ board/nvidia/p2371-2180/p2371-2180.c | 6 ------ board/toradex/apalis_t30/apalis_t30.c | 6 ------ 6 files changed, 1 insertion(+), 32 deletions(-)

diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig index 3906fc1..74ff6fca 100644 --- a/arch/arm/mach-tegra/Kconfig +++ b/arch/arm/mach-tegra/Kconfig @@ -14,6 +14,7 @@ config TEGRA_ARMV7_COMMON select DM_GPIO select DM_PCI select DM_PCI_COMPAT

•   select DM_ETH
  
  

choice prompt "Tegra SoC select" diff --git a/board/compulab/trimslice/trimslice.c b/board/compulab/trimslice/trimslice.c index c9da80d..723293f 100644 --- a/board/compulab/trimslice/trimslice.c +++ b/board/compulab/trimslice/trimslice.c @@ -13,7 +13,6 @@ #include <asm/arch/pinmux.h> #include <asm/gpio.h> #include <i2c.h> -#include <netdev.h>

void pin_mux_usb(void) { @@ -41,10 +40,3 @@ void pin_mux_mmc(void) /* For CD GPIO PP1 */ pinmux_tristate_disable(PMUX_PINGRP_DAP3); }

-#ifdef CONFIG_PCI -int board_eth_init(bd_t *bis) -{

•   return pci_eth_init(bis);
  

-} -#endif diff --git a/board/nvidia/cardhu/cardhu.c b/board/nvidia/cardhu/cardhu.c index 1540526..ba15e2e 100644 --- a/board/nvidia/cardhu/cardhu.c +++ b/board/nvidia/cardhu/cardhu.c @@ -13,7 +13,6 @@ #include <asm/gpio.h> #include "pinmux-config-cardhu.h" #include <i2c.h> -#include <netdev.h>

#define PMU_I2C_ADDRESS 0x2D #define MAX_I2C_RETRY 3 @@ -129,9 +128,4 @@ int tegra_pcie_board_init(void)

    return 0;

}

-int board_eth_init(bd_t *bis) -{

•   return pci_eth_init(bis);
  

-} #endif /* PCI */ diff --git a/board/nvidia/jetson-tk1/jetson-tk1.c b/board/nvidia/jetson-tk1/jetson-tk1.c index 52425a8..6f189aa 100644 --- a/board/nvidia/jetson-tk1/jetson-tk1.c +++ b/board/nvidia/jetson-tk1/jetson-tk1.c @@ -6,7 +6,6 @@ */

#include <common.h> -#include <netdev.h> #include <power/as3722.h>

#include <asm/arch/gpio.h> @@ -73,9 +72,4 @@ int tegra_pcie_board_init(void)

    return 0;

}

-int board_eth_init(bd_t *bis) -{

•   return pci_eth_init(bis);
  

-} #endif /* PCI */ diff --git a/board/nvidia/p2371-2180/p2371-2180.c b/board/nvidia/p2371-2180/p2371-2180.c index 57f577d8..0f587ea 100644 --- a/board/nvidia/p2371-2180/p2371-2180.c +++ b/board/nvidia/p2371-2180/p2371-2180.c @@ -6,7 +6,6 @@ */

#include <common.h> -#include <netdev.h> #include <i2c.h> #include <asm/arch/gpio.h> #include <asm/arch/pinmux.h> @@ -73,9 +72,4 @@ int tegra_pcie_board_init(void)

    return 0;

}

-int board_eth_init(bd_t *bis) -{

•   return pci_eth_init(bis);
  

-} #endif /* PCI */ diff --git a/board/toradex/apalis_t30/apalis_t30.c b/board/toradex/apalis_t30/apalis_t30.c index 879006f..3f56971 100644 --- a/board/toradex/apalis_t30/apalis_t30.c +++ b/board/toradex/apalis_t30/apalis_t30.c @@ -14,7 +14,6 @@ #include <asm/io.h> #include <dm.h> #include <i2c.h> -#include <netdev.h>

#include "pinmux-config-apalis_t30.h"

@@ -92,9 +91,4 @@ int tegra_pcie_board_init(void)

    return 0;

}

-int board_eth_init(bd_t *bis) -{

•   return pci_eth_init(bis);
  

-}

#endif /* CONFIG_PCI_TEGRA */

Reviewed-by: Bin Meng bmeng.cn@gmail.com

Hi Stephen,

On 18 November 2015 at 14:35, Stephen Warren swarren@wwwdotorg.org wrote:

On 11/16/2015 08:53 PM, Simon Glass wrote:

...

Adjust all Tegra boards to use driver model for Ethernet, now that the required drivers are converted.

...

diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig

...

@@ -14,6 +14,7 @@ config TEGRA_ARMV7_COMMON select DM_GPIO select DM_PCI select DM_PCI_COMPAT

•   select DM_ETH
  

This should be set for Tegra210 too. I suggest creating a "config TEGRA_COMMON" to contain all the shared settings, and making TEGRA_ARMV7_COMMON and TEGRA_ARMV8_COMMON (also new) select that.

Would you mind doing a patch for that? It seems like it should be a separate patch.

...

diff --git a/board/compulab/trimslice/trimslice.c b/board/compulab/trimslice/trimslice.c

...

-#ifdef CONFIG_PCI -int board_eth_init(bd_t *bis) -{

•   return pci_eth_init(bis);
  

-} -#endif

Has the need to call this function already been removed by this point in the series. In other words, does each commit in this series compile and support using Ethernet? If not, anyone running "git bisect" is going to be very sad.

It's connected with CONFIG_DM_ETH, so we should be OK.

Regards, Simon

Hi Bin,

On 17 November 2015 at 20:48, Bin Meng bmeng.cn@gmail.com wrote:

Hi Simon,

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

...

Update these tests to use driver model API functions.

Signed-off-by: Simon Glass sjg@chromium.org

test/dm/pci.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/test/dm/pci.c b/test/dm/pci.c index 3ab4ba8..a1750bf 100644 --- a/test/dm/pci.c +++ b/test/dm/pci.c @@ -35,20 +35,17 @@ DM_TEST(dm_test_pci_busnum, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); /* Test that we can use the swapcase device correctly */ static int dm_test_pci_swapcase(struct unit_test_state *uts) {

•   pci_dev_t pci_dev = PCI_BDF(0, 0x1f, 0);
  

•   struct pci_controller *hose;
  

•   struct udevice *bus, *swap;
  

•   struct udevice *emul, *swap;
    ulong io_addr, mem_addr;
    char *ptr;
  
    /* Check that asking for the device automatically fires up PCI */
  

•   ut_assertok(uclass_get_device(UCLASS_PCI_EMUL, 0, &swap));
  

•   ut_assertok(uclass_get_device(UCLASS_PCI, 0, &bus));
  

•   hose = dev_get_uclass_priv(bus);
  

•   ut_assertok(uclass_get_device(UCLASS_PCI_EMUL, 0, &emul));
  

•   ut_assertok(pci_bus_find_bdf(PCI_BDF(0, 0x1f, 0), &swap));
  

•   ut_assert(device_active(swap));
  

Why is this test needed? Isn't swap device already activated during the call to pci_bus_find_bdf()?

That function only finds the device, it does not probe it. Here, the probing is done by the line above, which gets the emulation device.

...

    /* First test I/O */

•   io_addr = pci_read_bar32(hose, pci_dev, 0);
  

•   io_addr = dm_pci_read_bar32(swap, 0);
    outb(2, io_addr);
    ut_asserteq(2, inb(io_addr));
  
  

@@ -56,7 +53,7 @@ static int dm_test_pci_swapcase(struct unit_test_state *uts) * Now test memory mapping - note we must unmap and remap to cause * the swapcase emulation to see our data and response. */

•   mem_addr = pci_read_bar32(hose, pci_dev, 1);
  

•   mem_addr = dm_pci_read_bar32(swap, 1);
    ptr = map_sysmem(mem_addr, 20);
    strcpy(ptr, "This is a TesT");
    unmap_sysmem(ptr);
  

--

Regards, Bin

Regards, Simon

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

Adjust these functions to use the driver model PCI API functions. At some point we should be able to remove these by requiring the caller to obtain the PCI bus and pass it to pci_bus_read/write_config(), or have a device driver and use dm_pci_config_read/write...().

Signed-off-by: Simon Glass sjg@chromium.org

arch/x86/cpu/pci.c | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c index 7a31260..f50183a 100644 --- a/arch/x86/cpu/pci.c +++ b/arch/x86/cpu/pci.c @@ -19,19 +19,30 @@

DECLARE_GLOBAL_DATA_PTR;

-static struct pci_controller *get_hose(void) +/*

• • TODO(sjg@chromium.org): Drop use of the x86_pci_read/write_config<n>()
• • functions from x86 code, and use dm_pci_read/write_config<n> instead.
• */

+static struct udevice *get_ctlr(void) {

•   struct udevice *dev;
  

•   int ret;
  

•   if (gd->hose)
  

•           return gd->hose;
  

•           return gd->hose->ctlr;
  

•   ret = uclass_first_device(UCLASS_PCI, &dev);
  

•   if (ret)
  

•           return ERR_PTR(ret);
  
  

•   return pci_bus_to_hose(0);
  

•   return dev;
  

}

unsigned int x86_pci_read_config8(pci_dev_t dev, unsigned where) {

•   uint8_t value;
  

•   ulong value;
  
  

•   if (pci_hose_read_config_byte(get_hose(), dev, where, &value))
  

•   if (pci_bus_read_config(get_ctlr(), dev, where, &value, PCI_SIZE_8))
            return -1U;
  
    return value;
  

@@ -39,9 +50,9 @@ unsigned int x86_pci_read_config8(pci_dev_t dev, unsigned where)

unsigned int x86_pci_read_config16(pci_dev_t dev, unsigned where) {

•   uint16_t value;
  

•   ulong value;
  
  

•   if (pci_hose_read_config_word(get_hose(), dev, where, &value))
  

•   if (pci_bus_read_config(get_ctlr(), dev, where, &value, PCI_SIZE_16))
            return -1U;
  
    return value;
  

@@ -49,9 +60,9 @@ unsigned int x86_pci_read_config16(pci_dev_t dev, unsigned where)

unsigned int x86_pci_read_config32(pci_dev_t dev, unsigned where) {

•   uint32_t value;
  

•   ulong value;
  
  

•   if (pci_hose_read_config_dword(get_hose(), dev, where, &value))
  

•   if (pci_bus_read_config(get_ctlr(), dev, where, &value, PCI_SIZE_32))
            return -1U;
  
    return value;
  

@@ -59,17 +70,17 @@ unsigned int x86_pci_read_config32(pci_dev_t dev, unsigned where)

void x86_pci_write_config8(pci_dev_t dev, unsigned where, unsigned value) {

•   pci_hose_write_config_byte(get_hose(), dev, where, value);
  

•   pci_bus_write_config(get_ctlr(), dev, where, value, PCI_SIZE_8);
  

}

void x86_pci_write_config16(pci_dev_t dev, unsigned where, unsigned value) {

•   pci_hose_write_config_word(get_hose(), dev, where, value);
  

•   pci_bus_write_config(get_ctlr(), dev, where, value, PCI_SIZE_16);
  

}

void x86_pci_write_config32(pci_dev_t dev, unsigned where, unsigned value) {

•   pci_hose_write_config_dword(get_hose(), dev, where, value);
  

•   pci_bus_write_config(get_ctlr(), dev, where, value, PCI_SIZE_32);
  

}

int pci_x86_read_config(struct udevice *bus, pci_dev_t bdf, uint offset,

Reviewed-by: Bin Meng bmeng.cn@gmail.com

Hi Simon,

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

At present the PCI address map functions use the old API. Add new functions for this so that drivers can be converted.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/pci/pci-uclass.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++ include/pci.h | 59 ++++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 4e37972..9c54e6c 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -11,6 +11,7 @@ #include <fdtdec.h> #include <inttypes.h> #include <pci.h> +#include <asm/io.h> #include <dm/lists.h> #include <dm/root.h> #include <dm/device-internal.h> @@ -1066,6 +1067,87 @@ u32 dm_pci_read_bar32(struct udevice *dev, int barnum) return addr & PCI_BASE_ADDRESS_MEM_MASK; }

+static int _dm_pci_hose_bus_to_phys(struct udevice *ctlr,

Why underscore? I understand this is an internal function, but we don't use underscore for many other internal function names. Also it takes udevice as the parameter, I think we should avoid using 'hose' in the function name.

•                               pci_addr_t bus_addr, unsigned long flags,
  

•                               unsigned long skip_mask, phys_addr_t *pa)
  

+{

•   struct pci_controller *hose = dev_get_uclass_priv(ctlr);
  

•   struct pci_region *res;
  

•   int i;
  

•   for (i = 0; i < hose->region_count; i++) {
  

•           res = &hose->regions[i];
  

•           if (((res->flags ^ flags) & PCI_REGION_TYPE) != 0)
  

•                   continue;
  

•           if (res->flags & skip_mask)
  

•                   continue;
  

•           if (bus_addr >= res->bus_start &&
  

•               (bus_addr - res->bus_start) < res->size) {
  

•                   *pa = (bus_addr - res->bus_start + res->phys_start);
  

•                   return 0;
  

•           }
  

•   }
  

•   return 1;
  

+}

+phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t bus_addr,

•                          unsigned long flags)
  

+{

•   phys_addr_t phys_addr = 0;
  

•   struct udevice *ctlr;
  

•   int ret;
  

•   /* The root controller has the region information */
  

•   ctlr = pci_get_controller(dev);
  

•   /*
  

•    * if PCI_REGION_MEM is set we do a two pass search with preference
  

•    * on matches that don't have PCI_REGION_SYS_MEMORY set
  

•    */
  

•   if ((flags & PCI_REGION_TYPE) == PCI_REGION_MEM) {
  

•           ret = _dm_pci_hose_bus_to_phys(ctlr, bus_addr,
  

•                                          flags, PCI_REGION_SYS_MEMORY,
  

•                                          &phys_addr);
  

•           if (!ret)
  

•                   return phys_addr;
  

•   }
  

•   ret = _dm_pci_hose_bus_to_phys(ctlr, bus_addr, flags, 0, &phys_addr);
  

•   if (ret)
  

•           puts("pci_hose_bus_to_phys: invalid physical address\n");
  

•   return phys_addr;
  

+}

+pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t addr,

•                         unsigned long flags)
  

+{

•   return 0;
  

Why returning 0? This looks wrong to me as every physical address will be mapped to pci address zero?

+}

+void *dm_pci_map_bar(struct udevice *dev, int bar, int flags) +{

•   pci_addr_t pci_bus_addr;
  

•   u32 bar_response;
  

•   /* read BAR address */
  

•   dm_pci_read_config32(dev, bar, &bar_response);
  

•   pci_bus_addr = (pci_addr_t)(bar_response & ~0xf);
  

•   /*
  

•    * Pass "0" as the length argument to pci_bus_to_virt.  The arg
  

•    * isn't actualy used on any platform because u-boot assumes a static
  

•    * linear mapping.  In the future, this could read the BAR size
  

•    * and pass that as the size if needed.
  

•    */
  

•   return dm_pci_bus_to_virt(dev, pci_bus_addr, flags, 0, MAP_NOCACHE);
  

+}

UCLASS_DRIVER(pci) = { .id = UCLASS_PCI, .name = "pci", diff --git a/include/pci.h b/include/pci.h index f1c0d71..ddf6b66 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1183,6 +1183,65 @@ void dm_pciauto_region_init(struct pci_region *res); u32 dm_pci_read_bar32(struct udevice *dev, int barnum);

/**

• • dm_pci_bus_to_phys() - convert a PCI bus address to a physical address
• • @dev: Device containing the PCI address
• • @addr: PCI address to convert
• • @flags: Flags for the region type (PCI_REGION_...)
• • @return physical address corresponding to that PCI bus address
• */

+phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t addr,

•                          unsigned long flags);
  

+/**

• • dm_pci_phys_to_bus() - convert a physical address to a PCI bus address
• • @dev: Device containing the bus address
• • @addr: Physical address to convert
• • @flags: Flags for the region type (PCI_REGION_...)
• • @return PCI bus address corresponding to that physical address
• */

+pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t addr,

•                         unsigned long flags);
  

+/**

• • dm_pci_map_bar() - get a physical address associated with a BAR region
• • Looks up a base address register and finds the physical memory address
• • that corresponds to it
• • @dev: Device to check
• • @bar: Bar number to read (numbered from 0)
• • @flags: Flags for the region type (PCI_REGION_...)
• • @return: pointer to the physical address to use
• */

+void *dm_pci_map_bar(struct udevice *dev, int bar, int flags);

+#define dm_pci_virt_to_bus(dev, addr, flags) \

•   dm_pci_phys_to_bus(dev, (virt_to_phys(addr)), (flags))
  

+#define dm_pci_bus_to_virt(dev, addr, flags, len, map_flags) \

•   map_physmem(dm_pci_bus_to_phys(dev, (addr), (flags)), \
  

•               (len), (map_flags))
  

+#define dm_pci_phys_to_mem(dev, addr) \

•   dm_pci_phys_to_bus((dev), (addr), PCI_REGION_MEM)
  

+#define dm_pci_mem_to_phys(dev, addr) \

•   dm_pci_bus_to_phys((dev), (addr), PCI_REGION_MEM)
  

+#define dm_pci_phys_to_io(dev, addr) \

•   dm_pci_phys_to_bus((dev), (addr), PCI_REGION_IO)
  

+#define dm_pci_io_to_phys(dev, addr) \

•   dm_pci_bus_to_phys((dev), (addr), PCI_REGION_IO)
  

+#define dm_pci_virt_to_mem(dev, addr) \

•   dm_pci_virt_to_bus((dev), (addr), PCI_REGION_MEM)
  

+#define dm_pci_mem_to_virt(dev, addr, len, map_flags) \

•   dm_pci_bus_to_virt((dev), (addr), PCI_REGION_MEM, (len), (map_flags))
  

+#define dm_pci_virt_to_io(dev, addr) \

•   dm_dm_pci_virt_to_bus((dev), (addr), PCI_REGION_IO)
  

+#define dm_pci_io_to_virt(dev, addr, len, map_flags) \

•   dm_dm_pci_bus_to_virt((dev), (addr), PCI_REGION_IO, (len), (map_flags))
  

+/**

• dm_pci_find_device() - find a device by vendor/device ID
• @vendor: Vendor ID

--

Regards, Bin

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

Update this driver to use the proper driver-model PCI API functions.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/net/rtl8169.c | 88 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 70 insertions(+), 18 deletions(-)

diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c index 19422c4..0a93668 100644 --- a/drivers/net/rtl8169.c +++ b/drivers/net/rtl8169.c @@ -513,8 +513,13 @@ static void rtl_flush_buffer(void *buf, size_t size) /************************************************************************** RECV - Receive a frame ***************************************************************************/ -static int rtl_recv_common(pci_dev_t bdf, unsigned long dev_iobase, +#ifdef CONFIG_DM_ETH +static int rtl_recv_common(struct udevice *dev, unsigned long dev_iobase,

•                      uchar **packetp)
  

+#else +static int rtl_recv_common(pci_dev_t dev, unsigned long dev_iobase, uchar **packetp) +#endif { /* return true if there's an ethernet packet ready to read */ /* nic->packet should contain data on return */ @@ -545,9 +550,16 @@ static int rtl_recv_common(pci_dev_t bdf, unsigned long dev_iobase, else tpc->RxDescArray[cur_rx].status = cpu_to_le32(OWNbit + RX_BUF_SIZE); +#ifdef CONFIG_DM_ETH tpc->RxDescArray[cur_rx].buf_addr = cpu_to_le32(

•                           pci_mem_to_phys(bdf, (pci_addr_t)(unsigned long)
  

•                           dm_pci_mem_to_phys(dev,
  

•                                   (pci_addr_t)(unsigned long)
  

•                                   tpc->RxBufferRing[cur_rx]));
  

+#else

•                   tpc->RxDescArray[cur_rx].buf_addr = cpu_to_le32(
  

•                           pci_mem_to_phys(dev, (pci_addr_t)(unsigned long)
                            tpc->RxBufferRing[cur_rx]));
  

+#endif rtl_flush_rx_desc(&tpc->RxDescArray[cur_rx]); #ifdef CONFIG_DM_ETH *packetp = rxdata; @@ -576,7 +588,7 @@ int rtl8169_eth_recv(struct udevice *dev, int flags, uchar **packetp) { struct rtl8169_private *priv = dev_get_priv(dev);

•   return rtl_recv_common(pci_get_bdf(dev), priv->iobase, packetp);
  

•   return rtl_recv_common(dev, priv->iobase, packetp);
  

} #else static int rtl_recv(struct eth_device *dev) @@ -590,8 +602,13 @@ static int rtl_recv(struct eth_device *dev) /************************************************************************** SEND - Transmit a frame ***************************************************************************/ -static int rtl_send_common(pci_dev_t bdf, unsigned long dev_iobase, +#ifdef CONFIG_DM_ETH +static int rtl_send_common(struct udevice *dev, unsigned long dev_iobase, void *packet, int length) +#else +static int rtl_send_common(pci_dev_t dev, unsigned long dev_iobase,

•                      void *packet, int length)
  

+#endif { /* send the packet to destination */

@@ -618,8 +635,13 @@ static int rtl_send_common(pci_dev_t bdf, unsigned long dev_iobase, ptxb[len++] = '\0';

    tpc->TxDescArray[entry].buf_Haddr = 0;

+#ifdef CONFIG_DM_ETH

•   tpc->TxDescArray[entry].buf_addr = cpu_to_le32(
  

•           dm_pci_mem_to_phys(dev, (pci_addr_t)(unsigned long)ptxb));
  

+#else tpc->TxDescArray[entry].buf_addr = cpu_to_le32(

•           pci_mem_to_phys(bdf, (pci_addr_t)(unsigned long)ptxb));
  

•           pci_mem_to_phys(dev, (pci_addr_t)(unsigned long)ptxb));
  

+#endif if (entry != (NUM_TX_DESC - 1)) { tpc->TxDescArray[entry].status = cpu_to_le32((OWNbit | FSbit | LSbit) | @@ -661,7 +683,7 @@ int rtl8169_eth_send(struct udevice *dev, void *packet, int length) { struct rtl8169_private *priv = dev_get_priv(dev);

•   return rtl_send_common(pci_get_bdf(dev), priv->iobase, packet, length);
  

•   return rtl_send_common(dev, priv->iobase, packet, length);
  

}

#else @@ -695,7 +717,11 @@ static void rtl8169_set_rx_mode(void) RTL_W32(MAR0 + 4, mc_filter[1]); }

-static void rtl8169_hw_start(pci_dev_t bdf) +#ifdef CONFIG_DM_ETH +static void rtl8169_hw_start(struct udevice *dev) +#else +static void rtl8169_hw_start(pci_dev_t dev) +#endif { u32 i;

@@ -740,11 +766,21 @@ static void rtl8169_hw_start(pci_dev_t bdf)

    tpc->cur_rx = 0;

•   RTL_W32(TxDescStartAddrLow, pci_mem_to_phys(bdf,
  

+#ifdef CONFIG_DM_ETH

•   RTL_W32(TxDescStartAddrLow, dm_pci_mem_to_phys(dev,
  

•                   (pci_addr_t)(unsigned long)tpc->TxDescArray));
  

+#else

•   RTL_W32(TxDescStartAddrLow, pci_mem_to_phys(dev,
                    (pci_addr_t)(unsigned long)tpc->TxDescArray));
  

+#endif RTL_W32(TxDescStartAddrHigh, (unsigned long)0); +#ifdef CONFIG_DM_ETH

•   RTL_W32(RxDescStartAddrLow, dm_pci_mem_to_phys(
  

•                   dev, (pci_addr_t)(unsigned long)tpc->RxDescArray));
  

+#else RTL_W32(RxDescStartAddrLow, pci_mem_to_phys(

•                   bdf, (pci_addr_t)(unsigned long)tpc->RxDescArray));
  

•                   dev, (pci_addr_t)(unsigned long)tpc->RxDescArray));
  

+#endif RTL_W32(RxDescStartAddrHigh, (unsigned long)0);

    /* RTL-8169sc/8110sc or later version */

@@ -766,7 +802,11 @@ static void rtl8169_hw_start(pci_dev_t bdf) #endif }

-static void rtl8169_init_ring(pci_dev_t bdf) +#ifdef CONFIG_DM_ETH +static void rtl8169_init_ring(struct udevice *dev) +#else +static void rtl8169_init_ring(pci_dev_t dev) +#endif { int i;

@@ -794,8 +834,13 @@ static void rtl8169_init_ring(pci_dev_t bdf) cpu_to_le32(OWNbit + RX_BUF_SIZE);

            tpc->RxBufferRing[i] = &rxb[i * RX_BUF_SIZE];

+#ifdef CONFIG_DM_ETH

•           tpc->RxDescArray[i].buf_addr = cpu_to_le32(dm_pci_mem_to_phys(
  

•                   dev, (pci_addr_t)(unsigned long)tpc->RxBufferRing[i]));
  

+#else tpc->RxDescArray[i].buf_addr = cpu_to_le32(pci_mem_to_phys(

•                   bdf, (pci_addr_t)(unsigned long)tpc->RxBufferRing[i]));
  

•                   dev, (pci_addr_t)(unsigned long)tpc->RxBufferRing[i]));
  

+#endif rtl_flush_rx_desc(&tpc->RxDescArray[i]); }

@@ -804,7 +849,11 @@ static void rtl8169_init_ring(pci_dev_t bdf) #endif }

-static void rtl8169_common_start(pci_dev_t bdf, unsigned char *enetaddr) +#ifdef CONFIG_DM_ETH +static void rtl8169_common_start(struct udevice *dev, unsigned char *enetaddr) +#else +static void rtl8169_common_start(pci_dev_t dev, unsigned char *enetaddr) +#endif { int i;

@@ -813,8 +862,8 @@ static void rtl8169_common_start(pci_dev_t bdf, unsigned char *enetaddr) printf ("%s\n", __FUNCTION__); #endif

•   rtl8169_init_ring(bdf);
  

•   rtl8169_hw_start(bdf);
  

•   rtl8169_init_ring(dev);
  

•   rtl8169_hw_start(dev);
    /* Construct a perfect filter frame with the mac address as first match
     * and broadcast for all others */
    for (i = 0; i < 192; i++)
  

@@ -837,7 +886,7 @@ static int rtl8169_eth_start(struct udevice *dev) { struct eth_pdata *plat = dev_get_platdata(dev);

•   rtl8169_common_start(pci_get_bdf(dev), plat->enetaddr);
  

•   rtl8169_common_start(dev, plat->enetaddr);
  
    return 0;
  

} @@ -1130,10 +1179,13 @@ static int rtl8169_eth_probe(struct udevice *dev) region = 1; break; }

•   pci_read_config32(pci_get_bdf(dev), PCI_BASE_ADDRESS_0 + region * 4,
  

•                     &iobase);
  

•   dm_pci_read_config32(dev, PCI_BASE_ADDRESS_0 + region * 4, &iobase);
    iobase &= ~0xf;
  

•   priv->iobase = (int)pci_mem_to_phys(pci_get_bdf(dev), iobase);
  

+#ifdef CONFIG_DM_ETH

•   priv->iobase = (int)dm_pci_mem_to_phys(dev, iobase);
  

+#else

•   priv->iobase = (int)pci_mem_to_phys(dev, iobase);
  

+#endif

    ret = rtl_init(priv->iobase, dev->name, plat->enetaddr);
    if (ret < 0) {

--

Reviewed-by: Bin Meng bmeng.cn@gmail.com

Hi Joe,

On 23 November 2015 at 16:32, Joe Hershberger joe.hershberger@gmail.com wrote:

Hi Simon,

On Mon, Nov 16, 2015 at 9:53 PM, Simon Glass sjg@chromium.org wrote:

...

Update this driver to use the proper driver-model PCI API functions.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/net/rtl8169.c | 88 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 70 insertions(+), 18 deletions(-)

diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c index 19422c4..0a93668 100644 --- a/drivers/net/rtl8169.c +++ b/drivers/net/rtl8169.c @@ -513,8 +513,13 @@ static void rtl_flush_buffer(void *buf, size_t size) /************************************************************************** RECV - Receive a frame ***************************************************************************/ -static int rtl_recv_common(pci_dev_t bdf, unsigned long dev_iobase, +#ifdef CONFIG_DM_ETH +static int rtl_recv_common(struct udevice *dev, unsigned long dev_iobase,

•                      uchar **packetp)
  

+#else +static int rtl_recv_common(pci_dev_t dev, unsigned long dev_iobase, uchar **packetp) +#endif { /* return true if there's an ethernet packet ready to read */ /* nic->packet should contain data on return */ @@ -545,9 +550,16 @@ static int rtl_recv_common(pci_dev_t bdf, unsigned long dev_iobase, else tpc->RxDescArray[cur_rx].status = cpu_to_le32(OWNbit + RX_BUF_SIZE); +#ifdef CONFIG_DM_ETH tpc->RxDescArray[cur_rx].buf_addr = cpu_to_le32(

•                           pci_mem_to_phys(bdf, (pci_addr_t)(unsigned long)
  

•                           dm_pci_mem_to_phys(dev,
  

•                                   (pci_addr_t)(unsigned long)
  

•                                   tpc->RxBufferRing[cur_rx]));
  

+#else

•                   tpc->RxDescArray[cur_rx].buf_addr = cpu_to_le32(
  

•                           pci_mem_to_phys(dev, (pci_addr_t)(unsigned long)
                            tpc->RxBufferRing[cur_rx]));
  

+#endif rtl_flush_rx_desc(&tpc->RxDescArray[cur_rx]); #ifdef CONFIG_DM_ETH *packetp = rxdata; @@ -576,7 +588,7 @@ int rtl8169_eth_recv(struct udevice *dev, int flags, uchar **packetp) { struct rtl8169_private *priv = dev_get_priv(dev);

•   return rtl_recv_common(pci_get_bdf(dev), priv->iobase, packetp);
  

•   return rtl_recv_common(dev, priv->iobase, packetp);
  

} #else static int rtl_recv(struct eth_device *dev) @@ -590,8 +602,13 @@ static int rtl_recv(struct eth_device *dev) /************************************************************************** SEND - Transmit a frame ***************************************************************************/ -static int rtl_send_common(pci_dev_t bdf, unsigned long dev_iobase, +#ifdef CONFIG_DM_ETH +static int rtl_send_common(struct udevice *dev, unsigned long dev_iobase, void *packet, int length) +#else +static int rtl_send_common(pci_dev_t dev, unsigned long dev_iobase,

•                      void *packet, int length)
  

+#endif { /* send the packet to destination */

@@ -618,8 +635,13 @@ static int rtl_send_common(pci_dev_t bdf, unsigned long dev_iobase, ptxb[len++] = '\0';

    tpc->TxDescArray[entry].buf_Haddr = 0;

+#ifdef CONFIG_DM_ETH

•   tpc->TxDescArray[entry].buf_addr = cpu_to_le32(
  

•           dm_pci_mem_to_phys(dev, (pci_addr_t)(unsigned long)ptxb));
  

+#else tpc->TxDescArray[entry].buf_addr = cpu_to_le32(

•           pci_mem_to_phys(bdf, (pci_addr_t)(unsigned long)ptxb));
  

•           pci_mem_to_phys(dev, (pci_addr_t)(unsigned long)ptxb));
  

+#endif if (entry != (NUM_TX_DESC - 1)) { tpc->TxDescArray[entry].status = cpu_to_le32((OWNbit | FSbit | LSbit) | @@ -661,7 +683,7 @@ int rtl8169_eth_send(struct udevice *dev, void *packet, int length) { struct rtl8169_private *priv = dev_get_priv(dev);

•   return rtl_send_common(pci_get_bdf(dev), priv->iobase, packet, length);
  

•   return rtl_send_common(dev, priv->iobase, packet, length);
  

}

#else @@ -695,7 +717,11 @@ static void rtl8169_set_rx_mode(void) RTL_W32(MAR0 + 4, mc_filter[1]); }

-static void rtl8169_hw_start(pci_dev_t bdf) +#ifdef CONFIG_DM_ETH +static void rtl8169_hw_start(struct udevice *dev) +#else +static void rtl8169_hw_start(pci_dev_t dev) +#endif { u32 i;

@@ -740,11 +766,21 @@ static void rtl8169_hw_start(pci_dev_t bdf)

    tpc->cur_rx = 0;

•   RTL_W32(TxDescStartAddrLow, pci_mem_to_phys(bdf,
  

+#ifdef CONFIG_DM_ETH

•   RTL_W32(TxDescStartAddrLow, dm_pci_mem_to_phys(dev,
  

•                   (pci_addr_t)(unsigned long)tpc->TxDescArray));
  

+#else

•   RTL_W32(TxDescStartAddrLow, pci_mem_to_phys(dev,
                    (pci_addr_t)(unsigned long)tpc->TxDescArray));
  

+#endif RTL_W32(TxDescStartAddrHigh, (unsigned long)0); +#ifdef CONFIG_DM_ETH

•   RTL_W32(RxDescStartAddrLow, dm_pci_mem_to_phys(
  

•                   dev, (pci_addr_t)(unsigned long)tpc->RxDescArray));
  

+#else RTL_W32(RxDescStartAddrLow, pci_mem_to_phys(

•                   bdf, (pci_addr_t)(unsigned long)tpc->RxDescArray));
  

•                   dev, (pci_addr_t)(unsigned long)tpc->RxDescArray));
  

+#endif RTL_W32(RxDescStartAddrHigh, (unsigned long)0);

    /* RTL-8169sc/8110sc or later version */

@@ -766,7 +802,11 @@ static void rtl8169_hw_start(pci_dev_t bdf) #endif }

-static void rtl8169_init_ring(pci_dev_t bdf) +#ifdef CONFIG_DM_ETH +static void rtl8169_init_ring(struct udevice *dev) +#else +static void rtl8169_init_ring(pci_dev_t dev) +#endif { int i;

@@ -794,8 +834,13 @@ static void rtl8169_init_ring(pci_dev_t bdf) cpu_to_le32(OWNbit + RX_BUF_SIZE);

            tpc->RxBufferRing[i] = &rxb[i * RX_BUF_SIZE];

+#ifdef CONFIG_DM_ETH

•           tpc->RxDescArray[i].buf_addr = cpu_to_le32(dm_pci_mem_to_phys(
  

•                   dev, (pci_addr_t)(unsigned long)tpc->RxBufferRing[i]));
  

+#else tpc->RxDescArray[i].buf_addr = cpu_to_le32(pci_mem_to_phys(

•                   bdf, (pci_addr_t)(unsigned long)tpc->RxBufferRing[i]));
  

•                   dev, (pci_addr_t)(unsigned long)tpc->RxBufferRing[i]));
  

+#endif rtl_flush_rx_desc(&tpc->RxDescArray[i]); }

@@ -804,7 +849,11 @@ static void rtl8169_init_ring(pci_dev_t bdf) #endif }

-static void rtl8169_common_start(pci_dev_t bdf, unsigned char *enetaddr) +#ifdef CONFIG_DM_ETH +static void rtl8169_common_start(struct udevice *dev, unsigned char *enetaddr) +#else +static void rtl8169_common_start(pci_dev_t dev, unsigned char *enetaddr) +#endif { int i;

@@ -813,8 +862,8 @@ static void rtl8169_common_start(pci_dev_t bdf, unsigned char *enetaddr) printf ("%s\n", __FUNCTION__); #endif

•   rtl8169_init_ring(bdf);
  

•   rtl8169_hw_start(bdf);
  

•   rtl8169_init_ring(dev);
  

•   rtl8169_hw_start(dev);
    /* Construct a perfect filter frame with the mac address as first match
     * and broadcast for all others */
    for (i = 0; i < 192; i++)
  

@@ -837,7 +886,7 @@ static int rtl8169_eth_start(struct udevice *dev) { struct eth_pdata *plat = dev_get_platdata(dev);

•   rtl8169_common_start(pci_get_bdf(dev), plat->enetaddr);
  

•   rtl8169_common_start(dev, plat->enetaddr);
  
    return 0;
  

} @@ -1130,10 +1179,13 @@ static int rtl8169_eth_probe(struct udevice *dev) region = 1; break; }

•   pci_read_config32(pci_get_bdf(dev), PCI_BASE_ADDRESS_0 + region * 4,
  

•                     &iobase);
  

•   dm_pci_read_config32(dev, PCI_BASE_ADDRESS_0 + region * 4, &iobase);
  

Should you be using this dm_* call for the non-dm case? It seems like no, since it won't compile when dev is of type pci_dev_t.

This is actually the DM case. The patch is confusing - I tidied this up in v2.

...

    iobase &= ~0xf;

•   priv->iobase = (int)pci_mem_to_phys(pci_get_bdf(dev), iobase);
  

+#ifdef CONFIG_DM_ETH

•   priv->iobase = (int)dm_pci_mem_to_phys(dev, iobase);
  

+#else

•   priv->iobase = (int)pci_mem_to_phys(dev, iobase);
  

+#endif

    ret = rtl_init(priv->iobase, dev->name, plat->enetaddr);
    if (ret < 0) {

-- 2.6.0.rc2.230.g3dd15c0

Regards, Simon

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

We should use the new address mapping functions unless we are in compatibility mode. Disable the old functions by default.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/pci/pci_common.c | 2 ++ include/pci.h | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci_common.c b/drivers/pci/pci_common.c index b46be1b..c685541 100644 --- a/drivers/pci/pci_common.c +++ b/drivers/pci/pci_common.c @@ -99,6 +99,7 @@ __weak int pci_skip_dev(struct pci_controller *hose, pci_dev_t dev) return 0; }

+#if !defined(CONFIG_DM_PCI) || defined(CONFIG_DM_PCI_COMPAT) /* Get a virtual address associated with a BAR region */ void *pci_map_bar(pci_dev_t pdev, int bar, int flags) { @@ -270,3 +271,4 @@ pci_addr_t pci_hose_phys_to_bus(struct pci_controller *hose,

    return bus_addr;

} +#endif /* !CONFIG_DM_PCI || CONFIG_DM_PCI_COMPAT */ diff --git a/include/pci.h b/include/pci.h index ddf6b66..3584ccb 100644 --- a/include/pci.h +++ b/include/pci.h @@ -621,6 +621,7 @@ static inline void pci_set_ops(struct pci_controller *hose, extern void pci_setup_indirect(struct pci_controller* hose, u32 cfg_addr, u32 cfg_data); #endif

+#if !defined(CONFIG_DM_PCI) || defined(CONFIG_DM_PCI_COMPAT) extern phys_addr_t pci_hose_bus_to_phys(struct pci_controller* hose, pci_addr_t addr, unsigned long flags); extern pci_addr_t pci_hose_phys_to_bus(struct pci_controller* hose, @@ -656,7 +657,6 @@ extern pci_addr_t pci_hose_phys_to_bus(struct pci_controller* hose, pci_bus_to_virt((dev), (addr), PCI_REGION_IO, (len), (map_flags))

/* For driver model these are defined in macros in pci_compat.c */ -#if !defined(CONFIG_DM_PCI) || defined(CONFIG_DM_PCI_COMPAT) extern int pci_hose_read_config_byte(struct pci_controller *hose, pci_dev_t dev, int where, u8 *val); extern int pci_hose_read_config_word(struct pci_controller *hose, --

Reviewed-by: Bin Meng bmeng.cn@gmail.com

Hi Simon,

On Tue, Nov 17, 2015 at 11:53 AM, Simon Glass sjg@chromium.org wrote:

Convert this driver to use the new driver model PCI API.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/block/ahci.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++--- include/ahci.h | 4 ++++ 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/drivers/block/ahci.c b/drivers/block/ahci.c index 1ad638e..9c76b2e 100644 --- a/drivers/block/ahci.c +++ b/drivers/block/ahci.c @@ -168,7 +168,11 @@ int ahci_reset(void __iomem *base) static int ahci_host_init(struct ahci_probe_ent *probe_ent) { #ifndef CONFIG_SCSI_AHCI_PLAT +# ifdef CONFIG_DM_PCI

•   struct udevice *dev = probe_ent->dev;
  

+# else pci_dev_t pdev = probe_ent->dev; +# endif u16 tmp16; unsigned short vendor; #endif @@ -193,6 +197,16 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) writel_with_flush(0xf, mmio + HOST_PORTS_IMPL);

#ifndef CONFIG_SCSI_AHCI_PLAT +# ifdef CONFIG_DM_PCI

•   dm_pci_read_config16(dev, PCI_VENDOR_ID, &vendor);
  

We should get the vendor via dev_get_parent_platdata(dev).

•   if (vendor == PCI_VENDOR_ID_INTEL) {
  

•           u16 tmp16;
  

•           dm_pci_read_config16(dev, 0x92, &tmp16);
  

•           dm_pci_write_config16(dev, 0x92, tmp16 | 0xf);
  

•   }
  

+# else pci_read_config_word(pdev, PCI_VENDOR_ID, &vendor);

    if (vendor == PCI_VENDOR_ID_INTEL) {

@@ -201,6 +215,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) tmp16 |= 0xf; pci_write_config_word(pdev, 0x92, tmp16); } +# endif #endif probe_ent->cap = readl(mmio + HOST_CAP); probe_ent->port_map = readl(mmio + HOST_PORTS_IMPL); @@ -313,9 +328,15 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) tmp = readl(mmio + HOST_CTL); debug("HOST_CTL 0x%x\n", tmp); #ifndef CONFIG_SCSI_AHCI_PLAT +# ifdef CONFIG_DM_PCI

•   dm_pci_read_config16(dev, PCI_COMMAND, &tmp16);
  

•   tmp |= PCI_COMMAND_MASTER;
  

•   dm_pci_write_config16(dev, PCI_COMMAND, tmp16);
  

+# else pci_read_config_word(pdev, PCI_COMMAND, &tmp16); tmp |= PCI_COMMAND_MASTER; pci_write_config_word(pdev, PCI_COMMAND, tmp16); +# endif #endif return 0; } @@ -324,7 +345,11 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) static void ahci_print_info(struct ahci_probe_ent *probe_ent) { #ifndef CONFIG_SCSI_AHCI_PLAT +# ifdef CONFIG_DM_PCI

•   struct udevice *dev = probe_ent->dev;
  

+# else pci_dev_t pdev = probe_ent->dev; +# endif u16 cc; #endif void __iomem *mmio = probe_ent->mmio_base; @@ -350,7 +375,11 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent) #ifdef CONFIG_SCSI_AHCI_PLAT scc_s = "SATA"; #else +# ifdef CONFIG_DM_PCI

•   dm_pci_read_config16(dev, 0x0a, &cc);
  

+# else pci_read_config_word(pdev, 0x0a, &cc); +# endif if (cc == 0x0101) scc_s = "IDE"; else if (cc == 0x0106) @@ -395,7 +424,11 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent) }

#ifndef CONFIG_SCSI_AHCI_PLAT -static int ahci_init_one(pci_dev_t pdev) +# ifdef CONFIG_DM_PCI +static int ahci_init_one(struct udevice *dev) +# else +static int ahci_init_one(pci_dev_t dev) +# endif { u16 vendor; int rc; @@ -407,7 +440,7 @@ static int ahci_init_one(pci_dev_t pdev) }

    memset(probe_ent, 0, sizeof(struct ahci_probe_ent));

•   probe_ent->dev = pdev;
  

•   probe_ent->dev = dev;
  
    probe_ent->host_flags = ATA_FLAG_SATA
                            | ATA_FLAG_NO_LEGACY
  

@@ -417,9 +450,20 @@ static int ahci_init_one(pci_dev_t pdev) probe_ent->pio_mask = 0x1f; probe_ent->udma_mask = 0x7f; /*Fixme,assume to support UDMA6 */

+#ifdef CONFIG_DM_PCI

•   probe_ent->mmio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_5,
  

•                                         PCI_REGION_MEM);
  

•   /* Take from kernel:
  

•    * JMicron-specific fixup:
  

•    * make sure we're in AHCI mode
  

•    */
  

•   dm_pci_read_config16(dev, PCI_VENDOR_ID, &vendor);
  

•   if (vendor == 0x197b)
  

•           dm_pci_write_config8(dev, 0x41, 0xa1);
  

+#else probe_ent->mmio_base = pci_map_bar(pdev, PCI_BASE_ADDRESS_5, PCI_REGION_MEM);

•   debug("ahci mmio_base=0x%p\n", probe_ent->mmio_base);
  
    /* Take from kernel:
     * JMicron-specific fixup:
  

@@ -428,7 +472,9 @@ static int ahci_init_one(pci_dev_t pdev) pci_read_config_word(pdev, PCI_VENDOR_ID, &vendor); if (vendor == 0x197b) pci_write_config_byte(pdev, 0x41, 0xa1); +#endif

•   debug("ahci mmio_base=0x%p\n", probe_ent->mmio_base);
    /* initialize adapter */
    rc = ahci_host_init(probe_ent);
    if (rc)
  

@@ -915,7 +961,17 @@ void scsi_low_level_init(int busdevfunc) u32 linkmap;

#ifndef CONFIG_SCSI_AHCI_PLAT +# ifdef CONFIG_DM_PCI

•   struct udevice *dev;
  

•   int ret;
  

•   ret = pci_bus_find_bdf(busdevfunc, &dev);
  

•   if (ret)
  

•           return;
  

•   ahci_init_one(dev);
  

+# else ahci_init_one(busdevfunc); +# endif #endif

    linkmap = probe_ent->link_port_map;

diff --git a/include/ahci.h b/include/ahci.h index 0bdedac..a956c6f 100644 --- a/include/ahci.h +++ b/include/ahci.h @@ -145,7 +145,11 @@ struct ahci_ioports { };

struct ahci_probe_ent { +#ifdef CONFIG_DM_PCI

•   struct udevice *dev;
  

+#else pci_dev_t dev; +#endif struct ahci_ioports port[AHCI_MAX_PORTS]; u32 n_ports; u32 hard_port_no; --

Regards, Bin

On Tuesday, November 17, 2015 at 04:53:59 AM, Simon Glass wrote:

Convert this driver to use the new driver model PCI API.

Nit, in the subject, it should be ehci, not echi ;-)

Signed-off-by: Simon Glass sjg@chromium.org

drivers/usb/host/ehci-pci.c | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-)

[...]

@@ -38,12 +40,12 @@ static void ehci_pci_common_init(pci_dev_t pdev, struct ehci_hccr **ret_hccr, *ret_hcor = hcor;

/* enable busmaster */

• pci_read_config_dword(pdev, PCI_COMMAND, &cmd);

• dm_pci_read_config32(dev, PCI_COMMAND, &cmd);

Is the dm_ prefix really needed on these functions ?

cmd |= PCI_COMMAND_MASTER;

• pci_write_config_dword(pdev, PCI_COMMAND, cmd);

• dm_pci_write_config32(dev, PCI_COMMAND, cmd);

}

-#ifndef CONFIG_DM_USB +#else

#ifdef CONFIG_PCI_EHCI_DEVICE static struct pci_device_id ehci_pci_ids[] = { @@ -55,6 +57,31 @@ static struct pci_device_id ehci_pci_ids[] = { }; #endif

+static void ehci_pci_legacy_init(pci_dev_t pdev, struct ehci_hccr **ret_hccr, + struct ehci_hcor **ret_hcor) +{

• struct ehci_hccr *hccr;
• struct ehci_hcor *hcor;
• uint32_t cmd;
• hccr = (struct ehci_hccr *)pci_map_bar(pdev,

• 	PCI_BASE_ADDRESS_0, PCI_REGION_MEM);
  

• hcor = (struct ehci_hcor *)((uint32_t) hccr +

This should be uintptr_t instead of uint32_t . Oh, and uint32_t should not be used, use u32 where applicable.

• 	HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
  

• debug("EHCI-PCI init hccr 0x%x and hcor 0x%x hc_length %d\n",

•      (uint32_t)hccr, (uint32_t)hcor,
  

•      (uint32_t)HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
  

• *ret_hccr = hccr;
• *ret_hcor = hcor;
• /* enable busmaster */
• pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
• cmd |= PCI_COMMAND_MASTER;
• pci_write_config_dword(pdev, PCI_COMMAND, cmd);

+}

[...]

Best regards, Marek Vasut

Hi Bin,

On 17 November 2015 at 21:16, Bin Meng bmeng.cn@gmail.com wrote:

Hi Simon,

On Tue, Nov 17, 2015 at 11:54 AM, Simon Glass sjg@chromium.org wrote:

...

At present pci_mmc_init() does not correctly use the PCI function since the list it passes is not terminated. The array size passed to pci_mmc_init() is actually not used correctly. Fix this and adjust the pci_mmc_init() to scan all available MMC devices.

Adjust this code to use the new driver model PCI API.

This should move over to the new MMC uclass at some point.

Signed-off-by: Simon Glass sjg@chromium.org

arch/x86/cpu/baytrail/valleyview.c | 4 ++-- arch/x86/cpu/quark/quark.c | 4 ++-- arch/x86/cpu/queensbay/topcliff.c | 4 ++-- drivers/mmc/pci_mmc.c | 15 +++++++-------- include/mmc.h | 6 ++---- 5 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/arch/x86/cpu/baytrail/valleyview.c b/arch/x86/cpu/baytrail/valleyview.c index a009c14..9790c6e 100644 --- a/arch/x86/cpu/baytrail/valleyview.c +++ b/arch/x86/cpu/baytrail/valleyview.c @@ -14,12 +14,12 @@ static struct pci_device_id mmc_supported[] = { { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VALLEYVIEW_SDIO }, { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VALLEYVIEW_SDCARD },

•   {},
  

};

int cpu_mmc_init(bd_t *bis) {

•   return pci_mmc_init("ValleyView SDHCI", mmc_supported,
  

•                       ARRAY_SIZE(mmc_supported));
  

•   return pci_mmc_init("ValleyView SDHCI", mmc_supported);
  

}

#ifndef CONFIG_EFI_APP diff --git a/arch/x86/cpu/quark/quark.c b/arch/x86/cpu/quark/quark.c index f737e19..77ae9b8 100644 --- a/arch/x86/cpu/quark/quark.c +++ b/arch/x86/cpu/quark/quark.c @@ -19,6 +19,7 @@

static struct pci_device_id mmc_supported[] = { { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_QRK_SDIO },

•   {},
  

};

/* @@ -340,8 +341,7 @@ int arch_early_init_r(void)

int cpu_mmc_init(bd_t *bis) {

•   return pci_mmc_init("Quark SDHCI", mmc_supported,
  

•                       ARRAY_SIZE(mmc_supported));
  

•   return pci_mmc_init("Quark SDHCI", mmc_supported);
  

}

void cpu_irq_init(void) diff --git a/arch/x86/cpu/queensbay/topcliff.c b/arch/x86/cpu/queensbay/topcliff.c index 9faf1b9..b76dd7d 100644 --- a/arch/x86/cpu/queensbay/topcliff.c +++ b/arch/x86/cpu/queensbay/topcliff.c @@ -11,10 +11,10 @@ static struct pci_device_id mmc_supported[] = { { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_TCF_SDIO_0 }, { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_TCF_SDIO_1 },

•   {},
  

};

int cpu_mmc_init(bd_t *bis) {

•   return pci_mmc_init("Topcliff SDHCI", mmc_supported,
  

•                       ARRAY_SIZE(mmc_supported));
  

•   return pci_mmc_init("Topcliff SDHCI", mmc_supported);
  

} diff --git a/drivers/mmc/pci_mmc.c b/drivers/mmc/pci_mmc.c index 37171bf..5fb7151 100644 --- a/drivers/mmc/pci_mmc.c +++ b/drivers/mmc/pci_mmc.c @@ -11,26 +11,25 @@ #include <sdhci.h> #include <asm/pci.h>

-int pci_mmc_init(const char *name, struct pci_device_id *mmc_supported,

•            int num_ids)
  

+int pci_mmc_init(const char *name, struct pci_device_id *mmc_supported) { struct sdhci_host *mmc_host;

•   pci_dev_t devbusfn;
    u32 iobase;
    int ret;
    int i;
  
  

•   for (i = 0; i < num_ids; i++) {
  

•           devbusfn = pci_find_devices(mmc_supported, i);
  

•           if (devbusfn == -1)
  

•                   return -ENODEV;
  

•   for (i = 0; ; i++) {
  

Use while (1)?

I want to pass the correct value to pci_find_device_id() each time.

...

•           struct udevice *dev;
  
  

•           ret = pci_find_device_id(mmc_supported, i, &dev);
  

•           if (ret)
  

•                   return ret;
            mmc_host = malloc(sizeof(struct sdhci_host));
            if (!mmc_host)
                    return -ENOMEM;
  
            mmc_host->name = (char *)name;
  

•           pci_read_config_dword(devbusfn, PCI_BASE_ADDRESS_0, &iobase);
  

•           dm_pci_read_config32(dev, PCI_BASE_ADDRESS_0, &iobase);
            mmc_host->ioaddr = (void *)iobase;
            mmc_host->quirks = 0;
            ret = add_sdhci(mmc_host, 0, 0);
  

diff --git a/include/mmc.h b/include/mmc.h index cda9a19..a844cf5 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -484,11 +484,9 @@ struct pci_device_id;

• This finds all the matching PCI IDs and sets them up as MMC devices.
• @name: Name to use for devices

• • @mmc_supported: PCI IDs to search for
• • @num_ids: Number of elements in @mmc_supported

• • @mmc_supported: PCI IDs to search for, terminated by {0, 0}
  */

-int pci_mmc_init(const char *name, struct pci_device_id *mmc_supported,

•            int num_ids);
  

+int pci_mmc_init(const char *name, struct pci_device_id *mmc_supported);

/* Set block count limit because of 16 bit register limit on some hardware*/

#ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT

Regards, Bin

Regards, Simon

On Tue, Nov 17, 2015 at 11:54 AM, Simon Glass sjg@chromium.org wrote:

The current comments are confusing. We don't actually bind a generic device when the device tree has no information. We try to scan available PCI drivers. Update the comments to reflect this.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/pci/pci-uclass.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 9c54e6c..6374e96 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -672,9 +672,7 @@ int pci_bind_bus_devices(struct udevice *bus) /* Find this device in the device tree */ ret = pci_bus_find_devfn(bus, PCI_MASK_BUS(bdf), &dev);

•           /* Search for a driver */
  

•           /* If nothing in the device tree, bind a generic device */
  

•           /* If nothing in the device tree, bind a device */
            if (ret == -ENODEV) {
                    struct pci_device_id find_id;
                    ulong val;
  

--

Reviewed-by: Bin Meng bmeng.cn@gmail.com