[U-Boot] [PATCH 01/27] Provide a generic io.h & address mapping functions

Simon Glass sjg at chromium.org
Mon Oct 3 23:49:33 CEST 2016


Hi Paul,

On 1 October 2016 at 08:19, Paul Burton <paul.burton at imgtec.com> wrote:
> Most architectures currently supported by U-Boot use trivial
> implementations of map_to_physmem & virt_to_phys which simply cast a
> physical address to a pointer for use a virtual address & vice-versa.
> This results in a lot of duplicate implementations of these mapping
> functions.
>
> The functions provided by different architectures also differs, with
> some having implementations of phys_to_virt & others not. A later patch
> in this series will make use of phys_to_virt, so requires that it be
> provided for all architectures.
>
> This patch introduces an asm-generic/io.h which provides generic
> implementations of address mapping functions, allowing the duplication
> of them between architectures to be removed. Once architectures are
> converted to make use of this generic header it will also ensure that
> all of phys_to_virt, virt_to_phys, map_physmem & unmap_physmem are
> provided. The 2 families of functions differ in that map_physmem may
> create dynamic mappings whilst phys_to_virt may not & therefore is more
> limited in scope but doesn't require information such as a length &
> flags.
>
> This patch doesn't convert any architectures to make use of this generic
> header - later patches in the series will do so.
>
> Signed-off-by: Paul Burton <paul.burton at imgtec.com>
> Cc: Albert Aribaud <albert.u.boot at aribaud.net>
> Cc: Alexey Brodkin <alexey.brodkin at synopsys.com>
> Cc: Alison Wang <alison.wang at freescale.com>
> Cc: Angelo Dureghello <angelo at sysam.it>
> Cc: Bin Meng <bmeng.cn at gmail.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
> Cc: Francois Retief <fgretief at spaceteq.co.za>
> Cc: Macpaul Lin <macpaul at andestech.com>
> Cc: Michal Simek <monstr at monstr.eu>
> Cc: Mike Frysinger <vapier at gentoo.org>
> Cc: Nobuhiro Iwamatsu <iwamatsu at nigauri.org>
> Cc: Scott McNutt <smcnutt at psyent.com>
> Cc: Sonic Zhang <sonic.adi at gmail.com>
> Cc: Thomas Chou <thomas at wytron.com.tw>
> Cc: Wolfgang Denk <wd at denx.de>
> ---
>
>  include/asm-generic/io.h | 110 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 110 insertions(+)
>  create mode 100644 include/asm-generic/io.h

Reviewed-by: Simon Glass <sjg at chromium.org>

Question and nits below.

>
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> new file mode 100644
> index 0000000..dd3a46d
> --- /dev/null
> +++ b/include/asm-generic/io.h
> @@ -0,0 +1,110 @@
> +/*
> + * Generic I/O functions.
> + *
> + * Copyright (c) 2016 Imagination Technologies Ltd.
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef __ASM_GENERIC_IO_H__
> +#define __ASM_GENERIC_IO_H__
> +
> +/*
> + * This file should be included at the end of each architecture-specific
> + * asm/io.h such that we may provide generic implementations without
> + * conflicting with architecture-specific code.
> + */
> +
> +#ifndef __ASSEMBLY__
> +
> +/**
> + * phys_to_virt() - Return a virtual address mapped to a given physical address
> + * @paddr: the physical address
> + *

@return

> + * Returns a virtual address which the CPU can access that maps to the physical
> + * address @paddr. This should only be used where it is known that no dynamic
> + * mapping is required. In general, map_physmem should be used instead.
> + *
> + * Returns: a virtual address which maps to @paddr

Why two Returns?

> + */
> +#ifndef phys_to_virt
> +static inline void *phys_to_virt(phys_addr_t paddr)
> +{
> +       return (void *)(unsigned long)paddr;
> +}
> +#endif
> +
> +/**
> + * virt_to_phys() - Return the physical address that a virtual address maps to
> + * @vaddr: the virtual address
> + *

@return

> + * Returns the physical address which the CPU-accessible virtual address @vaddr
> + * maps to.
> + *
> + * Returns: the physical address which @vaddr maps to

Why two Returns?

> + */
> +#ifndef virt_to_phys
> +static inline phys_addr_t virt_to_phys(void *vaddr)
> +{
> +       return (phys_addr_t)((unsigned long)vaddr);
> +}
> +#endif
> +
> +/*
> + * Flags for use with map_physmem() & unmap_physmem(). Architectures need not
> + * support all of these, in which case they will be defined as zero here &
> + * ignored. Callers that may run on multiple architectures should therefore
> + * treat them as hints rather than requirements.
> + */
> +#ifndef MAP_NOCACHE
> +# define MAP_NOCACHE   0       /* Produce an uncached mapping */
> +#endif
> +#ifndef MAP_WRCOMBINE
> +# define MAP_WRCOMBINE 0       /* Allow write-combining on the mapping */
> +#endif
> +#ifndef MAP_WRBACK
> +# define MAP_WRBACK    0       /* Map using write-back caching */
> +#endif
> +#ifndef MAP_WRTHROUGH
> +# define MAP_WRTHROUGH 0       /* Map using write-through caching */
> +#endif

It seems odd to make these 0 when not supported. How could an arch
know that it was requested, and then complain when an unsupported flag
is passed?

> +
> +/**
> + * map_physmem() - Return a virtual address mapped to a given physical address
> + * @paddr: the physical address
> + * @len: the length of the required mapping
> + * @flags: flags affecting the type of mapping

@return

> + *
> + * Return a virtual address through which the CPU may access the memory at
> + * physical address @paddr. The mapping will be valid for at least @len bytes,
> + * and may be affected by flags passed to the @flags argument. This function
> + * may create new mappings, so should generally be paired with a matching call
> + * to unmap_physmem once the caller is finished with the memory in question.
> + *

@return

> + * Returns: a virtual address suitably mapped to @paddr
> + */
> +#ifndef map_physmem
> +static inline void *
> +map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)

Can you join those two lines and wrap the args if needed??

> +{
> +       return phys_to_virt(paddr);
> +}
> +#endif
> +
> +/**
> + * unmap_physmem() - Remove mappings created by a prior call to map_physmem()
> + * @vaddr: the virtual address which map_physmem() previously returned
> + * @flags: flags matching those originally passed to map_physmem()
> + *
> + * Unmap memory which was previously mapped by a call to map_physmem(). If
> + * map_physmem() dynamically created a mapping for the memory in question then
> + * unmap_physmem() will remove that mapping.
> + */
> +#ifndef unmap_physmem
> +static inline void unmap_physmem(void *vaddr, unsigned long flags)
> +{
> +}
> +#endif
> +
> +#endif /* !__ASSEMBLY__ */
> +#endif /* __ASM_GENERIC_IO_H__ */
> --
> 2.10.0
>
REgards,
Simon


More information about the U-Boot mailing list