Re: [U-Boot] [PATCH 3/9] fdt: Support providing IORESOURCE_* flags from translation

Hi Paul,

On 29 January 2016 at 06:54, Paul Burton paul.burton@imgtec.com wrote:

Support providing flags indicating the type of resource that a translated address corresponds to. This will allow for callers to look out for IORESOURCE_IO to use I/O accessors instead of always assuming simple memory-mapped addresses.

Signed-off-by: Paul Burton paul.burton@imgtec.com

common/fdt_support.c | 32 ++++++++++++++++++++++++++++++-- drivers/core/device.c | 23 ++++++++++++++++++++--- include/dm/device.h | 23 +++++++++++++++++++++++ include/fdt_support.h | 2 ++ 4 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c index 0aba77d..d0c9d56 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -11,6 +11,7 @@ #include <inttypes.h> #include <stdio_dev.h> #include <linux/ctype.h> +#include <linux/ioport.h> #include <linux/types.h> #include <asm/global_data.h> #include <libfdt.h> @@ -965,6 +966,7 @@ struct of_bus { u64 (*map)(fdt32_t *addr, const fdt32_t *range, int na, int ns, int pna); int (*translate)(fdt32_t *addr, u64 offset, int na);

•   unsigned int    (*get_flags)(const fdt32_t *addr);
  

Please add a function comment here.

};

/* Default translator (generic bus) */ @@ -1063,6 +1065,19 @@ static int of_bus_isa_translate(fdt32_t *addr, u64 offset, int na) return of_bus_default_translate(addr + 1, offset, na - 1); }

+static unsigned int of_bus_isa_get_flags(const fdt32_t *addr) +{

•   unsigned int flags = 0;
  

•   u32 w = be32_to_cpup(addr);
  

•   if (w & 1)
  

•           flags |= IORESOURCE_IO;
  

•   else
  

•           flags |= IORESOURCE_MEM;
  

•   return flags;
  

+}

#endif /* CONFIG_OF_ISA_BUS */

/* Array of bus specific translators */ @@ -1076,6 +1091,7 @@ static struct of_bus of_busses[] = { .count_cells = of_bus_isa_count_cells, .map = of_bus_isa_map, .translate = of_bus_isa_translate,

•           .get_flags = of_bus_isa_get_flags,
    },
  

#endif /* CONFIG_OF_ISA_BUS */ /* Default */ @@ -1168,7 +1184,7 @@ static int of_translate_one(void * blob, int parent, struct of_bus *bus,

• that way, but this is traditionally the way IBM at least do things

*/ static u64 __of_translate_address(void *blob, int node_offset, const fdt32_t *in_addr,

•                             const char *rprop)
  

•                             const char *rprop, unsigned int *flags)
  

