Re: [U-Boot] [PATCH 033/126] dm: pci: Add a function to read a PCI BAR

On Sat, Oct 5, 2019 at 9:12 PM Bin Meng bmeng.cn@gmail.com wrote:

On Wed, Sep 25, 2019 at 10:58 PM Simon Glass sjg@chromium.org wrote:

...

At present PCI address transaction is not supported so drivers must manually read the correct BAR after reading the device tree info. The ns16550 has a suitable implementation, so move this code into the core DM support.

Note that there is no live-tree equivalent at present.

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

arch/sandbox/dts/test.dts | 7 ++++-- drivers/core/fdtaddr.c | 33 ++++++++++++++++++++++++++++ drivers/core/read.c | 11 ++++++++++ drivers/serial/ns16550.c | 31 +-------------------------- include/dm/fdtaddr.h | 8 +++++++ include/dm/read.h | 25 ++++++++++++++++++++++ test/dm/pci.c | 45 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 128 insertions(+), 32 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index a2e75981f0b..8733e0d7e19 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -456,12 +456,15 @@ }; pci@1,0 { compatible = "pci-generic";

•                   reg = <0x0800 0 0 0 0>;
  

•                   /* BAR0 is at 0x14, using FDT_PCI_SPACE_MEM32 */
  

reg 0

...

•                   reg = <0x02000814 0 0 0 0
  

•                           0x0100f810 0 0 0 0>;
  

it should be 0x01000810, and should have a similar comment like reg 0

...

                    sandbox,emul = <&swap_case_emul1>;
            };
            pci@1f,0 {
                    compatible = "pci-generic";

•                   reg = <0xf800 0 0 0 0>;
  

•                   /* BAR1 is at 0x10, using FDT_PCI_SPACE_IO */
  

reg 0

Corrected the above comments, and

...

•                   reg = <0x0100f810 0 0 0 0>;
                    sandbox,emul = <&swap_case_emul1f>;
            };
    };
  

diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c index 6850003a287..c9a941116a3 100644 --- a/drivers/core/fdtaddr.c +++ b/drivers/core/fdtaddr.c @@ -190,3 +190,36 @@ void *devfdt_map_physmem(struct udevice *dev, unsigned long size)

    return map_physmem(addr, size, MAP_NOCACHE);

}

+fdt_addr_t devfdt_get_addr_pci(struct udevice *dev) +{

•   ulong addr;
  

•   addr = devfdt_get_addr(dev);
  

•   if (CONFIG_IS_ENABLED(PCI) && IS_ENABLED(CONFIG_DM_PCI) &&
  

•       addr == FDT_ADDR_T_NONE) {
  

•           struct fdt_pci_addr pci_addr;
  

•           u32 bar;
  

•           int ret;
  

•           ret = fdtdec_get_pci_addr(gd->fdt_blob,
  

•                                     dev_of_offset(dev),
  

•                                     FDT_PCI_SPACE_MEM32, "reg",
  

•                                     &pci_addr);
  

•           if (ret) {
  

•                   /* try if there is any i/o-mapped register */
  

•                   ret = fdtdec_get_pci_addr(gd->fdt_blob,
  

•                                             dev_of_offset(dev),
  

•                                             FDT_PCI_SPACE_IO, "reg",
  

•                                             &pci_addr);
  

•                   if (ret)
  

•                           return FDT_ADDR_T_NONE;
  

•           }
  

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

•           if (ret)
  

•                   return FDT_ADDR_T_NONE;
  

•           addr = bar;
  

•   }
  

•   return addr;
  

+} diff --git a/drivers/core/read.c b/drivers/core/read.c index fb3dcd9a790..9602e52d1b1 100644 --- a/drivers/core/read.c +++ b/drivers/core/read.c @@ -307,3 +307,14 @@ int dev_read_alias_highest_id(const char *stem)

    return fdtdec_get_alias_highest_id(gd->fdt_blob, stem);

}

+fdt_addr_t dev_read_addr_pci(struct udevice *dev) +{

•   ulong addr;
  

•   addr = dev_read_addr(dev);
  

•   if (addr == FDT_ADDR_T_NONE && !of_live_active())
  

•           addr = devfdt_get_addr_pci(dev);
  

•   return addr;
  

+} diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 01f334938ea..754b6e99215 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -440,36 +440,7 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev) int err;

    /* try Processor Local Bus device first */

•   addr = dev_read_addr(dev);
  

-#if CONFIG_IS_ENABLED(PCI) && defined(CONFIG_DM_PCI)

•   if (addr == FDT_ADDR_T_NONE) {
  

•           /* then try pci device */
  

•           struct fdt_pci_addr pci_addr;
  

•           u32 bar;
  

•           int ret;
  

•           /* we prefer to use a memory-mapped register */
  

•           ret = fdtdec_get_pci_addr(gd->fdt_blob, dev_of_offset(dev),
  

•                                     FDT_PCI_SPACE_MEM32, "reg",
  

•                                     &pci_addr);
  

•           if (ret) {
  

•                   /* try if there is any i/o-mapped register */
  

•                   ret = fdtdec_get_pci_addr(gd->fdt_blob,
  

•                                             dev_of_offset(dev),
  

•                                             FDT_PCI_SPACE_IO,
  

•                                             "reg", &pci_addr);
  

•                   if (ret)
  

•                           return ret;
  

•           }
  

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

•           if (ret)
  

•                   return ret;
  

•           addr = bar;
  

•   }
  