{ int parent; struct of_bus *bus, *pbus; @@ -1198,6 +1214,11 @@ static u64 __of_translate_address(void *blob, int node_offset, const fdt32_t *in bus->name, na, ns, fdt_get_name(blob, parent, NULL)); of_dump_addr("OF: translating address:", addr, na);

•   if (flags && bus->get_flags)
  

•           *flags = bus->get_flags(in_addr);
  

•   else if (flags)
  

•           *flags = IORESOURCE_MEM;
  

•   /* Translate */
    for (;;) {
            /* Switch to parent bus */
  

@@ -1240,9 +1261,16 @@ static u64 __of_translate_address(void *blob, int node_offset, const fdt32_t *in return result; }

+u64 fdt_translate_address_flags(void *blob, int node_offset,

•                           const fdt32_t *in_addr, unsigned int *flags)
  

+{

•   return __of_translate_address(blob, node_offset, in_addr, "ranges",
  

•                                 flags);
  

+}

u64 fdt_translate_address(void *blob, int node_offset, const fdt32_t *in_addr) {

•   return __of_translate_address(blob, node_offset, in_addr, "ranges");
  

•   return fdt_translate_address_flags(blob, node_offset, in_addr, NULL);
  

}

/** diff --git a/drivers/core/device.c b/drivers/core/device.c index f5def35..0492dd7 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -590,7 +590,8 @@ const char *dev_get_uclass_name(struct udevice *dev) return dev->uclass->uc_drv->name; }

-fdt_addr_t dev_get_addr_index(struct udevice *dev, int index) +fdt_addr_t dev_get_addr_index_flags(struct udevice *dev, int index,

•                               unsigned int *flags)
  

{ #if CONFIG_IS_ENABLED(OF_CONTROL) fdt_addr_t addr; @@ -624,8 +625,8 @@ fdt_addr_t dev_get_addr_index(struct udevice *dev, int index) * Use the full-fledged translate function for complex * bus setups. */

•           addr = fdt_translate_address((void *)gd->fdt_blob,
  

•                                        dev->of_offset, reg);
  

•           addr = fdt_translate_address_flags((void *)gd->fdt_blob,
  

•                                              dev->of_offset, reg, flags);
    } else {
            /*
             * Use the "simple" translate function for less complex
  

@@ -640,6 +641,9 @@ fdt_addr_t dev_get_addr_index(struct udevice *dev, int index) UCLASS_SIMPLE_BUS) addr = simple_bus_translate(dev->parent, addr); }

•           if (flags)
  

•                   *flags = 0;
    }
  
    /*
  

@@ -652,10 +656,23 @@ fdt_addr_t dev_get_addr_index(struct udevice *dev, int index)

    return addr;

#else

•   if (flags)
  

•           *flags = 0;
  

•   return FDT_ADDR_T_NONE;
  

#endif }

+fdt_addr_t dev_get_addr_flags(struct udevice *dev, unsigned int *flags) +{

•   return dev_get_addr_index_flags(dev, 0, flags);
  

+}

+fdt_addr_t dev_get_addr_index(struct udevice *dev, int index) +{

•   return dev_get_addr_index_flags(dev, index, NULL);
  

+}

fdt_addr_t dev_get_addr(struct udevice *dev) { return dev_get_addr_index(dev, 0); diff --git a/include/dm/device.h b/include/dm/device.h index 1cf8150..b5dd62c 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -454,6 +454,16 @@ int device_find_next_child(struct udevice **devp); fdt_addr_t dev_get_addr(struct udevice *dev);

/**

• • dev_get_addr_flags() - Get the reg property of a device
• • @dev: Pointer to a device
• • @flags: Pointer to flags, if non-NULL will contain IORESOURCE_*

Meaning what? Can you expand on this a bit in the comment? Where do the flags come from?

• • @return addr
• */

+fdt_addr_t dev_get_addr_flags(struct udevice *dev, unsigned int *flags);

+/**

• dev_get_addr_index() - Get the indexed reg property of a device
• @dev: Pointer to a device

@@ -465,6 +475,19 @@ fdt_addr_t dev_get_addr(struct udevice *dev); fdt_addr_t dev_get_addr_index(struct udevice *dev, int index);

/**

• • dev_get_addr_index_flags() - Get the indexed reg property of a device
• • @dev: Pointer to a device
• • @index: the 'reg' property can hold a list of <addr, size> pairs

• •    and @index is used to select which one is required
    

• • @flags: Pointer to flags, if non-NULL will contain IORESOURCE_*
• • @return addr
• */

+fdt_addr_t dev_get_addr_index_flags(struct udevice *dev, int index,

•                               unsigned int *flags);
  

+/**

• device_has_children() - check if a device has any children
• @dev: Device to check

diff --git a/include/fdt_support.h b/include/fdt_support.h index 296add0..b716f3e 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -175,6 +175,8 @@ int fdt_fixup_nor_flash_size(void *blob); void fdt_fixup_mtdparts(void *fdt, void *node_info, int node_info_size); void fdt_del_node_and_alias(void *blob, const char *alias); u64 fdt_translate_address(void *blob, int node_offset, const __be32 *in_addr); +u64 fdt_translate_address_flags(void *blob, int node_offset,

•                           const fdt32_t *in_addr, unsigned int *flags);
  

Please add a full function comment here. Bonus points if you do the one immediately above also.

int fdt_node_offset_by_compat_reg(void *blob, const char *compat, phys_addr_t compat_off); int fdt_alloc_phandle(void *blob); -- 2.7.0

Regards, Simon