-#endif

•   addr = dev_read_addr_pci(dev);
    if (addr == FDT_ADDR_T_NONE)
            return -EINVAL;
  
  

diff --git a/include/dm/fdtaddr.h b/include/dm/fdtaddr.h index 57b326cb336..959d3bc2d69 100644 --- a/include/dm/fdtaddr.h +++ b/include/dm/fdtaddr.h @@ -138,4 +138,12 @@ fdt_addr_t devfdt_get_addr_name(struct udevice *dev, const char *name); fdt_addr_t devfdt_get_addr_size_name(struct udevice *dev, const char *name, fdt_size_t *size);

+/**

• • devfdt_get_addr_pci() - Read an address and handle PCI address translation
• • @dev: Device to read from
• • @return address or FDT_ADDR_T_NONE if not found
• */

+fdt_addr_t devfdt_get_addr_pci(struct udevice *dev);

#endif diff --git a/include/dm/read.h b/include/dm/read.h index 03189838ff7..4f02d07d001 100644 --- a/include/dm/read.h +++ b/include/dm/read.h @@ -247,6 +247,26 @@ fdt_addr_t dev_read_addr(struct udevice *dev); */ void *dev_read_addr_ptr(struct udevice *dev);

+/**

• • dev_read_addr_pci() - Read an address and handle PCI address translation
• • At present U-Boot does not have address translation logic for PCI in the
• • livetree implementation (of_addr.c). This special function supports this for
• • the flat tree implementation.
• • This function should be removed (and code should use dev_read() instead)
• • once:
• • 1. PCI address translation is added; and either
• • 2. everything uses livetree where PCI translation is used (which is feasible
• • in SPL and U-Boot proper) or PCI address translation is added to
• • fdtdec_get_addr() and friends.
• • @dev: Device to read from
• • @return address or FDT_ADDR_T_NONE if not found
• */

+fdt_addr_t dev_read_addr_pci(struct udevice *dev);

/**

• dev_remap_addr() - Get the reg property of a device as a

•                     memory-mapped I/O pointer
  

@@ -690,6 +710,11 @@ static inline void *dev_read_addr_ptr(struct udevice *dev) return devfdt_get_addr_ptr(dev); }

+static inline fdt_addr_t dev_read_addr_pci(struct udevice *dev) +{

•   return devfdt_get_addr_pci(dev);
  

+}

static inline void *dev_remap_addr(struct udevice *dev) { return devfdt_remap_addr(dev); diff --git a/test/dm/pci.c b/test/dm/pci.c index e70b65aea4a..fb93e4c78ae 100644 --- a/test/dm/pci.c +++ b/test/dm/pci.c @@ -294,3 +294,48 @@ static int dm_test_pci_ea(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_pci_ea, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);

+/* Test the dev_read_addr_pci() function */ +static int dm_test_pci_addr_flat(struct unit_test_state *uts) +{

•   struct udevice *swap1f, *swap1;
  

•   ulong io_addr, mem_addr;
  

•   ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(0, 0x1f, 0), &swap1f));
  

•   io_addr = dm_pci_read_bar32(swap1f, 0);
  

•   ut_asserteq(io_addr, dev_read_addr_pci(swap1f));
  

•   /*
  

•    * This device has both I/O and MEM spaces but the MEM space appears
  

•    * first
  

•    */
  

•   ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(0, 0x1, 0), &swap1));
  

•   mem_addr = dm_pci_read_bar32(swap1, 1);
  

•   ut_asserteq(mem_addr, dev_read_addr_pci(swap1));
  

•   return 0;
  

+} +DM_TEST(dm_test_pci_addr_flat, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT |

•           DM_TESTF_FLAT_TREE);
  

+/*

• • Test the dev_read_addr_pci() function with livetree. That function is
• • not currently fully implemented, in that it fails to return the BAR address.
• • Once that is implemented this test can be removed and dm_test_pci_addr_flat()
• • can be used for both flattree and livetree by removing the DM_TESTF_FLAT_TREE
• • flag above.
• */

+static int dm_test_pci_addr_live(struct unit_test_state *uts) +{

•   struct udevice *swap1f, *swap1;
  

•   ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(0, 0x1f, 0), &swap1f));
  

•   ut_asserteq(FDT_ADDR_T_NONE, dev_read_addr_pci(swap1f));
  

•   ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(0, 0x1, 0), &swap1));
  

•   ut_asserteq(FDT_ADDR_T_NONE, dev_read_addr_pci(swap1));
  

•   return 0;
  

+} +DM_TEST(dm_test_pci_addr_live, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT |

•           DM_TESTF_LIVE_TREE);
  

--

Other than above, Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com

applied to u-boot-x86/next, thanks!